28.3.2

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

rclcpp/CHANGELOG.rst

@@ -2,6 +2,64 @@
 Changelog for package rclcpp
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+28.3.2 (2024-07-24)
+-------------------
+* Updated rcpputils path API (`#2579 <https://github.com/ros2/rclcpp/issues/2579>`_)
+* Make the subscriber_triggered_to_receive_message test more reliable. (`#2584 <https://github.com/ros2/rclcpp/issues/2584>`_)
+  * Make the subscriber_triggered_to_receive_message test more reliable.
+  In the current code, inside of the timer we create the subscription
+  and the publisher, publish immediately, and expect the subscription
+  to get it immediately.  But it may be the case that discovery
+  hasn't even happened between the publisher and the subscription
+  by the time the publish call happens.
+  To make this more reliable, create the subscription and publish *before*
+  we ever create and spin on the timer.  This at least gives 100
+  milliseconds for discovery to happen.  That may not be quite enough
+  to make this reliable on all platforms, but in my local testing this
+  helps a lot.  Prior to this change I can make this fail one out of 10
+  times, and after the change I've run 100 times with no failures.
+* Have the EventsExecutor use more common code  (`#2570 <https://github.com/ros2/rclcpp/issues/2570>`_)
+  * move notify waitable setup to its own function
+  * move mutex lock to retrieve_entity utility
+  * use entities_need_rebuild\_ atomic bool in events-executors
+  * remove duplicated set_on_ready_callback for notify_waitable
+  * use mutex from base class rather than a new recursive mutex
+  * use current_collection\_ member in events-executor
+  * delay adding notify waitable to collection
+  * postpone clearing the current collection
+  * commonize notify waitable and collection
+  * commonize add/remove node/cbg methods
+  * fix linter errors
+  ---------
+* Removed deprecated methods and classes (`#2575 <https://github.com/ros2/rclcpp/issues/2575>`_)
+* Release ownership of entities after spinning cancelled (`#2556 <https://github.com/ros2/rclcpp/issues/2556>`_)
+  * Release ownership of entities after spinning cancelled
+  * Move release action to every exit point in different spin functions
+  * Move wait_result\_.reset() before setting spinning to false
+  * Update test code
+  * Move test code to test_executors.cpp
+  ---------
+* Split test_executors.cpp even further. (`#2572 <https://github.com/ros2/rclcpp/issues/2572>`_)
+  That's because it is too large for Windows Debug to compile,
+  so split into smaller bits.
+  Even with this split, the file is too big; that's likely
+  because we are using TYPED_TEST here, which generates multiple
+  symbols per test case.  To deal with this, without further
+  breaking up the file, also add in the /bigobj flag when
+  compiling on Windows Debug.
+* avoid adding notify waitable twice to events-executor collection (`#2564 <https://github.com/ros2/rclcpp/issues/2564>`_)
+  * avoid adding notify waitable twice to events-executor entities collection
+  * remove redundant mutex lock
+  ---------
+* Contributors: Alberto Soragna, Alejandro Hernández Cordero, Barry Xu, Chris Lalancette
+
+28.3.1 (2024-06-25)
+-------------------
+* Remove unnecessary msg includes in tests (`#2566 <https://github.com/ros2/rclcpp/issues/2566>`_)
+* Fix copy-paste errors in function docs (`#2565 <https://github.com/ros2/rclcpp/issues/2565>`_)
+* Fix typo in function doc (`#2563 <https://github.com/ros2/rclcpp/issues/2563>`_)
+* Contributors: Christophe Bedard
+
 28.3.0 (2024-06-17)
 -------------------
 * Add test creating two content filter topics with the same topic name (`#2546 <https://github.com/ros2/rclcpp/issues/2546>`_) (`#2549 <https://github.com/ros2/rclcpp/issues/2549>`_)

rclcpp/include/rclcpp/callback_group.hpp

@@ -59,46 +59,6 @@ class CallbackGroup
 public:
   RCLCPP_SMART_PTR_DEFINITIONS(CallbackGroup)
 
-  /// Constructor for CallbackGroup.
-  /**
-   * Callback Groups have a type, either 'Mutually Exclusive' or 'Reentrant'
-   * and when creating one the type must be specified.
-   *
-   * Callbacks in Reentrant Callback Groups must be able to:
-   *   - run at the same time as themselves (reentrant)
-   *   - run at the same time as other callbacks in their group
-   *   - run at the same time as other callbacks in other groups
-   *
-   * Callbacks in Mutually Exclusive Callback Groups:
-   *   - will not be run multiple times simultaneously (non-reentrant)
-   *   - will not be run at the same time as other callbacks in their group
-   *   - but must run at the same time as callbacks in other groups
-   *
-   * Additionally, callback groups have a property which determines whether or
-   * not they are added to an executor with their associated node automatically.
-   * When creating a callback group the automatically_add_to_executor_with_node
-   * argument determines this behavior, and if true it will cause the newly
-   * created callback group to be added to an executor with the node when the
-   * Executor::add_node method is used.
-   * If false, this callback group will not be added automatically and would
-   * have to be added to an executor manually using the
-   * Executor::add_callback_group method.
-   *
-   * Whether the node was added to the executor before creating the callback
-   * group, or after, is irrelevant; the callback group will be automatically
-   * added to the executor in either case.
-   *
-   * \param[in] group_type The type of the callback group.
-   * \param[in] automatically_add_to_executor_with_node A boolean that
-   *   determines whether a callback group is automatically added to an executor
-   *   with the node with which it is associated.
-   */
-  [[deprecated("Use CallbackGroup constructor with context function argument")]]
-  RCLCPP_PUBLIC
-  explicit CallbackGroup(
-    CallbackGroupType group_type,
-    bool automatically_add_to_executor_with_node = true);
-
   /// Constructor for CallbackGroup.
   /**
    * Callback Groups have a type, either 'Mutually Exclusive' or 'Reentrant'

rclcpp/include/rclcpp/client.hpp

@@ -70,14 +70,6 @@ struct FutureAndRequestId
   /// Allow implicit conversions to `std::future` by reference.
   operator FutureT &() {return this->future;}
 
-  /// Deprecated, use the `future` member variable instead.
-  /**
-   * Allow implicit conversions to `std::future` by value.
-   * \deprecated
-   */
-  [[deprecated("FutureAndRequestId: use .future instead of an implicit conversion")]]
-  operator FutureT() {return this->future;}
-
   // delegate future like methods in the std::future impl_
 
   /// See std::future::get().
@@ -436,15 +428,6 @@ public:
   {
     using detail::FutureAndRequestId<std::future<SharedResponse>>::FutureAndRequestId;
 
-    /// Deprecated, use `.future.share()` instead.
-    /**
-     * Allow implicit conversions to `std::shared_future` by value.
-     * \deprecated
-     */
-    [[deprecated(
-      "FutureAndRequestId: use .future.share() instead of an implicit conversion")]]
-    operator SharedFuture() {return this->future.share();}
-
     // delegate future like methods in the std::future impl_
 
     /// See std::future::share().
@@ -490,7 +473,7 @@ public:
    * \param[in] node_base NodeBaseInterface pointer that is used in part of the setup.
    * \param[in] node_graph The node graph interface of the corresponding node.
    * \param[in] service_name Name of the topic to publish to.
-   * \param[in] client_options options for the subscription.
+   * \param[in] client_options options for the client.
    */
   Client(
     rclcpp::node_interfaces::NodeBaseInterface * node_base,

rclcpp/include/rclcpp/clock.hpp

@@ -217,13 +217,13 @@ public:
   std::mutex &
   get_clock_mutex() noexcept;
 
-  // Add a callback to invoke if the jump threshold is exceeded.
+  /// Add a callback to invoke if the jump threshold is exceeded.
   /**
    * These callback functions must remain valid as long as the
    * returned shared pointer is valid.
    *
    * Function will register callbacks to the callback queue. On time jump all
-   * callbacks will be executed whose threshold is greater then the time jump;
+   * callbacks will be executed whose threshold is greater than the time jump;
    * The logic will first call selected pre_callbacks and then all selected
    * post_callbacks.
    *
@@ -232,7 +232,7 @@ public:
    * \param pre_callback Must be non-throwing
    * \param post_callback Must be non-throwing.
    * \param threshold Callbacks will be triggered if the time jump is greater
-   * then the threshold.
+   * than the threshold.
    * \throws anything rclcpp::exceptions::throw_from_rcl_error can throw.
    * \throws std::bad_alloc if the allocation of the JumpHandler fails.
    * \warning the instance of the clock must remain valid as long as any created

rclcpp/include/rclcpp/create_client.hpp

@@ -47,28 +47,9 @@ create_client(
   const std::string & service_name,
   const rclcpp::QoS & qos = rclcpp::ServicesQoS(),
   rclcpp::CallbackGroup::SharedPtr group = nullptr)
-{
-  return create_client<ServiceT>(
-    node_base, node_graph, node_services,
-    service_name,
-    qos.get_rmw_qos_profile(),
-    group);
-}
-
-/// Create a service client with a given type.
-/// \internal
-template<typename ServiceT>
-typename rclcpp::Client<ServiceT>::SharedPtr
-create_client(
-  std::shared_ptr<node_interfaces::NodeBaseInterface> node_base,
-  std::shared_ptr<node_interfaces::NodeGraphInterface> node_graph,
-  std::shared_ptr<node_interfaces::NodeServicesInterface> node_services,
-  const std::string & service_name,
-  const rmw_qos_profile_t & qos_profile,
-  rclcpp::CallbackGroup::SharedPtr group)
 {
   rcl_client_options_t options = rcl_client_get_default_options();
-  options.qos = qos_profile;
+  options.qos = qos.get_rmw_qos_profile();
 
   auto cli = rclcpp::Client<ServiceT>::make_shared(
     node_base.get(),
@@ -80,7 +61,6 @@ create_client(
   node_services->add_client(cli_base_ptr, group);
   return cli;
 }
-
 }  // namespace rclcpp
 
 #endif  // RCLCPP__CREATE_CLIENT_HPP_

rclcpp/include/rclcpp/event_handler.hpp

@@ -233,8 +233,6 @@ protected:
   size_t wait_set_event_index_;
 };
 
-using QOSEventHandlerBase [[deprecated("Use rclcpp::EventHandlerBase")]] = EventHandlerBase;
-
 template<typename EventCallbackT, typename ParentHandleT>
 class EventHandler : public EventHandlerBase
 {
@@ -311,11 +309,6 @@ private:
   ParentHandleT parent_handle_;
   EventCallbackT event_callback_;
 };
-
-template<typename EventCallbackT, typename ParentHandleT>
-using QOSEventHandler [[deprecated("Use rclcpp::EventHandler")]] = EventHandler<EventCallbackT,
-    ParentHandleT>;
-
 }  // namespace rclcpp
 
 #endif  // RCLCPP__EVENT_HANDLER_HPP_

rclcpp/include/rclcpp/executor.hpp

@@ -541,8 +541,9 @@ protected:
    *
    * \param[in] notify if true will execute a trigger that will wake up a waiting executor
    */
-  void
-  trigger_entity_recollect(bool notify);
+  RCLCPP_PUBLIC
+  virtual void
+  handle_updated_entities(bool notify);
 
   /// Spinning state, used to prevent multi threaded calls to spin and to cancel blocking spins.
   std::atomic_bool spinning;

rclcpp/include/rclcpp/executors/executor_notify_waitable.hpp

@@ -122,6 +122,14 @@ public:
   void
   clear_on_ready_callback() override;
 
+  /// Set a new callback to be called whenever this waitable is executed.
+  /**
+   * \param[in] on_execute_callback The new callback
+   */
+  RCLCPP_PUBLIC
+  void
+  set_execute_callback(std::function<void(void)> on_execute_callback);
+
   /// Remove a guard condition from being waited on.
   /**
    * \param[in] weak_guard_condition The guard condition to remove.
@@ -142,7 +150,10 @@ private:
   /// Callback to run when waitable executes
   std::function<void(void)> execute_callback_;
 
+  /// Mutex to procetect the guard conditions
   std::mutex guard_condition_mutex_;
+  /// Mutex to protect the execute callback
+  std::mutex execute_mutex_;
 
   std::function<void(size_t)> on_ready_callback_;
 

rclcpp/include/rclcpp/experimental/executors/events_executor/events_executor.hpp

@@ -59,8 +59,6 @@ namespace executors
  */
 class EventsExecutor : public rclcpp::Executor
 {
-  friend class EventsExecutorEntitiesCollector;
-
 public:
   RCLCPP_SMART_PTR_DEFINITIONS(EventsExecutor)
 
@@ -72,7 +70,7 @@ public:
    * \param[in] options Options used to configure the executor.
    */
   RCLCPP_PUBLIC
-  explicit EventsExecutor(
+  EventsExecutor(
     rclcpp::experimental::executors::EventsQueue::UniquePtr events_queue = std::make_unique<
       rclcpp::experimental::executors::SimpleEventsQueue>(),
     bool execute_timers_separate_thread = false,
@@ -128,87 +126,6 @@ public:
   void
   spin_all(std::chrono::nanoseconds max_duration) override;
 
-  /// Add a node to the executor.
-  /**
-   * \sa rclcpp::Executor::add_node
-   */
-  RCLCPP_PUBLIC
-  void
-  add_node(
-    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-    bool notify = true) override;
-
-  /// Convenience function which takes Node and forwards NodeBaseInterface.
-  /**
-   * \sa rclcpp::EventsExecutor::add_node
-   */
-  RCLCPP_PUBLIC
-  void
-  add_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify = true) override;
-
-  /// Remove a node from the executor.
-  /**
-   * \sa rclcpp::Executor::remove_node
-   */
-  RCLCPP_PUBLIC
-  void
-  remove_node(
-    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-    bool notify = true) override;
-
-  /// Convenience function which takes Node and forwards NodeBaseInterface.
-  /**
-   * \sa rclcpp::Executor::remove_node
-   */
-  RCLCPP_PUBLIC
-  void
-  remove_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify = true) override;
-
-  /// Add a callback group to an executor.
-  /**
-   * \sa rclcpp::Executor::add_callback_group
-   */
-  RCLCPP_PUBLIC
-  void
-  add_callback_group(
-    rclcpp::CallbackGroup::SharedPtr group_ptr,
-    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-    bool notify = true) override;
-
-  /// Remove callback group from the executor
-  /**
-   * \sa rclcpp::Executor::remove_callback_group
-   */
-  RCLCPP_PUBLIC
-  void
-  remove_callback_group(
-    rclcpp::CallbackGroup::SharedPtr group_ptr,
-    bool notify = true) override;
-
-  /// Get callback groups that belong to executor.
-  /**
-   * \sa rclcpp::Executor::get_all_callback_groups()
-   */
-  RCLCPP_PUBLIC
-  std::vector<rclcpp::CallbackGroup::WeakPtr>
-  get_all_callback_groups() override;
-
-  /// Get callback groups that belong to executor.
-  /**
-   * \sa rclcpp::Executor::get_manually_added_callback_groups()
-   */
-  RCLCPP_PUBLIC
-  std::vector<rclcpp::CallbackGroup::WeakPtr>
-  get_manually_added_callback_groups() override;
-
-  /// Get callback groups that belong to executor.
-  /**
-   * \sa rclcpp::Executor::get_automatically_added_callback_groups_from_nodes()
-   */
-  RCLCPP_PUBLIC
-  std::vector<rclcpp::CallbackGroup::WeakPtr>
-  get_automatically_added_callback_groups_from_nodes() override;
-
 protected:
   /// Internal implementation of spin_once
   RCLCPP_PUBLIC
@@ -220,6 +137,11 @@ protected:
   void
   spin_some_impl(std::chrono::nanoseconds max_duration, bool exhaustive);
 
+  /// Collect entities from callback groups and refresh the current collection with them
+  RCLCPP_PUBLIC
+  void
+  handle_updated_entities(bool notify) override;
+
 private:
   RCLCPP_DISABLE_COPY(EventsExecutor)
 
@@ -227,9 +149,9 @@ private:
   void
   execute_event(const ExecutorEvent & event);
 
-  /// Collect entities from callback groups and refresh the current collection with them
+  /// Rebuilds the executor's notify waitable, as we can't use the one built in the base class
   void
-  refresh_current_collection_from_callback_groups();
+  setup_notify_waitable();
 
   /// Refresh the current collection using the provided new_collection
   void
@@ -253,6 +175,11 @@ private:
   typename CollectionType::EntitySharedPtr
   retrieve_entity(typename CollectionType::Key entity_id, CollectionType & collection)
   {
+    // Note: we lock the mutex because we assume that you are trying to get an element from the
+    // current collection... If there will be a use-case to retrieve elements also from other
+    // collections, we can move the mutex back to the calling codes.
+    std::lock_guard<std::mutex> guard(mutex_);
+
     // Check if the entity_id is in the collection
     auto it = collection.find(entity_id);
     if (it == collection.end()) {
@@ -273,16 +200,6 @@ private:
   /// Queue where entities can push events
   rclcpp::experimental::executors::EventsQueue::UniquePtr events_queue_;
 
-  std::shared_ptr<rclcpp::executors::ExecutorEntitiesCollector> entities_collector_;
-  std::shared_ptr<rclcpp::executors::ExecutorNotifyWaitable> notify_waitable_;
-
-  /// Mutex to protect the current_entities_collection_
-  std::recursive_mutex collection_mutex_;
-  std::shared_ptr<rclcpp::executors::ExecutorEntitiesCollection> current_entities_collection_;
-
-  /// Flag used to reduce the number of unnecessary waitable events
-  std::atomic<bool> notify_waitable_event_pushed_ {false};
-
   /// Timers manager used to track and/or execute associated timers
   std::shared_ptr<rclcpp::experimental::TimersManager> timers_manager_;
 };

rclcpp/include/rclcpp/loaned_message.hpp

@@ -82,36 +82,6 @@ public:
     }
   }
 
-  /// Constructor of the LoanedMessage class.
-  /**
-   * The constructor of this class allocates memory for a given message type
-   * and associates this with a given publisher.
-   *
-   * Given the publisher instance, a case differentiation is being performaned
-   * which decides whether the underlying middleware is able to allocate the appropriate
-   * memory for this message type or not.
-   * In the case that the middleware can not loan messages, the passed in allocator instance
-   * is being used to allocate the message within the scope of this class.
-   * Otherwise, the allocator is being ignored and the allocation is solely performaned
-   * in the underlying middleware with its appropriate allocation strategy.
-   * The need for this arises as the user code can be written explicitly targeting a middleware
-   * capable of loaning messages.
-   * However, this user code is ought to be usable even when dynamically linked against
-   * a middleware which doesn't support message loaning in which case the allocator will be used.
-   *
-   * \param[in] pub rclcpp::Publisher instance to which the memory belongs
-   * \param[in] allocator Allocator instance in case middleware can not allocate messages
-   * \throws anything rclcpp::exceptions::throw_from_rcl_error can throw.
-   */
-  [[
-    deprecated("used the LoanedMessage constructor that does not use a shared_ptr to the allocator")
-  ]]
-  LoanedMessage(
-    const rclcpp::PublisherBase * pub,
-    std::shared_ptr<std::allocator<MessageT>> allocator)
-  : LoanedMessage(*pub, *allocator)
-  {}
-
   /// Move semantic for RVO
   LoanedMessage(LoanedMessage<MessageT> && other)
   : pub_(std::move(other.pub_)),

rclcpp/include/rclcpp/logger.hpp

@@ -15,6 +15,7 @@
 #ifndef RCLCPP__LOGGER_HPP_
 #define RCLCPP__LOGGER_HPP_
 
+#include <filesystem>
 #include <memory>
 #include <string>
 #include <utility>
@@ -77,6 +78,34 @@ RCLCPP_PUBLIC
 Logger
 get_node_logger(const rcl_node_t * node);
 
+// TODO(ahcorde): Remove deprecated class on the next release (in Rolling after Kilted).
+#if !defined(_WIN32)
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#else  // !defined(_WIN32)
+# pragma warning(push)
+# pragma warning(disable: 4996)
+#endif
+/// Get the current logging directory.
+/**
+ * For more details of how the logging directory is determined,
+ * see rcl_logging_get_logging_directory().
+ *
+ * \returns the logging directory being used.
+ * \throws rclcpp::exceptions::RCLError if an unexpected error occurs.
+ */
+[[deprecated("use rclcpp::get_log_directory instead of rclcpp::get_logging_directory")]]
+RCLCPP_PUBLIC
+rcpputils::fs::path
+get_logging_directory();
+
+// remove warning suppression
+#if !defined(_WIN32)
+# pragma GCC diagnostic pop
+#else  // !defined(_WIN32)
+# pragma warning(pop)
+#endif
+
 /// Get the current logging directory.
 /**
  * For more details of how the logging directory is determined,
@@ -86,8 +115,8 @@ get_node_logger(const rcl_node_t * node);
  * \throws rclcpp::exceptions::RCLError if an unexpected error occurs.
  */
 RCLCPP_PUBLIC
-rcpputils::fs::path
-get_logging_directory();
+std::filesystem::path
+get_log_directory();
 
 class Logger
 {

rclcpp/include/rclcpp/node.hpp

@@ -257,22 +257,6 @@ public:
     CallbackT callback,
     rclcpp::CallbackGroup::SharedPtr group = nullptr);
 
-  /// Create and return a Client.
-  /**
-   * \param[in] service_name The topic to service on.
-   * \param[in] qos_profile rmw_qos_profile_t Quality of service profile for client.
-   * \param[in] group Callback group to call the service.
-   * \return Shared pointer to the created client.
-   * \deprecated use rclcpp::QoS instead of rmw_qos_profile_t
-   */
-  template<typename ServiceT>
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  typename rclcpp::Client<ServiceT>::SharedPtr
-  create_client(
-    const std::string & service_name,
-    const rmw_qos_profile_t & qos_profile,
-    rclcpp::CallbackGroup::SharedPtr group = nullptr);
-
   /// Create and return a Client.
   /**
    * \param[in] service_name The name on which the service is accessible.
@@ -287,24 +271,6 @@ public:
     const rclcpp::QoS & qos = rclcpp::ServicesQoS(),
     rclcpp::CallbackGroup::SharedPtr group = nullptr);
 
-  /// Create and return a Service.
-  /**
-   * \param[in] service_name The topic to service on.
-   * \param[in] callback User-defined callback function.
-   * \param[in] qos_profile rmw_qos_profile_t Quality of service profile for client.
-   * \param[in] group Callback group to call the service.
-   * \return Shared pointer to the created service.
-   * \deprecated use rclcpp::QoS instead of rmw_qos_profile_t
-   */
-  template<typename ServiceT, typename CallbackT>
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  typename rclcpp::Service<ServiceT>::SharedPtr
-  create_service(
-    const std::string & service_name,
-    CallbackT && callback,
-    const rmw_qos_profile_t & qos_profile,
-    rclcpp::CallbackGroup::SharedPtr group = nullptr);
-
   /// Create and return a Service.
   /**
    * \param[in] service_name The topic to service on.
@@ -1015,8 +981,6 @@ public:
     rclcpp::node_interfaces::OnSetParametersCallbackHandle;
   using OnSetParametersCallbackType =
     rclcpp::node_interfaces::NodeParametersInterface::OnSetParametersCallbackType;
-  using OnParametersSetCallbackType [[deprecated("use OnSetParametersCallbackType instead")]] =
-    OnSetParametersCallbackType;
 
   using PostSetParametersCallbackHandle =
     rclcpp::node_interfaces::PostSetParametersCallbackHandle;

rclcpp/include/rclcpp/node_impl.hpp

@@ -154,22 +154,6 @@ Node::create_client(
     group);
 }
 
-template<typename ServiceT>
-typename Client<ServiceT>::SharedPtr
-Node::create_client(
-  const std::string & service_name,
-  const rmw_qos_profile_t & qos_profile,
-  rclcpp::CallbackGroup::SharedPtr group)
-{
-  return rclcpp::create_client<ServiceT>(
-    node_base_,
-    node_graph_,
-    node_services_,
-    extend_name_with_sub_namespace(service_name, this->get_sub_namespace()),
-    qos_profile,
-    group);
-}
-
 template<typename ServiceT, typename CallbackT>
 typename rclcpp::Service<ServiceT>::SharedPtr
 Node::create_service(
@@ -187,23 +171,6 @@ Node::create_service(
     group);
 }
 
-template<typename ServiceT, typename CallbackT>
-typename rclcpp::Service<ServiceT>::SharedPtr
-Node::create_service(
-  const std::string & service_name,
-  CallbackT && callback,
-  const rmw_qos_profile_t & qos_profile,
-  rclcpp::CallbackGroup::SharedPtr group)
-{
-  return rclcpp::create_service<ServiceT, CallbackT>(
-    node_base_,
-    node_services_,
-    extend_name_with_sub_namespace(service_name, this->get_sub_namespace()),
-    std::forward<CallbackT>(callback),
-    qos_profile,
-    group);
-}
-
 template<typename AllocatorT>
 std::shared_ptr<rclcpp::GenericPublisher>
 Node::create_generic_publisher(

rclcpp/include/rclcpp/node_interfaces/node_parameters_interface.hpp

@@ -52,8 +52,6 @@ struct OnSetParametersCallbackHandle
     std::function<
     rcl_interfaces::msg::SetParametersResult(
       const std::vector<rclcpp::Parameter> &)>;
-  using OnParametersSetCallbackType [[deprecated("use OnSetParametersCallbackType instead")]] =
-    OnSetParametersCallbackType;
 
   OnSetParametersCallbackType callback;
 };

rclcpp/include/rclcpp/parameter_client.hpp

@@ -52,37 +52,6 @@ class AsyncParametersClient
 public:
   RCLCPP_SMART_PTR_DEFINITIONS(AsyncParametersClient)
 
-  /// Create an async parameters client.
-  /**
-   * \param[in] node_base_interface The node base interface of the corresponding node.
-   * \param[in] node_topics_interface Node topic base interface.
-   * \param[in] node_graph_interface The node graph interface of the corresponding node.
-   * \param[in] node_services_interface Node service interface.
-   * \param[in] remote_node_name Name of the remote node
-   * \param[in] qos_profile The rmw qos profile to use to subscribe
-   * \param[in] group (optional) The async parameter client will be added to this callback group.
-   * \deprecated use rclcpp::QoS instead of rmw_qos_profile_t
-   */
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  RCLCPP_PUBLIC
-  AsyncParametersClient(
-    const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base_interface,
-    const rclcpp::node_interfaces::NodeTopicsInterface::SharedPtr node_topics_interface,
-    const rclcpp::node_interfaces::NodeGraphInterface::SharedPtr node_graph_interface,
-    const rclcpp::node_interfaces::NodeServicesInterface::SharedPtr node_services_interface,
-    const std::string & remote_node_name,
-    const rmw_qos_profile_t & qos_profile,
-    rclcpp::CallbackGroup::SharedPtr group = nullptr)
-  : AsyncParametersClient(
-      node_base_interface,
-      node_topics_interface,
-      node_graph_interface,
-      node_services_interface,
-      remote_node_name,
-      rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos_profile)),
-      group)
-  {}
-
   /// Create an async parameters client.
   /**
    * \param[in] node_base_interface The node base interface of the corresponding node.
@@ -103,31 +72,6 @@ public:
     const rclcpp::QoS & qos_profile = rclcpp::ParametersQoS(),
     rclcpp::CallbackGroup::SharedPtr group = nullptr);
 
-  /// Constructor
-  /**
-   * \param[in] node The async parameters client will be added to this node.
-   * \param[in] remote_node_name name of the remote node
-   * \param[in] qos_profile The rmw qos profile to use to subscribe
-   * \param[in] group (optional) The async parameter client will be added to this callback group.
-   * \deprecated use rclcpp::QoS instead of rmw_qos_profile_t
-   */
-  template<typename NodeT>
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  AsyncParametersClient(
-    const std::shared_ptr<NodeT> node,
-    const std::string & remote_node_name,
-    const rmw_qos_profile_t & qos_profile,
-    rclcpp::CallbackGroup::SharedPtr group = nullptr)
-  : AsyncParametersClient(
-      node->get_node_base_interface(),
-      node->get_node_topics_interface(),
-      node->get_node_graph_interface(),
-      node->get_node_services_interface(),
-      remote_node_name,
-      rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos_profile)),
-      group)
-  {}
-
   /**
    * \param[in] node The async parameters client will be added to this node.
    * \param[in] remote_node_name (optional) name of the remote node
@@ -150,31 +94,6 @@ public:
       group)
   {}
 
-  /// Constructor
-  /**
-   * \param[in] node The async parameters client will be added to this node.
-   * \param[in] remote_node_name Name of the remote node
-   * \param[in] qos_profile The rmw qos profile to use to subscribe
-   * \param[in] group (optional) The async parameter client will be added to this callback group.
-   * \deprecated use rclcpp::QoS instead of rmw_qos_profile_t
-   */
-  template<typename NodeT>
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  AsyncParametersClient(
-    NodeT * node,
-    const std::string & remote_node_name,
-    const rmw_qos_profile_t & qos_profile,
-    rclcpp::CallbackGroup::SharedPtr group = nullptr)
-  : AsyncParametersClient(
-      node->get_node_base_interface(),
-      node->get_node_topics_interface(),
-      node->get_node_graph_interface(),
-      node->get_node_services_interface(),
-      remote_node_name,
-      rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos_profile)),
-      group)
-  {}
-
   /**
    * \param[in] node The async parameters client will be added to this node.
    * \param[in] remote_node_name (optional) name of the remote node
@@ -383,19 +302,6 @@ class SyncParametersClient
 public:
   RCLCPP_SMART_PTR_DEFINITIONS(SyncParametersClient)
 
-  template<typename NodeT>
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  SyncParametersClient(
-    std::shared_ptr<NodeT> node,
-    const std::string & remote_node_name,
-    const rmw_qos_profile_t & qos_profile)
-  : SyncParametersClient(
-      std::make_shared<rclcpp::executors::SingleThreadedExecutor>(),
-      node,
-      remote_node_name,
-      rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos_profile)))
-  {}
-
   template<typename NodeT>
   explicit SyncParametersClient(
     std::shared_ptr<NodeT> node,
@@ -408,23 +314,6 @@ public:
       qos_profile)
   {}
 
-  template<typename NodeT>
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  SyncParametersClient(
-    rclcpp::Executor::SharedPtr executor,
-    std::shared_ptr<NodeT> node,
-    const std::string & remote_node_name,
-    const rmw_qos_profile_t & qos_profile)
-  : SyncParametersClient(
-      executor,
-      node->get_node_base_interface(),
-      node->get_node_topics_interface(),
-      node->get_node_graph_interface(),
-      node->get_node_services_interface(),
-      remote_node_name,
-      rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos_profile)))
-  {}
-
   template<typename NodeT>
   SyncParametersClient(
     rclcpp::Executor::SharedPtr executor,
@@ -441,19 +330,6 @@ public:
       qos_profile)
   {}
 
-  template<typename NodeT>
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  SyncParametersClient(
-    NodeT * node,
-    const std::string & remote_node_name,
-    const rmw_qos_profile_t & qos_profile)
-  : SyncParametersClient(
-      std::make_shared<rclcpp::executors::SingleThreadedExecutor>(),
-      node,
-      remote_node_name,
-      rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos_profile)))
-  {}
-
   template<typename NodeT>
   explicit SyncParametersClient(
     NodeT * node,
@@ -466,23 +342,6 @@ public:
       qos_profile)
   {}
 
-  template<typename NodeT>
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  SyncParametersClient(
-    rclcpp::Executor::SharedPtr executor,
-    NodeT * node,
-    const std::string & remote_node_name,
-    const rmw_qos_profile_t & qos_profile)
-  : SyncParametersClient(
-      executor,
-      node->get_node_base_interface(),
-      node->get_node_topics_interface(),
-      node->get_node_graph_interface(),
-      node->get_node_services_interface(),
-      remote_node_name,
-      rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos_profile)))
-  {}
-
   template<typename NodeT>
   SyncParametersClient(
     rclcpp::Executor::SharedPtr executor,
@@ -499,28 +358,6 @@ public:
       qos_profile)
   {}
 
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  RCLCPP_PUBLIC
-  SyncParametersClient(
-    rclcpp::Executor::SharedPtr executor,
-    const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base_interface,
-    const rclcpp::node_interfaces::NodeTopicsInterface::SharedPtr node_topics_interface,
-    const rclcpp::node_interfaces::NodeGraphInterface::SharedPtr node_graph_interface,
-    const rclcpp::node_interfaces::NodeServicesInterface::SharedPtr node_services_interface,
-    const std::string & remote_node_name,
-    const rmw_qos_profile_t & qos_profile)
-  : executor_(executor), node_base_interface_(node_base_interface)
-  {
-    async_parameters_client_ =
-      std::make_shared<AsyncParametersClient>(
-      node_base_interface,
-      node_topics_interface,
-      node_graph_interface,
-      node_services_interface,
-      remote_node_name,
-      rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos_profile)));
-  }
-
   RCLCPP_PUBLIC
   SyncParametersClient(
     rclcpp::Executor::SharedPtr executor,

rclcpp/include/rclcpp/parameter_service.hpp

@@ -40,20 +40,6 @@ class ParameterService
 public:
   RCLCPP_SMART_PTR_DEFINITIONS(ParameterService)
 
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  RCLCPP_PUBLIC
-  ParameterService(
-    const std::shared_ptr<node_interfaces::NodeBaseInterface> node_base,
-    const std::shared_ptr<node_interfaces::NodeServicesInterface> node_services,
-    rclcpp::node_interfaces::NodeParametersInterface * node_params,
-    const rmw_qos_profile_t & qos_profile)
-  : ParameterService(
-      node_base,
-      node_services,
-      node_params,
-      rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(qos_profile)))
-  {}
-
   RCLCPP_PUBLIC
   ParameterService(
     const std::shared_ptr<node_interfaces::NodeBaseInterface> node_base,

rclcpp/include/rclcpp/publisher.hpp

@@ -96,22 +96,6 @@ public:
   using ROSMessageTypeAllocator = typename ROSMessageTypeAllocatorTraits::allocator_type;
   using ROSMessageTypeDeleter = allocator::Deleter<ROSMessageTypeAllocator, ROSMessageType>;
 
-  using MessageAllocatorTraits
-  [[deprecated("use PublishedTypeAllocatorTraits")]] =
-    PublishedTypeAllocatorTraits;
-  using MessageAllocator
-  [[deprecated("use PublishedTypeAllocator")]] =
-    PublishedTypeAllocator;
-  using MessageDeleter
-  [[deprecated("use PublishedTypeDeleter")]] =
-    PublishedTypeDeleter;
-  using MessageUniquePtr
-  [[deprecated("use std::unique_ptr<PublishedType, PublishedTypeDeleter>")]] =
-    std::unique_ptr<PublishedType, PublishedTypeDeleter>;
-  using MessageSharedPtr
-  [[deprecated("use std::shared_ptr<const PublishedType>")]] =
-    std::shared_ptr<const PublishedType>;
-
   using BufferSharedPtr = typename rclcpp::experimental::buffers::IntraProcessBuffer<
     ROSMessageType,
     ROSMessageTypeAllocator,
@@ -128,8 +112,8 @@ public:
    *
    * \param[in] node_base NodeBaseInterface pointer that is used in part of the setup.
    * \param[in] topic Name of the topic to publish to.
-   * \param[in] qos QoS profile for Subcription.
-   * \param[in] options Options for the subscription.
+   * \param[in] qos QoS profile for the publisher.
+   * \param[in] options Options for the publisher.
    */
   Publisher(
     rclcpp::node_interfaces::NodeBaseInterface * node_base,
@@ -423,13 +407,6 @@ public:
     }
   }
 
-  [[deprecated("use get_published_type_allocator() or get_ros_message_type_allocator() instead")]]
-  std::shared_ptr<PublishedTypeAllocator>
-  get_allocator() const
-  {
-    return std::make_shared<PublishedTypeAllocator>(published_type_allocator_);
-  }
-
   PublishedTypeAllocator
   get_published_type_allocator() const
   {

rclcpp/include/rclcpp/rate.hpp

@@ -39,10 +39,6 @@ public:
   RCLCPP_PUBLIC
   virtual bool sleep() = 0;
 
-  [[deprecated("use get_type() instead")]]
-  RCLCPP_PUBLIC
-  virtual bool is_steady() const = 0;
-
   RCLCPP_PUBLIC
   virtual rcl_clock_type_t get_type() const = 0;
 
@@ -54,82 +50,6 @@ using std::chrono::duration;
 using std::chrono::duration_cast;
 using std::chrono::nanoseconds;
 
-template<class Clock = std::chrono::high_resolution_clock>
-class [[deprecated("use rclcpp::Rate class instead of GenericRate")]] GenericRate : public RateBase
-{
-public:
-  RCLCPP_SMART_PTR_DEFINITIONS(GenericRate)
-
-  explicit GenericRate(double rate)
-  : period_(duration_cast<nanoseconds>(duration<double>(1.0 / rate))), last_interval_(Clock::now())
-  {}
-  explicit GenericRate(std::chrono::nanoseconds period)
-  : period_(period), last_interval_(Clock::now())
-  {}
-
-  virtual bool
-  sleep()
-  {
-    // Time coming into sleep
-    auto now = Clock::now();
-    // Time of next interval
-    auto next_interval = last_interval_ + period_;
-    // Detect backwards time flow
-    if (now < last_interval_) {
-      // Best thing to do is to set the next_interval to now + period
-      next_interval = now + period_;
-    }
-    // Calculate the time to sleep
-    auto time_to_sleep = next_interval - now;
-    // Update the interval
-    last_interval_ += period_;
-    // If the time_to_sleep is negative or zero, don't sleep
-    if (time_to_sleep <= std::chrono::seconds(0)) {
-      // If an entire cycle was missed then reset next interval.
-      // This might happen if the loop took more than a cycle.
-      // Or if time jumps forward.
-      if (now > next_interval + period_) {
-        last_interval_ = now + period_;
-      }
-      // Either way do not sleep and return false
-      return false;
-    }
-    // Sleep (will get interrupted by ctrl-c, may not sleep full time)
-    rclcpp::sleep_for(time_to_sleep);
-    return true;
-  }
-
-  [[deprecated("use get_type() instead")]]
-  virtual bool
-  is_steady() const
-  {
-    return Clock::is_steady;
-  }
-
-  virtual rcl_clock_type_t get_type() const
-  {
-    return Clock::is_steady ? RCL_STEADY_TIME : RCL_SYSTEM_TIME;
-  }
-
-  virtual void
-  reset()
-  {
-    last_interval_ = Clock::now();
-  }
-
-  std::chrono::nanoseconds period() const
-  {
-    return period_;
-  }
-
-private:
-  RCLCPP_DISABLE_COPY(GenericRate)
-
-  std::chrono::nanoseconds period_;
-  using ClockDurationNano = std::chrono::duration<typename Clock::rep, std::nano>;
-  std::chrono::time_point<Clock, ClockDurationNano> last_interval_;
-};
-
 class Rate : public RateBase
 {
 public:
@@ -149,11 +69,6 @@ public:
   virtual bool
   sleep();
 
-  [[deprecated("use get_type() instead")]]
-  RCLCPP_PUBLIC
-  virtual bool
-  is_steady() const;
-
   RCLCPP_PUBLIC
   virtual rcl_clock_type_t
   get_type() const;

rclcpp/include/rclcpp/service.hpp

@@ -307,7 +307,7 @@ public:
    * \param[in] node_handle NodeBaseInterface pointer that is used in part of the setup.
    * \param[in] service_name Name of the topic to publish to.
    * \param[in] any_callback User defined callback to call when a client request is received.
-   * \param[in] service_options options for the subscription.
+   * \param[in] service_options options for the service.
    */
   Service(
     std::shared_ptr<rcl_node_t> node_handle,

rclcpp/include/rclcpp/subscription.hpp

@@ -90,18 +90,6 @@ public:
   using ROSMessageTypeAllocator = typename ROSMessageTypeAllocatorTraits::allocator_type;
   using ROSMessageTypeDeleter = allocator::Deleter<ROSMessageTypeAllocator, ROSMessageType>;
 
-  using MessageAllocatorTraits [[deprecated("use ROSMessageTypeAllocatorTraits")]] =
-    ROSMessageTypeAllocatorTraits;
-  using MessageAllocator [[deprecated("use ROSMessageTypeAllocator")]] =
-    ROSMessageTypeAllocator;
-  using MessageDeleter [[deprecated("use ROSMessageTypeDeleter")]] =
-    ROSMessageTypeDeleter;
-
-  using ConstMessageSharedPtr [[deprecated]] = std::shared_ptr<const ROSMessageType>;
-  using MessageUniquePtr
-  [[deprecated("use std::unique_ptr<ROSMessageType, ROSMessageTypeDeleter> instead")]] =
-    std::unique_ptr<ROSMessageType, ROSMessageTypeDeleter>;
-
 private:
   using SubscriptionTopicStatisticsSharedPtr =
     std::shared_ptr<rclcpp::topic_statistics::SubscriptionTopicStatistics>;

rclcpp/include/rclcpp/typesupport_helpers.hpp

@@ -38,25 +38,6 @@ RCLCPP_PUBLIC
 std::shared_ptr<rcpputils::SharedLibrary>
 get_typesupport_library(const std::string & type, const std::string & typesupport_identifier);
 
-/// Extract the type support handle from the library.
-/**
- * The library needs to match the topic type. The shared library must stay loaded for the lifetime of the result.
- *
- * \deprecated Use get_message_typesupport_handle() instead
- *
- * \param[in] type The topic type, e.g. "std_msgs/msg/String"
- * \param[in] typesupport_identifier Type support identifier, typically "rosidl_typesupport_cpp"
- * \param[in] library The shared type support library
- * \return A type support handle
- */
-[[deprecated("Use `get_message_typesupport_handle` instead")]]
-RCLCPP_PUBLIC
-const rosidl_message_type_support_t *
-get_typesupport_handle(
-  const std::string & type,
-  const std::string & typesupport_identifier,
-  rcpputils::SharedLibrary & library);
-
 /// Extract the message type support handle from the library.
 /**
  * The library needs to match the topic type. The shared library must stay loaded for the lifetime of the result.

rclcpp/include/rclcpp/waitable.hpp

@@ -101,23 +101,6 @@ public:
   size_t
   get_number_of_ready_guard_conditions();
 
-#ifdef __GNUC__
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Woverloaded-virtual"
-#endif
-  /// Deprecated.
-  /**
-   * \sa add_to_wait_set(rcl_wait_set_t &)
-   */
-  [[deprecated("Use add_to_wait_set(rcl_wait_set_t & wait_set) signature")]]
-  RCLCPP_PUBLIC
-  virtual
-  void
-  add_to_wait_set(rcl_wait_set_t * wait_set);
-#ifdef __GNUC__
-#pragma GCC diagnostic pop
-#endif
-
   /// Add the Waitable to a wait set.
   /**
    * \param[in] wait_set A handle to the wait set to add the Waitable to.
@@ -128,23 +111,6 @@ public:
   void
   add_to_wait_set(rcl_wait_set_t & wait_set);
 
-#ifdef __GNUC__
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Woverloaded-virtual"
-#endif
-  /// Deprecated.
-  /**
-   * \sa is_ready(const rcl_wait_set_t &)
-   */
-  [[deprecated("Use is_ready(const rcl_wait_set_t & wait_set) signature")]]
-  RCLCPP_PUBLIC
-  virtual
-  bool
-  is_ready(rcl_wait_set_t * wait_set);
-#ifdef __GNUC__
-#pragma GCC diagnostic pop
-#endif
-
   /// Check if the Waitable is ready.
   /**
    * The input wait set should be the same that was used in a previously call to
@@ -212,23 +178,6 @@ public:
   std::shared_ptr<void>
   take_data_by_entity_id(size_t id);
 
-#ifdef __GNUC__
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Woverloaded-virtual"
-#endif
-  /// Deprecated.
-  /**
-   * \sa execute(const std::shared_ptr<void> &)
-   */
-  [[deprecated("Use execute(const std::shared_ptr<void> & data) signature")]]
-  RCLCPP_PUBLIC
-  virtual
-  void
-  execute(std::shared_ptr<void> & data);
-#ifdef __GNUC__
-#pragma GCC diagnostic pop
-#endif
-
   /// Execute data that is passed in.
   /**
    * Before calling this method, the Waitable should be added to a wait set,

rclcpp/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp</name>
-  <version>28.3.0</version>
+  <version>28.3.2</version>
   <description>The ROS client library in C++.</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp/src/rclcpp/executor.cpp

@@ -129,7 +129,7 @@ Executor::~Executor()
 }
 
 void
-Executor::trigger_entity_recollect(bool notify)
+Executor::handle_updated_entities(bool notify)
 {
   this->entities_need_rebuild_.store(true);
 
@@ -174,11 +174,11 @@ Executor::add_callback_group(
   this->collector_.add_callback_group(group_ptr);
 
   try {
-    this->trigger_entity_recollect(notify);
+    this->handle_updated_entities(notify);
   } catch (const rclcpp::exceptions::RCLError & ex) {
     throw std::runtime_error(
             std::string(
-              "Failed to trigger guard condition on callback group add: ") + ex.what());
+              "Failed to handle entities update on callback group add: ") + ex.what());
   }
 }
 
@@ -188,11 +188,11 @@ Executor::add_node(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_pt
   this->collector_.add_node(node_ptr);
 
   try {
-    this->trigger_entity_recollect(notify);
+    this->handle_updated_entities(notify);
   } catch (const rclcpp::exceptions::RCLError & ex) {
     throw std::runtime_error(
             std::string(
-              "Failed to trigger guard condition on node add: ") + ex.what());
+              "Failed to handle entities update on node add: ") + ex.what());
   }
 }
 
@@ -204,11 +204,11 @@ Executor::remove_callback_group(
   this->collector_.remove_callback_group(group_ptr);
 
   try {
-    this->trigger_entity_recollect(notify);
+    this->handle_updated_entities(notify);
   } catch (const rclcpp::exceptions::RCLError & ex) {
     throw std::runtime_error(
             std::string(
-              "Failed to trigger guard condition on callback group remove: ") + ex.what());
+              "Failed to handle entities update on callback group remove: ") + ex.what());
   }
 }
 
@@ -224,11 +224,11 @@ Executor::remove_node(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node
   this->collector_.remove_node(node_ptr);
 
   try {
-    this->trigger_entity_recollect(notify);
+    this->handle_updated_entities(notify);
   } catch (const rclcpp::exceptions::RCLError & ex) {
     throw std::runtime_error(
             std::string(
-              "Failed to trigger guard condition on node remove: ") + ex.what());
+              "Failed to handle entities update on node remove: ") + ex.what());
   }
 }
 
@@ -275,7 +275,7 @@ Executor::spin_until_future_complete_impl(
   if (spinning.exchange(true)) {
     throw std::runtime_error("spin_until_future_complete() called while already spinning");
   }
-  RCPPUTILS_SCOPE_EXIT(this->spinning.store(false); );
+  RCPPUTILS_SCOPE_EXIT(wait_result_.reset();this->spinning.store(false););
   while (rclcpp::ok(this->context_) && spinning.load()) {
     // Do one item of work.
     spin_once_impl(timeout_left);
@@ -364,7 +364,7 @@ Executor::spin_some_impl(std::chrono::nanoseconds max_duration, bool exhaustive)
   if (spinning.exchange(true)) {
     throw std::runtime_error("spin_some() called while already spinning");
   }
-  RCPPUTILS_SCOPE_EXIT(this->spinning.store(false); );
+  RCPPUTILS_SCOPE_EXIT(wait_result_.reset();this->spinning.store(false););
 
   // clear the wait result and wait for work without blocking to collect the work
   // for the first time
@@ -431,7 +431,7 @@ Executor::spin_once(std::chrono::nanoseconds timeout)
   if (spinning.exchange(true)) {
     throw std::runtime_error("spin_once() called while already spinning");
   }
-  RCPPUTILS_SCOPE_EXIT(this->spinning.store(false); );
+  RCPPUTILS_SCOPE_EXIT(wait_result_.reset();this->spinning.store(false););
   spin_once_impl(timeout);
 }
 

rclcpp/src/rclcpp/executors/executor_notify_waitable.cpp

@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include <iostream>
-
 #include "rclcpp/exceptions.hpp"
 #include "rclcpp/executors/executor_notify_waitable.hpp"
 
@@ -91,9 +89,9 @@ ExecutorNotifyWaitable::is_ready(const rcl_wait_set_t & wait_set)
 }
 
 void
-ExecutorNotifyWaitable::execute(const std::shared_ptr<void> & data)
+ExecutorNotifyWaitable::execute(const std::shared_ptr<void> & /*data*/)
 {
-  (void) data;
+  std::lock_guard<std::mutex> lock(execute_mutex_);
   this->execute_callback_();
 }
 
@@ -149,6 +147,14 @@ ExecutorNotifyWaitable::clear_on_ready_callback()
   }
 }
 
+RCLCPP_PUBLIC
+void
+ExecutorNotifyWaitable::set_execute_callback(std::function<void(void)> on_execute_callback)
+{
+  std::lock_guard<std::mutex> lock(execute_mutex_);
+  execute_callback_ = on_execute_callback;
+}
+
 void
 ExecutorNotifyWaitable::add_guard_condition(rclcpp::GuardCondition::WeakPtr weak_guard_condition)
 {

rclcpp/src/rclcpp/executors/multi_threaded_executor.cpp

@@ -55,7 +55,7 @@ MultiThreadedExecutor::spin()
   if (spinning.exchange(true)) {
     throw std::runtime_error("spin() called while already spinning");
   }
-  RCPPUTILS_SCOPE_EXIT(this->spinning.store(false););
+  RCPPUTILS_SCOPE_EXIT(wait_result_.reset();this->spinning.store(false););
   std::vector<std::thread> threads;
   size_t thread_id = 0;
   {

rclcpp/src/rclcpp/executors/single_threaded_executor.cpp

@@ -30,7 +30,7 @@ SingleThreadedExecutor::spin()
   if (spinning.exchange(true)) {
     throw std::runtime_error("spin() called while already spinning");
   }
-  RCPPUTILS_SCOPE_EXIT(this->spinning.store(false); );
+  RCPPUTILS_SCOPE_EXIT(wait_result_.reset();this->spinning.store(false););
 
   // Clear any previous result and rebuild the waitset
   this->wait_result_.reset();

rclcpp/src/rclcpp/experimental/executors/events_executor/events_executor.cpp

@@ -52,30 +52,37 @@ EventsExecutor::EventsExecutor(
   timers_manager_ =
     std::make_shared<rclcpp::experimental::TimersManager>(context_, timer_on_ready_cb);
 
-  this->current_entities_collection_ =
-    std::make_shared<rclcpp::executors::ExecutorEntitiesCollection>();
+  entities_need_rebuild_ = false;
 
-  notify_waitable_ = std::make_shared<rclcpp::executors::ExecutorNotifyWaitable>(
+  this->setup_notify_waitable();
+
+  // Ensure that the entities collection is empty (the base class may have added elements
+  // that we are not interested in)
+  this->current_collection_.clear();
+
+  // Make sure that the notify waitable is immediately added to the collection
+  // to avoid missing events
+  this->add_notify_waitable_to_collection(current_collection_.waitables);
+}
+
+void
+EventsExecutor::setup_notify_waitable()
+{
+  // The base class already created this object but the events-executor
+  // needs different callbacks.
+  assert(notify_waitable_ && "The notify waitable should have already been constructed");
+
+  notify_waitable_->set_execute_callback(
     [this]() {
       // This callback is invoked when:
       // - the interrupt or shutdown guard condition is triggered:
       //    ---> we need to wake up the executor so that it can terminate
       // - a node or callback group guard condition is triggered:
       //    ---> the entities collection is changed, we need to update callbacks
-      notify_waitable_event_pushed_ = false;
-      this->refresh_current_collection_from_callback_groups();
+      entities_need_rebuild_ = false;
+      this->handle_updated_entities(false);
     });
 
-  // Make sure that the notify waitable is immediately added to the collection
-  // to avoid missing events
-  this->add_notify_waitable_to_collection(current_entities_collection_->waitables);
-
-  notify_waitable_->add_guard_condition(interrupt_guard_condition_);
-  notify_waitable_->add_guard_condition(shutdown_guard_condition_);
-
-  notify_waitable_->set_on_ready_callback(
-    this->create_waitable_callback(notify_waitable_.get()));
-
   auto notify_waitable_entity_id = notify_waitable_.get();
   notify_waitable_->set_on_ready_callback(
     [this, notify_waitable_entity_id](size_t num_events, int waitable_data) {
@@ -85,7 +92,7 @@ EventsExecutor::EventsExecutor(
       // For the same reason, if an event of this type has already been pushed but it has not been
       // processed yet, we avoid pushing additional events.
       (void)num_events;
-      if (notify_waitable_event_pushed_.exchange(true)) {
+      if (entities_need_rebuild_.exchange(true)) {
         return;
       }
 
@@ -93,9 +100,6 @@ EventsExecutor::EventsExecutor(
       {notify_waitable_entity_id, nullptr, waitable_data, ExecutorEventType::WAITABLE_EVENT, 1};
       this->events_queue_->enqueue(event);
     });
-
-  this->entities_collector_ =
-    std::make_shared<rclcpp::executors::ExecutorEntitiesCollector>(notify_waitable_);
 }
 
 EventsExecutor::~EventsExecutor()
@@ -229,46 +233,6 @@ EventsExecutor::spin_once_impl(std::chrono::nanoseconds timeout)
   }
 }
 
-void
-EventsExecutor::add_node(
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify)
-{
-  // This field is unused because we don't have to wake up the executor when a node is added.
-  (void) notify;
-
-  // Add node to entities collector
-  this->entities_collector_->add_node(node_ptr);
-
-  this->refresh_current_collection_from_callback_groups();
-}
-
-void
-EventsExecutor::add_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify)
-{
-  this->add_node(node_ptr->get_node_base_interface(), notify);
-}
-
-void
-EventsExecutor::remove_node(
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify)
-{
-  // This field is unused because we don't have to wake up the executor when a node is removed.
-  (void)notify;
-
-  // Remove node from entities collector.
-  // This will result in un-setting all the event callbacks from its entities.
-  // After this function returns, this executor will not receive any more events associated
-  // to these entities.
-  this->entities_collector_->remove_node(node_ptr);
-
-  this->refresh_current_collection_from_callback_groups();
-}
-
-void
-EventsExecutor::remove_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify)
-{
-  this->remove_node(node_ptr->get_node_base_interface(), notify);
-}
 
 void
 EventsExecutor::execute_event(const ExecutorEvent & event)
@@ -278,10 +242,9 @@ EventsExecutor::execute_event(const ExecutorEvent & event)
       {
         rclcpp::ClientBase::SharedPtr client;
         {
-          std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
           client = this->retrieve_entity(
             static_cast<const rcl_client_t *>(event.entity_key),
-            current_entities_collection_->clients);
+            current_collection_.clients);
         }
         if (client) {
           for (size_t i = 0; i < event.num_events; i++) {
@@ -295,10 +258,9 @@ EventsExecutor::execute_event(const ExecutorEvent & event)
       {
         rclcpp::SubscriptionBase::SharedPtr subscription;
         {
-          std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
           subscription = this->retrieve_entity(
             static_cast<const rcl_subscription_t *>(event.entity_key),
-            current_entities_collection_->subscriptions);
+            current_collection_.subscriptions);
         }
         if (subscription) {
           for (size_t i = 0; i < event.num_events; i++) {
@@ -311,10 +273,9 @@ EventsExecutor::execute_event(const ExecutorEvent & event)
       {
         rclcpp::ServiceBase::SharedPtr service;
         {
-          std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
           service = this->retrieve_entity(
             static_cast<const rcl_service_t *>(event.entity_key),
-            current_entities_collection_->services);
+            current_collection_.services);
         }
         if (service) {
           for (size_t i = 0; i < event.num_events; i++) {
@@ -334,10 +295,9 @@ EventsExecutor::execute_event(const ExecutorEvent & event)
       {
         rclcpp::Waitable::SharedPtr waitable;
         {
-          std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
           waitable = this->retrieve_entity(
             static_cast<const rclcpp::Waitable *>(event.entity_key),
-            current_entities_collection_->waitables);
+            current_collection_.waitables);
         }
         if (waitable) {
           for (size_t i = 0; i < event.num_events; i++) {
@@ -351,61 +311,12 @@ EventsExecutor::execute_event(const ExecutorEvent & event)
 }
 
 void
-EventsExecutor::add_callback_group(
-  rclcpp::CallbackGroup::SharedPtr group_ptr,
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-  bool notify)
+EventsExecutor::handle_updated_entities(bool notify)
 {
-  // This field is unused because we don't have to wake up
-  // the executor when a callback group is added.
   (void)notify;
-  (void)node_ptr;
-
-  this->entities_collector_->add_callback_group(group_ptr);
-
-  this->refresh_current_collection_from_callback_groups();
-}
-
-void
-EventsExecutor::remove_callback_group(
-  rclcpp::CallbackGroup::SharedPtr group_ptr, bool notify)
-{
-  // This field is unused because we don't have to wake up
-  // the executor when a callback group is removed.
-  (void)notify;
-
-  this->entities_collector_->remove_callback_group(group_ptr);
-
-  this->refresh_current_collection_from_callback_groups();
-}
-
-std::vector<rclcpp::CallbackGroup::WeakPtr>
-EventsExecutor::get_all_callback_groups()
-{
-  this->entities_collector_->update_collections();
-  return this->entities_collector_->get_all_callback_groups();
-}
-
-std::vector<rclcpp::CallbackGroup::WeakPtr>
-EventsExecutor::get_manually_added_callback_groups()
-{
-  this->entities_collector_->update_collections();
-  return this->entities_collector_->get_manually_added_callback_groups();
-}
-
-std::vector<rclcpp::CallbackGroup::WeakPtr>
-EventsExecutor::get_automatically_added_callback_groups_from_nodes()
-{
-  this->entities_collector_->update_collections();
-  return this->entities_collector_->get_automatically_added_callback_groups();
-}
-
-void
-EventsExecutor::refresh_current_collection_from_callback_groups()
-{
   // Build the new collection
-  this->entities_collector_->update_collections();
-  auto callback_groups = this->entities_collector_->get_all_callback_groups();
+  this->collector_.update_collections();
+  auto callback_groups = this->collector_.get_all_callback_groups();
   rclcpp::executors::ExecutorEntitiesCollection new_collection;
   rclcpp::executors::build_entities_collection(callback_groups, new_collection);
 
@@ -415,14 +326,11 @@ EventsExecutor::refresh_current_collection_from_callback_groups()
   // We could explicitly check for the notify waitable ID when we receive a waitable event
   // but I think that it's better if the waitable was in the collection and it could be
   // retrieved in the "standard" way.
-  // To do it, we need to add the notify waitable as an entry in both the new and
-  // current collections such that it's neither added or removed.
+  // To do it, we need to add the notify waitable as an entry in the new collection
+  // such that it's neither added or removed (it should have already been added
+  // to the current collection in the constructor)
   this->add_notify_waitable_to_collection(new_collection.waitables);
 
-  // Acquire lock before modifying the current collection
-  std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
-  this->add_notify_waitable_to_collection(current_entities_collection_->waitables);
-
   this->refresh_current_collection(new_collection);
 }
 
@@ -431,14 +339,14 @@ EventsExecutor::refresh_current_collection(
   const rclcpp::executors::ExecutorEntitiesCollection & new_collection)
 {
   // Acquire lock before modifying the current collection
-  std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
+  std::lock_guard<std::mutex> guard(mutex_);
 
-  current_entities_collection_->timers.update(
+  current_collection_.timers.update(
     new_collection.timers,
     [this](rclcpp::TimerBase::SharedPtr timer) {timers_manager_->add_timer(timer);},
     [this](rclcpp::TimerBase::SharedPtr timer) {timers_manager_->remove_timer(timer);});
 
-  current_entities_collection_->subscriptions.update(
+  current_collection_.subscriptions.update(
     new_collection.subscriptions,
     [this](auto subscription) {
       subscription->set_on_new_message_callback(
@@ -447,7 +355,7 @@ EventsExecutor::refresh_current_collection(
     },
     [](auto subscription) {subscription->clear_on_new_message_callback();});
 
-  current_entities_collection_->clients.update(
+  current_collection_.clients.update(
     new_collection.clients,
     [this](auto client) {
       client->set_on_new_response_callback(
@@ -456,7 +364,7 @@ EventsExecutor::refresh_current_collection(
     },
     [](auto client) {client->clear_on_new_response_callback();});
 
-  current_entities_collection_->services.update(
+  current_collection_.services.update(
     new_collection.services,
     [this](auto service) {
       service->set_on_new_request_callback(
@@ -467,12 +375,12 @@ EventsExecutor::refresh_current_collection(
 
   // DO WE NEED THIS? WE ARE NOT DOING ANYTHING WITH GUARD CONDITIONS
   /*
-  current_entities_collection_->guard_conditions.update(new_collection.guard_conditions,
+  current_collection_.guard_conditions.update(new_collection.guard_conditions,
     [](auto guard_condition) {(void)guard_condition;},
     [](auto guard_condition) {guard_condition->set_on_trigger_callback(nullptr);});
   */
 
-  current_entities_collection_->waitables.update(
+  current_collection_.waitables.update(
     new_collection.waitables,
     [this](auto waitable) {
       waitable->set_on_ready_callback(

rclcpp/src/rclcpp/logger.cpp

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <filesystem>
 #include <memory>
 #include <string>
 #include <utility>
@@ -54,6 +55,14 @@ get_node_logger(const rcl_node_t * node)
   return rclcpp::get_logger(logger_name);
 }
 
+// TODO(ahcorde): Remove deprecated class on the next release (in Rolling after Kilted).
+#if !defined(_WIN32)
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#else  // !defined(_WIN32)
+# pragma warning(push)
+# pragma warning(disable: 4996)
+#endif
 rcpputils::fs::path
 get_logging_directory()
 {
@@ -67,6 +76,26 @@ get_logging_directory()
   allocator.deallocate(log_dir, allocator.state);
   return path;
 }
+// remove warning suppression
+#if !defined(_WIN32)
+# pragma GCC diagnostic pop
+#else  // !defined(_WIN32)
+# pragma warning(pop)
+#endif
+
+std::filesystem::path
+get_log_directory()
+{
+  char * log_dir = NULL;
+  auto allocator = rcutils_get_default_allocator();
+  rcl_logging_ret_t ret = rcl_logging_get_logging_directory(allocator, &log_dir);
+  if (RCL_LOGGING_RET_OK != ret) {
+    rclcpp::exceptions::throw_from_rcl_error(ret);
+  }
+  std::string path{log_dir};
+  allocator.deallocate(log_dir, allocator.state);
+  return path;
+}
 
 Logger
 Logger::get_child(const std::string & suffix)

rclcpp/src/rclcpp/rate.cpp

@@ -71,12 +71,6 @@ Rate::sleep()
   return true;
 }
 
-bool
-Rate::is_steady() const
-{
-  return clock_->get_clock_type() == RCL_STEADY_TIME;
-}
-
 rcl_clock_type_t
 Rate::get_type() const
 {

rclcpp/src/rclcpp/waitable.cpp

@@ -87,81 +87,21 @@ Waitable::clear_on_ready_callback()
           "want to use it and make sure to call it on the waitable destructor.");
 }
 
-void
-Waitable::add_to_wait_set(rcl_wait_set_t * wait_set)
+bool
+Waitable::is_ready(const rcl_wait_set_t & wait_set)
 {
-  this->add_to_wait_set(*wait_set);
+  return this->is_ready(wait_set);
 }
 
 void
 Waitable::add_to_wait_set(rcl_wait_set_t & wait_set)
 {
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-  this->add_to_wait_set(&wait_set);
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
-}
-
-bool
-Waitable::is_ready(rcl_wait_set_t * wait_set)
-{
-  const rcl_wait_set_t & const_wait_set_ref = *wait_set;
-  return this->is_ready(const_wait_set_ref);
-}
-
-bool
-Waitable::is_ready(const rcl_wait_set_t & wait_set)
-{
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-  // note this const cast is only required to support a deprecated function
-  return this->is_ready(&const_cast<rcl_wait_set_t &>(wait_set));
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
-}
-
-void
-Waitable::execute(std::shared_ptr<void> & data)
-{
-  const std::shared_ptr<void> & const_data = data;
-  this->execute(const_data);
+  this->add_to_wait_set(wait_set);
 }
 
 void
 Waitable::execute(const std::shared_ptr<void> & data)
 {
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
   // note this const cast is only required to support a deprecated function
   this->execute(const_cast<std::shared_ptr<void> &>(data));
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
 }

rclcpp/test/rclcpp/CMakeLists.txt

@@ -486,8 +486,12 @@ endif()
 ament_add_gtest(
   test_executors
   executors/test_executors.cpp
+  executors/test_waitable.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}"
   TIMEOUT 180)
+if(CMAKE_BUILD_TYPE STREQUAL "Debug" AND MSVC)
+  target_compile_options(test_executors PRIVATE "/bigobj")
+endif()
 if(TARGET test_executors)
   target_link_libraries(test_executors ${PROJECT_NAME} rcl::rcl ${test_msgs_TARGETS})
 endif()
@@ -519,6 +523,16 @@ if(TARGET test_executors)
   target_link_libraries(test_executors_intraprocess ${PROJECT_NAME} ${test_msgs_TARGETS})
 endif()
 
+ament_add_gtest(
+  test_executors_busy_waiting
+  executors/test_executors_busy_waiting.cpp
+  executors/test_waitable.cpp
+  APPEND_LIBRARY_DIRS "${append_library_dirs}"
+  TIMEOUT 180)
+if(TARGET test_executors)
+  target_link_libraries(test_executors_busy_waiting ${PROJECT_NAME})
+endif()
+
 ament_add_gtest(test_static_single_threaded_executor executors/test_static_single_threaded_executor.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}")
 ament_add_test_label(test_static_single_threaded_executor mimick)
@@ -627,7 +641,7 @@ ament_add_gtest(test_executor test_executor.cpp
   TIMEOUT 120)
 ament_add_test_label(test_executor mimick)
 if(TARGET test_executor)
-  target_link_libraries(test_executor ${PROJECT_NAME} mimick)
+  target_link_libraries(test_executor ${PROJECT_NAME} mimick ${test_msgs_TARGETS})
 endif()
 
 ament_add_gtest(test_graph_listener test_graph_listener.cpp)

rclcpp/test/rclcpp/executors/test_executors.cpp

@@ -39,8 +39,10 @@
 #include "rclcpp/time_source.hpp"
 
 #include "test_msgs/msg/empty.hpp"
+#include "test_msgs/srv/empty.hpp"
 
 #include "./executor_types.hpp"
+#include "./test_waitable.hpp"
 
 using namespace std::chrono_literals;
 
@@ -331,114 +333,6 @@ TYPED_TEST(TestExecutors, testSpinUntilFutureCompleteWithTimeout)
   spinner.join();
 }
 
-class TestWaitable : public rclcpp::Waitable
-{
-public:
-  TestWaitable() = default;
-
-  void
-  add_to_wait_set(rcl_wait_set_t & wait_set) override
-  {
-    if (trigger_count_ > 0) {
-      // Keep the gc triggered until the trigger count is reduced back to zero.
-      // This is necessary if trigger() results in the wait set waking, but not
-      // executing this waitable, in which case it needs to be re-triggered.
-      gc_.trigger();
-    }
-    rclcpp::detail::add_guard_condition_to_rcl_wait_set(wait_set, gc_);
-  }
-
-  void trigger()
-  {
-    trigger_count_++;
-    gc_.trigger();
-  }
-
-  bool
-  is_ready(const rcl_wait_set_t & wait_set) override
-  {
-    is_ready_count_++;
-    for (size_t i = 0; i < wait_set.size_of_guard_conditions; ++i) {
-      auto rcl_guard_condition = wait_set.guard_conditions[i];
-      if (&gc_.get_rcl_guard_condition() == rcl_guard_condition) {
-        return true;
-      }
-    }
-    return false;
-  }
-
-  std::shared_ptr<void>
-  take_data() override
-  {
-    return nullptr;
-  }
-
-  std::shared_ptr<void>
-  take_data_by_entity_id(size_t id) override
-  {
-    (void) id;
-    return nullptr;
-  }
-
-  void
-  execute(const std::shared_ptr<void> &) override
-  {
-    trigger_count_--;
-    count_++;
-    if (nullptr != on_execute_callback_) {
-      on_execute_callback_();
-    } else {
-      // TODO(wjwwood): I don't know why this was here, but probably it should
-      //   not be there, or test cases where that is important should use the
-      //   on_execute_callback?
-      std::this_thread::sleep_for(3ms);
-    }
-  }
-
-  void
-  set_on_execute_callback(std::function<void()> on_execute_callback)
-  {
-    on_execute_callback_ = on_execute_callback;
-  }
-
-  void
-  set_on_ready_callback(std::function<void(size_t, int)> callback) override
-  {
-    auto gc_callback = [callback](size_t count) {
-        callback(count, 0);
-      };
-    gc_.set_on_trigger_callback(gc_callback);
-  }
-
-  void
-  clear_on_ready_callback() override
-  {
-    gc_.set_on_trigger_callback(nullptr);
-  }
-
-  size_t
-  get_number_of_ready_guard_conditions() override {return 1;}
-
-  size_t
-  get_count() const
-  {
-    return count_;
-  }
-
-  size_t
-  get_is_ready_call_count() const
-  {
-    return is_ready_count_;
-  }
-
-private:
-  std::atomic<size_t> trigger_count_ = 0;
-  std::atomic<size_t> is_ready_count_ = 0;
-  std::atomic<size_t> count_ = 0;
-  rclcpp::GuardCondition gc_;
-  std::function<void()> on_execute_callback_ = nullptr;
-};
-
 TYPED_TEST(TestExecutors, spinAll)
 {
   using ExecutorType = TypeParam;
@@ -744,7 +638,7 @@ TYPED_TEST(TestExecutors, testSpinUntilFutureCompleteInterrupted)
 // and b) refreshing the executor collections.
 // The inconsistent state would happen if the event was processed before the collections were
 // finished to be refreshed: the executor would pick up the event but be unable to process it.
-// This would leave the `notify_waitable_event_pushed_` flag to true, preventing additional
+// This would leave the `entities_need_rebuild_` flag to true, preventing additional
 // notify waitable events to be pushed.
 // The behavior is observable only under heavy load, so this test spawns several worker
 // threads. Due to the nature of the bug, this test may still succeed even if the
@@ -807,6 +701,67 @@ TYPED_TEST(TestExecutors, testRaceConditionAddNode)
   }
 }
 
+// Check that executors are correctly notified while they are spinning
+// we notify twice to ensure that the notify waitable is still working
+// after the first notification
+TYPED_TEST(TestExecutors, notifyTwiceWhileSpinning)
+{
+  using ExecutorType = TypeParam;
+
+  // Create executor, add the node and start spinning
+  ExecutorType executor;
+  executor.add_node(this->node);
+  std::thread spinner([&]() {executor.spin();});
+
+  // Wait for executor to be spinning
+  while (!executor.is_spinning()) {
+    std::this_thread::sleep_for(std::chrono::milliseconds(1));
+  }
+
+  // Create the first subscription while the executor is already spinning
+  std::atomic<size_t> sub1_msg_count {0};
+  auto sub1 = this->node->template create_subscription<test_msgs::msg::Empty>(
+    this->publisher->get_topic_name(),
+    rclcpp::QoS(10),
+    [&sub1_msg_count](test_msgs::msg::Empty::ConstSharedPtr) {
+      sub1_msg_count++;
+    });
+
+  // Publish a message and verify it's received
+  this->publisher->publish(test_msgs::msg::Empty());
+  auto start = std::chrono::steady_clock::now();
+  while (sub1_msg_count == 0 && (std::chrono::steady_clock::now() - start) < 10s) {
+    std::this_thread::sleep_for(1ms);
+  }
+  EXPECT_EQ(sub1_msg_count, 1u);
+
+  // Create a second subscription while the executor is already spinning
+  std::atomic<size_t> sub2_msg_count {0};
+  auto sub2 = this->node->template create_subscription<test_msgs::msg::Empty>(
+    this->publisher->get_topic_name(),
+    rclcpp::QoS(10),
+    [&sub2_msg_count](test_msgs::msg::Empty::ConstSharedPtr) {
+      sub2_msg_count++;
+    });
+
+  // Publish a message and verify it's received by both subscriptions
+  this->publisher->publish(test_msgs::msg::Empty());
+  start = std::chrono::steady_clock::now();
+  while (
+    sub1_msg_count == 1 &&
+    sub2_msg_count == 0 &&
+    (std::chrono::steady_clock::now() - start) < 10s)
+  {
+    std::this_thread::sleep_for(1ms);
+  }
+  EXPECT_EQ(sub1_msg_count, 2u);
+  EXPECT_EQ(sub2_msg_count, 1u);
+
+  // Cancel needs to be called before join, so that executor.spin() returns.
+  executor.cancel();
+  spinner.join();
+}
+
 // Check spin_until_future_complete with node base pointer (instantiates its own executor)
 TEST(TestExecutors, testSpinUntilFutureCompleteNodeBasePtr)
 {
@@ -878,154 +833,26 @@ TEST(TestExecutors, testSpinWithNonDefaultContext)
   rclcpp::shutdown(non_default_context);
 }
 
-template<typename T>
-class TestBusyWaiting : public ::testing::Test
+TYPED_TEST(TestExecutors, release_ownership_entity_after_spinning_cancel)
 {
-public:
-  void SetUp() override
-  {
-    rclcpp::init(0, nullptr);
+  using ExecutorType = TypeParam;
+  ExecutorType executor;
 
-    const auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
-    std::stringstream test_name;
-    test_name << test_info->test_case_name() << "_" << test_info->name();
-    node = std::make_shared<rclcpp::Node>("node", test_name.str());
-    callback_group = node->create_callback_group(
-      rclcpp::CallbackGroupType::MutuallyExclusive,
-      /* automatically_add_to_executor_with_node =*/ false);
+  auto future = std::async(std::launch::async, [&executor] {executor.spin();});
 
-    auto waitable_interfaces = node->get_node_waitables_interface();
-    waitable = std::make_shared<TestWaitable>();
-    waitable_interfaces->add_waitable(waitable, callback_group);
-
-    executor = std::make_shared<T>();
-    executor->add_callback_group(callback_group, node->get_node_base_interface());
+  auto node = std::make_shared<rclcpp::Node>("test_node");
+  auto callback = [](
+    const test_msgs::srv::Empty::Request::SharedPtr, test_msgs::srv::Empty::Response::SharedPtr) {
+    };
+  auto server = node->create_service<test_msgs::srv::Empty>("test_service", callback);
+  while (!executor.is_spinning()) {
+    std::this_thread::sleep_for(50ms);
   }
+  executor.add_node(node);
+  std::this_thread::sleep_for(50ms);
+  executor.cancel();
+  std::future_status future_status = future.wait_for(1s);
+  EXPECT_EQ(future_status, std::future_status::ready);
 
-  void TearDown() override
-  {
-    rclcpp::shutdown();
-  }
-
-  void
-  set_up_and_trigger_waitable(std::function<void()> extra_callback = nullptr)
-  {
-    this->has_executed = false;
-    this->waitable->set_on_execute_callback([this, extra_callback]() {
-        if (!this->has_executed) {
-        // trigger once to see if the second trigger is handled or not
-        // this follow up trigger simulates new entities becoming ready while
-        // the executor is executing something else, e.g. subscription got data
-        // or a timer expired, etc.
-        // spin_some would not handle this second trigger, since it collects
-        // work only once, whereas spin_all should handle it since it
-        // collects work multiple times
-          this->waitable->trigger();
-          this->has_executed = true;
-        }
-        if (nullptr != extra_callback) {
-          extra_callback();
-        }
-    });
-    this->waitable->trigger();
-  }
-
-  void
-  check_for_busy_waits(std::chrono::steady_clock::time_point start_time)
-  {
-    // rough time based check, since the work to be done was very small it
-    // should be safe to check that we didn't use more than half the
-    // max duration, which itself is much larger than necessary
-    // however, it could still produce a false-positive
-    EXPECT_LT(
-      std::chrono::steady_clock::now() - start_time,
-      max_duration / 2)
-      << "executor took a long time to execute when it should have done "
-      << "nothing and should not have blocked either, but this could be a "
-      << "false negative if the computer is really slow";
-
-    // this check is making some assumptions about the implementation of the
-    // executors, but it should be safe to say that a busy wait may result in
-    // hundreds or thousands of calls to is_ready(), but "normal" executor
-    // behavior should be within an order of magnitude of the number of
-    // times that the waitable was executed
-    ASSERT_LT(waitable->get_is_ready_call_count(), 10u * this->waitable->get_count());
-  }
-
-  static constexpr auto max_duration = 10s;
-
-  rclcpp::Node::SharedPtr node;
-  rclcpp::CallbackGroup::SharedPtr callback_group;
-  std::shared_ptr<TestWaitable> waitable;
-  std::chrono::steady_clock::time_point start_time;
-  std::shared_ptr<T> executor;
-  bool has_executed;
-};
-
-TYPED_TEST_SUITE(TestBusyWaiting, ExecutorTypes, ExecutorTypeNames);
-
-TYPED_TEST(TestBusyWaiting, test_spin_all)
-{
-  this->set_up_and_trigger_waitable();
-
-  auto start_time = std::chrono::steady_clock::now();
-  this->executor->spin_all(this->max_duration);
-  this->check_for_busy_waits(start_time);
-  // this should get the initial trigger, and the follow up from in the callback
-  ASSERT_EQ(this->waitable->get_count(), 2u);
-}
-
-TYPED_TEST(TestBusyWaiting, test_spin_some)
-{
-  this->set_up_and_trigger_waitable();
-
-  auto start_time = std::chrono::steady_clock::now();
-  this->executor->spin_some(this->max_duration);
-  this->check_for_busy_waits(start_time);
-  // this should get the inital trigger, but not the follow up in the callback
-  ASSERT_EQ(this->waitable->get_count(), 1u);
-}
-
-TYPED_TEST(TestBusyWaiting, test_spin)
-{
-  std::condition_variable cv;
-  std::mutex cv_m;
-  bool first_check_passed = false;
-
-  this->set_up_and_trigger_waitable([&cv, &cv_m, &first_check_passed]() {
-      cv.notify_one();
-      if (!first_check_passed) {
-        std::unique_lock<std::mutex> lk(cv_m);
-        cv.wait_for(lk, 1s, [&]() {return first_check_passed;});
-      }
-  });
-
-  auto start_time = std::chrono::steady_clock::now();
-  std::thread t([this]() {
-      this->executor->spin();
-    });
-
-  // wait until thread has started (first execute of waitable)
-  {
-    std::unique_lock<std::mutex> lk(cv_m);
-    cv.wait_for(lk, 10s);
-  }
-  EXPECT_GT(this->waitable->get_count(), 0u);
-
-  first_check_passed = true;
-  cv.notify_one();
-
-  // wait until the executor has finished (second execute of waitable)
-  {
-    std::unique_lock<std::mutex> lk(cv_m);
-    cv.wait_for(lk, 10s);
-  }
-  EXPECT_EQ(this->waitable->get_count(), 2u);
-
-  this->executor->cancel();
-  t.join();
-
-  this->check_for_busy_waits(start_time);
-  // this should get the initial trigger, and the follow up from in the callback
-  ASSERT_EQ(this->waitable->get_count(), 2u);
+  EXPECT_EQ(server.use_count(), 1);
 }

rclcpp/test/rclcpp/executors/test_executors_busy_waiting.cpp

@@ -0,0 +1,181 @@
+// Copyright 2024 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gtest/gtest.h>
+
+#include <chrono>
+#include <functional>
+#include <memory>
+#include <mutex>
+#include <sstream>
+#include <thread>
+
+#include "rclcpp/rclcpp.hpp"
+
+#include "./executor_types.hpp"
+#include "./test_waitable.hpp"
+
+using namespace std::chrono_literals;
+
+template<typename T>
+class TestBusyWaiting : public ::testing::Test
+{
+public:
+  void SetUp() override
+  {
+    rclcpp::init(0, nullptr);
+
+    const auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
+    std::stringstream test_name;
+    test_name << test_info->test_case_name() << "_" << test_info->name();
+    node = std::make_shared<rclcpp::Node>("node", test_name.str());
+    callback_group = node->create_callback_group(
+      rclcpp::CallbackGroupType::MutuallyExclusive,
+      /* automatically_add_to_executor_with_node =*/ false);
+
+    auto waitable_interfaces = node->get_node_waitables_interface();
+    waitable = std::make_shared<TestWaitable>();
+    waitable_interfaces->add_waitable(waitable, callback_group);
+
+    executor = std::make_shared<T>();
+    executor->add_callback_group(callback_group, node->get_node_base_interface());
+  }
+
+  void TearDown() override
+  {
+    rclcpp::shutdown();
+  }
+
+  void
+  set_up_and_trigger_waitable(std::function<void()> extra_callback = nullptr)
+  {
+    this->has_executed = false;
+    this->waitable->set_on_execute_callback([this, extra_callback]() {
+        if (!this->has_executed) {
+        // trigger once to see if the second trigger is handled or not
+        // this follow up trigger simulates new entities becoming ready while
+        // the executor is executing something else, e.g. subscription got data
+        // or a timer expired, etc.
+        // spin_some would not handle this second trigger, since it collects
+        // work only once, whereas spin_all should handle it since it
+        // collects work multiple times
+          this->waitable->trigger();
+          this->has_executed = true;
+        }
+        if (nullptr != extra_callback) {
+          extra_callback();
+        }
+    });
+    this->waitable->trigger();
+  }
+
+  void
+  check_for_busy_waits(std::chrono::steady_clock::time_point start_time)
+  {
+    // rough time based check, since the work to be done was very small it
+    // should be safe to check that we didn't use more than half the
+    // max duration, which itself is much larger than necessary
+    // however, it could still produce a false-positive
+    EXPECT_LT(
+      std::chrono::steady_clock::now() - start_time,
+      max_duration / 2)
+      << "executor took a long time to execute when it should have done "
+      << "nothing and should not have blocked either, but this could be a "
+      << "false negative if the computer is really slow";
+
+    // this check is making some assumptions about the implementation of the
+    // executors, but it should be safe to say that a busy wait may result in
+    // hundreds or thousands of calls to is_ready(), but "normal" executor
+    // behavior should be within an order of magnitude of the number of
+    // times that the waitable was executed
+    ASSERT_LT(waitable->get_is_ready_call_count(), 10u * this->waitable->get_count());
+  }
+
+  static constexpr auto max_duration = 10s;
+
+  rclcpp::Node::SharedPtr node;
+  rclcpp::CallbackGroup::SharedPtr callback_group;
+  std::shared_ptr<TestWaitable> waitable;
+  std::chrono::steady_clock::time_point start_time;
+  std::shared_ptr<T> executor;
+  bool has_executed;
+};
+
+TYPED_TEST_SUITE(TestBusyWaiting, ExecutorTypes, ExecutorTypeNames);
+
+TYPED_TEST(TestBusyWaiting, test_spin_all)
+{
+  this->set_up_and_trigger_waitable();
+
+  auto start_time = std::chrono::steady_clock::now();
+  this->executor->spin_all(this->max_duration);
+  this->check_for_busy_waits(start_time);
+  // this should get the initial trigger, and the follow up from in the callback
+  ASSERT_EQ(this->waitable->get_count(), 2u);
+}
+
+TYPED_TEST(TestBusyWaiting, test_spin_some)
+{
+  this->set_up_and_trigger_waitable();
+
+  auto start_time = std::chrono::steady_clock::now();
+  this->executor->spin_some(this->max_duration);
+  this->check_for_busy_waits(start_time);
+  // this should get the inital trigger, but not the follow up in the callback
+  ASSERT_EQ(this->waitable->get_count(), 1u);
+}
+
+TYPED_TEST(TestBusyWaiting, test_spin)
+{
+  std::condition_variable cv;
+  std::mutex cv_m;
+  bool first_check_passed = false;
+
+  this->set_up_and_trigger_waitable([&cv, &cv_m, &first_check_passed]() {
+      cv.notify_one();
+      if (!first_check_passed) {
+        std::unique_lock<std::mutex> lk(cv_m);
+        cv.wait_for(lk, 1s, [&]() {return first_check_passed;});
+      }
+  });
+
+  auto start_time = std::chrono::steady_clock::now();
+  std::thread t([this]() {
+      this->executor->spin();
+    });
+
+  // wait until thread has started (first execute of waitable)
+  {
+    std::unique_lock<std::mutex> lk(cv_m);
+    cv.wait_for(lk, 10s);
+  }
+  EXPECT_GT(this->waitable->get_count(), 0u);
+
+  first_check_passed = true;
+  cv.notify_one();
+
+  // wait until the executor has finished (second execute of waitable)
+  {
+    std::unique_lock<std::mutex> lk(cv_m);
+    cv.wait_for(lk, 10s);
+  }
+  EXPECT_EQ(this->waitable->get_count(), 2u);
+
+  this->executor->cancel();
+  t.join();
+
+  this->check_for_busy_waits(start_time);
+  // this should get the initial trigger, and the follow up from in the callback
+  ASSERT_EQ(this->waitable->get_count(), 2u);
+}

rclcpp/test/rclcpp/executors/test_static_single_threaded_executor.cpp

@@ -56,7 +56,7 @@ TEST_F(TestStaticSingleThreadedExecutor, add_callback_group_trigger_guard_failed
       "lib:rclcpp", rcl_trigger_guard_condition, RCL_RET_ERROR);
     RCLCPP_EXPECT_THROW_EQ(
       executor.add_callback_group(cb_group, node->get_node_base_interface(), true),
-      std::runtime_error("Failed to trigger guard condition on callback group add: error not set"));
+      std::runtime_error("Failed to handle entities update on callback group add: error not set"));
   }
 }
 
@@ -69,7 +69,7 @@ TEST_F(TestStaticSingleThreadedExecutor, add_node_trigger_guard_failed) {
       "lib:rclcpp", rcl_trigger_guard_condition, RCL_RET_ERROR);
     RCLCPP_EXPECT_THROW_EQ(
       executor.add_node(node),
-      std::runtime_error("Failed to trigger guard condition on node add: error not set"));
+      std::runtime_error("Failed to handle entities update on node add: error not set"));
   }
 }
 
@@ -87,7 +87,7 @@ TEST_F(TestStaticSingleThreadedExecutor, remove_callback_group_trigger_guard_fai
     RCLCPP_EXPECT_THROW_EQ(
       executor.remove_callback_group(cb_group, true),
       std::runtime_error(
-        "Failed to trigger guard condition on callback group remove: error not set"));
+        "Failed to handle entities update on callback group remove: error not set"));
   }
 }
 
@@ -115,7 +115,7 @@ TEST_F(TestStaticSingleThreadedExecutor, remove_node_trigger_guard_failed) {
       "lib:rclcpp", rcl_trigger_guard_condition, RCL_RET_ERROR);
     RCLCPP_EXPECT_THROW_EQ(
       executor.remove_node(node, true),
-      std::runtime_error("Failed to trigger guard condition on node remove: error not set"));
+      std::runtime_error("Failed to handle entities update on node remove: error not set"));
   }
 }
 

rclcpp/test/rclcpp/executors/test_waitable.cpp

@@ -0,0 +1,125 @@
+// Copyright 2024 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <atomic>
+#include <functional>
+#include <memory>
+#include <thread>
+
+#include "rclcpp/detail/add_guard_condition_to_rcl_wait_set.hpp"
+#include "rclcpp/waitable.hpp"
+
+#include "rcl/wait.h"
+
+#include "test_waitable.hpp"
+
+using namespace std::chrono_literals;
+
+void
+TestWaitable::add_to_wait_set(rcl_wait_set_t & wait_set)
+{
+  if (trigger_count_ > 0) {
+    // Keep the gc triggered until the trigger count is reduced back to zero.
+    // This is necessary if trigger() results in the wait set waking, but not
+    // executing this waitable, in which case it needs to be re-triggered.
+    gc_.trigger();
+  }
+  rclcpp::detail::add_guard_condition_to_rcl_wait_set(wait_set, gc_);
+}
+
+void TestWaitable::trigger()
+{
+  trigger_count_++;
+  gc_.trigger();
+}
+
+bool
+TestWaitable::is_ready(const rcl_wait_set_t & wait_set)
+{
+  is_ready_count_++;
+  for (size_t i = 0; i < wait_set.size_of_guard_conditions; ++i) {
+    auto rcl_guard_condition = wait_set.guard_conditions[i];
+    if (&gc_.get_rcl_guard_condition() == rcl_guard_condition) {
+      return true;
+    }
+  }
+  return false;
+}
+
+std::shared_ptr<void>
+TestWaitable::take_data()
+{
+  return nullptr;
+}
+
+std::shared_ptr<void>
+TestWaitable::take_data_by_entity_id(size_t id)
+{
+  (void) id;
+  return nullptr;
+}
+
+void
+TestWaitable::execute(const std::shared_ptr<void> &)
+{
+  trigger_count_--;
+  count_++;
+  if (nullptr != on_execute_callback_) {
+    on_execute_callback_();
+  } else {
+    // TODO(wjwwood): I don't know why this was here, but probably it should
+    //   not be there, or test cases where that is important should use the
+    //   on_execute_callback?
+    std::this_thread::sleep_for(3ms);
+  }
+}
+
+void
+TestWaitable::set_on_execute_callback(std::function<void()> on_execute_callback)
+{
+  on_execute_callback_ = on_execute_callback;
+}
+
+void
+TestWaitable::set_on_ready_callback(std::function<void(size_t, int)> callback)
+{
+  auto gc_callback = [callback](size_t count) {
+      callback(count, 0);
+    };
+  gc_.set_on_trigger_callback(gc_callback);
+}
+
+void
+TestWaitable::clear_on_ready_callback()
+{
+  gc_.set_on_trigger_callback(nullptr);
+}
+
+size_t
+TestWaitable::get_number_of_ready_guard_conditions()
+{
+  return 1;
+}
+
+size_t
+TestWaitable::get_count() const
+{
+  return count_;
+}
+
+size_t
+TestWaitable::get_is_ready_call_count() const
+{
+  return is_ready_count_;
+}

rclcpp/test/rclcpp/executors/test_waitable.hpp

@@ -0,0 +1,75 @@
+// Copyright 2024 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef RCLCPP__EXECUTORS__TEST_WAITABLE_HPP_
+#define RCLCPP__EXECUTORS__TEST_WAITABLE_HPP_
+
+#include <atomic>
+#include <functional>
+#include <memory>
+
+#include "rclcpp/waitable.hpp"
+#include "rclcpp/guard_condition.hpp"
+
+#include "rcl/wait.h"
+
+class TestWaitable : public rclcpp::Waitable
+{
+public:
+  TestWaitable() = default;
+
+  void
+  add_to_wait_set(rcl_wait_set_t & wait_set) override;
+
+  void trigger();
+
+  bool
+  is_ready(const rcl_wait_set_t & wait_set) override;
+
+  std::shared_ptr<void>
+  take_data() override;
+
+  std::shared_ptr<void>
+  take_data_by_entity_id(size_t id) override;
+
+  void
+  execute(const std::shared_ptr<void> &) override;
+
+  void
+  set_on_execute_callback(std::function<void()> on_execute_callback);
+
+  void
+  set_on_ready_callback(std::function<void(size_t, int)> callback) override;
+
+  void
+  clear_on_ready_callback() override;
+
+  size_t
+  get_number_of_ready_guard_conditions() override;
+
+  size_t
+  get_count() const;
+
+  size_t
+  get_is_ready_call_count() const;
+
+private:
+  std::atomic<size_t> trigger_count_ = 0;
+  std::atomic<size_t> is_ready_count_ = 0;
+  std::atomic<size_t> count_ = 0;
+  rclcpp::GuardCondition gc_;
+  std::function<void()> on_execute_callback_ = nullptr;
+};
+
+#endif  // RCLCPP__EXECUTORS__TEST_WAITABLE_HPP_

rclcpp/test/rclcpp/node_interfaces/test_node_parameters.cpp

@@ -21,6 +21,7 @@
 #include <gtest/gtest.h>
 
 #include <algorithm>
+#include <filesystem>
 #include <memory>
 #include <string>
 #include <vector>
@@ -61,7 +62,7 @@ protected:
   std::shared_ptr<rclcpp::Node> node;
   rclcpp::node_interfaces::NodeParameters * node_parameters;
 
-  rcpputils::fs::path test_resources_path{TEST_RESOURCES_DIRECTORY};
+  std::filesystem::path test_resources_path{TEST_RESOURCES_DIRECTORY};
 };
 
 TEST_F(TestNodeParameters, construct_destruct_rcl_errors) {

rclcpp/test/rclcpp/test_add_callback_groups_to_executor.cpp

@@ -23,7 +23,6 @@
 
 #include "rclcpp/node.hpp"
 #include "test_msgs/msg/empty.hpp"
-#include "test_msgs/msg/empty.h"
 
 #include "rclcpp/exceptions.hpp"
 #include "rclcpp/executors.hpp"
@@ -216,7 +215,6 @@ TYPED_TEST(TestAddCallbackGroupsToExecutor, add_callback_groups_after_add_node_t
   std::atomic_size_t timer_count {0};
   auto timer_callback = [&executor, &timer_count]() {
       auto cur_timer_count = timer_count++;
-      printf("in timer_callback(%zu)\n", cur_timer_count);
       if (cur_timer_count > 0) {
         executor.cancel();
       }
@@ -345,32 +343,30 @@ TYPED_TEST(TestAddCallbackGroupsToExecutorStable, subscriber_triggered_to_receiv
       received_message_promise.set_value(true);
     };
 
-  rclcpp::Subscription<test_msgs::msg::Empty>::SharedPtr subscription;
-  rclcpp::Publisher<test_msgs::msg::Empty>::SharedPtr publisher;
-  // to create a timer with a callback run on another executor
-  rclcpp::TimerBase::SharedPtr timer = nullptr;
   std::promise<void> timer_promise;
+  // create a subscription using the 'cb_grp' callback group
+  rclcpp::QoS qos = rclcpp::QoS(1).reliable();
+  auto options = rclcpp::SubscriptionOptions();
+  options.callback_group = cb_grp;
+  rclcpp::Subscription<test_msgs::msg::Empty>::SharedPtr subscription =
+    node->create_subscription<test_msgs::msg::Empty>("topic_name", qos, sub_callback, options);
+  // create a publisher to send data
+  rclcpp::Publisher<test_msgs::msg::Empty>::SharedPtr publisher =
+    node->create_publisher<test_msgs::msg::Empty>("topic_name", qos);
   auto timer_callback =
-    [&subscription, &publisher, &timer, &cb_grp, &node, &sub_callback, &timer_promise]() {
-      if (timer) {
-        timer.reset();
+    [&publisher, &timer_promise]() {
+      if (publisher->get_subscription_count() == 0) {
+        // If discovery hasn't happened yet, get out.
+        return;
       }
-
-      // create a subscription using the `cb_grp` callback group
-      rclcpp::QoS qos = rclcpp::QoS(1).reliable();
-      auto options = rclcpp::SubscriptionOptions();
-      options.callback_group = cb_grp;
-      subscription =
-        node->create_subscription<test_msgs::msg::Empty>("topic_name", qos, sub_callback, options);
-      // create a publisher to send data
-      publisher =
-        node->create_publisher<test_msgs::msg::Empty>("topic_name", qos);
       publisher->publish(test_msgs::msg::Empty());
       timer_promise.set_value();
     };
 
+  // Another executor to run the timer with a callback
   ExecutorType timer_executor;
-  timer = node->create_wall_timer(100ms, timer_callback);
+
+  rclcpp::TimerBase::SharedPtr timer = node->create_wall_timer(100ms, timer_callback);
   timer_executor.add_node(node);
   auto future = timer_promise.get_future();
   timer_executor.spin_until_future_complete(future);

rclcpp/test/rclcpp/test_any_service_callback.cpp

@@ -24,7 +24,6 @@
 #include "rclcpp/any_service_callback.hpp"
 #include "rclcpp/service.hpp"
 #include "test_msgs/srv/empty.hpp"
-#include "test_msgs/srv/empty.h"
 
 class TestAnyServiceCallback : public ::testing::Test
 {

rclcpp/test/rclcpp/test_any_subscription_callback.cpp

@@ -21,7 +21,6 @@
 
 #include "rclcpp/any_subscription_callback.hpp"
 #include "test_msgs/msg/empty.hpp"
-#include "test_msgs/msg/empty.h"
 
 // Type adapter to be used in tests.
 struct MyEmpty {};

rclcpp/test/rclcpp/test_client.cpp

@@ -89,31 +89,10 @@ TEST_F(TestClient, construction_and_destruction) {
   {
     auto client = node->create_client<ListParameters>("service");
   }
-  {
-    // suppress deprecated function warning
-    #if !defined(_WIN32)
-    # pragma GCC diagnostic push
-    # pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-    #else  // !defined(_WIN32)
-    # pragma warning(push)
-    # pragma warning(disable: 4996)
-    #endif
-
-    auto client = node->create_client<ListParameters>(
-      "service", rmw_qos_profile_services_default);
-
-    // remove warning suppression
-    #if !defined(_WIN32)
-    # pragma GCC diagnostic pop
-    #else  // !defined(_WIN32)
-    # pragma warning(pop)
-    #endif
-  }
   {
     auto client = node->create_client<ListParameters>(
       "service", rclcpp::ServicesQoS());
   }
-
   {
     ASSERT_THROW(
     {
@@ -123,27 +102,6 @@ TEST_F(TestClient, construction_and_destruction) {
 }
 
 TEST_F(TestClient, construction_with_free_function) {
-  {
-    auto client = rclcpp::create_client<rcl_interfaces::srv::ListParameters>(
-      node->get_node_base_interface(),
-      node->get_node_graph_interface(),
-      node->get_node_services_interface(),
-      "service",
-      rmw_qos_profile_services_default,
-      nullptr);
-  }
-  {
-    ASSERT_THROW(
-    {
-      auto client = rclcpp::create_client<rcl_interfaces::srv::ListParameters>(
-        node->get_node_base_interface(),
-        node->get_node_graph_interface(),
-        node->get_node_services_interface(),
-        "invalid_?service",
-        rmw_qos_profile_services_default,
-        nullptr);
-    }, rclcpp::exceptions::InvalidServiceNameError);
-  }
   {
     auto client = rclcpp::create_client<rcl_interfaces::srv::ListParameters>(
       node->get_node_base_interface(),

rclcpp/test/rclcpp/test_create_subscription.cpp

@@ -20,7 +20,6 @@
 #include "rclcpp/create_subscription.hpp"
 #include "rclcpp/node.hpp"
 #include "test_msgs/msg/empty.hpp"
-#include "test_msgs/msg/empty.h"
 
 using namespace std::chrono_literals;
 

rclcpp/test/rclcpp/test_executor.cpp

@@ -138,7 +138,7 @@ TEST_F(TestExecutor, add_callback_group_failed_trigger_guard_condition) {
     "lib:rclcpp", rcl_trigger_guard_condition, RCL_RET_ERROR);
   RCLCPP_EXPECT_THROW_EQ(
     dummy.add_callback_group(cb_group, node->get_node_base_interface(), true),
-    std::runtime_error("Failed to trigger guard condition on callback group add: error not set"));
+    std::runtime_error("Failed to handle entities update on callback group add: error not set"));
 }
 
 TEST_F(TestExecutor, remove_callback_group_null_node) {
@@ -175,7 +175,7 @@ TEST_F(TestExecutor, remove_callback_group_failed_trigger_guard_condition) {
   RCLCPP_EXPECT_THROW_EQ(
     dummy.remove_callback_group(cb_group, true),
     std::runtime_error(
-      "Failed to trigger guard condition on callback group remove: error not set"));
+      "Failed to handle entities update on callback group remove: error not set"));
 }
 
 TEST_F(TestExecutor, remove_node_not_associated) {

rclcpp/test/rclcpp/test_loaned_message.cpp

@@ -40,53 +40,6 @@ protected:
   }
 };
 
-TEST_F(TestLoanedMessage, initialize) {
-  auto node = std::make_shared<rclcpp::Node>("loaned_message_test_node");
-  auto pub = node->create_publisher<MessageT>("loaned_message_test_topic", 1);
-
-// suppress deprecated function warning
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-
-  auto pub_allocator = pub->get_allocator();
-
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
-
-// suppress deprecated function warning
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-
-  auto loaned_msg = rclcpp::LoanedMessage<MessageT>(pub.get(), pub_allocator);
-
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
-
-  ASSERT_TRUE(loaned_msg.is_valid());
-  loaned_msg.get().float32_value = 42.0f;
-  ASSERT_EQ(42.0f, loaned_msg.get().float32_value);
-
-  SUCCEED();
-}
-
 TEST_F(TestLoanedMessage, loan_from_pub) {
   auto node = std::make_shared<rclcpp::Node>("loaned_message_test_node");
   auto pub = node->create_publisher<MessageT>("loaned_message_test_topic", 1);

rclcpp/test/rclcpp/test_logger.cpp

@@ -14,6 +14,7 @@
 
 #include <gtest/gtest.h>
 
+#include <filesystem>
 #include <memory>
 #include <string>
 
@@ -210,15 +211,7 @@ TEST(TestLogger, get_logging_directory) {
   ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", nullptr));
   ASSERT_EQ(true, rcutils_set_env("ROS_HOME", nullptr));
 
-  auto path = rclcpp::get_logging_directory();
-  auto expected_path = rcpputils::fs::path{"/fake_home_dir"} / ".ros" / "log";
-
-  // TODO(ivanpauno): Add operator== to rcpputils::fs::path
-  auto it = path.cbegin();
-  auto eit = expected_path.cbegin();
-  for (; it != path.cend() && eit != expected_path.cend(); ++it, ++eit) {
-    EXPECT_EQ(*eit, *it);
-  }
-  EXPECT_EQ(it, path.cend());
-  EXPECT_EQ(eit, expected_path.cend());
+  auto path = rclcpp::get_log_directory();
+  auto expected_path = std::filesystem::path{"/fake_home_dir"} / ".ros" / "log";
+  EXPECT_EQ(path, expected_path);
 }

rclcpp/test/rclcpp/test_node.cpp

@@ -15,6 +15,7 @@
 #include <gtest/gtest.h>
 
 #include <chrono>
+#include <filesystem>
 #include <functional>
 #include <limits>
 #include <map>
@@ -56,7 +57,7 @@ protected:
     test_resources_path /= "test_node";
   }
 
-  rcpputils::fs::path test_resources_path{TEST_RESOURCES_DIRECTORY};
+  std::filesystem::path test_resources_path{TEST_RESOURCES_DIRECTORY};
 };
 
 /*

rclcpp/test/rclcpp/test_parameter_client.cpp

@@ -15,6 +15,7 @@
 #include <gmock/gmock.h>
 
 #include <chrono>
+#include <filesystem>
 #include <functional>
 #include <future>
 #include <memory>
@@ -935,7 +936,7 @@ TEST_F(TestParameterClient, async_parameter_load_parameters) {
   auto asynchronous_client =
     std::make_shared<rclcpp::AsyncParametersClient>(load_node, "/namespace/load_node");
   // load parameters
-  rcpputils::fs::path test_resources_path{TEST_RESOURCES_DIRECTORY};
+  std::filesystem::path test_resources_path{TEST_RESOURCES_DIRECTORY};
   const std::string parameters_filepath = (
     test_resources_path / "test_node" / "load_parameters.yaml").string();
   auto load_future = asynchronous_client->load_parameters(parameters_filepath);
@@ -961,7 +962,7 @@ TEST_F(TestParameterClient, sync_parameter_load_parameters) {
   auto synchronous_client =
     std::make_shared<rclcpp::SyncParametersClient>(load_node);
   // load parameters
-  rcpputils::fs::path test_resources_path{TEST_RESOURCES_DIRECTORY};
+  std::filesystem::path test_resources_path{TEST_RESOURCES_DIRECTORY};
   const std::string parameters_filepath = (
     test_resources_path / "test_node" / "load_parameters.yaml").string();
   auto load_future = synchronous_client->load_parameters(parameters_filepath);
@@ -983,7 +984,7 @@ TEST_F(TestParameterClient, async_parameter_load_parameters_complicated_regex) {
   auto asynchronous_client =
     std::make_shared<rclcpp::AsyncParametersClient>(load_node, "/namespace/load_node");
   // load parameters
-  rcpputils::fs::path test_resources_path{TEST_RESOURCES_DIRECTORY};
+  std::filesystem::path test_resources_path{TEST_RESOURCES_DIRECTORY};
   const std::string parameters_filepath = (
     test_resources_path / "test_node" / "load_complicated_parameters.yaml").string();
   auto load_future = asynchronous_client->load_parameters(parameters_filepath);
@@ -1013,7 +1014,7 @@ TEST_F(TestParameterClient, async_parameter_load_no_valid_parameter) {
   auto asynchronous_client =
     std::make_shared<rclcpp::AsyncParametersClient>(load_node, "/namespace/load_node");
   // load parameters
-  rcpputils::fs::path test_resources_path{TEST_RESOURCES_DIRECTORY};
+  std::filesystem::path test_resources_path{TEST_RESOURCES_DIRECTORY};
   const std::string parameters_filepath = (
     test_resources_path / "test_node" / "no_valid_parameters.yaml").string();
   EXPECT_THROW(

rclcpp/test/rclcpp/test_rate.cpp

@@ -47,21 +47,6 @@ TEST_F(TestRate, rate_basics) {
   auto start = std::chrono::system_clock::now();
   rclcpp::Rate r(period);
   EXPECT_EQ(rclcpp::Duration(period), r.period());
-// suppress deprecated warnings
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-  ASSERT_FALSE(r.is_steady());
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
   ASSERT_EQ(RCL_SYSTEM_TIME, r.get_type());
   ASSERT_TRUE(r.sleep());
   auto one = std::chrono::system_clock::now();
@@ -102,23 +87,6 @@ TEST_F(TestRate, wall_rate_basics) {
   auto start = std::chrono::steady_clock::now();
   rclcpp::WallRate r(period);
   EXPECT_EQ(rclcpp::Duration(period), r.period());
-// suppress deprecated warnings
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-  ASSERT_TRUE(r.is_steady());
-// suppress deprecated warnings
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
   ASSERT_EQ(RCL_STEADY_TIME, r.get_type());
   ASSERT_TRUE(r.sleep());
   auto one = std::chrono::steady_clock::now();
@@ -150,124 +118,6 @@ TEST_F(TestRate, wall_rate_basics) {
   EXPECT_GT(epsilon, delta);
 }
 
-/*
-   Basic test for the deprecated GenericRate class.
- */
-TEST_F(TestRate, generic_rate) {
-  auto period = std::chrono::milliseconds(100);
-  auto offset = std::chrono::milliseconds(50);
-  auto epsilon = std::chrono::milliseconds(1);
-  double overrun_ratio = 1.5;
-
-  {
-    auto start = std::chrono::system_clock::now();
-
-// suppress deprecated warnings
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-
-    rclcpp::GenericRate<std::chrono::system_clock> gr(period);
-    ASSERT_FALSE(gr.is_steady());
-
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
-
-    ASSERT_EQ(gr.get_type(), RCL_SYSTEM_TIME);
-    EXPECT_EQ(period, gr.period());
-    ASSERT_TRUE(gr.sleep());
-    auto one = std::chrono::system_clock::now();
-    auto delta = one - start;
-    EXPECT_LT(period, delta + epsilon);
-    EXPECT_GT(period * overrun_ratio, delta);
-
-    rclcpp::sleep_for(offset);
-    ASSERT_TRUE(gr.sleep());
-    auto two = std::chrono::system_clock::now();
-    delta = two - start;
-    EXPECT_LT(2 * period, delta);
-    EXPECT_GT(2 * period * overrun_ratio, delta);
-
-    rclcpp::sleep_for(offset);
-    auto two_offset = std::chrono::system_clock::now();
-    gr.reset();
-    ASSERT_TRUE(gr.sleep());
-    auto three = std::chrono::system_clock::now();
-    delta = three - two_offset;
-    EXPECT_LT(period, delta + epsilon);
-    EXPECT_GT(period * overrun_ratio, delta);
-
-    rclcpp::sleep_for(offset + period);
-    auto four = std::chrono::system_clock::now();
-    ASSERT_FALSE(gr.sleep());
-    auto five = std::chrono::system_clock::now();
-    delta = five - four;
-    ASSERT_TRUE(epsilon > delta);
-  }
-
-  {
-    auto start = std::chrono::steady_clock::now();
-
-// suppress deprecated warnings
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-
-    rclcpp::GenericRate<std::chrono::steady_clock> gr(period);
-    ASSERT_TRUE(gr.is_steady());
-
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
-
-    ASSERT_EQ(gr.get_type(), RCL_STEADY_TIME);
-    EXPECT_EQ(period, gr.period());
-    ASSERT_TRUE(gr.sleep());
-    auto one = std::chrono::steady_clock::now();
-    auto delta = one - start;
-    EXPECT_LT(period, delta);
-    EXPECT_GT(period * overrun_ratio, delta);
-
-    rclcpp::sleep_for(offset);
-    ASSERT_TRUE(gr.sleep());
-    auto two = std::chrono::steady_clock::now();
-    delta = two - start;
-    EXPECT_LT(2 * period, delta + epsilon);
-    EXPECT_GT(2 * period * overrun_ratio, delta);
-
-    rclcpp::sleep_for(offset);
-    auto two_offset = std::chrono::steady_clock::now();
-    gr.reset();
-    ASSERT_TRUE(gr.sleep());
-    auto three = std::chrono::steady_clock::now();
-    delta = three - two_offset;
-    EXPECT_LT(period, delta);
-    EXPECT_GT(period * overrun_ratio, delta);
-
-    rclcpp::sleep_for(offset + period);
-    auto four = std::chrono::steady_clock::now();
-    ASSERT_FALSE(gr.sleep());
-    auto five = std::chrono::steady_clock::now();
-    delta = five - four;
-    EXPECT_GT(epsilon, delta);
-  }
-}
-
 TEST_F(TestRate, from_double) {
   {
     rclcpp::Rate rate(1.0);
@@ -290,59 +140,14 @@ TEST_F(TestRate, from_double) {
 TEST_F(TestRate, clock_types) {
   {
     rclcpp::Rate rate(1.0, std::make_shared<rclcpp::Clock>(RCL_SYSTEM_TIME));
-// suppress deprecated warnings
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-    EXPECT_FALSE(rate.is_steady());
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
     EXPECT_EQ(RCL_SYSTEM_TIME, rate.get_type());
   }
   {
     rclcpp::Rate rate(1.0, std::make_shared<rclcpp::Clock>(RCL_STEADY_TIME));
-// suppress deprecated warnings
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-    EXPECT_TRUE(rate.is_steady());
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
     EXPECT_EQ(RCL_STEADY_TIME, rate.get_type());
   }
   {
     rclcpp::Rate rate(1.0, std::make_shared<rclcpp::Clock>(RCL_ROS_TIME));
-// suppress deprecated warnings
-#if !defined(_WIN32)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#else  // !defined(_WIN32)
-# pragma warning(push)
-# pragma warning(disable: 4996)
-#endif
-    EXPECT_FALSE(rate.is_steady());
-// remove warning suppression
-#if !defined(_WIN32)
-# pragma GCC diagnostic pop
-#else  // !defined(_WIN32)
-# pragma warning(pop)
-#endif
     EXPECT_EQ(RCL_ROS_TIME, rate.get_type());
   }
 }

rclcpp_action/CHANGELOG.rst

@@ -3,6 +3,14 @@ Changelog for package rclcpp_action
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+28.3.2 (2024-07-24)
+-------------------
+
+28.3.1 (2024-06-25)
+-------------------
+* Fix typo in function doc (`#2563 <https://github.com/ros2/rclcpp/issues/2563>`_)
+* Contributors: Christophe Bedard
+
 28.3.0 (2024-06-17)
 -------------------
 * Add 'mimick' label to tests which use Mimick (`#2516 <https://github.com/ros2/rclcpp/issues/2516>`_)

rclcpp_action/include/rclcpp_action/create_server.hpp

@@ -116,7 +116,7 @@ create_server(
  *
  * \sa Server::Server() for more information.
  *
- * \param[in] node] The action server will be added to this node.
+ * \param[in] node The action server will be added to this node.
  * \param[in] name The action name.
  * \param[in] handle_goal A callback that decides if a goal should be accepted or rejected.
  * \param[in] handle_cancel A callback that decides if a goal should be attempted to be canceled.

rclcpp_action/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp_action</name>
-  <version>28.3.0</version>
+  <version>28.3.2</version>
   <description>Adds action APIs for C++.</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp_components/CHANGELOG.rst

@@ -2,6 +2,15 @@
 Changelog for package rclcpp_components
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+28.3.2 (2024-07-24)
+-------------------
+* Updated rcpputils path API (`#2579 <https://github.com/ros2/rclcpp/issues/2579>`_)
+* remove deprecated APIs from component_manager.hpp (`#2585 <https://github.com/ros2/rclcpp/issues/2585>`_)
+* Contributors: Alberto Soragna, Alejandro Hernández Cordero
+
+28.3.1 (2024-06-25)
+-------------------
+
 28.3.0 (2024-06-17)
 -------------------
 

rclcpp_components/include/rclcpp_components/component_manager.hpp

@@ -185,20 +185,6 @@ protected:
     const std::shared_ptr<LoadNode::Request> request,
     std::shared_ptr<LoadNode::Response> response);
 
-  /**
-   * \deprecated Use on_load_node() instead
-   */
-  [[deprecated("Use on_load_node() instead")]]
-  RCLCPP_COMPONENTS_PUBLIC
-  virtual void
-  OnLoadNode(
-    const std::shared_ptr<rmw_request_id_t> request_header,
-    const std::shared_ptr<LoadNode::Request> request,
-    std::shared_ptr<LoadNode::Response> response)
-  {
-    on_load_node(request_header, request, response);
-  }
-
   /// Service callback to unload a node in the component
   /**
    * \param request_header unused
@@ -213,20 +199,6 @@ protected:
     const std::shared_ptr<UnloadNode::Request> request,
     std::shared_ptr<UnloadNode::Response> response);
 
-  /**
-   * \deprecated Use on_unload_node() instead
-   */
-  [[deprecated("Use on_unload_node() instead")]]
-  RCLCPP_COMPONENTS_PUBLIC
-  virtual void
-  OnUnloadNode(
-    const std::shared_ptr<rmw_request_id_t> request_header,
-    const std::shared_ptr<UnloadNode::Request> request,
-    std::shared_ptr<UnloadNode::Response> response)
-  {
-    on_unload_node(request_header, request, response);
-  }
-
   /// Service callback to get the list of nodes in the component
   /**
    * Return a two list: one with the unique identifiers and other with full name of the nodes.
@@ -242,20 +214,6 @@ protected:
     const std::shared_ptr<ListNodes::Request> request,
     std::shared_ptr<ListNodes::Response> response);
 
-  /**
-   * \deprecated Use on_list_nodes() instead
-   */
-  [[deprecated("Use on_list_nodes() instead")]]
-  RCLCPP_COMPONENTS_PUBLIC
-  virtual void
-  OnListNodes(
-    const std::shared_ptr<rmw_request_id_t> request_header,
-    const std::shared_ptr<ListNodes::Request> request,
-    std::shared_ptr<ListNodes::Response> response)
-  {
-    on_list_nodes(request_header, request, response);
-  }
-
 protected:
   std::weak_ptr<rclcpp::Executor> executor_;
 

rclcpp_components/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp_components</name>
-  <version>28.3.0</version>
+  <version>28.3.2</version>
   <description>Package containing tools for dynamically loadable components</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp_components/src/component_manager.cpp

@@ -14,6 +14,7 @@
 
 #include "rclcpp_components/component_manager.hpp"
 
+#include <filesystem>
 #include <functional>
 #include <memory>
 #include <string>
@@ -95,11 +96,11 @@ ComponentManager::get_component_resources(
       throw ComponentManagerException("Invalid resource entry");
     }
 
-    std::string library_path = parts[1];
-    if (!rcpputils::fs::path(library_path).is_absolute()) {
-      library_path = base_path + "/" + library_path;
+    std::filesystem::path library_path = parts[1];
+    if (!library_path.is_absolute()) {
+      library_path = (base_path / library_path);
     }
-    resources.push_back({parts[0], library_path});
+    resources.push_back({parts[0], library_path.string()});
   }
   return resources;
 }

rclcpp_components/test/test_component_manager.cpp

@@ -14,6 +14,7 @@
 
 #include <gtest/gtest.h>
 
+#include <filesystem>
 #include <memory>
 
 #include "rclcpp_components/component_manager.hpp"
@@ -51,7 +52,7 @@ TEST_F(TestComponentManager, get_component_resources_valid)
   EXPECT_EQ("test_rclcpp_components::TestComponentBar", resources[1].first);
   EXPECT_EQ("test_rclcpp_components::TestComponentNoNode", resources[2].first);
 
-  namespace fs = rcpputils::fs;
+  namespace fs = std::filesystem;
   EXPECT_TRUE(fs::exists(fs::path(resources[0].second)));
   EXPECT_TRUE(fs::exists(fs::path(resources[1].second)));
   EXPECT_TRUE(fs::exists(fs::path(resources[2].second)));

rclcpp_lifecycle/CHANGELOG.rst

@@ -3,6 +3,21 @@ Changelog for package rclcpp_lifecycle
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+28.3.2 (2024-07-24)
+-------------------
+* Removed deprecated methods and classes (`#2575 <https://github.com/ros2/rclcpp/issues/2575>`_)
+* Fix the lifecycle tests on RHEL-9. (`#2583 <https://github.com/ros2/rclcpp/issues/2583>`_)
+  * Fix the lifecycle tests on RHEL-9.
+  The full explanation is in the comment, but basically since
+  RHEL doesn't support mocking_utils::inject_on_return, we have
+  to split out certain tests to make sure resources within a
+  process don't collide.
+  Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
+* Contributors: Alejandro Hernández Cordero, Chris Lalancette
+
+28.3.1 (2024-06-25)
+-------------------
+
 28.3.0 (2024-06-17)
 -------------------
 * revert call shutdown in LifecycleNode destructor (`#2557 <https://github.com/ros2/rclcpp/issues/2557>`_)

rclcpp_lifecycle/CMakeLists.txt

@@ -99,6 +99,13 @@ if(BUILD_TESTING)
   if(TARGET test_lifecycle_node)
     target_link_libraries(test_lifecycle_node ${PROJECT_NAME} mimick rcl_lifecycle::rcl_lifecycle rclcpp::rclcpp rcutils::rcutils)
   endif()
+
+  ament_add_gtest(test_lifecycle_node_errors test/test_lifecycle_node_errors.cpp)
+  ament_add_test_label(test_lifecycle_node_errors mimick)
+  if(TARGET test_lifecycle_node_errors)
+    target_link_libraries(test_lifecycle_node_errors ${PROJECT_NAME} mimick rcl_lifecycle::rcl_lifecycle)
+  endif()
+
   ament_add_gtest(test_lifecycle_publisher test/test_lifecycle_publisher.cpp)
   if(TARGET test_lifecycle_publisher)
     target_link_libraries(test_lifecycle_publisher ${PROJECT_NAME} rcl_lifecycle::rcl_lifecycle rclcpp::rclcpp ${test_msgs_TARGETS})

rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp

@@ -280,19 +280,6 @@ public:
     CallbackT callback,
     rclcpp::CallbackGroup::SharedPtr group = nullptr);
 
-  /// Create and return a Client.
-  /**
-   * \sa rclcpp::Node::create_client
-   * \deprecated use rclcpp::QoS instead of rmw_qos_profile_t
-   */
-  template<typename ServiceT>
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  typename rclcpp::Client<ServiceT>::SharedPtr
-  create_client(
-    const std::string & service_name,
-    const rmw_qos_profile_t & qos_profile,
-    rclcpp::CallbackGroup::SharedPtr group = nullptr);
-
   /// Create and return a Client.
   /**
    * \param[in] service_name The name on which the service is accessible.
@@ -307,20 +294,6 @@ public:
     const rclcpp::QoS & qos = rclcpp::ServicesQoS(),
     rclcpp::CallbackGroup::SharedPtr group = nullptr);
 
-  /// Create and return a Service.
-  /**
-   * \sa rclcpp::Node::create_service
-   * \deprecated use rclcpp::QoS instead of rmw_qos_profile_t
-   */
-  template<typename ServiceT, typename CallbackT>
-  [[deprecated("use rclcpp::QoS instead of rmw_qos_profile_t")]]
-  typename rclcpp::Service<ServiceT>::SharedPtr
-  create_service(
-    const std::string & service_name,
-    CallbackT && callback,
-    const rmw_qos_profile_t & qos_profile,
-    rclcpp::CallbackGroup::SharedPtr group = nullptr);
-
   /// Create and return a Service.
   /**
    * \sa rclcpp::Node::create_service
@@ -572,8 +545,6 @@ public:
     rclcpp::node_interfaces::OnSetParametersCallbackHandle;
   using OnSetParametersCallbackType =
     rclcpp::node_interfaces::NodeParametersInterface::OnSetParametersCallbackType;
-  using OnParametersSetCallbackType [[deprecated("use OnSetParametersCallbackType instead")]] =
-    OnSetParametersCallbackType;
 
   using PostSetParametersCallbackHandle =
     rclcpp::node_interfaces::PostSetParametersCallbackHandle;

rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node_impl.hpp

@@ -120,18 +120,6 @@ LifecycleNode::create_timer(
     this->node_timers_.get());
 }
 
-template<typename ServiceT>
-typename rclcpp::Client<ServiceT>::SharedPtr
-LifecycleNode::create_client(
-  const std::string & service_name,
-  const rmw_qos_profile_t & qos_profile,
-  rclcpp::CallbackGroup::SharedPtr group)
-{
-  return rclcpp::create_client<ServiceT>(
-    node_base_, node_graph_, node_services_,
-    service_name, qos_profile, group);
-}
-
 template<typename ServiceT>
 typename rclcpp::Client<ServiceT>::SharedPtr
 LifecycleNode::create_client(
@@ -144,19 +132,6 @@ LifecycleNode::create_client(
     service_name, qos, group);
 }
 
-template<typename ServiceT, typename CallbackT>
-typename rclcpp::Service<ServiceT>::SharedPtr
-LifecycleNode::create_service(
-  const std::string & service_name,
-  CallbackT && callback,
-  const rmw_qos_profile_t & qos_profile,
-  rclcpp::CallbackGroup::SharedPtr group)
-{
-  return rclcpp::create_service<ServiceT, CallbackT>(
-    node_base_, node_services_,
-    service_name, std::forward<CallbackT>(callback), qos_profile, group);
-}
-
 template<typename ServiceT, typename CallbackT>
 typename rclcpp::Service<ServiceT>::SharedPtr
 LifecycleNode::create_service(

rclcpp_lifecycle/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp_lifecycle</name>
-  <version>28.3.0</version>
+  <version>28.3.2</version>
   <description>Package containing a prototype for lifecycle implementation</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp_lifecycle/test/test_client.cpp

@@ -61,27 +61,6 @@ TEST_F(TestClient, construction_and_destruction) {
     auto client = node_->create_client<ListParameters>("service");
     EXPECT_TRUE(client);
   }
-  {
-    // suppress deprecated function warning
-    #if !defined(_WIN32)
-    # pragma GCC diagnostic push
-    # pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-    #else  // !defined(_WIN32)
-    # pragma warning(push)
-    # pragma warning(disable: 4996)
-    #endif
-
-    auto client = node_->create_client<ListParameters>(
-      "service", rmw_qos_profile_services_default);
-    EXPECT_TRUE(client);
-
-    // remove warning suppression
-    #if !defined(_WIN32)
-    # pragma GCC diagnostic pop
-    #else  // !defined(_WIN32)
-    # pragma warning(pop)
-    #endif
-  }
   {
     auto client = node_->create_client<ListParameters>(
       "service", rclcpp::ServicesQoS());
@@ -97,28 +76,6 @@ TEST_F(TestClient, construction_and_destruction) {
 }
 
 TEST_F(TestClient, construction_with_free_function) {
-  {
-    auto client = rclcpp::create_client<rcl_interfaces::srv::ListParameters>(
-      node_->get_node_base_interface(),
-      node_->get_node_graph_interface(),
-      node_->get_node_services_interface(),
-      "service",
-      rmw_qos_profile_services_default,
-      nullptr);
-    EXPECT_TRUE(client);
-  }
-  {
-    ASSERT_THROW(
-    {
-      auto client = rclcpp::create_client<rcl_interfaces::srv::ListParameters>(
-        node_->get_node_base_interface(),
-        node_->get_node_graph_interface(),
-        node_->get_node_services_interface(),
-        "invalid_?service",
-        rmw_qos_profile_services_default,
-        nullptr);
-    }, rclcpp::exceptions::InvalidServiceNameError);
-  }
   {
     auto client = rclcpp::create_client<rcl_interfaces::srv::ListParameters>(
       node_->get_node_base_interface(),

rclcpp_lifecycle/test/test_lifecycle_node.cpp

@@ -237,22 +237,6 @@ TEST_F(TestDefaultStateMachine, empty_initializer) {
   EXPECT_EQ(State::PRIMARY_STATE_UNCONFIGURED, test_node->get_current_state().id());
 }
 
-TEST_F(TestDefaultStateMachine, empty_initializer_rcl_errors) {
-  {
-    auto patch = mocking_utils::inject_on_return(
-      "lib:rclcpp_lifecycle", rcl_lifecycle_state_machine_init, RCL_RET_ERROR);
-    EXPECT_THROW(
-      std::make_shared<EmptyLifecycleNode>("testnode").reset(),
-      std::runtime_error);
-  }
-  {
-    auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
-    auto patch = mocking_utils::inject_on_return(
-      "lib:rclcpp_lifecycle", rcl_lifecycle_state_machine_fini, RCL_RET_ERROR);
-    EXPECT_NO_THROW(test_node.reset());
-  }
-}
-
 TEST_F(TestDefaultStateMachine, check_logger_services_exist) {
   // Logger level services are disabled
   {

rclcpp_lifecycle/test/test_lifecycle_node_errors.cpp

@@ -0,0 +1,69 @@
+// Copyright 2024 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gtest/gtest.h>
+
+#include <memory>
+#include <stdexcept>
+#include <string>
+
+#include "rcl_lifecycle/rcl_lifecycle.h"
+
+#include "rclcpp_lifecycle/lifecycle_node.hpp"
+
+#include "./mocking_utils/patch.hpp"
+
+class TestDefaultStateMachine : public ::testing::Test
+{
+protected:
+  static void SetUpTestCase()
+  {
+    rclcpp::init(0, nullptr);
+  }
+  static void TearDownTestCase()
+  {
+    rclcpp::shutdown();
+  }
+};
+
+class EmptyLifecycleNode : public rclcpp_lifecycle::LifecycleNode
+{
+public:
+  explicit EmptyLifecycleNode(const std::string & node_name)
+  : rclcpp_lifecycle::LifecycleNode(node_name)
+  {}
+};
+
+// This test is split out of test_lifecycle_node.cpp for an esoteric reason.  When running on
+// RedHat-based distributions (like Fedora or RHEL), the way that glibc is compiled does not
+// allow mocking_utils::inject_on_return to work.  Thus the test has to patch_and_return().
+// Unfortunately, this means that the resources are not actually cleaned up, and thus other tests
+// may return incorrect results.  By having it in a separate process we ensure that the resources
+// will at least be cleaned up by the process dying.
+TEST_F(TestDefaultStateMachine, empty_initializer_rcl_errors)
+{
+  {
+    auto patch = mocking_utils::patch_and_return(
+      "lib:rclcpp_lifecycle", rcl_lifecycle_state_machine_init, RCL_RET_ERROR);
+    EXPECT_THROW(
+      std::make_shared<EmptyLifecycleNode>("testnode").reset(),
+      std::runtime_error);
+  }
+  {
+    auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
+    auto patch = mocking_utils::patch_and_return(
+      "lib:rclcpp_lifecycle", rcl_lifecycle_state_machine_fini, RCL_RET_ERROR);
+    EXPECT_NO_THROW(test_node.reset());
+  }
+}

rclcpp_lifecycle/test/test_service.cpp

@@ -68,31 +68,6 @@ TEST_F(TestService, construction_and_destruction) {
     const rclcpp::ServiceBase * const_service_base = service.get();
     EXPECT_NE(nullptr, const_service_base->get_service_handle());
   }
-  {
-    // suppress deprecated function warning
-    #if !defined(_WIN32)
-    # pragma GCC diagnostic push
-    # pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-    #else  // !defined(_WIN32)
-    # pragma warning(push)
-    # pragma warning(disable: 4996)
-    #endif
-
-    auto service = node->create_service<Empty>(
-      "service", callback, rmw_qos_profile_services_default);
-
-    // remove warning suppression
-    #if !defined(_WIN32)
-    # pragma GCC diagnostic pop
-    #else  // !defined(_WIN32)
-    # pragma warning(pop)
-    #endif
-
-    EXPECT_NE(nullptr, service->get_service_handle());
-    const rclcpp::ServiceBase * const_service_base = service.get();
-    EXPECT_NE(nullptr, const_service_base->get_service_handle());
-  }
-
   {
     ASSERT_THROW(
     {