@wip

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>

rclcpp/CHANGELOG.rst

@@ -2,53 +2,6 @@
 Changelog for package rclcpp
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-21.0.6 (2024-04-19)
--------------------
-* address ambiguous auto variable. (`#2486 <https://github.com/ros2/rclcpp/issues/2486>`_)
-* Contributors: Tomoya Fujita
-
-21.0.5 (2024-02-07)
--------------------
-* Fix data race in EventHandlerBase (`#2387 <https://github.com/ros2/rclcpp/issues/2387>`_)
-* Contributors: mauropasse
-
-21.0.4 (2023-11-17)
--------------------
-* Disable the loaned messages inside the executor. (`#2365 <https://github.com/ros2/rclcpp/issues/2365>`_)
-* Add missing 'enable_rosout' comments (`#2346 <https://github.com/ros2/rclcpp/issues/2346>`_)
-* Address rate related flaky tests. (`#2341 <https://github.com/ros2/rclcpp/issues/2341>`_)
-* Add missing stdexcept include (`#2333 <https://github.com/ros2/rclcpp/issues/2333>`_)
-* Update SignalHandler get_global_signal_handler to avoid complex types in static memory (`#2322 <https://github.com/ros2/rclcpp/issues/2322>`_)
-* Fix C++20 allocator construct deprecation (`#2318 <https://github.com/ros2/rclcpp/issues/2318>`_)
-* Topic correct typeadapter deduction (`#2298 <https://github.com/ros2/rclcpp/issues/2298>`_)
-* Contributors: AiVerisimilitude, Chen Lihui, Chris Lalancette, Jiaqi Li, Øystein Sture, Tomoya Fujita, William Woodall
-
-21.0.3 (2023-09-08)
--------------------
-* Do not crash Executor when send_response fails due to client failure. (`#2279 <https://github.com/ros2/rclcpp/issues/2279>`_)
-* Add new node interface TypeDescriptionsInterface to provide GetTypeDescription service (`#2236 <https://github.com/ros2/rclcpp/issues/2236>`_)
-* Contributors: Emerson Knapp, Tomoya Fujita, Zang MingJie
-
-21.0.2 (2023-07-14)
--------------------
-* Fix warnings related to comparison of integer expressions of different signedness (`#2222 <https://github.com/ros2/rclcpp/issues/2222>`_)
-* Fix race condition in events-executor (`#2191 <https://github.com/ros2/rclcpp/issues/2191>`_)
-* Contributors: Alberto Soragna, Tomoya Fujita
-
-21.0.1 (2023-05-11)
--------------------
-* Fix delivered message kind (`#2175 <https://github.com/ros2/rclcpp/issues/2175>`_) (`#2178 <https://github.com/ros2/rclcpp/issues/2178>`_)
-* Contributors: mergify[bot]
-
-21.0.0 (2023-04-18)
--------------------
-* Add support for logging service. (`#2122 <https://github.com/ros2/rclcpp/issues/2122>`_)
-* Picking ABI-incompatible executor changes (`#2170 <https://github.com/ros2/rclcpp/issues/2170>`_)
-* add events-executor and timers-manager in rclcpp (`#2155 <https://github.com/ros2/rclcpp/issues/2155>`_)
-* Create common structures for executors to use (`#2143 <https://github.com/ros2/rclcpp/issues/2143>`_)
-* Implement deliver message kind (`#2168 <https://github.com/ros2/rclcpp/issues/2168>`_)
-* Contributors: Alberto Soragna, Lei Liu, Michael Carroll, methylDragon
-
 20.0.0 (2023-04-13)
 -------------------
 * applied tracepoints for ring_buffer (`#2091 <https://github.com/ros2/rclcpp/issues/2091>`_)

rclcpp/CMakeLists.txt

@@ -64,7 +64,6 @@ set(${PROJECT_NAME}_SRCS
   src/rclcpp/executors/executor_notify_waitable.cpp
   src/rclcpp/executors/multi_threaded_executor.cpp
   src/rclcpp/executors/single_threaded_executor.cpp
-  src/rclcpp/executors/static_executor_entities_collector.cpp
   src/rclcpp/executors/static_single_threaded_executor.cpp
   src/rclcpp/expand_topic_or_service_name.cpp
   src/rclcpp/experimental/executors/events_executor/events_executor.cpp
@@ -92,7 +91,6 @@ set(${PROJECT_NAME}_SRCS
   src/rclcpp/node_interfaces/node_time_source.cpp
   src/rclcpp/node_interfaces/node_timers.cpp
   src/rclcpp/node_interfaces/node_topics.cpp
-  src/rclcpp/node_interfaces/node_type_descriptions.cpp
   src/rclcpp/node_interfaces/node_waitables.cpp
   src/rclcpp/node_options.cpp
   src/rclcpp/parameter.cpp

rclcpp/include/rclcpp/callback_group.hpp

@@ -187,14 +187,35 @@ public:
   RCLCPP_PUBLIC
   size_t size() const;
 
+  /// Return a reference to the 'can be taken' atomic boolean.
+  /**
+   * The resulting bool will be true in the case that no executor is currently
+   * using an executable entity from this group.
+   * The resulting bool will be false in the case that an executor is currently
+   * using an executable entity from this group, and the group policy doesn't
+   * allow a second take (eg mutual exclusion)
+   * \return a reference to the flag
+   */
   RCLCPP_PUBLIC
   std::atomic_bool &
   can_be_taken_from();
 
+  /// Get the group type.
+  /**
+   * \return the group type
+   */
   RCLCPP_PUBLIC
   const CallbackGroupType &
   type() const;
 
+  /// Collect all of the entity pointers contained in this callback group.
+  /**
+   * \param[in] sub_func Function to execute for each subscription
+   * \param[in] service_func Function to execute for each service
+   * \param[in] client_func Function to execute for each client
+   * \param[in] timer_func Function to execute for each timer
+   * \param[in] waitable_fuinc Function to execute for each waitable
+   */
   RCLCPP_PUBLIC
   void collect_all_ptrs(
     std::function<void(const rclcpp::SubscriptionBase::SharedPtr &)> sub_func,

rclcpp/include/rclcpp/client.hpp

@@ -845,7 +845,7 @@ protected:
         "Received invalid sequence number. Ignoring...");
       return std::nullopt;
     }
-    std::optional<CallbackInfoVariant> value = std::move(it->second.second);
+    auto value = std::move(it->second.second);
     this->pending_requests_.erase(request_number);
     return value;
   }

rclcpp/include/rclcpp/context.hpp

@@ -26,7 +26,6 @@
 #include <unordered_set>
 #include <utility>
 #include <vector>
-#include <stdexcept>
 
 #include "rcl/context.h"
 #include "rcl/guard_condition.h"

rclcpp/include/rclcpp/event_handler.hpp

@@ -260,16 +260,6 @@ public:
     }
   }
 
-  ~EventHandler()
-  {
-    // Since the rmw event listener holds a reference to the
-    // "on ready" callback, we need to clear it on destruction of this class.
-    // This clearing is not needed for other rclcpp entities like pub/subs, since
-    // they do own the underlying rmw entities, which are destroyed
-    // on their rclcpp destructors, thus no risk of dangling pointers.
-    clear_on_ready_callback();
-  }
-
   /// Take data so that the callback cannot be scheduled again
   std::shared_ptr<void>
   take_data() override

rclcpp/include/rclcpp/executor.hpp

@@ -19,6 +19,7 @@
 #include <cassert>
 #include <chrono>
 #include <cstdlib>
+#include <deque>
 #include <iostream>
 #include <list>
 #include <map>
@@ -29,26 +30,24 @@
 
 #include "rcl/guard_condition.h"
 #include "rcl/wait.h"
+#include "rclcpp/executors/executor_notify_waitable.hpp"
 #include "rcpputils/scope_exit.hpp"
 
 #include "rclcpp/context.hpp"
 #include "rclcpp/contexts/default_context.hpp"
 #include "rclcpp/guard_condition.hpp"
 #include "rclcpp/executor_options.hpp"
+#include "rclcpp/executors/executor_entities_collection.hpp"
+#include "rclcpp/executors/executor_entities_collector.hpp"
 #include "rclcpp/future_return_code.hpp"
-#include "rclcpp/memory_strategies.hpp"
-#include "rclcpp/memory_strategy.hpp"
 #include "rclcpp/node_interfaces/node_base_interface.hpp"
 #include "rclcpp/utilities.hpp"
 #include "rclcpp/visibility_control.hpp"
+#include "rclcpp/wait_set.hpp"
 
 namespace rclcpp
 {
 
-typedef std::map<rclcpp::CallbackGroup::WeakPtr,
-    rclcpp::node_interfaces::NodeBaseInterface::WeakPtr,
-    std::owner_less<rclcpp::CallbackGroup::WeakPtr>> WeakCallbackGroupsToNodesMap;
-
 // Forward declaration is used in convenience method signature.
 class Node;
 class ExecutorImplementation;
@@ -403,17 +402,6 @@ public:
   void
   cancel();
 
-  /// Support dynamic switching of the memory strategy.
-  /**
-   * Switching the memory strategy while the executor is spinning in another threading could have
-   * unintended consequences.
-   * \param[in] memory_strategy Shared pointer to the memory strategy to set.
-   * \throws std::runtime_error if memory_strategy is null
-   */
-  RCLCPP_PUBLIC
-  void
-  set_memory_strategy(memory_strategy::MemoryStrategy::SharedPtr memory_strategy);
-
   /// Returns true if the executor is currently spinning.
   /**
    * This function can be called asynchronously from any thread.
@@ -498,6 +486,11 @@ protected:
   static void
   execute_client(rclcpp::ClientBase::SharedPtr client);
 
+  /// Gather all of the waitable entities from associated nodes and callback groups.
+  RCLCPP_PUBLIC
+  void
+  collect_entities();
+
   /// Block until more work becomes avilable or timeout is reached.
   /**
    * Builds a set of waitable entities, which are passed to the middleware.
@@ -509,62 +502,6 @@ protected:
   void
   wait_for_work(std::chrono::nanoseconds timeout = std::chrono::nanoseconds(-1));
 
-  /// Find node associated with a callback group
-  /**
-   * \param[in] weak_groups_to_nodes map of callback groups to nodes
-   * \param[in] group callback group to find assocatiated node
-   * \return Pointer to associated node if found, else nullptr
-   */
-  RCLCPP_PUBLIC
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr
-  get_node_by_group(
-    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
-    rclcpp::CallbackGroup::SharedPtr group);
-
-  /// Return true if the node has been added to this executor.
-  /**
-   * \param[in] node_ptr a shared pointer that points to a node base interface
-   * \param[in] weak_groups_to_nodes map to nodes to lookup
-   * \return true if the node is associated with the executor, otherwise false
-   */
-  RCLCPP_PUBLIC
-  bool
-  has_node(
-    const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes) const;
-
-  /// Find the callback group associated with a timer
-  /**
-   * \param[in] timer Timer to find associated callback group
-   * \return Pointer to callback group node if found, else nullptr
-   */
-  RCLCPP_PUBLIC
-  rclcpp::CallbackGroup::SharedPtr
-  get_group_by_timer(rclcpp::TimerBase::SharedPtr timer);
-
-  /// Add a callback group to an executor
-  /**
-   * \see rclcpp::Executor::add_callback_group
-   */
-  RCLCPP_PUBLIC
-  virtual void
-  add_callback_group_to_map(
-    rclcpp::CallbackGroup::SharedPtr group_ptr,
-    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-    WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
-    bool notify = true) RCPPUTILS_TSA_REQUIRES(mutex_);
-
-  /// Remove a callback group from the executor.
-  /**
-   * \see rclcpp::Executor::remove_callback_group
-   */
-  RCLCPP_PUBLIC
-  virtual void
-  remove_callback_group_from_map(
-    rclcpp::CallbackGroup::SharedPtr group_ptr,
-    WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
-    bool notify = true) RCPPUTILS_TSA_REQUIRES(mutex_);
-
   /// Check for executable in ready state and populate union structure.
   /**
    * \param[out] any_executable populated union structure of ready executable
@@ -575,33 +512,6 @@ protected:
   bool
   get_next_ready_executable(AnyExecutable & any_executable);
 
-  /// Check for executable in ready state and populate union structure.
-  /**
-   * This is the implementation of get_next_ready_executable that takes into
-   * account the current state of callback groups' association with nodes and
-   * executors.
-   *
-   * This checks in a particular order for available work:
-   *   * Timers
-   *   * Subscriptions
-   *   * Services
-   *   * Clients
-   *   * Waitable
-   *
-   * If the next executable is not associated with this executor/node pair,
-   * then this method will return false.
-   *
-   * \param[out] any_executable populated union structure of ready executable
-   * \param[in] weak_groups_to_nodes mapping of callback groups to nodes
-   * \return true if an executable was ready and any_executable was populated,
-   *   otherwise false
-   */
-  RCLCPP_PUBLIC
-  bool
-  get_next_ready_executable_from_map(
-    AnyExecutable & any_executable,
-    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes);
-
   /// Wait for executable in ready state and populate union structure.
   /**
    * If an executable is ready, it will return immediately, otherwise
@@ -619,21 +529,6 @@ protected:
     AnyExecutable & any_executable,
     std::chrono::nanoseconds timeout = std::chrono::nanoseconds(-1));
 
-  /// Add all callback groups that can be automatically added from associated nodes.
-  /**
-   * The executor, before collecting entities, verifies if any callback group from
-   * nodes associated with the executor, which is not already associated to an executor,
-   * can be automatically added to this executor.
-   * This takes care of any callback group that has been added to a node but not explicitly added
-   * to the executor.
-   * It is important to note that in order for the callback groups to be automatically added to an
-   * executor through this function, the node of the callback groups needs to have been added
-   * through the `add_node` method.
-   */
-  RCLCPP_PUBLIC
-  virtual void
-  add_callback_groups_from_nodes_associated_to_executor() RCPPUTILS_TSA_REQUIRES(mutex_);
-
   /// Spinning state, used to prevent multi threaded calls to spin and to cancel blocking spins.
   std::atomic_bool spinning;
 
@@ -643,16 +538,8 @@ protected:
   /// Guard condition for signaling the rmw layer to wake up for system shutdown.
   std::shared_ptr<rclcpp::GuardCondition> shutdown_guard_condition_;
 
-  /// Wait set for managing entities that the rmw layer waits on.
-  rcl_wait_set_t wait_set_ = rcl_get_zero_initialized_wait_set();
-
-  // Mutex to protect the subsequent memory_strategy_.
   mutable std::mutex mutex_;
 
-  /// The memory strategy: an interface for handling user-defined memory allocation strategies.
-  memory_strategy::MemoryStrategy::SharedPtr
-  memory_strategy_ RCPPUTILS_TSA_PT_GUARDED_BY(mutex_);
-
   /// The context associated with this executor.
   std::shared_ptr<rclcpp::Context> context_;
 
@@ -662,39 +549,33 @@ protected:
   virtual void
   spin_once_impl(std::chrono::nanoseconds timeout);
 
-  typedef std::map<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr,
-      const rclcpp::GuardCondition *,
-      std::owner_less<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr>>
-    WeakNodesToGuardConditionsMap;
+  /// Waitable containing guard conditions controlling the executor flow.
+  /**
+   * This waitable contains the interrupt and shutdown guard condition, as well
+   * as the guard condition associated with each node and callback group.
+   * By default, if any change is detected in the monitored entities, the notify
+   * waitable will awake the executor and rebuild  the collections.
+   */
+  std::shared_ptr<rclcpp::executors::ExecutorNotifyWaitable> notify_waitable_;
 
-  typedef std::map<rclcpp::CallbackGroup::WeakPtr,
-      const rclcpp::GuardCondition *,
-      std::owner_less<rclcpp::CallbackGroup::WeakPtr>>
-    WeakCallbackGroupsToGuardConditionsMap;
+  std::atomic_bool entities_need_rebuild_;
 
-  /// maps nodes to guard conditions
-  WeakNodesToGuardConditionsMap
-  weak_nodes_to_guard_conditions_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
+  /// Collector used to associate executable entities from nodes and guard conditions
+  rclcpp::executors::ExecutorEntitiesCollector collector_;
 
-  /// maps callback groups to guard conditions
-  WeakCallbackGroupsToGuardConditionsMap
-  weak_groups_to_guard_conditions_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
+  /// Waitset to be waited on.
+  rclcpp::WaitSet wait_set_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
-  /// maps callback groups associated to nodes
-  WeakCallbackGroupsToNodesMap
-  weak_groups_associated_with_executor_to_nodes_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
+  /// Hold the current state of the collection being waited on by the waitset
+  rclcpp::executors::ExecutorEntitiesCollection current_collection_ RCPPUTILS_TSA_GUARDED_BY(
+    mutex_);
 
-  /// maps callback groups to nodes associated with executor
-  WeakCallbackGroupsToNodesMap
-  weak_groups_to_nodes_associated_with_executor_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
+  /// Hold the current state of the notify waitable being waited on by the waitset
+  std::shared_ptr<rclcpp::executors::ExecutorNotifyWaitable> current_notify_waitable_
+  RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
-  /// maps all callback groups to nodes
-  WeakCallbackGroupsToNodesMap
-  weak_groups_to_nodes_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
-
-  /// nodes that are associated with the executor
-  std::list<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr>
-  weak_nodes_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
+  /// Hold the list of executables currently available to be executed.
+  std::deque<rclcpp::AnyExecutable> ready_executables_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
   /// shutdown callback handle registered to Context
   rclcpp::OnShutdownCallbackHandle shutdown_callback_handle_;

rclcpp/include/rclcpp/executors/static_executor_entities_collector.hpp

@@ -1,357 +0,0 @@
-// Copyright 2020 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__STATIC_EXECUTOR_ENTITIES_COLLECTOR_HPP_
-#define RCLCPP__EXECUTORS__STATIC_EXECUTOR_ENTITIES_COLLECTOR_HPP_
-
-#include <chrono>
-#include <list>
-#include <map>
-#include <memory>
-#include <vector>
-
-#include "rcl/guard_condition.h"
-#include "rcl/wait.h"
-
-#include "rclcpp/experimental/executable_list.hpp"
-#include "rclcpp/macros.hpp"
-#include "rclcpp/memory_strategy.hpp"
-#include "rclcpp/visibility_control.hpp"
-#include "rclcpp/waitable.hpp"
-
-namespace rclcpp
-{
-namespace executors
-{
-typedef std::map<rclcpp::CallbackGroup::WeakPtr,
-    rclcpp::node_interfaces::NodeBaseInterface::WeakPtr,
-    std::owner_less<rclcpp::CallbackGroup::WeakPtr>> WeakCallbackGroupsToNodesMap;
-
-class StaticExecutorEntitiesCollector final
-  : public rclcpp::Waitable,
-  public std::enable_shared_from_this<StaticExecutorEntitiesCollector>
-{
-public:
-  RCLCPP_SMART_PTR_DEFINITIONS(StaticExecutorEntitiesCollector)
-
-  // Constructor
-  RCLCPP_PUBLIC
-  StaticExecutorEntitiesCollector() = default;
-
-  // Destructor
-  RCLCPP_PUBLIC
-  ~StaticExecutorEntitiesCollector();
-
-  /// Initialize StaticExecutorEntitiesCollector
-  /**
-   * \param p_wait_set A reference to the wait set to be used in the executor
-   * \param memory_strategy Shared pointer to the memory strategy to set.
-   * \throws std::runtime_error if memory strategy is null
-   */
-  RCLCPP_PUBLIC
-  void
-  init(
-    rcl_wait_set_t * p_wait_set,
-    rclcpp::memory_strategy::MemoryStrategy::SharedPtr memory_strategy);
-
-  /// Finalize StaticExecutorEntitiesCollector to clear resources
-  RCLCPP_PUBLIC
-  bool
-  is_init() {return initialized_;}
-
-  RCLCPP_PUBLIC
-  void
-  fini();
-
-  /// Execute the waitable.
-  RCLCPP_PUBLIC
-  void
-  execute(std::shared_ptr<void> & data) override;
-
-  /// Take the data so that it can be consumed with `execute`.
-  /**
-   * For `StaticExecutorEntitiesCollector`, this always return `nullptr`.
-   * \sa rclcpp::Waitable::take_data()
-   */
-  RCLCPP_PUBLIC
-  std::shared_ptr<void>
-  take_data() override;
-
-  /// Function to add_handles_to_wait_set and wait for work and
-  /**
-   * block until the wait set is ready or until the timeout has been exceeded.
-   * \throws std::runtime_error if wait set couldn't be cleared or filled.
-   * \throws any rcl errors from rcl_wait, \see rclcpp::exceptions::throw_from_rcl_error()
-   */
-  RCLCPP_PUBLIC
-  void
-  refresh_wait_set(std::chrono::nanoseconds timeout = std::chrono::nanoseconds(-1));
-
-  /**
-   * \throws std::runtime_error if it couldn't add guard condition to wait set
-   */
-  RCLCPP_PUBLIC
-  void
-  add_to_wait_set(rcl_wait_set_t * wait_set) override;
-
-  RCLCPP_PUBLIC
-  size_t
-  get_number_of_ready_guard_conditions() override;
-
-  /// Add a callback group to an executor.
-  /**
-   * \see rclcpp::Executor::add_callback_group
-   */
-  RCLCPP_PUBLIC
-  bool
-  add_callback_group(
-    rclcpp::CallbackGroup::SharedPtr group_ptr,
-    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr);
-
-  /// Add a callback group to an executor.
-  /**
-   * \see rclcpp::Executor::add_callback_group
-   * \return boolean whether the node from the callback group is new
-   */
-  RCLCPP_PUBLIC
-  bool
-  add_callback_group(
-    rclcpp::CallbackGroup::SharedPtr group_ptr,
-    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-    WeakCallbackGroupsToNodesMap & weak_groups_to_nodes);
-
-  /// Remove a callback group from the executor.
-  /**
-   * \see rclcpp::Executor::remove_callback_group
-   */
-  RCLCPP_PUBLIC
-  bool
-  remove_callback_group(
-    rclcpp::CallbackGroup::SharedPtr group_ptr);
-
-  /// Remove a callback group from the executor.
-  /**
-   * \see rclcpp::Executor::remove_callback_group_from_map
-   */
-  RCLCPP_PUBLIC
-  bool
-  remove_callback_group_from_map(
-    rclcpp::CallbackGroup::SharedPtr group_ptr,
-    WeakCallbackGroupsToNodesMap & weak_groups_to_nodes);
-
-  /**
-   * \see rclcpp::Executor::add_node()
-   * \throw std::runtime_error if node was already added
-   */
-  RCLCPP_PUBLIC
-  bool
-  add_node(
-    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr);
-
-  /**
-   * \see rclcpp::Executor::remove_node()
-   * \throw std::runtime_error if no guard condition is associated with node.
-   */
-  RCLCPP_PUBLIC
-  bool
-  remove_node(
-    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr);
-
-  RCLCPP_PUBLIC
-  std::vector<rclcpp::CallbackGroup::WeakPtr>
-  get_all_callback_groups();
-
-  /// Get callback groups that belong to executor.
-  /**
-   * \see rclcpp::Executor::get_manually_added_callback_groups()
-   */
-  RCLCPP_PUBLIC
-  std::vector<rclcpp::CallbackGroup::WeakPtr>
-  get_manually_added_callback_groups();
-
-  /// Get callback groups that belong to executor.
-  /**
-   * \see rclcpp::Executor::get_automatically_added_callback_groups_from_nodes()
-   */
-  RCLCPP_PUBLIC
-  std::vector<rclcpp::CallbackGroup::WeakPtr>
-  get_automatically_added_callback_groups_from_nodes();
-
-  /// Complete all available queued work without blocking.
-  /**
-   * This function checks if after the guard condition was triggered
-   * (or a spurious wakeup happened) we are really ready to execute
-   * i.e. re-collect entities
-   */
-  RCLCPP_PUBLIC
-  bool
-  is_ready(rcl_wait_set_t * wait_set) override;
-
-  /// Return number of timers
-  /**
-   * \return number of timers
-   */
-  RCLCPP_PUBLIC
-  size_t
-  get_number_of_timers() {return exec_list_.number_of_timers;}
-
-  /// Return number of subscriptions
-  /**
-   * \return number of subscriptions
-   */
-  RCLCPP_PUBLIC
-  size_t
-  get_number_of_subscriptions() {return exec_list_.number_of_subscriptions;}
-
-  /// Return number of services
-  /**
-   * \return number of services
-   */
-  RCLCPP_PUBLIC
-  size_t
-  get_number_of_services() {return exec_list_.number_of_services;}
-
-  /// Return number of clients
-  /**
-   * \return number of clients
-   */
-  RCLCPP_PUBLIC
-  size_t
-  get_number_of_clients() {return exec_list_.number_of_clients;}
-
-  /// Return number of waitables
-  /**
-   * \return number of waitables
-   */
-  RCLCPP_PUBLIC
-  size_t
-  get_number_of_waitables() {return exec_list_.number_of_waitables;}
-
-  /** Return a SubscritionBase Sharedptr by index.
-   * \param[in] i The index of the SubscritionBase
-   * \return a SubscritionBase shared pointer
-   * \throws std::out_of_range if the argument is higher than the size of the structrue.
-   */
-  RCLCPP_PUBLIC
-  rclcpp::SubscriptionBase::SharedPtr
-  get_subscription(size_t i) {return exec_list_.subscription[i];}
-
-  /** Return a TimerBase Sharedptr by index.
-   * \param[in] i The index of the TimerBase
-   * \return a TimerBase shared pointer
-   * \throws std::out_of_range if the argument is higher than the size.
-   */
-  RCLCPP_PUBLIC
-  rclcpp::TimerBase::SharedPtr
-  get_timer(size_t i) {return exec_list_.timer[i];}
-
-  /** Return a ServiceBase Sharedptr by index.
-   * \param[in] i The index of the ServiceBase
-   * \return a ServiceBase shared pointer
-   * \throws std::out_of_range if the argument is higher than the size.
-   */
-  RCLCPP_PUBLIC
-  rclcpp::ServiceBase::SharedPtr
-  get_service(size_t i) {return exec_list_.service[i];}
-
-  /** Return a ClientBase Sharedptr by index
-   * \param[in] i The index of the ClientBase
-   * \return a ClientBase shared pointer
-   * \throws std::out_of_range if the argument is higher than the size.
-   */
-  RCLCPP_PUBLIC
-  rclcpp::ClientBase::SharedPtr
-  get_client(size_t i) {return exec_list_.client[i];}
-
-  /** Return a Waitable Sharedptr by index
-   * \param[in] i The index of the Waitable
-   * \return a Waitable shared pointer
-   * \throws std::out_of_range if the argument is higher than the size.
-   */
-  RCLCPP_PUBLIC
-  rclcpp::Waitable::SharedPtr
-  get_waitable(size_t i) {return exec_list_.waitable[i];}
-
-private:
-  /// Function to reallocate space for entities in the wait set.
-  /**
-   * \throws std::runtime_error if wait set couldn't be cleared or resized.
-   */
-  void
-  prepare_wait_set();
-
-  void
-  fill_executable_list();
-
-  void
-  fill_memory_strategy();
-
-  /// Return true if the node belongs to the collector
-  /**
-   * \param[in] node_ptr a node base interface shared pointer
-   * \param[in] weak_groups_to_nodes map to nodes to lookup
-   * \return boolean whether a node belongs the collector
-   */
-  bool
-  has_node(
-    const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes) const;
-
-  /// Add all callback groups that can be automatically added by any executor
-  /// and is not already associated with an executor from nodes
-  /// that are associated with executor
-  /**
-   * \see rclcpp::Executor::add_callback_groups_from_nodes_associated_to_executor()
-   */
-  void
-  add_callback_groups_from_nodes_associated_to_executor();
-
-  void
-  fill_executable_list_from_map(const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes);
-
-  /// Memory strategy: an interface for handling user-defined memory allocation strategies.
-  rclcpp::memory_strategy::MemoryStrategy::SharedPtr memory_strategy_;
-
-  // maps callback groups to nodes.
-  WeakCallbackGroupsToNodesMap weak_groups_associated_with_executor_to_nodes_;
-  // maps callback groups to nodes.
-  WeakCallbackGroupsToNodesMap weak_groups_to_nodes_associated_with_executor_;
-
-  typedef std::map<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr,
-      const rclcpp::GuardCondition *,
-      std::owner_less<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr>>
-    WeakNodesToGuardConditionsMap;
-  WeakNodesToGuardConditionsMap weak_nodes_to_guard_conditions_;
-
-  /// List of weak nodes registered in the static executor
-  std::list<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr> weak_nodes_;
-
-  // Mutex to protect vector of new nodes.
-  std::mutex new_nodes_mutex_;
-  std::vector<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr> new_nodes_;
-
-  /// Wait set for managing entities that the rmw layer waits on.
-  rcl_wait_set_t * p_wait_set_ = nullptr;
-
-  /// Executable list: timers, subscribers, clients, services and waitables
-  rclcpp::experimental::ExecutableList exec_list_;
-
-  /// Bool to check if the entities collector has been initialized
-  bool initialized_ = false;
-};
-
-}  // namespace executors
-}  // namespace rclcpp
-
-#endif  // RCLCPP__EXECUTORS__STATIC_EXECUTOR_ENTITIES_COLLECTOR_HPP_

rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp

@@ -15,24 +15,13 @@
 #ifndef RCLCPP__EXECUTORS__STATIC_SINGLE_THREADED_EXECUTOR_HPP_
 #define RCLCPP__EXECUTORS__STATIC_SINGLE_THREADED_EXECUTOR_HPP_
 
+#include <atomic>
 #include <chrono>
-#include <cassert>
-#include <cstdlib>
 #include <memory>
-#include <vector>
-#include <string>
-
-#include "rmw/rmw.h"
 
 #include "rclcpp/executor.hpp"
-#include "rclcpp/executors/static_executor_entities_collector.hpp"
-#include "rclcpp/experimental/executable_list.hpp"
-#include "rclcpp/macros.hpp"
-#include "rclcpp/memory_strategies.hpp"
-#include "rclcpp/node.hpp"
-#include "rclcpp/rate.hpp"
-#include "rclcpp/utilities.hpp"
-#include "rclcpp/visibility_control.hpp"
+#include "rclcpp/executors/executor_entities_collection.hpp"
+#include "rclcpp/executors/single_threaded_executor.hpp"
 
 namespace rclcpp
 {
@@ -65,7 +54,7 @@ public:
   explicit StaticSingleThreadedExecutor(
     const rclcpp::ExecutorOptions & options = rclcpp::ExecutorOptions());
 
-  /// Default destrcutor.
+  /// Default destructor.
   RCLCPP_PUBLIC
   virtual ~StaticSingleThreadedExecutor();
 
@@ -116,92 +105,20 @@ public:
   void
   spin_all(std::chrono::nanoseconds max_duration) 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;
-
-  /// 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::StaticSingleThreadedExecutor::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;
-
-  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:
   /**
    * @brief Executes ready executables from wait set.
+   * @param collection entities to evaluate for ready executables.
+   * @param wait_result result to check for ready executables.
    * @param spin_once if true executes only the first ready executable.
    * @return true if any executable was ready.
    */
   RCLCPP_PUBLIC
   bool
-  execute_ready_executables(bool spin_once = false);
+  execute_ready_executables(
+    const rclcpp::executors::ExecutorEntitiesCollection & collection,
+    rclcpp::WaitResult<rclcpp::WaitSet> & wait_result,
+    bool spin_once);
 
   RCLCPP_PUBLIC
   void
@@ -213,8 +130,6 @@ protected:
 
 private:
   RCLCPP_DISABLE_COPY(StaticSingleThreadedExecutor)
-
-  StaticExecutorEntitiesCollector::SharedPtr entities_collector_;
 };
 
 }  // namespace executors

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

@@ -243,11 +243,6 @@ private:
   std::function<void(size_t, int)>
   create_waitable_callback(const rclcpp::Waitable * waitable_id);
 
-  /// Utility to add the notify waitable to an entities collection
-  void
-  add_notify_waitable_to_collection(
-    rclcpp::executors::ExecutorEntitiesCollection::WaitableCollection & collection);
-
   /// Searches for the provided entity_id in the collection and returns the entity if valid
   template<typename CollectionType>
   typename CollectionType::EntitySharedPtr

rclcpp/include/rclcpp/experimental/intra_process_manager.hpp

@@ -481,13 +481,13 @@ private:
                 "subscription use different allocator types, which is not supported");
       }
 
-      if constexpr (rclcpp::TypeAdapter<MessageT, ROSMessageType>::is_specialized::value) {
+      if constexpr (rclcpp::TypeAdapter<MessageT>::is_specialized::value) {
         ROSMessageTypeAllocator ros_message_alloc(allocator);
-        auto ptr = ROSMessageTypeAllocatorTraits::allocate(ros_message_alloc, 1);
-        ROSMessageTypeAllocatorTraits::construct(ros_message_alloc, ptr);
+        auto ptr = ros_message_alloc.allocate(1);
+        ros_message_alloc.construct(ptr);
         ROSMessageTypeDeleter deleter;
         allocator::set_allocator_for_deleter(&deleter, &allocator);
-        rclcpp::TypeAdapter<MessageT, ROSMessageType>::convert_to_ros_message(*message, *ptr);
+        rclcpp::TypeAdapter<MessageT>::convert_to_ros_message(*message, *ptr);
         auto ros_msg = std::unique_ptr<ROSMessageType, ROSMessageTypeDeleter>(ptr, deleter);
         ros_message_subscription->provide_intra_process_message(std::move(ros_msg));
       } else {

rclcpp/include/rclcpp/node.hpp

@@ -56,7 +56,6 @@
 #include "rclcpp/node_interfaces/node_time_source_interface.hpp"
 #include "rclcpp/node_interfaces/node_timers_interface.hpp"
 #include "rclcpp/node_interfaces/node_topics_interface.hpp"
-#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp"
 #include "rclcpp/node_interfaces/node_waitables_interface.hpp"
 #include "rclcpp/node_options.hpp"
 #include "rclcpp/parameter.hpp"
@@ -1455,11 +1454,6 @@ public:
   rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr
   get_node_time_source_interface();
 
-  /// Return the Node's internal NodeTypeDescriptionsInterface implementation.
-  RCLCPP_PUBLIC
-  rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
-  get_node_type_descriptions_interface();
-
   /// Return the sub-namespace, if this is a sub-node, otherwise an empty string.
   /**
    * The returned sub-namespace is either the accumulated sub-namespaces which
@@ -1597,11 +1591,6 @@ private:
   const rclcpp::NodeOptions node_options_;
   const std::string sub_namespace_;
   const std::string effective_namespace_;
-
-  /// Static map(s) containing extra member variables for Node without changing its ABI.
-  // See node.cpp for more details.
-  class BackportMembers;
-  static BackportMembers backport_members_;
 };
 
 }  // namespace rclcpp

rclcpp/include/rclcpp/node_interfaces/node_interfaces.hpp

@@ -30,7 +30,6 @@
   rclcpp::node_interfaces::NodeTimeSourceInterface, \
   rclcpp::node_interfaces::NodeTimersInterface, \
   rclcpp::node_interfaces::NodeTopicsInterface, \
-  rclcpp::node_interfaces::NodeTypeDescriptionsInterface, \
   rclcpp::node_interfaces::NodeWaitablesInterface
 
 
@@ -119,7 +118,6 @@ public:
    *   - rclcpp::node_interfaces::NodeTimeSourceInterface
    *   - rclcpp::node_interfaces::NodeTimersInterface
    *   - rclcpp::node_interfaces::NodeTopicsInterface
-   *   - rclcpp::node_interfaces::NodeTypeDescriptionsInterface
    *   - rclcpp::node_interfaces::NodeWaitablesInterface
    *
    * Or you use custom interfaces as long as you make a template specialization

rclcpp/include/rclcpp/node_interfaces/node_logging.hpp

@@ -23,9 +23,6 @@
 #include "rclcpp/node_interfaces/node_logging_interface.hpp"
 #include "rclcpp/visibility_control.hpp"
 
-#include "rcl_interfaces/srv/get_logger_levels.hpp"
-#include "rcl_interfaces/srv/set_logger_levels.hpp"
-
 namespace rclcpp
 {
 namespace node_interfaces
@@ -38,7 +35,7 @@ public:
   RCLCPP_SMART_PTR_ALIASES_ONLY(NodeLoggingInterface)
 
   RCLCPP_PUBLIC
-  explicit NodeLogging(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base);
+  explicit NodeLogging(rclcpp::node_interfaces::NodeBaseInterface * node_base);
 
   RCLCPP_PUBLIC
   virtual
@@ -52,21 +49,13 @@ public:
   const char *
   get_logger_name() const override;
 
-  RCLCPP_PUBLIC
-  void
-  create_logger_services(
-    node_interfaces::NodeServicesInterface::SharedPtr node_services) override;
-
 private:
   RCLCPP_DISABLE_COPY(NodeLogging)
 
   /// Handle to the NodeBaseInterface given in the constructor.
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base_;
+  rclcpp::node_interfaces::NodeBaseInterface * node_base_;
 
   rclcpp::Logger logger_;
-
-  rclcpp::Service<rcl_interfaces::srv::GetLoggerLevels>::SharedPtr get_loggers_service_;
-  rclcpp::Service<rcl_interfaces::srv::SetLoggerLevels>::SharedPtr set_loggers_service_;
 };
 
 }  // namespace node_interfaces

rclcpp/include/rclcpp/node_interfaces/node_logging_interface.hpp

@@ -19,7 +19,6 @@
 
 #include "rclcpp/logger.hpp"
 #include "rclcpp/macros.hpp"
-#include "rclcpp/node_interfaces/node_services_interface.hpp"
 #include "rclcpp/node_interfaces/detail/node_interfaces_helpers.hpp"
 #include "rclcpp/visibility_control.hpp"
 
@@ -55,13 +54,6 @@ public:
   virtual
   const char *
   get_logger_name() const = 0;
-
-  /// create logger services
-  RCLCPP_PUBLIC
-  virtual
-  void
-  create_logger_services(
-    node_interfaces::NodeServicesInterface::SharedPtr node_services) = 0;
 };
 
 }  // namespace node_interfaces

rclcpp/include/rclcpp/node_interfaces/node_type_descriptions.hpp

@@ -1,63 +0,0 @@
-// Copyright 2023 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__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_
-#define RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_
-
-#include <memory>
-
-#include "rclcpp/macros.hpp"
-#include "rclcpp/node_interfaces/node_base_interface.hpp"
-#include "rclcpp/node_interfaces/node_logging_interface.hpp"
-#include "rclcpp/node_interfaces/node_parameters_interface.hpp"
-#include "rclcpp/node_interfaces/node_services_interface.hpp"
-#include "rclcpp/node_interfaces/node_topics_interface.hpp"
-#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp"
-#include "rclcpp/visibility_control.hpp"
-
-namespace rclcpp
-{
-namespace node_interfaces
-{
-
-/// Implementation of the NodeTypeDescriptions part of the Node API.
-class NodeTypeDescriptions : public NodeTypeDescriptionsInterface
-{
-public:
-  RCLCPP_SMART_PTR_ALIASES_ONLY(NodeTypeDescriptions)
-
-  RCLCPP_PUBLIC
-  explicit NodeTypeDescriptions(
-    NodeBaseInterface::SharedPtr node_base,
-    NodeLoggingInterface::SharedPtr node_logging,
-    NodeParametersInterface::SharedPtr node_parameters,
-    NodeServicesInterface::SharedPtr node_services);
-
-  RCLCPP_PUBLIC
-  virtual
-  ~NodeTypeDescriptions();
-
-private:
-  RCLCPP_DISABLE_COPY(NodeTypeDescriptions)
-
-  // Pimpl hides helper types and functions used for wrapping a C service, which would be
-  // awkward to expose in this header.
-  class NodeTypeDescriptionsImpl;
-  std::unique_ptr<NodeTypeDescriptionsImpl> impl_;
-};
-
-}  // namespace node_interfaces
-}  // namespace rclcpp
-
-#endif  // RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_

rclcpp/include/rclcpp/node_interfaces/node_type_descriptions_interface.hpp

@@ -1,44 +0,0 @@
-// Copyright 2023 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__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_
-#define RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_
-
-#include "rclcpp/macros.hpp"
-#include "rclcpp/node_interfaces/detail/node_interfaces_helpers.hpp"
-#include "rclcpp/visibility_control.hpp"
-
-namespace rclcpp
-{
-namespace node_interfaces
-{
-
-/// Pure virtual interface class for the NodeTypeDescriptions part of the Node API.
-class NodeTypeDescriptionsInterface
-{
-public:
-  RCLCPP_SMART_PTR_ALIASES_ONLY(NodeTypeDescriptionsInterface)
-
-  RCLCPP_PUBLIC
-  virtual
-  ~NodeTypeDescriptionsInterface() = default;
-};
-
-}  // namespace node_interfaces
-}  // namespace rclcpp
-
-RCLCPP_NODE_INTERFACE_HELPERS_SUPPORT(
-  rclcpp::node_interfaces::NodeTypeDescriptionsInterface, type_descriptions)
-
-#endif  // RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_

rclcpp/include/rclcpp/node_options.hpp

@@ -43,7 +43,6 @@ public:
    *   - arguments = {}
    *   - parameter_overrides = {}
    *   - use_global_arguments = true
-   *   - enable_rosout = true
    *   - use_intra_process_comms = false
    *   - enable_topic_statistics = false
    *   - start_parameter_services = true
@@ -51,7 +50,6 @@ public:
    *   - clock_type = RCL_ROS_TIME
    *   - clock_qos = rclcpp::ClockQoS()
    *   - use_clock_thread = true
-   *   - enable_logger_service = false
    *   - rosout_qos = rclcpp::RosoutQoS()
    *   - parameter_event_qos = rclcpp::ParameterEventQoS
    *     - with history setting and depth from rmw_qos_profile_parameter_events
@@ -234,24 +232,6 @@ public:
   NodeOptions &
   start_parameter_services(bool start_parameter_services);
 
-  /// Return the enable_logger_service flag.
-  RCLCPP_PUBLIC
-  bool
-  enable_logger_service() const;
-
-  /// Set the enable_logger_service flag, return this for logger idiom.
-  /**
-   * If true, ROS services are created to allow external nodes to get
-   * and set logger levels of this node.
-   *
-   * If false, loggers will still be configured and set logger levels locally,
-   * but logger levels cannot be changed remotely .
-   *
-   */
-  RCLCPP_PUBLIC
-  NodeOptions &
-  enable_logger_service(bool enable_log_service);
-
   /// Return the start_parameter_event_publisher flag.
   RCLCPP_PUBLIC
   bool
@@ -441,8 +421,6 @@ private:
 
   bool use_clock_thread_ {true};
 
-  bool enable_logger_service_ {false};
-
   rclcpp::QoS parameter_event_qos_ = rclcpp::ParameterEventsQoS(
     rclcpp::QoSInitialization::from_rmw(rmw_qos_profile_parameter_events)
   );

rclcpp/include/rclcpp/service.hpp

@@ -482,14 +482,6 @@ public:
   {
     rcl_ret_t ret = rcl_send_response(get_service_handle().get(), &req_id, &response);
 
-    if (ret == RCL_RET_TIMEOUT) {
-      RCLCPP_WARN(
-        node_logger_.get_child("rclcpp"),
-        "failed to send response to %s (timeout): %s",
-        this->get_service_name(), rcl_get_error_string().str);
-      rcl_reset_error();
-      return;
-    }
     if (ret != RCL_RET_OK) {
       rclcpp::exceptions::throw_from_rcl_error(ret, "failed to send response");
     }

rclcpp/include/rclcpp/subscription_base.hpp

@@ -260,13 +260,13 @@ public:
   bool
   is_serialized() const;
 
-  /// Return the delivered message kind.
+  /// Return the type of the subscription.
   /**
    * \return `DeliveredMessageKind`, which adjusts how messages are received and delivered.
    */
   RCLCPP_PUBLIC
   DeliveredMessageKind
-  get_delivered_message_kind() const;
+  get_subscription_type() const;
 
   /// Get matching publisher count.
   /** \return The number of publishers on this topic. */
@@ -663,7 +663,7 @@ private:
   RCLCPP_DISABLE_COPY(SubscriptionBase)
 
   rosidl_message_type_support_t type_support_;
-  DeliveredMessageKind delivered_message_kind_;
+  DeliveredMessageKind delivered_message_type_;
 
   std::atomic<bool> subscription_in_use_by_wait_set_{false};
   std::atomic<bool> intra_process_subscription_waitable_in_use_by_wait_set_{false};

rclcpp/include/rclcpp/wait_set_policies/detail/storage_policy_common.hpp

@@ -104,7 +104,7 @@ protected:
       // TODO(wjwwood): support custom allocator, maybe restrict to polymorphic allocator
       rcl_get_default_allocator());
     if (RCL_RET_OK != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret);
+      rclcpp::exceptions::throw_from_rcl_error(ret, "Failed to create wait set");
     }
 
     // (Re)build the wait set for the first time.
@@ -192,8 +192,7 @@ protected:
       size_t services_from_waitables = 0;
       size_t events_from_waitables = 0;
       for (const auto & waitable_entry : waitables) {
-        auto waitable_ptr_pair = get_raw_pointer_from_smart_pointer(waitable_entry.waitable);
-        if (nullptr == waitable_ptr_pair.second) {
+        if (!waitable_entry.waitable) {
           // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
           if (HasStrongOwnership) {
             // This will not happen in fixed sized storage, as it holds
@@ -204,13 +203,13 @@ protected:
           needs_pruning_ = true;
           continue;
         }
-        rclcpp::Waitable & waitable = *waitable_ptr_pair.second;
-        subscriptions_from_waitables += waitable.get_number_of_ready_subscriptions();
-        guard_conditions_from_waitables += waitable.get_number_of_ready_guard_conditions();
-        timers_from_waitables += waitable.get_number_of_ready_timers();
-        clients_from_waitables += waitable.get_number_of_ready_clients();
-        services_from_waitables += waitable.get_number_of_ready_services();
-        events_from_waitables += waitable.get_number_of_ready_events();
+        const auto & waitable = waitable_entry.waitable;
+        subscriptions_from_waitables += waitable->get_number_of_ready_subscriptions();
+        guard_conditions_from_waitables += waitable->get_number_of_ready_guard_conditions();
+        timers_from_waitables += waitable->get_number_of_ready_timers();
+        clients_from_waitables += waitable->get_number_of_ready_clients();
+        services_from_waitables += waitable->get_number_of_ready_services();
+        events_from_waitables += waitable->get_number_of_ready_events();
       }
       rcl_ret_t ret = rcl_wait_set_resize(
         &rcl_wait_set_,
@@ -222,7 +221,7 @@ protected:
         events_from_waitables
       );
       if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(ret);
+        rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't resize the wait set");
       }
       was_resized = true;
       // Assumption: the calling code ensures this function is not called
@@ -238,15 +237,13 @@ protected:
     if (!was_resized) {
       rcl_ret_t ret = rcl_wait_set_clear(&rcl_wait_set_);
       if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(ret);
+        rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't clear the wait set");
       }
     }
 
     // Add subscriptions.
     for (const auto & subscription_entry : subscriptions) {
-      auto subscription_ptr_pair =
-        get_raw_pointer_from_smart_pointer(subscription_entry.subscription);
-      if (nullptr == subscription_ptr_pair.second) {
+      if (!subscription_entry.subscription) {
         // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
         if (HasStrongOwnership) {
           // This will not happen in fixed sized storage, as it holds
@@ -257,12 +254,13 @@ protected:
         needs_pruning_ = true;
         continue;
       }
+
       rcl_ret_t ret = rcl_wait_set_add_subscription(
         &rcl_wait_set_,
-        subscription_ptr_pair.second->get_subscription_handle().get(),
+        subscription_entry.subscription->get_subscription_handle().get(),
         nullptr);
       if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(ret);
+        rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't fill wait set");
       }
     }
 
@@ -271,8 +269,7 @@ protected:
       [this](const auto & inner_guard_conditions)
       {
         for (const auto & guard_condition : inner_guard_conditions) {
-          auto guard_condition_ptr_pair = get_raw_pointer_from_smart_pointer(guard_condition);
-          if (nullptr == guard_condition_ptr_pair.second) {
+          if (!guard_condition) {
             // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
             if (HasStrongOwnership) {
               // This will not happen in fixed sized storage, as it holds
@@ -285,10 +282,10 @@ protected:
           }
           rcl_ret_t ret = rcl_wait_set_add_guard_condition(
             &rcl_wait_set_,
-            &guard_condition_ptr_pair.second->get_rcl_guard_condition(),
+            &guard_condition->get_rcl_guard_condition(),
             nullptr);
           if (RCL_RET_OK != ret) {
-            rclcpp::exceptions::throw_from_rcl_error(ret);
+            rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't fill wait set");
           }
         }
       };
@@ -301,8 +298,7 @@ protected:
 
     // Add timers.
     for (const auto & timer : timers) {
-      auto timer_ptr_pair = get_raw_pointer_from_smart_pointer(timer);
-      if (nullptr == timer_ptr_pair.second) {
+      if (!timer) {
         // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
         if (HasStrongOwnership) {
           // This will not happen in fixed sized storage, as it holds
@@ -315,17 +311,16 @@ protected:
       }
       rcl_ret_t ret = rcl_wait_set_add_timer(
         &rcl_wait_set_,
-        timer_ptr_pair.second->get_timer_handle().get(),
+        timer->get_timer_handle().get(),
         nullptr);
       if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(ret);
+        rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't fill wait set");
       }
     }
 
     // Add clients.
     for (const auto & client : clients) {
-      auto client_ptr_pair = get_raw_pointer_from_smart_pointer(client);
-      if (nullptr == client_ptr_pair.second) {
+      if (!client) {
         // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
         if (HasStrongOwnership) {
           // This will not happen in fixed sized storage, as it holds
@@ -338,17 +333,17 @@ protected:
       }
       rcl_ret_t ret = rcl_wait_set_add_client(
         &rcl_wait_set_,
-        client_ptr_pair.second->get_client_handle().get(),
+        client->get_client_handle().get(),
         nullptr);
       if (RCL_RET_OK != ret) {
         rclcpp::exceptions::throw_from_rcl_error(ret);
       }
     }
 
+
     // Add services.
     for (const auto & service : services) {
-      auto service_ptr_pair = get_raw_pointer_from_smart_pointer(service);
-      if (nullptr == service_ptr_pair.second) {
+      if (!service) {
         // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
         if (HasStrongOwnership) {
           // This will not happen in fixed sized storage, as it holds
@@ -361,17 +356,16 @@ protected:
       }
       rcl_ret_t ret = rcl_wait_set_add_service(
         &rcl_wait_set_,
-        service_ptr_pair.second->get_service_handle().get(),
+        service->get_service_handle().get(),
         nullptr);
       if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(ret);
+        rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't fill wait set");
       }
     }
 
     // Add waitables.
     for (auto & waitable_entry : waitables) {
-      auto waitable_ptr_pair = get_raw_pointer_from_smart_pointer(waitable_entry.waitable);
-      if (nullptr == waitable_ptr_pair.second) {
+      if (!waitable_entry.waitable) {
         // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
         if (HasStrongOwnership) {
           // This will not happen in fixed sized storage, as it holds
@@ -382,8 +376,7 @@ protected:
         needs_pruning_ = true;
         continue;
       }
-      rclcpp::Waitable & waitable = *waitable_ptr_pair.second;
-      waitable.add_to_wait_set(&rcl_wait_set_);
+      waitable_entry.waitable->add_to_wait_set(&rcl_wait_set_);
     }
   }
 

rclcpp/include/rclcpp/wait_set_policies/dynamic_storage.hpp

@@ -204,15 +204,19 @@ public:
   void
   storage_rebuild_rcl_wait_set(const ArrayOfExtraGuardConditions & extra_guard_conditions)
   {
+    this->storage_acquire_ownerships();
+
     this->storage_rebuild_rcl_wait_set_with_sets(
-      subscriptions_,
-      guard_conditions_,
+      shared_subscriptions_,
+      shared_guard_conditions_,
       extra_guard_conditions,
-      timers_,
-      clients_,
-      services_,
-      waitables_
+      shared_timers_,
+      shared_clients_,
+      shared_services_,
+      shared_waitables_
     );
+
+    this->storage_release_ownerships();
   }
 
   template<class EntityT, class SequenceOfEntitiesT>
@@ -407,6 +411,7 @@ public:
         }
       };
     // Lock all the weak pointers and hold them until released.
+    lock_all(subscriptions_, shared_subscriptions_);
     lock_all(guard_conditions_, shared_guard_conditions_);
     lock_all(timers_, shared_timers_);
     lock_all(clients_, shared_clients_);
@@ -438,6 +443,7 @@ public:
           shared_ptr.reset();
         }
       };
+    reset_all(shared_subscriptions_);
     reset_all(shared_guard_conditions_);
     reset_all(shared_timers_);
     reset_all(shared_clients_);

rclcpp/include/rclcpp/wait_set_policies/sequential_synchronization.hpp

@@ -290,7 +290,7 @@ protected:
         return create_wait_result(WaitResultKind::Empty);
       } else {
         // Some other error case, throw.
-        rclcpp::exceptions::throw_from_rcl_error(ret);
+        rclcpp::exceptions::throw_from_rcl_error(ret, "rcl_wait() failed");
       }
     } while (should_loop());
 

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>21.0.6</version>
+  <version>20.0.0</version>
   <description>The ROS client library in C++.</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp/src/rclcpp/any_executable.cpp

@@ -16,13 +16,16 @@
 
 using rclcpp::AnyExecutable;
 
+RCLCPP_PUBLIC
 AnyExecutable::AnyExecutable()
 : subscription(nullptr),
   timer(nullptr),
   service(nullptr),
   client(nullptr),
+  waitable(nullptr),
   callback_group(nullptr),
-  node_base(nullptr)
+  node_base(nullptr),
+  data(nullptr)
 {}
 
 AnyExecutable::~AnyExecutable()

rclcpp/src/rclcpp/callback_group.cpp

@@ -66,6 +66,7 @@ CallbackGroup::size() const
     timer_ptrs_.size() +
     waitable_ptrs_.size();
 }
+
 void CallbackGroup::collect_all_ptrs(
   std::function<void(const rclcpp::SubscriptionBase::SharedPtr &)> sub_func,
   std::function<void(const rclcpp::ServiceBase::SharedPtr &)> service_func,

rclcpp/src/rclcpp/context.cpp

@@ -496,7 +496,7 @@ Context::sleep_for(const std::chrono::nanoseconds & nanoseconds)
       std::unique_lock<std::mutex> lock(interrupt_mutex_);
       auto start = std::chrono::steady_clock::now();
       // this will release the lock while waiting
-      interrupt_condition_variable_.wait_for(lock, time_left);
+      interrupt_condition_variable_.wait_for(lock, nanoseconds);
       time_left -= std::chrono::steady_clock::now() - start;
     }
   } while (time_left > std::chrono::nanoseconds::zero() && this->is_valid());

rclcpp/src/rclcpp/event_handler.cpp

@@ -39,6 +39,13 @@ UnsupportedEventTypeException::UnsupportedEventTypeException(
 
 EventHandlerBase::~EventHandlerBase()
 {
+  // Since the rmw event listener holds a reference to
+  // this callback, we need to clear it on destruction of this class.
+  // This clearing is not needed for other rclcpp entities like pub/subs, since
+  // they do own the underlying rmw entities, which are destroyed
+  // on their rclcpp destructors, thus no risk of dangling pointers.
+  clear_on_ready_callback();
+
   if (rcl_event_fini(&event_handle_) != RCL_RET_OK) {
     RCUTILS_LOG_ERROR_NAMED(
       "rclcpp",

rclcpp/src/rclcpp/executor.cpp

@@ -13,6 +13,8 @@
 // limitations under the License.
 
 #include <algorithm>
+#include <chrono>
+#include <iterator>
 #include <memory>
 #include <map>
 #include <string>
@@ -22,13 +24,14 @@
 
 #include "rcl/allocator.h"
 #include "rcl/error_handling.h"
+#include "rclcpp/executors/executor_notify_waitable.hpp"
+#include "rclcpp/subscription_wait_set_mask.hpp"
 #include "rcpputils/scope_exit.hpp"
 
 #include "rclcpp/dynamic_typesupport/dynamic_message.hpp"
 #include "rclcpp/exceptions.hpp"
 #include "rclcpp/executor.hpp"
 #include "rclcpp/guard_condition.hpp"
-#include "rclcpp/memory_strategy.hpp"
 #include "rclcpp/node.hpp"
 #include "rclcpp/utilities.hpp"
 
@@ -38,16 +41,24 @@
 
 using namespace std::chrono_literals;
 
-using rclcpp::exceptions::throw_from_rcl_error;
 using rclcpp::Executor;
-
 class rclcpp::ExecutorImplementation {};
 
+/// Mask to indicate to the waitset to only add the subscription.
+/// The events and intraprocess waitable are already added via the callback group.
+static constexpr rclcpp::SubscriptionWaitSetMask kDefaultSubscriptionMask = {true, false, false};
+
 Executor::Executor(const rclcpp::ExecutorOptions & options)
 : spinning(false),
   interrupt_guard_condition_(std::make_shared<rclcpp::GuardCondition>(options.context)),
   shutdown_guard_condition_(std::make_shared<rclcpp::GuardCondition>(options.context)),
-  memory_strategy_(options.memory_strategy),
+  notify_waitable_(std::make_shared<rclcpp::executors::ExecutorNotifyWaitable>(
+      [this]() {
+        this->entities_need_rebuild_.store(true);
+      })),
+  collector_(notify_waitable_),
+  wait_set_({}, {}, {}, {}, {}, {}, options.context),
+  current_notify_waitable_(notify_waitable_),
   impl_(std::make_unique<rclcpp::ExecutorImplementation>())
 {
   // Store the context for later use.
@@ -61,74 +72,56 @@ Executor::Executor(const rclcpp::ExecutorOptions & options)
       }
     });
 
-  // The number of guard conditions is always at least 2: 1 for the ctrl-c guard cond,
-  // and one for the executor's guard cond (interrupt_guard_condition_)
-  memory_strategy_->add_guard_condition(*shutdown_guard_condition_.get());
-
-  // Put the executor's guard condition in
-  memory_strategy_->add_guard_condition(*interrupt_guard_condition_.get());
-  rcl_allocator_t allocator = memory_strategy_->get_allocator();
-
-  rcl_ret_t ret = rcl_wait_set_init(
-    &wait_set_,
-    0, 2, 0, 0, 0, 0,
-    context_->get_rcl_context().get(),
-    allocator);
-  if (RCL_RET_OK != ret) {
-    RCUTILS_LOG_ERROR_NAMED(
-      "rclcpp",
-      "failed to create wait set: %s", rcl_get_error_string().str);
-    rcl_reset_error();
-    throw_from_rcl_error(ret, "Failed to create wait set in Executor constructor");
-  }
+  notify_waitable_->add_guard_condition(interrupt_guard_condition_);
+  notify_waitable_->add_guard_condition(shutdown_guard_condition_);
 }
 
 Executor::~Executor()
 {
-  // Disassociate all callback groups.
-  for (auto & pair : weak_groups_to_nodes_) {
-    auto group = pair.first.lock();
-    if (group) {
-      std::atomic_bool & has_executor = group->get_associated_with_executor_atomic();
-      has_executor.store(false);
-    }
-  }
-  // Disassociate all nodes.
-  std::for_each(
-    weak_nodes_.begin(), weak_nodes_.end(), []
-      (rclcpp::node_interfaces::NodeBaseInterface::WeakPtr weak_node_ptr) {
-      auto shared_node_ptr = weak_node_ptr.lock();
-      if (shared_node_ptr) {
-        std::atomic_bool & has_executor = shared_node_ptr->get_associated_with_executor_atomic();
-        has_executor.store(false);
-      }
+  std::lock_guard<std::mutex> guard(mutex_);
+
+  notify_waitable_->remove_guard_condition(interrupt_guard_condition_);
+  notify_waitable_->remove_guard_condition(shutdown_guard_condition_);
+
+  std::cout << "Executor::~Executor" << std::endl;
+
+  current_collection_.timers.update(
+    {}, {},
+    [this](auto timer) {
+      std::cout << "remove_timer(" << timer.get() << ")" << std::endl;
+      wait_set_.remove_timer(timer);
     });
-  weak_nodes_.clear();
-  weak_groups_associated_with_executor_to_nodes_.clear();
-  weak_groups_to_nodes_associated_with_executor_.clear();
-  weak_groups_to_nodes_.clear();
-  for (const auto & pair : weak_groups_to_guard_conditions_) {
-    auto guard_condition = pair.second;
-    memory_strategy_->remove_guard_condition(guard_condition);
-  }
-  weak_groups_to_guard_conditions_.clear();
 
-  for (const auto & pair : weak_nodes_to_guard_conditions_) {
-    auto guard_condition = pair.second;
-    memory_strategy_->remove_guard_condition(guard_condition);
-  }
-  weak_nodes_to_guard_conditions_.clear();
+  current_collection_.subscriptions.update(
+    {}, {},
+    [this](auto subscription) {
+      std::cout << "remove_subscription(" << subscription.get() << ")" << std::endl;
+      wait_set_.remove_subscription(subscription, kDefaultSubscriptionMask);
+    });
 
-  // Finalize the wait set.
-  if (rcl_wait_set_fini(&wait_set_) != RCL_RET_OK) {
-    RCUTILS_LOG_ERROR_NAMED(
-      "rclcpp",
-      "failed to destroy wait set: %s", rcl_get_error_string().str);
-    rcl_reset_error();
-  }
-  // Remove and release the sigint guard condition
-  memory_strategy_->remove_guard_condition(shutdown_guard_condition_.get());
-  memory_strategy_->remove_guard_condition(interrupt_guard_condition_.get());
+  current_collection_.clients.update(
+    {}, {},
+    [this](auto client) {
+      std::cout << "remove_client(" << client.get() << ")" << std::endl;
+      wait_set_.remove_client(client);});
+
+  current_collection_.services.update(
+    {}, {},
+    [this](auto service) {
+      std::cout << "remove_service(" << service.get() << ")" << std::endl;
+      wait_set_.remove_service(service);});
+
+  current_collection_.guard_conditions.update(
+    {}, {},
+    [this](auto guard_condition) {
+      std::cout << "remove_guard_condition(" << guard_condition.get() << ")" << std::endl;
+        wait_set_.remove_guard_condition(guard_condition);});
+
+  current_collection_.waitables.update(
+    {}, {},
+    [this](auto waitable) {
+      std::cout << "remove_waitable(" << waitable.get() << ")" << std::endl;
+      wait_set_.remove_waitable(waitable);});
 
   // Remove shutdown callback handle registered to Context
   if (!context_->remove_on_shutdown_callback(shutdown_callback_handle_)) {
@@ -142,95 +135,39 @@ Executor::~Executor()
 std::vector<rclcpp::CallbackGroup::WeakPtr>
 Executor::get_all_callback_groups()
 {
-  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
-  std::lock_guard<std::mutex> guard{mutex_};
-  for (const auto & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
-    groups.push_back(group_node_ptr.first);
-  }
-  for (auto const & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
-    groups.push_back(group_node_ptr.first);
-  }
-  return groups;
+  this->collector_.update_collections();
+  return this->collector_.get_all_callback_groups();
 }
 
 std::vector<rclcpp::CallbackGroup::WeakPtr>
 Executor::get_manually_added_callback_groups()
 {
-  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
-  std::lock_guard<std::mutex> guard{mutex_};
-  for (auto const & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
-    groups.push_back(group_node_ptr.first);
-  }
-  return groups;
+  this->collector_.update_collections();
+  return this->collector_.get_manually_added_callback_groups();
 }
 
 std::vector<rclcpp::CallbackGroup::WeakPtr>
 Executor::get_automatically_added_callback_groups_from_nodes()
 {
-  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
-  std::lock_guard<std::mutex> guard{mutex_};
-  for (auto const & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
-    groups.push_back(group_node_ptr.first);
-  }
-  return groups;
+  this->collector_.update_collections();
+  return this->collector_.get_automatically_added_callback_groups();
 }
 
 void
-Executor::add_callback_groups_from_nodes_associated_to_executor()
-{
-  for (auto & weak_node : weak_nodes_) {
-    auto node = weak_node.lock();
-    if (node) {
-      node->for_each_callback_group(
-        [this, node](rclcpp::CallbackGroup::SharedPtr shared_group_ptr)
-        {
-          if (
-            shared_group_ptr->automatically_add_to_executor_with_node() &&
-            !shared_group_ptr->get_associated_with_executor_atomic().load())
-          {
-            this->add_callback_group_to_map(
-              shared_group_ptr,
-              node,
-              weak_groups_to_nodes_associated_with_executor_,
-              true);
-          }
-        });
-    }
-  }
-}
-
-void
-Executor::add_callback_group_to_map(
+Executor::add_callback_group(
   rclcpp::CallbackGroup::SharedPtr group_ptr,
   rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
   bool notify)
 {
-  // If the callback_group already has an executor
-  std::atomic_bool & has_executor = group_ptr->get_associated_with_executor_atomic();
-  if (has_executor.exchange(true)) {
-    throw std::runtime_error("Callback group has already been added to an executor.");
-  }
+  (void) node_ptr;
+  this->collector_.add_callback_group(group_ptr);
 
-  rclcpp::CallbackGroup::WeakPtr weak_group_ptr = group_ptr;
-  auto insert_info =
-    weak_groups_to_nodes.insert(std::make_pair(weak_group_ptr, node_ptr));
-  bool was_inserted = insert_info.second;
-  if (!was_inserted) {
-    throw std::runtime_error("Callback group was already added to executor.");
-  }
-  // Also add to the map that contains all callback groups
-  weak_groups_to_nodes_.insert(std::make_pair(weak_group_ptr, node_ptr));
-
-  if (node_ptr->get_context()->is_valid()) {
-    auto callback_group_guard_condition = group_ptr->get_notify_guard_condition();
-    weak_groups_to_guard_conditions_[weak_group_ptr] = callback_group_guard_condition.get();
-    // Add the callback_group's notify condition to the guard condition handles
-    memory_strategy_->add_guard_condition(*callback_group_guard_condition);
+  if (!spinning.load()) {
+    std::lock_guard<std::mutex> guard(mutex_);
+    this->collect_entities();
   }
 
   if (notify) {
-    // Interrupt waiting to handle new node
     try {
       interrupt_guard_condition_->trigger();
     } catch (const rclcpp::exceptions::RCLError & ex) {
@@ -241,91 +178,23 @@ Executor::add_callback_group_to_map(
   }
 }
 
-void
-Executor::add_callback_group(
-  rclcpp::CallbackGroup::SharedPtr group_ptr,
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-  bool notify)
-{
-  std::lock_guard<std::mutex> guard{mutex_};
-  this->add_callback_group_to_map(
-    group_ptr,
-    node_ptr,
-    weak_groups_associated_with_executor_to_nodes_,
-    notify);
-}
-
 void
 Executor::add_node(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify)
 {
-  // If the node already has an executor
-  std::atomic_bool & has_executor = node_ptr->get_associated_with_executor_atomic();
-  if (has_executor.exchange(true)) {
-    throw std::runtime_error(
-            std::string("Node '") + node_ptr->get_fully_qualified_name() +
-            "' has already been added to an executor.");
+  this->collector_.add_node(node_ptr);
+
+  if (!spinning.load()) {
+    std::lock_guard<std::mutex> guard(mutex_);
+    this->collect_entities();
   }
-  std::lock_guard<std::mutex> guard{mutex_};
-  node_ptr->for_each_callback_group(
-    [this, node_ptr, notify](rclcpp::CallbackGroup::SharedPtr group_ptr)
-    {
-      if (!group_ptr->get_associated_with_executor_atomic().load() &&
-      group_ptr->automatically_add_to_executor_with_node())
-      {
-        this->add_callback_group_to_map(
-          group_ptr,
-          node_ptr,
-          weak_groups_to_nodes_associated_with_executor_,
-          notify);
-      }
-    });
 
-  const auto gc = node_ptr->get_shared_notify_guard_condition();
-  weak_nodes_to_guard_conditions_[node_ptr] = gc.get();
-  // Add the node's notify condition to the guard condition handles
-  memory_strategy_->add_guard_condition(*gc);
-  weak_nodes_.push_back(node_ptr);
-}
-
-void
-Executor::remove_callback_group_from_map(
-  rclcpp::CallbackGroup::SharedPtr group_ptr,
-  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
-  bool notify)
-{
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr;
-  rclcpp::CallbackGroup::WeakPtr weak_group_ptr = group_ptr;
-  auto iter = weak_groups_to_nodes.find(weak_group_ptr);
-  if (iter != weak_groups_to_nodes.end()) {
-    node_ptr = iter->second.lock();
-    if (node_ptr == nullptr) {
-      throw std::runtime_error("Node must not be deleted before its callback group(s).");
-    }
-    weak_groups_to_nodes.erase(iter);
-    weak_groups_to_nodes_.erase(group_ptr);
-    std::atomic_bool & has_executor = group_ptr->get_associated_with_executor_atomic();
-    has_executor.store(false);
-  } else {
-    throw std::runtime_error("Callback group needs to be associated with executor.");
-  }
-  // If the node was matched and removed, interrupt waiting.
-  if (!has_node(node_ptr, weak_groups_to_nodes_associated_with_executor_) &&
-    !has_node(node_ptr, weak_groups_associated_with_executor_to_nodes_))
-  {
-    auto iter = weak_groups_to_guard_conditions_.find(weak_group_ptr);
-    if (iter != weak_groups_to_guard_conditions_.end()) {
-      memory_strategy_->remove_guard_condition(iter->second);
-    }
-    weak_groups_to_guard_conditions_.erase(weak_group_ptr);
-
-    if (notify) {
-      try {
-        interrupt_guard_condition_->trigger();
-      } catch (const rclcpp::exceptions::RCLError & ex) {
-        throw std::runtime_error(
-                std::string(
-                  "Failed to trigger guard condition on callback group remove: ") + ex.what());
-      }
+  if (notify) {
+    try {
+      interrupt_guard_condition_->trigger();
+    } catch (const rclcpp::exceptions::RCLError & ex) {
+      throw std::runtime_error(
+              std::string(
+                "Failed to trigger guard condition on node add: ") + ex.what());
     }
   }
 }
@@ -335,11 +204,21 @@ Executor::remove_callback_group(
   rclcpp::CallbackGroup::SharedPtr group_ptr,
   bool notify)
 {
-  std::lock_guard<std::mutex> guard{mutex_};
-  this->remove_callback_group_from_map(
-    group_ptr,
-    weak_groups_associated_with_executor_to_nodes_,
-    notify);
+  this->collector_.remove_callback_group(group_ptr);
+
+  if (!spinning.load()) {
+    std::lock_guard<std::mutex> guard(mutex_);
+    this->collect_entities();
+  }
+  if (notify) {
+    try {
+      interrupt_guard_condition_->trigger();
+    } catch (const rclcpp::exceptions::RCLError & ex) {
+      throw std::runtime_error(
+              std::string(
+                "Failed to trigger guard condition on callback group remove: ") + ex.what());
+    }
+  }
 }
 
 void
@@ -351,48 +230,22 @@ Executor::add_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify)
 void
 Executor::remove_node(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify)
 {
-  if (!node_ptr->get_associated_with_executor_atomic().load()) {
-    throw std::runtime_error("Node needs to be associated with an executor.");
+  this->collector_.remove_node(node_ptr);
+
+  if (!spinning.load()) {
+    std::lock_guard<std::mutex> guard(mutex_);
+    this->collect_entities();
   }
 
-  std::lock_guard<std::mutex> guard{mutex_};
-  bool found_node = false;
-  auto node_it = weak_nodes_.begin();
-  while (node_it != weak_nodes_.end()) {
-    bool matched = (node_it->lock() == node_ptr);
-    if (matched) {
-      found_node = true;
-      node_it = weak_nodes_.erase(node_it);
-    } else {
-      ++node_it;
+  if (notify) {
+    try {
+      interrupt_guard_condition_->trigger();
+    } catch (const rclcpp::exceptions::RCLError & ex) {
+      throw std::runtime_error(
+              std::string(
+                "Failed to trigger guard condition on node remove: ") + ex.what());
     }
   }
-  if (!found_node) {
-    throw std::runtime_error("Node needs to be associated with this executor.");
-  }
-
-  for (auto it = weak_groups_to_nodes_associated_with_executor_.begin();
-    it != weak_groups_to_nodes_associated_with_executor_.end(); )
-  {
-    auto weak_node_ptr = it->second;
-    auto shared_node_ptr = weak_node_ptr.lock();
-    auto group_ptr = it->first.lock();
-
-    // Increment iterator before removing in case it's invalidated
-    it++;
-    if (shared_node_ptr == node_ptr) {
-      remove_callback_group_from_map(
-        group_ptr,
-        weak_groups_to_nodes_associated_with_executor_,
-        notify);
-    }
-  }
-
-  memory_strategy_->remove_guard_condition(node_ptr->get_shared_notify_guard_condition().get());
-  weak_nodes_to_guard_conditions_.erase(node_ptr);
-
-  std::atomic_bool & has_executor = node_ptr->get_associated_with_executor_atomic();
-  has_executor.store(false);
 }
 
 void
@@ -459,21 +312,44 @@ Executor::spin_some_impl(std::chrono::nanoseconds max_duration, bool exhaustive)
     throw std::runtime_error("spin_some() called while already spinning");
   }
   RCPPUTILS_SCOPE_EXIT(this->spinning.store(false); );
-  bool work_available = false;
+
+  size_t work_in_queue = 0;
+  bool has_waited = false;
+
+  {
+    std::lock_guard<std::mutex> lock(mutex_);
+    work_in_queue = ready_executables_.size();
+  }
+  // The logic below is to guarantee that we:
+  // a) run all of the work in the queue before we spin the first time
+  // b) spin at least once
+  // c) run all of the work in the queue after we spin
+
   while (rclcpp::ok(context_) && spinning.load() && max_duration_not_elapsed()) {
     AnyExecutable any_exec;
-    if (!work_available) {
-      wait_for_work(std::chrono::milliseconds::zero());
-    }
-    if (get_next_ready_executable(any_exec)) {
-      execute_any_executable(any_exec);
-      work_available = true;
-    } else {
-      if (!work_available || !exhaustive) {
-        break;
+    if (work_in_queue > 0) {
+      // If there is work in the queue, then execute it
+      // This covers the case that there are things left in the queue from a
+      // previous spin.
+      if (get_next_ready_executable(any_exec)) {
+        execute_any_executable(any_exec);
+      }
+    } else if (!has_waited && !work_in_queue) {
+      // Once the ready queue is empty, then we need to wait at least once.
+      wait_for_work(std::chrono::milliseconds(0));
+      has_waited = true;
+    } else if (has_waited && !work_in_queue) {
+      // Once we have emptied the ready queue, but have already waited:
+      if (!exhaustive) {
+        // In the case of spin some, then we can exit
+        break;
+      } else {
+        // In the case of spin all, then we will allow ourselves to wait again.
+        has_waited = false;
       }
-      work_available = false;
     }
+    std::lock_guard<std::mutex> lock(mutex_);
+    work_in_queue = ready_executables_.size();
   }
 }
 
@@ -508,22 +384,13 @@ Executor::cancel()
   }
 }
 
-void
-Executor::set_memory_strategy(rclcpp::memory_strategy::MemoryStrategy::SharedPtr memory_strategy)
-{
-  if (memory_strategy == nullptr) {
-    throw std::runtime_error("Received NULL memory strategy in executor.");
-  }
-  std::lock_guard<std::mutex> guard{mutex_};
-  memory_strategy_ = memory_strategy;
-}
-
 void
 Executor::execute_any_executable(AnyExecutable & any_exec)
 {
   if (!spinning.load()) {
     return;
   }
+
   if (any_exec.timer) {
     TRACEPOINT(
       rclcpp_executor_execute,
@@ -545,16 +412,10 @@ Executor::execute_any_executable(AnyExecutable & any_exec)
   if (any_exec.waitable) {
     any_exec.waitable->execute(any_exec.data);
   }
+
   // Reset the callback_group, regardless of type
-  any_exec.callback_group->can_be_taken_from().store(true);
-  // Wake the wait, because it may need to be recalculated or work that
-  // was previously blocked is now available.
-  try {
-    interrupt_guard_condition_->trigger();
-  } catch (const rclcpp::exceptions::RCLError & ex) {
-    throw std::runtime_error(
-            std::string(
-              "Failed to trigger guard condition from execute_any_executable: ") + ex.what());
+  if (any_exec.callback_group) {
+    any_exec.callback_group->can_be_taken_from().store(true);
   }
 }
 
@@ -603,7 +464,7 @@ Executor::execute_subscription(rclcpp::SubscriptionBase::SharedPtr subscription)
   rclcpp::MessageInfo message_info;
   message_info.get_rmw_message_info().from_intra_process = false;
 
-  switch (subscription->get_delivered_message_kind()) {
+  switch (subscription->get_subscription_type()) {
     // Deliver ROS message
     case rclcpp::DeliveredMessageKind::ROS_MESSAGE:
       {
@@ -653,11 +514,6 @@ Executor::execute_subscription(rclcpp::SubscriptionBase::SharedPtr subscription)
             subscription->get_topic_name(),
             [&]() {return subscription->take_type_erased(message.get(), message_info);},
             [&]() {subscription->handle_message(message, message_info);});
-          // TODO(clalancette): In the case that the user is using the MessageMemoryPool,
-          // and they take a shared_ptr reference to the message in the callback, this can
-          // inadvertently return the message to the pool when the user is still using it.
-          // This is a bug that needs to be fixed in the pool, and we should probably have
-          // a custom deleter for the message that actually does the return_message().
           subscription->return_message(message);
         }
         break;
@@ -728,228 +584,117 @@ Executor::execute_client(
     [&]() {client->handle_response(request_header, response);});
 }
 
+void
+Executor::collect_entities()
+{
+  // Get the current list of available waitables from the collector.
+  rclcpp::executors::ExecutorEntitiesCollection collection;
+  this->collector_.update_collections();
+  auto callback_groups = this->collector_.get_all_callback_groups();
+  rclcpp::executors::build_entities_collection(callback_groups, collection);
+
+  // Make a copy of notify waitable so we can continue to mutate the original
+  // one outside of the execute loop.
+  // This prevents the collection of guard conditions in the waitable from changing
+  // while we are waiting on it.
+  if (notify_waitable_) {
+    current_notify_waitable_ = std::make_shared<rclcpp::executors::ExecutorNotifyWaitable>(
+      *notify_waitable_);
+    auto notify_waitable = std::static_pointer_cast<rclcpp::Waitable>(current_notify_waitable_);
+    collection.waitables.insert({notify_waitable.get(), {notify_waitable, {}}});
+  }
+
+  std::cout << "current_collection.timers: " << current_collection_.timers.size() << std::endl;
+  std::cout << "next_collection.timers: " << collection.timers.size() << std::endl;
+  // Update each of the groups of entities in the current collection, adding or removing
+  // from the wait set as necessary.
+  current_collection_.timers.update(
+    collection.timers,
+    [this](auto timer) {
+        std::cout << "add_timer(" << timer << ")" << std::endl;
+        wait_set_.add_timer(timer);},
+    [this](auto timer) {
+        std::cout << "remove_timer(" << timer << ")" << std::endl;
+        wait_set_.remove_timer(timer);});
+
+  std::cout << "current_collection.subscriptions: " << current_collection_.subscriptions.size() << std::endl;
+  std::cout << "next_collection.subscriptions: " << collection.subscriptions.size() << std::endl;
+  current_collection_.subscriptions.update(
+    collection.subscriptions,
+    [this](auto subscription) {
+      std::cout << "add_subscription(" << subscription << ")" << std::endl;
+      wait_set_.add_subscription(subscription, kDefaultSubscriptionMask);
+    },
+    [this](auto subscription) {
+      std::cout << "remove_subscription(" << subscription << ")" << std::endl;
+      wait_set_.remove_subscription(subscription, kDefaultSubscriptionMask);
+    });
+
+  current_collection_.clients.update(
+    collection.clients,
+    [this](auto client) {wait_set_.add_client(client);},
+    [this](auto client) {wait_set_.remove_client(client);});
+
+  current_collection_.services.update(
+    collection.services,
+    [this](auto service) {wait_set_.add_service(service);},
+    [this](auto service) {wait_set_.remove_service(service);});
+
+  current_collection_.guard_conditions.update(
+    collection.guard_conditions,
+    [this](auto guard_condition) {wait_set_.add_guard_condition(guard_condition);},
+    [this](auto guard_condition) {wait_set_.remove_guard_condition(guard_condition);});
+
+  current_collection_.waitables.update(
+    collection.waitables,
+    [this](auto waitable) {
+      wait_set_.add_waitable(waitable);
+    },
+    [this](auto waitable) {wait_set_.remove_waitable(waitable);});
+
+  this->entities_need_rebuild_.store(false);
+}
+
 void
 Executor::wait_for_work(std::chrono::nanoseconds timeout)
 {
   TRACEPOINT(rclcpp_executor_wait_for_work, timeout.count());
-  {
-    std::lock_guard<std::mutex> guard(mutex_);
 
-    // Check weak_nodes_ to find any callback group that is not owned
-    // by an executor and add it to the list of callbackgroups for
-    // collect entities. Also exchange to false so it is not
-    // allowed to add to another executor
-    add_callback_groups_from_nodes_associated_to_executor();
-
-    // Collect the subscriptions and timers to be waited on
-    memory_strategy_->clear_handles();
-    bool has_invalid_weak_groups_or_nodes =
-      memory_strategy_->collect_entities(weak_groups_to_nodes_);
-
-    if (has_invalid_weak_groups_or_nodes) {
-      std::vector<rclcpp::CallbackGroup::WeakPtr> invalid_group_ptrs;
-      for (auto pair : weak_groups_to_nodes_) {
-        auto weak_group_ptr = pair.first;
-        auto weak_node_ptr = pair.second;
-        if (weak_group_ptr.expired() || weak_node_ptr.expired()) {
-          invalid_group_ptrs.push_back(weak_group_ptr);
-          auto node_guard_pair = weak_nodes_to_guard_conditions_.find(weak_node_ptr);
-          if (node_guard_pair != weak_nodes_to_guard_conditions_.end()) {
-            auto guard_condition = node_guard_pair->second;
-            weak_nodes_to_guard_conditions_.erase(weak_node_ptr);
-            memory_strategy_->remove_guard_condition(guard_condition);
-          }
-        }
-      }
-      std::for_each(
-        invalid_group_ptrs.begin(), invalid_group_ptrs.end(),
-        [this](rclcpp::CallbackGroup::WeakPtr group_ptr) {
-          if (weak_groups_to_nodes_associated_with_executor_.find(group_ptr) !=
-          weak_groups_to_nodes_associated_with_executor_.end())
-          {
-            weak_groups_to_nodes_associated_with_executor_.erase(group_ptr);
-          }
-          if (weak_groups_associated_with_executor_to_nodes_.find(group_ptr) !=
-          weak_groups_associated_with_executor_to_nodes_.end())
-          {
-            weak_groups_associated_with_executor_to_nodes_.erase(group_ptr);
-          }
-          auto callback_guard_pair = weak_groups_to_guard_conditions_.find(group_ptr);
-          if (callback_guard_pair != weak_groups_to_guard_conditions_.end()) {
-            auto guard_condition = callback_guard_pair->second;
-            weak_groups_to_guard_conditions_.erase(group_ptr);
-            memory_strategy_->remove_guard_condition(guard_condition);
-          }
-          weak_groups_to_nodes_.erase(group_ptr);
-        });
-    }
-
-    // clear wait set
-    rcl_ret_t ret = rcl_wait_set_clear(&wait_set_);
-    if (ret != RCL_RET_OK) {
-      throw_from_rcl_error(ret, "Couldn't clear wait set");
-    }
-
-    // The size of waitables are accounted for in size of the other entities
-    ret = rcl_wait_set_resize(
-      &wait_set_, memory_strategy_->number_of_ready_subscriptions(),
-      memory_strategy_->number_of_guard_conditions(), memory_strategy_->number_of_ready_timers(),
-      memory_strategy_->number_of_ready_clients(), memory_strategy_->number_of_ready_services(),
-      memory_strategy_->number_of_ready_events());
-    if (RCL_RET_OK != ret) {
-      throw_from_rcl_error(ret, "Couldn't resize the wait set");
-    }
-
-    if (!memory_strategy_->add_handles_to_wait_set(&wait_set_)) {
-      throw std::runtime_error("Couldn't fill wait set");
-    }
+  std::lock_guard<std::mutex> guard(mutex_);
+  if (current_collection_.empty() || this->entities_need_rebuild_.load()) {
+    this->collect_entities();
   }
 
-  rcl_ret_t status =
-    rcl_wait(&wait_set_, std::chrono::duration_cast<std::chrono::nanoseconds>(timeout).count());
-  if (status == RCL_RET_WAIT_SET_EMPTY) {
+  auto wait_result = wait_set_.wait(timeout);
+  if (wait_result.kind() == WaitResultKind::Empty) {
     RCUTILS_LOG_WARN_NAMED(
       "rclcpp",
-      "empty wait set received in rcl_wait(). This should never happen.");
-  } else if (status != RCL_RET_OK && status != RCL_RET_TIMEOUT) {
-    using rclcpp::exceptions::throw_from_rcl_error;
-    throw_from_rcl_error(status, "rcl_wait() failed");
+      "empty wait set received in wait(). This should never happen.");
   }
-
-  // check the null handles in the wait set and remove them from the handles in memory strategy
-  // for callback-based entities
-  std::lock_guard<std::mutex> guard(mutex_);
-  memory_strategy_->remove_null_handles(&wait_set_);
-}
-
-rclcpp::node_interfaces::NodeBaseInterface::SharedPtr
-Executor::get_node_by_group(
-  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
-  weak_groups_to_nodes,
-  rclcpp::CallbackGroup::SharedPtr group)
-{
-  if (!group) {
-    return nullptr;
-  }
-  rclcpp::CallbackGroup::WeakPtr weak_group_ptr(group);
-  const auto finder = weak_groups_to_nodes.find(weak_group_ptr);
-  if (finder != weak_groups_to_nodes.end()) {
-    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr = finder->second.lock();
-    return node_ptr;
-  }
-  return nullptr;
-}
-
-rclcpp::CallbackGroup::SharedPtr
-Executor::get_group_by_timer(rclcpp::TimerBase::SharedPtr timer)
-{
-  std::lock_guard<std::mutex> guard{mutex_};
-  for (const auto & pair : weak_groups_associated_with_executor_to_nodes_) {
-    auto group = pair.first.lock();
-    if (!group) {
-      continue;
-    }
-    auto timer_ref = group->find_timer_ptrs_if(
-      [timer](const rclcpp::TimerBase::SharedPtr & timer_ptr) -> bool {
-        return timer_ptr == timer;
-      });
-    if (timer_ref) {
-      return group;
-    }
-  }
-
-  for (const auto & pair : weak_groups_to_nodes_associated_with_executor_) {
-    auto group = pair.first.lock();
-    if (!group) {
-      continue;
-    }
-    auto timer_ref = group->find_timer_ptrs_if(
-      [timer](const rclcpp::TimerBase::SharedPtr & timer_ptr) -> bool {
-        return timer_ptr == timer;
-      });
-    if (timer_ref) {
-      return group;
-    }
-  }
-  return nullptr;
+  rclcpp::executors::ready_executables(current_collection_, wait_result, ready_executables_);
 }
 
 bool
 Executor::get_next_ready_executable(AnyExecutable & any_executable)
-{
-  bool success = get_next_ready_executable_from_map(any_executable, weak_groups_to_nodes_);
-  return success;
-}
-
-bool
-Executor::get_next_ready_executable_from_map(
-  AnyExecutable & any_executable,
-  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
-  weak_groups_to_nodes)
 {
   TRACEPOINT(rclcpp_executor_get_next_ready);
-  bool success = false;
-  std::lock_guard<std::mutex> guard{mutex_};
-  // Check the timers to see if there are any that are ready
-  memory_strategy_->get_next_timer(any_executable, weak_groups_to_nodes);
-  if (any_executable.timer) {
-    success = true;
-  }
-  if (!success) {
-    // Check the subscriptions to see if there are any that are ready
-    memory_strategy_->get_next_subscription(any_executable, weak_groups_to_nodes);
-    if (any_executable.subscription) {
-      success = true;
-    }
-  }
-  if (!success) {
-    // Check the services to see if there are any that are ready
-    memory_strategy_->get_next_service(any_executable, weak_groups_to_nodes);
-    if (any_executable.service) {
-      success = true;
-    }
-  }
-  if (!success) {
-    // Check the clients to see if there are any that are ready
-    memory_strategy_->get_next_client(any_executable, weak_groups_to_nodes);
-    if (any_executable.client) {
-      success = true;
-    }
-  }
-  if (!success) {
-    // Check the waitables to see if there are any that are ready
-    memory_strategy_->get_next_waitable(any_executable, weak_groups_to_nodes);
-    if (any_executable.waitable) {
-      any_executable.data = any_executable.waitable->take_data();
-      success = true;
-    }
-  }
-  // At this point any_executable should be valid with either a valid subscription
-  // or a valid timer, or it should be a null shared_ptr
-  if (success) {
-    rclcpp::CallbackGroup::WeakPtr weak_group_ptr = any_executable.callback_group;
-    auto iter = weak_groups_to_nodes.find(weak_group_ptr);
-    if (iter == weak_groups_to_nodes.end()) {
-      success = false;
-    }
+
+  std::lock_guard<std::mutex> guard(mutex_);
+  if (ready_executables_.size() == 0) {
+    return false;
   }
 
-  if (success) {
-    // If it is valid, check to see if the group is mutually exclusive or
-    // not, then mark it accordingly ..Check if the callback_group belongs to this executor
-    if (any_executable.callback_group && any_executable.callback_group->type() == \
-      CallbackGroupType::MutuallyExclusive)
-    {
-      // It should not have been taken otherwise
-      assert(any_executable.callback_group->can_be_taken_from().load());
-      // Set to false to indicate something is being run from this group
-      // This is reset to true either when the any_executable is executed or when the
-      // any_executable is destructued
-      any_executable.callback_group->can_be_taken_from().store(false);
-    }
+  any_executable = ready_executables_.front();
+  ready_executables_.pop_front();
+
+  if (any_executable.callback_group &&
+    any_executable.callback_group->type() == CallbackGroupType::MutuallyExclusive)
+  {
+    assert(any_executable.callback_group->can_be_taken_from().load());
+    any_executable.callback_group->can_be_taken_from().store(false);
   }
-  // If there is no ready executable, return false
-  return success;
+
+  return true;
 }
 
 bool
@@ -972,22 +717,6 @@ Executor::get_next_executable(AnyExecutable & any_executable, std::chrono::nanos
   return success;
 }
 
-// Returns true iff the weak_groups_to_nodes map has node_ptr as the value in any of its entry.
-bool
-Executor::has_node(
-  const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
-  weak_groups_to_nodes) const
-{
-  return std::find_if(
-    weak_groups_to_nodes.begin(),
-    weak_groups_to_nodes.end(),
-    [&](const WeakCallbackGroupsToNodesMap::value_type & other) -> bool {
-      auto other_ptr = other.second.lock();
-      return other_ptr == node_ptr;
-    }) != weak_groups_to_nodes.end();
-}
-
 bool
 Executor::is_spinning()
 {

rclcpp/src/rclcpp/executors/executor_entities_collection.cpp

@@ -222,9 +222,7 @@ ready_executables(
     executables.push_back(exec);
     added++;
   }
-
   return added;
 }
-
 }  // namespace executors
 }  // namespace rclcpp

rclcpp/src/rclcpp/executors/executor_entities_collector.cpp

@@ -105,7 +105,8 @@ void
 ExecutorEntitiesCollector::remove_node(
   rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr)
 {
-  if (!node_ptr->get_associated_with_executor_atomic().load()) {
+  std::atomic_bool & has_executor = node_ptr->get_associated_with_executor_atomic();
+  if (!has_executor.exchange(false)) {
     throw std::runtime_error(
             std::string("Node '") + node_ptr->get_fully_qualified_name() +
             "' needs to be associated with an executor.");
@@ -161,7 +162,6 @@ ExecutorEntitiesCollector::remove_callback_group(rclcpp::CallbackGroup::SharedPt
     throw std::runtime_error("Node must not be deleted before its callback group(s).");
   }
   */
-
   auto weak_group_ptr = rclcpp::CallbackGroup::WeakPtr(group_ptr);
   std::lock_guard<std::mutex> lock(mutex_);
   bool associated = manually_added_groups_.count(group_ptr) != 0;
@@ -314,7 +314,11 @@ ExecutorEntitiesCollector::process_queues()
     if (node_it != weak_nodes_.end()) {
       remove_weak_node(node_it);
     } else {
-      throw std::runtime_error("Node needs to be associated with this executor.");
+      // The node may have been destroyed and removed from the colletion before
+      // we processed the queues.  Don't throw if the pointer is already expired.
+      if (!weak_node_ptr.expired()) {
+        throw std::runtime_error("Node needs to be associated with this executor.");
+      }
     }
 
     auto node_ptr = weak_node_ptr.lock();

rclcpp/src/rclcpp/executors/executor_notify_waitable.cpp

@@ -46,20 +46,18 @@ void
 ExecutorNotifyWaitable::add_to_wait_set(rcl_wait_set_t * wait_set)
 {
   std::lock_guard<std::mutex> lock(guard_condition_mutex_);
-
   for (auto weak_guard_condition : this->notify_guard_conditions_) {
     auto guard_condition = weak_guard_condition.lock();
-    if (guard_condition) {
-      auto rcl_guard_condition = &guard_condition->get_rcl_guard_condition();
+    if (!guard_condition) {continue;}
 
-      rcl_ret_t ret = rcl_wait_set_add_guard_condition(
-        wait_set,
-        rcl_guard_condition, NULL);
+    rcl_guard_condition_t * cond = &guard_condition->get_rcl_guard_condition();
 
-      if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(
-          ret, "failed to add guard condition to wait set");
-      }
+    rcl_ret_t ret = rcl_wait_set_add_guard_condition(
+      wait_set, cond, NULL);
+
+    if (RCL_RET_OK != ret) {
+      rclcpp::exceptions::throw_from_rcl_error(
+        ret, "failed to add guard condition to wait set");
     }
   }
 }

rclcpp/src/rclcpp/executors/multi_threaded_executor.cpp

@@ -99,6 +99,19 @@ MultiThreadedExecutor::run(size_t this_thread_number)
 
     execute_any_executable(any_exec);
 
+    if (any_exec.callback_group &&
+      any_exec.callback_group->type() == CallbackGroupType::MutuallyExclusive &&
+      any_exec.callback_group->size() > 1)
+    {
+      try {
+        interrupt_guard_condition_->trigger();
+      } catch (const rclcpp::exceptions::RCLError & ex) {
+        throw std::runtime_error(
+                std::string(
+                  "Failed to trigger guard condition on callback group change: ") + ex.what());
+      }
+    }
+
     // Clear the callback_group to prevent the AnyExecutable destructor from
     // resetting the callback group `can_be_taken_from`
     any_exec.callback_group.reset();

rclcpp/src/rclcpp/executors/static_executor_entities_collector.cpp

@@ -1,524 +0,0 @@
-// Copyright 2020 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 "rclcpp/executors/static_executor_entities_collector.hpp"
-
-#include <algorithm>
-#include <memory>
-#include <stdexcept>
-#include <string>
-#include <utility>
-#include <vector>
-
-#include "rclcpp/memory_strategy.hpp"
-#include "rclcpp/executors/static_single_threaded_executor.hpp"
-#include "rclcpp/detail/add_guard_condition_to_rcl_wait_set.hpp"
-
-using rclcpp::executors::StaticExecutorEntitiesCollector;
-
-StaticExecutorEntitiesCollector::~StaticExecutorEntitiesCollector()
-{
-  // Disassociate all callback groups and thus nodes.
-  for (const auto & pair : weak_groups_associated_with_executor_to_nodes_) {
-    auto group = pair.first.lock();
-    if (group) {
-      std::atomic_bool & has_executor = group->get_associated_with_executor_atomic();
-      has_executor.store(false);
-    }
-  }
-  for (const auto & pair : weak_groups_to_nodes_associated_with_executor_) {
-    auto group = pair.first.lock();
-    if (group) {
-      std::atomic_bool & has_executor = group->get_associated_with_executor_atomic();
-      has_executor.store(false);
-    }
-  }
-  // Disassociate all nodes
-  for (const auto & weak_node : weak_nodes_) {
-    auto node = weak_node.lock();
-    if (node) {
-      std::atomic_bool & has_executor = node->get_associated_with_executor_atomic();
-      has_executor.store(false);
-    }
-  }
-  weak_groups_associated_with_executor_to_nodes_.clear();
-  weak_groups_to_nodes_associated_with_executor_.clear();
-  exec_list_.clear();
-  weak_nodes_.clear();
-  weak_nodes_to_guard_conditions_.clear();
-}
-
-void
-StaticExecutorEntitiesCollector::init(
-  rcl_wait_set_t * p_wait_set,
-  rclcpp::memory_strategy::MemoryStrategy::SharedPtr memory_strategy)
-{
-  // Empty initialize executable list
-  exec_list_ = rclcpp::experimental::ExecutableList();
-  // Get executor's wait_set_ pointer
-  p_wait_set_ = p_wait_set;
-  // Get executor's memory strategy ptr
-  if (memory_strategy == nullptr) {
-    throw std::runtime_error("Received NULL memory strategy in executor waitable.");
-  }
-  memory_strategy_ = memory_strategy;
-
-  // Get memory strategy and executable list. Prepare wait_set_
-  std::shared_ptr<void> shared_ptr;
-  execute(shared_ptr);
-
-  // The entities collector is now initialized
-  initialized_ = true;
-}
-
-void
-StaticExecutorEntitiesCollector::fini()
-{
-  memory_strategy_->clear_handles();
-  exec_list_.clear();
-}
-
-std::shared_ptr<void>
-StaticExecutorEntitiesCollector::take_data()
-{
-  return nullptr;
-}
-
-void
-StaticExecutorEntitiesCollector::execute(std::shared_ptr<void> & data)
-{
-  (void) data;
-  // Fill memory strategy with entities coming from weak_nodes_
-  fill_memory_strategy();
-  // Fill exec_list_ with entities coming from weak_nodes_ (same as memory strategy)
-  fill_executable_list();
-  // Resize the wait_set_ based on memory_strategy handles (rcl_wait_set_resize)
-  prepare_wait_set();
-  // Add new nodes guard conditions to map
-  std::lock_guard<std::mutex> guard{new_nodes_mutex_};
-  for (const auto & weak_node : new_nodes_) {
-    if (auto node_ptr = weak_node.lock()) {
-      weak_nodes_to_guard_conditions_[node_ptr] =
-        node_ptr->get_shared_notify_guard_condition().get();
-    }
-  }
-  new_nodes_.clear();
-}
-
-void
-StaticExecutorEntitiesCollector::fill_memory_strategy()
-{
-  memory_strategy_->clear_handles();
-  bool has_invalid_weak_groups_or_nodes =
-    memory_strategy_->collect_entities(weak_groups_to_nodes_associated_with_executor_);
-  // Clean up any invalid nodes, if they were detected
-  if (has_invalid_weak_groups_or_nodes) {
-    std::vector<rclcpp::CallbackGroup::WeakPtr> invalid_group_ptrs;
-    for (const auto & pair : weak_groups_to_nodes_associated_with_executor_) {
-      auto & weak_group_ptr = pair.first;
-      auto & weak_node_ptr = pair.second;
-      if (weak_group_ptr.expired() || weak_node_ptr.expired()) {
-        invalid_group_ptrs.push_back(weak_group_ptr);
-      }
-    }
-    std::for_each(
-      invalid_group_ptrs.begin(), invalid_group_ptrs.end(),
-      [this](rclcpp::CallbackGroup::WeakPtr group_ptr) {
-        weak_groups_to_nodes_associated_with_executor_.erase(group_ptr);
-      });
-  }
-  has_invalid_weak_groups_or_nodes =
-    memory_strategy_->collect_entities(weak_groups_associated_with_executor_to_nodes_);
-  // Clean up any invalid nodes, if they were detected
-  if (has_invalid_weak_groups_or_nodes) {
-    std::vector<rclcpp::CallbackGroup::WeakPtr> invalid_group_ptrs;
-    for (const auto & pair : weak_groups_associated_with_executor_to_nodes_) {
-      auto & weak_group_ptr = pair.first;
-      const auto & weak_node_ptr = pair.second;
-      if (weak_group_ptr.expired() || weak_node_ptr.expired()) {
-        invalid_group_ptrs.push_back(weak_group_ptr);
-      }
-    }
-    std::for_each(
-      invalid_group_ptrs.begin(), invalid_group_ptrs.end(),
-      [this](rclcpp::CallbackGroup::WeakPtr group_ptr) {
-        weak_groups_associated_with_executor_to_nodes_.erase(group_ptr);
-      });
-  }
-
-  // Add the static executor waitable to the memory strategy
-  memory_strategy_->add_waitable_handle(this->shared_from_this());
-}
-
-void
-StaticExecutorEntitiesCollector::fill_executable_list()
-{
-  exec_list_.clear();
-  add_callback_groups_from_nodes_associated_to_executor();
-  fill_executable_list_from_map(weak_groups_associated_with_executor_to_nodes_);
-  fill_executable_list_from_map(weak_groups_to_nodes_associated_with_executor_);
-  // Add the executor's waitable to the executable list
-  exec_list_.add_waitable(shared_from_this());
-}
-void
-StaticExecutorEntitiesCollector::fill_executable_list_from_map(
-  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
-  weak_groups_to_nodes)
-{
-  for (const auto & pair : weak_groups_to_nodes) {
-    auto group = pair.first.lock();
-    auto node = pair.second.lock();
-    if (!node || !group || !group->can_be_taken_from().load()) {
-      continue;
-    }
-    group->find_timer_ptrs_if(
-      [this](const rclcpp::TimerBase::SharedPtr & timer) {
-        if (timer) {
-          exec_list_.add_timer(timer);
-        }
-        return false;
-      });
-    group->find_subscription_ptrs_if(
-      [this](const rclcpp::SubscriptionBase::SharedPtr & subscription) {
-        if (subscription) {
-          exec_list_.add_subscription(subscription);
-        }
-        return false;
-      });
-    group->find_service_ptrs_if(
-      [this](const rclcpp::ServiceBase::SharedPtr & service) {
-        if (service) {
-          exec_list_.add_service(service);
-        }
-        return false;
-      });
-    group->find_client_ptrs_if(
-      [this](const rclcpp::ClientBase::SharedPtr & client) {
-        if (client) {
-          exec_list_.add_client(client);
-        }
-        return false;
-      });
-    group->find_waitable_ptrs_if(
-      [this](const rclcpp::Waitable::SharedPtr & waitable) {
-        if (waitable) {
-          exec_list_.add_waitable(waitable);
-        }
-        return false;
-      });
-  }
-}
-
-void
-StaticExecutorEntitiesCollector::prepare_wait_set()
-{
-  // clear wait set
-  if (rcl_wait_set_clear(p_wait_set_) != RCL_RET_OK) {
-    throw std::runtime_error("Couldn't clear wait set");
-  }
-
-  // The size of waitables are accounted for in size of the other entities
-  rcl_ret_t ret = rcl_wait_set_resize(
-    p_wait_set_, memory_strategy_->number_of_ready_subscriptions(),
-    memory_strategy_->number_of_guard_conditions(), memory_strategy_->number_of_ready_timers(),
-    memory_strategy_->number_of_ready_clients(), memory_strategy_->number_of_ready_services(),
-    memory_strategy_->number_of_ready_events());
-
-  if (RCL_RET_OK != ret) {
-    throw std::runtime_error(
-            std::string("Couldn't resize the wait set: ") + rcl_get_error_string().str);
-  }
-}
-
-void
-StaticExecutorEntitiesCollector::refresh_wait_set(std::chrono::nanoseconds timeout)
-{
-  // clear wait set (memset to '0' all wait_set_ entities
-  // but keeps the wait_set_ number of entities)
-  if (rcl_wait_set_clear(p_wait_set_) != RCL_RET_OK) {
-    throw std::runtime_error("Couldn't clear wait set");
-  }
-
-  if (!memory_strategy_->add_handles_to_wait_set(p_wait_set_)) {
-    throw std::runtime_error("Couldn't fill wait set");
-  }
-
-  rcl_ret_t status =
-    rcl_wait(p_wait_set_, std::chrono::duration_cast<std::chrono::nanoseconds>(timeout).count());
-
-  if (status == RCL_RET_WAIT_SET_EMPTY) {
-    RCUTILS_LOG_WARN_NAMED(
-      "rclcpp",
-      "empty wait set received in rcl_wait(). This should never happen.");
-  } else if (status != RCL_RET_OK && status != RCL_RET_TIMEOUT) {
-    using rclcpp::exceptions::throw_from_rcl_error;
-    throw_from_rcl_error(status, "rcl_wait() failed");
-  }
-}
-
-void
-StaticExecutorEntitiesCollector::add_to_wait_set(rcl_wait_set_t * wait_set)
-{
-  // Add waitable guard conditions (one for each registered node) into the wait set.
-  for (const auto & pair : weak_nodes_to_guard_conditions_) {
-    auto & gc = pair.second;
-    detail::add_guard_condition_to_rcl_wait_set(*wait_set, *gc);
-  }
-}
-
-size_t StaticExecutorEntitiesCollector::get_number_of_ready_guard_conditions()
-{
-  std::lock_guard<std::mutex> guard{new_nodes_mutex_};
-  return weak_nodes_to_guard_conditions_.size() + new_nodes_.size();
-}
-
-bool
-StaticExecutorEntitiesCollector::add_node(
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr)
-{
-  bool is_new_node = false;
-  // If the node already has an executor
-  std::atomic_bool & has_executor = node_ptr->get_associated_with_executor_atomic();
-  if (has_executor.exchange(true)) {
-    throw std::runtime_error("Node has already been added to an executor.");
-  }
-  node_ptr->for_each_callback_group(
-    [this, node_ptr, &is_new_node](rclcpp::CallbackGroup::SharedPtr group_ptr)
-    {
-      if (
-        !group_ptr->get_associated_with_executor_atomic().load() &&
-        group_ptr->automatically_add_to_executor_with_node())
-      {
-        is_new_node = (add_callback_group(
-          group_ptr,
-          node_ptr,
-          weak_groups_to_nodes_associated_with_executor_) ||
-        is_new_node);
-      }
-    });
-  weak_nodes_.push_back(node_ptr);
-  return is_new_node;
-}
-
-bool
-StaticExecutorEntitiesCollector::add_callback_group(
-  rclcpp::CallbackGroup::SharedPtr group_ptr,
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap & weak_groups_to_nodes)
-{
-  // If the callback_group already has an executor
-  std::atomic_bool & has_executor = group_ptr->get_associated_with_executor_atomic();
-  if (has_executor.exchange(true)) {
-    throw std::runtime_error("Callback group has already been added to an executor.");
-  }
-  bool is_new_node = !has_node(node_ptr, weak_groups_associated_with_executor_to_nodes_) &&
-    !has_node(node_ptr, weak_groups_to_nodes_associated_with_executor_);
-  rclcpp::CallbackGroup::WeakPtr weak_group_ptr = group_ptr;
-  auto insert_info = weak_groups_to_nodes.insert(
-    std::make_pair(weak_group_ptr, node_ptr));
-  bool was_inserted = insert_info.second;
-  if (!was_inserted) {
-    throw std::runtime_error("Callback group was already added to executor.");
-  }
-  if (is_new_node) {
-    std::lock_guard<std::mutex> guard{new_nodes_mutex_};
-    new_nodes_.push_back(node_ptr);
-    return true;
-  }
-  return false;
-}
-
-bool
-StaticExecutorEntitiesCollector::add_callback_group(
-  rclcpp::CallbackGroup::SharedPtr group_ptr,
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr)
-{
-  return add_callback_group(group_ptr, node_ptr, weak_groups_associated_with_executor_to_nodes_);
-}
-
-bool
-StaticExecutorEntitiesCollector::remove_callback_group(
-  rclcpp::CallbackGroup::SharedPtr group_ptr)
-{
-  return this->remove_callback_group_from_map(
-    group_ptr,
-    weak_groups_associated_with_executor_to_nodes_);
-}
-
-bool
-StaticExecutorEntitiesCollector::remove_callback_group_from_map(
-  rclcpp::CallbackGroup::SharedPtr group_ptr,
-  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap & weak_groups_to_nodes)
-{
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr;
-  rclcpp::CallbackGroup::WeakPtr weak_group_ptr = group_ptr;
-  auto iter = weak_groups_to_nodes.find(weak_group_ptr);
-  if (iter != weak_groups_to_nodes.end()) {
-    node_ptr = iter->second.lock();
-    if (node_ptr == nullptr) {
-      throw std::runtime_error("Node must not be deleted before its callback group(s).");
-    }
-    weak_groups_to_nodes.erase(iter);
-  } else {
-    throw std::runtime_error("Callback group needs to be associated with executor.");
-  }
-  // If the node was matched and removed, interrupt waiting.
-  if (!has_node(node_ptr, weak_groups_associated_with_executor_to_nodes_) &&
-    !has_node(node_ptr, weak_groups_to_nodes_associated_with_executor_))
-  {
-    rclcpp::node_interfaces::NodeBaseInterface::WeakPtr node_weak_ptr(node_ptr);
-    weak_nodes_to_guard_conditions_.erase(node_weak_ptr);
-    return true;
-  }
-  return false;
-}
-
-bool
-StaticExecutorEntitiesCollector::remove_node(
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr)
-{
-  if (!node_ptr->get_associated_with_executor_atomic().load()) {
-    return false;
-  }
-  bool node_found = false;
-  auto node_it = weak_nodes_.begin();
-  while (node_it != weak_nodes_.end()) {
-    bool matched = (node_it->lock() == node_ptr);
-    if (matched) {
-      weak_nodes_.erase(node_it);
-      node_found = true;
-      break;
-    }
-    ++node_it;
-  }
-  if (!node_found) {
-    return false;
-  }
-  std::vector<rclcpp::CallbackGroup::SharedPtr> found_group_ptrs;
-  std::for_each(
-    weak_groups_to_nodes_associated_with_executor_.begin(),
-    weak_groups_to_nodes_associated_with_executor_.end(),
-    [&found_group_ptrs, node_ptr](std::pair<rclcpp::CallbackGroup::WeakPtr,
-    rclcpp::node_interfaces::NodeBaseInterface::WeakPtr> key_value_pair) {
-      auto & weak_node_ptr = key_value_pair.second;
-      auto shared_node_ptr = weak_node_ptr.lock();
-      auto group_ptr = key_value_pair.first.lock();
-      if (shared_node_ptr == node_ptr) {
-        found_group_ptrs.push_back(group_ptr);
-      }
-    });
-  std::for_each(
-    found_group_ptrs.begin(), found_group_ptrs.end(), [this]
-      (rclcpp::CallbackGroup::SharedPtr group_ptr) {
-      this->remove_callback_group_from_map(
-        group_ptr,
-        weak_groups_to_nodes_associated_with_executor_);
-    });
-  std::atomic_bool & has_executor = node_ptr->get_associated_with_executor_atomic();
-  has_executor.store(false);
-  return true;
-}
-
-bool
-StaticExecutorEntitiesCollector::is_ready(rcl_wait_set_t * p_wait_set)
-{
-  // Check wait_set guard_conditions for added/removed entities to/from a node
-  for (size_t i = 0; i < p_wait_set->size_of_guard_conditions; ++i) {
-    if (p_wait_set->guard_conditions[i] != NULL) {
-      auto found_guard_condition = std::find_if(
-        weak_nodes_to_guard_conditions_.begin(), weak_nodes_to_guard_conditions_.end(),
-        [&](std::pair<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr,
-        const GuardCondition *> pair) -> bool {
-          const rcl_guard_condition_t & rcl_gc = pair.second->get_rcl_guard_condition();
-          return &rcl_gc == p_wait_set->guard_conditions[i];
-        });
-      if (found_guard_condition != weak_nodes_to_guard_conditions_.end()) {
-        return true;
-      }
-    }
-  }
-  // None of the guard conditions triggered belong to a registered node
-  return false;
-}
-
-// Returns true iff the weak_groups_to_nodes map has node_ptr as the value in any of its entry.
-bool
-StaticExecutorEntitiesCollector::has_node(
-  const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
-  weak_groups_to_nodes) const
-{
-  return std::find_if(
-    weak_groups_to_nodes.begin(),
-    weak_groups_to_nodes.end(),
-    [&](const WeakCallbackGroupsToNodesMap::value_type & other) -> bool {
-      auto other_ptr = other.second.lock();
-      return other_ptr == node_ptr;
-    }) != weak_groups_to_nodes.end();
-}
-
-void
-StaticExecutorEntitiesCollector::add_callback_groups_from_nodes_associated_to_executor()
-{
-  for (const auto & weak_node : weak_nodes_) {
-    auto node = weak_node.lock();
-    if (node) {
-      node->for_each_callback_group(
-        [this, node](rclcpp::CallbackGroup::SharedPtr shared_group_ptr)
-        {
-          if (shared_group_ptr->automatically_add_to_executor_with_node() &&
-          !shared_group_ptr->get_associated_with_executor_atomic().load())
-          {
-            add_callback_group(
-              shared_group_ptr,
-              node,
-              weak_groups_to_nodes_associated_with_executor_);
-          }
-        });
-    }
-  }
-}
-
-std::vector<rclcpp::CallbackGroup::WeakPtr>
-StaticExecutorEntitiesCollector::get_all_callback_groups()
-{
-  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
-  for (const auto & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
-    groups.push_back(group_node_ptr.first);
-  }
-  for (const auto & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
-    groups.push_back(group_node_ptr.first);
-  }
-  return groups;
-}
-
-std::vector<rclcpp::CallbackGroup::WeakPtr>
-StaticExecutorEntitiesCollector::get_manually_added_callback_groups()
-{
-  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
-  for (const auto & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
-    groups.push_back(group_node_ptr.first);
-  }
-  return groups;
-}
-
-std::vector<rclcpp::CallbackGroup::WeakPtr>
-StaticExecutorEntitiesCollector::get_automatically_added_callback_groups_from_nodes()
-{
-  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
-  for (const auto & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
-    groups.push_back(group_node_ptr.first);
-  }
-  return groups;
-}

rclcpp/src/rclcpp/executors/static_single_threaded_executor.cpp

@@ -12,31 +12,21 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "rclcpp/executors/static_single_threaded_executor.hpp"
-
-#include <chrono>
-#include <memory>
-#include <utility>
-#include <vector>
-
+#include "rclcpp/executors/executor_entities_collection.hpp"
+#include "rclcpp/executors/executor_notify_waitable.hpp"
 #include "rcpputils/scope_exit.hpp"
 
-using rclcpp::executors::StaticSingleThreadedExecutor;
-using rclcpp::experimental::ExecutableList;
+#include "rclcpp/executors/static_single_threaded_executor.hpp"
+#include "rclcpp/any_executable.hpp"
 
-StaticSingleThreadedExecutor::StaticSingleThreadedExecutor(
-  const rclcpp::ExecutorOptions & options)
+using rclcpp::executors::StaticSingleThreadedExecutor;
+
+StaticSingleThreadedExecutor::StaticSingleThreadedExecutor(const rclcpp::ExecutorOptions & options)
 : rclcpp::Executor(options)
 {
-  entities_collector_ = std::make_shared<StaticExecutorEntitiesCollector>();
 }
 
-StaticSingleThreadedExecutor::~StaticSingleThreadedExecutor()
-{
-  if (entities_collector_->is_init()) {
-    entities_collector_->fini();
-  }
-}
+StaticSingleThreadedExecutor::~StaticSingleThreadedExecutor() {}
 
 void
 StaticSingleThreadedExecutor::spin()
@@ -46,14 +36,25 @@ StaticSingleThreadedExecutor::spin()
   }
   RCPPUTILS_SCOPE_EXIT(this->spinning.store(false); );
 
-  // Set memory_strategy_ and exec_list_ based on weak_nodes_
-  // Prepare wait_set_ based on memory_strategy_
-  entities_collector_->init(&wait_set_, memory_strategy_);
-
+  // This is essentially the contents of the rclcpp::Executor::wait_for_work method,
+  // except we need to keep the wait result to reproduce the StaticSingleThreadedExecutor
+  // behavior.
   while (rclcpp::ok(this->context_) && spinning.load()) {
-    // Refresh wait set and wait for work
-    entities_collector_->refresh_wait_set();
-    execute_ready_executables();
+    std::deque<rclcpp::AnyExecutable> to_exec;
+
+    std::lock_guard<std::mutex> guard(mutex_);
+    if (current_collection_.empty() || this->entities_need_rebuild_.load()) {
+      this->collect_entities();
+    }
+
+    auto wait_result = wait_set_.wait(std::chrono::nanoseconds(-1));
+    if (wait_result.kind() == WaitResultKind::Empty) {
+      RCUTILS_LOG_WARN_NAMED(
+        "rclcpp",
+        "empty wait set received in wait(). This should never happen.");
+      continue;
+    }
+    execute_ready_executables(current_collection_, wait_result, false);
   }
 }
 
@@ -80,11 +81,6 @@ StaticSingleThreadedExecutor::spin_all(std::chrono::nanoseconds max_duration)
 void
 StaticSingleThreadedExecutor::spin_some_impl(std::chrono::nanoseconds max_duration, bool exhaustive)
 {
-  // Make sure the entities collector has been initialized
-  if (!entities_collector_->is_init()) {
-    entities_collector_->init(&wait_set_, memory_strategy_);
-  }
-
   auto start = std::chrono::steady_clock::now();
   auto max_duration_not_elapsed = [max_duration, start]() {
       if (std::chrono::nanoseconds(0) == max_duration) {
@@ -105,9 +101,21 @@ StaticSingleThreadedExecutor::spin_some_impl(std::chrono::nanoseconds max_durati
 
   while (rclcpp::ok(context_) && spinning.load() && max_duration_not_elapsed()) {
     // Get executables that are ready now
-    entities_collector_->refresh_wait_set(std::chrono::milliseconds::zero());
+    std::lock_guard<std::mutex> guard(mutex_);
+    if (current_collection_.empty() || this->entities_need_rebuild_.load()) {
+      this->collect_entities();
+    }
+
+    auto wait_result = wait_set_.wait(std::chrono::nanoseconds(0));
+    if (wait_result.kind() == WaitResultKind::Empty) {
+      RCUTILS_LOG_WARN_NAMED(
+        "rclcpp",
+        "empty wait set received in wait(). This should never happen.");
+      continue;
+    }
+
     // Execute ready executables
-    bool work_available = execute_ready_executables();
+    bool work_available = execute_ready_executables(current_collection_, wait_result, false);
     if (!work_available || !exhaustive) {
       break;
     }
@@ -117,164 +125,122 @@ StaticSingleThreadedExecutor::spin_some_impl(std::chrono::nanoseconds max_durati
 void
 StaticSingleThreadedExecutor::spin_once_impl(std::chrono::nanoseconds timeout)
 {
-  // Make sure the entities collector has been initialized
-  if (!entities_collector_->is_init()) {
-    entities_collector_->init(&wait_set_, memory_strategy_);
-  }
-
   if (rclcpp::ok(context_) && spinning.load()) {
-    // Wait until we have a ready entity or timeout expired
-    entities_collector_->refresh_wait_set(timeout);
+    std::lock_guard<std::mutex> guard(mutex_);
+    if (current_collection_.empty() || this->entities_need_rebuild_.load()) {
+      this->collect_entities();
+    }
+
+    auto wait_result = wait_set_.wait(std::chrono::nanoseconds(timeout));
+    if (wait_result.kind() == WaitResultKind::Empty) {
+      RCUTILS_LOG_WARN_NAMED(
+        "rclcpp",
+        "empty wait set received in wait(). This should never happen.");
+      return;
+    }
+
     // Execute ready executables
-    execute_ready_executables(true);
+    execute_ready_executables(current_collection_, wait_result, true);
   }
 }
 
-void
-StaticSingleThreadedExecutor::add_callback_group(
-  rclcpp::CallbackGroup::SharedPtr group_ptr,
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-  bool notify)
-{
-  bool is_new_node = entities_collector_->add_callback_group(group_ptr, node_ptr);
-  if (is_new_node && notify) {
-    // Interrupt waiting to handle new node
-    interrupt_guard_condition_->trigger();
-  }
-}
-
-void
-StaticSingleThreadedExecutor::add_node(
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify)
-{
-  bool is_new_node = entities_collector_->add_node(node_ptr);
-  if (is_new_node && notify) {
-    // Interrupt waiting to handle new node
-    interrupt_guard_condition_->trigger();
-  }
-}
-
-void
-StaticSingleThreadedExecutor::add_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify)
-{
-  this->add_node(node_ptr->get_node_base_interface(), notify);
-}
-
-void
-StaticSingleThreadedExecutor::remove_callback_group(
-  rclcpp::CallbackGroup::SharedPtr group_ptr, bool notify)
-{
-  bool node_removed = entities_collector_->remove_callback_group(group_ptr);
-  // If the node was matched and removed, interrupt waiting
-  if (node_removed && notify) {
-    interrupt_guard_condition_->trigger();
-  }
-}
-
-void
-StaticSingleThreadedExecutor::remove_node(
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify)
-{
-  bool node_removed = entities_collector_->remove_node(node_ptr);
-  if (!node_removed) {
-    throw std::runtime_error("Node needs to be associated with this executor.");
-  }
-  // If the node was matched and removed, interrupt waiting
-  if (notify) {
-    interrupt_guard_condition_->trigger();
-  }
-}
-
-std::vector<rclcpp::CallbackGroup::WeakPtr>
-StaticSingleThreadedExecutor::get_all_callback_groups()
-{
-  return entities_collector_->get_all_callback_groups();
-}
-
-std::vector<rclcpp::CallbackGroup::WeakPtr>
-StaticSingleThreadedExecutor::get_manually_added_callback_groups()
-{
-  return entities_collector_->get_manually_added_callback_groups();
-}
-
-std::vector<rclcpp::CallbackGroup::WeakPtr>
-StaticSingleThreadedExecutor::get_automatically_added_callback_groups_from_nodes()
-{
-  return entities_collector_->get_automatically_added_callback_groups_from_nodes();
-}
-
-void
-StaticSingleThreadedExecutor::remove_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify)
-{
-  this->remove_node(node_ptr->get_node_base_interface(), notify);
-}
-
-bool
-StaticSingleThreadedExecutor::execute_ready_executables(bool spin_once)
+// This preserves the "scheduling semantics" of the StaticSingleThreadedExecutor
+// from the original implementation.
+bool StaticSingleThreadedExecutor::execute_ready_executables(
+  const rclcpp::executors::ExecutorEntitiesCollection & collection,
+  rclcpp::WaitResult<rclcpp::WaitSet> & wait_result,
+  bool spin_once)
 {
   bool any_ready_executable = false;
 
-  // Execute all the ready subscriptions
-  for (size_t i = 0; i < wait_set_.size_of_subscriptions; ++i) {
-    if (i < entities_collector_->get_number_of_subscriptions()) {
-      if (wait_set_.subscriptions[i]) {
-        execute_subscription(entities_collector_->get_subscription(i));
-        if (spin_once) {
-          return true;
-        }
-        any_ready_executable = true;
-      }
-    }
+  if (wait_result.kind() != rclcpp::WaitResultKind::Ready) {
+    return any_ready_executable;
   }
-  // Execute all the ready timers
-  for (size_t i = 0; i < wait_set_.size_of_timers; ++i) {
-    if (i < entities_collector_->get_number_of_timers()) {
-      if (wait_set_.timers[i] && entities_collector_->get_timer(i)->is_ready()) {
-        auto timer = entities_collector_->get_timer(i);
-        timer->call();
-        execute_timer(std::move(timer));
-        if (spin_once) {
-          return true;
-        }
-        any_ready_executable = true;
+
+  auto rcl_wait_set = wait_result.get_wait_set().get_rcl_wait_set();
+
+  for (size_t ii = 0; ii < rcl_wait_set.size_of_timers; ++ii) {
+    if (nullptr == rcl_wait_set.timers[ii]) {continue;}
+    auto entity_iter = collection.timers.find(rcl_wait_set.timers[ii]);
+    if (entity_iter != collection.timers.end()) {
+      auto entity = entity_iter->second.entity.lock();
+      if (!entity) {
+        continue;
       }
-    }
-  }
-  // Execute all the ready services
-  for (size_t i = 0; i < wait_set_.size_of_services; ++i) {
-    if (i < entities_collector_->get_number_of_services()) {
-      if (wait_set_.services[i]) {
-        execute_service(entities_collector_->get_service(i));
-        if (spin_once) {
-          return true;
-        }
-        any_ready_executable = true;
+      if (!entity->call()) {
+        continue;
       }
-    }
-  }
-  // Execute all the ready clients
-  for (size_t i = 0; i < wait_set_.size_of_clients; ++i) {
-    if (i < entities_collector_->get_number_of_clients()) {
-      if (wait_set_.clients[i]) {
-        execute_client(entities_collector_->get_client(i));
-        if (spin_once) {
-          return true;
-        }
-        any_ready_executable = true;
-      }
-    }
-  }
-  // Execute all the ready waitables
-  for (size_t i = 0; i < entities_collector_->get_number_of_waitables(); ++i) {
-    auto waitable = entities_collector_->get_waitable(i);
-    if (waitable->is_ready(&wait_set_)) {
-      auto data = waitable->take_data();
-      waitable->execute(data);
+      execute_timer(entity);
       if (spin_once) {
         return true;
       }
       any_ready_executable = true;
     }
   }
+
+  for (size_t ii = 0; ii < rcl_wait_set.size_of_subscriptions; ++ii) {
+    if (nullptr == rcl_wait_set.subscriptions[ii]) {continue;}
+    auto entity_iter = collection.subscriptions.find(rcl_wait_set.subscriptions[ii]);
+    if (entity_iter != collection.subscriptions.end()) {
+      auto entity = entity_iter->second.entity.lock();
+      if (!entity) {
+        continue;
+      }
+      execute_subscription(entity);
+      if (spin_once) {
+        return true;
+      }
+      any_ready_executable = true;
+    }
+  }
+
+  for (size_t ii = 0; ii < rcl_wait_set.size_of_services; ++ii) {
+    if (nullptr == rcl_wait_set.services[ii]) {continue;}
+    auto entity_iter = collection.services.find(rcl_wait_set.services[ii]);
+    if (entity_iter != collection.services.end()) {
+      auto entity = entity_iter->second.entity.lock();
+      if (!entity) {
+        continue;
+      }
+      execute_service(entity);
+      if (spin_once) {
+        return true;
+      }
+      any_ready_executable = true;
+    }
+  }
+
+  for (size_t ii = 0; ii < rcl_wait_set.size_of_clients; ++ii) {
+    if (nullptr == rcl_wait_set.clients[ii]) {continue;}
+    auto entity_iter = collection.clients.find(rcl_wait_set.clients[ii]);
+    if (entity_iter != collection.clients.end()) {
+      auto entity = entity_iter->second.entity.lock();
+      if (!entity) {
+        continue;
+      }
+      execute_client(entity);
+      if (spin_once) {
+        return true;
+      }
+      any_ready_executable = true;
+    }
+  }
+
+  for (auto & [handle, entry] : collection.waitables) {
+    auto waitable = entry.entity.lock();
+    if (!waitable) {
+      continue;
+    }
+    if (!waitable->is_ready(&rcl_wait_set)) {
+      continue;
+    }
+
+    auto data = waitable->take_data();
+    waitable->execute(data);
+    if (spin_once) {
+      return true;
+    }
+    any_ready_executable = true;
+  }
   return any_ready_executable;
 }

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

@@ -50,9 +50,6 @@ 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>();
-
   notify_waitable_ = std::make_shared<rclcpp::executors::ExecutorNotifyWaitable>(
     [this]() {
       // This callback is invoked when:
@@ -64,10 +61,6 @@ EventsExecutor::EventsExecutor(
       this->refresh_current_collection_from_callback_groups();
     });
 
-  // 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_);
 
@@ -94,6 +87,9 @@ EventsExecutor::EventsExecutor(
 
   this->entities_collector_ =
     std::make_shared<rclcpp::executors::ExecutorEntitiesCollector>(notify_waitable_);
+
+  this->current_entities_collection_ =
+    std::make_shared<rclcpp::executors::ExecutorEntitiesCollection>();
 }
 
 EventsExecutor::~EventsExecutor()
@@ -399,8 +395,18 @@ EventsExecutor::refresh_current_collection_from_callback_groups()
   // 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.
-  this->add_notify_waitable_to_collection(new_collection.waitables);
-  this->add_notify_waitable_to_collection(current_entities_collection_->waitables);
+  rclcpp::CallbackGroup::WeakPtr weak_group_ptr;
+  new_collection.waitables.insert(
+  {
+    this->notify_waitable_.get(),
+    {this->notify_waitable_, weak_group_ptr}
+  });
+
+  this->current_entities_collection_->waitables.insert(
+  {
+    this->notify_waitable_.get(),
+    {this->notify_waitable_, weak_group_ptr}
+  });
 
   this->refresh_current_collection(new_collection);
 }
@@ -480,16 +486,3 @@ EventsExecutor::create_waitable_callback(const rclcpp::Waitable * entity_key)
     };
   return callback;
 }
-
-void
-EventsExecutor::add_notify_waitable_to_collection(
-  rclcpp::executors::ExecutorEntitiesCollection::WaitableCollection & collection)
-{
-  // The notify waitable is not associated to any group, so use an invalid one
-  rclcpp::CallbackGroup::WeakPtr weak_group_ptr;
-  collection.insert(
-  {
-    this->notify_waitable_.get(),
-    {this->notify_waitable_, weak_group_ptr}
-  });
-}

rclcpp/src/rclcpp/logging_mutex.cpp

@@ -14,7 +14,6 @@
 
 #include <memory>
 #include <mutex>
-#include <stdexcept>
 
 #include "rcutils/macros.h"
 

rclcpp/src/rclcpp/node.cpp

@@ -17,10 +17,7 @@
 #include <limits>
 #include <map>
 #include <memory>
-#include <mutex>
-#include <shared_mutex>
 #include <string>
-#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -39,7 +36,6 @@
 #include "rclcpp/node_interfaces/node_time_source.hpp"
 #include "rclcpp/node_interfaces/node_timers.hpp"
 #include "rclcpp/node_interfaces/node_topics.hpp"
-#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
 #include "rclcpp/node_interfaces/node_waitables.hpp"
 #include "rclcpp/qos_overriding_options.hpp"
 
@@ -113,72 +109,6 @@ create_effective_namespace(const std::string & node_namespace, const std::string
 
 }  // namespace
 
-/// \brief Associate new extra member variables with instances of Node without changing ABI.
-/**
- * It is used only for bugfixes or backported features that require new members.
- * Atomically constructs/destroys all extra members.
- * Node instance will register and remove itself, and use its methods to retrieve members.
- * Note for performance consideration that accessing these members uses a map lookup.
- */
-class Node::BackportMembers
-{
-public:
-  BackportMembers() = default;
-  ~BackportMembers() = default;
-
-  /// \brief Add all backported members for a new Node.
-  /**
-   * \param[in] key Raw pointer to the Node instance that will use new members.
-   */
-  void add(Node * key)
-  {
-    // Adding a new instance to the maps requires exclusive access
-    std::unique_lock lock(map_access_mutex_);
-    type_descriptions_map_.emplace(
-      key,
-      std::make_shared<rclcpp::node_interfaces::NodeTypeDescriptions>(
-        key->get_node_base_interface(),
-        key->get_node_logging_interface(),
-        key->get_node_parameters_interface(),
-        key->get_node_services_interface()));
-  }
-
-  /// \brief Remove the members for an instance of Node
-  /**
-   * \param[in] key Raw pointer to the Node
-   */
-  void remove(const Node * key)
-  {
-    // Removing an instance from the maps requires exclusive access
-    std::unique_lock lock(map_access_mutex_);
-    type_descriptions_map_.erase(key);
-  }
-
-  /// \brief Retrieve the NodeTypeDescriptionsInterface for a Node.
-  /**
-   * \param[in] key Raw pointer to an instance of Node.
-   * \return A shared ptr to this Node's NodeTypeDescriptionsInterface instance.
-   */
-  rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
-  get_node_type_descriptions_interface(const Node * key) const
-  {
-    // Multiple threads can retrieve from the maps at the same time
-    std::shared_lock lock(map_access_mutex_);
-    return type_descriptions_map_.at(key);
-  }
-
-private:
-  /// \brief Map that stored TypeDescriptionsInterface members
-  std::unordered_map<
-    const Node *, rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
-  > type_descriptions_map_;
-
-  /// \brief Controls access to all private maps
-  mutable std::shared_mutex map_access_mutex_;
-};
-// Definition of static member declaration
-Node::BackportMembers Node::backport_members_;
-
 Node::Node(
   const std::string & node_name,
   const NodeOptions & options)
@@ -237,7 +167,7 @@ Node::Node(
       options.use_intra_process_comms(),
       options.enable_topic_statistics())),
   node_graph_(new rclcpp::node_interfaces::NodeGraph(node_base_.get())),
-  node_logging_(new rclcpp::node_interfaces::NodeLogging(node_base_)),
+  node_logging_(new rclcpp::node_interfaces::NodeLogging(node_base_.get())),
   node_timers_(new rclcpp::node_interfaces::NodeTimers(node_base_.get())),
   node_topics_(new rclcpp::node_interfaces::NodeTopics(node_base_.get(), node_timers_.get())),
   node_services_(new rclcpp::node_interfaces::NodeServices(node_base_.get())),
@@ -281,8 +211,6 @@ Node::Node(
   sub_namespace_(""),
   effective_namespace_(create_effective_namespace(this->get_namespace(), sub_namespace_))
 {
-  backport_members_.add(this);
-
   // we have got what we wanted directly from the overrides,
   // but declare the parameters anyway so they are visible.
   rclcpp::detail::declare_qos_parameters(
@@ -297,10 +225,6 @@ Node::Node(
     node_topics_->resolve_topic_name("/parameter_events"),
     options.parameter_event_qos(),
     rclcpp::detail::PublisherQosParametersTraits{});
-
-  if (options.enable_logger_service()) {
-    node_logging_->create_logger_services(node_services_);
-  }
 }
 
 Node::Node(
@@ -344,7 +268,6 @@ Node::Node(
 Node::~Node()
 {
   // release sub-interfaces in an order that allows them to consult with node_base during tear-down
-  backport_members_.remove(this);
   node_waitables_.reset();
   node_time_source_.reset();
   node_parameters_.reset();
@@ -664,12 +587,6 @@ Node::get_node_topics_interface()
   return node_topics_;
 }
 
-rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
-Node::get_node_type_descriptions_interface()
-{
-  return backport_members_.get_node_type_descriptions_interface(this);
-}
-
 rclcpp::node_interfaces::NodeServicesInterface::SharedPtr
 Node::get_node_services_interface()
 {

rclcpp/src/rclcpp/node_interfaces/node_logging.cpp

@@ -12,13 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "rclcpp/node_impl.hpp"
 #include "rclcpp/node_interfaces/node_logging.hpp"
-#include "rclcpp/node_interfaces/node_services_interface.hpp"
 
 using rclcpp::node_interfaces::NodeLogging;
 
-NodeLogging::NodeLogging(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base)
+NodeLogging::NodeLogging(rclcpp::node_interfaces::NodeBaseInterface * node_base)
 : node_base_(node_base)
 {
   logger_ = rclcpp::get_logger(NodeLogging::get_logger_name());
@@ -39,55 +37,3 @@ NodeLogging::get_logger_name() const
 {
   return rcl_node_get_logger_name(node_base_->get_rcl_node_handle());
 }
-
-void NodeLogging::create_logger_services(
-  node_interfaces::NodeServicesInterface::SharedPtr node_services)
-{
-  rclcpp::ServicesQoS qos_profile;
-  const std::string node_name = node_base_->get_name();
-  auto callback_group = node_base_->get_default_callback_group();
-
-  get_loggers_service_ = rclcpp::create_service<rcl_interfaces::srv::GetLoggerLevels>(
-    node_base_, node_services,
-    node_name + "/get_logger_levels",
-    [](
-      const std::shared_ptr<rmw_request_id_t>,
-      const std::shared_ptr<rcl_interfaces::srv::GetLoggerLevels::Request> request,
-      std::shared_ptr<rcl_interfaces::srv::GetLoggerLevels::Response> response)
-    {
-      for (auto & name : request->names) {
-        rcl_interfaces::msg::LoggerLevel logger_level;
-        logger_level.name = name;
-        auto ret = rcutils_logging_get_logger_level(name.c_str());
-        if (ret < 0) {
-          logger_level.level = 0;
-        } else {
-          logger_level.level = static_cast<uint8_t>(ret);
-        }
-        response->levels.push_back(std::move(logger_level));
-      }
-    },
-    qos_profile, callback_group);
-
-  set_loggers_service_ = rclcpp::create_service<rcl_interfaces::srv::SetLoggerLevels>(
-    node_base_, node_services,
-    node_name + "/set_logger_levels",
-    [](
-      const std::shared_ptr<rmw_request_id_t>,
-      const std::shared_ptr<rcl_interfaces::srv::SetLoggerLevels::Request> request,
-      std::shared_ptr<rcl_interfaces::srv::SetLoggerLevels::Response> response)
-    {
-      rcl_interfaces::msg::SetLoggerLevelsResult result;
-      for (auto & level : request->levels) {
-        auto ret = rcutils_logging_set_logger_level(level.name.c_str(), level.level);
-        if (ret != RCUTILS_RET_OK) {
-          result.successful = false;
-          result.reason = rcutils_get_error_string().str;
-        } else {
-          result.successful = true;
-        }
-        response->results.push_back(std::move(result));
-      }
-    },
-    qos_profile, callback_group);
-}

rclcpp/src/rclcpp/node_interfaces/node_type_descriptions.cpp

@@ -1,157 +0,0 @@
-// Copyright 2023 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 <memory>
-#include <string>
-#include <thread>
-
-#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
-#include "rclcpp/parameter_client.hpp"
-
-#include "type_description_interfaces/srv/get_type_description.h"
-
-namespace
-{
-// Helper wrapper for rclcpp::Service to access ::Request and ::Response types for allocation.
-struct GetTypeDescription__C
-{
-  using Request = type_description_interfaces__srv__GetTypeDescription_Request;
-  using Response = type_description_interfaces__srv__GetTypeDescription_Response;
-  using Event = type_description_interfaces__srv__GetTypeDescription_Event;
-};
-}  // namespace
-
-// Helper function for C typesupport.
-namespace rosidl_typesupport_cpp
-{
-template<>
-rosidl_service_type_support_t const *
-get_service_type_support_handle<GetTypeDescription__C>()
-{
-  return ROSIDL_GET_SRV_TYPE_SUPPORT(type_description_interfaces, srv, GetTypeDescription);
-}
-}  // namespace rosidl_typesupport_cpp
-
-namespace rclcpp
-{
-namespace node_interfaces
-{
-
-class NodeTypeDescriptions::NodeTypeDescriptionsImpl
-{
-public:
-  using ServiceT = GetTypeDescription__C;
-
-  rclcpp::Logger logger_;
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base_;
-  rclcpp::Service<ServiceT>::SharedPtr type_description_srv_;
-
-  NodeTypeDescriptionsImpl(
-    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base,
-    rclcpp::node_interfaces::NodeLoggingInterface::SharedPtr node_logging,
-    rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters,
-    rclcpp::node_interfaces::NodeServicesInterface::SharedPtr node_services)
-  : logger_(node_logging->get_logger()),
-    node_base_(node_base)
-  {
-    const std::string enable_param_name = "start_type_description_service";
-
-    bool enabled = false;
-    try {
-      auto enable_param = node_parameters->declare_parameter(
-        enable_param_name,
-        rclcpp::ParameterValue(true),
-        rcl_interfaces::msg::ParameterDescriptor()
-        .set__name(enable_param_name)
-        .set__type(rclcpp::PARAMETER_BOOL)
-        .set__description("Start the ~/get_type_description service for this node.")
-        .set__read_only(true));
-      enabled = enable_param.get<bool>();
-    } catch (const rclcpp::exceptions::InvalidParameterTypeException & exc) {
-      RCLCPP_ERROR(logger_, "%s", exc.what());
-      throw;
-    }
-
-    if (enabled) {
-      auto rcl_node = node_base->get_rcl_node_handle();
-      rcl_ret_t rcl_ret = rcl_node_type_description_service_init(rcl_node);
-      if (rcl_ret != RCL_RET_OK) {
-        RCLCPP_ERROR(
-          logger_, "Failed to initialize ~/get_type_description_service: %s",
-          rcl_get_error_string().str);
-        throw std::runtime_error(
-                "Failed to initialize ~/get_type_description service.");
-      }
-
-      rcl_service_t * rcl_srv = nullptr;
-      rcl_ret = rcl_node_get_type_description_service(rcl_node, &rcl_srv);
-      if (rcl_ret != RCL_RET_OK) {
-        throw std::runtime_error(
-                "Failed to get initialized ~/get_type_description service from rcl.");
-      }
-
-      rclcpp::AnyServiceCallback<ServiceT> cb;
-      cb.set(
-        [this](
-          std::shared_ptr<rmw_request_id_t> header,
-          std::shared_ptr<ServiceT::Request> request,
-          std::shared_ptr<ServiceT::Response> response
-        ) {
-          rcl_node_type_description_service_handle_request(
-            node_base_->get_rcl_node_handle(),
-            header.get(),
-            request.get(),
-            response.get());
-        });
-
-      type_description_srv_ = std::make_shared<Service<ServiceT>>(
-        node_base_->get_shared_rcl_node_handle(),
-        rcl_srv,
-        cb);
-      node_services->add_service(
-        std::dynamic_pointer_cast<ServiceBase>(type_description_srv_),
-        nullptr);
-    }
-  }
-
-  ~NodeTypeDescriptionsImpl()
-  {
-    if (
-      type_description_srv_ &&
-      RCL_RET_OK != rcl_node_type_description_service_fini(node_base_->get_rcl_node_handle()))
-    {
-      RCLCPP_ERROR(
-        logger_,
-        "Error in shutdown of get_type_description service: %s", rcl_get_error_string().str);
-    }
-  }
-};
-
-NodeTypeDescriptions::NodeTypeDescriptions(
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base,
-  rclcpp::node_interfaces::NodeLoggingInterface::SharedPtr node_logging,
-  rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters,
-  rclcpp::node_interfaces::NodeServicesInterface::SharedPtr node_services)
-: impl_(new NodeTypeDescriptionsImpl(
-      node_base,
-      node_logging,
-      node_parameters,
-      node_services))
-{}
-
-NodeTypeDescriptions::~NodeTypeDescriptions()
-{}
-
-}  // namespace node_interfaces
-}  // namespace rclcpp

rclcpp/src/rclcpp/node_options.cpp

@@ -248,19 +248,6 @@ NodeOptions::start_parameter_services(bool start_parameter_services)
   return *this;
 }
 
-bool
-NodeOptions::enable_logger_service() const
-{
-  return this->enable_logger_service_;
-}
-
-NodeOptions &
-NodeOptions::enable_logger_service(bool enable_logger_service)
-{
-  this->enable_logger_service_ = enable_logger_service;
-  return *this;
-}
-
 bool
 NodeOptions::start_parameter_event_publisher() const
 {

rclcpp/src/rclcpp/signal_handler.cpp

@@ -113,7 +113,7 @@ SignalHandler::get_logger()
 SignalHandler &
 SignalHandler::get_global_signal_handler()
 {
-  static SignalHandler & signal_handler = *new SignalHandler();
+  static SignalHandler signal_handler;
   return signal_handler;
 }
 

rclcpp/src/rclcpp/subscription_base.cpp

@@ -52,7 +52,7 @@ SubscriptionBase::SubscriptionBase(
   intra_process_subscription_id_(0),
   event_callbacks_(event_callbacks),
   type_support_(type_support_handle),
-  delivered_message_kind_(delivered_message_kind)
+  delivered_message_type_(delivered_message_kind)
 {
   auto custom_deletor = [node_handle = this->node_handle_](rcl_subscription_t * rcl_subs)
     {
@@ -261,13 +261,13 @@ SubscriptionBase::get_message_type_support_handle() const
 bool
 SubscriptionBase::is_serialized() const
 {
-  return delivered_message_kind_ == rclcpp::DeliveredMessageKind::SERIALIZED_MESSAGE;
+  return delivered_message_type_ == rclcpp::DeliveredMessageKind::SERIALIZED_MESSAGE;
 }
 
 rclcpp::DeliveredMessageKind
-SubscriptionBase::get_delivered_message_kind() const
+SubscriptionBase::get_subscription_type() const
 {
-  return delivered_message_kind_;
+  return delivered_message_type_;
 }
 
 size_t
@@ -298,20 +298,7 @@ SubscriptionBase::setup_intra_process(
 bool
 SubscriptionBase::can_loan_messages() const
 {
-  bool retval = rcl_subscription_can_loan_messages(subscription_handle_.get());
-  if (retval) {
-    // TODO(clalancette): The loaned message interface is currently not safe to use with
-    // shared_ptr callbacks.  If a user takes a copy of the shared_ptr, it can get freed from
-    // underneath them via rcl_return_loaned_message_from_subscription().  The correct solution is
-    // to return the loaned message in a custom deleter, but that needs to be carefully handled
-    // with locking.  Warn the user about this until we fix it.
-    RCLCPP_WARN_ONCE(
-      this->node_logger_,
-      "Loaned messages are only safe with const ref subscription callbacks. "
-      "If you are using any other kind of subscriptions, "
-      "set the ROS_DISABLE_LOANED_MESSAGES environment variable to 1 (the default).");
-  }
-  return retval;
+  return rcl_subscription_can_loan_messages(subscription_handle_.get());
 }
 
 rclcpp::Waitable::SharedPtr

rclcpp/test/benchmark/benchmark_executor.cpp

@@ -362,42 +362,3 @@ BENCHMARK_F(PerformanceTestExecutorSimple, spin_until_future_complete)(benchmark
     }
   }
 }
-
-BENCHMARK_F(
-  PerformanceTestExecutorSimple,
-  static_executor_entities_collector_execute)(benchmark::State & st)
-{
-  rclcpp::executors::StaticExecutorEntitiesCollector::SharedPtr entities_collector_ =
-    std::make_shared<rclcpp::executors::StaticExecutorEntitiesCollector>();
-  entities_collector_->add_node(node->get_node_base_interface());
-
-  rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
-  rcl_allocator_t allocator = rcl_get_default_allocator();
-  auto shared_context = node->get_node_base_interface()->get_context();
-  rcl_context_t * context = shared_context->get_rcl_context().get();
-  rcl_ret_t ret = rcl_wait_set_init(&wait_set, 100, 100, 100, 100, 100, 100, context, allocator);
-  if (ret != RCL_RET_OK) {
-    st.SkipWithError(rcutils_get_error_string().str);
-  }
-  RCPPUTILS_SCOPE_EXIT(
-  {
-    rcl_ret_t ret = rcl_wait_set_fini(&wait_set);
-    if (ret != RCL_RET_OK) {
-      st.SkipWithError(rcutils_get_error_string().str);
-    }
-  });
-
-  auto memory_strategy = rclcpp::memory_strategies::create_default_strategy();
-  rclcpp::GuardCondition guard_condition(shared_context);
-
-  entities_collector_->init(&wait_set, memory_strategy);
-  RCPPUTILS_SCOPE_EXIT(entities_collector_->fini());
-
-  reset_heap_counters();
-
-  for (auto _ : st) {
-    (void)_;
-    std::shared_ptr<void> data = entities_collector_->take_data();
-    entities_collector_->execute(data);
-  }
-}

rclcpp/test/rclcpp/CMakeLists.txt

@@ -262,11 +262,6 @@ if(TARGET test_node_interfaces__node_topics)
     "test_msgs")
   target_link_libraries(test_node_interfaces__node_topics ${PROJECT_NAME} mimick)
 endif()
-ament_add_gtest(test_node_interfaces__node_type_descriptions
-  node_interfaces/test_node_type_descriptions.cpp)
-if(TARGET test_node_interfaces__node_type_descriptions)
-  target_link_libraries(test_node_interfaces__node_type_descriptions ${PROJECT_NAME} mimick)
-endif()
 ament_add_gtest(test_node_interfaces__node_waitables
   node_interfaces/test_node_waitables.cpp)
 if(TARGET test_node_interfaces__node_waitables)
@@ -654,13 +649,6 @@ if(TARGET test_wait_for_message)
   target_link_libraries(test_wait_for_message ${PROJECT_NAME})
 endif()
 
-ament_add_gtest(test_logger_service test_logger_service.cpp)
-if(TARGET test_logger_service)
-  ament_target_dependencies(test_logger_service
-    "rcl_interfaces")
-  target_link_libraries(test_logger_service ${PROJECT_NAME})
-endif()
-
 ament_add_gtest(test_interface_traits test_interface_traits.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}")
 if(TARGET test_interface_traits)
@@ -683,14 +671,6 @@ if(TARGET test_executors)
   target_link_libraries(test_executors ${PROJECT_NAME})
 endif()
 
-ament_add_gtest(test_static_single_threaded_executor executors/test_static_single_threaded_executor.cpp
-  APPEND_LIBRARY_DIRS "${append_library_dirs}")
-if(TARGET test_static_single_threaded_executor)
-  ament_target_dependencies(test_static_single_threaded_executor
-    "test_msgs")
-  target_link_libraries(test_static_single_threaded_executor ${PROJECT_NAME} mimick)
-endif()
-
 ament_add_gtest(test_multi_threaded_executor executors/test_multi_threaded_executor.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}")
 if(TARGET test_multi_threaded_executor)
@@ -699,15 +679,6 @@ if(TARGET test_multi_threaded_executor)
   target_link_libraries(test_multi_threaded_executor ${PROJECT_NAME})
 endif()
 
-ament_add_gtest(test_static_executor_entities_collector executors/test_static_executor_entities_collector.cpp
-  APPEND_LIBRARY_DIRS "${append_library_dirs}" TIMEOUT 120)
-if(TARGET test_static_executor_entities_collector)
-  ament_target_dependencies(test_static_executor_entities_collector
-    "rcl"
-    "test_msgs")
-  target_link_libraries(test_static_executor_entities_collector ${PROJECT_NAME} mimick)
-endif()
-
 ament_add_gtest(test_entities_collector executors/test_entities_collector.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}" TIMEOUT 120)
 if(TARGET test_entities_collector)

rclcpp/test/rclcpp/executors/test_executors.cpp

@@ -20,14 +20,12 @@
 #include <gtest/gtest.h>
 
 #include <algorithm>
-#include <atomic>
 #include <chrono>
 #include <limits>
 #include <memory>
 #include <string>
 #include <thread>
 #include <utility>
-#include <vector>
 
 #include "rcl/error_handling.h"
 #include "rcl/time.h"
@@ -45,10 +43,18 @@ template<typename T>
 class TestExecutors : public ::testing::Test
 {
 public:
-  void SetUp()
+  static void SetUpTestCase()
   {
     rclcpp::init(0, nullptr);
+  }
 
+  static void TearDownTestCase()
+  {
+    rclcpp::shutdown();
+  }
+
+  void SetUp()
+  {
     const auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
     std::stringstream test_name;
     test_name << test_info->test_case_name() << "_" << test_info->name();
@@ -69,8 +75,6 @@ public:
     publisher.reset();
     subscription.reset();
     node.reset();
-
-    rclcpp::shutdown();
   }
 
   rclcpp::Node::SharedPtr node;
@@ -79,8 +83,6 @@ public:
   int callback_count;
 };
 
-// spin_all and spin_some are not implemented correctly in StaticSingleThreadedExecutor, see:
-// https://github.com/ros2/rclcpp/issues/1219 for tracking
 template<typename T>
 class TestExecutorsStable : public TestExecutors<T> {};
 
@@ -122,15 +124,6 @@ public:
 // is updated.
 TYPED_TEST_SUITE(TestExecutors, ExecutorTypes, ExecutorTypeNames);
 
-// StaticSingleThreadedExecutor is not included in these tests for now, due to:
-// https://github.com/ros2/rclcpp/issues/1219
-using StandardExecutors =
-  ::testing::Types<
-  rclcpp::executors::SingleThreadedExecutor,
-  rclcpp::executors::MultiThreadedExecutor,
-  rclcpp::experimental::executors::EventsExecutor>;
-TYPED_TEST_SUITE(TestExecutorsStable, StandardExecutors, ExecutorTypeNames);
-
 // Make sure that executors detach from nodes when destructing
 TYPED_TEST(TestExecutors, detachOnDestruction)
 {
@@ -154,10 +147,7 @@ TYPED_TEST(TestExecutors, detachOnDestruction)
 }
 
 // Make sure that the executor can automatically remove expired nodes correctly
-// Currently fails for StaticSingleThreadedExecutor so it is being skipped, see:
-// https://github.com/ros2/rclcpp/issues/1231
-TYPED_TEST(TestExecutorsStable, addTemporaryNode)
-{
+TYPED_TEST(TestExecutors, addTemporaryNode) {
   using ExecutorType = TypeParam;
   // rmw_connextdds doesn't support events-executor
   if (
@@ -267,14 +257,17 @@ TYPED_TEST(TestExecutors, spinWhileAlreadySpinning)
   ExecutorType executor;
   executor.add_node(this->node);
 
-  bool timer_completed = false;
-  auto timer = this->node->create_wall_timer(1ms, [&]() {timer_completed = true;});
+  std::atomic_bool timer_completed = false;
+  auto timer = this->node->create_wall_timer(
+    1ms, [&]() {
+      timer_completed.store(true);
+    });
 
   std::thread spinner([&]() {executor.spin();});
   // Sleep for a short time to verify executor.spin() is going, and didn't throw.
 
   auto start = std::chrono::steady_clock::now();
-  while (!timer_completed && (std::chrono::steady_clock::now() - start) < 10s) {
+  while (!timer_completed.load() && (std::chrono::steady_clock::now() - start) < 10s) {
     std::this_thread::sleep_for(1ms);
   }
 
@@ -725,77 +718,6 @@ TYPED_TEST(TestExecutors, testSpinUntilFutureCompleteInterrupted)
   spinner.join();
 }
 
-// This test verifies that the add_node operation is robust wrt race conditions.
-// It's mostly meant to prevent regressions in the events-executor, but the operation should be
-// thread-safe in all executor implementations.
-// The initial implementation of the events-executor contained a bug where the executor
-// would end up in an inconsistent state and stop processing interrupt/shutdown notifications.
-// Manually adding a node to the executor results in a) producing a notify waitable event
-// 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
-// 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
-// bug is present. However repeated runs will show its flakiness nature and indicate
-// an eventual regression.
-TYPED_TEST(TestExecutors, testRaceConditionAddNode)
-{
-  using ExecutorType = TypeParam;
-  // rmw_connextdds doesn't support events-executor
-  if (
-    std::is_same<ExecutorType, rclcpp::experimental::executors::EventsExecutor>() &&
-    std::string(rmw_get_implementation_identifier()).find("rmw_connextdds") == 0)
-  {
-    GTEST_SKIP();
-  }
-
-  // Spawn some threads to do some heavy work
-  std::atomic<bool> should_cancel = false;
-  std::vector<std::thread> stress_threads;
-  for (size_t i = 0; i < 5 * std::thread::hardware_concurrency(); i++) {
-    stress_threads.emplace_back(
-      [&should_cancel, i]() {
-        // This is just some arbitrary heavy work
-        volatile size_t total = 0;
-        for (size_t k = 0; k < 549528914167; k++) {
-          if (should_cancel) {
-            break;
-          }
-          total += k * (i + 42);
-        }
-      });
-  }
-
-  // Create an executor
-  auto executor = std::make_shared<ExecutorType>();
-  // Start spinning
-  auto executor_thread = std::thread(
-    [executor]() {
-      executor->spin();
-    });
-  // Add a node to the executor
-  executor->add_node(this->node);
-
-  // Cancel the executor (make sure that it's already spinning first)
-  while (!executor->is_spinning() && rclcpp::ok()) {
-    continue;
-  }
-  executor->cancel();
-
-  // Try to join the thread after cancelling the executor
-  // This is the "test". We want to make sure that we can still cancel the executor
-  // regardless of the presence of race conditions
-  executor_thread.join();
-
-  // The test is now completed: we can join the stress threads
-  should_cancel = true;
-  for (auto & t : stress_threads) {
-    t.join();
-  }
-}
-
 // Check spin_until_future_complete with node base pointer (instantiates its own executor)
 TEST(TestExecutors, testSpinUntilFutureCompleteNodeBasePtr)
 {
@@ -858,7 +780,7 @@ public:
     test_name << test_info->test_case_name() << "_" << test_info->name();
     node = std::make_shared<rclcpp::Node>("node", test_name.str());
 
-    callback_count = 0u;
+    callback_count = 0;
 
     const std::string topic_name = std::string("topic_") + test_name.str();
 
@@ -867,7 +789,7 @@ public:
     publisher = node->create_publisher<test_msgs::msg::Empty>(topic_name, rclcpp::QoS(1), po);
 
     auto callback = [this](test_msgs::msg::Empty::ConstSharedPtr) {
-        this->callback_count.fetch_add(1u);
+        this->callback_count.fetch_add(1);
       };
 
     rclcpp::SubscriptionOptions so;
@@ -889,7 +811,7 @@ public:
   rclcpp::Node::SharedPtr node;
   rclcpp::Publisher<test_msgs::msg::Empty>::SharedPtr publisher;
   rclcpp::Subscription<test_msgs::msg::Empty>::SharedPtr subscription;
-  std::atomic_size_t callback_count;
+  std::atomic_int callback_count;
 };
 
 TYPED_TEST_SUITE(TestIntraprocessExecutors, ExecutorTypes, ExecutorTypeNames);
@@ -905,7 +827,7 @@ TYPED_TEST(TestIntraprocessExecutors, testIntraprocessRetrigger) {
   ExecutorType executor;
   executor.add_node(this->node);
 
-  EXPECT_EQ(0u, this->callback_count.load());
+  EXPECT_EQ(0, this->callback_count.load());
   this->publisher->publish(test_msgs::msg::Empty());
 
   // Wait for up to 5 seconds for the first message to come available.
@@ -919,7 +841,7 @@ TYPED_TEST(TestIntraprocessExecutors, testIntraprocessRetrigger) {
   EXPECT_EQ(1u, this->callback_count.load());
 
   // reset counter
-  this->callback_count.store(0u);
+  this->callback_count.store(0);
 
   for (size_t ii = 0; ii < kNumMessages; ++ii) {
     this->publisher->publish(test_msgs::msg::Empty());

rclcpp/test/rclcpp/node_interfaces/test_node_parameters.cpp

@@ -77,9 +77,9 @@ TEST_F(TestNodeParameters, list_parameters)
   std::vector<std::string> prefixes;
   const auto list_result = node_parameters->list_parameters(prefixes, 1u);
 
-  // Currently the default parameters are 'use_sim_time' and 'start_type_description_service'
+  // Currently the only default parameter is 'use_sim_time', but that may change.
   size_t number_of_parameters = list_result.names.size();
-  EXPECT_GE(2u, number_of_parameters);
+  EXPECT_GE(1u, number_of_parameters);
 
   const std::string parameter_name = "new_parameter";
   const rclcpp::ParameterValue value(true);

rclcpp/test/rclcpp/node_interfaces/test_node_type_descriptions.cpp

@@ -1,63 +0,0 @@
-// Copyright 2023 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 "rclcpp/node.hpp"
-#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
-
-class TestNodeTypeDescriptions : public ::testing::Test
-{
-public:
-  void SetUp()
-  {
-    rclcpp::init(0, nullptr);
-  }
-
-  void TearDown()
-  {
-    rclcpp::shutdown();
-  }
-};
-
-TEST_F(TestNodeTypeDescriptions, interface_created)
-{
-  rclcpp::Node node{"node", "ns"};
-  ASSERT_NE(nullptr, node.get_node_type_descriptions_interface());
-}
-
-TEST_F(TestNodeTypeDescriptions, disabled_no_service)
-{
-  rclcpp::NodeOptions node_options;
-  node_options.append_parameter_override("start_type_description_service", false);
-  rclcpp::Node node{"node", "ns", node_options};
-
-  rcl_node_t * rcl_node = node.get_node_base_interface()->get_rcl_node_handle();
-  rcl_service_t * srv = nullptr;
-  rcl_ret_t ret = rcl_node_get_type_description_service(rcl_node, &srv);
-  ASSERT_EQ(RCL_RET_NOT_INIT, ret);
-}
-
-TEST_F(TestNodeTypeDescriptions, enabled_creates_service)
-{
-  rclcpp::NodeOptions node_options;
-  node_options.append_parameter_override("start_type_description_service", true);
-  rclcpp::Node node{"node", "ns", node_options};
-
-  rcl_node_t * rcl_node = node.get_node_base_interface()->get_rcl_node_handle();
-  rcl_service_t * srv = nullptr;
-  rcl_ret_t ret = rcl_node_get_type_description_service(rcl_node, &srv);
-  ASSERT_EQ(RCL_RET_OK, ret);
-  ASSERT_NE(nullptr, srv);
-}

rclcpp/test/rclcpp/test_executor.cpp

@@ -46,23 +46,6 @@ public:
   {
     spin_node_once_nanoseconds(node, std::chrono::milliseconds(100));
   }
-
-  rclcpp::memory_strategy::MemoryStrategy * memory_strategy_ptr()
-  {
-    return memory_strategy_.get();
-  }
-
-  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr local_get_node_by_group(
-    rclcpp::CallbackGroup::SharedPtr group)
-  {
-    std::lock_guard<std::mutex> guard_{mutex_};  // only to make the TSA happy
-    return get_node_by_group(weak_groups_to_nodes_, group);
-  }
-
-  rclcpp::CallbackGroup::SharedPtr local_get_group_by_timer(rclcpp::TimerBase::SharedPtr timer)
-  {
-    return get_group_by_timer(timer);
-  }
 };
 
 class TestExecutor : public ::testing::Test
@@ -130,7 +113,7 @@ TEST_F(TestExecutor, constructor_bad_wait_set_init) {
   auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_wait_set_init, RCL_RET_ERROR);
   RCLCPP_EXPECT_THROW_EQ(
     static_cast<void>(std::make_unique<DummyExecutor>()),
-    std::runtime_error("Failed to create wait set in Executor constructor: error not set"));
+    std::runtime_error("Failed to create wait set: error not set"));
 }
 
 TEST_F(TestExecutor, add_callback_group_twice) {
@@ -142,7 +125,7 @@ TEST_F(TestExecutor, add_callback_group_twice) {
   cb_group->get_associated_with_executor_atomic().exchange(false);
   RCLCPP_EXPECT_THROW_EQ(
     dummy.add_callback_group(cb_group, node->get_node_base_interface(), false),
-    std::runtime_error("Callback group was already added to executor."));
+    std::runtime_error("Callback group has already been added to this executor."));
 }
 
 TEST_F(TestExecutor, add_callback_group_failed_trigger_guard_condition) {
@@ -168,9 +151,15 @@ TEST_F(TestExecutor, remove_callback_group_null_node) {
 
   node.reset();
 
+
+  /**
+   * TODO(mjcarroll): Assert this when we are enforcing that nodes must be destroyed
+   * after their created callback groups.
   RCLCPP_EXPECT_THROW_EQ(
     dummy.remove_callback_group(cb_group, false),
     std::runtime_error("Node must not be deleted before its callback group(s)."));
+   */
+  EXPECT_NO_THROW(dummy.remove_callback_group(cb_group, false));
 }
 
 TEST_F(TestExecutor, remove_callback_group_failed_trigger_guard_condition) {
@@ -197,7 +186,7 @@ TEST_F(TestExecutor, remove_node_not_associated) {
 
   RCLCPP_EXPECT_THROW_EQ(
     dummy.remove_node(node->get_node_base_interface(), false),
-    std::runtime_error("Node needs to be associated with an executor."));
+    std::runtime_error("Node '/ns/node' needs to be associated with an executor."));
 }
 
 TEST_F(TestExecutor, remove_node_associated_with_different_executor) {
@@ -211,7 +200,7 @@ TEST_F(TestExecutor, remove_node_associated_with_different_executor) {
 
   RCLCPP_EXPECT_THROW_EQ(
     dummy2.remove_node(node1->get_node_base_interface(), false),
-    std::runtime_error("Node needs to be associated with this executor."));
+    std::runtime_error("Node '/ns/node1' needs to be associated with this executor."));
 }
 
 TEST_F(TestExecutor, spin_node_once_nanoseconds) {
@@ -328,42 +317,14 @@ TEST_F(TestExecutor, cancel_failed_trigger_guard_condition) {
     std::runtime_error("Failed to trigger guard condition in cancel: error not set"));
 }
 
-TEST_F(TestExecutor, set_memory_strategy_nullptr) {
-  DummyExecutor dummy;
-
+TEST_F(TestExecutor, create_executor_fail_wait_set_clear) {
+  auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_wait_set_clear, RCL_RET_ERROR);
   RCLCPP_EXPECT_THROW_EQ(
-    dummy.set_memory_strategy(nullptr),
-    std::runtime_error("Received NULL memory strategy in executor."));
+    DummyExecutor dummy,
+    std::runtime_error("Couldn't clear the wait set: error not set"));
 }
 
-TEST_F(TestExecutor, set_memory_strategy) {
-  DummyExecutor dummy;
-  rclcpp::memory_strategy::MemoryStrategy::SharedPtr strategy =
-    std::make_shared<
-    rclcpp::memory_strategies::allocator_memory_strategy::AllocatorMemoryStrategy<>>();
-
-  dummy.set_memory_strategy(strategy);
-  EXPECT_EQ(dummy.memory_strategy_ptr(), strategy.get());
-}
-
-TEST_F(TestExecutor, spin_once_failed_trigger_guard_condition) {
-  DummyExecutor dummy;
-  auto node = std::make_shared<rclcpp::Node>("node", "ns");
-  auto timer =
-    node->create_wall_timer(std::chrono::milliseconds(1), [&]() {});
-
-  dummy.add_node(node);
-  // Wait for the wall timer to have expired.
-  std::this_thread::sleep_for(std::chrono::milliseconds(50));
-  auto mock = mocking_utils::patch_and_return(
-    "lib:rclcpp", rcl_trigger_guard_condition, RCL_RET_ERROR);
-  RCLCPP_EXPECT_THROW_EQ(
-    dummy.spin_once(std::chrono::milliseconds(1)),
-    std::runtime_error(
-      "Failed to trigger guard condition from execute_any_executable: error not set"));
-}
-
-TEST_F(TestExecutor, spin_some_fail_wait_set_clear) {
+TEST_F(TestExecutor, spin_all_fail_wait_set_clear) {
   DummyExecutor dummy;
   auto node = std::make_shared<rclcpp::Node>("node", "ns");
   auto timer =
@@ -371,9 +332,10 @@ TEST_F(TestExecutor, spin_some_fail_wait_set_clear) {
 
   dummy.add_node(node);
   auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_wait_set_clear, RCL_RET_ERROR);
+
   RCLCPP_EXPECT_THROW_EQ(
-    dummy.spin_some(std::chrono::milliseconds(1)),
-    std::runtime_error("Couldn't clear wait set: error not set"));
+    dummy.spin_all(std::chrono::milliseconds(1)),
+    std::runtime_error("Couldn't clear the wait set: error not set"));
 }
 
 TEST_F(TestExecutor, spin_some_fail_wait_set_resize) {
@@ -401,7 +363,7 @@ TEST_F(TestExecutor, spin_some_fail_add_handles_to_wait_set) {
     RCL_RET_ERROR);
   RCLCPP_EXPECT_THROW_EQ(
     dummy.spin_some(std::chrono::milliseconds(1)),
-    std::runtime_error("Couldn't fill wait set"));
+    std::runtime_error("Couldn't fill wait set: error not set"));
 }
 
 TEST_F(TestExecutor, spin_some_fail_wait) {
@@ -417,71 +379,6 @@ TEST_F(TestExecutor, spin_some_fail_wait) {
     std::runtime_error("rcl_wait() failed: error not set"));
 }
 
-TEST_F(TestExecutor, get_node_by_group_null_group) {
-  DummyExecutor dummy;
-  ASSERT_EQ(nullptr, dummy.local_get_node_by_group(nullptr));
-}
-
-TEST_F(TestExecutor, get_node_by_group) {
-  DummyExecutor dummy;
-  auto node = std::make_shared<rclcpp::Node>("node", "ns");
-  rclcpp::CallbackGroup::SharedPtr cb_group = node->create_callback_group(
-    rclcpp::CallbackGroupType::MutuallyExclusive);
-  dummy.add_callback_group(cb_group, node->get_node_base_interface(), false);
-  ASSERT_EQ(node->get_node_base_interface().get(), dummy.local_get_node_by_group(cb_group).get());
-}
-
-TEST_F(TestExecutor, get_node_by_group_not_found) {
-  DummyExecutor dummy;
-  auto node = std::make_shared<rclcpp::Node>("node", "ns");
-  rclcpp::CallbackGroup::SharedPtr cb_group = node->create_callback_group(
-    rclcpp::CallbackGroupType::MutuallyExclusive);
-  ASSERT_EQ(nullptr, dummy.local_get_node_by_group(cb_group).get());
-}
-
-TEST_F(TestExecutor, get_group_by_timer_nullptr) {
-  DummyExecutor dummy;
-  ASSERT_EQ(nullptr, dummy.local_get_group_by_timer(nullptr));
-}
-
-TEST_F(TestExecutor, get_group_by_timer) {
-  DummyExecutor dummy;
-  auto node = std::make_shared<rclcpp::Node>("node", "ns");
-  rclcpp::CallbackGroup::SharedPtr cb_group = node->create_callback_group(
-    rclcpp::CallbackGroupType::MutuallyExclusive);
-  auto timer =
-    node->create_wall_timer(std::chrono::milliseconds(1), [&]() {}, cb_group);
-  dummy.add_node(node);
-
-  ASSERT_EQ(cb_group.get(), dummy.local_get_group_by_timer(timer).get());
-}
-
-TEST_F(TestExecutor, get_group_by_timer_with_deleted_group) {
-  DummyExecutor dummy;
-  auto node = std::make_shared<rclcpp::Node>("node", "ns");
-  rclcpp::CallbackGroup::SharedPtr cb_group = node->create_callback_group(
-    rclcpp::CallbackGroupType::MutuallyExclusive);
-  auto timer =
-    node->create_wall_timer(std::chrono::milliseconds(1), [&]() {}, cb_group);
-  dummy.add_node(node);
-
-  cb_group.reset();
-
-  ASSERT_EQ(nullptr, dummy.local_get_group_by_timer(timer).get());
-}
-
-TEST_F(TestExecutor, get_group_by_timer_add_callback_group) {
-  DummyExecutor dummy;
-  auto node = std::make_shared<rclcpp::Node>("node", "ns");
-  rclcpp::CallbackGroup::SharedPtr cb_group = node->create_callback_group(
-    rclcpp::CallbackGroupType::MutuallyExclusive);
-  auto timer =
-    node->create_wall_timer(std::chrono::milliseconds(1), [&]() {}, cb_group);
-  dummy.add_callback_group(cb_group, node->get_node_base_interface(), false);
-
-  ASSERT_EQ(cb_group.get(), dummy.local_get_group_by_timer(timer).get());
-}
-
 TEST_F(TestExecutor, spin_until_future_complete_in_spin_until_future_complete) {
   DummyExecutor dummy;
   auto node = std::make_shared<rclcpp::Node>("node", "ns");

rclcpp/test/rclcpp/test_logger_service.cpp

@@ -1,214 +0,0 @@
-// Copyright 2023 Sony Group Corporation.
-//
-// 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 <string>
-#include <vector>
-
-#include "rclcpp/rclcpp.hpp"
-#include "rclcpp/node.hpp"
-#include "rcl_interfaces/srv/get_logger_levels.hpp"
-#include "rcl_interfaces/srv/set_logger_levels.hpp"
-
-using namespace std::chrono_literals;
-
-class TestLoggerService : public ::testing::Test
-{
-protected:
-  void SetUp()
-  {
-    rclcpp::init(0, nullptr);
-    rclcpp::NodeOptions options = rclcpp::NodeOptions();
-    options.enable_logger_service(true);
-    node_ = std::make_shared<rclcpp::Node>("test_logger_service", "/test", options);
-  }
-
-  void TearDown()
-  {
-    rclcpp::shutdown();
-  }
-
-  rclcpp::Node::SharedPtr node_;
-  std::thread thread_;
-};
-
-TEST_F(TestLoggerService, check_connect_get_logger_service) {
-  auto client = node_->create_client<rcl_interfaces::srv::GetLoggerLevels>(
-    "/test/test_logger_service/get_logger_levels");
-  ASSERT_TRUE(client->wait_for_service(2s));
-}
-
-TEST_F(TestLoggerService, check_connect_set_logger_service) {
-  auto client = node_->create_client<rcl_interfaces::srv::SetLoggerLevels>(
-    "/test/test_logger_service/set_logger_levels");
-  ASSERT_TRUE(client->wait_for_service(2s));
-}
-
-TEST_F(TestLoggerService, test_set_and_get_one_logging_level) {
-  std::string test_logger_name = "rcl";
-  uint8_t test_logger_level = 20;
-  {
-    auto client = node_->create_client<rcl_interfaces::srv::SetLoggerLevels>(
-      "/test/test_logger_service/set_logger_levels");
-    ASSERT_TRUE(client->wait_for_service(1s));
-    auto request = std::make_shared<rcl_interfaces::srv::SetLoggerLevels::Request>();
-    auto level = rcl_interfaces::msg::LoggerLevel();
-    level.name = test_logger_name;
-    level.level = test_logger_level;
-    request->levels.push_back(level);
-    auto result = client->async_send_request(request);
-    ASSERT_EQ(
-      rclcpp::spin_until_future_complete(node_, result),
-      rclcpp::FutureReturnCode::SUCCESS);
-    auto result_get = result.get();
-    ASSERT_EQ(result_get->results.size(), 1u);
-    ASSERT_TRUE(result_get->results[0].successful);
-    ASSERT_STREQ(result_get->results[0].reason.c_str(), "");
-  }
-
-  {
-    auto client = node_->create_client<rcl_interfaces::srv::GetLoggerLevels>(
-      "/test/test_logger_service/get_logger_levels");
-    ASSERT_TRUE(client->wait_for_service(1s));
-    auto request = std::make_shared<rcl_interfaces::srv::GetLoggerLevels::Request>();
-    request->names.emplace_back(test_logger_name);
-    auto result = client->async_send_request(request);
-    ASSERT_EQ(
-      rclcpp::spin_until_future_complete(node_, result),
-      rclcpp::FutureReturnCode::SUCCESS);
-    auto result_get = result.get();
-    ASSERT_EQ(result_get->levels.size(), 1u);
-    ASSERT_STREQ(result_get->levels[0].name.c_str(), test_logger_name.c_str());
-    ASSERT_EQ(result_get->levels[0].level, test_logger_level);
-  }
-}
-
-TEST_F(TestLoggerService, test_set_and_get_multi_logging_level) {
-  std::vector<std::pair<std::string, uint8_t>> test_data {
-    {"rcl", 30},
-    {"rclcpp", 40},
-    {"/test/test_logger_service", 50}
-  };
-
-  // Set multi log levels
-  {
-    auto client = node_->create_client<rcl_interfaces::srv::SetLoggerLevels>(
-      "/test/test_logger_service/set_logger_levels");
-    ASSERT_TRUE(client->wait_for_service(1s));
-    auto request = std::make_shared<rcl_interfaces::srv::SetLoggerLevels::Request>();
-    for (auto & set_level : test_data) {
-      auto level = rcl_interfaces::msg::LoggerLevel();
-      level.name = std::get<0>(set_level);
-      level.level = std::get<1>(set_level);
-      request->levels.push_back(level);
-    }
-    auto result = client->async_send_request(request);
-    ASSERT_EQ(
-      rclcpp::spin_until_future_complete(node_, result),
-      rclcpp::FutureReturnCode::SUCCESS);
-    auto result_get = result.get();
-    ASSERT_EQ(result_get->results.size(), test_data.size());
-    for (uint32_t i = 0; i < test_data.size(); i++) {
-      ASSERT_TRUE(result_get->results[0].successful);
-    }
-  }
-
-  // Get multi log levels
-  {
-    auto client = node_->create_client<rcl_interfaces::srv::GetLoggerLevels>(
-      "/test/test_logger_service/get_logger_levels");
-    ASSERT_TRUE(client->wait_for_service(1s));
-    auto request = std::make_shared<rcl_interfaces::srv::GetLoggerLevels::Request>();
-    for (auto & set_level : test_data) {
-      request->names.emplace_back(std::get<0>(set_level));
-    }
-    auto result = client->async_send_request(request);
-    ASSERT_EQ(
-      rclcpp::spin_until_future_complete(node_, result),
-      rclcpp::FutureReturnCode::SUCCESS);
-    auto result_get = result.get();
-    ASSERT_EQ(result_get->levels.size(), test_data.size());
-    for (uint32_t i = 0; i < test_data.size(); i++) {
-      ASSERT_STREQ(result_get->levels[i].name.c_str(), std::get<0>(test_data[i]).c_str());
-      ASSERT_EQ(result_get->levels[i].level, std::get<1>(test_data[i]));
-    }
-  }
-}
-
-TEST_F(TestLoggerService, test_set_logging_level_with_invalid_param) {
-  std::vector<std::pair<std::string, uint8_t>> test_data {
-    {"rcl", 12},
-    {"/test/test_logger_service", 22}
-  };
-
-  // Set multi log levels
-  {
-    auto client = node_->create_client<rcl_interfaces::srv::SetLoggerLevels>(
-      "/test/test_logger_service/set_logger_levels");
-    ASSERT_TRUE(client->wait_for_service(1s));
-    auto request = std::make_shared<rcl_interfaces::srv::SetLoggerLevels::Request>();
-    for (auto & set_level : test_data) {
-      auto level = rcl_interfaces::msg::LoggerLevel();
-      level.name = std::get<0>(set_level);
-      level.level = std::get<1>(set_level);
-      request->levels.push_back(level);
-    }
-    auto result = client->async_send_request(request);
-    ASSERT_EQ(
-      rclcpp::spin_until_future_complete(node_, result),
-      rclcpp::FutureReturnCode::SUCCESS);
-    auto result_get = result.get();
-    ASSERT_EQ(result_get->results.size(), test_data.size());
-    for (uint32_t i = 0; i < test_data.size(); i++) {
-      ASSERT_FALSE(result_get->results[i].successful);
-      // Check string starts with prefix
-      ASSERT_EQ(
-        result_get->results[i].reason.rfind("Unable to determine severity_string for severity", 0),
-        0);
-    }
-  }
-}
-
-TEST_F(TestLoggerService, test_set_logging_level_with_partial_invalid_param) {
-  std::vector<std::pair<std::string, uint8_t>> test_data {
-    {"rcl", 20},
-    {"rclcpp", 22},
-    {"/test/test_logger_service", 30}
-  };
-
-  // Set multi log levels
-  {
-    auto client = node_->create_client<rcl_interfaces::srv::SetLoggerLevels>(
-      "/test/test_logger_service/set_logger_levels");
-    ASSERT_TRUE(client->wait_for_service(1s));
-    auto request = std::make_shared<rcl_interfaces::srv::SetLoggerLevels::Request>();
-    for (auto & set_level : test_data) {
-      auto level = rcl_interfaces::msg::LoggerLevel();
-      level.name = std::get<0>(set_level);
-      level.level = std::get<1>(set_level);
-      request->levels.push_back(level);
-    }
-    auto result = client->async_send_request(request);
-    ASSERT_EQ(
-      rclcpp::spin_until_future_complete(node_, result),
-      rclcpp::FutureReturnCode::SUCCESS);
-    auto result_get = result.get();
-    ASSERT_EQ(result_get->results.size(), test_data.size());
-    ASSERT_TRUE(result_get->results[0].successful);
-    ASSERT_FALSE(result_get->results[1].successful);
-    ASSERT_TRUE(result_get->results[2].successful);
-  }
-}

rclcpp/test/rclcpp/test_node.cpp

@@ -78,7 +78,6 @@ TEST_F(TestNode, construction_and_destruction) {
     EXPECT_NE(nullptr, node->get_node_options().get_rcl_node_options());
     EXPECT_NE(nullptr, node->get_graph_event());
     EXPECT_NE(nullptr, node->get_clock());
-    EXPECT_NE(nullptr, node->get_node_type_descriptions_interface());
   }
 
   {

rclcpp/test/rclcpp/test_node_options.cpp

@@ -266,11 +266,6 @@ TEST(TestNodeOptions, bool_setters_and_getters) {
   EXPECT_FALSE(options.automatically_declare_parameters_from_overrides());
   options.automatically_declare_parameters_from_overrides(true);
   EXPECT_TRUE(options.automatically_declare_parameters_from_overrides());
-
-  options.enable_logger_service(false);
-  EXPECT_FALSE(options.enable_logger_service());
-  options.enable_logger_service(true);
-  EXPECT_TRUE(options.enable_logger_service());
 }
 
 TEST(TestNodeOptions, parameter_event_qos) {

rclcpp/test/rclcpp/test_parameter_client.cpp

@@ -59,8 +59,6 @@ protected:
     node_with_option.reset();
   }
 
-  // "start_type_description_service" and "use_sim_time"
-  const uint64_t builtin_param_count = 2;
   rclcpp::Node::SharedPtr node;
   rclcpp::Node::SharedPtr node_with_option;
 };
@@ -927,7 +925,6 @@ TEST_F(TestParameterClient, sync_parameter_delete_parameters) {
   Coverage for async load_parameters
  */
 TEST_F(TestParameterClient, async_parameter_load_parameters) {
-  const uint64_t expected_param_count = 4 + builtin_param_count;
   auto load_node = std::make_shared<rclcpp::Node>(
     "load_node",
     "namespace",
@@ -947,13 +944,12 @@ TEST_F(TestParameterClient, async_parameter_load_parameters) {
   auto list_parameters = asynchronous_client->list_parameters({}, 3);
   rclcpp::spin_until_future_complete(
     load_node, list_parameters, std::chrono::milliseconds(100));
-  ASSERT_EQ(list_parameters.get().names.size(), expected_param_count);
+  ASSERT_EQ(list_parameters.get().names.size(), static_cast<uint64_t>(5));
 }
 /*
   Coverage for sync load_parameters
  */
 TEST_F(TestParameterClient, sync_parameter_load_parameters) {
-  const uint64_t expected_param_count = 4 + builtin_param_count;
   auto load_node = std::make_shared<rclcpp::Node>(
     "load_node",
     "namespace",
@@ -968,14 +964,13 @@ TEST_F(TestParameterClient, sync_parameter_load_parameters) {
   ASSERT_EQ(load_future[0].successful, true);
   // list parameters
   auto list_parameters = synchronous_client->list_parameters({}, 3);
-  ASSERT_EQ(list_parameters.names.size(), static_cast<uint64_t>(expected_param_count));
+  ASSERT_EQ(list_parameters.names.size(), static_cast<uint64_t>(5));
 }
 
 /*
   Coverage for async load_parameters with complicated regex expression
  */
 TEST_F(TestParameterClient, async_parameter_load_parameters_complicated_regex) {
-  const uint64_t expected_param_count = 5 + builtin_param_count;
   auto load_node = std::make_shared<rclcpp::Node>(
     "load_node",
     "namespace",
@@ -995,7 +990,7 @@ TEST_F(TestParameterClient, async_parameter_load_parameters_complicated_regex) {
   auto list_parameters = asynchronous_client->list_parameters({}, 3);
   rclcpp::spin_until_future_complete(
     load_node, list_parameters, std::chrono::milliseconds(100));
-  ASSERT_EQ(list_parameters.get().names.size(), expected_param_count);
+  ASSERT_EQ(list_parameters.get().names.size(), static_cast<uint64_t>(6));
   // to check the parameter "a_value"
   std::string param_name = "a_value";
   auto param = load_node->get_parameter(param_name);
@@ -1025,7 +1020,6 @@ TEST_F(TestParameterClient, async_parameter_load_no_valid_parameter) {
   Coverage for async load_parameters from maps with complicated regex expression
  */
 TEST_F(TestParameterClient, async_parameter_load_parameters_from_map) {
-  const uint64_t expected_param_count = 5 + builtin_param_count;
   auto load_node = std::make_shared<rclcpp::Node>(
     "load_node",
     "namespace",
@@ -1074,7 +1068,7 @@ TEST_F(TestParameterClient, async_parameter_load_parameters_from_map) {
   auto list_parameters = asynchronous_client->list_parameters({}, 3);
   rclcpp::spin_until_future_complete(
     load_node, list_parameters, std::chrono::milliseconds(100));
-  ASSERT_EQ(list_parameters.get().names.size(), expected_param_count);
+  ASSERT_EQ(list_parameters.get().names.size(), static_cast<uint64_t>(6));
   // to check the parameter "a_value"
   std::string param_name = "a_value";
   auto param = load_node->get_parameter(param_name);

rclcpp/test/rclcpp/test_publisher_with_type_adapter.cpp

@@ -152,54 +152,33 @@ TEST_F(TestPublisher, conversion_exception_is_passed_up) {
   }
 }
 
-using UseTakeSharedMethod = bool;
-class TestPublisherFixture
-  : public TestPublisher,
-  public ::testing::WithParamInterface<UseTakeSharedMethod>
-{
-};
-
 /*
  * Testing that publisher sends type adapted types and ROS message types with intra proccess communications.
  */
-TEST_P(
-  TestPublisherFixture,
+TEST_F(
+  TestPublisher,
   check_type_adapted_message_is_sent_and_received_intra_process) {
   using StringTypeAdapter = rclcpp::TypeAdapter<std::string, rclcpp::msg::String>;
   const std::string message_data = "Message Data";
   const std::string topic_name = "topic_name";
   bool is_received;
 
+  auto callback =
+    [message_data, &is_received](
+    const rclcpp::msg::String::ConstSharedPtr msg,
+    const rclcpp::MessageInfo & message_info
+    ) -> void
+    {
+      is_received = true;
+      ASSERT_STREQ(message_data.c_str(), msg->data.c_str());
+      ASSERT_TRUE(message_info.get_rmw_message_info().from_intra_process);
+    };
+
   auto node = rclcpp::Node::make_shared(
     "test_intra_process",
     rclcpp::NodeOptions().use_intra_process_comms(true));
   auto pub = node->create_publisher<StringTypeAdapter>(topic_name, 10);
-  rclcpp::Subscription<rclcpp::msg::String>::SharedPtr sub;
-  if (GetParam()) {
-    auto callback =
-      [message_data, &is_received](
-      const rclcpp::msg::String::ConstSharedPtr msg,
-      const rclcpp::MessageInfo & message_info
-      ) -> void
-      {
-        is_received = true;
-        ASSERT_STREQ(message_data.c_str(), msg->data.c_str());
-        ASSERT_TRUE(message_info.get_rmw_message_info().from_intra_process);
-      };
-    sub = node->create_subscription<rclcpp::msg::String>(topic_name, 1, callback);
-  } else {
-    auto callback_unique =
-      [message_data, &is_received](
-      rclcpp::msg::String::UniquePtr msg,
-      const rclcpp::MessageInfo & message_info
-      ) -> void
-      {
-        is_received = true;
-        ASSERT_STREQ(message_data.c_str(), msg->data.c_str());
-        ASSERT_TRUE(message_info.get_rmw_message_info().from_intra_process);
-      };
-    sub = node->create_subscription<rclcpp::msg::String>(topic_name, 1, callback_unique);
-  }
+  auto sub = node->create_subscription<rclcpp::msg::String>(topic_name, 1, callback);
 
   auto wait_for_message_to_be_received = [&is_received, &node]() {
       rclcpp::executors::SingleThreadedExecutor executor;
@@ -260,14 +239,6 @@ TEST_P(
   }
 }
 
-INSTANTIATE_TEST_SUITE_P(
-  TestPublisherFixtureWithParam,
-  TestPublisherFixture,
-  ::testing::Values(
-    true,   // use take shared method
-    false   // not use take shared method
-));
-
 /*
  * Testing that publisher sends type adapted types and ROS message types with inter proccess communications.
  */

rclcpp/test/rclcpp/test_rate.cpp

@@ -18,24 +18,10 @@
 
 #include "rclcpp/rate.hpp"
 
-class TestRate : public ::testing::Test
-{
-public:
-  void SetUp()
-  {
-    rclcpp::init(0, nullptr);
-  }
-
-  void TearDown()
-  {
-    rclcpp::shutdown();
-  }
-};
-
 /*
    Basic tests for the Rate and WallRate classes.
  */
-TEST_F(TestRate, rate_basics) {
+TEST(TestRate, rate_basics) {
   auto period = std::chrono::milliseconds(1000);
   auto offset = std::chrono::milliseconds(500);
   auto epsilon = std::chrono::milliseconds(100);
@@ -75,7 +61,7 @@ TEST_F(TestRate, rate_basics) {
   ASSERT_TRUE(epsilon > delta);
 }
 
-TEST_F(TestRate, wall_rate_basics) {
+TEST(TestRate, wall_rate_basics) {
   auto period = std::chrono::milliseconds(100);
   auto offset = std::chrono::milliseconds(50);
   auto epsilon = std::chrono::milliseconds(1);
@@ -115,7 +101,7 @@ TEST_F(TestRate, wall_rate_basics) {
   EXPECT_GT(epsilon, delta);
 }
 
-TEST_F(TestRate, from_double) {
+TEST(TestRate, from_double) {
   {
     rclcpp::WallRate rate(1.0);
     EXPECT_EQ(std::chrono::seconds(1), rate.period());

rclcpp_action/CHANGELOG.rst

@@ -3,27 +3,6 @@ Changelog for package rclcpp_action
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
-21.0.6 (2024-04-19)
--------------------
-
-21.0.5 (2024-02-07)
--------------------
-
-21.0.4 (2023-11-17)
--------------------
-
-21.0.3 (2023-09-08)
--------------------
-
-21.0.2 (2023-07-14)
--------------------
-
-21.0.1 (2023-05-11)
--------------------
-
-21.0.0 (2023-04-18)
--------------------
-
 20.0.0 (2023-04-13)
 -------------------
 * extract the result response before the callback is issued. (`#2132 <https://github.com/ros2/rclcpp/issues/2132>`_)

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>21.0.6</version>
+  <version>20.0.0</version>
   <description>Adds action APIs for C++.</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp_components/CHANGELOG.rst

@@ -2,31 +2,6 @@
 Changelog for package rclcpp_components
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-21.0.6 (2024-04-19)
--------------------
-
-21.0.5 (2024-02-07)
--------------------
-* Increase the service queue sizes in component_container (`#2381 <https://github.com/ros2/rclcpp/issues/2381>`_)
-* Contributors: M. Fatih Cırıt
-
-21.0.4 (2023-11-17)
--------------------
-* Add missing header required by the rclcpp::NodeOptions type (`#2325 <https://github.com/ros2/rclcpp/issues/2325>`_)
-* Contributors: Ignacio Vizzo
-
-21.0.3 (2023-09-08)
--------------------
-
-21.0.2 (2023-07-14)
--------------------
-
-21.0.1 (2023-05-11)
--------------------
-
-21.0.0 (2023-04-18)
--------------------
-
 20.0.0 (2023-04-13)
 -------------------
 * Update all rclcpp packages to C++17. (`#2121 <https://github.com/ros2/rclcpp/issues/2121>`_)

rclcpp_components/include/rclcpp_components/node_factory.hpp

@@ -15,7 +15,6 @@
 #ifndef RCLCPP_COMPONENTS__NODE_FACTORY_HPP__
 #define RCLCPP_COMPONENTS__NODE_FACTORY_HPP__
 
-#include "rclcpp/node_options.hpp"
 #include "rclcpp_components/node_instance_wrapper.hpp"
 
 namespace rclcpp_components

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>21.0.6</version>
+  <version>20.0.0</version>
   <description>Package containing tools for dynamically loadable components</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp_components/src/component_manager.cpp

@@ -39,12 +39,10 @@ ComponentManager::ComponentManager(
 {
   loadNode_srv_ = create_service<LoadNode>(
     "~/_container/load_node",
-    std::bind(&ComponentManager::on_load_node, this, _1, _2, _3),
-    rclcpp::ServicesQoS().keep_last(200));
+    std::bind(&ComponentManager::on_load_node, this, _1, _2, _3));
   unloadNode_srv_ = create_service<UnloadNode>(
     "~/_container/unload_node",
-    std::bind(&ComponentManager::on_unload_node, this, _1, _2, _3),
-    rclcpp::ServicesQoS().keep_last(200));
+    std::bind(&ComponentManager::on_unload_node, this, _1, _2, _3));
   listNodes_srv_ = create_service<ListNodes>(
     "~/_container/list_nodes",
     std::bind(&ComponentManager::on_list_nodes, this, _1, _2, _3));

rclcpp_lifecycle/CHANGELOG.rst

@@ -3,35 +3,6 @@ Changelog for package rclcpp_lifecycle
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
-21.0.6 (2024-04-19)
--------------------
-* call shutdown in LifecycleNode dtor to avoid leaving the device in un… (`#2490 <https://github.com/ros2/rclcpp/issues/2490>`_)
-* Contributors: Tomoya Fujita
-
-21.0.5 (2024-02-07)
--------------------
-
-21.0.4 (2023-11-17)
--------------------
-
-21.0.3 (2023-09-08)
--------------------
-* Switch lifecycle to use the RCLCPP macros. (`#2244 <https://github.com/ros2/rclcpp/issues/2244>`_)
-* Add new node interface TypeDescriptionsInterface to provide GetTypeDescription service (`#2236 <https://github.com/ros2/rclcpp/issues/2236>`_)
-* Contributors: Emerson Knapp
-
-21.0.2 (2023-07-14)
--------------------
-
-21.0.1 (2023-05-11)
--------------------
-
-21.0.0 (2023-04-18)
--------------------
-* Add support for logging service. (`#2122 <https://github.com/ros2/rclcpp/issues/2122>`_)
-* Support publishing loaned messages in LifecyclePublisher (`#2159 <https://github.com/ros2/rclcpp/issues/2159>`_)
-* Contributors: Lei Liu, Michael Babenko
-
 20.0.0 (2023-04-13)
 -------------------
 * Fixes to silence some clang warnings. (`#2127 <https://github.com/ros2/rclcpp/issues/2127>`_)

rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp

@@ -72,7 +72,6 @@
 #include "rclcpp/node_interfaces/node_time_source_interface.hpp"
 #include "rclcpp/node_interfaces/node_timers_interface.hpp"
 #include "rclcpp/node_interfaces/node_topics_interface.hpp"
-#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp"
 #include "rclcpp/node_interfaces/node_waitables_interface.hpp"
 #include "rclcpp/parameter.hpp"
 #include "rclcpp/publisher.hpp"
@@ -824,14 +823,6 @@ public:
   rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr
   get_node_time_source_interface();
 
-  /// Return the Node's internal NodeTypeDescriptionsInterface implementation.
-  /**
-   * \sa rclcpp::Node::get_node_type_descriptions_interface
-   */
-  RCLCPP_LIFECYCLE_PUBLIC
-  rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
-  get_node_type_descriptions_interface();
-
   /// Return the Node's internal NodeWaitablesInterface implementation.
   /**
    * \sa rclcpp::Node::get_node_waitables_interface

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>21.0.6</version>
+  <version>20.0.0</version>
   <description>Package containing a prototype for lifecycle implementation</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp_lifecycle/src/lifecycle_node.cpp

@@ -43,7 +43,6 @@
 #include "rclcpp/node_interfaces/node_time_source.hpp"
 #include "rclcpp/node_interfaces/node_timers.hpp"
 #include "rclcpp/node_interfaces/node_topics.hpp"
-#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
 #include "rclcpp/node_interfaces/node_waitables.hpp"
 #include "rclcpp/parameter_service.hpp"
 #include "rclcpp/qos.hpp"
@@ -77,7 +76,7 @@ LifecycleNode::LifecycleNode(
       options.use_intra_process_comms(),
       options.enable_topic_statistics())),
   node_graph_(new rclcpp::node_interfaces::NodeGraph(node_base_.get())),
-  node_logging_(new rclcpp::node_interfaces::NodeLogging(node_base_)),
+  node_logging_(new rclcpp::node_interfaces::NodeLogging(node_base_.get())),
   node_timers_(new rclcpp::node_interfaces::NodeTimers(node_base_.get())),
   node_topics_(new rclcpp::node_interfaces::NodeTopics(node_base_.get(), node_timers_.get())),
   node_services_(new rclcpp::node_interfaces::NodeServices(node_base_.get())),
@@ -116,11 +115,7 @@ LifecycleNode::LifecycleNode(
     )),
   node_waitables_(new rclcpp::node_interfaces::NodeWaitables(node_base_.get())),
   node_options_(options),
-  impl_(new LifecycleNodeInterfaceImpl(
-      node_base_,
-      node_logging_,
-      node_parameters_,
-      node_services_))
+  impl_(new LifecycleNodeInterfaceImpl(node_base_, node_services_))
 {
   impl_->init(enable_communication_interface);
 
@@ -146,22 +141,6 @@ LifecycleNode::LifecycleNode(
 
 LifecycleNode::~LifecycleNode()
 {
-  // shutdown if necessary to avoid leaving the device in unknown state
-  if (LifecycleNode::get_current_state().id() !=
-    lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED)
-  {
-    auto ret = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
-    auto finalized = LifecycleNode::shutdown(ret);
-    if (finalized.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED ||
-      ret != rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS)
-    {
-      RCLCPP_WARN(
-        rclcpp::get_logger("rclcpp_lifecycle"),
-        "Shutdown error in destruction of LifecycleNode: final state(%s)",
-        finalized.label().c_str());
-    }
-  }
-
   // release sub-interfaces in an order that allows them to consult with node_base during tear-down
   node_waitables_.reset();
   node_time_source_.reset();
@@ -495,12 +474,6 @@ LifecycleNode::get_node_services_interface()
   return node_services_;
 }
 
-rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
-LifecycleNode::get_node_type_descriptions_interface()
-{
-  return impl_->get_node_type_descriptions_interface();
-}
-
 rclcpp::node_interfaces::NodeParametersInterface::SharedPtr
 LifecycleNode::get_node_parameters_interface()
 {

rclcpp_lifecycle/src/lifecycle_node_interface_impl.cpp

@@ -29,9 +29,7 @@
 #include "lifecycle_msgs/srv/get_available_transitions.hpp"
 
 #include "rclcpp/node_interfaces/node_base_interface.hpp"
-#include "rclcpp/node_interfaces/node_logging_interface.hpp"
 #include "rclcpp/node_interfaces/node_services_interface.hpp"
-#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
 
 #include "rclcpp_lifecycle/node_interfaces/lifecycle_node_interface.hpp"
 
@@ -52,17 +50,9 @@ namespace rclcpp_lifecycle
 
 LifecycleNode::LifecycleNodeInterfaceImpl::LifecycleNodeInterfaceImpl(
   std::shared_ptr<rclcpp::node_interfaces::NodeBaseInterface> node_base_interface,
-  std::shared_ptr<rclcpp::node_interfaces::NodeLoggingInterface> node_logging_interface,
-  std::shared_ptr<rclcpp::node_interfaces::NodeParametersInterface> node_parameters_interface,
   std::shared_ptr<rclcpp::node_interfaces::NodeServicesInterface> node_services_interface)
 : node_base_interface_(node_base_interface),
-  node_services_interface_(node_services_interface),
-  node_logging_interface_(node_logging_interface),
-  node_type_descriptions_(new rclcpp::node_interfaces::NodeTypeDescriptions(
-      node_base_interface,
-      node_logging_interface,
-      node_parameters_interface,
-      node_services_interface))
+  node_services_interface_(node_services_interface)
 {
 }
 
@@ -75,8 +65,8 @@ LifecycleNode::LifecycleNodeInterfaceImpl::~LifecycleNodeInterfaceImpl()
     ret = rcl_lifecycle_state_machine_fini(&state_machine_, node_handle);
   }
   if (ret != RCL_RET_OK) {
-    RCLCPP_FATAL(
-      node_logging_interface_->get_logger(),
+    RCUTILS_LOG_FATAL_NAMED(
+      "rclcpp_lifecycle",
       "failed to destroy rcl_state_machine");
   }
 }
@@ -408,8 +398,7 @@ LifecycleNode::LifecycleNodeInterfaceImpl::change_state(
   {
     std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
     if (rcl_lifecycle_state_machine_is_initialized(&state_machine_) != RCL_RET_OK) {
-      RCLCPP_ERROR(
-        node_logging_interface_->get_logger(),
+      RCUTILS_LOG_ERROR(
         "Unable to change state for state machine for %s: %s",
         node_base_interface_->get_name(), rcl_get_error_string().str);
       return RCL_RET_ERROR;
@@ -422,8 +411,7 @@ LifecycleNode::LifecycleNodeInterfaceImpl::change_state(
       rcl_lifecycle_trigger_transition_by_id(
         &state_machine_, transition_id, publish_update) != RCL_RET_OK)
     {
-      RCLCPP_ERROR(
-        node_logging_interface_->get_logger(),
+      RCUTILS_LOG_ERROR(
         "Unable to start transition %u from current state %s: %s",
         transition_id, state_machine_.current_state->label, rcl_get_error_string().str);
       rcutils_reset_error();
@@ -455,8 +443,7 @@ LifecycleNode::LifecycleNodeInterfaceImpl::change_state(
       rcl_lifecycle_trigger_transition_by_label(
         &state_machine_, transition_label, publish_update) != RCL_RET_OK)
     {
-      RCLCPP_ERROR(
-        node_logging_interface_->get_logger(),
+      RCUTILS_LOG_ERROR(
         "Failed to finish transition %u. Current state is now: %s (%s)",
         transition_id, state_machine_.current_state->label, rcl_get_error_string().str);
       rcutils_reset_error();
@@ -471,9 +458,7 @@ LifecycleNode::LifecycleNodeInterfaceImpl::change_state(
   // error handling ?!
   // TODO(karsten1987): iterate over possible ret value
   if (cb_return_code == node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR) {
-    RCLCPP_WARN(
-      node_logging_interface_->get_logger(),
-      "Error occurred while doing error handling.");
+    RCUTILS_LOG_WARN("Error occurred while doing error handling.");
 
     auto error_cb_code = execute_callback(current_state_id, initial_state);
     auto error_cb_label = get_label_for_return_code(error_cb_code);
@@ -482,9 +467,7 @@ LifecycleNode::LifecycleNodeInterfaceImpl::change_state(
       rcl_lifecycle_trigger_transition_by_label(
         &state_machine_, error_cb_label, publish_update) != RCL_RET_OK)
     {
-      RCLCPP_ERROR(
-        node_logging_interface_->get_logger(),
-        "Failed to call cleanup on error state: %s", rcl_get_error_string().str);
+      RCUTILS_LOG_ERROR("Failed to call cleanup on error state: %s", rcl_get_error_string().str);
       rcutils_reset_error();
       return RCL_RET_ERROR;
     }
@@ -512,12 +495,8 @@ LifecycleNode::LifecycleNodeInterfaceImpl::execute_callback(
     try {
       cb_success = callback(State(previous_state));
     } catch (const std::exception & e) {
-      RCLCPP_ERROR(
-        node_logging_interface_->get_logger(),
-        "Caught exception in callback for transition %d", it->first);
-      RCLCPP_ERROR(
-        node_logging_interface_->get_logger(),
-        "Original error: %s", e.what());
+      RCUTILS_LOG_ERROR("Caught exception in callback for transition %d", it->first);
+      RCUTILS_LOG_ERROR("Original error: %s", e.what());
       cb_success = node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
     }
   }
@@ -602,10 +581,4 @@ LifecycleNode::LifecycleNodeInterfaceImpl::on_deactivate() const
   }
 }
 
-rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
-LifecycleNode::LifecycleNodeInterfaceImpl::get_node_type_descriptions_interface()
-{
-  return node_type_descriptions_;
-}
-
 }  // namespace rclcpp_lifecycle

rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp

@@ -32,7 +32,6 @@
 
 #include "rclcpp/macros.hpp"
 #include "rclcpp/node_interfaces/node_base_interface.hpp"
-#include "rclcpp/node_interfaces/node_logging_interface.hpp"
 #include "rclcpp/node_interfaces/node_services_interface.hpp"
 
 #include "rclcpp_lifecycle/node_interfaces/lifecycle_node_interface.hpp"
@@ -53,8 +52,6 @@ class LifecycleNode::LifecycleNodeInterfaceImpl final
 public:
   LifecycleNodeInterfaceImpl(
     std::shared_ptr<rclcpp::node_interfaces::NodeBaseInterface> node_base_interface,
-    std::shared_ptr<rclcpp::node_interfaces::NodeLoggingInterface> node_logging_interface,
-    std::shared_ptr<rclcpp::node_interfaces::NodeParametersInterface> node_parameters_interface,
     std::shared_ptr<rclcpp::node_interfaces::NodeServicesInterface> node_services_interface);
 
   ~LifecycleNodeInterfaceImpl();
@@ -105,9 +102,6 @@ public:
   void
   add_timer_handle(std::shared_ptr<rclcpp::TimerBase> timer);
 
-  rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
-  get_node_type_descriptions_interface();
-
 private:
   RCLCPP_DISABLE_COPY(LifecycleNodeInterfaceImpl)
 
@@ -158,7 +152,6 @@ private:
 
   using NodeBasePtr = std::shared_ptr<rclcpp::node_interfaces::NodeBaseInterface>;
   using NodeServicesPtr = std::shared_ptr<rclcpp::node_interfaces::NodeServicesInterface>;
-  using NodeLoggingPtr = std::shared_ptr<rclcpp::node_interfaces::NodeLoggingInterface>;
   using ChangeStateSrvPtr = std::shared_ptr<rclcpp::Service<ChangeStateSrv>>;
   using GetStateSrvPtr = std::shared_ptr<rclcpp::Service<GetStateSrv>>;
   using GetAvailableStatesSrvPtr =
@@ -170,7 +163,6 @@ private:
 
   NodeBasePtr node_base_interface_;
   NodeServicesPtr node_services_interface_;
-  NodeLoggingPtr node_logging_interface_;
   ChangeStateSrvPtr srv_change_state_;
   GetStateSrvPtr srv_get_state_;
   GetAvailableStatesSrvPtr srv_get_available_states_;
@@ -180,9 +172,6 @@ private:
   // to controllable things
   std::vector<std::weak_ptr<rclcpp_lifecycle::ManagedEntityInterface>> weak_managed_entities_;
   std::vector<std::weak_ptr<rclcpp::TimerBase>> weak_timers_;
-
-  // Backported members hidden in impl
-  rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr node_type_descriptions_;
 };
 
 }  // namespace rclcpp_lifecycle

rclcpp_lifecycle/test/test_lifecycle_node.cpp

@@ -414,146 +414,6 @@ TEST_F(TestDefaultStateMachine, bad_mood) {
   EXPECT_EQ(1u, test_node->number_of_callbacks);
 }
 
-
-TEST_F(TestDefaultStateMachine, shutdown_from_each_primary_state) {
-  auto success = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
-  auto reset_key = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
-
-  // PRIMARY_STATE_UNCONFIGURED to shutdown
-  {
-    auto ret = reset_key;
-    auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
-    auto finalized = test_node->shutdown(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
-  }
-
-  // PRIMARY_STATE_INACTIVE to shutdown
-  {
-    auto ret = reset_key;
-    auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
-    auto configured = test_node->configure(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
-    ret = reset_key;
-    auto finalized = test_node->shutdown(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
-  }
-
-  // PRIMARY_STATE_ACTIVE to shutdown
-  {
-    auto ret = reset_key;
-    auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
-    auto configured = test_node->configure(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
-    ret = reset_key;
-    auto activated = test_node->activate(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
-    ret = reset_key;
-    auto finalized = test_node->shutdown(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
-  }
-
-  // PRIMARY_STATE_FINALIZED to shutdown
-  {
-    auto ret = reset_key;
-    auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
-    auto configured = test_node->configure(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
-    ret = reset_key;
-    auto activated = test_node->activate(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
-    ret = reset_key;
-    auto finalized = test_node->shutdown(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
-    ret = reset_key;
-    auto finalized_again = test_node->shutdown(ret);
-    EXPECT_EQ(reset_key, ret);
-    EXPECT_EQ(finalized_again.id(), State::PRIMARY_STATE_FINALIZED);
-  }
-}
-
-TEST_F(TestDefaultStateMachine, test_shutdown_on_dtor) {
-  auto success = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
-  auto reset_key = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
-
-  bool shutdown_cb_called = false;
-  auto on_shutdown_callback =
-    [&shutdown_cb_called](const rclcpp_lifecycle::State &) ->
-    rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn {
-      shutdown_cb_called = true;
-      return rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
-    };
-
-  // PRIMARY_STATE_UNCONFIGURED to shutdown via dtor
-  shutdown_cb_called = false;
-  {
-    auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
-    test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
-    EXPECT_EQ(State::PRIMARY_STATE_UNCONFIGURED, test_node->get_current_state().id());
-    EXPECT_FALSE(shutdown_cb_called);
-  }
-  EXPECT_TRUE(shutdown_cb_called);
-
-  // PRIMARY_STATE_INACTIVE to shutdown via dtor
-  shutdown_cb_called = false;
-  {
-    auto ret = reset_key;
-    auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
-    test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
-    auto configured = test_node->configure(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
-    EXPECT_FALSE(shutdown_cb_called);
-  }
-  EXPECT_TRUE(shutdown_cb_called);
-
-  // PRIMARY_STATE_ACTIVE to shutdown via dtor
-  shutdown_cb_called = false;
-  {
-    auto ret = reset_key;
-    auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
-    test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
-    auto configured = test_node->configure(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
-    ret = reset_key;
-    auto activated = test_node->activate(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
-    EXPECT_FALSE(shutdown_cb_called);
-  }
-  EXPECT_TRUE(shutdown_cb_called);
-
-  // PRIMARY_STATE_FINALIZED to shutdown via dtor
-  shutdown_cb_called = false;
-  {
-    auto ret = reset_key;
-    auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
-    test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
-    auto configured = test_node->configure(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
-    ret = reset_key;
-    auto activated = test_node->activate(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
-    ret = reset_key;
-    auto finalized = test_node->shutdown(ret);
-    EXPECT_EQ(success, ret);
-    EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
-    EXPECT_TRUE(shutdown_cb_called);  // should be called already
-  }
-  EXPECT_TRUE(shutdown_cb_called);
-}
-
 TEST_F(TestDefaultStateMachine, lifecycle_subscriber) {
   auto test_node = std::make_shared<MoodyLifecycleNode<GoodMood>>("testnode");
 
@@ -567,15 +427,11 @@ TEST_F(TestDefaultStateMachine, lifecycle_subscriber) {
 // Parameters are tested more thoroughly in rclcpp's test_node.cpp
 // These are provided for coverage of lifecycle node's API
 TEST_F(TestDefaultStateMachine, declare_parameters) {
-  // "start_type_description_service" and "use_sim_time"
-  const uint64_t builtin_param_count = 2;
-  const uint64_t expected_param_count = 6 + builtin_param_count;
   auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
 
   auto list_result = test_node->list_parameters({}, 0u);
-  EXPECT_EQ(list_result.names.size(), builtin_param_count);
-  EXPECT_STREQ(list_result.names[0].c_str(), "start_type_description_service");
-  EXPECT_STREQ(list_result.names[1].c_str(), "use_sim_time");
+  EXPECT_EQ(list_result.names.size(), 1u);
+  EXPECT_STREQ(list_result.names[0].c_str(), "use_sim_time");
 
   const std::string bool_name = "test_boolean";
   const std::string int_name = "test_int";
@@ -613,11 +469,10 @@ TEST_F(TestDefaultStateMachine, declare_parameters) {
   test_node->declare_parameters("test_double", double_parameters);
 
   list_result = test_node->list_parameters({}, 0u);
-  EXPECT_EQ(list_result.names.size(), expected_param_count);
+  EXPECT_EQ(list_result.names.size(), 7u);
 
   // The order of these names is not controlled by lifecycle_node, doing set equality
   std::set<std::string> expected_names = {
-    "start_type_description_service",
     "test_boolean",
     "test_double.double_one",
     "test_double.double_two",
@@ -632,13 +487,11 @@ TEST_F(TestDefaultStateMachine, declare_parameters) {
 }
 
 TEST_F(TestDefaultStateMachine, check_parameters) {
-  const uint64_t builtin_param_count = 2;
   auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
 
   auto list_result = test_node->list_parameters({}, 0u);
-  EXPECT_EQ(list_result.names.size(), builtin_param_count);
-  EXPECT_STREQ(list_result.names[0].c_str(), "start_type_description_service");
-  EXPECT_STREQ(list_result.names[1].c_str(), "use_sim_time");
+  EXPECT_EQ(list_result.names.size(), 1u);
+  EXPECT_STREQ(list_result.names[0].c_str(), "use_sim_time");
 
   const std::string bool_name = "test_boolean";
   const std::string int_name = "test_int";
@@ -696,7 +549,8 @@ TEST_F(TestDefaultStateMachine, check_parameters) {
   std::map<std::string, rclcpp::ParameterValue> parameter_map;
   EXPECT_TRUE(test_node->get_parameters({}, parameter_map));
 
-  EXPECT_EQ(parameter_map.size(), parameter_names.size() + builtin_param_count);
+  // int param, bool param, and use_sim_time
+  EXPECT_EQ(parameter_map.size(), 3u);
 
   // Check parameter types
   auto parameter_types = test_node->get_parameter_types(parameter_names);
@@ -731,12 +585,10 @@ TEST_F(TestDefaultStateMachine, check_parameters) {
 
   // List parameters
   list_result = test_node->list_parameters({}, 0u);
-  EXPECT_EQ(list_result.names.size(), parameter_names.size() + builtin_param_count);
-  size_t index = 0;
-  EXPECT_STREQ(list_result.names[index++].c_str(), "start_type_description_service");
-  EXPECT_STREQ(list_result.names[index++].c_str(), parameter_names[0].c_str());
-  EXPECT_STREQ(list_result.names[index++].c_str(), parameter_names[1].c_str());
-  EXPECT_STREQ(list_result.names[index++].c_str(), "use_sim_time");
+  EXPECT_EQ(list_result.names.size(), 3u);
+  EXPECT_STREQ(list_result.names[0].c_str(), parameter_names[0].c_str());
+  EXPECT_STREQ(list_result.names[1].c_str(), parameter_names[1].c_str());
+  EXPECT_STREQ(list_result.names[2].c_str(), "use_sim_time");
 
   // Undeclare parameter
   test_node->undeclare_parameter(bool_name);
@@ -781,7 +633,6 @@ TEST_F(TestDefaultStateMachine, test_getters) {
   EXPECT_LT(0u, test_node->now().nanoseconds());
   EXPECT_STREQ("testnode", test_node->get_logger().get_name());
   EXPECT_NE(nullptr, const_cast<const EmptyLifecycleNode *>(test_node.get())->get_clock());
-  EXPECT_NE(nullptr, test_node->get_node_type_descriptions_interface());
 }
 
 TEST_F(TestDefaultStateMachine, test_graph_topics) {