6.3.0

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>

rclcpp/CHANGELOG.rst

@@ -2,6 +2,55 @@
 Changelog for package rclcpp
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+6.3.0 (2021-01-25)
+------------------
+* Add instrumentation for linking a timer to a node (`#1500 <https://github.com/ros2/rclcpp/issues/1500>`_)
+* Fix error when using IPC with StaticSingleThreadExecutor (`#1520 <https://github.com/ros2/rclcpp/issues/1520>`_)
+* Change to using unique_ptrs for DummyExecutor. (`#1517 <https://github.com/ros2/rclcpp/issues/1517>`_)
+* Allow reconfiguring 'clock' topic qos (`#1512 <https://github.com/ros2/rclcpp/issues/1512>`_)
+* Allow to add/remove nodes thread safely in rclcpp::Executor  (`#1505 <https://github.com/ros2/rclcpp/issues/1505>`_)
+* Call rclcpp::shutdown in test_node for clean shutdown on Windows (`#1515 <https://github.com/ros2/rclcpp/issues/1515>`_)
+* Reapply "Add get_logging_directory method to rclcpp::Logger (`#1509 <https://github.com/ros2/rclcpp/issues/1509>`_)" (`#1513 <https://github.com/ros2/rclcpp/issues/1513>`_)
+* use describe_parameters of parameter client for test (`#1499 <https://github.com/ros2/rclcpp/issues/1499>`_)
+* Revert "Add get_logging_directory method to rclcpp::Logger (`#1509 <https://github.com/ros2/rclcpp/issues/1509>`_)" (`#1511 <https://github.com/ros2/rclcpp/issues/1511>`_)
+* Add get_logging_directory method to rclcpp::Logger (`#1509 <https://github.com/ros2/rclcpp/issues/1509>`_)
+* Contributors: Chris Lalancette, Christophe Bedard, Ivan Santiago Paunovic, eboasson, mauropasse, tomoya
+
+6.2.0 (2021-01-08)
+------------------
+* Better documentation for the QoS class (`#1508 <https://github.com/ros2/rclcpp/issues/1508>`_)
+* Modify excluding callback duration from topic statistics (`#1492 <https://github.com/ros2/rclcpp/issues/1492>`_)
+* Make the test of graph users more robust. (`#1504 <https://github.com/ros2/rclcpp/issues/1504>`_)
+* Make sure to wait for graph change events in test_node_graph. (`#1503 <https://github.com/ros2/rclcpp/issues/1503>`_)
+* add timeout to SyncParametersClient methods (`#1493 <https://github.com/ros2/rclcpp/issues/1493>`_)
+* Fix wrong test expectations (`#1497 <https://github.com/ros2/rclcpp/issues/1497>`_)
+* Update create_publisher/subscription documentation, clarifying when a parameters interface is required (`#1494 <https://github.com/ros2/rclcpp/issues/1494>`_)
+* Fix string literal warnings (`#1442 <https://github.com/ros2/rclcpp/issues/1442>`_)
+* support describe_parameters methods to parameter client. (`#1453 <https://github.com/ros2/rclcpp/issues/1453>`_)
+* Contributors: Audrow Nash, Chris Lalancette, Ivan Santiago Paunovic, Nikolai Morin, hsgwa, tomoya
+
+6.1.0 (2020-12-10)
+------------------
+* Add getters to rclcpp::qos and rclcpp::Policy enum classes (`#1467 <https://github.com/ros2/rclcpp/issues/1467>`_)
+* Change nullptr checks to use ASSERT_TRUE. (`#1486 <https://github.com/ros2/rclcpp/issues/1486>`_)
+* Adjust logic around finding and erasing guard_condition (`#1474 <https://github.com/ros2/rclcpp/issues/1474>`_)
+* Update QDs to QL 1 (`#1477 <https://github.com/ros2/rclcpp/issues/1477>`_)
+* Add performance tests for parameter transport (`#1463 <https://github.com/ros2/rclcpp/issues/1463>`_)
+* Contributors: Chris Lalancette, Ivan Santiago Paunovic, Scott K Logan, Stephen Brawner
+
+6.0.0 (2020-11-18)
+------------------
+* Move ownership of shutdown_guard_condition to executors/graph_listener (`#1404 <https://github.com/ros2/rclcpp/issues/1404>`_)
+* Add options to automatically declare qos parameters when creating a publisher/subscription (`#1465 <https://github.com/ros2/rclcpp/issues/1465>`_)
+* Add `take_data` to `Waitable` and `data` to `AnyExecutable` (`#1241 <https://github.com/ros2/rclcpp/issues/1241>`_)
+* Add benchmarks for node parameters interface (`#1444 <https://github.com/ros2/rclcpp/issues/1444>`_)
+* Remove allocation from executor::remove_node() (`#1448 <https://github.com/ros2/rclcpp/issues/1448>`_)
+* Fix test crashes on CentOS 7 (`#1449 <https://github.com/ros2/rclcpp/issues/1449>`_)
+* Bump rclcpp packages to Quality Level 2 (`#1445 <https://github.com/ros2/rclcpp/issues/1445>`_)
+* Added executor benchmark tests (`#1413 <https://github.com/ros2/rclcpp/issues/1413>`_)
+* Add fully-qualified namespace to WeakCallbackGroupsToNodesMap (`#1435 <https://github.com/ros2/rclcpp/issues/1435>`_)
+* Contributors: Alejandro Hernández Cordero, Audrow Nash, Chris Lalancette, Ivan Santiago Paunovic, Louise Poubel, Scott K Logan, brawner
+
 5.1.0 (2020-11-02)
 ------------------
 * Deprecate Duration(rcl_duration_value_t) in favor of static Duration::from_nanoseconds(rcl_duration_value_t) (`#1432 <https://github.com/ros2/rclcpp/issues/1432>`_)

rclcpp/CMakeLists.txt

@@ -85,6 +85,7 @@ set(${PROJECT_NAME}_SRCS
   src/rclcpp/publisher_base.cpp
   src/rclcpp/qos.cpp
   src/rclcpp/qos_event.cpp
+  src/rclcpp/qos_overriding_options.cpp
   src/rclcpp/serialization.cpp
   src/rclcpp/serialized_message.cpp
   src/rclcpp/service.cpp

rclcpp/QUALITY_DECLARATION.md

@@ -2,7 +2,7 @@ This document is a declaration of software quality for the `rclcpp` package, bas
 
 # rclcpp Quality Declaration
 
-The package `rclcpp` claims to be in the **Quality Level 2** category.
+The package `rclcpp` claims to be in the **Quality Level 1** category when it is used with a **Quality Level 1** middleware.
 
 Below are the rationales, notes, and caveats for this claim, organized by each requirement listed in the [Package Quality Categories in REP-2004](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#package-quality-categories) of the ROS2 developer guide.
 
@@ -82,13 +82,13 @@ The API is publicly available in its [ROS 2 API documentation](http://docs.ros2.
 
 The license for `rclcpp` is Apache 2.0, and a summary is in each source file, the type is declared in the [`package.xml`](./package.xml) manifest file, and a full copy of the license is in the [`LICENSE`](../LICENSE) file.
 
-There is an automated test which runs a linter that ensures each file has a license statement. [Here](http://build.ros2.org/view/Fpr/job/Fpr__rclcpp__ubuntu_focal_amd64/lastSuccessfulBuild/testReport/rclcpp/) can be found a list with the latest results of the various linters being run on the package.
+There is an automated test which runs a linter that ensures each file has a license statement. [Here](http://build.ros2.org/view/Rpr/job/Rpr__rclcpp__ubuntu_focal_amd64/lastCompletedBuild/testReport/rclcpp/) can be found a list with the latest results of the various linters being run on the package.
 
 ### Copyright Statements [3.iv]
 
 The copyright holders each provide a statement of copyright in each source code file in `rclcpp`.
 
-There is an automated test which runs a linter that ensures each file has at least one copyright statement. Latest linter result report can be seen [here](http://build.ros2.org/view/Fpr/job/Fpr__rclcpp__ubuntu_focal_amd64/lastSuccessfulBuild/testReport/rclcpp/copyright/).
+There is an automated test which runs a linter that ensures each file has at least one copyright statement. Latest linter result report can be seen [here](http://build.ros2.org/view/Rpr/job/Rpr__rclcpp__ubuntu_focal_amd64/lastCompletedBuild/testReport/rclcpp/copyright/).
 
 ## Testing [4]
 
@@ -121,13 +121,21 @@ This includes:
 
 Changes are required to make a best effort to keep or increase coverage before being accepted, but decreases are allowed if properly justified and accepted by reviewers.
 
-Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastSuccessfulBuild/cobertura/src_ros2_rclcpp_rclcpp_src_rclcpp/). A description of how coverage statistics are calculated is summarized in this page ["ROS 2 Onboarding Guide"](https://index.ros.org/doc/ros2/Contributing/ROS-2-On-boarding-Guide/#note-on-coverage-runs).
+Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastCompletedBuild/cobertura/src_ros2_rclcpp_rclcpp_src_rclcpp/). A description of how coverage statistics are calculated is summarized in this page ["ROS 2 Onboarding Guide"](https://index.ros.org/doc/ros2/Contributing/ROS-2-On-boarding-Guide/#note-on-coverage-runs).
 
 `rclcpp` has a line coverage `>= 95%`, which is calculated over all directories within `rclcpp` with the exception of the `experimental` directory.
 
 ### Performance [4.iv]
 
-It is not yet defined if this package requires performance testing and how addresses this topic.
+`rclcpp` follows the recommendations for performance testing of C/C++ code in the [ROS 2 Developer Guide](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#performance), and opts to do performance analysis on each release rather than each change.
+
+The performance tests of `rclcpp` are located in the [test/benchmark directory](https://github.com/ros2/rclcpp/tree/master/rclcpp/test/benchmark).
+
+System level performance benchmarks that cover features of `rclcpp` can be found at:
+* [Benchmarks](http://build.ros2.org/view/Rci/job/Rci__benchmark_ubuntu_focal_amd64/BenchmarkTable/)
+* [Performance](http://build.ros2.org/view/Rci/job/Rci__nightly-performance_ubuntu_focal_amd64/lastCompletedBuild/)
+
+Changes that introduce regressions in performance must be adequately justified in order to be accepted and merged.
 
 ### Linters and Static Analysis [4.v]
 
@@ -155,43 +163,43 @@ It also has several test dependencies, which do not affect the resulting quality
 
 The `libstatistics_collector` package provides lightweight aggregation utilities to collect statistics and measure message metrics.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros-tooling/libstatistics_collector/tree/master/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros-tooling/libstatistics_collector/tree/master/QUALITY_DECLARATION.md).
 
 #### `rcl`
 
 `rcl` a library to support implementation of language specific ROS 2 Client Libraries.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcl/blob/master/rcl/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl/blob/master/rcl/QUALITY_DECLARATION.md).
 
 #### `rcl_yaml_param_parser`
 
 The `rcl_yaml_param_parser` package provides an API that is used to parse YAML configuration files which may be used to configure ROS and specific nodes.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcl/tree/master/rcl_yaml_param_parser/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl/tree/master/rcl_yaml_param_parser/QUALITY_DECLARATION.md).
 
 #### `rcpputils`
 
 The `rcpputils` package provides an API which contains common utilities and data structures useful when programming in C++.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcpputils/blob/master/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcpputils/blob/master/QUALITY_DECLARATION.md).
 
 #### `rcutils`
 
 The `rcutils` package provides an API which contains common utilities and data structures useful when programming in C.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcutils/blob/master/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcutils/blob/master/QUALITY_DECLARATION.md).
 
 #### `rmw`
 
 `rmw` is the ROS 2 middleware library.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rmw/blob/master/rmw/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rmw/blob/master/rmw/QUALITY_DECLARATION.md).
 
 #### `statistics_msgs`
 
 The `statistics_msgs` package contains ROS 2 message definitions for reporting statistics for topics and system resources.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcl_interfaces/blob/master/statistics_msgs/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl_interfaces/blob/master/statistics_msgs/QUALITY_DECLARATION.md).
 
 #### `tracetools`
 

rclcpp/README.md

@@ -6,4 +6,4 @@ Visit the [rclcpp API documentation](http://docs.ros2.org/latest/api/rclcpp/) fo
 
 ## Quality Declaration
 
-This package claims to be in the **Quality Level 2** category, see the [Quality Declaration](QUALITY_DECLARATION.md) for more details.
+This package claims to be in the **Quality Level 1** category, see the [Quality Declaration](QUALITY_DECLARATION.md) for more details.

rclcpp/include/rclcpp/any_executable.hpp

@@ -47,6 +47,7 @@ struct AnyExecutable
   // These are used to keep the scope on the containing items
   rclcpp::CallbackGroup::SharedPtr callback_group;
   rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base;
+  std::shared_ptr<void> data;
 };
 
 namespace executor

rclcpp/include/rclcpp/context.hpp

@@ -248,67 +248,6 @@ public:
   void
   interrupt_all_sleep_for();
 
-  /// Get a handle to the guard condition which is triggered when interrupted.
-  /**
-   * This guard condition is triggered any time interrupt_all_wait_sets() is
-   * called, which may be called by the user, or shutdown().
-   * And in turn, shutdown() may be called by the user, the destructor of this
-   * context, or the signal handler if installed and shutdown_on_sigint is true
-   * for this context.
-   *
-   * The first time that this function is called for a given wait set a new guard
-   * condition will be created and returned; thereafter the same guard condition
-   * will be returned for the same wait set.
-   * This mechanism is designed to ensure that the same guard condition is not
-   * reused across wait sets (e.g., when using multiple executors in the same
-   * process).
-   * This method will throw an exception if initialization of the guard
-   * condition fails.
-   *
-   * The returned guard condition needs to be released with the
-   * release_interrupt_guard_condition() method in order to reclaim resources.
-   *
-   * \param[in] wait_set Pointer to the rcl_wait_set_t that will be using the
-   *   resulting guard condition.
-   * \return Pointer to the guard condition.
-   * \throws anything rclcpp::exceptions::throw_from_rcl_error can throw.
-   */
-  RCLCPP_PUBLIC
-  rcl_guard_condition_t *
-  get_interrupt_guard_condition(rcl_wait_set_t * wait_set);
-
-  /// Release the previously allocated guard condition which is triggered when interrupted.
-  /**
-   * If you previously called get_interrupt_guard_condition() for a given wait
-   * set to get a interrupt guard condition, then you should call
-   * release_interrupt_guard_condition() when you're done, to free that
-   * condition.
-   * Will throw an exception if get_interrupt_guard_condition() wasn't
-   * previously called for the given wait set.
-   *
-   * After calling this, the pointer returned by get_interrupt_guard_condition()
-   * for the given wait_set is invalid.
-   *
-   * \param[in] wait_set Pointer to the rcl_wait_set_t that was using the
-   *   resulting guard condition.
-   * \throws anything rclcpp::exceptions::throw_from_rcl_error can throw.
-   * \throws std::runtime_error if a nonexistent wait set is trying to release sigint guard condition.
-   */
-  RCLCPP_PUBLIC
-  void
-  release_interrupt_guard_condition(rcl_wait_set_t * wait_set);
-
-  /// Nothrow version of release_interrupt_guard_condition(), logs to RCLCPP_ERROR instead.
-  RCLCPP_PUBLIC
-  void
-  release_interrupt_guard_condition(rcl_wait_set_t * wait_set, const std::nothrow_t &) noexcept;
-
-  /// Interrupt any blocking executors, or wait sets associated with this context.
-  RCLCPP_PUBLIC
-  virtual
-  void
-  interrupt_all_wait_sets();
-
   /// Return a singleton instance for the SubContext type, constructing one if necessary.
   template<typename SubContext, typename ... Args>
   std::shared_ptr<SubContext>
@@ -368,11 +307,6 @@ private:
   /// Mutex for protecting the global condition variable.
   std::mutex interrupt_mutex_;
 
-  /// Mutex to protect sigint_guard_cond_handles_.
-  std::mutex interrupt_guard_cond_handles_mutex_;
-  /// Guard conditions for interrupting of associated wait sets on interrupt_all_wait_sets().
-  std::unordered_map<rcl_wait_set_t *, rcl_guard_condition_t> interrupt_guard_cond_handles_;
-
   /// Keep shared ownership of global vector of weak contexts
   std::shared_ptr<WeakContextsWrapper> weak_contexts_;
 };

rclcpp/include/rclcpp/create_publisher.hpp

@@ -17,6 +17,7 @@
 
 #include <memory>
 #include <string>
+#include <utility>
 
 #include "rclcpp/node_interfaces/get_node_topics_interface.hpp"
 #include "rclcpp/node_interfaces/node_topics_interface.hpp"
@@ -24,15 +25,65 @@
 #include "rclcpp/publisher_factory.hpp"
 #include "rclcpp/publisher_options.hpp"
 #include "rclcpp/qos.hpp"
+#include "rclcpp/qos_overriding_options.hpp"
+#include "rclcpp/detail/qos_parameters.hpp"
+
 #include "rmw/qos_profiles.h"
 
 namespace rclcpp
 {
 
+namespace detail
+{
+/// Create and return a publisher of the given MessageT type.
+template<
+  typename MessageT,
+  typename AllocatorT = std::allocator<void>,
+  typename PublisherT = rclcpp::Publisher<MessageT, AllocatorT>,
+  typename NodeParametersT,
+  typename NodeTopicsT>
+std::shared_ptr<PublisherT>
+create_publisher(
+  NodeParametersT & node_parameters,
+  NodeTopicsT & node_topics,
+  const std::string & topic_name,
+  const rclcpp::QoS & qos,
+  const rclcpp::PublisherOptionsWithAllocator<AllocatorT> & options = (
+    rclcpp::PublisherOptionsWithAllocator<AllocatorT>()
+  )
+)
+{
+  auto node_topics_interface = rclcpp::node_interfaces::get_node_topics_interface(node_topics);
+  const rclcpp::QoS & actual_qos = options.qos_overriding_options.get_policy_kinds().size() ?
+    rclcpp::detail::declare_qos_parameters(
+    options.qos_overriding_options, node_parameters,
+    node_topics_interface->resolve_topic_name(topic_name),
+    qos, rclcpp::detail::PublisherQosParametersTraits{}) :
+    qos;
+
+  // Create the publisher.
+  auto pub = node_topics_interface->create_publisher(
+    topic_name,
+    rclcpp::create_publisher_factory<MessageT, AllocatorT, PublisherT>(options),
+    actual_qos
+  );
+
+  // Add the publisher to the node topics interface.
+  node_topics_interface->add_publisher(pub, options.callback_group);
+
+  return std::dynamic_pointer_cast<PublisherT>(pub);
+}
+}  // namespace detail
+
+
 /// Create and return a publisher of the given MessageT type.
 /**
  * The NodeT type only needs to have a method called get_node_topics_interface()
  * which returns a shared_ptr to a NodeTopicsInterface.
+ *
+ * In case `options.qos_overriding_options` is enabling qos parameter overrides,
+ * NodeT must also have a method called get_node_parameters_interface()
+ * which returns a shared_ptr to a NodeParametersInterface.
  */
 template<
   typename MessageT,
@@ -49,21 +100,28 @@ create_publisher(
   )
 )
 {
-  // Extract the NodeTopicsInterface from the NodeT.
-  using rclcpp::node_interfaces::get_node_topics_interface;
-  auto node_topics = get_node_topics_interface(node);
+  return detail::create_publisher<MessageT, AllocatorT, PublisherT>(
+    node, node, topic_name, qos, options);
+}
 
-  // Create the publisher.
-  auto pub = node_topics->create_publisher(
-    topic_name,
-    rclcpp::create_publisher_factory<MessageT, AllocatorT, PublisherT>(options),
-    qos
-  );
-
-  // Add the publisher to the node topics interface.
-  node_topics->add_publisher(pub, options.callback_group);
-
-  return std::dynamic_pointer_cast<PublisherT>(pub);
+/// Create and return a publisher of the given MessageT type.
+template<
+  typename MessageT,
+  typename AllocatorT = std::allocator<void>,
+  typename PublisherT = rclcpp::Publisher<MessageT, AllocatorT>>
+std::shared_ptr<PublisherT>
+create_publisher(
+  rclcpp::node_interfaces::NodeParametersInterface::SharedPtr & node_parameters,
+  rclcpp::node_interfaces::NodeTopicsInterface::SharedPtr & node_topics,
+  const std::string & topic_name,
+  const rclcpp::QoS & qos,
+  const rclcpp::PublisherOptionsWithAllocator<AllocatorT> & options = (
+    rclcpp::PublisherOptionsWithAllocator<AllocatorT>()
+  )
+)
+{
+  return detail::create_publisher<MessageT, AllocatorT, PublisherT>(
+    node_parameters, node_topics, topic_name, qos, options);
 }
 
 }  // namespace rclcpp

rclcpp/include/rclcpp/create_subscription.hpp

@@ -41,12 +41,115 @@
 namespace rclcpp
 {
 
+namespace detail
+{
+template<
+  typename MessageT,
+  typename CallbackT,
+  typename AllocatorT,
+  typename CallbackMessageT,
+  typename SubscriptionT,
+  typename MessageMemoryStrategyT,
+  typename NodeParametersT,
+  typename NodeTopicsT>
+typename std::shared_ptr<SubscriptionT>
+create_subscription(
+  NodeParametersT & node_parameters,
+  NodeTopicsT & node_topics,
+  const std::string & topic_name,
+  const rclcpp::QoS & qos,
+  CallbackT && callback,
+  const rclcpp::SubscriptionOptionsWithAllocator<AllocatorT> & options = (
+    rclcpp::SubscriptionOptionsWithAllocator<AllocatorT>()
+  ),
+  typename MessageMemoryStrategyT::SharedPtr msg_mem_strat = (
+    MessageMemoryStrategyT::create_default()
+  )
+)
+{
+  using rclcpp::node_interfaces::get_node_topics_interface;
+  auto node_topics_interface = get_node_topics_interface(node_topics);
+
+  std::shared_ptr<rclcpp::topic_statistics::SubscriptionTopicStatistics<CallbackMessageT>>
+  subscription_topic_stats = nullptr;
+
+  if (rclcpp::detail::resolve_enable_topic_statistics(
+      options,
+      *node_topics_interface->get_node_base_interface()))
+  {
+    if (options.topic_stats_options.publish_period <= std::chrono::milliseconds(0)) {
+      throw std::invalid_argument(
+              "topic_stats_options.publish_period must be greater than 0, specified value of " +
+              std::to_string(options.topic_stats_options.publish_period.count()) +
+              " ms");
+    }
+
+    std::shared_ptr<Publisher<statistics_msgs::msg::MetricsMessage>>
+    publisher = rclcpp::detail::create_publisher<statistics_msgs::msg::MetricsMessage>(
+      node_parameters,
+      node_topics_interface,
+      options.topic_stats_options.publish_topic,
+      qos);
+
+    subscription_topic_stats = std::make_shared<
+      rclcpp::topic_statistics::SubscriptionTopicStatistics<CallbackMessageT>
+      >(node_topics_interface->get_node_base_interface()->get_name(), publisher);
+
+    std::weak_ptr<
+      rclcpp::topic_statistics::SubscriptionTopicStatistics<CallbackMessageT>
+    > weak_subscription_topic_stats(subscription_topic_stats);
+    auto sub_call_back = [weak_subscription_topic_stats]() {
+        auto subscription_topic_stats = weak_subscription_topic_stats.lock();
+        if (subscription_topic_stats) {
+          subscription_topic_stats->publish_message();
+        }
+      };
+
+    auto node_timer_interface = node_topics_interface->get_node_timers_interface();
+
+    auto timer = create_wall_timer(
+      std::chrono::duration_cast<std::chrono::nanoseconds>(
+        options.topic_stats_options.publish_period),
+      sub_call_back,
+      options.callback_group,
+      node_topics_interface->get_node_base_interface(),
+      node_timer_interface
+    );
+
+    subscription_topic_stats->set_publisher_timer(timer);
+  }
+
+  auto factory = rclcpp::create_subscription_factory<MessageT>(
+    std::forward<CallbackT>(callback),
+    options,
+    msg_mem_strat,
+    subscription_topic_stats
+  );
+
+  const rclcpp::QoS & actual_qos = options.qos_overriding_options.get_policy_kinds().size() ?
+    rclcpp::detail::declare_qos_parameters(
+    options.qos_overriding_options, node_parameters,
+    node_topics_interface->resolve_topic_name(topic_name),
+    qos, rclcpp::detail::SubscriptionQosParametersTraits{}) :
+    qos;
+
+  auto sub = node_topics_interface->create_subscription(topic_name, factory, actual_qos);
+  node_topics_interface->add_subscription(sub, options.callback_group);
+
+  return std::dynamic_pointer_cast<SubscriptionT>(sub);
+}
+}  // namespace detail
+
 /// Create and return a subscription of the given MessageT type.
 /**
  * The NodeT type only needs to have a method called get_node_topics_interface()
  * which returns a shared_ptr to a NodeTopicsInterface, or be a
  * NodeTopicsInterface pointer itself.
  *
+ * In case `options.qos_overriding_options` is enabling qos parameter overrides,
+ * NodeT must also have a method called get_node_parameters_interface()
+ * which returns a shared_ptr to a NodeParametersInterface.
+ *
  * \tparam MessageT
  * \tparam CallbackT
  * \tparam AllocatorT
@@ -78,7 +181,7 @@ template<
   typename NodeT>
 typename std::shared_ptr<SubscriptionT>
 create_subscription(
-  NodeT && node,
+  NodeT & node,
   const std::string & topic_name,
   const rclcpp::QoS & qos,
   CallbackT && callback,
@@ -90,68 +193,45 @@ create_subscription(
   )
 )
 {
-  using rclcpp::node_interfaces::get_node_topics_interface;
-  auto node_topics = get_node_topics_interface(std::forward<NodeT>(node));
+  return rclcpp::detail::create_subscription<
+    MessageT, CallbackT, AllocatorT, CallbackMessageT, SubscriptionT, MessageMemoryStrategyT>(
+    node, node, topic_name, qos, std::forward<CallbackT>(callback), options, msg_mem_strat);
+}
 
-  std::shared_ptr<rclcpp::topic_statistics::SubscriptionTopicStatistics<CallbackMessageT>>
-  subscription_topic_stats = nullptr;
-
-  if (rclcpp::detail::resolve_enable_topic_statistics(
-      options,
-      *node_topics->get_node_base_interface()))
-  {
-    if (options.topic_stats_options.publish_period <= std::chrono::milliseconds(0)) {
-      throw std::invalid_argument(
-              "topic_stats_options.publish_period must be greater than 0, specified value of " +
-              std::to_string(options.topic_stats_options.publish_period.count()) +
-              " ms");
-    }
-
-    std::shared_ptr<Publisher<statistics_msgs::msg::MetricsMessage>> publisher =
-      create_publisher<statistics_msgs::msg::MetricsMessage>(
-      node,
-      options.topic_stats_options.publish_topic,
-      qos);
-
-    subscription_topic_stats = std::make_shared<
-      rclcpp::topic_statistics::SubscriptionTopicStatistics<CallbackMessageT>
-      >(node_topics->get_node_base_interface()->get_name(), publisher);
-
-    std::weak_ptr<
-      rclcpp::topic_statistics::SubscriptionTopicStatistics<CallbackMessageT>
-    > weak_subscription_topic_stats(subscription_topic_stats);
-    auto sub_call_back = [weak_subscription_topic_stats]() {
-        auto subscription_topic_stats = weak_subscription_topic_stats.lock();
-        if (subscription_topic_stats) {
-          subscription_topic_stats->publish_message();
-        }
-      };
-
-    auto node_timer_interface = node_topics->get_node_timers_interface();
-
-    auto timer = create_wall_timer(
-      std::chrono::duration_cast<std::chrono::nanoseconds>(
-        options.topic_stats_options.publish_period),
-      sub_call_back,
-      options.callback_group,
-      node_topics->get_node_base_interface(),
-      node_timer_interface
-    );
-
-    subscription_topic_stats->set_publisher_timer(timer);
-  }
-
-  auto factory = rclcpp::create_subscription_factory<MessageT>(
-    std::forward<CallbackT>(callback),
-    options,
-    msg_mem_strat,
-    subscription_topic_stats
-  );
-
-  auto sub = node_topics->create_subscription(topic_name, factory, qos);
-  node_topics->add_subscription(sub, options.callback_group);
-
-  return std::dynamic_pointer_cast<SubscriptionT>(sub);
+/// Create and return a subscription of the given MessageT type.
+/**
+ * See \ref create_subscription().
+ */
+template<
+  typename MessageT,
+  typename CallbackT,
+  typename AllocatorT = std::allocator<void>,
+  typename CallbackMessageT =
+  typename rclcpp::subscription_traits::has_message_type<CallbackT>::type,
+  typename SubscriptionT = rclcpp::Subscription<CallbackMessageT, AllocatorT>,
+  typename MessageMemoryStrategyT = rclcpp::message_memory_strategy::MessageMemoryStrategy<
+    CallbackMessageT,
+    AllocatorT
+  >>
+typename std::shared_ptr<SubscriptionT>
+create_subscription(
+  rclcpp::node_interfaces::NodeParametersInterface::SharedPtr & node_parameters,
+  rclcpp::node_interfaces::NodeTopicsInterface::SharedPtr & node_topics,
+  const std::string & topic_name,
+  const rclcpp::QoS & qos,
+  CallbackT && callback,
+  const rclcpp::SubscriptionOptionsWithAllocator<AllocatorT> & options = (
+    rclcpp::SubscriptionOptionsWithAllocator<AllocatorT>()
+  ),
+  typename MessageMemoryStrategyT::SharedPtr msg_mem_strat = (
+    MessageMemoryStrategyT::create_default()
+  )
+)
+{
+  return rclcpp::detail::create_subscription<
+    MessageT, CallbackT, AllocatorT, CallbackMessageT, SubscriptionT, MessageMemoryStrategyT>(
+    node_parameters, node_topics, topic_name, qos,
+    std::forward<CallbackT>(callback), options, msg_mem_strat);
 }
 
 }  // namespace rclcpp

rclcpp/include/rclcpp/detail/qos_parameters.hpp

@@ -0,0 +1,326 @@
+// 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__DETAIL__QOS_PARAMETERS_HPP_
+#define RCLCPP__DETAIL__QOS_PARAMETERS_HPP_
+
+#include <algorithm>
+#include <array>
+#include <functional>
+#include <initializer_list>
+#include <map>
+#include <string>
+#include <type_traits>
+#include <vector>
+
+#include "rcl_interfaces/msg/parameter_descriptor.hpp"
+#include "rcpputils/pointer_traits.hpp"
+#include "rmw/qos_string_conversions.h"
+
+#include "rclcpp/duration.hpp"
+#include "rclcpp/node_interfaces/get_node_parameters_interface.hpp"
+#include "rclcpp/node_interfaces/node_parameters_interface.hpp"
+#include "rclcpp/qos_overriding_options.hpp"
+
+namespace rclcpp
+{
+namespace detail
+{
+
+/// \internal Trait used to specialize `declare_qos_parameters()` for publishers.
+struct PublisherQosParametersTraits
+{
+  static constexpr const char * entity_type() {return "publisher";}
+  static constexpr auto allowed_policies()
+  {
+    return std::array<::rclcpp::QosPolicyKind, 9> {
+      QosPolicyKind::AvoidRosNamespaceConventions,
+      QosPolicyKind::Deadline,
+      QosPolicyKind::Durability,
+      QosPolicyKind::History,
+      QosPolicyKind::Depth,
+      QosPolicyKind::Lifespan,
+      QosPolicyKind::Liveliness,
+      QosPolicyKind::LivelinessLeaseDuration,
+      QosPolicyKind::Reliability,
+    };
+  }
+};
+
+/// \internal Trait used to specialize `declare_qos_parameters()` for subscriptions.
+struct SubscriptionQosParametersTraits
+{
+  static constexpr const char * entity_type() {return "subscription";}
+  static constexpr auto allowed_policies()
+  {
+    return std::array<::rclcpp::QosPolicyKind, 8> {
+      QosPolicyKind::AvoidRosNamespaceConventions,
+      QosPolicyKind::Deadline,
+      QosPolicyKind::Durability,
+      QosPolicyKind::History,
+      QosPolicyKind::Depth,
+      QosPolicyKind::Liveliness,
+      QosPolicyKind::LivelinessLeaseDuration,
+      QosPolicyKind::Reliability,
+    };
+  }
+};
+
+/// \internal Returns the given `policy` of the profile `qos` converted to a parameter value.
+inline
+::rclcpp::ParameterValue
+get_default_qos_param_value(rclcpp::QosPolicyKind policy, const rclcpp::QoS & qos);
+
+/// \internal Modify the given `policy` in `qos` to be `value`.
+inline
+void
+apply_qos_override(
+  rclcpp::QosPolicyKind policy, rclcpp::ParameterValue value, rclcpp::QoS & qos);
+
+inline
+rclcpp::ParameterValue
+declare_parameter_or_get(
+  rclcpp::node_interfaces::NodeParametersInterface & parameters_interface,
+  const std::string & param_name,
+  rclcpp::ParameterValue param_value,
+  rcl_interfaces::msg::ParameterDescriptor descriptor)
+{
+  try {
+    return parameters_interface.declare_parameter(
+      param_name, param_value, descriptor);
+  } catch (const rclcpp::exceptions::ParameterAlreadyDeclaredException &) {
+    return parameters_interface.get_parameter(param_name).get_parameter_value();
+  }
+}
+
+/// \internal Declare QoS parameters for the given entity.
+/**
+ * \tparam NodeT Node pointer or reference type.
+ * \tparam EntityQosParametersTraits A class with two static methods: `entity_type()` and
+ *  `allowed_policies()`. See `PublisherQosParametersTraits` and `SubscriptionQosParametersTraits`.
+ * \param options User provided options that indicate if QoS parameter overrides should be
+ *  declared or not, which policy can have overrides, and optionally a callback to validate the profile.
+ * \param node Parameters will be declared using this node.
+ * \param topic_name Name of the topic of the entity.
+ * \param default_qos User provided qos. It will be used as a default for the parameters declared.
+ * \return qos profile based on the user provided parameter overrides.
+ */
+template<typename NodeT, typename EntityQosParametersTraits>
+std::enable_if_t<
+  rclcpp::node_interfaces::has_node_parameters_interface<
+    decltype(std::declval<typename rcpputils::remove_pointer<NodeT>::type>())>::value ||
+  std::is_same<typename std::decay_t<NodeT>,
+  rclcpp::node_interfaces::NodeParametersInterface::SharedPtr>::value,
+  rclcpp::QoS>
+declare_qos_parameters(
+  const ::rclcpp::QosOverridingOptions & options,
+  NodeT & node,
+  const std::string & topic_name,
+  const ::rclcpp::QoS & default_qos,
+  EntityQosParametersTraits)
+{
+  auto & parameters_interface = *rclcpp::node_interfaces::get_node_parameters_interface(node);
+  std::string param_prefix;
+  const auto & id = options.get_id();
+  {
+    std::ostringstream oss{"qos_overrides.", std::ios::ate};
+    oss << topic_name << "." << EntityQosParametersTraits::entity_type();
+    if (!id.empty()) {
+      oss << "_" << id;
+    }
+    oss << ".";
+    param_prefix = oss.str();
+  }
+  std::string param_description_suffix;
+  {
+    std::ostringstream oss{"} for ", std::ios::ate};
+    oss << EntityQosParametersTraits::entity_type() << " {" << topic_name << "}";
+    if (!id.empty()) {
+      oss << " with id {" << id << "}";
+    }
+    param_description_suffix = oss.str();
+  }
+  rclcpp::QoS qos = default_qos;
+  for (auto policy : EntityQosParametersTraits::allowed_policies()) {
+    if (
+      std::count(options.get_policy_kinds().begin(), options.get_policy_kinds().end(), policy))
+    {
+      std::ostringstream param_name{param_prefix, std::ios::ate};
+      param_name << qos_policy_kind_to_cstr(policy);
+      std::ostringstream param_desciption{"qos policy {", std::ios::ate};
+      param_desciption << qos_policy_kind_to_cstr(policy) << param_description_suffix;
+      rcl_interfaces::msg::ParameterDescriptor descriptor{};
+      descriptor.description = param_desciption.str();
+      descriptor.read_only = true;
+      auto value = declare_parameter_or_get(
+        parameters_interface, param_name.str(),
+        get_default_qos_param_value(policy, qos), descriptor);
+      ::rclcpp::detail::apply_qos_override(policy, value, qos);
+    }
+  }
+  const auto & validation_callback = options.get_validation_callback();
+  if (validation_callback) {
+    auto result = validation_callback(qos);
+    if (!result.successful) {
+      throw rclcpp::exceptions::InvalidQosOverridesException{
+              "validation callback failed: " + result.reason};
+    }
+  }
+  return qos;
+}
+
+// TODO(ivanpauno): This overload cannot declare the QoS parameters, as a node parameters interface
+// was not provided.
+template<typename NodeT, typename EntityQosParametersTraits>
+std::enable_if_t<
+  !(rclcpp::node_interfaces::has_node_parameters_interface<
+    decltype(std::declval<typename rcpputils::remove_pointer<NodeT>::type>())>::value ||
+  std::is_same<typename std::decay_t<NodeT>,
+  rclcpp::node_interfaces::NodeParametersInterface::SharedPtr>::value),
+  rclcpp::QoS>
+declare_qos_parameters(
+  const ::rclcpp::QosOverridingOptions & options,
+  NodeT &,
+  const std::string &,
+  const ::rclcpp::QoS & default_qos,
+  EntityQosParametersTraits)
+{
+  if (options.get_policy_kinds().size()) {
+    std::runtime_error exc{
+      "passed non-default qos overriding options without providing a parameters interface"};
+    throw exc;
+  }
+  return default_qos;
+}
+
+/// \internal Helper function to get a rmw qos policy value from a string.
+#define RCLCPP_DETAIL_APPLY_QOS_OVERRIDE_FROM_PARAMETER_STRING( \
+    kind_lower, kind_upper, parameter_value, rclcpp_qos) \
+  do { \
+    auto policy_string = (parameter_value).get<std::string>(); \
+    auto policy_value = rmw_qos_ ## kind_lower ## _policy_from_str(policy_string.c_str()); \
+    if (RMW_QOS_POLICY_ ## kind_upper ## _UNKNOWN == policy_value) { \
+      throw std::invalid_argument{"unknown QoS policy " #kind_lower " value: " + policy_string}; \
+    } \
+    ((rclcpp_qos).kind_lower)(policy_value); \
+  } while (0)
+
+inline
+void
+apply_qos_override(
+  rclcpp::QosPolicyKind policy, rclcpp::ParameterValue value, rclcpp::QoS & qos)
+{
+  switch (policy) {
+    case QosPolicyKind::AvoidRosNamespaceConventions:
+      qos.avoid_ros_namespace_conventions(value.get<bool>());
+      break;
+    case QosPolicyKind::Deadline:
+      qos.deadline(::rclcpp::Duration::from_nanoseconds(value.get<int64_t>()));
+      break;
+    case QosPolicyKind::Durability:
+      RCLCPP_DETAIL_APPLY_QOS_OVERRIDE_FROM_PARAMETER_STRING(
+        durability, DURABILITY, value, qos);
+      break;
+    case QosPolicyKind::History:
+      RCLCPP_DETAIL_APPLY_QOS_OVERRIDE_FROM_PARAMETER_STRING(
+        history, HISTORY, value, qos);
+      break;
+    case QosPolicyKind::Depth:
+      qos.get_rmw_qos_profile().depth = static_cast<size_t>(value.get<int64_t>());
+      break;
+    case QosPolicyKind::Lifespan:
+      qos.lifespan(::rclcpp::Duration::from_nanoseconds(value.get<int64_t>()));
+      break;
+    case QosPolicyKind::Liveliness:
+      RCLCPP_DETAIL_APPLY_QOS_OVERRIDE_FROM_PARAMETER_STRING(
+        liveliness, LIVELINESS, value, qos);
+      break;
+    case QosPolicyKind::LivelinessLeaseDuration:
+      qos.liveliness_lease_duration(::rclcpp::Duration::from_nanoseconds(value.get<int64_t>()));
+      break;
+    case QosPolicyKind::Reliability:
+      RCLCPP_DETAIL_APPLY_QOS_OVERRIDE_FROM_PARAMETER_STRING(
+        reliability, RELIABILITY, value, qos);
+      break;
+    default:
+      throw std::invalid_argument{"unknown QosPolicyKind"};
+  }
+}
+
+/// Convert `rmw_time_t` to `int64_t` that can be used as a parameter value.
+inline
+int64_t
+rmw_duration_to_int64_t(rmw_time_t rmw_duration)
+{
+  return ::rclcpp::Duration(
+    static_cast<int32_t>(rmw_duration.sec),
+    static_cast<uint32_t>(rmw_duration.nsec)
+  ).nanoseconds();
+}
+
+/// \internal Throw an exception if `policy_value_stringified` is NULL.
+inline
+const char *
+check_if_stringified_policy_is_null(const char * policy_value_stringified, QosPolicyKind kind)
+{
+  if (!policy_value_stringified) {
+    std::ostringstream oss{"unknown value for policy kind {", std::ios::ate};
+    oss << kind << "}";
+    throw std::invalid_argument{oss.str()};
+  }
+  return policy_value_stringified;
+}
+
+inline
+::rclcpp::ParameterValue
+get_default_qos_param_value(rclcpp::QosPolicyKind kind, const rclcpp::QoS & qos)
+{
+  using ParameterValue = ::rclcpp::ParameterValue;
+  const auto & rmw_qos = qos.get_rmw_qos_profile();
+  switch (kind) {
+    case QosPolicyKind::AvoidRosNamespaceConventions:
+      return ParameterValue(rmw_qos.avoid_ros_namespace_conventions);
+    case QosPolicyKind::Deadline:
+      return ParameterValue(rmw_duration_to_int64_t(rmw_qos.deadline));
+    case QosPolicyKind::Durability:
+      return ParameterValue(
+        check_if_stringified_policy_is_null(
+          rmw_qos_durability_policy_to_str(rmw_qos.durability), kind));
+    case QosPolicyKind::History:
+      return ParameterValue(
+        check_if_stringified_policy_is_null(
+          rmw_qos_history_policy_to_str(rmw_qos.history), kind));
+    case QosPolicyKind::Depth:
+      return ParameterValue(static_cast<int64_t>(rmw_qos.depth));
+    case QosPolicyKind::Lifespan:
+      return ParameterValue(rmw_duration_to_int64_t(rmw_qos.lifespan));
+    case QosPolicyKind::Liveliness:
+      return ParameterValue(
+        check_if_stringified_policy_is_null(
+          rmw_qos_liveliness_policy_to_str(rmw_qos.liveliness), kind));
+    case QosPolicyKind::LivelinessLeaseDuration:
+      return ParameterValue(rmw_duration_to_int64_t(rmw_qos.liveliness_lease_duration));
+    case QosPolicyKind::Reliability:
+      return ParameterValue(
+        check_if_stringified_policy_is_null(
+          rmw_qos_reliability_policy_to_str(rmw_qos.reliability), kind));
+    default:
+      throw std::invalid_argument{"unknown QoS policy kind"};
+  }
+}
+
+}  // namespace detail
+}  // namespace rclcpp
+
+#endif  // RCLCPP__DETAIL__QOS_PARAMETERS_HPP_

rclcpp/include/rclcpp/duration.hpp

@@ -137,6 +137,9 @@ public:
   static Duration
   from_nanoseconds(rcl_duration_value_t nanoseconds);
 
+  static Duration
+  from_rmw_time(rmw_time_t duration);
+
   /// Convert Duration into a std::chrono::Duration.
   template<class DurationT>
   DurationT

rclcpp/include/rclcpp/exceptions/exceptions.hpp

@@ -282,6 +282,13 @@ class ParameterModifiedInCallbackException : public std::runtime_error
   using std::runtime_error::runtime_error;
 };
 
+/// Thrown if the QoS overrides provided aren't valid.
+class InvalidQosOverridesException : public std::runtime_error
+{
+  // Inherit constructors from runtime_error.
+  using std::runtime_error::runtime_error;
+};
+
 }  // namespace exceptions
 }  // namespace rclcpp
 

rclcpp/include/rclcpp/executor.hpp

@@ -31,6 +31,7 @@
 #include "rcl/wait.h"
 
 #include "rclcpp/contexts/default_context.hpp"
+#include "rclcpp/guard_condition.hpp"
 #include "rclcpp/executor_options.hpp"
 #include "rclcpp/future_return_code.hpp"
 #include "rclcpp/memory_strategies.hpp"
@@ -447,7 +448,7 @@ protected:
   RCLCPP_PUBLIC
   rclcpp::node_interfaces::NodeBaseInterface::SharedPtr
   get_node_by_group(
-    WeakCallbackGroupsToNodesMap weak_groups_to_nodes,
+    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
     rclcpp::CallbackGroup::SharedPtr group);
 
   /// Return true if the node has been added to this executor.
@@ -459,7 +460,7 @@ protected:
   bool
   has_node(
     const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-    WeakCallbackGroupsToNodesMap weak_groups_to_nodes) const;
+    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes) const;
 
   RCLCPP_PUBLIC
   rclcpp::CallbackGroup::SharedPtr
@@ -475,7 +476,7 @@ protected:
     rclcpp::CallbackGroup::SharedPtr group_ptr,
     rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
     WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
-    bool notify = true);
+    bool notify = true) RCPPUTILS_TSA_REQUIRES(mutex_);
 
   /// Remove a callback group from the executor.
   /**
@@ -486,7 +487,7 @@ protected:
   remove_callback_group_from_map(
     rclcpp::CallbackGroup::SharedPtr group_ptr,
     WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
-    bool notify = true);
+    bool notify = true) RCPPUTILS_TSA_REQUIRES(mutex_);
 
   RCLCPP_PUBLIC
   bool
@@ -496,7 +497,7 @@ protected:
   bool
   get_next_ready_executable_from_map(
     AnyExecutable & any_executable,
-    WeakCallbackGroupsToNodesMap weak_groups_to_nodes);
+    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes);
 
   RCLCPP_PUBLIC
   bool
@@ -517,7 +518,7 @@ protected:
    */
   RCLCPP_PUBLIC
   virtual void
-  add_callback_groups_from_nodes_associated_to_executor();
+  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;
@@ -525,14 +526,17 @@ protected:
   /// Guard condition for signaling the rmw layer to wake up for special events.
   rcl_guard_condition_t interrupt_guard_condition_ = rcl_get_zero_initialized_guard_condition();
 
+  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_.
-  std::mutex memory_strategy_mutex_;
+  mutable std::mutex mutex_;
 
   /// The memory strategy: an interface for handling user-defined memory allocation strategies.
-  memory_strategy::MemoryStrategy::SharedPtr memory_strategy_;
+  memory_strategy::MemoryStrategy::SharedPtr
+  memory_strategy_ RCPPUTILS_TSA_PT_GUARDED_BY(mutex_);
 
   /// The context associated with this executor.
   std::shared_ptr<rclcpp::Context> context_;
@@ -549,19 +553,24 @@ protected:
     WeakNodesToGuardConditionsMap;
 
   /// maps nodes to guard conditions
-  WeakNodesToGuardConditionsMap weak_nodes_to_guard_conditions_;
+  WeakNodesToGuardConditionsMap
+  weak_nodes_to_guard_conditions_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
   /// maps callback groups associated to nodes
-  WeakCallbackGroupsToNodesMap weak_groups_associated_with_executor_to_nodes_;
+  WeakCallbackGroupsToNodesMap
+  weak_groups_associated_with_executor_to_nodes_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
   /// maps callback groups to nodes associated with executor
-  WeakCallbackGroupsToNodesMap weak_groups_to_nodes_associated_with_executor_;
+  WeakCallbackGroupsToNodesMap
+  weak_groups_to_nodes_associated_with_executor_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
   /// maps all callback groups to nodes
-  WeakCallbackGroupsToNodesMap weak_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_;
+  std::list<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr>
+  weak_nodes_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 };
 
 namespace executor

rclcpp/include/rclcpp/executors/static_executor_entities_collector.hpp

@@ -72,9 +72,19 @@ public:
   void
   fini();
 
+  /// Execute the waitable.
   RCLCPP_PUBLIC
   void
-  execute() override;
+  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
   /**

rclcpp/include/rclcpp/experimental/subscription_intra_process.hpp

@@ -18,7 +18,9 @@
 #include <rmw/rmw.h>
 
 #include <functional>
+#include <map>
 #include <memory>
+#include <stdexcept>
 #include <string>
 #include <utility>
 
@@ -115,13 +117,31 @@ public:
   bool
   is_ready(rcl_wait_set_t * wait_set)
   {
-    (void)wait_set;
+    (void) wait_set;
     return buffer_->has_data();
   }
 
-  void execute()
+  std::shared_ptr<void>
+  take_data()
   {
-    execute_impl<CallbackMessageT>();
+    ConstMessageSharedPtr shared_msg;
+    MessageUniquePtr unique_msg;
+
+    if (any_callback_.use_take_shared_method()) {
+      shared_msg = buffer_->consume_shared();
+    } else {
+      unique_msg = buffer_->consume_unique();
+    }
+    return std::static_pointer_cast<void>(
+      std::make_shared<std::pair<ConstMessageSharedPtr, MessageUniquePtr>>(
+        std::pair<ConstMessageSharedPtr, MessageUniquePtr>(
+          shared_msg, std::move(unique_msg)))
+    );
+  }
+
+  void execute(std::shared_ptr<void> & data)
+  {
+    execute_impl<CallbackMessageT>(data);
   }
 
   void
@@ -154,26 +174,35 @@ private:
 
   template<typename T>
   typename std::enable_if<std::is_same<T, rcl_serialized_message_t>::value, void>::type
-  execute_impl()
+  execute_impl(std::shared_ptr<void> & data)
   {
+    (void)data;
     throw std::runtime_error("Subscription intra-process can't handle serialized messages");
   }
 
   template<class T>
   typename std::enable_if<!std::is_same<T, rcl_serialized_message_t>::value, void>::type
-  execute_impl()
+  execute_impl(std::shared_ptr<void> & data)
   {
+    if (!data) {
+      throw std::runtime_error("'data' is empty");
+    }
+
     rmw_message_info_t msg_info;
     msg_info.publisher_gid = {0, {0}};
     msg_info.from_intra_process = true;
 
+    auto shared_ptr = std::static_pointer_cast<std::pair<ConstMessageSharedPtr, MessageUniquePtr>>(
+      data);
+
     if (any_callback_.use_take_shared_method()) {
-      ConstMessageSharedPtr msg = buffer_->consume_shared();
-      any_callback_.dispatch_intra_process(msg, msg_info);
+      ConstMessageSharedPtr shared_msg = shared_ptr->first;
+      any_callback_.dispatch_intra_process(shared_msg, msg_info);
     } else {
-      MessageUniquePtr msg = buffer_->consume_unique();
-      any_callback_.dispatch_intra_process(std::move(msg), msg_info);
+      MessageUniquePtr unique_msg = std::move(shared_ptr->second);
+      any_callback_.dispatch_intra_process(std::move(unique_msg), msg_info);
     }
+    shared_ptr.reset();
   }
 
   AnySubscriptionCallback<CallbackMessageT, Alloc> any_callback_;

rclcpp/include/rclcpp/experimental/subscription_intra_process_base.hpp

@@ -56,8 +56,12 @@ public:
   virtual bool
   is_ready(rcl_wait_set_t * wait_set) = 0;
 
+  virtual
+  std::shared_ptr<void>
+  take_data() = 0;
+
   virtual void
-  execute() = 0;
+  execute(std::shared_ptr<void> & data) = 0;
 
   virtual bool
   use_take_shared_method() const = 0;

rclcpp/include/rclcpp/graph_listener.hpp

@@ -63,7 +63,7 @@ class GraphListener : public std::enable_shared_from_this<GraphListener>
 {
 public:
   RCLCPP_PUBLIC
-  explicit GraphListener(std::shared_ptr<rclcpp::Context> parent_context);
+  explicit GraphListener(const rclcpp::Context::SharedPtr & parent_context);
 
   RCLCPP_PUBLIC
   virtual ~GraphListener();
@@ -161,19 +161,22 @@ protected:
   run_loop();
 
   RCLCPP_PUBLIC
-  void init_wait_set();
+  void
+  init_wait_set();
 
   RCLCPP_PUBLIC
-  void cleanup_wait_set();
+  void
+  cleanup_wait_set();
 
 private:
   RCLCPP_DISABLE_COPY(GraphListener)
 
   /** \internal */
   void
-  __shutdown(bool);
+  __shutdown();
 
-  rclcpp::Context::WeakPtr parent_context_;
+  std::weak_ptr<rclcpp::Context> weak_parent_context_;
+  std::shared_ptr<rcl_context_t> rcl_parent_context_;
 
   std::thread listener_thread_;
   bool is_started_;
@@ -185,7 +188,6 @@ private:
   std::vector<rclcpp::node_interfaces::NodeGraphInterface *> node_graph_interfaces_;
 
   rcl_guard_condition_t interrupt_guard_condition_ = rcl_get_zero_initialized_guard_condition();
-  rcl_guard_condition_t * shutdown_guard_condition_;
   rcl_wait_set_t wait_set_ = rcl_get_zero_initialized_wait_set();
 };
 

rclcpp/include/rclcpp/logger.hpp

@@ -22,6 +22,7 @@
 
 #include "rcl/node.h"
 #include "rcutils/logging.h"
+#include "rcpputils/filesystem_helper.hpp"
 
 /**
  * \def RCLCPP_LOGGING_ENABLED
@@ -75,6 +76,18 @@ RCLCPP_PUBLIC
 Logger
 get_node_logger(const rcl_node_t * node);
 
+/// Get the current logging directory.
+/**
+ * For more details of how the logging directory is determined,
+ * see \ref rcl_logging_get_logging_directory.
+ *
+ * \returns the logging directory being used.
+ * \throws rclcpp::exceptions::RCLError if an unexpected error occurs.
+ */
+RCLCPP_PUBLIC
+rcpputils::fs::path
+get_logging_directory();
+
 class Logger
 {
 public:

rclcpp/include/rclcpp/parameter_client.hpp

@@ -15,6 +15,8 @@
 #ifndef RCLCPP__PARAMETER_CLIENT_HPP_
 #define RCLCPP__PARAMETER_CLIENT_HPP_
 
+#include <functional>
+#include <future>
 #include <memory>
 #include <string>
 #include <utility>
@@ -120,6 +122,14 @@ public:
       void(std::shared_future<std::vector<rclcpp::Parameter>>)
     > callback = nullptr);
 
+  RCLCPP_PUBLIC
+  std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>>
+  describe_parameters(
+    const std::vector<std::string> & names,
+    std::function<
+      void(std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>>)
+    > callback = nullptr);
+
   RCLCPP_PUBLIC
   std::shared_future<std::vector<rclcpp::ParameterType>>
   get_parameter_types(
@@ -332,9 +342,17 @@ public:
       qos_profile);
   }
 
-  RCLCPP_PUBLIC
+  template<typename RepT = int64_t, typename RatioT = std::milli>
   std::vector<rclcpp::Parameter>
-  get_parameters(const std::vector<std::string> & parameter_names);
+  get_parameters(
+    const std::vector<std::string> & parameter_names,
+    std::chrono::duration<RepT, RatioT> timeout = std::chrono::duration<RepT, RatioT>(-1))
+  {
+    return get_parameters(
+      parameter_names,
+      std::chrono::duration_cast<std::chrono::nanoseconds>(timeout)
+    );
+  }
 
   RCLCPP_PUBLIC
   bool
@@ -378,23 +396,67 @@ public:
     );
   }
 
-  RCLCPP_PUBLIC
+  template<typename RepT = int64_t, typename RatioT = std::milli>
+  std::vector<rcl_interfaces::msg::ParameterDescriptor>
+  describe_parameters(
+    const std::vector<std::string> & parameter_names,
+    std::chrono::duration<RepT, RatioT> timeout = std::chrono::duration<RepT, RatioT>(-1))
+  {
+    return describe_parameters(
+      parameter_names,
+      std::chrono::duration_cast<std::chrono::nanoseconds>(timeout)
+    );
+  }
+
+  template<typename RepT = int64_t, typename RatioT = std::milli>
   std::vector<rclcpp::ParameterType>
-  get_parameter_types(const std::vector<std::string> & parameter_names);
+  get_parameter_types(
+    const std::vector<std::string> & parameter_names,
+    std::chrono::duration<RepT, RatioT> timeout = std::chrono::duration<RepT, RatioT>(-1))
+  {
+    return get_parameter_types(
+      parameter_names,
+      std::chrono::duration_cast<std::chrono::nanoseconds>(timeout)
+    );
+  }
 
-  RCLCPP_PUBLIC
+  template<typename RepT = int64_t, typename RatioT = std::milli>
   std::vector<rcl_interfaces::msg::SetParametersResult>
-  set_parameters(const std::vector<rclcpp::Parameter> & parameters);
+  set_parameters(
+    const std::vector<rclcpp::Parameter> & parameters,
+    std::chrono::duration<RepT, RatioT> timeout = std::chrono::duration<RepT, RatioT>(-1))
+  {
+    return set_parameters(
+      parameters,
+      std::chrono::duration_cast<std::chrono::nanoseconds>(timeout)
+    );
+  }
 
-  RCLCPP_PUBLIC
+  template<typename RepT = int64_t, typename RatioT = std::milli>
   rcl_interfaces::msg::SetParametersResult
-  set_parameters_atomically(const std::vector<rclcpp::Parameter> & parameters);
+  set_parameters_atomically(
+    const std::vector<rclcpp::Parameter> & parameters,
+    std::chrono::duration<RepT, RatioT> timeout = std::chrono::duration<RepT, RatioT>(-1))
+  {
+    return set_parameters_atomically(
+      parameters,
+      std::chrono::duration_cast<std::chrono::nanoseconds>(timeout)
+    );
+  }
 
-  RCLCPP_PUBLIC
+  template<typename RepT = int64_t, typename RatioT = std::milli>
   rcl_interfaces::msg::ListParametersResult
   list_parameters(
     const std::vector<std::string> & parameter_prefixes,
-    uint64_t depth);
+    uint64_t depth,
+    std::chrono::duration<RepT, RatioT> timeout = std::chrono::duration<RepT, RatioT>(-1))
+  {
+    return list_parameters(
+      parameter_prefixes,
+      depth,
+      std::chrono::duration_cast<std::chrono::nanoseconds>(timeout)
+    );
+  }
 
   template<typename CallbackT>
   typename rclcpp::Subscription<rcl_interfaces::msg::ParameterEvent>::SharedPtr
@@ -437,6 +499,44 @@ public:
     return async_parameters_client_->wait_for_service(timeout);
   }
 
+protected:
+  RCLCPP_PUBLIC
+  std::vector<rclcpp::Parameter>
+  get_parameters(
+    const std::vector<std::string> & parameter_names,
+    std::chrono::nanoseconds timeout);
+
+  RCLCPP_PUBLIC
+  std::vector<rcl_interfaces::msg::ParameterDescriptor>
+  describe_parameters(
+    const std::vector<std::string> & parameter_names,
+    std::chrono::nanoseconds timeout);
+
+  RCLCPP_PUBLIC
+  std::vector<rclcpp::ParameterType>
+  get_parameter_types(
+    const std::vector<std::string> & parameter_names,
+    std::chrono::nanoseconds timeout);
+
+  RCLCPP_PUBLIC
+  std::vector<rcl_interfaces::msg::SetParametersResult>
+  set_parameters(
+    const std::vector<rclcpp::Parameter> & parameters,
+    std::chrono::nanoseconds timeout);
+
+  RCLCPP_PUBLIC
+  rcl_interfaces::msg::SetParametersResult
+  set_parameters_atomically(
+    const std::vector<rclcpp::Parameter> & parameters,
+    std::chrono::nanoseconds timeout);
+
+  RCLCPP_PUBLIC
+  rcl_interfaces::msg::ListParametersResult
+  list_parameters(
+    const std::vector<std::string> & parameter_prefixes,
+    uint64_t depth,
+    std::chrono::nanoseconds timeout);
+
 private:
   rclcpp::Executor::SharedPtr executor_;
   const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base_interface_;

rclcpp/include/rclcpp/publisher_options.hpp

@@ -26,6 +26,7 @@
 #include "rclcpp/intra_process_setting.hpp"
 #include "rclcpp/qos.hpp"
 #include "rclcpp/qos_event.hpp"
+#include "rclcpp/qos_overriding_options.hpp"
 
 namespace rclcpp
 {
@@ -50,6 +51,8 @@ struct PublisherOptionsBase
   /// Optional RMW implementation specific payload to be used during creation of the publisher.
   std::shared_ptr<rclcpp::detail::RMWImplementationSpecificPublisherPayload>
   rmw_implementation_payload = nullptr;
+
+  QosOverridingOptions qos_overriding_options;
 };
 
 /// Structure containing optional configuration for Publishers.

rclcpp/include/rclcpp/qos.hpp

@@ -30,6 +30,38 @@ namespace rclcpp
 RCLCPP_PUBLIC
 std::string qos_policy_name_from_kind(rmw_qos_policy_kind_t policy_kind);
 
+enum class HistoryPolicy
+{
+  KeepLast = RMW_QOS_POLICY_HISTORY_KEEP_LAST,
+  KeepAll = RMW_QOS_POLICY_HISTORY_KEEP_ALL,
+  SystemDefault = RMW_QOS_POLICY_HISTORY_SYSTEM_DEFAULT,
+  Unknown = RMW_QOS_POLICY_HISTORY_UNKNOWN,
+};
+
+enum class ReliabilityPolicy
+{
+  BestEffort = RMW_QOS_POLICY_RELIABILITY_BEST_EFFORT,
+  Reliable = RMW_QOS_POLICY_RELIABILITY_RELIABLE,
+  SystemDefault = RMW_QOS_POLICY_RELIABILITY_SYSTEM_DEFAULT,
+  Unknown = RMW_QOS_POLICY_RELIABILITY_UNKNOWN,
+};
+
+enum class DurabilityPolicy
+{
+  Volatile = RMW_QOS_POLICY_DURABILITY_VOLATILE,
+  TransientLocal = RMW_QOS_POLICY_DURABILITY_TRANSIENT_LOCAL,
+  SystemDefault = RMW_QOS_POLICY_DURABILITY_SYSTEM_DEFAULT,
+  Unknown = RMW_QOS_POLICY_DURABILITY_UNKNOWN,
+};
+
+enum class LivelinessPolicy
+{
+  Automatic = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC,
+  ManualByTopic = RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC,
+  SystemDefault = RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT,
+  Unknown = RMW_QOS_POLICY_LIVELINESS_UNKNOWN,
+};
+
 /// QoS initialization values, cannot be created directly, use KeepAll or KeepLast instead.
 struct RCLCPP_PUBLIC QoSInitialization
 {
@@ -58,10 +90,32 @@ struct RCLCPP_PUBLIC KeepLast : public rclcpp::QoSInitialization
 };
 
 /// Encapsulation of Quality of Service settings.
+/**
+ * Quality of Service settings control the behavior of publishers, subscriptions,
+ * and other entities, and includes things like how data is sent or resent,
+ * how data is buffered on the publishing and subscribing side, and other things.
+ * See:
+ *   <a href="https://index.ros.org/doc/ros2/Concepts/About-Quality-of-Service-Settings">
+ *     https://index.ros.org/doc/ros2/Concepts/About-Quality-of-Service-Settings
+ *   </a>
+ */
 class RCLCPP_PUBLIC QoS
 {
 public:
-  /// Constructor which allows you to construct a QoS by giving the only required settings.
+  /// Create a QoS by specifying only the history policy and history depth.
+  /**
+   * When using the default initial profile, the defaults will include:
+   *
+   *   - \link rclcpp::ReliabilityPolicy::Reliable ReliabilityPolicy::Reliable\endlink
+   *   - \link rclcpp::DurabilityPolicy::Volatile DurabilityPolicy::Volatile\endlink
+   *
+   * See rmw_qos_profile_default for a full list of default settings.
+   * If some other rmw_qos_profile_t is passed to initial_profile, then the defaults will derive from
+   * that profile instead.
+   *
+   * \param[in] qos_initialization Specifies history policy and history depth.
+   * \param[in] initial_profile The rmw_qos_profile_t instance on which to base the default settings.
+   */
   explicit
   QoS(
     const QoSInitialization & qos_initialization,
@@ -69,7 +123,11 @@ public:
 
   /// Conversion constructor to ease construction in the common case of just specifying depth.
   /**
-   * Convenience constructor, equivalent to QoS(KeepLast(history_depth)).
+   * This is a convenience constructor that calls QoS(KeepLast(history_depth)).
+   *
+   * \param[in] history_depth How many messages can be queued when publishing
+   *   with a Publisher, or how many messages can be queued before being replaced
+   *   by a Subscription.
    */
   // cppcheck-suppress noExplicitConstructor
   QoS(size_t history_depth);  // NOLINT(runtime/explicit): conversion constructor
@@ -82,6 +140,10 @@ public:
   const rmw_qos_profile_t &
   get_rmw_qos_profile() const;
 
+  /// Set the history policy.
+  QoS &
+  history(HistoryPolicy history);
+
   /// Set the history policy.
   QoS &
   history(rmw_qos_history_policy_t history);
@@ -98,6 +160,10 @@ public:
   QoS &
   reliability(rmw_qos_reliability_policy_t reliability);
 
+  /// Set the reliability setting.
+  QoS &
+  reliability(ReliabilityPolicy reliability);
+
   /// Set the reliability setting to reliable.
   QoS &
   reliable();
@@ -110,6 +176,10 @@ public:
   QoS &
   durability(rmw_qos_durability_policy_t durability);
 
+  /// Set the durability setting.
+  QoS &
+  durability(DurabilityPolicy durability);
+
   /// Set the durability setting to volatile.
   /**
     * Note that this cannot be named `volatile` because it is a C++ keyword.
@@ -141,6 +211,10 @@ public:
   QoS &
   liveliness(rmw_qos_liveliness_policy_t liveliness);
 
+  /// Set the liveliness setting.
+  QoS &
+  liveliness(LivelinessPolicy liveliness);
+
   /// Set the liveliness_lease_duration setting.
   QoS &
   liveliness_lease_duration(rmw_time_t liveliness_lease_duration);
@@ -153,6 +227,42 @@ public:
   QoS &
   avoid_ros_namespace_conventions(bool avoid_ros_namespace_conventions);
 
+  /// Get the history qos policy.
+  HistoryPolicy
+  history() const;
+
+  /// Get the history depth.
+  size_t
+  depth() const;
+
+  /// Get the reliability policy.
+  ReliabilityPolicy
+  reliability() const;
+
+  /// Get the durability policy.
+  DurabilityPolicy
+  durability() const;
+
+  /// Get the deadline duration setting.
+  rclcpp::Duration
+  deadline() const;
+
+  /// Get the lifespan duration setting.
+  rclcpp::Duration
+  lifespan() const;
+
+  /// Get the liveliness policy.
+  LivelinessPolicy
+  liveliness() const;
+
+  /// Get the liveliness lease duration setting.
+  rclcpp::Duration
+  liveliness_lease_duration() const;
+
+  /// Get the `avoid ros namespace convention` setting.
+  bool
+  avoid_ros_namespace_conventions() const;
+
 private:
   rmw_qos_profile_t rmw_qos_profile_;
 };

rclcpp/include/rclcpp/qos_event.hpp

@@ -16,6 +16,8 @@
 #define RCLCPP__QOS_EVENT_HPP_
 
 #include <functional>
+#include <memory>
+#include <stdexcept>
 #include <string>
 
 #include "rcl/error_handling.h"
@@ -131,21 +133,31 @@ public:
     }
   }
 
-  /// Execute any entities of the Waitable that are ready.
-  void
-  execute() override
+  /// Take data so that the callback cannot be scheduled again
+  std::shared_ptr<void>
+  take_data() override
   {
     EventCallbackInfoT callback_info;
-
     rcl_ret_t ret = rcl_take_event(&event_handle_, &callback_info);
     if (ret != RCL_RET_OK) {
       RCUTILS_LOG_ERROR_NAMED(
         "rclcpp",
         "Couldn't take event info: %s", rcl_get_error_string().str);
-      return;
+      return nullptr;
     }
+    return std::static_pointer_cast<void>(std::make_shared<EventCallbackInfoT>(callback_info));
+  }
 
-    event_callback_(callback_info);
+  /// Execute any entities of the Waitable that are ready.
+  void
+  execute(std::shared_ptr<void> & data) override
+  {
+    if (!data) {
+      throw std::runtime_error("'data' is empty");
+    }
+    auto callback_ptr = std::static_pointer_cast<EventCallbackInfoT>(data);
+    event_callback_(*callback_ptr);
+    callback_ptr.reset();
   }
 
 private:

rclcpp/include/rclcpp/qos_overriding_options.hpp

@@ -0,0 +1,154 @@
+// 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__QOS_OVERRIDING_OPTIONS_HPP_
+#define RCLCPP__QOS_OVERRIDING_OPTIONS_HPP_
+
+#include <functional>
+#include <initializer_list>
+#include <ostream>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "rclcpp/qos.hpp"
+#include "rclcpp/visibility_control.hpp"
+
+#include "rcl_interfaces/msg/set_parameters_result.hpp"
+#include "rmw/qos_policy_kind.h"
+
+namespace rclcpp
+{
+
+enum class RCLCPP_PUBLIC_TYPE QosPolicyKind
+{
+  AvoidRosNamespaceConventions = RMW_QOS_POLICY_AVOID_ROS_NAMESPACE_CONVENTIONS,
+  Deadline = RMW_QOS_POLICY_DEADLINE,
+  Depth = RMW_QOS_POLICY_DEPTH,
+  Durability = RMW_QOS_POLICY_DURABILITY,
+  History = RMW_QOS_POLICY_HISTORY,
+  Lifespan = RMW_QOS_POLICY_LIFESPAN,
+  Liveliness = RMW_QOS_POLICY_LIVELINESS,
+  LivelinessLeaseDuration = RMW_QOS_POLICY_LIVELINESS_LEASE_DURATION,
+  Reliability = RMW_QOS_POLICY_RELIABILITY,
+  Invalid = RMW_QOS_POLICY_INVALID,
+};
+
+RCLCPP_PUBLIC
+const char *
+qos_policy_kind_to_cstr(const QosPolicyKind & qpk);
+
+RCLCPP_PUBLIC
+std::ostream &
+operator<<(std::ostream & os, const QosPolicyKind & qpk);
+
+using QosCallbackResult = rcl_interfaces::msg::SetParametersResult;
+using QosCallback = std::function<QosCallbackResult(const rclcpp::QoS &)>;
+
+namespace detail
+{
+// forward declare
+template<typename T>
+class QosParameters;
+}
+
+/// Options that are passed in subscription/publisher constructor to specify QoSConfigurability.
+/**
+ * This options struct allows configuring:
+ * - Which policy kinds will have declared parameters.
+ * - An optional callback, that will be called to validate the final qos profile.
+ * - An optional id. In the case that different qos are desired for two publishers/subscriptions in
+ *   the same topic, this id will allow disambiguating them.
+ *
+ * Example parameter file:
+ *
+ * ```yaml
+ * my_node_name:
+ *  ros__parameters:
+ *    qos_overrides:
+ *      /my/topic/name:
+ *        publisher:  # publisher without provided id
+ *          reliability: reliable
+ *          depth: 100
+ *        publisher_my_id:  # publisher with `id="my_id"
+ *          reliability: reliable
+ *          depth: 10
+ * ```
+ */
+class QosOverridingOptions
+{
+public:
+  /// Default constructor, no overrides allowed.
+  RCLCPP_PUBLIC
+  QosOverridingOptions() = default;
+
+  /// Construct passing a list of QoS policies and a verification callback.
+  /**
+   * This constructor is implicit, e.g.:
+   * ```cpp
+   * node->create_publisher(
+   *   "topic_name",
+   *   default_qos_profile,
+   *   {
+   *     {QosPolicyKind::Reliability},
+   *     [] (auto && qos) {return check_qos_validity(qos)},
+   *     "my_id"
+   *   });
+   * ```
+   * \param policy_kinds list of policy kinds that will be reconfigurable.
+   * \param validation_callback callbak that will be called to validate the validity of
+   *   the qos profile set by the user.
+   * \param id id of the entity.
+   */
+  RCLCPP_PUBLIC
+  QosOverridingOptions(
+    std::initializer_list<QosPolicyKind> policy_kinds,
+    QosCallback validation_callback = nullptr,
+    std::string id = {});
+
+  RCLCPP_PUBLIC
+  const std::string &
+  get_id() const;
+
+  RCLCPP_PUBLIC
+  const std::vector<QosPolicyKind> &
+  get_policy_kinds() const;
+
+  RCLCPP_PUBLIC
+  const QosCallback &
+  get_validation_callback() const;
+
+  /// Construct passing a list of QoS policies and a verification callback.
+  /**
+   * Same as `QosOverridingOptions` constructor, but only declares the default policies:
+   *
+   * History, Depth, Reliability.
+   */
+  RCLCPP_PUBLIC
+  static
+  QosOverridingOptions
+  with_default_policies(QosCallback validation_callback = nullptr, std::string id = {});
+
+private:
+  /// \internal Id of the entity requesting to create parameters.
+  std::string id_;
+  /// \internal Policy kinds that are allowed to be reconfigured.
+  std::vector<QosPolicyKind> policy_kinds_;
+  /// \internal Validation callback that will be called to verify the profile.
+  QosCallback validation_callback_;
+};
+
+}  // namespace rclcpp
+
+#endif  // RCLCPP__QOS_OVERRIDING_OPTIONS_HPP_

rclcpp/include/rclcpp/subscription.hpp

@@ -277,11 +277,18 @@ public:
       return;
     }
     auto typed_message = std::static_pointer_cast<CallbackMessageT>(message);
+
+    std::chrono::time_point<std::chrono::system_clock> now;
+    if (subscription_topic_statistics_) {
+      // get current time before executing callback to
+      // exclude callback duration from topic statistics result.
+      now = std::chrono::system_clock::now();
+    }
+
     any_callback_.dispatch(typed_message, message_info);
 
     if (subscription_topic_statistics_) {
-      const auto nanos = std::chrono::time_point_cast<std::chrono::nanoseconds>(
-        std::chrono::system_clock::now());
+      const auto nanos = std::chrono::time_point_cast<std::chrono::nanoseconds>(now);
       const auto time = rclcpp::Time(nanos.time_since_epoch().count());
       subscription_topic_statistics_->handle_message(*typed_message, time);
     }

rclcpp/include/rclcpp/subscription_options.hpp

@@ -26,6 +26,7 @@
 #include "rclcpp/intra_process_setting.hpp"
 #include "rclcpp/qos.hpp"
 #include "rclcpp/qos_event.hpp"
+#include "rclcpp/qos_overriding_options.hpp"
 #include "rclcpp/topic_statistics_state.hpp"
 #include "rclcpp/visibility_control.hpp"
 
@@ -72,6 +73,8 @@ struct SubscriptionOptionsBase
   };
 
   TopicStatisticsOptions topic_stats_options;
+
+  QosOverridingOptions qos_overriding_options;
 };
 
 /// Structure containing optional configuration for Subscriptions.

rclcpp/include/rclcpp/time_source.hpp

@@ -32,6 +32,20 @@ namespace rclcpp
 {
 class Clock;
 
+/**
+ * Time source that will drive the attached clocks.
+ *
+ * If the attached node `use_sim_time` parameter is `true`, the attached clocks will
+ * be updated based on messages received.
+ *
+ * The subscription to the clock topic created by the time source can have it's qos reconfigured
+ * using parameter overrides, particularly the following ones are accepted:
+ *
+ * - qos_overrides./clock.depth
+ * - qos_overrides./clock.durability
+ * - qos_overrides./clock.history
+ * - qos_overrides./clock.reliability
+ */
 class TimeSource
 {
 public:

rclcpp/include/rclcpp/waitable.hpp

@@ -16,6 +16,7 @@
 #define RCLCPP__WAITABLE_HPP_
 
 #include <atomic>
+#include <memory>
 
 #include "rclcpp/macros.hpp"
 #include "rclcpp/visibility_control.hpp"
@@ -125,8 +126,17 @@ public:
   bool
   is_ready(rcl_wait_set_t * wait_set) = 0;
 
-  /// Execute any entities of the Waitable that are ready.
+  /// Take the data so that it can be consumed with `execute`.
   /**
+   * NOTE: take_data is a partial fix to a larger design issue with the
+   * multithreaded executor. This method is likely to be removed when
+   * a more permanent fix is implemented. A longterm fix is currently
+   * being discussed here: https://github.com/ros2/rclcpp/pull/1276
+   *
+   * This method takes the data from the underlying data structure and
+   * writes it to the void shared pointer `data` that is passed into the
+   * method. The `data` can then be executed with the `execute` method.
+   *
    * Before calling this method, the Waitable should be added to a wait set,
    * waited on, and then updated.
    *
@@ -143,13 +153,41 @@ public:
    * // Update the Waitable
    * waitable.update(wait_set);
    * // Execute any entities of the Waitable that may be ready
-   * waitable.execute();
+   * std::shared_ptr<void> data = waitable.take_data();
+   * ```
+   */
+  RCLCPP_PUBLIC
+  virtual
+  std::shared_ptr<void>
+  take_data() = 0;
+
+  /// Execute data that is passed in.
+  /**
+   * Before calling this method, the Waitable should be added to a wait set,
+   * waited on, and then updated - and the `take_data` method should be
+   * called.
+   *
+   * Example usage:
+   *
+   * ```cpp
+   * // ... create a wait set and a Waitable
+   * // Add the Waitable to the wait set
+   * bool add_ret = waitable.add_to_wait_set(wait_set);
+   * // ... error handling
+   * // Wait
+   * rcl_ret_t wait_ret = rcl_wait(wait_set);
+   * // ... error handling
+   * // Update the Waitable
+   * waitable.update(wait_set);
+   * // Execute any entities of the Waitable that may be ready
+   * std::shared_ptr<void> data = waitable.take_data();
+   * waitable.execute(data);
    * ```
    */
   RCLCPP_PUBLIC
   virtual
   void
-  execute() = 0;
+  execute(std::shared_ptr<void> & data) = 0;
 
   /// Exchange the "in use by wait set" state for this timer.
   /**

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>5.1.0</version>
+  <version>6.3.0</version>
   <description>The ROS client library in C++.</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="mabel@openrobotics.org">Mabel Zhang</maintainer>

rclcpp/resource/logging.hpp.em

@@ -151,10 +151,9 @@ def get_rclcpp_suffix_from_features(features):
 @[ end if]@
       logger.get_name(), \
 @[ if 'stream' not in feature_combination]@
-      rclcpp::get_c_string(RCLCPP_FIRST_ARG(__VA_ARGS__, "")), \
-        RCLCPP_ALL_BUT_FIRST_ARGS(__VA_ARGS__,"")); \
+      __VA_ARGS__); \
 @[ else]@
-      "%s", rclcpp::get_c_string(ss.str())); \
+      "%s", ss.str().c_str()); \
 @[ end if]@
   } while (0)
 

rclcpp/src/rclcpp/context.cpp

@@ -320,7 +320,6 @@ Context::shutdown(const std::string & reason)
   }
   // interrupt all blocking sleep_for() and all blocking executors or wait sets
   this->interrupt_all_sleep_for();
-  this->interrupt_all_wait_sets();
   // remove self from the global contexts
   weak_contexts_->remove_context(this);
   // shutdown logger
@@ -391,75 +390,6 @@ Context::interrupt_all_sleep_for()
   interrupt_condition_variable_.notify_all();
 }
 
-rcl_guard_condition_t *
-Context::get_interrupt_guard_condition(rcl_wait_set_t * wait_set)
-{
-  std::lock_guard<std::mutex> lock(interrupt_guard_cond_handles_mutex_);
-  auto kv = interrupt_guard_cond_handles_.find(wait_set);
-  if (kv != interrupt_guard_cond_handles_.end()) {
-    return &kv->second;
-  } else {
-    rcl_guard_condition_t handle = rcl_get_zero_initialized_guard_condition();
-    rcl_guard_condition_options_t options = rcl_guard_condition_get_default_options();
-    auto ret = rcl_guard_condition_init(&handle, this->get_rcl_context().get(), options);
-    if (RCL_RET_OK != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't initialize guard condition");
-    }
-    interrupt_guard_cond_handles_.emplace(wait_set, handle);
-    return &interrupt_guard_cond_handles_[wait_set];
-  }
-}
-
-void
-Context::release_interrupt_guard_condition(rcl_wait_set_t * wait_set)
-{
-  std::lock_guard<std::mutex> lock(interrupt_guard_cond_handles_mutex_);
-  auto kv = interrupt_guard_cond_handles_.find(wait_set);
-  if (kv != interrupt_guard_cond_handles_.end()) {
-    rcl_ret_t ret = rcl_guard_condition_fini(&kv->second);
-    if (RCL_RET_OK != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "Failed to destroy sigint guard condition");
-    }
-    interrupt_guard_cond_handles_.erase(kv);
-  } else {
-    throw std::runtime_error("Tried to release sigint guard condition for nonexistent wait set");
-  }
-}
-
-void
-Context::release_interrupt_guard_condition(
-  rcl_wait_set_t * wait_set,
-  const std::nothrow_t &) noexcept
-{
-  try {
-    this->release_interrupt_guard_condition(wait_set);
-  } catch (const std::exception & exc) {
-    RCLCPP_ERROR(
-      rclcpp::get_logger("rclcpp"),
-      "caught %s exception when releasing interrupt guard condition: %s",
-      rmw::impl::cpp::demangle(exc).c_str(), exc.what());
-  } catch (...) {
-    RCLCPP_ERROR(
-      rclcpp::get_logger("rclcpp"),
-      "caught unknown exception when releasing interrupt guard condition");
-  }
-}
-
-void
-Context::interrupt_all_wait_sets()
-{
-  std::lock_guard<std::mutex> lock(interrupt_guard_cond_handles_mutex_);
-  for (auto & kv : interrupt_guard_cond_handles_) {
-    rcl_ret_t status = rcl_trigger_guard_condition(&(kv.second));
-    if (status != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR_NAMED(
-        "rclcpp",
-        "failed to trigger guard condition in Context::interrupt_all_wait_sets(): %s",
-        rcl_get_error_string().str);
-    }
-  }
-}
-
 void
 Context::clean_up()
 {

rclcpp/src/rclcpp/duration.cpp

@@ -246,6 +246,27 @@ Duration::to_rmw_time() const
   return result;
 }
 
+Duration
+Duration::from_rmw_time(rmw_time_t duration)
+{
+  Duration ret;
+  constexpr rcl_duration_value_t limit_ns = std::numeric_limits<rcl_duration_value_t>::max();
+  constexpr rcl_duration_value_t limit_sec = RCL_NS_TO_S(limit_ns);
+  if (duration.sec > limit_sec || duration.nsec > limit_ns) {
+    // saturate if will overflow
+    ret.rcl_duration_.nanoseconds = limit_ns;
+    return ret;
+  }
+  uint64_t total_ns = RCL_S_TO_NS(duration.sec) + duration.nsec;
+  if (total_ns > limit_ns) {
+    // saturate if will overflow
+    ret.rcl_duration_.nanoseconds = limit_ns;
+    return ret;
+  }
+  ret.rcl_duration_.nanoseconds = static_cast<rcl_duration_value_t>(total_ns);
+  return ret;
+}
+
 Duration
 Duration::from_seconds(double seconds)
 {

rclcpp/src/rclcpp/executor.cpp

@@ -25,6 +25,7 @@
 
 #include "rclcpp/exceptions.hpp"
 #include "rclcpp/executor.hpp"
+#include "rclcpp/guard_condition.hpp"
 #include "rclcpp/memory_strategy.hpp"
 #include "rclcpp/node.hpp"
 #include "rclcpp/scope_exit.hpp"
@@ -42,28 +43,35 @@ using rclcpp::FutureReturnCode;
 
 Executor::Executor(const rclcpp::ExecutorOptions & options)
 : spinning(false),
+  shutdown_guard_condition_(std::make_shared<rclcpp::GuardCondition>(options.context)),
   memory_strategy_(options.memory_strategy)
 {
+  // Store the context for later use.
+  context_ = options.context;
+
   rcl_guard_condition_options_t guard_condition_options = rcl_guard_condition_get_default_options();
   rcl_ret_t ret = rcl_guard_condition_init(
-    &interrupt_guard_condition_, options.context->get_rcl_context().get(), guard_condition_options);
+    &interrupt_guard_condition_, context_->get_rcl_context().get(), guard_condition_options);
   if (RCL_RET_OK != ret) {
     throw_from_rcl_error(ret, "Failed to create interrupt guard condition in Executor constructor");
   }
 
+  context_->on_shutdown(
+    [weak_gc = std::weak_ptr<rclcpp::GuardCondition>{shutdown_guard_condition_}]() {
+      auto strong_gc = weak_gc.lock();
+      if (strong_gc) {
+        strong_gc->trigger();
+      }
+    });
+
   // 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_)
-
-  // Put the global ctrl-c guard condition in
-  memory_strategy_->add_guard_condition(options.context->get_interrupt_guard_condition(&wait_set_));
+  memory_strategy_->add_guard_condition(&shutdown_guard_condition_->get_rcl_guard_condition());
 
   // Put the executor's guard condition in
   memory_strategy_->add_guard_condition(&interrupt_guard_condition_);
   rcl_allocator_t allocator = memory_strategy_->get_allocator();
 
-  // Store the context for later use.
-  context_ = options.context;
-
   ret = rcl_wait_set_init(
     &wait_set_,
     0, 2, 0, 0, 0, 0,
@@ -129,14 +137,14 @@ Executor::~Executor()
     rcl_reset_error();
   }
   // Remove and release the sigint guard condition
-  memory_strategy_->remove_guard_condition(context_->get_interrupt_guard_condition(&wait_set_));
-  context_->release_interrupt_guard_condition(&wait_set_, std::nothrow);
+  memory_strategy_->remove_guard_condition(&shutdown_guard_condition_->get_rcl_guard_condition());
 }
 
 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);
   }
@@ -150,6 +158,7 @@ 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);
   }
@@ -160,6 +169,7 @@ 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);
   }
@@ -226,7 +236,6 @@ Executor::add_callback_group_to_map(
       }
     }
     // Add the node's notify condition to the guard condition handles
-    std::unique_lock<std::mutex> lock(memory_strategy_mutex_);
     memory_strategy_->add_guard_condition(node_ptr->get_notify_guard_condition());
   }
 }
@@ -237,6 +246,7 @@ Executor::add_callback_group(
   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,
@@ -252,6 +262,7 @@ Executor::add_node(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_pt
   if (has_executor.exchange(true)) {
     throw std::runtime_error("Node has already been added to an executor.");
   }
+  std::lock_guard<std::mutex> guard{mutex_};
   for (auto & weak_group : node_ptr->get_callback_groups()) {
     auto group_ptr = weak_group.lock();
     if (group_ptr != nullptr && !group_ptr->get_associated_with_executor_atomic().load() &&
@@ -300,7 +311,6 @@ Executor::remove_callback_group_from_map(
         throw_from_rcl_error(ret, "Failed to trigger guard condition on callback group remove");
       }
     }
-    std::unique_lock<std::mutex> lock(memory_strategy_mutex_);
     memory_strategy_->remove_guard_condition(node_ptr->get_notify_guard_condition());
   }
 }
@@ -310,6 +320,7 @@ 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_,
@@ -329,6 +340,7 @@ Executor::remove_node(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node
     throw std::runtime_error("Node needs to be associated with an executor.");
   }
 
+  std::lock_guard<std::mutex> guard{mutex_};
   bool found_node = false;
   auto node_it = weak_nodes_.begin();
   while (node_it != weak_nodes_.end()) {
@@ -482,6 +494,7 @@ Executor::set_memory_strategy(rclcpp::memory_strategy::MemoryStrategy::SharedPtr
   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;
 }
 
@@ -504,7 +517,7 @@ Executor::execute_any_executable(AnyExecutable & any_exec)
     execute_client(any_exec.client);
   }
   if (any_exec.waitable) {
-    any_exec.waitable->execute();
+    any_exec.waitable->execute(any_exec.data);
   }
   // Reset the callback_group, regardless of type
   any_exec.callback_group->can_be_taken_from().store(true);
@@ -655,7 +668,7 @@ void
 Executor::wait_for_work(std::chrono::nanoseconds timeout)
 {
   {
-    std::unique_lock<std::mutex> lock(memory_strategy_mutex_);
+    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
@@ -676,8 +689,11 @@ Executor::wait_for_work(std::chrono::nanoseconds timeout)
         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);
-          weak_nodes_to_guard_conditions_.erase(weak_node_ptr);
-          memory_strategy_->remove_guard_condition(node_guard_pair->second);
+          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(
@@ -731,12 +747,14 @@ Executor::wait_for_work(std::chrono::nanoseconds timeout)
 
   // 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(
-  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap weak_groups_to_nodes,
+  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
+  weak_groups_to_nodes,
   rclcpp::CallbackGroup::SharedPtr group)
 {
   if (!group) {
@@ -754,6 +772,7 @@ Executor::get_node_by_group(
 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) {
@@ -794,9 +813,11 @@ Executor::get_next_ready_executable(AnyExecutable & any_executable)
 bool
 Executor::get_next_ready_executable_from_map(
   AnyExecutable & any_executable,
-  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap weak_groups_to_nodes)
+  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
+  weak_groups_to_nodes)
 {
   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) {
@@ -827,6 +848,7 @@ Executor::get_next_ready_executable_from_map(
     // 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;
     }
   }
@@ -882,7 +904,8 @@ Executor::get_next_executable(AnyExecutable & any_executable, std::chrono::nanos
 bool
 Executor::has_node(
   const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
-  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap weak_groups_to_nodes) const
+  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
+  weak_groups_to_nodes) const
 {
   return std::find_if(
     weak_groups_to_nodes.begin(),

rclcpp/src/rclcpp/executors/static_executor_entities_collector.cpp

@@ -77,7 +77,8 @@ StaticExecutorEntitiesCollector::init(
   // Add executor's guard condition
   memory_strategy_->add_guard_condition(executor_guard_condition);
   // Get memory strategy and executable list. Prepare wait_set_
-  execute();
+  std::shared_ptr<void> shared_ptr;
+  execute(shared_ptr);
 }
 
 void
@@ -87,9 +88,16 @@ StaticExecutorEntitiesCollector::fini()
   exec_list_.clear();
 }
 
-void
-StaticExecutorEntitiesCollector::execute()
+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)
@@ -315,8 +323,7 @@ StaticExecutorEntitiesCollector::add_callback_group(
     throw std::runtime_error("Callback group was already added to executor.");
   }
   if (is_new_node) {
-    rclcpp::node_interfaces::NodeBaseInterface::WeakPtr node_weak_ptr(node_ptr);
-    weak_nodes_to_guard_conditions_[node_weak_ptr] = node_ptr->get_notify_guard_condition();
+    weak_nodes_to_guard_conditions_[node_ptr] = node_ptr->get_notify_guard_condition();
     return true;
   }
   return false;

rclcpp/src/rclcpp/executors/static_single_threaded_executor.cpp

@@ -175,8 +175,10 @@ StaticSingleThreadedExecutor::execute_ready_executables()
   }
   // Execute all the ready waitables
   for (size_t i = 0; i < entities_collector_->get_number_of_waitables(); ++i) {
-    if (entities_collector_->get_waitable(i)->is_ready(&wait_set_)) {
-      entities_collector_->get_waitable(i)->execute();
+    auto waitable = entities_collector_->get_waitable(i);
+    if (waitable->is_ready(&wait_set_)) {
+      auto data = waitable->take_data();
+      waitable->execute(data);
     }
   }
 }

rclcpp/src/rclcpp/graph_listener.cpp

@@ -36,11 +36,11 @@ namespace rclcpp
 namespace graph_listener
 {
 
-GraphListener::GraphListener(std::shared_ptr<rclcpp::Context> parent_context)
-: parent_context_(parent_context),
+GraphListener::GraphListener(const std::shared_ptr<Context> & parent_context)
+: weak_parent_context_(parent_context),
+  rcl_parent_context_(parent_context->get_rcl_context()),
   is_started_(false),
-  is_shutdown_(false),
-  shutdown_guard_condition_(nullptr)
+  is_shutdown_(false)
 {
   // TODO(wjwwood): make a guard condition class in rclcpp so this can be tracked
   //   automatically with the rcl guard condition
@@ -48,13 +48,11 @@ GraphListener::GraphListener(std::shared_ptr<rclcpp::Context> parent_context)
   // guard condition is using it.
   rcl_ret_t ret = rcl_guard_condition_init(
     &interrupt_guard_condition_,
-    parent_context->get_rcl_context().get(),
+    rcl_parent_context_.get(),
     rcl_guard_condition_get_default_options());
   if (RCL_RET_OK != ret) {
     throw_from_rcl_error(ret, "failed to create interrupt guard condition");
   }
-
-  shutdown_guard_condition_ = parent_context->get_interrupt_guard_condition(&wait_set_);
 }
 
 GraphListener::~GraphListener()
@@ -64,10 +62,6 @@ GraphListener::~GraphListener()
 
 void GraphListener::init_wait_set()
 {
-  auto parent_context = parent_context_.lock();
-  if (!parent_context) {
-    throw std::runtime_error("parent context was destroyed");
-  }
   rcl_ret_t ret = rcl_wait_set_init(
     &wait_set_,
     0,  // number_of_subscriptions
@@ -76,7 +70,7 @@ void GraphListener::init_wait_set()
     0,  // number_of_clients
     0,  // number_of_services
     0,  // number_of_events
-    parent_context->get_rcl_context().get(),
+    rcl_parent_context_.get(),
     rcl_get_default_allocator());
   if (RCL_RET_OK != ret) {
     throw_from_rcl_error(ret, "failed to initialize wait set");
@@ -90,14 +84,13 @@ GraphListener::start_if_not_started()
   if (is_shutdown_.load()) {
     throw GraphListenerShutdownError();
   }
-  if (!is_started_) {
-    // Initialize the wait set before starting.
-    init_wait_set();
-    // Register an on_shutdown hook to shutdown the graph listener.
+  auto parent_context = weak_parent_context_.lock();
+  if (!is_started_ && parent_context) {
+    // Register an on_shutdown hook to shtudown the graph listener.
     // This is important to ensure that the wait set is finalized before
     // destruction of static objects occurs.
     std::weak_ptr<GraphListener> weak_this = shared_from_this();
-    rclcpp::on_shutdown(
+    parent_context->on_shutdown(
       [weak_this]() {
         auto shared_this = weak_this.lock();
         if (shared_this) {
@@ -105,6 +98,8 @@ GraphListener::start_if_not_started()
           shared_this->shutdown(std::nothrow);
         }
       });
+    // Initialize the wait set before starting.
+    init_wait_set();
     // Start the listener thread.
     listener_thread_ = std::thread(&GraphListener::run, this);
     is_started_ = true;
@@ -149,14 +144,6 @@ GraphListener::run_loop()
     }
     // This lock is released when the loop continues or exits.
     std::lock_guard<std::mutex> nodes_lock(node_graph_interfaces_mutex_, std::adopt_lock);
-    // Ensure that the context doesn't go out of scope.
-    auto parent_context = parent_context_.lock();
-    if (!parent_context) {
-      // The parent context may be destroyed before this loop is stopped.
-      // In that case, the loop is broken and the function just returns silently.
-      return;
-    }
-
     // Resize the wait set if necessary.
     const size_t node_graph_interfaces_size = node_graph_interfaces_.size();
     // Add 2 for the interrupt and shutdown guard conditions
@@ -176,13 +163,7 @@ GraphListener::run_loop()
     if (RCL_RET_OK != ret) {
       throw_from_rcl_error(ret, "failed to add interrupt guard condition to wait set");
     }
-    // Put the shutdown guard condition in the wait set.
-    size_t shutdown_guard_condition_index = 0u;
-    ret = rcl_wait_set_add_guard_condition(
-      &wait_set_, shutdown_guard_condition_, &shutdown_guard_condition_index);
-    if (RCL_RET_OK != ret) {
-      throw_from_rcl_error(ret, "failed to add shutdown guard condition to wait set");
-    }
+
     // Put graph guard conditions for each node into the wait set.
     std::vector<size_t> graph_gc_indexes(node_graph_interfaces_size, 0u);
     for (size_t i = 0u; i < node_graph_interfaces_size; ++i) {
@@ -211,9 +192,6 @@ GraphListener::run_loop()
       throw_from_rcl_error(ret, "failed to wait on wait set");
     }
 
-    // Check to see if the shutdown guard condition has been triggered.
-    bool shutdown_guard_condition_triggered =
-      (shutdown_guard_condition_ == wait_set_.guard_conditions[shutdown_guard_condition_index]);
     // Notify nodes who's guard conditions are set (triggered).
     for (size_t i = 0u; i < node_graph_interfaces_size; ++i) {
       const auto node_ptr = node_graph_interfaces_[i];
@@ -224,7 +202,7 @@ GraphListener::run_loop()
       if (graph_gc == wait_set_.guard_conditions[graph_gc_indexes[i]]) {
         node_ptr->notify_graph_change();
       }
-      if (shutdown_guard_condition_triggered) {
+      if (is_shutdown_) {
         // If shutdown, then notify the node of this as well.
         node_ptr->notify_shutdown();
       }
@@ -365,7 +343,7 @@ GraphListener::cleanup_wait_set()
 }
 
 void
-GraphListener::__shutdown(bool should_throw)
+GraphListener::__shutdown()
 {
   std::lock_guard<std::mutex> shutdown_lock(shutdown_mutex_);
   if (!is_shutdown_.exchange(true)) {
@@ -377,28 +355,6 @@ GraphListener::__shutdown(bool should_throw)
     if (RCL_RET_OK != ret) {
       throw_from_rcl_error(ret, "failed to finalize interrupt guard condition");
     }
-    if (shutdown_guard_condition_) {
-      auto parent_context_ptr = parent_context_.lock();
-      if (parent_context_ptr) {
-        if (should_throw) {
-          parent_context_ptr->release_interrupt_guard_condition(&wait_set_);
-        } else {
-          parent_context_ptr->release_interrupt_guard_condition(&wait_set_, std::nothrow);
-        }
-      } else {
-        ret = rcl_guard_condition_fini(shutdown_guard_condition_);
-        if (RCL_RET_OK != ret) {
-          if (should_throw) {
-            throw_from_rcl_error(ret, "failed to finalize shutdown guard condition");
-          } else {
-            RCLCPP_ERROR(
-              rclcpp::get_logger("rclcpp"),
-              "failed to finalize shutdown guard condition");
-          }
-        }
-      }
-      shutdown_guard_condition_ = nullptr;
-    }
     if (is_started_) {
       cleanup_wait_set();
     }
@@ -408,14 +364,14 @@ GraphListener::__shutdown(bool should_throw)
 void
 GraphListener::shutdown()
 {
-  this->__shutdown(true);
+  this->__shutdown();
 }
 
 void
 GraphListener::shutdown(const std::nothrow_t &) noexcept
 {
   try {
-    this->__shutdown(false);
+    this->__shutdown();
   } catch (const std::exception & exc) {
     RCLCPP_ERROR(
       rclcpp::get_logger("rclcpp"),

rclcpp/src/rclcpp/logger.cpp

@@ -14,6 +14,8 @@
 
 #include <string>
 
+#include "rcl_logging_interface/rcl_logging_interface.h"
+
 #include "rclcpp/exceptions.hpp"
 #include "rclcpp/logger.hpp"
 #include "rclcpp/logging.hpp"
@@ -38,12 +40,28 @@ get_node_logger(const rcl_node_t * node)
   const char * logger_name = rcl_node_get_logger_name(node);
   if (nullptr == logger_name) {
     auto logger = rclcpp::get_logger("rclcpp");
-    RCLCPP_ERROR(logger, "failed to get logger name from node at address %p", node);
+    RCLCPP_ERROR(
+      logger, "failed to get logger name from node at address %p",
+      static_cast<void *>(const_cast<rcl_node_t *>(node)));
     return logger;
   }
   return rclcpp::get_logger(logger_name);
 }
 
+rcpputils::fs::path
+get_logging_directory()
+{
+  char * log_dir = NULL;
+  auto allocator = rcutils_get_default_allocator();
+  rcl_logging_ret_t ret = rcl_logging_get_logging_directory(allocator, &log_dir);
+  if (RCL_LOGGING_RET_OK != ret) {
+    rclcpp::exceptions::throw_from_rcl_error(ret);
+  }
+  std::string path{log_dir};
+  allocator.deallocate(log_dir, allocator.state);
+  return path;
+}
+
 void
 Logger::set_level(Level level)
 {

rclcpp/src/rclcpp/node_interfaces/node_base.cpp

@@ -70,6 +70,7 @@ NodeBase::NodeBase(
   std::shared_ptr<std::recursive_mutex> logging_mutex = get_global_logging_mutex();
   {
     std::lock_guard<std::recursive_mutex> guard(*logging_mutex);
+    // TODO(ivanpauno): /rosout Qos should be reconfigurable.
     // TODO(ivanpauno): Instead of mutually excluding rcl_node_init with the global logger mutex,
     // rcl_logging_rosout_init_publisher_for_node could be decoupled from there and be called
     // here directly.

rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp

@@ -67,6 +67,7 @@ NodeParameters::NodeParameters(
   }
 
   if (start_parameter_event_publisher) {
+    // TODO(ivanpauno): Qos of the `/parameters_event` topic should be somehow overridable.
     events_publisher_ = rclcpp::create_publisher<MessageT, AllocatorT, PublisherT>(
       node_topics,
       "/parameter_events",

rclcpp/src/rclcpp/node_interfaces/node_timers.cpp

@@ -16,6 +16,8 @@
 
 #include <string>
 
+#include "tracetools/tracetools.h"
+
 using rclcpp::node_interfaces::NodeTimers;
 
 NodeTimers::NodeTimers(rclcpp::node_interfaces::NodeBaseInterface * node_base)
@@ -44,4 +46,8 @@ NodeTimers::add_timer(
             std::string("Failed to notify wait set on timer creation: ") +
             rmw_get_error_string().str);
   }
+  TRACEPOINT(
+    rclcpp_timer_link_node,
+    static_cast<const void *>(timer->get_timer_handle().get()),
+    static_cast<const void *>(node_base_->get_rcl_node_handle()));
 }

rclcpp/src/rclcpp/parameter_client.cpp

@@ -144,6 +144,35 @@ AsyncParametersClient::get_parameters(
   return future_result;
 }
 
+std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>>
+AsyncParametersClient::describe_parameters(
+  const std::vector<std::string> & names,
+  std::function<
+    void(std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>>)
+  > callback)
+{
+  auto promise_result =
+    std::make_shared<std::promise<std::vector<rcl_interfaces::msg::ParameterDescriptor>>>();
+  auto future_result = promise_result->get_future().share();
+
+  auto request = std::make_shared<rcl_interfaces::srv::DescribeParameters::Request>();
+  request->names = names;
+
+  describe_parameters_client_->async_send_request(
+    request,
+    [promise_result, future_result, callback](
+      rclcpp::Client<rcl_interfaces::srv::DescribeParameters>::SharedFuture cb_f)
+    {
+      promise_result->set_value(cb_f.get()->descriptors);
+      if (callback != nullptr) {
+        callback(future_result);
+      }
+    }
+  );
+
+  return future_result;
+}
+
 std::shared_future<std::vector<rclcpp::ParameterType>>
 AsyncParametersClient::get_parameter_types(
   const std::vector<std::string> & names,
@@ -313,14 +342,16 @@ AsyncParametersClient::wait_for_service_nanoseconds(std::chrono::nanoseconds tim
 }
 
 std::vector<rclcpp::Parameter>
-SyncParametersClient::get_parameters(const std::vector<std::string> & parameter_names)
+SyncParametersClient::get_parameters(
+  const std::vector<std::string> & parameter_names,
+  std::chrono::nanoseconds timeout)
 {
   auto f = async_parameters_client_->get_parameters(parameter_names);
   using rclcpp::executors::spin_node_until_future_complete;
   if (
     spin_node_until_future_complete(
-      *executor_, node_base_interface_,
-      f) == rclcpp::FutureReturnCode::SUCCESS)
+      *executor_, node_base_interface_, f,
+      timeout) == rclcpp::FutureReturnCode::SUCCESS)
   {
     return f.get();
   }
@@ -337,16 +368,34 @@ SyncParametersClient::has_parameter(const std::string & parameter_name)
   return vars.names.size() > 0;
 }
 
+std::vector<rcl_interfaces::msg::ParameterDescriptor>
+SyncParametersClient::describe_parameters(
+  const std::vector<std::string> & parameter_names,
+  std::chrono::nanoseconds timeout)
+{
+  auto f = async_parameters_client_->describe_parameters(parameter_names);
+
+  using rclcpp::executors::spin_node_until_future_complete;
+  rclcpp::FutureReturnCode future =
+    spin_node_until_future_complete(*executor_, node_base_interface_, f, timeout);
+  if (future == rclcpp::FutureReturnCode::SUCCESS) {
+    return f.get();
+  }
+  return std::vector<rcl_interfaces::msg::ParameterDescriptor>();
+}
+
 std::vector<rclcpp::ParameterType>
-SyncParametersClient::get_parameter_types(const std::vector<std::string> & parameter_names)
+SyncParametersClient::get_parameter_types(
+  const std::vector<std::string> & parameter_names,
+  std::chrono::nanoseconds timeout)
 {
   auto f = async_parameters_client_->get_parameter_types(parameter_names);
 
   using rclcpp::executors::spin_node_until_future_complete;
   if (
     spin_node_until_future_complete(
-      *executor_, node_base_interface_,
-      f) == rclcpp::FutureReturnCode::SUCCESS)
+      *executor_, node_base_interface_, f,
+      timeout) == rclcpp::FutureReturnCode::SUCCESS)
   {
     return f.get();
   }
@@ -355,15 +404,16 @@ SyncParametersClient::get_parameter_types(const std::vector<std::string> & param
 
 std::vector<rcl_interfaces::msg::SetParametersResult>
 SyncParametersClient::set_parameters(
-  const std::vector<rclcpp::Parameter> & parameters)
+  const std::vector<rclcpp::Parameter> & parameters,
+  std::chrono::nanoseconds timeout)
 {
   auto f = async_parameters_client_->set_parameters(parameters);
 
   using rclcpp::executors::spin_node_until_future_complete;
   if (
     spin_node_until_future_complete(
-      *executor_, node_base_interface_,
-      f) == rclcpp::FutureReturnCode::SUCCESS)
+      *executor_, node_base_interface_, f,
+      timeout) == rclcpp::FutureReturnCode::SUCCESS)
   {
     return f.get();
   }
@@ -372,15 +422,16 @@ SyncParametersClient::set_parameters(
 
 rcl_interfaces::msg::SetParametersResult
 SyncParametersClient::set_parameters_atomically(
-  const std::vector<rclcpp::Parameter> & parameters)
+  const std::vector<rclcpp::Parameter> & parameters,
+  std::chrono::nanoseconds timeout)
 {
   auto f = async_parameters_client_->set_parameters_atomically(parameters);
 
   using rclcpp::executors::spin_node_until_future_complete;
   if (
     spin_node_until_future_complete(
-      *executor_, node_base_interface_,
-      f) == rclcpp::FutureReturnCode::SUCCESS)
+      *executor_, node_base_interface_, f,
+      timeout) == rclcpp::FutureReturnCode::SUCCESS)
   {
     return f.get();
   }
@@ -391,15 +442,16 @@ SyncParametersClient::set_parameters_atomically(
 rcl_interfaces::msg::ListParametersResult
 SyncParametersClient::list_parameters(
   const std::vector<std::string> & parameter_prefixes,
-  uint64_t depth)
+  uint64_t depth,
+  std::chrono::nanoseconds timeout)
 {
   auto f = async_parameters_client_->list_parameters(parameter_prefixes, depth);
 
   using rclcpp::executors::spin_node_until_future_complete;
   if (
     spin_node_until_future_complete(
-      *executor_, node_base_interface_,
-      f) == rclcpp::FutureReturnCode::SUCCESS)
+      *executor_, node_base_interface_, f,
+      timeout) == rclcpp::FutureReturnCode::SUCCESS)
   {
     return f.get();
   }

rclcpp/src/rclcpp/qos.cpp

@@ -99,6 +99,13 @@ QoS::history(rmw_qos_history_policy_t history)
   return *this;
 }
 
+QoS &
+QoS::history(HistoryPolicy history)
+{
+  rmw_qos_profile_.history = static_cast<rmw_qos_history_policy_t>(history);
+  return *this;
+}
+
 QoS &
 QoS::keep_last(size_t depth)
 {
@@ -122,6 +129,13 @@ QoS::reliability(rmw_qos_reliability_policy_t reliability)
   return *this;
 }
 
+QoS &
+QoS::reliability(ReliabilityPolicy reliability)
+{
+  rmw_qos_profile_.reliability = static_cast<rmw_qos_reliability_policy_t>(reliability);
+  return *this;
+}
+
 QoS &
 QoS::reliable()
 {
@@ -141,6 +155,13 @@ QoS::durability(rmw_qos_durability_policy_t durability)
   return *this;
 }
 
+QoS &
+QoS::durability(DurabilityPolicy durability)
+{
+  rmw_qos_profile_.durability = static_cast<rmw_qos_durability_policy_t>(durability);
+  return *this;
+}
+
 QoS &
 QoS::durability_volatile()
 {
@@ -186,6 +207,14 @@ QoS::liveliness(rmw_qos_liveliness_policy_t liveliness)
   return *this;
 }
 
+QoS &
+QoS::liveliness(LivelinessPolicy liveliness)
+{
+  rmw_qos_profile_.liveliness = static_cast<rmw_qos_liveliness_policy_t>(liveliness);
+  return *this;
+}
+
+
 QoS &
 QoS::liveliness_lease_duration(rmw_time_t liveliness_lease_duration)
 {
@@ -206,6 +235,51 @@ QoS::avoid_ros_namespace_conventions(bool avoid_ros_namespace_conventions)
   return *this;
 }
 
+HistoryPolicy
+QoS::history() const
+{
+  return static_cast<HistoryPolicy>(rmw_qos_profile_.history);
+}
+
+size_t
+QoS::depth() const {return rmw_qos_profile_.depth;}
+
+ReliabilityPolicy
+QoS::reliability() const
+{
+  return static_cast<ReliabilityPolicy>(rmw_qos_profile_.reliability);
+}
+
+DurabilityPolicy
+QoS::durability() const
+{
+  return static_cast<DurabilityPolicy>(rmw_qos_profile_.durability);
+}
+
+Duration
+QoS::deadline() const {return Duration::from_rmw_time(rmw_qos_profile_.deadline);}
+
+Duration
+QoS::lifespan() const {return Duration::from_rmw_time(rmw_qos_profile_.lifespan);}
+
+LivelinessPolicy
+QoS::liveliness() const
+{
+  return static_cast<LivelinessPolicy>(rmw_qos_profile_.liveliness);
+}
+
+Duration
+QoS::liveliness_lease_duration() const
+{
+  return Duration::from_rmw_time(rmw_qos_profile_.liveliness_lease_duration);
+}
+
+bool
+QoS::avoid_ros_namespace_conventions() const
+{
+  return rmw_qos_profile_.avoid_ros_namespace_conventions;
+}
+
 namespace
 {
 /// Check if two rmw_time_t have the same values.

rclcpp/src/rclcpp/qos_overriding_options.cpp

@@ -0,0 +1,84 @@
+// 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/qos_overriding_options.hpp"
+
+#include <initializer_list>
+#include <ostream>
+#include <stdexcept>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "rmw/qos_policy_kind.h"
+#include "rmw/qos_string_conversions.h"
+
+namespace rclcpp
+{
+
+const char *
+qos_policy_kind_to_cstr(const QosPolicyKind & qpk)
+{
+  const char * ret = rmw_qos_policy_kind_to_str(static_cast<rmw_qos_policy_kind_t>(qpk));
+  if (!ret) {
+    throw std::invalid_argument{"unknown QoS policy kind"};
+  }
+  return ret;
+}
+
+std::ostream &
+operator<<(std::ostream & oss, const QosPolicyKind & qpk)
+{
+  return oss << qos_policy_kind_to_cstr(qpk);
+}
+
+static std::initializer_list<QosPolicyKind> kDefaultPolicies =
+{QosPolicyKind::History, QosPolicyKind::Depth, QosPolicyKind::Reliability};
+
+QosOverridingOptions::QosOverridingOptions(
+  std::initializer_list<QosPolicyKind> policy_kinds,
+  QosCallback validation_callback,
+  std::string id)
+: id_{std::move(id)},
+  policy_kinds_{policy_kinds},
+  validation_callback_{std::move(validation_callback)}
+{}
+
+QosOverridingOptions
+QosOverridingOptions::with_default_policies(
+  QosCallback validation_callback,
+  std::string id)
+{
+  return QosOverridingOptions{kDefaultPolicies, validation_callback, id};
+}
+
+const std::string &
+QosOverridingOptions::get_id() const
+{
+  return id_;
+}
+
+const std::vector<QosPolicyKind> &
+QosOverridingOptions::get_policy_kinds() const
+{
+  return policy_kinds_;
+}
+
+const QosCallback &
+QosOverridingOptions::get_validation_callback() const
+{
+  return validation_callback_;
+}
+
+}  // namespace rclcpp

rclcpp/src/rclcpp/signal_handler.cpp

@@ -243,7 +243,7 @@ SignalHandler::deferred_signal_handler()
             get_logger(),
             "deferred_signal_handler(): "
             "shutting down rclcpp::Context @ %p, because it had shutdown_on_sigint == true",
-            context_ptr.get());
+            static_cast<void *>(context_ptr.get()));
           context_ptr->shutdown("signal handler");
         }
       }

rclcpp/src/rclcpp/time_source.cpp

@@ -233,11 +233,22 @@ void TimeSource::create_clock_sub()
     return;
   }
 
+  rclcpp::SubscriptionOptions options;
+  options.qos_overriding_options = rclcpp::QosOverridingOptions(
+    {
+      rclcpp::QosPolicyKind::Depth,
+      rclcpp::QosPolicyKind::Durability,
+      rclcpp::QosPolicyKind::History,
+      rclcpp::QosPolicyKind::Reliability,
+    });
+
   clock_subscription_ = rclcpp::create_subscription<rosgraph_msgs::msg::Clock>(
+    node_parameters_,
     node_topics_,
     "/clock",
     rclcpp::QoS(KeepLast(1)).best_effort(),
-    std::bind(&TimeSource::clock_cb, this, std::placeholders::_1)
+    std::bind(&TimeSource::clock_cb, this, std::placeholders::_1),
+    options
   );
 }
 

rclcpp/test/benchmark/CMakeLists.txt

@@ -1,3 +1,4 @@
+find_package(ament_cmake_google_benchmark REQUIRED)
 find_package(performance_test_fixture REQUIRED)
 
 # These benchmarks are only being created and run for the default RMW
@@ -31,6 +32,11 @@ if(TARGET benchmark_node_parameters_interface)
   target_link_libraries(benchmark_node_parameters_interface ${PROJECT_NAME})
 endif()
 
+ament_add_google_benchmark(benchmark_parameter_client benchmark_parameter_client.cpp)
+if(TARGET benchmark_parameter_client)
+  target_link_libraries(benchmark_parameter_client ${PROJECT_NAME})
+endif()
+
 add_performance_test(benchmark_service benchmark_service.cpp)
 if(TARGET benchmark_service)
   target_link_libraries(benchmark_service ${PROJECT_NAME})

rclcpp/test/benchmark/benchmark_executor.cpp

@@ -384,6 +384,7 @@ BENCHMARK_F(
   reset_heap_counters();
 
   for (auto _ : st) {
-    entities_collector_->execute();
+    std::shared_ptr<void> data = entities_collector_->take_data();
+    entities_collector_->execute(data);
   }
 }

rclcpp/test/benchmark/benchmark_parameter_client.cpp

@@ -0,0 +1,324 @@
+// 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 <memory>
+#include <vector>
+#include <string>
+
+#include "benchmark/benchmark.h"
+
+#include "rclcpp/rclcpp.hpp"
+
+class RemoteNodeTest : public benchmark::Fixture
+{
+public:
+  RemoteNodeTest()
+  : remote_node_name("my_remote_node")
+  {
+  }
+
+#ifdef __GNUC__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Woverloaded-virtual"
+#endif
+  void SetUp(benchmark::State &)
+  {
+    remote_context = std::make_shared<rclcpp::Context>();
+    remote_context->init(0, nullptr, rclcpp::InitOptions().auto_initialize_logging(false));
+
+    rclcpp::ExecutorOptions exec_options;
+    exec_options.context = remote_context;
+
+    remote_executor = std::make_shared<rclcpp::executors::SingleThreadedExecutor>(exec_options);
+
+    remote_node = std::make_shared<rclcpp::Node>(
+      remote_node_name, rclcpp::NodeOptions().context(remote_context));
+    remote_executor->add_node(remote_node);
+
+    remote_thread = std::thread(&rclcpp::executors::SingleThreadedExecutor::spin, remote_executor);
+  }
+
+  void TearDown(benchmark::State &)
+  {
+    remote_executor->cancel();
+    remote_context->shutdown("Test is complete");
+    remote_thread.join();
+
+    remote_node.reset();
+    remote_executor.reset();
+    remote_context.reset();
+  }
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+
+  const std::string remote_node_name;
+
+protected:
+  rclcpp::Context::SharedPtr remote_context;
+  rclcpp::executors::SingleThreadedExecutor::SharedPtr remote_executor;
+  rclcpp::Node::SharedPtr remote_node;
+  std::thread remote_thread;
+};
+
+class ParameterClientTest : public RemoteNodeTest
+{
+public:
+  ParameterClientTest()
+  : node_name("my_node"),
+    param_prefix("my_prefix"),
+    param1_name(param_prefix + ".my_param_1"),
+    param2_name(param_prefix + ".my_param_2"),
+    param3_name(param_prefix + ".my_param_3")
+  {
+  }
+
+  void SetUp(benchmark::State & state)
+  {
+    RemoteNodeTest::SetUp(state);
+
+    rclcpp::init(0, nullptr);
+    node = std::make_shared<rclcpp::Node>(node_name);
+
+    remote_node->declare_parameter(
+      param1_name, rclcpp::ParameterValue("param1_value"));
+    remote_node->declare_parameter(
+      param2_name, rclcpp::ParameterValue(std::vector<int> {1, 2, 3}));
+    remote_node->declare_parameter(param3_name);
+    remote_node->undeclare_parameter(param3_name);
+
+    params_client = std::make_shared<rclcpp::SyncParametersClient>(node, remote_node_name);
+
+    if (!params_client->wait_for_service()) {
+      state.SkipWithError("Client failed to become ready");
+    }
+  }
+
+  void TearDown(benchmark::State & state)
+  {
+    RemoteNodeTest::TearDown(state);
+
+    rclcpp::shutdown();
+    node.reset();
+    params_client.reset();
+  }
+
+  const std::string node_name;
+  const std::string param_prefix;
+  const std::string param1_name;
+  const std::string param2_name;
+  const std::string param3_name;
+
+protected:
+  rclcpp::Node::SharedPtr node;
+  rclcpp::SyncParametersClient::SharedPtr params_client;
+};
+
+static bool result_is_successful(rcl_interfaces::msg::SetParametersResult result)
+{
+  return result.successful;
+}
+
+BENCHMARK_F(ParameterClientTest, create_destroy_client)(benchmark::State & state)
+{
+  for (auto _ : state) {
+    params_client.reset();
+    params_client = std::make_shared<rclcpp::SyncParametersClient>(node, remote_node_name);
+    if (!params_client->wait_for_service()) {
+      state.SkipWithError("Client failed to become ready");
+      break;
+    }
+  }
+}
+
+BENCHMARK_F(ParameterClientTest, has_parameter_hit)(benchmark::State & state)
+{
+  for (auto _ : state) {
+    if (!params_client->has_parameter(param1_name)) {
+      state.SkipWithError("Parameter was expected");
+      break;
+    }
+  }
+}
+
+BENCHMARK_F(ParameterClientTest, has_parameter_miss)(benchmark::State & state)
+{
+  for (auto _ : state) {
+    if (params_client->has_parameter(param3_name)) {
+      state.SkipWithError("Parameter was not expected");
+      break;
+    }
+  }
+}
+
+BENCHMARK_F(ParameterClientTest, set_parameters_bool)(benchmark::State & state)
+{
+  const std::vector<rclcpp::Parameter> param_values1
+  {
+    rclcpp::Parameter(param1_name, true),
+    rclcpp::Parameter(param2_name, false),
+  };
+  const std::vector<rclcpp::Parameter> param_values2
+  {
+    rclcpp::Parameter(param1_name, false),
+    rclcpp::Parameter(param2_name, true),
+  };
+
+  for (auto _ : state) {
+    std::vector<rcl_interfaces::msg::SetParametersResult> results =
+      params_client->set_parameters(param_values2);
+    if (!std::all_of(results.begin(), results.end(), result_is_successful)) {
+      state.SkipWithError("Failed to set one or more parameters");
+      break;
+    }
+
+    results = params_client->set_parameters(param_values1);
+    if (!std::all_of(results.begin(), results.end(), result_is_successful)) {
+      state.SkipWithError("Failed to set one or more parameters");
+      break;
+    }
+  }
+}
+
+BENCHMARK_F(ParameterClientTest, set_parameters_atomically_bool)(benchmark::State & state)
+{
+  const std::vector<rclcpp::Parameter> param_values1
+  {
+    rclcpp::Parameter(param1_name, true),
+    rclcpp::Parameter(param2_name, false),
+  };
+  const std::vector<rclcpp::Parameter> param_values2
+  {
+    rclcpp::Parameter(param1_name, false),
+    rclcpp::Parameter(param2_name, true),
+  };
+
+  for (auto _ : state) {
+    rcl_interfaces::msg::SetParametersResult result =
+      params_client->set_parameters_atomically(param_values2);
+    if (!result.successful) {
+      state.SkipWithError(("Failed to set parameters: " + result.reason).c_str());
+      break;
+    }
+
+    result = params_client->set_parameters_atomically(param_values1);
+    if (!result.successful) {
+      state.SkipWithError(("Failed to set parameters: " + result.reason).c_str());
+      break;
+    }
+  }
+}
+
+BENCHMARK_F(ParameterClientTest, set_parameters_string)(benchmark::State & state)
+{
+  const std::vector<rclcpp::Parameter> param_values1
+  {
+    rclcpp::Parameter(param1_name, "param 1 value A"),
+    rclcpp::Parameter(param2_name, "param 2 value B"),
+  };
+  const std::vector<rclcpp::Parameter> param_values2
+  {
+    rclcpp::Parameter(param1_name, "param 1 value B"),
+    rclcpp::Parameter(param2_name, "param 2 value A"),
+  };
+
+  for (auto _ : state) {
+    std::vector<rcl_interfaces::msg::SetParametersResult> results =
+      params_client->set_parameters(param_values2);
+    if (!std::all_of(results.begin(), results.end(), result_is_successful)) {
+      state.SkipWithError("Failed to set one or more parameters");
+      break;
+    }
+
+    results = params_client->set_parameters(param_values1);
+    if (!std::all_of(results.begin(), results.end(), result_is_successful)) {
+      state.SkipWithError("Failed to set one or more parameters");
+      break;
+    }
+  }
+}
+
+BENCHMARK_F(ParameterClientTest, set_parameters_array)(benchmark::State & state)
+{
+  const std::vector<rclcpp::Parameter> param_values1
+  {
+    rclcpp::Parameter(param1_name, std::vector<int> {0, 1, 2}),
+    rclcpp::Parameter(param2_name, std::vector<int> {3, 4, 5}),
+  };
+  const std::vector<rclcpp::Parameter> param_values2
+  {
+    rclcpp::Parameter(param1_name, std::vector<int> {4, 5, 6}),
+    rclcpp::Parameter(param2_name, std::vector<int> {7, 8, 9}),
+  };
+
+  for (auto _ : state) {
+    std::vector<rcl_interfaces::msg::SetParametersResult> results =
+      params_client->set_parameters(param_values2);
+    if (!std::all_of(results.begin(), results.end(), result_is_successful)) {
+      state.SkipWithError("Failed to set one or more parameters");
+      break;
+    }
+
+    results = params_client->set_parameters(param_values1);
+    if (!std::all_of(results.begin(), results.end(), result_is_successful)) {
+      state.SkipWithError("Failed to set one or more parameters");
+      break;
+    }
+  }
+}
+
+BENCHMARK_F(ParameterClientTest, get_parameters)(benchmark::State & state)
+{
+  for (auto _ : state) {
+    std::vector<rclcpp::Parameter> results = params_client->get_parameters({param1_name});
+    if (results.size() != 1 || results[0].get_name() != param1_name) {
+      state.SkipWithError("Got the wrong parameter(s)");
+      break;
+    }
+  }
+}
+
+BENCHMARK_F(ParameterClientTest, list_parameters_hit)(benchmark::State & state)
+{
+  const std::vector<std::string> prefixes
+  {
+    param_prefix,
+  };
+
+  for (auto _ : state) {
+    rcl_interfaces::msg::ListParametersResult param_list =
+      params_client->list_parameters(prefixes, 10);
+    if (param_list.names.size() != 2) {
+      state.SkipWithError("Expected parameters");
+      break;
+    }
+  }
+}
+
+BENCHMARK_F(ParameterClientTest, list_parameters_miss)(benchmark::State & state)
+{
+  const std::vector<std::string> prefixes
+  {
+    "your_prefix",
+  };
+
+  for (auto _ : state) {
+    rcl_interfaces::msg::ListParametersResult param_list =
+      params_client->list_parameters(prefixes, 10);
+    if (param_list.names.size() != 0) {
+      state.SkipWithError("Expected no parameters");
+      break;
+    }
+  }
+}

rclcpp/test/rclcpp/CMakeLists.txt

@@ -369,6 +369,18 @@ if(TARGET test_qos_event)
     mimick
   )
 endif()
+ament_add_gmock(test_qos_overriding_options test_qos_overriding_options.cpp)
+if(TARGET test_qos_overriding_options)
+  target_link_libraries(test_qos_overriding_options
+    ${PROJECT_NAME}
+  )
+endif()
+ament_add_gmock(test_qos_parameters test_qos_parameters.cpp)
+if(TARGET test_qos_parameters)
+  target_link_libraries(test_qos_parameters
+    ${PROJECT_NAME}
+  )
+endif()
 ament_add_gtest(test_rate test_rate.cpp)
 if(TARGET test_rate)
   ament_target_dependencies(test_rate

rclcpp/test/rclcpp/allocator/test_allocator_common.cpp

@@ -23,7 +23,9 @@ TEST(TestAllocatorCommon, retyped_allocate) {
   void * untyped_allocator = &allocator;
   void * allocated_mem =
     rclcpp::allocator::retyped_allocate<std::allocator<char>>(1u, untyped_allocator);
-  ASSERT_NE(nullptr, allocated_mem);
+  // The more natural check here is ASSERT_NE(nullptr, ptr), but clang static
+  // analysis throws a false-positive memory leak warning.  Use ASSERT_TRUE instead.
+  ASSERT_TRUE(nullptr != allocated_mem);
 
   auto code = [&untyped_allocator, allocated_mem]() {
       rclcpp::allocator::retyped_deallocate<int, std::allocator<int>>(
@@ -32,11 +34,15 @@ TEST(TestAllocatorCommon, retyped_allocate) {
   EXPECT_NO_THROW(code());
 
   allocated_mem = allocator.allocate(1);
-  ASSERT_NE(nullptr, allocated_mem);
+  // The more natural check here is ASSERT_NE(nullptr, ptr), but clang static
+  // analysis throws a false-positive memory leak warning.  Use ASSERT_TRUE instead.
+  ASSERT_TRUE(nullptr != allocated_mem);
   void * reallocated_mem =
     rclcpp::allocator::retyped_reallocate<int, std::allocator<int>>(
     allocated_mem, 2u, untyped_allocator);
-  ASSERT_NE(nullptr, reallocated_mem);
+  // The more natural check here is ASSERT_NE(nullptr, ptr), but clang static
+  // analysis throws a false-positive memory leak warning.  Use ASSERT_TRUE instead.
+  ASSERT_TRUE(nullptr != reallocated_mem);
 
   auto code2 = [&untyped_allocator, reallocated_mem]() {
       rclcpp::allocator::retyped_deallocate<int, std::allocator<int>>(

rclcpp/test/rclcpp/allocator/test_allocator_deleter.cpp

@@ -38,7 +38,9 @@ TEST(TestAllocatorDeleter, construct_destruct) {
 TEST(TestAllocatorDeleter, delete) {
   std::allocator<int> allocator;
   int * some_mem = allocator.allocate(1u);
-  ASSERT_NE(nullptr, some_mem);
+  // The more natural check here is ASSERT_NE(nullptr, ptr), but clang static
+  // analysis throws a false-positive memory leak warning.  Use ASSERT_TRUE instead.
+  ASSERT_TRUE(nullptr != some_mem);
 
   rclcpp::allocator::AllocatorDeleter<std::allocator<int>> deleter(&allocator);
   EXPECT_NO_THROW(deleter(some_mem));

rclcpp/test/rclcpp/executors/test_executors.cpp

@@ -436,9 +436,16 @@ public:
     return true;
   }
 
-  void
-  execute() override
+  std::shared_ptr<void>
+  take_data() override
   {
+    return nullptr;
+  }
+
+  void
+  execute(std::shared_ptr<void> & data) override
+  {
+    (void) data;
     count_++;
     std::this_thread::sleep_for(1ms);
   }

rclcpp/test/rclcpp/executors/test_static_executor_entities_collector.cpp

@@ -235,7 +235,16 @@ public:
 
   bool is_ready(rcl_wait_set_t *) override {return true;}
 
-  void execute() override {}
+  std::shared_ptr<void>
+  take_data() override
+  {
+    return nullptr;
+  }
+  void
+  execute(std::shared_ptr<void> & data) override
+  {
+    (void) data;
+  }
 };
 
 TEST_F(TestStaticExecutorEntitiesCollector, add_remove_node_with_entities) {
@@ -363,8 +372,9 @@ TEST_F(TestStaticExecutorEntitiesCollector, prepare_wait_set_rcl_wait_set_clear_
 
   {
     auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_wait_set_clear, RCL_RET_ERROR);
+    std::shared_ptr<void> data = entities_collector_->take_data();
     RCLCPP_EXPECT_THROW_EQ(
-      entities_collector_->execute(),
+      entities_collector_->execute(data),
       std::runtime_error("Couldn't clear wait set"));
   }
 
@@ -393,8 +403,9 @@ TEST_F(TestStaticExecutorEntitiesCollector, prepare_wait_set_rcl_wait_set_resize
 
   {
     auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_wait_set_resize, RCL_RET_ERROR);
+    std::shared_ptr<void> data = entities_collector_->take_data();
     RCLCPP_EXPECT_THROW_EQ(
-      entities_collector_->execute(),
+      entities_collector_->execute(data),
       std::runtime_error("Couldn't resize the wait set: error not set"));
   }
 
@@ -616,7 +627,8 @@ TEST_F(
   RCLCPP_SCOPE_EXIT(entities_collector_->fini());
 
   cb_group->get_associated_with_executor_atomic().exchange(false);
-  entities_collector_->execute();
+  std::shared_ptr<void> data = entities_collector_->take_data();
+  entities_collector_->execute(data);
 
   EXPECT_TRUE(entities_collector_->remove_node(node->get_node_base_interface()));
 }

rclcpp/test/rclcpp/node_interfaces/test_node_graph.cpp

@@ -14,9 +14,11 @@
 
 #include <gtest/gtest.h>
 
+#include <functional>
 #include <memory>
 #include <string>
 #include <utility>
+#include <vector>
 
 #include "rcl/graph.h"
 #include "rcl/node_options.h"
@@ -66,6 +68,43 @@ protected:
 
   const rclcpp::node_interfaces::NodeGraph * node_graph() const {return node_graph_;}
 
+  size_t get_num_graph_things(std::function<size_t()> predicate)
+  {
+    constexpr std::chrono::milliseconds timeout(100);
+
+    size_t tries = 0;
+    size_t num_things = 0;
+    while (tries++ < 5) {
+      num_things = predicate();
+      if (num_things >= 1) {
+        break;
+      }
+
+      auto event = node()->get_graph_event();
+      EXPECT_NO_THROW(node()->wait_for_graph_change(event, timeout));
+    }
+
+    return num_things;
+  }
+
+  size_t get_num_topics()
+  {
+    return get_num_graph_things(
+      [this]() -> size_t {
+        auto topic_names_and_types = node_graph()->get_topic_names_and_types();
+        return topic_names_and_types.size();
+      });
+  }
+
+  size_t get_num_services()
+  {
+    return get_num_graph_things(
+      [this]() -> size_t {
+        auto service_names_and_types = node_graph()->get_service_names_and_types();
+        return service_names_and_types.size();
+      });
+  }
+
 private:
   std::shared_ptr<rclcpp::Node> node_;
   rclcpp::node_interfaces::NodeGraph * node_graph_;
@@ -73,11 +112,9 @@ private:
 
 TEST_F(TestNodeGraph, construct_from_node)
 {
-  auto topic_names_and_types = node_graph()->get_topic_names_and_types(false);
-  EXPECT_LT(0u, topic_names_and_types.size());
+  EXPECT_LT(0u, get_num_topics());
 
-  auto service_names_and_types = node_graph()->get_service_names_and_types();
-  EXPECT_LT(0u, service_names_and_types.size());
+  EXPECT_LT(0u, get_num_services());
 
   auto names = node_graph()->get_node_names();
   EXPECT_EQ(1u, names.size());
@@ -91,13 +128,12 @@ TEST_F(TestNodeGraph, construct_from_node)
 
   // get_graph_event is non-const
   EXPECT_NE(nullptr, node()->get_node_graph_interface()->get_graph_event());
-  EXPECT_EQ(1u, node_graph()->count_graph_users());
+  EXPECT_LE(1u, node_graph()->count_graph_users());
 }
 
 TEST_F(TestNodeGraph, get_topic_names_and_types)
 {
-  auto topic_names_and_types = node_graph()->get_topic_names_and_types();
-  EXPECT_LT(0u, topic_names_and_types.size());
+  ASSERT_LT(0u, get_num_topics());
 }
 
 TEST_F(TestNodeGraph, get_topic_names_and_types_rcl_error)
@@ -126,8 +162,7 @@ TEST_F(TestNodeGraph, get_topic_names_and_types_rcl_names_and_types_fini_error)
 
 TEST_F(TestNodeGraph, get_service_names_and_types)
 {
-  auto service_names_and_types = node_graph()->get_service_names_and_types();
-  EXPECT_LT(0u, service_names_and_types.size());
+  ASSERT_LT(0u, get_num_services());
 }
 
 TEST_F(TestNodeGraph, get_service_names_and_types_rcl_error)
@@ -309,8 +344,13 @@ TEST_F(TestNodeGraph, get_info_by_topic)
 
   EXPECT_EQ(0u, node_graph()->get_publishers_info_by_topic("topic", true).size());
 
-  auto publishers = node_graph()->get_publishers_info_by_topic("topic", false);
-  ASSERT_EQ(1u, publishers.size());
+  std::vector<rclcpp::TopicEndpointInfo> publishers;
+  size_t num_publishers = get_num_graph_things(
+    [this, &publishers]() {
+      publishers = node_graph()->get_publishers_info_by_topic("topic", false);
+      return publishers.size();
+    });
+  ASSERT_EQ(1u, num_publishers);
 
   auto publisher_endpoint_info = publishers[0];
   const auto const_publisher_endpoint_info = publisher_endpoint_info;

rclcpp/test/rclcpp/node_interfaces/test_node_waitables.cpp

@@ -30,7 +30,17 @@ class TestWaitable : public rclcpp::Waitable
 public:
   bool add_to_wait_set(rcl_wait_set_t *) override {return false;}
   bool is_ready(rcl_wait_set_t *) override {return false;}
-  void execute() override {}
+
+  std::shared_ptr<void>
+  take_data() override
+  {
+    return nullptr;
+  }
+
+  void execute(std::shared_ptr<void> & data) override
+  {
+    (void) data;
+  }
 };
 
 class TestNodeWaitables : public ::testing::Test

rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp

@@ -49,7 +49,16 @@ public:
     return test_waitable_result;
   }
 
-  void execute() override {}
+  std::shared_ptr<void>
+  take_data() override
+  {
+    return nullptr;
+  }
+
+  void execute(std::shared_ptr<void> & data) override
+  {
+    (void) data;
+  }
 };
 
 struct RclWaitSetSizes

rclcpp/test/rclcpp/test_create_subscription.cpp

@@ -50,6 +50,34 @@ TEST_F(TestCreateSubscription, create) {
   EXPECT_STREQ("/ns/topic_name", subscription->get_topic_name());
 }
 
+TEST_F(TestCreateSubscription, create_with_overriding_options) {
+  auto node = std::make_shared<rclcpp::Node>("my_node", "/ns");
+  const rclcpp::QoS qos(10);
+  auto options = rclcpp::SubscriptionOptions();
+  options.qos_overriding_options = rclcpp::QosOverridingOptions::with_default_policies();
+  auto callback = [](const test_msgs::msg::Empty::SharedPtr) {};
+  auto subscription =
+    rclcpp::create_subscription<test_msgs::msg::Empty>(node, "topic_name", qos, callback, options);
+
+  ASSERT_NE(nullptr, subscription);
+  EXPECT_STREQ("/ns/topic_name", subscription->get_topic_name());
+}
+
+TEST_F(TestCreateSubscription, create_separated_node_topics_and_parameters) {
+  auto node = std::make_shared<rclcpp::Node>("my_node", "/ns");
+  const rclcpp::QoS qos(10);
+  auto options = rclcpp::SubscriptionOptions();
+  auto callback = [](const test_msgs::msg::Empty::SharedPtr) {};
+
+  auto node_parameters = node->get_node_parameters_interface();
+  auto node_topics = node->get_node_topics_interface();
+  auto subscription = rclcpp::create_subscription<test_msgs::msg::Empty>(
+    node_parameters, node_topics, "topic_name", qos, callback, options);
+
+  ASSERT_NE(nullptr, subscription);
+  EXPECT_STREQ("/ns/topic_name", subscription->get_topic_name());
+}
+
 TEST_F(TestCreateSubscription, create_with_statistics) {
   auto node = std::make_shared<rclcpp::Node>("my_node", "/ns");
   const rclcpp::QoS qos(10);

rclcpp/test/rclcpp/test_duration.cpp

@@ -148,6 +148,34 @@ TEST_F(TestDuration, from_seconds) {
     rclcpp::Duration::from_seconds(-1.5));
 }
 
+TEST_F(TestDuration, from_rmw_time) {
+  constexpr auto max_rcl_duration = std::numeric_limits<rcl_duration_value_t>::max();
+  {
+    rmw_time_t rmw_duration{};
+    rmw_duration.sec = RCL_NS_TO_S(max_rcl_duration) + 1uLL;
+    EXPECT_EQ(rclcpp::Duration::from_rmw_time(rmw_duration).nanoseconds(), max_rcl_duration);
+  }
+  {
+    rmw_time_t rmw_duration{};
+    rmw_duration.nsec = max_rcl_duration + 1uLL;
+    EXPECT_EQ(rclcpp::Duration::from_rmw_time(rmw_duration).nanoseconds(), max_rcl_duration);
+  }
+  {
+    rmw_time_t rmw_duration{};
+    rmw_duration.nsec = max_rcl_duration;
+    rmw_duration.sec = RCL_NS_TO_S(max_rcl_duration);
+    EXPECT_EQ(rclcpp::Duration::from_rmw_time(rmw_duration).nanoseconds(), max_rcl_duration);
+  }
+  {
+    rmw_time_t rmw_duration{};
+    rmw_duration.sec = 1u;
+    rmw_duration.nsec = 1000u;
+    EXPECT_EQ(
+      rclcpp::Duration::from_rmw_time(rmw_duration).nanoseconds(),
+      static_cast<rcl_duration_value_t>(RCL_S_TO_NS(rmw_duration.sec) + rmw_duration.nsec));
+  }
+}
+
 TEST_F(TestDuration, std_chrono_constructors) {
   EXPECT_EQ(rclcpp::Duration(0ns), rclcpp::Duration(0.0s));
   EXPECT_EQ(rclcpp::Duration(0ns), rclcpp::Duration(0s));

rclcpp/test/rclcpp/test_executor.cpp

@@ -21,6 +21,7 @@
 
 #include "rclcpp/executor.hpp"
 #include "rclcpp/memory_strategy.hpp"
+#include "rclcpp/executors/single_threaded_executor.hpp"
 #include "rclcpp/strategies/allocator_memory_strategy.hpp"
 
 #include "../mocking_utils/patch.hpp"
@@ -54,6 +55,7 @@ public:
   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);
   }
 
@@ -87,19 +89,47 @@ MOCKING_UTILS_BOOL_OPERATOR_RETURNS_FALSE(rcutils_allocator_t, !=)
 MOCKING_UTILS_BOOL_OPERATOR_RETURNS_FALSE(rcutils_allocator_t, <)
 MOCKING_UTILS_BOOL_OPERATOR_RETURNS_FALSE(rcutils_allocator_t, >)
 
+TEST_F(TestExecutor, add_remove_node_thread_safe) {
+  using namespace std::chrono_literals;
+
+  // Create an Executor
+  rclcpp::executors::SingleThreadedExecutor executor;
+
+  auto future = std::async(std::launch::async, [&executor] {executor.spin();});
+
+  // Add and remove nodes repeatedly
+  // Test that this does not cause a segfault
+  size_t num_nodes = 100;
+  for (size_t i = 0; i < num_nodes; ++i) {
+    std::ostringstream name;
+    name << "node_" << i;
+    auto node = std::make_shared<rclcpp::Node>(name.str());
+    executor.add_node(node);
+    // Sleeping here helps exaggerate the issue
+    std::this_thread::sleep_for(std::chrono::milliseconds(1));
+    executor.remove_node(node);
+  }
+  std::future_status future_status = std::future_status::timeout;
+  do {
+    executor.cancel();
+    future_status = future.wait_for(1s);
+  } while (future_status == std::future_status::timeout);
+  EXPECT_EQ(future_status, std::future_status::ready);
+  future.get();
+}
+
 TEST_F(TestExecutor, constructor_bad_guard_condition_init) {
   auto mock = mocking_utils::patch_and_return(
     "lib:rclcpp", rcl_guard_condition_init, RCL_RET_ERROR);
-  RCLCPP_EXPECT_THROW_EQ(
-    new DummyExecutor(),
-    std::runtime_error(
-      "Failed to create interrupt guard condition in Executor constructor: error not set"));
+  EXPECT_THROW(
+    static_cast<void>(std::make_unique<DummyExecutor>()),
+    rclcpp::exceptions::RCLError);
 }
 
 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(
-    new DummyExecutor(),
+    static_cast<void>(std::make_unique<DummyExecutor>()),
     std::runtime_error("Failed to create wait set in Executor constructor: error not set"));
 }
 

rclcpp/test/rclcpp/test_graph_listener.cpp

@@ -143,7 +143,9 @@ TEST_F(TestGraphListener, error_run_graph_listener_destroy_context) {
   auto graph_listener_error =
     std::make_shared<TestGraphListenerProtectedMethods>(context_to_destroy);
   context_to_destroy.reset();
-  EXPECT_NO_THROW(graph_listener_error->run_protected());
+  EXPECT_THROW(
+    graph_listener_error->run_protected(),
+    rclcpp::exceptions::RCLError);
 }
 
 TEST_F(TestGraphListener, error_run_graph_listener_mock_wait_set_clear) {
@@ -170,26 +172,6 @@ TEST_F(TestGraphListener, error_run_graph_listener_mock_wait_set_add_guard_condi
     std::runtime_error("failed to add interrupt guard condition to wait set: error not set"));
 }
 
-TEST_F(TestGraphListener, error_run_graph_listener_mock_wait_set_add_guard_condition_twice) {
-  auto global_context = rclcpp::contexts::get_global_default_context();
-  auto graph_listener_test =
-    std::make_shared<TestGraphListenerProtectedMethods>(global_context);
-  graph_listener_test->mock_init_wait_set();
-  auto mock = mocking_utils::patch(
-    "lib:rclcpp", rcl_wait_set_add_guard_condition, [](auto, ...) {
-      static int counter = 1;
-      if (counter == 1) {
-        counter++;
-        return RCL_RET_OK;
-      } else {
-        return RCL_RET_ERROR;
-      }
-    });
-  RCLCPP_EXPECT_THROW_EQ(
-    graph_listener_test->run_protected(),
-    std::runtime_error("failed to add shutdown guard condition to wait set: error not set"));
-}
-
 TEST_F(TestGraphListener, error_run_graph_listener_mock_wait_error) {
   auto global_context = rclcpp::contexts::get_global_default_context();
   auto graph_listener_test =

rclcpp/test/rclcpp/test_logger.cpp

@@ -17,6 +17,8 @@
 #include <memory>
 #include <string>
 
+#include "rcutils/env.h"
+
 #include "rclcpp/logger.hpp"
 #include "rclcpp/logging.hpp"
 #include "rclcpp/node.hpp"
@@ -80,78 +82,99 @@ TEST(TestLogger, set_level) {
   rcutils_logging_set_output_handler(rcutils_logging_console_output_handler);
 
   // default
-  RCLCPP_DEBUG(logger, std::string("message %s"), "debug");
+  RCLCPP_DEBUG(logger, "message %s", "debug");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_INFO(logger, std::string("message %s"), "info");
+  RCLCPP_INFO(logger, "message %s", "info");
   EXPECT_TRUE(g_last_log_event.console_output_handler_called);
   EXPECT_EQ("message info", g_last_log_event.message);
 
   // unset
   g_last_log_event.console_output_handler_called = false;
   logger.set_level(rclcpp::Logger::Level::Unset);
-  RCLCPP_DEBUG(logger, std::string("message %s"), "debug");
+  RCLCPP_DEBUG(logger, "message");
+  RCLCPP_DEBUG(logger, "message %s", "debug");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_INFO(logger, std::string("message %s"), "info");
+  RCLCPP_INFO(logger, "message");
+  RCLCPP_INFO(logger, "message %s", "info");
   EXPECT_TRUE(g_last_log_event.console_output_handler_called);
   EXPECT_EQ("message info", g_last_log_event.message);
 
   // debug
   g_last_log_event.console_output_handler_called = false;
   logger.set_level(rclcpp::Logger::Level::Debug);
-  RCLCPP_DEBUG(logger, std::string("message %s"), "debug");
+  RCLCPP_DEBUG(logger, "message %s", "debug");
   EXPECT_TRUE(g_last_log_event.console_output_handler_called);
   EXPECT_EQ("message debug", g_last_log_event.message);
-  RCLCPP_INFO(logger, std::string("message %s"), "info");
+  RCLCPP_INFO(logger, "message %s", "info");
   EXPECT_EQ("message info", g_last_log_event.message);
 
   // info
   g_last_log_event.console_output_handler_called = false;
   logger.set_level(rclcpp::Logger::Level::Info);
-  RCLCPP_DEBUG(logger, std::string("message %s"), "debug");
+  RCLCPP_DEBUG(logger, "message %s", "debug");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_INFO(logger, std::string("message %s"), "info");
+  RCLCPP_INFO(logger, "message %s", "info");
   EXPECT_TRUE(g_last_log_event.console_output_handler_called);
   EXPECT_EQ("message info", g_last_log_event.message);
 
   // warn
   g_last_log_event.console_output_handler_called = false;
   logger.set_level(rclcpp::Logger::Level::Warn);
-  RCLCPP_DEBUG(logger, std::string("message %s"), "debug");
+  RCLCPP_DEBUG(logger, "message %s", "debug");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_INFO(logger, std::string("message %s"), "info");
+  RCLCPP_INFO(logger, "message %s", "info");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_WARN(logger, std::string("message %s"), "warn");
+  RCLCPP_WARN(logger, "message %s", "warn");
   EXPECT_TRUE(g_last_log_event.console_output_handler_called);
   EXPECT_EQ("message warn", g_last_log_event.message);
 
   // error
   g_last_log_event.console_output_handler_called = false;
   logger.set_level(rclcpp::Logger::Level::Error);
-  RCLCPP_DEBUG(logger, std::string("message %s"), "debug");
+  RCLCPP_DEBUG(logger, "message %s", "debug");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_INFO(logger, std::string("message %s"), "info");
+  RCLCPP_INFO(logger, "message %s", "info");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_WARN(logger, std::string("message %s"), "warn");
+  RCLCPP_WARN(logger, "message %s", "warn");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_ERROR(logger, std::string("message %s"), "error");
+  RCLCPP_ERROR(logger, "message %s", "error");
   EXPECT_TRUE(g_last_log_event.console_output_handler_called);
   EXPECT_EQ("message error", g_last_log_event.message);
 
   // fatal
   g_last_log_event.console_output_handler_called = false;
   logger.set_level(rclcpp::Logger::Level::Fatal);
-  RCLCPP_DEBUG(logger, std::string("message %s"), "debug");
+  RCLCPP_DEBUG(logger, "message %s", "debug");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_INFO(logger, std::string("message %s"), "info");
+  RCLCPP_INFO(logger, "message %s", "info");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_WARN(logger, std::string("message %s"), "warn");
+  RCLCPP_WARN(logger, "message %s", "warn");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_ERROR(logger, std::string("message %s"), "error");
+  RCLCPP_ERROR(logger, "message %s", "error");
   EXPECT_FALSE(g_last_log_event.console_output_handler_called);
-  RCLCPP_FATAL(logger, std::string("message %s"), "fatal");
+  RCLCPP_FATAL(logger, "message %s", "fatal");
   EXPECT_TRUE(g_last_log_event.console_output_handler_called);
   EXPECT_EQ("message fatal", g_last_log_event.message);
 
   rcutils_logging_set_output_handler(previous_output_handler);
   EXPECT_EQ(RCUTILS_RET_OK, rcutils_logging_shutdown());
 }
+
+TEST(TestLogger, get_logging_directory) {
+  ASSERT_EQ(true, rcutils_set_env("HOME", "/fake_home_dir"));
+  ASSERT_EQ(true, rcutils_set_env("USERPROFILE", nullptr));
+  ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", nullptr));
+  ASSERT_EQ(true, rcutils_set_env("ROS_HOME", nullptr));
+
+  auto path = rclcpp::get_logging_directory();
+  auto expected_path = rcpputils::fs::path{"/fake_home_dir"} / ".ros" / "log";
+
+  // TODO(ivanpauno): Add operator== to rcpputils::fs::path
+  auto it = path.cbegin();
+  auto eit = expected_path.cbegin();
+  for (; it != path.cend() && eit != expected_path.cend(); ++it, ++eit) {
+    EXPECT_EQ(*eit, *it);
+  }
+  EXPECT_EQ(it, path.cend());
+  EXPECT_EQ(eit, expected_path.cend());
+}

rclcpp/test/rclcpp/test_logging.cpp

@@ -110,26 +110,6 @@ TEST_F(TestLoggingMacros, test_logging_named) {
   EXPECT_EQ("message 3", g_last_log_event.message);
 }
 
-TEST_F(TestLoggingMacros, test_logging_string) {
-  for (std::string i : {"one", "two", "three"}) {
-    RCLCPP_DEBUG(g_logger, "message " + i);
-  }
-  EXPECT_EQ(3u, g_log_calls);
-  EXPECT_EQ("message three", g_last_log_event.message);
-
-  RCLCPP_DEBUG(g_logger, "message " "four");
-  EXPECT_EQ("message four", g_last_log_event.message);
-
-  RCLCPP_DEBUG(g_logger, std::string("message " "five"));
-  EXPECT_EQ("message five", g_last_log_event.message);
-
-  RCLCPP_DEBUG(g_logger, std::string("message %s"), "six");
-  EXPECT_EQ("message six", g_last_log_event.message);
-
-  RCLCPP_DEBUG(g_logger, "message seven");
-  EXPECT_EQ("message seven", g_last_log_event.message);
-}
-
 TEST_F(TestLoggingMacros, test_logging_stream) {
   for (std::string i : {"one", "two", "three"}) {
     RCLCPP_DEBUG_STREAM(g_logger, "message " << i);

rclcpp/test/rclcpp/test_memory_strategy.cpp

@@ -37,7 +37,8 @@ class TestWaitable : public rclcpp::Waitable
 public:
   bool add_to_wait_set(rcl_wait_set_t *) override {return true;}
   bool is_ready(rcl_wait_set_t *) override {return true;}
-  void execute() override {}
+  std::shared_ptr<void> take_data() override {return nullptr;}
+  void execute(std::shared_ptr<void> & data) override {(void)data;}
 };
 
 class TestMemoryStrategy : public ::testing::Test

rclcpp/test/rclcpp/test_node.cpp

@@ -43,6 +43,11 @@ protected:
     rclcpp::init(0, nullptr);
   }
 
+  static void TearDownTestCase()
+  {
+    rclcpp::shutdown();
+  }
+
   void SetUp() override
   {
     test_resources_path /= "test_node";

rclcpp/test/rclcpp/test_parameter_client.cpp

@@ -28,6 +28,8 @@
 
 #include "rcl_interfaces/msg/parameter_event.hpp"
 
+using namespace std::chrono_literals;
+
 class TestParameterClient : public ::testing::Test
 {
 public:
@@ -45,14 +47,20 @@ protected:
   void SetUp()
   {
     node = std::make_shared<rclcpp::Node>("test_parameter_client", "/ns");
+    node_with_option =
+      std::make_shared<rclcpp::Node>(
+      "test_parameter_client_allow_undeclare", "/ns",
+      rclcpp::NodeOptions().allow_undeclared_parameters(true));
   }
 
   void TearDown()
   {
     node.reset();
+    node_with_option.reset();
   }
 
   rclcpp::Node::SharedPtr node;
+  rclcpp::Node::SharedPtr node_with_option;
 };
 
 /*
@@ -287,7 +295,337 @@ TEST_F(TestParameterClient, sync_parameter_get_parameter_types) {
   auto synchronous_client = std::make_shared<rclcpp::SyncParametersClient>(node);
   std::vector<std::string> names{"foo"};
   std::vector<rclcpp::ParameterType> parameter_types =
-    synchronous_client->get_parameter_types(names);
+    synchronous_client->get_parameter_types(names, 10s);
   ASSERT_EQ(1u, parameter_types.size());
   ASSERT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_types[0]);
 }
+
+/*
+  Coverage for async describe_parameters
+ */
+TEST_F(TestParameterClient, async_parameter_describe_parameters) {
+  node->declare_parameter("foo", 4);
+  node->declare_parameter("bar", "this is bar");
+  auto asynchronous_client = std::make_shared<rclcpp::AsyncParametersClient>(node);
+
+  {
+    bool callback_called = false;
+    auto callback = [&callback_called](
+      std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> result)
+      {
+        // We expect the result to be empty since we tried to get a parameter that didn't exist.
+        if (result.valid() && result.get().size() == 0) {
+          callback_called = true;
+        }
+      };
+    std::vector<std::string> names{"none"};
+    std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> future =
+      asynchronous_client->describe_parameters(names, callback);
+    auto return_code = rclcpp::spin_until_future_complete(
+      node, future, std::chrono::milliseconds(100));
+    ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
+    ASSERT_TRUE(callback_called);
+  }
+
+  {
+    bool callback_called = false;
+    auto callback = [&callback_called](
+      std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> result)
+      {
+        if (result.valid() && result.get().size() == 1) {
+          callback_called = true;
+        }
+      };
+    std::vector<std::string> names{"foo"};
+    std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> future =
+      asynchronous_client->describe_parameters(names, callback);
+    auto return_code = rclcpp::spin_until_future_complete(
+      node, future, std::chrono::milliseconds(100));
+    ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs = future.get();
+    ASSERT_EQ(1u, parameter_descs.size());
+    ASSERT_EQ("foo", parameter_descs[0].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_descs[0].type);
+    ASSERT_EQ("", parameter_descs[0].description);
+    ASSERT_EQ("", parameter_descs[0].additional_constraints);
+    ASSERT_FALSE(parameter_descs[0].read_only);
+    ASSERT_TRUE(callback_called);
+  }
+
+  {
+    bool callback_called = false;
+    auto callback = [&callback_called](
+      std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> result)
+      {
+        // We expect the result to be empty since we tried to get a parameter that didn't exist.
+        if (result.valid() && result.get().size() == 0) {
+          callback_called = true;
+        }
+      };
+    std::vector<std::string> names{"foo", "baz"};
+    std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> future =
+      asynchronous_client->describe_parameters(names, callback);
+    auto return_code = rclcpp::spin_until_future_complete(
+      node, future, std::chrono::milliseconds(100));
+    ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
+    ASSERT_TRUE(callback_called);
+  }
+
+  {
+    bool callback_called = false;
+    auto callback = [&callback_called](
+      std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> result)
+      {
+        // We expect the result to be empty since we tried to get a parameter that didn't exist.
+        if (result.valid() && result.get().size() == 0) {
+          callback_called = true;
+        }
+      };
+    std::vector<std::string> names{"baz", "foo"};
+    std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> future =
+      asynchronous_client->describe_parameters(names, callback);
+    auto return_code = rclcpp::spin_until_future_complete(
+      node, future, std::chrono::milliseconds(100));
+    ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
+    ASSERT_TRUE(callback_called);
+  }
+
+  {
+    bool callback_called = false;
+    auto callback = [&callback_called](
+      std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> result)
+      {
+        if (result.valid() && result.get().size() == 2) {
+          callback_called = true;
+        }
+      };
+    std::vector<std::string> names{"foo", "bar"};
+    std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> future =
+      asynchronous_client->describe_parameters(names, callback);
+    auto return_code = rclcpp::spin_until_future_complete(
+      node, future, std::chrono::milliseconds(100));
+    ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs = future.get();
+    ASSERT_EQ(2u, parameter_descs.size());
+    ASSERT_EQ("foo", parameter_descs[0].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_descs[0].type);
+    ASSERT_EQ("", parameter_descs[0].description);
+    ASSERT_EQ("", parameter_descs[0].additional_constraints);
+    ASSERT_FALSE(parameter_descs[0].read_only);
+    ASSERT_EQ("bar", parameter_descs[1].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_STRING, parameter_descs[1].type);
+    ASSERT_EQ("", parameter_descs[1].description);
+    ASSERT_EQ("", parameter_descs[1].additional_constraints);
+    ASSERT_FALSE(parameter_descs[1].read_only);
+    ASSERT_TRUE(callback_called);
+  }
+}
+/*
+  Coverage for sync describe_parameters
+ */
+TEST_F(TestParameterClient, sync_parameter_describe_parameters) {
+  node->declare_parameter("foo", 4);
+  node->declare_parameter("bar", "this is bar");
+  auto synchronous_client = std::make_shared<rclcpp::SyncParametersClient>(node);
+
+  {
+    std::vector<std::string> names{"none"};
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs =
+      synchronous_client->describe_parameters(names, 10s);
+    ASSERT_EQ(0u, parameter_descs.size());
+  }
+
+  {
+    std::vector<std::string> names{"foo"};
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs =
+      synchronous_client->describe_parameters(names, 10s);
+    ASSERT_EQ(1u, parameter_descs.size());
+    ASSERT_EQ("foo", parameter_descs[0].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_descs[0].type);
+    ASSERT_EQ("", parameter_descs[0].description);
+    ASSERT_EQ("", parameter_descs[0].additional_constraints);
+    ASSERT_FALSE(parameter_descs[0].read_only);
+  }
+
+  {
+    std::vector<std::string> names{"foo", "baz"};
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs =
+      synchronous_client->describe_parameters(names, 10s);
+    ASSERT_EQ(0u, parameter_descs.size());
+  }
+
+  {
+    std::vector<std::string> names{"baz", "foo"};
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs =
+      synchronous_client->describe_parameters(names, 10s);
+    ASSERT_EQ(0u, parameter_descs.size());
+  }
+
+  {
+    std::vector<std::string> names{"foo", "bar"};
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs =
+      synchronous_client->describe_parameters(names, 10s);
+    ASSERT_EQ(2u, parameter_descs.size());
+    ASSERT_EQ("foo", parameter_descs[0].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_descs[0].type);
+    ASSERT_EQ("", parameter_descs[0].description);
+    ASSERT_EQ("", parameter_descs[0].additional_constraints);
+    ASSERT_FALSE(parameter_descs[0].read_only);
+    ASSERT_EQ("bar", parameter_descs[1].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_STRING, parameter_descs[1].type);
+    ASSERT_EQ("", parameter_descs[1].description);
+    ASSERT_EQ("", parameter_descs[1].additional_constraints);
+    ASSERT_FALSE(parameter_descs[1].read_only);
+  }
+}
+
+/*
+  Coverage for async describe_parameters with allow_undeclared_ enabled
+ */
+TEST_F(TestParameterClient, async_parameter_describe_parameters_allow_undeclared) {
+  node_with_option->declare_parameter("foo", 4);
+  node_with_option->declare_parameter("bar", "this is bar");
+  auto asynchronous_client =
+    std::make_shared<rclcpp::AsyncParametersClient>(node_with_option);
+
+  {
+    bool callback_called = false;
+    auto callback = [&callback_called](
+      std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> result)
+      {
+        // We expect the result to be defaut since we tried to get a parameter that didn't exist.
+        if (result.valid() && result.get().size() == 1) {
+          callback_called = true;
+        }
+      };
+    std::vector<std::string> names{"none"};
+    std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> future =
+      asynchronous_client->describe_parameters(names, callback);
+    auto return_code = rclcpp::spin_until_future_complete(
+      node_with_option, future, std::chrono::milliseconds(100));
+    ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs = future.get();
+    ASSERT_EQ(1u, parameter_descs.size());
+    ASSERT_EQ("none", parameter_descs[0].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_NOT_SET, parameter_descs[0].type);
+    ASSERT_EQ("", parameter_descs[0].description);
+    ASSERT_EQ("", parameter_descs[0].additional_constraints);
+    ASSERT_FALSE(parameter_descs[0].read_only);
+    ASSERT_TRUE(callback_called);
+  }
+
+  {
+    bool callback_called = false;
+    auto callback = [&callback_called](
+      std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> result)
+      {
+        if (result.valid() && result.get().size() == 2) {
+          callback_called = true;
+        }
+      };
+    std::vector<std::string> names{"foo", "baz"};
+    std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> future =
+      asynchronous_client->describe_parameters(names, callback);
+    auto return_code = rclcpp::spin_until_future_complete(
+      node_with_option, future, std::chrono::milliseconds(100));
+    ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs = future.get();
+    ASSERT_EQ(2u, parameter_descs.size());
+    ASSERT_EQ("foo", parameter_descs[0].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_descs[0].type);
+    ASSERT_EQ("", parameter_descs[0].description);
+    ASSERT_EQ("", parameter_descs[0].additional_constraints);
+    ASSERT_FALSE(parameter_descs[0].read_only);
+    ASSERT_EQ("baz", parameter_descs[1].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_NOT_SET, parameter_descs[1].type);
+    ASSERT_EQ("", parameter_descs[1].description);
+    ASSERT_EQ("", parameter_descs[1].additional_constraints);
+    ASSERT_FALSE(parameter_descs[1].read_only);
+    ASSERT_TRUE(callback_called);
+  }
+
+  {
+    bool callback_called = false;
+    auto callback = [&callback_called](
+      std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> result)
+      {
+        if (result.valid() && result.get().size() == 2) {
+          callback_called = true;
+        }
+      };
+    std::vector<std::string> names{"baz", "foo"};
+    std::shared_future<std::vector<rcl_interfaces::msg::ParameterDescriptor>> future =
+      asynchronous_client->describe_parameters(names, callback);
+    auto return_code = rclcpp::spin_until_future_complete(
+      node_with_option, future, std::chrono::milliseconds(100));
+    ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs = future.get();
+    ASSERT_EQ(2u, parameter_descs.size());
+    ASSERT_EQ("baz", parameter_descs[0].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_NOT_SET, parameter_descs[0].type);
+    ASSERT_EQ("", parameter_descs[0].description);
+    ASSERT_EQ("", parameter_descs[0].additional_constraints);
+    ASSERT_FALSE(parameter_descs[0].read_only);
+    ASSERT_EQ("foo", parameter_descs[1].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_descs[1].type);
+    ASSERT_EQ("", parameter_descs[1].description);
+    ASSERT_EQ("", parameter_descs[1].additional_constraints);
+    ASSERT_FALSE(parameter_descs[1].read_only);
+    ASSERT_TRUE(callback_called);
+  }
+}
+/*
+  Coverage for sync describe_parameters with allow_undeclared_ enabled
+ */
+TEST_F(TestParameterClient, sync_parameter_describe_parameters_allow_undeclared) {
+  node_with_option->declare_parameter("foo", 4);
+  node_with_option->declare_parameter("bar", "this is bar");
+  auto synchronous_client =
+    std::make_shared<rclcpp::SyncParametersClient>(node_with_option);
+
+  {
+    std::vector<std::string> names{"none"};
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs =
+      synchronous_client->describe_parameters(names, 10s);
+    ASSERT_EQ(1u, parameter_descs.size());
+    ASSERT_EQ("none", parameter_descs[0].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_NOT_SET, parameter_descs[0].type);
+    ASSERT_EQ("", parameter_descs[0].description);
+    ASSERT_EQ("", parameter_descs[0].additional_constraints);
+    ASSERT_FALSE(parameter_descs[0].read_only);
+  }
+
+  {
+    std::vector<std::string> names{"foo", "baz"};
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs =
+      synchronous_client->describe_parameters(names, 10s);
+    ASSERT_EQ(2u, parameter_descs.size());
+    ASSERT_EQ("foo", parameter_descs[0].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_descs[0].type);
+    ASSERT_EQ("", parameter_descs[0].description);
+    ASSERT_EQ("", parameter_descs[0].additional_constraints);
+    ASSERT_FALSE(parameter_descs[0].read_only);
+    ASSERT_EQ("baz", parameter_descs[1].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_NOT_SET, parameter_descs[1].type);
+    ASSERT_EQ("", parameter_descs[1].description);
+    ASSERT_EQ("", parameter_descs[1].additional_constraints);
+    ASSERT_FALSE(parameter_descs[1].read_only);
+  }
+
+  {
+    std::vector<std::string> names{"baz", "foo"};
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs =
+      synchronous_client->describe_parameters(names, 10s);
+    ASSERT_EQ(2u, parameter_descs.size());
+    ASSERT_EQ("baz", parameter_descs[0].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_NOT_SET, parameter_descs[0].type);
+    ASSERT_EQ("", parameter_descs[0].description);
+    ASSERT_EQ("", parameter_descs[0].additional_constraints);
+    ASSERT_FALSE(parameter_descs[0].read_only);
+    ASSERT_EQ("foo", parameter_descs[1].name);
+    ASSERT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_descs[1].type);
+    ASSERT_EQ("", parameter_descs[1].description);
+    ASSERT_EQ("", parameter_descs[1].additional_constraints);
+    ASSERT_FALSE(parameter_descs[1].read_only);
+  }
+}

rclcpp/test/rclcpp/test_parameter_service.cpp

@@ -22,6 +22,8 @@
 #include "rclcpp/rclcpp.hpp"
 #include "../../src/rclcpp/parameter_service_names.hpp"
 
+using namespace std::chrono_literals;
+
 // This tests the ParameterService as it is included in an rclcpp::Node. Creating a separate
 // ParameterService would interfere with the built-in one
 class TestParameterService : public ::testing::Test
@@ -59,12 +61,12 @@ TEST_F(TestParameterService, get_parameter_types) {
   node->declare_parameter("parameter1", rclcpp::ParameterValue(42));
 
   const std::vector<std::string> declared_parameters = {"parameter1"};
-  const auto parameter_types = client->get_parameter_types(declared_parameters);
+  const auto parameter_types = client->get_parameter_types(declared_parameters, 10s);
   ASSERT_EQ(1u, parameter_types.size());
   EXPECT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_types[0]);
 
   const std::vector<std::string> undeclared_parameters = {"parameter2"};
-  const auto undeclared_parameter_types = client->get_parameter_types(undeclared_parameters);
+  const auto undeclared_parameter_types = client->get_parameter_types(undeclared_parameters, 10s);
   EXPECT_EQ(0u, undeclared_parameter_types.size());
 }
 
@@ -75,7 +77,7 @@ TEST_F(TestParameterService, set_parameters) {
   const std::vector<rclcpp::Parameter> parameters = {
     rclcpp::Parameter("parameter1", 0),
   };
-  client->set_parameters(parameters);
+  client->set_parameters(parameters, 10s);
   EXPECT_EQ(0, client->get_parameter("parameter1", 100));
 }
 
@@ -86,54 +88,34 @@ TEST_F(TestParameterService, set_parameters_atomically) {
   const std::vector<rclcpp::Parameter> parameters = {
     rclcpp::Parameter("parameter1", 0),
   };
-  client->set_parameters_atomically(parameters);
+  client->set_parameters_atomically(parameters, 10s);
   EXPECT_EQ(0, client->get_parameter("parameter1", 100));
 }
 
 TEST_F(TestParameterService, list_parameters) {
-  const size_t number_parameters_in_basic_node = client->list_parameters({}, 1).names.size();
+  const size_t number_parameters_in_basic_node = client->list_parameters({}, 1, 10s).names.size();
   node->declare_parameter("parameter1", rclcpp::ParameterValue(42));
 
-  const auto list_result = client->list_parameters({}, 1);
+  const auto list_result = client->list_parameters({}, 1, 10s);
   EXPECT_EQ(1u + number_parameters_in_basic_node, list_result.names.size());
 }
 
 TEST_F(TestParameterService, describe_parameters) {
-  // There is no current API in ParameterClient for calling the describe_parameters service
-  // Update this test when https://github.com/ros2/rclcpp/issues/1354 is resolved
-
-  const std::string describe_parameters_service_name =
-    node->get_fully_qualified_name() + std::string("/") +
-    rclcpp::parameter_service_names::describe_parameters;
-  using ServiceT = rcl_interfaces::srv::DescribeParameters;
-  auto client =
-    node->create_client<ServiceT>(describe_parameters_service_name);
-
-  ASSERT_TRUE(client->wait_for_service(std::chrono::seconds(1)));
   node->declare_parameter("parameter1", rclcpp::ParameterValue(42));
 
   {
-    auto request = std::make_shared<rcl_interfaces::srv::DescribeParameters::Request>();
-    request->names.push_back("parameter1");
-    auto future = client->async_send_request(request);
-
-    rclcpp::spin_until_future_complete(node, future, std::chrono::seconds(1));
-    auto response = future.get();
-    ASSERT_NE(nullptr, response);
-    EXPECT_EQ(1u, response->descriptors.size());
-
-    auto descriptor = response->descriptors[0];
-    EXPECT_EQ("parameter1", descriptor.name);
-    EXPECT_EQ(rcl_interfaces::msg::ParameterType::PARAMETER_INTEGER, descriptor.type);
+    const std::vector<std::string> names{"parameter1"};
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs =
+      client->describe_parameters(names, 10s);
+    ASSERT_EQ(1u, parameter_descs.size());
+    EXPECT_EQ("parameter1", parameter_descs[0].name);
+    EXPECT_EQ(rclcpp::ParameterType::PARAMETER_INTEGER, parameter_descs[0].type);
   }
-  {
-    auto request = std::make_shared<rcl_interfaces::srv::DescribeParameters::Request>();
-    request->names.push_back("undeclared_parameter");
-    auto future = client->async_send_request(request);
 
-    rclcpp::spin_until_future_complete(node, future, std::chrono::seconds(1));
-    auto response = future.get();
-    ASSERT_NE(nullptr, response);
-    EXPECT_EQ(0u, response->descriptors.size());
+  {
+    const std::vector<std::string> names{"undeclared_parameter"};
+    std::vector<rcl_interfaces::msg::ParameterDescriptor> parameter_descs =
+      client->describe_parameters(names, 10s);
+    EXPECT_EQ(0u, parameter_descs.size());
   }
 }

rclcpp/test/rclcpp/test_publisher.cpp

@@ -151,6 +151,20 @@ TEST_F(TestPublisher, various_creation_signatures) {
       rclcpp::create_publisher<Empty>(node, "topic", 42, rclcpp::PublisherOptions());
     (void)publisher;
   }
+  {
+    rclcpp::PublisherOptions options;
+    options.qos_overriding_options = rclcpp::QosOverridingOptions::with_default_policies();
+    auto publisher =
+      rclcpp::create_publisher<Empty>(node, "topic", 42, options);
+    (void)publisher;
+  }
+  {
+    auto node_parameters = node->get_node_parameters_interface();
+    auto node_topics = node->get_node_topics_interface();
+    auto publisher = rclcpp::create_publisher<Empty>(
+      node_parameters, node_topics, "topic", 42, rclcpp::PublisherOptions());
+    (void)publisher;
+  }
 }
 
 /*
@@ -489,7 +503,9 @@ TEST_F(TestPublisher, default_incompatible_qos_callback) {
 TEST_F(TestPublisher, run_event_handlers) {
   initialize();
   auto publisher = node->create_publisher<test_msgs::msg::Empty>("topic", 10);
+
   for (const auto & handler : publisher->get_event_handlers()) {
-    EXPECT_NO_THROW(handler->execute());
+    std::shared_ptr<void> data = handler->take_data();
+    handler->execute(data);
   }
 }

rclcpp/test/rclcpp/test_qos.cpp

@@ -78,33 +78,41 @@ TEST(TestQoS, equality_namespace) {
   EXPECT_NE(a, b);
 }
 
-TEST(TestQoS, setters) {
+TEST(TestQoS, setters_and_getters) {
   rclcpp::QoS qos(10);
 
   qos.keep_all();
-  EXPECT_EQ(RMW_QOS_POLICY_HISTORY_KEEP_ALL, qos.get_rmw_qos_profile().history);
+  EXPECT_EQ(rclcpp::HistoryPolicy::KeepAll, qos.history());
 
   qos.keep_last(20);
-  EXPECT_EQ(RMW_QOS_POLICY_HISTORY_KEEP_LAST, qos.get_rmw_qos_profile().history);
-  EXPECT_EQ(20u, qos.get_rmw_qos_profile().depth);
+  EXPECT_EQ(rclcpp::HistoryPolicy::KeepLast, qos.history());
+  EXPECT_EQ(20u, qos.depth());
 
   qos.reliable();
-  EXPECT_EQ(RMW_QOS_POLICY_RELIABILITY_RELIABLE, qos.get_rmw_qos_profile().reliability);
+  EXPECT_EQ(rclcpp::ReliabilityPolicy::Reliable, qos.reliability());
+
+  qos.reliability(rclcpp::ReliabilityPolicy::BestEffort);
+  EXPECT_EQ(rclcpp::ReliabilityPolicy::BestEffort, qos.reliability());
 
   qos.durability_volatile();
-  EXPECT_EQ(RMW_QOS_POLICY_DURABILITY_VOLATILE, qos.get_rmw_qos_profile().durability);
+  EXPECT_EQ(rclcpp::DurabilityPolicy::Volatile, qos.durability());
 
   qos.transient_local();
-  EXPECT_EQ(RMW_QOS_POLICY_DURABILITY_TRANSIENT_LOCAL, qos.get_rmw_qos_profile().durability);
+  EXPECT_EQ(rclcpp::DurabilityPolicy::TransientLocal, qos.durability());
+
+  qos.durability(rclcpp::DurabilityPolicy::Volatile);
+  EXPECT_EQ(rclcpp::DurabilityPolicy::Volatile, qos.durability());
 
   qos.history(RMW_QOS_POLICY_HISTORY_KEEP_ALL);
-  EXPECT_EQ(RMW_QOS_POLICY_HISTORY_KEEP_ALL, qos.get_rmw_qos_profile().history);
+  EXPECT_EQ(rclcpp::HistoryPolicy::KeepAll, qos.history());
 
-  constexpr size_t duration_ns = 12345;
+  qos.history(rclcpp::HistoryPolicy::KeepLast);
+  EXPECT_EQ(rclcpp::HistoryPolicy::KeepLast, qos.history());
+
+  constexpr rcl_duration_value_t duration_ns = 12345;
   constexpr std::chrono::nanoseconds duration(duration_ns);
   qos.deadline(duration);
-  EXPECT_EQ(0u, qos.get_rmw_qos_profile().deadline.sec);
-  EXPECT_EQ(duration_ns, qos.get_rmw_qos_profile().deadline.nsec);
+  EXPECT_EQ(duration_ns, qos.deadline().nanoseconds());
 
   const rmw_time_t rmw_time {0, 54321};
   qos.deadline(rmw_time);
@@ -112,28 +120,29 @@ TEST(TestQoS, setters) {
   EXPECT_EQ(rmw_time.nsec, qos.get_rmw_qos_profile().deadline.nsec);
 
   qos.lifespan(duration);
-  EXPECT_EQ(0u, qos.get_rmw_qos_profile().lifespan.sec);
-  EXPECT_EQ(duration_ns, qos.get_rmw_qos_profile().lifespan.nsec);
+  EXPECT_EQ(duration_ns, qos.lifespan().nanoseconds());
 
   qos.lifespan(rmw_time);
   EXPECT_EQ(rmw_time.sec, qos.get_rmw_qos_profile().lifespan.sec);
   EXPECT_EQ(rmw_time.nsec, qos.get_rmw_qos_profile().lifespan.nsec);
 
   qos.liveliness(RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC);
-  EXPECT_EQ(RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC, qos.get_rmw_qos_profile().liveliness);
+  EXPECT_EQ(rclcpp::LivelinessPolicy::ManualByTopic, qos.liveliness());
+
+  qos.liveliness(rclcpp::LivelinessPolicy::Automatic);
+  EXPECT_EQ(rclcpp::LivelinessPolicy::Automatic, qos.liveliness());
 
   qos.liveliness_lease_duration(duration);
-  EXPECT_EQ(0u, qos.get_rmw_qos_profile().liveliness_lease_duration.sec);
-  EXPECT_EQ(duration_ns, qos.get_rmw_qos_profile().liveliness_lease_duration.nsec);
+  EXPECT_EQ(duration_ns, qos.liveliness_lease_duration().nanoseconds());
 
   qos.liveliness_lease_duration(rmw_time);
   EXPECT_EQ(rmw_time.sec, qos.get_rmw_qos_profile().liveliness_lease_duration.sec);
   EXPECT_EQ(rmw_time.nsec, qos.get_rmw_qos_profile().liveliness_lease_duration.nsec);
 
   qos.avoid_ros_namespace_conventions(true);
-  EXPECT_TRUE(qos.get_rmw_qos_profile().avoid_ros_namespace_conventions);
+  EXPECT_TRUE(qos.avoid_ros_namespace_conventions());
   qos.avoid_ros_namespace_conventions(false);
-  EXPECT_FALSE(qos.get_rmw_qos_profile().avoid_ros_namespace_conventions);
+  EXPECT_FALSE(qos.avoid_ros_namespace_conventions());
 }
 
 bool operator==(const rmw_qos_profile_t & lhs, const rmw_qos_profile_t & rhs)

rclcpp/test/rclcpp/test_qos_event.cpp

@@ -285,14 +285,16 @@ TEST_F(TestQosEvent, execute) {
   rclcpp::QOSEventHandler<decltype(callback), decltype(rcl_handle)> handler(
     callback, rcl_publisher_event_init, rcl_handle, event_type);
 
-  EXPECT_NO_THROW(handler.execute());
+  std::shared_ptr<void> data = handler.take_data();
+  EXPECT_NO_THROW(handler.execute(data));
   EXPECT_TRUE(handler_callback_executed);
 
   {
     handler_callback_executed = false;
     // Logs error and returns early
     auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_take_event, RCL_RET_ERROR);
-    EXPECT_NO_THROW(handler.execute());
+    std::shared_ptr<void> data = handler.take_data();
+    EXPECT_THROW(handler.execute(data), std::runtime_error);
     EXPECT_FALSE(handler_callback_executed);
   }
 }

rclcpp/test/rclcpp/test_qos_overriding_options.cpp

@@ -0,0 +1,34 @@
+// 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 "gmock/gmock.h"
+
+#include "rclcpp/qos_overriding_options.hpp"
+
+TEST(TestQosOverridingOptions, test_overriding_options) {
+  auto options = rclcpp::QosOverridingOptions::with_default_policies();
+  EXPECT_EQ(options.get_id(), "");
+  EXPECT_EQ(options.get_validation_callback(), nullptr);
+  EXPECT_THAT(
+    options.get_policy_kinds(), testing::ElementsAre(
+      rclcpp::QosPolicyKind::History,
+      rclcpp::QosPolicyKind::Depth,
+      rclcpp::QosPolicyKind::Reliability));
+}
+
+TEST(TestQosOverridingOptions, test_qos_policy_kind_to_cstr) {
+  EXPECT_THROW(
+    rclcpp::qos_policy_kind_to_cstr(rclcpp::QosPolicyKind::Invalid),
+    std::invalid_argument);
+}

rclcpp/test/rclcpp/test_qos_parameters.cpp

@@ -0,0 +1,256 @@
+// 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 <map>
+#include <memory>
+#include <string>
+
+#include "gmock/gmock.h"
+
+#include "rclcpp/rclcpp.hpp"
+#include "rclcpp/qos_overriding_options.hpp"
+#include "rclcpp/detail/qos_parameters.hpp"
+
+TEST(TestQosParameters, declare) {
+  rclcpp::init(0, nullptr);
+  auto node = std::make_shared<rclcpp::Node>(
+    "my_node", "/ns", rclcpp::NodeOptions().parameter_overrides(
+  {
+    rclcpp::Parameter(
+      "qos_overrides./my/fully/qualified/topic_name.publisher.reliability", "best_effort"),
+  }));
+
+  for (size_t i = 0; i < 2; ++i) {
+    // The first iteration will declare parameters, the second will get the previosuly declared
+    // ones, check both have the same result.
+    rclcpp::QoS qos{rclcpp::KeepLast(10)};
+    qos = rclcpp::detail::declare_qos_parameters(
+      rclcpp::QosOverridingOptions::with_default_policies(),
+      node,
+      "/my/fully/qualified/topic_name",
+      qos,
+      rclcpp::detail::PublisherQosParametersTraits{});
+
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.publisher.history").get_value<std::string>(),
+      "keep_last");
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.publisher.depth").get_value<int64_t>(),
+      10);
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.publisher.reliability"
+      ).get_value<std::string>(),
+      "best_effort");
+    EXPECT_EQ(RMW_QOS_POLICY_HISTORY_KEEP_LAST, qos.get_rmw_qos_profile().history);
+    EXPECT_EQ(RMW_QOS_POLICY_RELIABILITY_BEST_EFFORT, qos.get_rmw_qos_profile().reliability);
+    EXPECT_EQ(10u, qos.get_rmw_qos_profile().depth);
+
+    std::map<std::string, rclcpp::Parameter> qos_params;
+    EXPECT_TRUE(
+      node->get_node_parameters_interface()->get_parameters_by_prefix(
+        "qos_overrides./my/fully/qualified/topic_name.publisher", qos_params));
+    EXPECT_EQ(3u, qos_params.size());
+  }
+
+  rclcpp::shutdown();
+}
+
+TEST(TestQosParameters, declare_with_callback) {
+  rclcpp::init(0, nullptr);
+  auto node = std::make_shared<rclcpp::Node>(
+    "my_node", "/ns", rclcpp::NodeOptions().parameter_overrides(
+  {
+    rclcpp::Parameter(
+      "qos_overrides./my/fully/qualified/topic_name.publisher.reliability", "best_effort"),
+  }));
+
+  rclcpp::QoS qos{rclcpp::KeepLast(10)};
+  // *INDENT-OFF*, uncrustify suggestion makes the code unreadable
+  EXPECT_THROW(
+    rclcpp::detail::declare_qos_parameters(
+      {
+        {rclcpp::QosPolicyKind::Lifespan},
+        [](const rclcpp::QoS &) {
+          return rclcpp::QosCallbackResult{};
+        }
+      },
+      node,
+      "/my/fully/qualified/topic_name/fails_validation",
+      qos,
+      rclcpp::detail::PublisherQosParametersTraits{}),
+    rclcpp::exceptions::InvalidQosOverridesException);
+
+  rclcpp::detail::declare_qos_parameters(
+    rclcpp::QosOverridingOptions::with_default_policies([](const rclcpp::QoS &) {
+      rclcpp::QosCallbackResult result;
+      result.successful = true;
+      return result;
+    }),
+    node,
+    "/my/fully/qualified/topic_name",
+    qos,
+    rclcpp::detail::PublisherQosParametersTraits{});
+  //  *INDENT-ON*
+
+  rclcpp::shutdown();
+}
+
+constexpr int64_t kDuration{1000000};
+
+TEST(TestQosParameters, declare_qos_subscription_parameters) {
+  rclcpp::init(0, nullptr);
+  auto node = std::make_shared<rclcpp::Node>(
+    "my_node", "/ns", rclcpp::NodeOptions().parameter_overrides(
+  {
+    rclcpp::Parameter(
+      "qos_overrides./my/fully/qualified/topic_name.subscription.reliability", "best_effort"),
+    rclcpp::Parameter(
+      "qos_overrides./my/fully/qualified/topic_name.subscription.deadline", kDuration),
+    rclcpp::Parameter(
+      "qos_overrides./my/fully/qualified/topic_name.subscription.liveliness_lease_duration",
+      kDuration),
+  }));
+
+  for (size_t i = 0; i < 2; ++i) {
+    // The first iteration will declare parameters, the second will get the previosuly declared
+    // ones, check both have the same result.
+    rclcpp::QoS qos{rclcpp::KeepLast(10)};
+    qos = rclcpp::detail::declare_qos_parameters(
+    {
+      rclcpp::QosPolicyKind::AvoidRosNamespaceConventions, rclcpp::QosPolicyKind::Deadline,
+      rclcpp::QosPolicyKind::Depth, rclcpp::QosPolicyKind::Durability,
+      // lifespan will be ignored
+      rclcpp::QosPolicyKind::History, rclcpp::QosPolicyKind::Lifespan,
+      rclcpp::QosPolicyKind::Liveliness, rclcpp::QosPolicyKind::LivelinessLeaseDuration,
+      rclcpp::QosPolicyKind::Reliability
+    },
+      node,
+      "/my/fully/qualified/topic_name",
+      qos,
+      rclcpp::detail::SubscriptionQosParametersTraits{});
+
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.subscription.avoid_ros_namespace_conventions"
+      ).get_value<bool>(), false);
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.subscription.deadline"
+      ).get_value<int64_t>(), kDuration);
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.subscription.depth"
+      ).get_value<int64_t>(), 10);
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.subscription.durability"
+      ).get_value<std::string>(), "volatile");
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.subscription.history"
+      ).get_value<std::string>(), "keep_last");
+    EXPECT_FALSE(
+      node->has_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.subscription.lifespan"));
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.subscription.liveliness"
+      ).get_value<std::string>(), "system_default");
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.subscription.liveliness_lease_duration"
+      ).get_value<int64_t>(), kDuration);
+    EXPECT_EQ(
+      node->get_parameter(
+        "qos_overrides./my/fully/qualified/topic_name.subscription.reliability"
+      ).get_value<std::string>(),
+      "best_effort");
+
+    std::map<std::string, rclcpp::Parameter> qos_params;
+    EXPECT_TRUE(
+      node->get_node_parameters_interface()->get_parameters_by_prefix(
+        "qos_overrides./my/fully/qualified/topic_name.subscription", qos_params));
+    EXPECT_EQ(8u, qos_params.size());
+  }
+  rclcpp::shutdown();
+}
+
+TEST(TestQosParameters, declare_with_id) {
+  rclcpp::init(0, nullptr);
+  auto node = std::make_shared<rclcpp::Node>("my_node", "/ns");
+
+  rclcpp::QoS qos{rclcpp::KeepLast{10}};
+  qos = rclcpp::detail::declare_qos_parameters(
+    rclcpp::QosOverridingOptions::with_default_policies(nullptr, "my_id"),
+    node,
+    "/my/fully/qualified/topic_name",
+    qos,
+    rclcpp::detail::PublisherQosParametersTraits{});
+
+  std::map<std::string, rclcpp::Parameter> qos_params;
+  EXPECT_TRUE(
+    node->get_node_parameters_interface()->get_parameters_by_prefix(
+      "qos_overrides./my/fully/qualified/topic_name.publisher_my_id", qos_params));
+  EXPECT_EQ(3u, qos_params.size());
+
+  rclcpp::shutdown();
+}
+
+TEST(TestQosParameters, declare_no_parameters_interface) {
+  rclcpp::init(0, nullptr);
+  auto node = std::make_shared<rclcpp::Node>("my_node", "/ns");
+
+  rclcpp::QoS qos{rclcpp::KeepLast{10}};
+  auto node_base_interface = node->get_node_base_interface();
+  EXPECT_THROW(
+    rclcpp::detail::declare_qos_parameters(
+      rclcpp::QosOverridingOptions::with_default_policies(),
+      node_base_interface,
+      "/my/fully/qualified/topic_name",
+      qos,
+      rclcpp::detail::PublisherQosParametersTraits{}),
+    std::runtime_error);
+
+  qos = rclcpp::detail::declare_qos_parameters(
+    rclcpp::QosOverridingOptions{},
+    node_base_interface,
+    "/my/fully/qualified/topic_name",
+    qos,
+    rclcpp::detail::PublisherQosParametersTraits{});
+
+  std::map<std::string, rclcpp::Parameter> qos_params;
+  EXPECT_FALSE(
+    node->get_node_parameters_interface()->get_parameters_by_prefix("qos_overrides", qos_params));
+
+  rclcpp::shutdown();
+}
+
+TEST(TestQosParameters, internal_functions_failure_modes) {
+  rclcpp::QoS qos{rclcpp::KeepLast{10}};
+  EXPECT_THROW(
+    rclcpp::detail::apply_qos_override(
+      rclcpp::QosPolicyKind::Invalid, rclcpp::ParameterValue{}, qos),
+    std::invalid_argument);
+  EXPECT_THROW(
+    rclcpp::detail::get_default_qos_param_value(
+      rclcpp::QosPolicyKind::Invalid, qos),
+    std::invalid_argument);
+  EXPECT_THROW(
+    rclcpp::detail::check_if_stringified_policy_is_null(
+      nullptr, rclcpp::QosPolicyKind::Reliability),
+    std::invalid_argument);
+}

rclcpp/test/rclcpp/test_utilities.cpp

@@ -131,59 +131,6 @@ MOCKING_UTILS_BOOL_OPERATOR_RETURNS_FALSE(rcl_guard_condition_options_t, !=)
 MOCKING_UTILS_BOOL_OPERATOR_RETURNS_FALSE(rcl_guard_condition_options_t, >)
 MOCKING_UTILS_BOOL_OPERATOR_RETURNS_FALSE(rcl_guard_condition_options_t, <)
 
-TEST(TestUtilities, test_context_release_interrupt_guard_condition) {
-  auto context1 = std::make_shared<rclcpp::contexts::DefaultContext>();
-  context1->init(0, nullptr);
-  RCLCPP_SCOPE_EXIT(rclcpp::shutdown(context1););
-
-  rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
-  rcl_ret_t ret = rcl_wait_set_init(
-    &wait_set, 0, 2, 0, 0, 0, 0, context1->get_rcl_context().get(),
-    rcl_get_default_allocator());
-  ASSERT_EQ(RCL_RET_OK, ret);
-
-  // Expected usage
-  rcl_guard_condition_t * interrupt_guard_condition =
-    context1->get_interrupt_guard_condition(&wait_set);
-  EXPECT_NE(nullptr, interrupt_guard_condition);
-  EXPECT_NO_THROW(context1->release_interrupt_guard_condition(&wait_set));
-
-  {
-    auto mock = mocking_utils::patch_and_return(
-      "lib:rclcpp", rcl_guard_condition_init, RCL_RET_ERROR);
-    EXPECT_THROW(
-      {interrupt_guard_condition = context1->get_interrupt_guard_condition(&wait_set);},
-      rclcpp::exceptions::RCLError);
-  }
-
-  {
-    interrupt_guard_condition = context1->get_interrupt_guard_condition(&wait_set);
-    auto mock = mocking_utils::inject_on_return(
-      "lib:rclcpp", rcl_guard_condition_fini, RCL_RET_ERROR);
-    EXPECT_THROW(
-      {context1->release_interrupt_guard_condition(&wait_set);},
-      rclcpp::exceptions::RCLError);
-  }
-
-  {
-    interrupt_guard_condition = context1->get_interrupt_guard_condition(&wait_set);
-    auto mock = mocking_utils::inject_on_return(
-      "lib:rclcpp", rcl_guard_condition_fini, RCL_RET_ERROR);
-    EXPECT_NO_THROW({context1->release_interrupt_guard_condition(&wait_set, std::nothrow);});
-  }
-
-  {
-    EXPECT_THROW(
-      context1->release_interrupt_guard_condition(nullptr),
-      std::runtime_error);
-  }
-
-  // Test it works after restore mocks
-  interrupt_guard_condition = context1->get_interrupt_guard_condition(&wait_set);
-  EXPECT_NE(nullptr, interrupt_guard_condition);
-  EXPECT_NO_THROW(context1->release_interrupt_guard_condition(&wait_set));
-}
-
 TEST(TestUtilities, test_context_init_shutdown_fails) {
   {
     auto context = std::make_shared<rclcpp::contexts::DefaultContext>();

rclcpp/test/rclcpp/wait_set_policies/test_dynamic_storage.cpp

@@ -55,7 +55,10 @@ public:
 
   bool is_ready(rcl_wait_set_t *) override {return is_ready_;}
 
-  void execute() override {}
+  std::shared_ptr<void> take_data() override {return nullptr;}
+
+  void
+  execute(std::shared_ptr<void> & data) override {(void)data;}
 
   void set_is_ready(bool value) {is_ready_ = value;}
 

rclcpp/test/rclcpp/wait_set_policies/test_static_storage.cpp

@@ -54,7 +54,10 @@ public:
 
   bool is_ready(rcl_wait_set_t *) override {return is_ready_;}
 
-  void execute() override {}
+  std::shared_ptr<void> take_data() override {return nullptr;}
+
+  void
+  execute(std::shared_ptr<void> & data) override {(void)data;}
 
   void set_is_ready(bool value) {is_ready_ = value;}
 

rclcpp/test/rclcpp/wait_set_policies/test_storage_policy_common.cpp

@@ -54,7 +54,10 @@ public:
 
   bool is_ready(rcl_wait_set_t *) override {return is_ready_;}
 
-  void execute() override {}
+  std::shared_ptr<void> take_data() override {return nullptr;}
+
+  void
+  execute(std::shared_ptr<void> & data) override {(void)data;}
 
   void set_is_ready(bool value) {is_ready_ = value;}
 

rclcpp/test/rclcpp/wait_set_policies/test_thread_safe_synchronization.cpp

@@ -54,7 +54,10 @@ public:
 
   bool is_ready(rcl_wait_set_t *) override {return is_ready_;}
 
-  void execute() override {}
+  std::shared_ptr<void> take_data() override {return nullptr;}
+
+  void
+  execute(std::shared_ptr<void> & data) override {(void)data;}
 
   void set_is_ready(bool value) {is_ready_ = value;}
 

rclcpp_action/CHANGELOG.rst

@@ -3,6 +3,30 @@ Changelog for package rclcpp_action
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+6.3.0 (2021-01-25)
+------------------
+* Fix action server deadlock (`#1285 <https://github.com/ros2/rclcpp/issues/1285>`_) (`#1313 <https://github.com/ros2/rclcpp/issues/1313>`_)
+* Contributors: Daisuke Sato
+
+6.2.0 (2021-01-08)
+------------------
+* Goal response callback compatibility shim with deprecation of old signature (`#1495 <https://github.com/ros2/rclcpp/issues/1495>`_)
+* [rclcpp_action] Add warnings (`#1405 <https://github.com/ros2/rclcpp/issues/1405>`_)
+* Contributors: Audrow Nash, Ivan Santiago Paunovic
+
+6.1.0 (2020-12-10)
+------------------
+* Update QDs to QL 1 (`#1477 <https://github.com/ros2/rclcpp/issues/1477>`_)
+* Contributors: Stephen Brawner
+
+6.0.0 (2020-11-18)
+------------------
+* Add `take_data` to `Waitable` and `data` to `AnyExecutable` (`#1241 <https://github.com/ros2/rclcpp/issues/1241>`_)
+* Fix test crashes on CentOS 7 (`#1449 <https://github.com/ros2/rclcpp/issues/1449>`_)
+* Bump rclcpp packages to Quality Level 2 (`#1445 <https://github.com/ros2/rclcpp/issues/1445>`_)
+* Add rclcpp_action action_server benchmarks (`#1433 <https://github.com/ros2/rclcpp/issues/1433>`_)
+* Contributors: Audrow Nash, Chris Lalancette, Louise Poubel, brawner
+
 5.1.0 (2020-11-02)
 ------------------
 * Benchmark rclcpp_action action_client (`#1429 <https://github.com/ros2/rclcpp/issues/1429>`_)

rclcpp_action/CMakeLists.txt

@@ -13,7 +13,10 @@ if(NOT CMAKE_CXX_STANDARD)
   set(CMAKE_CXX_STANDARD 14)
 endif()
 if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-  add_compile_options(-Wall -Wextra -Wpedantic -Wnon-virtual-dtor -Woverloaded-virtual)
+  add_compile_options(
+    -Wall -Wextra -Wpedantic -Wnon-virtual-dtor -Woverloaded-virtual
+    -Wformat=2 -Wconversion -Wshadow -Wsign-conversion -Wcast-qual
+  )
 endif()
 
 set(${PROJECT_NAME}_SRCS

rclcpp_action/QUALITY_DECLARATION.md

@@ -2,7 +2,7 @@ This document is a declaration of software quality for the `rclcpp_action` packa
 
 # rclcpp_action Quality Declaration
 
-The package `rclcpp_action` claims to be in the **Quality Level 2** category.
+The package `rclcpp_action` claims to be in the **Quality Level 1** category when it is used with a **Quality Level 1** middleware.
 
 Below are the rationales, notes, and caveats for this claim, organized by each requirement listed in the [Package Quality Categories in REP-2004](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#package-quality-categories) of the ROS2 developer guide.
 
@@ -80,13 +80,13 @@ The API is publicly available in its [ROS 2 API documentation](http://docs.ros2.
 
 The license for `rclcpp_action` is Apache 2.0, and a summary is in each source file, the type is declared in the [`package.xml`](./package.xml) manifest file, and a full copy of the license is in the [`LICENSE`](../LICENSE) file.
 
-There is an automated test which runs a linter that ensures each file has a license statement. [Here](http://build.ros2.org/view/Fpr/job/Fpr__rclcpp__ubuntu_focal_amd64/lastSuccessfulBuild/testReport/rclcpp_action/) can be found a list with the latest results of the various linters being run on the package.
+There is an automated test which runs a linter that ensures each file has a license statement. [Here](http://build.ros2.org/view/Rpr/job/Rpr__rclcpp__ubuntu_focal_amd64/lastCompletedBuild/testReport/rclcpp_action/) can be found a list with the latest results of the various linters being run on the package.
 
 ### Copyright Statements [3.iv]
 
 The copyright holders each provide a statement of copyright in each source code file in `rclcpp_action`.
 
-There is an automated test which runs a linter that ensures each file has at least one copyright statement. Latest linter result report can be seen [here](http://build.ros2.org/view/Fpr/job/Fpr__rclcpp__ubuntu_focal_amd64/lastSuccessfulBuild/testReport/rclcpp_action/copyright/).
+There is an automated test which runs a linter that ensures each file has at least one copyright statement. Latest linter result report can be seen [here](http://build.ros2.org/view/Rpr/job/Rpr__rclcpp__ubuntu_focal_amd64/lastCompletedBuild/testReport/rclcpp_action/copyright/).
 
 ## Testing [4]
 
@@ -119,11 +119,19 @@ This includes:
 
 Changes are required to make a best effort to keep or increase coverage before being accepted, but decreases are allowed if properly justified and accepted by reviewers.
 
-Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastSuccessfulBuild/cobertura/src_ros2_rclcpp_rclcpp_action_src/). A description of how coverage statistics are calculated is summarized in this page ["ROS 2 Onboarding Guide"](https://index.ros.org/doc/ros2/Contributing/ROS-2-On-boarding-Guide/#note-on-coverage-runs).
+Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastCompletedBuild/cobertura/src_ros2_rclcpp_rclcpp_action_src/). A description of how coverage statistics are calculated is summarized in this page ["ROS 2 Onboarding Guide"](https://index.ros.org/doc/ros2/Contributing/ROS-2-On-boarding-Guide/#note-on-coverage-runs).
 
 ### Performance [4.iv]
 
-It is not yet defined if this package requires performance testing and how addresses this topic.
+`rclcpp_action` follows the recommendations for performance testing of C/C++ code in the [ROS 2 Developer Guide](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#performance), and opts to do performance analysis on each release rather than each change.
+
+The performance tests of `rclcpp_action` are located in the [test/benchmark directory](https://github.com/ros2/rclcpp/tree/master/rclcpp_action/test/benchmark).
+
+System level performance benchmarks that cover features of `rclcpp_action` can be found at:
+* [Benchmarks](http://build.ros2.org/view/Rci/job/Rci__benchmark_ubuntu_focal_amd64/BenchmarkTable/)
+* [Performance](http://build.ros2.org/view/Rci/job/Rci__nightly-performance_ubuntu_focal_amd64/lastCompletedBuild/)
+
+Changes that introduce regressions in performance must be adequately justified in order to be accepted and merged.
 
 ### Linters and Static Analysis [4.v]
 
@@ -151,19 +159,19 @@ It also has several test dependencies, which do not affect the resulting quality
 
 `action_msgs` provides messages and services for ROS 2 actions.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcl_interfaces/blob/master/action_msgs/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl_interfaces/blob/master/action_msgs/QUALITY_DECLARATION.md).
 
 #### `rclcpp`
 
 The `rclcpp` package provides the ROS client library in C++.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rclcpp/blob/master/rclcpp/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rclcpp/blob/master/rclcpp/QUALITY_DECLARATION.md).
 
 #### `rcl_action`
 
 The `rcl_action` package provides C-based ROS action implementation.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcl/blob/master/rcl_action/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl/blob/master/rcl_action/QUALITY_DECLARATION.md).
 
 ### Direct Runtime non-ROS Dependency [5.iii]
 

rclcpp_action/README.md

@@ -6,4 +6,4 @@ Visit the [rclcpp_action API documentation](http://docs.ros2.org/latest/api/rclc
 
 ## Quality Declaration
 
-This package claims to be in the **Quality Level 2** category, see the [Quality Declaration](QUALITY_DECLARATION.md) for more details.
+This package claims to be in the **Quality Level 1** category, see the [Quality Declaration](QUALITY_DECLARATION.md) for more details.

rclcpp_action/include/rclcpp_action/client.hpp

@@ -116,10 +116,15 @@ public:
   bool
   is_ready(rcl_wait_set_t * wait_set) override;
 
+  /// \internal
+  RCLCPP_ACTION_PUBLIC
+  std::shared_ptr<void>
+  take_data() override;
+
   /// \internal
   RCLCPP_ACTION_PUBLIC
   void
-  execute() override;
+  execute(std::shared_ptr<void> & data) override;
 
   // End Waitables API
   // -----------------
@@ -263,13 +268,107 @@ public:
   using Feedback = typename ActionT::Feedback;
   using GoalHandle = ClientGoalHandle<ActionT>;
   using WrappedResult = typename GoalHandle::WrappedResult;
-  using GoalResponseCallback = std::function<void (typename GoalHandle::SharedPtr)>;
   using FeedbackCallback = typename GoalHandle::FeedbackCallback;
   using ResultCallback = typename GoalHandle::ResultCallback;
   using CancelRequest = typename ActionT::Impl::CancelGoalService::Request;
   using CancelResponse = typename ActionT::Impl::CancelGoalService::Response;
   using CancelCallback = std::function<void (typename CancelResponse::SharedPtr)>;
 
+  /// Compatibility wrapper for `goal_response_callback`.
+  class GoalResponseCallback
+  {
+public:
+    using NewSignature = std::function<void (typename GoalHandle::SharedPtr)>;
+    using OldSignature = std::function<void (std::shared_future<typename GoalHandle::SharedPtr>)>;
+
+    GoalResponseCallback() = default;
+
+    GoalResponseCallback(std::nullptr_t) {}  // NOLINT, intentionally implicit.
+
+    // implicit constructor
+    [[deprecated(
+      "Use new goal response callback signature "
+      "`std::function<void (Client<ActionT>::GoalHandle::SharedPtr)>` "
+      "instead of the old "
+      "`std::function<void (std::shared_future<Client<ActionT>::GoalHandle::SharedPtr>)>`.\n"
+      "e.g.:\n"
+      "```cpp\n"
+      "Client<ActionT>::SendGoalOptions options;\n"
+      "options.goal_response_callback = [](Client<ActionT>::GoalHandle::SharedPtr goal) {\n"
+      "  // do something with `goal` here\n"
+      "};")]]
+    GoalResponseCallback(OldSignature old_callback)  // NOLINT, intentionally implicit.
+    : old_callback_(std::move(old_callback)) {}
+
+    GoalResponseCallback(NewSignature new_callback)  // NOLINT, intentionally implicit.
+    : new_callback_(std::move(new_callback)) {}
+
+    GoalResponseCallback &
+    operator=(OldSignature old_callback) {old_callback_ = std::move(old_callback); return *this;}
+
+    GoalResponseCallback &
+    operator=(NewSignature new_callback) {new_callback_ = std::move(new_callback); return *this;}
+
+    void
+    operator()(typename GoalHandle::SharedPtr goal_handle) const
+    {
+      if (new_callback_) {
+        new_callback_(std::move(goal_handle));
+        return;
+      }
+      if (old_callback_) {
+        throw std::runtime_error{
+                "Cannot call GoalResponseCallback(GoalHandle::SharedPtr) "
+                "if using the old goal response callback signature."};
+      }
+      throw std::bad_function_call{};
+    }
+
+    [[deprecated(
+      "Calling "
+      "`void goal_response_callback("
+      "   std::shared_future<Client<ActionT>::GoalHandle::SharedPtr> goal_handle_shared_future)`"
+      " is deprecated.")]]
+    void
+    operator()(std::shared_future<typename GoalHandle::SharedPtr> goal_handle_future) const
+    {
+      if (old_callback_) {
+        old_callback_(std::move(goal_handle_future));
+        return;
+      }
+      if (new_callback_) {
+        new_callback_(std::move(goal_handle_future).get_future().share());
+        return;
+      }
+      throw std::bad_function_call{};
+    }
+
+    explicit operator bool() const noexcept {
+      return new_callback_ || old_callback_;
+    }
+
+private:
+    friend class Client;
+    void
+    operator()(
+      typename GoalHandle::SharedPtr goal_handle,
+      std::shared_future<typename GoalHandle::SharedPtr> goal_handle_future) const
+    {
+      if (new_callback_) {
+        new_callback_(std::move(goal_handle));
+        return;
+      }
+      if (old_callback_) {
+        old_callback_(std::move(goal_handle_future));
+        return;
+      }
+      throw std::bad_function_call{};
+    }
+
+    NewSignature new_callback_;
+    OldSignature old_callback_;
+  };
+
   /// Options for sending a goal.
   /**
    * This struct is used to pass parameters to the function `async_send_goal`.
@@ -358,7 +457,7 @@ public:
         if (!goal_response->accepted) {
           promise->set_value(nullptr);
           if (options.goal_response_callback) {
-            options.goal_response_callback(nullptr);
+            options.goal_response_callback(nullptr, future);
           }
           return;
         }
@@ -374,7 +473,7 @@ public:
         }
         promise->set_value(goal_handle);
         if (options.goal_response_callback) {
-          options.goal_response_callback(goal_handle);
+          options.goal_response_callback(goal_handle, future);
         }
 
         if (options.result_callback) {

rclcpp_action/include/rclcpp_action/server.hpp

@@ -118,11 +118,15 @@ public:
   bool
   is_ready(rcl_wait_set_t *) override;
 
+  RCLCPP_ACTION_PUBLIC
+  std::shared_ptr<void>
+  take_data() override;
+
   /// Act on entities in the wait set which are ready to be acted upon.
   /// \internal
   RCLCPP_ACTION_PUBLIC
   void
-  execute() override;
+  execute(std::shared_ptr<void> & data) override;
 
   // End Waitables API
   // -----------------
@@ -229,19 +233,19 @@ private:
   /// \internal
   RCLCPP_ACTION_PUBLIC
   void
-  execute_goal_request_received();
+  execute_goal_request_received(std::shared_ptr<void> & data);
 
   /// Handle a request to cancel goals on the server
   /// \internal
   RCLCPP_ACTION_PUBLIC
   void
-  execute_cancel_request_received();
+  execute_cancel_request_received(std::shared_ptr<void> & data);
 
   /// Handle a request to get the result of an action
   /// \internal
   RCLCPP_ACTION_PUBLIC
   void
-  execute_result_request_received();
+  execute_result_request_received(std::shared_ptr<void> & data);
 
   /// Handle a timeout indicating a completed goal should be forgotten by the server
   /// \internal
@@ -377,31 +381,31 @@ protected:
     std::weak_ptr<Server<ActionT>> weak_this = this->shared_from_this();
 
     std::function<void(const GoalUUID &, std::shared_ptr<void>)> on_terminal_state =
-      [weak_this](const GoalUUID & uuid, std::shared_ptr<void> result_message)
+      [weak_this](const GoalUUID & goal_uuid, std::shared_ptr<void> result_message)
       {
         std::shared_ptr<Server<ActionT>> shared_this = weak_this.lock();
         if (!shared_this) {
           return;
         }
         // Send result message to anyone that asked
-        shared_this->publish_result(uuid, result_message);
+        shared_this->publish_result(goal_uuid, result_message);
         // Publish a status message any time a goal handle changes state
         shared_this->publish_status();
         // notify base so it can recalculate the expired goal timer
         shared_this->notify_goal_terminal_state();
         // Delete data now (ServerBase and rcl_action_server_t keep data until goal handle expires)
         std::lock_guard<std::mutex> lock(shared_this->goal_handles_mutex_);
-        shared_this->goal_handles_.erase(uuid);
+        shared_this->goal_handles_.erase(goal_uuid);
       };
 
     std::function<void(const GoalUUID &)> on_executing =
-      [weak_this](const GoalUUID & uuid)
+      [weak_this](const GoalUUID & goal_uuid)
       {
         std::shared_ptr<Server<ActionT>> shared_this = weak_this.lock();
         if (!shared_this) {
           return;
         }
-        (void)uuid;
+        (void)goal_uuid;
         // Publish a status message any time a goal handle changes state
         shared_this->publish_status();
       };

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>5.1.0</version>
+  <version>6.3.0</version>
   <description>Adds action APIs for C++.</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="mabel@openrobotics.org">Mabel Zhang</maintainer>

rclcpp_action/src/client.cpp

@@ -22,6 +22,8 @@
 #include <memory>
 #include <random>
 #include <string>
+#include <tuple>
+#include <utility>
 
 #include "rclcpp_action/client.hpp"
 #include "rclcpp_action/exceptions.hpp"
@@ -380,58 +382,111 @@ ClientBase::generate_goal_id()
   return goal_id;
 }
 
-void
-ClientBase::execute()
+std::shared_ptr<void>
+ClientBase::take_data()
 {
   if (pimpl_->is_feedback_ready) {
     std::shared_ptr<void> feedback_message = this->create_feedback_message();
     rcl_ret_t ret = rcl_action_take_feedback(
       pimpl_->client_handle.get(), feedback_message.get());
-    pimpl_->is_feedback_ready = false;
-    if (RCL_RET_OK == ret) {
-      this->handle_feedback_message(feedback_message);
-    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking feedback");
-    }
+    return std::static_pointer_cast<void>(
+      std::make_shared<std::tuple<rcl_ret_t, std::shared_ptr<void>>>(
+        ret, feedback_message));
   } else if (pimpl_->is_status_ready) {
     std::shared_ptr<void> status_message = this->create_status_message();
     rcl_ret_t ret = rcl_action_take_status(
       pimpl_->client_handle.get(), status_message.get());
-    pimpl_->is_status_ready = false;
-    if (RCL_RET_OK == ret) {
-      this->handle_status_message(status_message);
-    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking status");
-    }
+    return std::static_pointer_cast<void>(
+      std::make_shared<std::tuple<rcl_ret_t, std::shared_ptr<void>>>(
+        ret, status_message));
   } else if (pimpl_->is_goal_response_ready) {
     rmw_request_id_t response_header;
     std::shared_ptr<void> goal_response = this->create_goal_response();
     rcl_ret_t ret = rcl_action_take_goal_response(
       pimpl_->client_handle.get(), &response_header, goal_response.get());
-    pimpl_->is_goal_response_ready = false;
-    if (RCL_RET_OK == ret) {
-      this->handle_goal_response(response_header, goal_response);
-    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking goal response");
-    }
+    return std::static_pointer_cast<void>(
+      std::make_shared<std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(
+        ret, response_header, goal_response));
   } else if (pimpl_->is_result_response_ready) {
     rmw_request_id_t response_header;
     std::shared_ptr<void> result_response = this->create_result_response();
     rcl_ret_t ret = rcl_action_take_result_response(
       pimpl_->client_handle.get(), &response_header, result_response.get());
-    pimpl_->is_result_response_ready = false;
-    if (RCL_RET_OK == ret) {
-      this->handle_result_response(response_header, result_response);
-    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking result response");
-    }
+    return std::static_pointer_cast<void>(
+      std::make_shared<std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(
+        ret, response_header, result_response));
   } else if (pimpl_->is_cancel_response_ready) {
     rmw_request_id_t response_header;
     std::shared_ptr<void> cancel_response = this->create_cancel_response();
     rcl_ret_t ret = rcl_action_take_cancel_response(
       pimpl_->client_handle.get(), &response_header, cancel_response.get());
+    return std::static_pointer_cast<void>(
+      std::make_shared<std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(
+        ret, response_header, cancel_response));
+  } else {
+    throw std::runtime_error("Taking data from action client but nothing is ready");
+  }
+}
+
+void
+ClientBase::execute(std::shared_ptr<void> & data)
+{
+  if (!data) {
+    throw std::runtime_error("'data' is empty");
+  }
+
+  if (pimpl_->is_feedback_ready) {
+    auto shared_ptr = std::static_pointer_cast<std::tuple<rcl_ret_t, std::shared_ptr<void>>>(data);
+    auto ret = std::get<0>(*shared_ptr);
+    pimpl_->is_feedback_ready = false;
+    if (RCL_RET_OK == ret) {
+      auto feedback_message = std::get<1>(*shared_ptr);
+      this->handle_feedback_message(feedback_message);
+    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
+      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking feedback");
+    }
+  } else if (pimpl_->is_status_ready) {
+    auto shared_ptr = std::static_pointer_cast<std::tuple<rcl_ret_t, std::shared_ptr<void>>>(data);
+    auto ret = std::get<0>(*shared_ptr);
+    pimpl_->is_status_ready = false;
+    if (RCL_RET_OK == ret) {
+      auto status_message = std::get<1>(*shared_ptr);
+      this->handle_status_message(status_message);
+    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
+      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking status");
+    }
+  } else if (pimpl_->is_goal_response_ready) {
+    auto shared_ptr = std::static_pointer_cast<
+      std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(data);
+    auto ret = std::get<0>(*shared_ptr);
+    pimpl_->is_goal_response_ready = false;
+    if (RCL_RET_OK == ret) {
+      auto response_header = std::get<1>(*shared_ptr);
+      auto goal_response = std::get<2>(*shared_ptr);
+      this->handle_goal_response(response_header, goal_response);
+    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
+      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking goal response");
+    }
+  } else if (pimpl_->is_result_response_ready) {
+    auto shared_ptr = std::static_pointer_cast<
+      std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(data);
+    auto ret = std::get<0>(*shared_ptr);
+    pimpl_->is_result_response_ready = false;
+    if (RCL_RET_OK == ret) {
+      auto response_header = std::get<1>(*shared_ptr);
+      auto result_response = std::get<2>(*shared_ptr);
+      this->handle_result_response(response_header, result_response);
+    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
+      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking result response");
+    }
+  } else if (pimpl_->is_cancel_response_ready) {
+    auto shared_ptr = std::static_pointer_cast<
+      std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(data);
+    auto ret = std::get<0>(*shared_ptr);
     pimpl_->is_cancel_response_ready = false;
     if (RCL_RET_OK == ret) {
+      auto response_header = std::get<1>(*shared_ptr);
+      auto cancel_response = std::get<2>(*shared_ptr);
       this->handle_cancel_response(response_header, cancel_response);
     } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
       rclcpp::exceptions::throw_from_rcl_error(ret, "error taking cancel response");

rclcpp_action/src/server.cpp

@@ -24,7 +24,9 @@
 #include <memory>
 #include <mutex>
 #include <string>
+#include <tuple>
 #include <unordered_map>
+#include <utility>
 #include <vector>
 
 using rclcpp_action::ServerBase;
@@ -43,7 +45,7 @@ public:
   {
   }
 
-  // Lock everything except user callbacks
+  // Lock for action_server_
   std::recursive_mutex action_server_reentrant_mutex_;
 
   std::shared_ptr<rcl_action_server_t> action_server_;
@@ -56,10 +58,13 @@ public:
   size_t num_services_ = 0;
   size_t num_guard_conditions_ = 0;
 
-  bool goal_request_ready_ = false;
-  bool cancel_request_ready_ = false;
-  bool result_request_ready_ = false;
-  bool goal_expired_ = false;
+  std::atomic<bool> goal_request_ready_{false};
+  std::atomic<bool> cancel_request_ready_{false};
+  std::atomic<bool> result_request_ready_{false};
+  std::atomic<bool> goal_expired_{false};
+
+  // Lock for unordered_maps
+  std::recursive_mutex unordered_map_mutex_;
 
   // Results to be kept until the goal expires after reaching a terminal state
   std::unordered_map<GoalUUID, std::shared_ptr<void>> goal_results_;
@@ -169,35 +174,109 @@ ServerBase::add_to_wait_set(rcl_wait_set_t * wait_set)
 bool
 ServerBase::is_ready(rcl_wait_set_t * wait_set)
 {
-  std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
-  rcl_ret_t ret = rcl_action_server_wait_set_get_entities_ready(
-    wait_set,
-    pimpl_->action_server_.get(),
-    &pimpl_->goal_request_ready_,
-    &pimpl_->cancel_request_ready_,
-    &pimpl_->result_request_ready_,
-    &pimpl_->goal_expired_);
+  bool goal_request_ready;
+  bool cancel_request_ready;
+  bool result_request_ready;
+  bool goal_expired;
+  rcl_ret_t ret;
+  {
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+    ret = rcl_action_server_wait_set_get_entities_ready(
+      wait_set,
+      pimpl_->action_server_.get(),
+      &goal_request_ready,
+      &cancel_request_ready,
+      &result_request_ready,
+      &goal_expired);
+  }
+
+  pimpl_->goal_request_ready_ = goal_request_ready;
+  pimpl_->cancel_request_ready_ = cancel_request_ready;
+  pimpl_->result_request_ready_ = result_request_ready;
+  pimpl_->goal_expired_ = goal_expired;
 
   if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
 
-  return pimpl_->goal_request_ready_ ||
-         pimpl_->cancel_request_ready_ ||
-         pimpl_->result_request_ready_ ||
-         pimpl_->goal_expired_;
+  return pimpl_->goal_request_ready_.load() ||
+         pimpl_->cancel_request_ready_.load() ||
+         pimpl_->result_request_ready_.load() ||
+         pimpl_->goal_expired_.load();
+}
+
+std::shared_ptr<void>
+ServerBase::take_data()
+{
+  if (pimpl_->goal_request_ready_.load()) {
+    rcl_ret_t ret;
+    rcl_action_goal_info_t goal_info = rcl_action_get_zero_initialized_goal_info();
+    rmw_request_id_t request_header;
+
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+
+    std::shared_ptr<void> message = create_goal_request();
+    ret = rcl_action_take_goal_request(
+      pimpl_->action_server_.get(),
+      &request_header,
+      message.get());
+
+    return std::static_pointer_cast<void>(
+      std::make_shared
+      <std::tuple<rcl_ret_t, rcl_action_goal_info_t, rmw_request_id_t, std::shared_ptr<void>>>(
+        ret,
+        goal_info,
+        request_header, message));
+  } else if (pimpl_->cancel_request_ready_.load()) {
+    rcl_ret_t ret;
+    rmw_request_id_t request_header;
+
+    // Initialize cancel request
+    auto request = std::make_shared<action_msgs::srv::CancelGoal::Request>();
+
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+    ret = rcl_action_take_cancel_request(
+      pimpl_->action_server_.get(),
+      &request_header,
+      request.get());
+
+    return std::static_pointer_cast<void>(
+      std::make_shared
+      <std::tuple<rcl_ret_t, std::shared_ptr<action_msgs::srv::CancelGoal::Request>,
+      rmw_request_id_t>>(ret, request, request_header));
+  } else if (pimpl_->result_request_ready_.load()) {
+    rcl_ret_t ret;
+    // Get the result request message
+    rmw_request_id_t request_header;
+    std::shared_ptr<void> result_request = create_result_request();
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+    ret = rcl_action_take_result_request(
+      pimpl_->action_server_.get(), &request_header, result_request.get());
+
+    return std::static_pointer_cast<void>(
+      std::make_shared<std::tuple<rcl_ret_t, std::shared_ptr<void>, rmw_request_id_t>>(
+        ret, result_request, request_header));
+  } else if (pimpl_->goal_expired_.load()) {
+    return nullptr;
+  } else {
+    throw std::runtime_error("Taking data from action server but nothing is ready");
+  }
 }
 
 void
-ServerBase::execute()
+ServerBase::execute(std::shared_ptr<void> & data)
 {
-  if (pimpl_->goal_request_ready_) {
-    execute_goal_request_received();
-  } else if (pimpl_->cancel_request_ready_) {
-    execute_cancel_request_received();
-  } else if (pimpl_->result_request_ready_) {
-    execute_result_request_received();
-  } else if (pimpl_->goal_expired_) {
+  if (!data && !pimpl_->goal_expired_.load()) {
+    throw std::runtime_error("'data' is empty");
+  }
+
+  if (pimpl_->goal_request_ready_.load()) {
+    execute_goal_request_received(data);
+  } else if (pimpl_->cancel_request_ready_.load()) {
+    execute_cancel_request_received(data);
+  } else if (pimpl_->result_request_ready_.load()) {
+    execute_result_request_received(data);
+  } else if (pimpl_->goal_expired_.load()) {
     execute_check_expired_goals();
   } else {
     throw std::runtime_error("Executing action server but nothing is ready");
@@ -205,22 +284,11 @@ ServerBase::execute()
 }
 
 void
-ServerBase::execute_goal_request_received()
+ServerBase::execute_goal_request_received(std::shared_ptr<void> & data)
 {
-  rcl_ret_t ret;
-  rcl_action_goal_info_t goal_info = rcl_action_get_zero_initialized_goal_info();
-  rmw_request_id_t request_header;
-
-  std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
-
-  std::shared_ptr<void> message = create_goal_request();
-  ret = rcl_action_take_goal_request(
-    pimpl_->action_server_.get(),
-    &request_header,
-    message.get());
-
-  pimpl_->goal_request_ready_ = false;
-
+  auto shared_ptr = std::static_pointer_cast
+    <std::tuple<rcl_ret_t, rcl_action_goal_info_t, rmw_request_id_t, std::shared_ptr<void>>>(data);
+  rcl_ret_t ret = std::get<0>(*shared_ptr);
   if (RCL_RET_ACTION_SERVER_TAKE_FAILED == ret) {
     // Ignore take failure because connext fails if it receives a sample without valid data.
     // This happens when a client shuts down and connext receives a sample saying the client is
@@ -229,6 +297,14 @@ ServerBase::execute_goal_request_received()
   } else if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
+  rcl_action_goal_info_t goal_info = std::get<1>(*shared_ptr);
+  rmw_request_id_t request_header = std::get<2>(*shared_ptr);
+  std::shared_ptr<void> message = std::get<3>(*shared_ptr);
+
+  bool expected = true;
+  if (!pimpl_->goal_request_ready_.compare_exchange_strong(expected, false)) {
+    return;
+  }
 
   GoalUUID uuid = get_goal_id_from_goal_request(message.get());
   convert(uuid, &goal_info);
@@ -236,10 +312,13 @@ ServerBase::execute_goal_request_received()
   // Call user's callback, getting the user's response and a ros message to send back
   auto response_pair = call_handle_goal_callback(uuid, message);
 
-  ret = rcl_action_send_goal_response(
-    pimpl_->action_server_.get(),
-    &request_header,
-    response_pair.second.get());
+  {
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+    ret = rcl_action_send_goal_response(
+      pimpl_->action_server_.get(),
+      &request_header,
+      response_pair.second.get());
+  }
 
   if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
@@ -264,7 +343,10 @@ ServerBase::execute_goal_request_received()
         }
       };
     rcl_action_goal_handle_t * rcl_handle;
-    rcl_handle = rcl_action_accept_new_goal(pimpl_->action_server_.get(), &goal_info);
+    {
+      std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+      rcl_handle = rcl_action_accept_new_goal(pimpl_->action_server_.get(), &goal_info);
+    }
     if (!rcl_handle) {
       throw std::runtime_error("Failed to accept new goal\n");
     }
@@ -273,7 +355,10 @@ ServerBase::execute_goal_request_received()
     // Copy out goal handle since action server storage disappears when it is fini'd
     *handle = *rcl_handle;
 
-    pimpl_->goal_handles_[uuid] = handle;
+    {
+      std::lock_guard<std::recursive_mutex> lock(pimpl_->unordered_map_mutex_);
+      pimpl_->goal_handles_[uuid] = handle;
+    }
 
     if (GoalResponse::ACCEPT_AND_EXECUTE == status) {
       // Change status to executing
@@ -288,25 +373,16 @@ ServerBase::execute_goal_request_received()
     // Tell user to start executing action
     call_goal_accepted_callback(handle, uuid, message);
   }
+  data.reset();
 }
 
 void
-ServerBase::execute_cancel_request_received()
+ServerBase::execute_cancel_request_received(std::shared_ptr<void> & data)
 {
-  rcl_ret_t ret;
-  rmw_request_id_t request_header;
-
-  // Initialize cancel request
-  auto request = std::make_shared<action_msgs::srv::CancelGoal::Request>();
-
-  std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
-  ret = rcl_action_take_cancel_request(
-    pimpl_->action_server_.get(),
-    &request_header,
-    request.get());
-
-  pimpl_->cancel_request_ready_ = false;
-
+  auto shared_ptr = std::static_pointer_cast
+    <std::tuple<rcl_ret_t, std::shared_ptr<action_msgs::srv::CancelGoal::Request>,
+      rmw_request_id_t>>(data);
+  auto ret = std::get<0>(*shared_ptr);
   if (RCL_RET_ACTION_SERVER_TAKE_FAILED == ret) {
     // Ignore take failure because connext fails if it receives a sample without valid data.
     // This happens when a client shuts down and connext receives a sample saying the client is
@@ -315,6 +391,8 @@ ServerBase::execute_cancel_request_received()
   } else if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
+  auto request = std::get<1>(*shared_ptr);
+  auto request_header = std::get<2>(*shared_ptr);
 
   // Convert c++ message to C message
   rcl_action_cancel_request_t cancel_request = rcl_action_get_zero_initialized_cancel_request();
@@ -325,10 +403,14 @@ ServerBase::execute_cancel_request_received()
   // Get a list of goal info that should be attempted to be cancelled
   rcl_action_cancel_response_t cancel_response = rcl_action_get_zero_initialized_cancel_response();
 
-  ret = rcl_action_process_cancel_request(
-    pimpl_->action_server_.get(),
-    &cancel_request,
-    &cancel_response);
+  {
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+    ret = rcl_action_process_cancel_request(
+      pimpl_->action_server_.get(),
+      &cancel_request,
+      &cancel_response);
+  }
+
   if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
@@ -371,26 +453,24 @@ ServerBase::execute_cancel_request_received()
     publish_status();
   }
 
-  ret = rcl_action_send_cancel_response(
-    pimpl_->action_server_.get(), &request_header, response.get());
+  {
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+    ret = rcl_action_send_cancel_response(
+      pimpl_->action_server_.get(), &request_header, response.get());
+  }
+
   if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
+  data.reset();
 }
 
 void
-ServerBase::execute_result_request_received()
+ServerBase::execute_result_request_received(std::shared_ptr<void> & data)
 {
-  rcl_ret_t ret;
-  // Get the result request message
-  rmw_request_id_t request_header;
-  std::shared_ptr<void> result_request = create_result_request();
-  std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
-  ret = rcl_action_take_result_request(
-    pimpl_->action_server_.get(), &request_header, result_request.get());
-
-  pimpl_->result_request_ready_ = false;
-
+  auto shared_ptr = std::static_pointer_cast
+    <std::tuple<rcl_ret_t, std::shared_ptr<void>, rmw_request_id_t>>(data);
+  auto ret = std::get<0>(*shared_ptr);
   if (RCL_RET_ACTION_SERVER_TAKE_FAILED == ret) {
     // Ignore take failure because connext fails if it receives a sample without valid data.
     // This happens when a client shuts down and connext receives a sample saying the client is
@@ -399,7 +479,10 @@ ServerBase::execute_result_request_received()
   } else if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
+  auto result_request = std::get<1>(*shared_ptr);
+  auto request_header = std::get<2>(*shared_ptr);
 
+  pimpl_->result_request_ready_ = false;
   std::shared_ptr<void> result_response;
 
   // check if the goal exists
@@ -407,7 +490,10 @@ ServerBase::execute_result_request_received()
   rcl_action_goal_info_t goal_info;
   convert(uuid, &goal_info);
   bool goal_exists;
-  goal_exists = rcl_action_server_goal_exists(pimpl_->action_server_.get(), &goal_info);
+  {
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+    goal_exists = rcl_action_server_goal_exists(pimpl_->action_server_.get(), &goal_info);
+  }
   if (!goal_exists) {
     // Goal does not exists
     result_response = create_result_response(action_msgs::msg::GoalStatus::STATUS_UNKNOWN);
@@ -421,15 +507,18 @@ ServerBase::execute_result_request_received()
 
   if (result_response) {
     // Send the result now
-    ret = rcl_action_send_result_response(
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+    rcl_ret_t rcl_ret = rcl_action_send_result_response(
       pimpl_->action_server_.get(), &request_header, result_response.get());
-    if (RCL_RET_OK != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret);
+    if (RCL_RET_OK != rcl_ret) {
+      rclcpp::exceptions::throw_from_rcl_error(rcl_ret);
     }
   } else {
     // Store the request so it can be responded to later
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->unordered_map_mutex_);
     pimpl_->result_requests_[uuid].push_back(request_header);
   }
+  data.reset();
 }
 
 void
@@ -441,9 +530,11 @@ ServerBase::execute_check_expired_goals()
 
   // Loop in case more than 1 goal expired
   while (num_expired > 0u) {
-    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
     rcl_ret_t ret;
-    ret = rcl_action_expire_goals(pimpl_->action_server_.get(), expired_goals, 1, &num_expired);
+    {
+      std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+      ret = rcl_action_expire_goals(pimpl_->action_server_.get(), expired_goals, 1, &num_expired);
+    }
     if (RCL_RET_OK != ret) {
       rclcpp::exceptions::throw_from_rcl_error(ret);
     } else if (num_expired) {
@@ -451,6 +542,7 @@ ServerBase::execute_check_expired_goals()
       GoalUUID uuid;
       convert(expired_goals[0], &uuid);
       RCLCPP_DEBUG(pimpl_->logger_, "Expired goal %s", to_string(uuid).c_str());
+      std::lock_guard<std::recursive_mutex> lock(pimpl_->unordered_map_mutex_);
       pimpl_->goal_results_.erase(uuid);
       pimpl_->result_requests_.erase(uuid);
       pimpl_->goal_handles_.erase(uuid);
@@ -523,20 +615,26 @@ ServerBase::publish_result(const GoalUUID & uuid, std::shared_ptr<void> result_m
   // Check that the goal exists
   rcl_action_goal_info_t goal_info;
   convert(uuid, &goal_info);
-  std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
   bool goal_exists;
-  goal_exists = rcl_action_server_goal_exists(pimpl_->action_server_.get(), &goal_info);
+  {
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+    goal_exists = rcl_action_server_goal_exists(pimpl_->action_server_.get(), &goal_info);
+  }
 
   if (!goal_exists) {
     throw std::runtime_error("Asked to publish result for goal that does not exist");
   }
 
-  pimpl_->goal_results_[uuid] = result_msg;
+  {
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->unordered_map_mutex_);
+    pimpl_->goal_results_[uuid] = result_msg;
+  }
 
   // if there are clients who already asked for the result, send it to them
   auto iter = pimpl_->result_requests_.find(uuid);
   if (iter != pimpl_->result_requests_.end()) {
     for (auto & request_header : iter->second) {
+      std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
       rcl_ret_t ret = rcl_action_send_result_response(
         pimpl_->action_server_.get(), &request_header, result_msg.get());
       if (RCL_RET_OK != ret) {

rclcpp_action/test/benchmark/benchmark_action_client.cpp

@@ -81,7 +81,7 @@ public:
 
     // Should be checked by the server above
     assert(goal->order > 0);
-    result->sequence.resize(goal->order);
+    result->sequence.resize(static_cast<size_t>(goal->order));
     result->sequence[0] = 0;
     if (goal->order == 1) {
       current_goal_handle->succeed(result);
@@ -92,7 +92,7 @@ public:
       current_goal_handle->succeed(result);
       return;
     }
-    for (int i = 2; i < goal->order; ++i) {
+    for (size_t i = 2; i < static_cast<size_t>(goal->order); ++i) {
       result->sequence[i] =
         result->sequence[i - 1] + result->sequence[i - 2];
     }
@@ -310,8 +310,8 @@ BENCHMARK_F(ActionClientPerformanceTest, async_cancel_goal)(benchmark::State & s
     rclcpp::spin_until_future_complete(node, future_cancel, std::chrono::seconds(1));
     auto cancel_response = future_cancel.get();
 
-    using CancelResponse = test_msgs::action::Fibonacci::Impl::CancelGoalService::Response;
-    if (CancelResponse::ERROR_NONE != cancel_response->return_code) {
+    using CancelActionResponse = test_msgs::action::Fibonacci::Impl::CancelGoalService::Response;
+    if (CancelActionResponse::ERROR_NONE != cancel_response->return_code) {
       state.SkipWithError("Cancel request did not succeed");
       break;
     }
@@ -345,8 +345,8 @@ BENCHMARK_F(ActionClientPerformanceTest, async_cancel_all_goals)(benchmark::Stat
     rclcpp::spin_until_future_complete(node, future_cancel_all, std::chrono::seconds(1));
     auto cancel_response = future_cancel_all.get();
 
-    using CancelResponse = test_msgs::action::Fibonacci::Impl::CancelGoalService::Response;
-    if (CancelResponse::ERROR_NONE != cancel_response->return_code) {
+    using CancelActionResponse = test_msgs::action::Fibonacci::Impl::CancelGoalService::Response;
+    if (CancelActionResponse::ERROR_NONE != cancel_response->return_code) {
       state.SkipWithError("Cancel request did not succeed");
       break;
     }

rclcpp_action/test/benchmark/benchmark_action_server.cpp

@@ -187,8 +187,8 @@ BENCHMARK_F(ActionServerPerformanceTest, action_server_cancel_goal)(benchmark::S
 
     rclcpp::spin_until_future_complete(node, future_cancel, std::chrono::seconds(1));
     auto cancel_response = future_cancel.get();
-    using CancelResponse = test_msgs::action::Fibonacci::Impl::CancelGoalService::Response;
-    if (CancelResponse::ERROR_NONE != cancel_response->return_code) {
+    using CancelActionResponse = test_msgs::action::Fibonacci::Impl::CancelGoalService::Response;
+    if (CancelActionResponse::ERROR_NONE != cancel_response->return_code) {
       state.SkipWithError("Cancel request did not succeed");
       break;
     }
@@ -247,12 +247,12 @@ BENCHMARK_F(ActionServerPerformanceTest, action_server_set_success)(benchmark::S
   // too wide, they at least could agree it was fine. In my testing MSVC errored if goal_order was
   // not captured, but clang would warn if it was explicitly captured.
   const auto result = [&]() {
-      auto result = std::make_shared<Fibonacci::Result>();
+      auto action_result = std::make_shared<Fibonacci::Result>();
       for (int i = 0; i < goal_order; ++i) {
         // Not the fibonacci sequence, but that's not important to this benchmark
-        result->sequence.push_back(i);
+        action_result->sequence.push_back(i);
       }
-      return result;
+      return action_result;
     } ();
 
   reset_heap_counters();
@@ -291,12 +291,12 @@ BENCHMARK_F(ActionServerPerformanceTest, action_server_abort)(benchmark::State &
 
   // Capturing with & because MSVC and Clang disagree about how to capture goal_order
   const auto result = [&]() {
-      auto result = std::make_shared<Fibonacci::Result>();
+      auto action_result = std::make_shared<Fibonacci::Result>();
       for (int i = 0; i < goal_order; ++i) {
         // Not the fibonacci sequence, but that's not important to this benchmark
-        result->sequence.push_back(i);
+        action_result->sequence.push_back(i);
       }
-      return result;
+      return action_result;
     } ();
 
   reset_heap_counters();

rclcpp_action/test/test_client.cpp

@@ -132,7 +132,7 @@ protected:
             feedback_message.feedback.sequence.push_back(1);
             feedback_publisher->publish(feedback_message);
             client_executor.spin_once();
-            for (int i = 1; i < goal_request->goal.order; ++i) {
+            for (size_t i = 1; i < static_cast<size_t>(goal_request->goal.order); ++i) {
               feedback_message.feedback.sequence.push_back(
                 feedback_message.feedback.sequence[i] +
                 feedback_message.feedback.sequence[i - 1]);
@@ -536,6 +536,64 @@ TEST_F(TestClientAgainstServer, async_send_goal_with_goal_response_callback_wait
   }
 }
 
+TEST_F(TestClientAgainstServer, async_send_goal_with_deprecated_goal_response_callback)
+{
+  auto action_client = rclcpp_action::create_client<ActionType>(client_node, action_name);
+  ASSERT_TRUE(action_client->wait_for_action_server(WAIT_FOR_SERVER_TIMEOUT));
+
+  bool goal_response_received = false;
+  auto send_goal_ops = rclcpp_action::Client<ActionType>::SendGoalOptions();
+
+#if !defined(_WIN32)
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#else
+# pragma warning(push)
+# pragma warning(disable: 4996)
+#endif
+  send_goal_ops.goal_response_callback =
+    [&goal_response_received]
+      (std::shared_future<typename ActionGoalHandle::SharedPtr> goal_future)
+    {
+      if (goal_future.get()) {
+        goal_response_received = true;
+      }
+    };
+#if !defined(_WIN32)
+# pragma GCC diagnostic pop
+#else
+# pragma warning(pop)
+#endif
+
+  {
+    ActionGoal bad_goal;
+    bad_goal.order = -1;
+    auto future_goal_handle = action_client->async_send_goal(bad_goal, send_goal_ops);
+    dual_spin_until_future_complete(future_goal_handle);
+    auto goal_handle = future_goal_handle.get();
+    EXPECT_FALSE(goal_response_received);
+    EXPECT_EQ(nullptr, goal_handle);
+  }
+
+  {
+    ActionGoal goal;
+    goal.order = 4;
+    auto future_goal_handle = action_client->async_send_goal(goal, send_goal_ops);
+    dual_spin_until_future_complete(future_goal_handle);
+    auto goal_handle = future_goal_handle.get();
+    EXPECT_TRUE(goal_response_received);
+    EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status());
+    EXPECT_FALSE(goal_handle->is_feedback_aware());
+    EXPECT_FALSE(goal_handle->is_result_aware());
+    auto future_result = action_client->async_get_result(goal_handle);
+    EXPECT_TRUE(goal_handle->is_result_aware());
+    dual_spin_until_future_complete(future_result);
+    auto wrapped_result = future_result.get();
+    ASSERT_EQ(5u, wrapped_result.result->sequence.size());
+    EXPECT_EQ(3, wrapped_result.result->sequence.back());
+  }
+}
+
 TEST_F(TestClientAgainstServer, async_send_goal_with_feedback_callback_wait_for_result)
 {
   auto action_client = rclcpp_action::create_client<ActionType>(client_node, action_name);

rclcpp_action/test/test_server.cpp

@@ -1221,3 +1221,92 @@ TEST_F(TestCancelRequestServer, publish_status_send_cancel_response_errors)
 
   EXPECT_THROW(SendClientCancelRequest(), std::runtime_error);
 }
+
+class TestDeadlockServer : public TestServer
+{
+public:
+  void SetUp()
+  {
+    node_ = std::make_shared<rclcpp::Node>("goal_request", "/rclcpp_action/goal_request");
+    uuid1_ = {{1, 2, 3, 4, 5, 6, 70, 80, 9, 1, 11, 120, 13, 140, 15, 160}};
+    uuid2_ = {{2, 2, 3, 4, 5, 6, 70, 80, 9, 1, 11, 120, 13, 140, 15, 160}};
+    action_server_ = rclcpp_action::create_server<Fibonacci>(
+      node_, "fibonacci",
+      [this](const GoalUUID &, std::shared_ptr<const Fibonacci::Goal>) {
+        // instead of making a deadlock, check if it can acquire the lock in a second
+        std::unique_lock<std::recursive_timed_mutex> lock(server_mutex_, std::defer_lock);
+        this->TryLockFor(lock, std::chrono::milliseconds(1000));
+        return rclcpp_action::GoalResponse::ACCEPT_AND_EXECUTE;
+      },
+      [this](std::shared_ptr<GoalHandle>) {
+        // instead of making a deadlock, check if it can acquire the lock in a second
+        std::unique_lock<std::recursive_timed_mutex> lock(server_mutex_, std::defer_lock);
+        this->TryLockFor(lock, std::chrono::milliseconds(1000));
+        return rclcpp_action::CancelResponse::ACCEPT;
+      },
+      [this](std::shared_ptr<GoalHandle> handle) {
+        // instead of making a deadlock, check if it can acquire the lock in a second
+        std::unique_lock<std::recursive_timed_mutex> lock(server_mutex_, std::defer_lock);
+        this->TryLockFor(lock, std::chrono::milliseconds(1000));
+        goal_handle_ = handle;
+      });
+  }
+
+  void GoalSucceeded()
+  {
+    std::lock_guard<std::recursive_timed_mutex> lock(server_mutex_);
+    rclcpp::sleep_for(std::chrono::milliseconds(100));
+    auto result = std::make_shared<Fibonacci::Result>();
+    result->sequence = {5, 8, 13, 21};
+    goal_handle_->succeed(result);
+  }
+
+  void GoalCanceled()
+  {
+    std::lock_guard<std::recursive_timed_mutex> lock(server_mutex_);
+    rclcpp::sleep_for(std::chrono::milliseconds(100));
+    auto result = std::make_shared<Fibonacci::Result>();
+    goal_handle_->canceled(result);
+  }
+
+  void TryLockFor(
+    std::unique_lock<std::recursive_timed_mutex> & lock,
+    std::chrono::milliseconds timeout
+  )
+  {
+    ASSERT_TRUE(lock.try_lock_for(timeout));
+  }
+
+protected:
+  std::recursive_timed_mutex server_mutex_;
+  GoalUUID uuid1_, uuid2_;
+  std::shared_ptr<rclcpp::Node> node_;
+  std::shared_ptr<rclcpp_action::Server<Fibonacci>> action_server_;
+
+  using GoalHandle = rclcpp_action::ServerGoalHandle<Fibonacci>;
+  std::shared_ptr<GoalHandle> goal_handle_;
+};
+
+TEST_F(TestDeadlockServer, deadlock_while_succeed)
+{
+  send_goal_request(node_, uuid1_);
+  // this will lock wrapper's mutex and intentionally wait 100ms for calling succeed
+  // to try to acquire the lock of rclcpp_action mutex
+  std::thread t(&TestDeadlockServer::GoalSucceeded, this);
+  // after the wrapper's mutex is locked and before succeed is called
+  rclcpp::sleep_for(std::chrono::milliseconds(50));
+  // call next goal request to intentionally reproduce deadlock
+  // this first locks rclcpp_action mutex and then call callback to lock wrapper's mutex
+  send_goal_request(node_, uuid2_);
+  t.join();
+}
+
+TEST_F(TestDeadlockServer, deadlock_while_canceled)
+{
+  send_goal_request(node_, uuid1_);
+  send_cancel_request(node_, uuid1_);
+  std::thread t(&TestDeadlockServer::GoalCanceled, this);
+  rclcpp::sleep_for(std::chrono::milliseconds(50));
+  send_goal_request(node_, uuid2_);  // deadlock here
+  t.join();
+}

rclcpp_action/test/test_types.cpp

@@ -25,12 +25,12 @@ TEST(TestActionTypes, goal_uuid_to_string) {
   EXPECT_STREQ("0123456789abcdef", rclcpp_action::to_string(goal_id).c_str());
 
   for (uint8_t i = 0; i < UUID_SIZE; ++i) {
-    goal_id[i] = 16u + i;
+    goal_id[i] = static_cast<uint8_t>(16u + i);
   }
   EXPECT_STREQ("101112131415161718191a1b1c1d1e1f", rclcpp_action::to_string(goal_id).c_str());
 
   for (uint8_t i = 0; i < UUID_SIZE; ++i) {
-    goal_id[i] = std::numeric_limits<uint8_t>::max() - i;
+    goal_id[i] = static_cast<uint8_t>(std::numeric_limits<uint8_t>::max() - i);
   }
   EXPECT_STREQ("fffefdfcfbfaf9f8f7f6f5f4f3f2f1f0", rclcpp_action::to_string(goal_id).c_str());
 }

rclcpp_components/CHANGELOG.rst

@@ -2,6 +2,26 @@
 Changelog for package rclcpp_components
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+6.3.0 (2021-01-25)
+------------------
+
+6.2.0 (2021-01-08)
+------------------
+* Use std compliant non-method std::filesystem::exists function (`#1502 <https://github.com/ros2/rclcpp/issues/1502>`_)
+* Fix string literal warnings (`#1442 <https://github.com/ros2/rclcpp/issues/1442>`_)
+* Contributors: Audrow Nash, Josh Langsfeld
+
+6.1.0 (2020-12-10)
+------------------
+* Update QDs to QL 1 (`#1477 <https://github.com/ros2/rclcpp/issues/1477>`_)
+* Add benchmarks for components (`#1476 <https://github.com/ros2/rclcpp/issues/1476>`_)
+* Contributors: Scott K Logan, Stephen Brawner
+
+6.0.0 (2020-11-18)
+------------------
+* Bump rclcpp packages to Quality Level 2 (`#1445 <https://github.com/ros2/rclcpp/issues/1445>`_)
+* Contributors: Louise Poubel
+
 5.1.0 (2020-11-02)
 ------------------
 * Update maintainers (`#1384 <https://github.com/ros2/rclcpp/issues/1384>`_)

rclcpp_components/CMakeLists.txt

@@ -7,7 +7,10 @@ if(NOT CMAKE_CXX_STANDARD)
   set(CMAKE_CXX_STANDARD 14)
 endif()
 if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-  add_compile_options(-Wall -Wextra -Wpedantic -Wnon-virtual-dtor -Woverloaded-virtual)
+  add_compile_options(
+    -Wall -Wextra -Wpedantic -Wnon-virtual-dtor -Woverloaded-virtual
+    -Wformat=2 -Wconversion -Wshadow -Wsign-conversion -Wold-style-cast -Wcast-qual
+  )
 endif()
 
 find_package(ament_cmake_ros REQUIRED)
@@ -65,6 +68,10 @@ endif()
 
 if(BUILD_TESTING)
   find_package(ament_lint_auto REQUIRED)
+  find_package(ament_cmake_google_benchmark REQUIRED)
+  find_package(benchmark REQUIRED)
+  # Give cppcheck hints about macro definitions coming from outside this package
+  get_target_property(ament_cmake_cppcheck_ADDITIONAL_INCLUDE_DIRS benchmark::benchmark INTERFACE_INCLUDE_DIRECTORIES)
   ament_lint_auto_find_test_dependencies()
 
   set(components "")
@@ -109,6 +116,14 @@ if(BUILD_TESTING)
   if(TARGET test_component_manager_api)
     target_link_libraries(test_component_manager_api component_manager)
   endif()
+
+  ament_add_google_benchmark(benchmark_components
+    test/benchmark/benchmark_components.cpp
+    APPEND_ENV AMENT_PREFIX_PATH=${CMAKE_CURRENT_BINARY_DIR}/test_ament_index/$<CONFIG>
+    APPEND_LIBRARY_DIRS "${append_library_dirs}")
+  if(TARGET benchmark_components)
+    target_link_libraries(benchmark_components component_manager)
+  endif()
 endif()
 
 install(

rclcpp_components/QUALITY_DECLARATION.md

@@ -2,7 +2,7 @@ This document is a declaration of software quality for the `rclcpp_components` p
 
 # rclcpp_components Quality Declaration
 
-The package `rclcpp_components` claims to be in the **Quality Level 2** category.
+The package `rclcpp_components` claims to be in the **Quality Level 1** category.
 
 Below are the rationales, notes, and caveats for this claim, organized by each requirement listed in the [Package Quality Categories in REP-2004](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#package-quality-categories) of the ROS2 developer guide.
 
@@ -80,13 +80,13 @@ The API is publicly available in its [ROS 2 API documentation](http://docs.ros2.
 
 The license for `rclcpp_components` is Apache 2.0, and a summary is in each source file, the type is declared in the [`package.xml`](./package.xml) manifest file, and a full copy of the license is in the [`LICENSE`](../LICENSE) file.
 
-There is an automated test which runs a linter that ensures each file has a license statement. [Here](http://build.ros2.org/view/Fpr/job/Fpr__rclcpp__ubuntu_focal_amd64/lastSuccessfulBuild/testReport/rclcpp_components/) can be found a list with the latest results of the various linters being run on the package.
+There is an automated test which runs a linter that ensures each file has a license statement. [Here](http://build.ros2.org/view/Rpr/job/Rpr__rclcpp__ubuntu_focal_amd64/lastCompletedBuild/testReport/rclcpp_components/) can be found a list with the latest results of the various linters being run on the package.
 
 ### Copyright Statements [3.iv]
 
 The copyright holders each provide a statement of copyright in each source code file in `rclcpp_components`.
 
-There is an automated test which runs a linter that ensures each file has at least one copyright statement. Latest linter result report can be seen [here](http://build.ros2.org/view/Fpr/job/Fpr__rclcpp__ubuntu_focal_amd64/lastSuccessfulBuild/testReport/rclcpp_components/copyright).
+There is an automated test which runs a linter that ensures each file has at least one copyright statement. Latest linter result report can be seen [here](http://build.ros2.org/view/Rpr/job/Rpr__rclcpp__ubuntu_focal_amd64/lastCompletedBuild/testReport/rclcpp_components/copyright).
 
 ## Testing [4]
 
@@ -119,11 +119,19 @@ This includes:
 
 Changes are required to make a best effort to keep or increase coverage before being accepted, but decreases are allowed if properly justified and accepted by reviewers.
 
-Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastSuccessfulBuild/cobertura/src_ros2_rclcpp_rclcpp_components_src/). A description of how coverage statistics are calculated is summarized in this page ["ROS 2 Onboarding Guide"](https://index.ros.org/doc/ros2/Contributing/ROS-2-On-boarding-Guide/#note-on-coverage-runs).
+Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastCompletedBuild/cobertura/src_ros2_rclcpp_rclcpp_components_src/). A description of how coverage statistics are calculated is summarized in this page ["ROS 2 Onboarding Guide"](https://index.ros.org/doc/ros2/Contributing/ROS-2-On-boarding-Guide/#note-on-coverage-runs).
 
 ### Performance [4.iv]
 
-It is not yet defined if this package requires performance testing and how addresses this topic.
+`rclcpp_components` follows the recommendations for performance testing of C/C++ code in the [ROS 2 Developer Guide](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#performance), and opts to do performance analysis on each release rather than each change.
+
+The performance tests of `rclcpp_components` are located in the [test/benchmark directory](https://github.com/ros2/rclcpp/tree/master/rclcpp_components/test/benchmark).
+
+Package and system level performance benchmarks that cover features of `rclcpp_components` can be found at:
+* [Benchmarks](http://build.ros2.org/view/Rci/job/Rci__benchmark_ubuntu_focal_amd64/BenchmarkTable/)
+* [Performance](http://build.ros2.org/view/Rci/job/Rci__nightly-performance_ubuntu_focal_amd64/lastCompletedBuild/)
+
+Changes that introduce regressions in performance must be adequately justified in order to be accepted and merged.
 
 ### Linters and Static Analysis [4.v]
 
@@ -157,25 +165,25 @@ It is **Quality Level 1**, see its [Quality Declaration document](https://github
 
 The `class_loader` package provides a ROS-independent package for loading plugins during runtime
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros/class_loader/blob/ros2/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros/class_loader/blob/ros2/QUALITY_DECLARATION.md).
 
 #### `composition_interfaces`
 
 The `composition_interfaces` package contains message and service definitions for managing composable nodes in a container process.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcl_interfaces/blob/master/composition_interfaces/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl_interfaces/blob/master/composition_interfaces/QUALITY_DECLARATION.md).
 
 #### `rclcpp`
 
 The `rclcpp` package provides the ROS client library in C++.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rclcpp/blob/master/rclcpp/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rclcpp/blob/master/rclcpp/QUALITY_DECLARATION.md).
 
 #### `rcpputils`
 
 The `rcpputils` package provides an API which contains common utilities and data structures useful when programming in C++.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcpputils/blob/master/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcpputils/blob/master/QUALITY_DECLARATION.md).
 
 ### Direct Runtime non-ROS Dependency [5.iii]
 

rclcpp_components/README.md

@@ -6,4 +6,4 @@ Visit the [rclcpp_components API documentation](http://docs.ros2.org/latest/api/
 
 ## Quality Declaration
 
-This package claims to be in the **Quality Level 2** category, see the [Quality Declaration](QUALITY_DECLARATION.md) for more details.
+This package claims to be in the **Quality Level 1** category, see the [Quality Declaration](QUALITY_DECLARATION.md) for more details.

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>5.1.0</version>
+  <version>6.3.0</version>
   <description>Package containing tools for dynamically loadable components</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="mabel@openrobotics.org">Mabel Zhang</maintainer>
@@ -23,6 +23,7 @@
   <exec_depend>composition_interfaces</exec_depend>
   <exec_depend>rclcpp</exec_depend>
 
+  <test_depend>ament_cmake_google_benchmark</test_depend>
   <test_depend>ament_cmake_gtest</test_depend>
   <test_depend>ament_lint_auto</test_depend>
   <test_depend>ament_lint_common</test_depend>

rclcpp_components/src/component_manager.cpp

@@ -223,7 +223,7 @@ ComponentManager::OnLoadNode(
     response->error_message = "Failed to find class with the requested plugin name.";
     response->success = false;
   } catch (const ComponentManagerException & ex) {
-    RCLCPP_ERROR(get_logger(), ex.what());
+    RCLCPP_ERROR(get_logger(), "%s", ex.what());
     response->error_message = ex.what();
     response->success = false;
   }
@@ -244,7 +244,7 @@ ComponentManager::OnUnloadNode(
     std::stringstream ss;
     ss << "No node found with unique_id: " << request->unique_id;
     response->error_message = ss.str();
-    RCLCPP_WARN(get_logger(), ss.str());
+    RCLCPP_WARN(get_logger(), "%s", ss.str().c_str());
   } else {
     if (auto exec = executor_.lock()) {
       exec->remove_node(wrapper->second.get_node_base_interface());

rclcpp_components/test/benchmark/benchmark_components.cpp

@@ -0,0 +1,121 @@
+// 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 "benchmark/benchmark.h"
+
+#include <rcutils/logging.h>
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "rclcpp_components/component_manager.hpp"
+
+class ComponentTest : public benchmark::Fixture
+{
+public:
+  ComponentTest()
+  : component_manager_name("my_manager")
+  {
+  }
+
+#ifdef __GNUC__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Woverloaded-virtual"
+#endif
+  void SetUp(benchmark::State &) override
+  {
+    rcutils_logging_set_default_logger_level(RCUTILS_LOG_SEVERITY_WARN);
+
+    context = std::make_shared<rclcpp::Context>();
+    context->init(0, nullptr, rclcpp::InitOptions().auto_initialize_logging(false));
+
+    rclcpp::ExecutorOptions exec_options;
+    exec_options.context = context;
+
+    executor = std::make_shared<rclcpp::executors::SingleThreadedExecutor>(exec_options);
+
+    manager = std::make_shared<rclcpp_components::ComponentManager>(
+      executor, component_manager_name, rclcpp::NodeOptions().context(context));
+    executor->add_node(manager);
+  }
+
+  void TearDown(benchmark::State &) override
+  {
+    context->shutdown("Test is complete");
+
+    manager.reset();
+    executor.reset();
+    context.reset();
+  }
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+
+  const std::string component_manager_name;
+
+protected:
+  rclcpp::Context::SharedPtr context;
+  rclcpp::executors::SingleThreadedExecutor::SharedPtr executor;
+  std::shared_ptr<rclcpp_components::ComponentManager> manager;
+};
+
+BENCHMARK_F(ComponentTest, get_component_resources)(benchmark::State & state)
+{
+  for (auto _ : state) {
+    std::vector<rclcpp_components::ComponentManager::ComponentResource> resources =
+      manager->get_component_resources("rclcpp_components");
+    if (resources.size() != 3) {
+      state.SkipWithError("Wrong number of components found");
+      break;
+    }
+  }
+}
+
+BENCHMARK_F(ComponentTest, create_component_factory)(benchmark::State & state)
+{
+  const std::vector<rclcpp_components::ComponentManager::ComponentResource> resources =
+    manager->get_component_resources("rclcpp_components");
+  if (resources.size() != 3) {
+    state.SkipWithError("Wrong number of components found");
+    return;
+  }
+
+  for (auto _ : state) {
+    manager->create_component_factory(resources[0]).reset();
+  }
+}
+
+BENCHMARK_F(ComponentTest, create_node_instance)(benchmark::State & state)
+{
+  const std::vector<rclcpp_components::ComponentManager::ComponentResource> resources =
+    manager->get_component_resources("rclcpp_components");
+  if (resources.size() != 3) {
+    state.SkipWithError("Wrong number of components found");
+    return;
+  }
+
+  // Choosing resource 0 - the other two test components were shown empirically to yield
+  // the same performance charactarisitics, so they shouldn't need their own benchmarks.
+  const std::shared_ptr<rclcpp_components::NodeFactory> factory =
+    manager->create_component_factory(resources[0]);
+
+  const rclcpp::NodeOptions options = rclcpp::NodeOptions().context(context);
+
+  for (auto _ : state) {
+    rclcpp_components::NodeInstanceWrapper node = factory->create_node_instance(options);
+    benchmark::DoNotOptimize(node);
+    benchmark::ClobberMemory();
+  }
+}

rclcpp_components/test/test_component_manager.cpp

@@ -51,9 +51,10 @@ TEST_F(TestComponentManager, get_component_resources_valid)
   EXPECT_EQ("test_rclcpp_components::TestComponentBar", resources[1].first);
   EXPECT_EQ("test_rclcpp_components::TestComponentNoNode", resources[2].first);
 
-  EXPECT_TRUE(rcpputils::fs::path(resources[0].second).exists());
-  EXPECT_TRUE(rcpputils::fs::path(resources[1].second).exists());
-  EXPECT_TRUE(rcpputils::fs::path(resources[2].second).exists());
+  namespace fs = rcpputils::fs;
+  EXPECT_TRUE(fs::exists(fs::path(resources[0].second)));
+  EXPECT_TRUE(fs::exists(fs::path(resources[1].second)));
+  EXPECT_TRUE(fs::exists(fs::path(resources[2].second)));
 }
 
 TEST_F(TestComponentManager, create_component_factory_valid)
@@ -85,8 +86,8 @@ TEST_F(TestComponentManager, create_component_factory_invalid)
     rclcpp_components::ComponentManagerException);
 
   // Test valid library with invalid class
-  auto resources = manager->get_component_resources("rclcpp_components");
-  auto factory = manager->create_component_factory({"foo_class", resources[0].second});
+  auto component_resources = manager->get_component_resources("rclcpp_components");
+  auto factory = manager->create_component_factory({"foo_class", component_resources[0].second});
   EXPECT_EQ(nullptr, factory);
 
   // Test improperly formed resources file

rclcpp_components/test/test_component_manager_api.cpp

@@ -45,10 +45,10 @@ TEST_F(TestComponentManager, components_api)
   exec->add_node(manager);
   exec->add_node(node);
 
-  auto client = node->create_client<composition_interfaces::srv::LoadNode>(
+  auto composition_client = node->create_client<composition_interfaces::srv::LoadNode>(
     "/ComponentManager/_container/load_node");
 
-  if (!client->wait_for_service(20s)) {
+  if (!composition_client->wait_for_service(20s)) {
     ASSERT_TRUE(false) << "service not available after waiting";
   }
 
@@ -57,7 +57,7 @@ TEST_F(TestComponentManager, components_api)
     request->package_name = "rclcpp_components";
     request->plugin_name = "test_rclcpp_components::TestComponentFoo";
 
-    auto result = client->async_send_request(request);
+    auto result = composition_client->async_send_request(request);
     auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
     EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
     EXPECT_EQ(result.get()->success, true);
@@ -71,7 +71,7 @@ TEST_F(TestComponentManager, components_api)
     request->package_name = "rclcpp_components";
     request->plugin_name = "test_rclcpp_components::TestComponentBar";
 
-    auto result = client->async_send_request(request);
+    auto result = composition_client->async_send_request(request);
     auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
     EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
     EXPECT_EQ(result.get()->success, true);
@@ -87,7 +87,7 @@ TEST_F(TestComponentManager, components_api)
     request->plugin_name = "test_rclcpp_components::TestComponentFoo";
     request->node_name = "test_component_baz";
 
-    auto result = client->async_send_request(request);
+    auto result = composition_client->async_send_request(request);
     auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
     EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
     EXPECT_EQ(result.get()->success, true);
@@ -104,7 +104,7 @@ TEST_F(TestComponentManager, components_api)
     request->node_namespace = "/ns";
     request->node_name = "test_component_bing";
 
-    auto result = client->async_send_request(request);
+    auto result = composition_client->async_send_request(request);
     auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
     EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
     EXPECT_EQ(result.get()->success, true);
@@ -119,7 +119,7 @@ TEST_F(TestComponentManager, components_api)
     request->package_name = "rclcpp_components";
     request->plugin_name = "test_rclcpp_components::TestComponent";
 
-    auto result = client->async_send_request(request);
+    auto result = composition_client->async_send_request(request);
     auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
     EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
     EXPECT_EQ(result.get()->success, false);
@@ -134,7 +134,7 @@ TEST_F(TestComponentManager, components_api)
     request->package_name = "rclcpp_components_foo";
     request->plugin_name = "test_rclcpp_components::TestComponentFoo";
 
-    auto result = client->async_send_request(request);
+    auto result = composition_client->async_send_request(request);
     auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
     EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
     EXPECT_EQ(result.get()->success, false);
@@ -151,7 +151,7 @@ TEST_F(TestComponentManager, components_api)
     request->node_name = "test_component_remap";
     request->remap_rules.push_back("alice:=bob");
 
-    auto result = client->async_send_request(request);
+    auto result = composition_client->async_send_request(request);
     auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
     EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
     EXPECT_EQ(result.get()->success, true);
@@ -170,7 +170,7 @@ TEST_F(TestComponentManager, components_api)
       rclcpp::ParameterValue(true));
     request->extra_arguments.push_back(use_intraprocess_comms.to_parameter_msg());
 
-    auto result = client->async_send_request(request);
+    auto result = composition_client->async_send_request(request);
     auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
     EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
     EXPECT_EQ(result.get()->success, true);
@@ -191,7 +191,7 @@ TEST_F(TestComponentManager, components_api)
       rclcpp::ParameterValue("hello"));
     request->extra_arguments.push_back(use_intraprocess_comms.to_parameter_msg());
 
-    auto result = client->async_send_request(request);
+    auto result = composition_client->async_send_request(request);
     auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
     EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
     EXPECT_EQ(result.get()->success, false);
@@ -226,23 +226,23 @@ TEST_F(TestComponentManager, components_api)
       auto result = client->async_send_request(request);
       auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
       EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
-      auto node_names = result.get()->full_node_names;
-      auto unique_ids = result.get()->unique_ids;
+      auto result_node_names = result.get()->full_node_names;
+      auto result_unique_ids = result.get()->unique_ids;
 
-      EXPECT_EQ(node_names.size(), 6u);
-      EXPECT_EQ(node_names[0], "/test_component_foo");
-      EXPECT_EQ(node_names[1], "/test_component_bar");
-      EXPECT_EQ(node_names[2], "/test_component_baz");
-      EXPECT_EQ(node_names[3], "/ns/test_component_bing");
-      EXPECT_EQ(node_names[4], "/test_component_remap");
-      EXPECT_EQ(node_names[5], "/test_component_intra_process");
-      EXPECT_EQ(unique_ids.size(), 6u);
-      EXPECT_EQ(unique_ids[0], 1u);
-      EXPECT_EQ(unique_ids[1], 2u);
-      EXPECT_EQ(unique_ids[2], 3u);
-      EXPECT_EQ(unique_ids[3], 4u);
-      EXPECT_EQ(unique_ids[4], 5u);
-      EXPECT_EQ(unique_ids[5], 6u);
+      EXPECT_EQ(result_node_names.size(), 6u);
+      EXPECT_EQ(result_node_names[0], "/test_component_foo");
+      EXPECT_EQ(result_node_names[1], "/test_component_bar");
+      EXPECT_EQ(result_node_names[2], "/test_component_baz");
+      EXPECT_EQ(result_node_names[3], "/ns/test_component_bing");
+      EXPECT_EQ(result_node_names[4], "/test_component_remap");
+      EXPECT_EQ(result_node_names[5], "/test_component_intra_process");
+      EXPECT_EQ(result_unique_ids.size(), 6u);
+      EXPECT_EQ(result_unique_ids[0], 1u);
+      EXPECT_EQ(result_unique_ids[1], 2u);
+      EXPECT_EQ(result_unique_ids[2], 3u);
+      EXPECT_EQ(result_unique_ids[3], 4u);
+      EXPECT_EQ(result_unique_ids[4], 5u);
+      EXPECT_EQ(result_unique_ids[5], 6u);
     }
   }
 
@@ -290,21 +290,21 @@ TEST_F(TestComponentManager, components_api)
       auto result = client->async_send_request(request);
       auto ret = exec->spin_until_future_complete(result, 5s);  // Wait for the result.
       EXPECT_EQ(ret, rclcpp::FutureReturnCode::SUCCESS);
-      auto node_names = result.get()->full_node_names;
-      auto unique_ids = result.get()->unique_ids;
+      auto result_node_names = result.get()->full_node_names;
+      auto result_unique_ids = result.get()->unique_ids;
 
-      EXPECT_EQ(node_names.size(), 5u);
-      EXPECT_EQ(node_names[0], "/test_component_bar");
-      EXPECT_EQ(node_names[1], "/test_component_baz");
-      EXPECT_EQ(node_names[2], "/ns/test_component_bing");
-      EXPECT_EQ(node_names[3], "/test_component_remap");
-      EXPECT_EQ(node_names[4], "/test_component_intra_process");
-      EXPECT_EQ(unique_ids.size(), 5u);
-      EXPECT_EQ(unique_ids[0], 2u);
-      EXPECT_EQ(unique_ids[1], 3u);
-      EXPECT_EQ(unique_ids[2], 4u);
-      EXPECT_EQ(unique_ids[3], 5u);
-      EXPECT_EQ(unique_ids[4], 6u);
+      EXPECT_EQ(result_node_names.size(), 5u);
+      EXPECT_EQ(result_node_names[0], "/test_component_bar");
+      EXPECT_EQ(result_node_names[1], "/test_component_baz");
+      EXPECT_EQ(result_node_names[2], "/ns/test_component_bing");
+      EXPECT_EQ(result_node_names[3], "/test_component_remap");
+      EXPECT_EQ(result_node_names[4], "/test_component_intra_process");
+      EXPECT_EQ(result_unique_ids.size(), 5u);
+      EXPECT_EQ(result_unique_ids[0], 2u);
+      EXPECT_EQ(result_unique_ids[1], 3u);
+      EXPECT_EQ(result_unique_ids[2], 4u);
+      EXPECT_EQ(result_unique_ids[3], 5u);
+      EXPECT_EQ(result_unique_ids[4], 6u);
     }
   }
 }

rclcpp_lifecycle/CHANGELOG.rst

@@ -3,6 +3,26 @@ Changelog for package rclcpp_lifecycle
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+6.3.0 (2021-01-25)
+------------------
+
+6.2.0 (2021-01-08)
+------------------
+
+6.1.0 (2020-12-10)
+------------------
+* add LifecycleNode::get_transition_graph to match services. (`#1472 <https://github.com/ros2/rclcpp/issues/1472>`_)
+* Update QDs to QL 1 (`#1477 <https://github.com/ros2/rclcpp/issues/1477>`_)
+* Benchmark lifecycle features (`#1462 <https://github.com/ros2/rclcpp/issues/1462>`_)
+* Contributors: Stephen Brawner, brawner, tomoya
+
+6.0.0 (2020-11-18)
+------------------
+* Reserve vector capacities and use emplace_back for constructing vectors (`#1464 <https://github.com/ros2/rclcpp/issues/1464>`_)
+* [rclcpp_lifecycle] Change uint8_t iterator variables to size_t (`#1461 <https://github.com/ros2/rclcpp/issues/1461>`_)
+* Bump rclcpp packages to Quality Level 2 (`#1445 <https://github.com/ros2/rclcpp/issues/1445>`_)
+* Contributors: Louise Poubel, brawner
+
 5.1.0 (2020-11-02)
 ------------------
 * Increase test timeouts of slow running tests with rmw_connext_cpp (`#1400 <https://github.com/ros2/rclcpp/issues/1400>`_)

rclcpp_lifecycle/CMakeLists.txt

@@ -51,6 +51,35 @@ if(BUILD_TESTING)
   set(ament_cmake_cppcheck_ADDITIONAL_INCLUDE_DIRS ${rclcpp_INCLUDE_DIRS})
   ament_lint_auto_find_test_dependencies()
 
+  find_package(performance_test_fixture REQUIRED)
+
+  add_performance_test(
+    benchmark_lifecycle_client
+    test/benchmark/benchmark_lifecycle_client.cpp)
+  if(TARGET benchmark_lifecycle_client)
+    target_link_libraries(benchmark_lifecycle_client ${PROJECT_NAME})
+    ament_target_dependencies(benchmark_lifecycle_client rclcpp)
+  endif()
+  add_performance_test(
+    benchmark_lifecycle_node
+    test/benchmark/benchmark_lifecycle_node.cpp)
+  if(TARGET benchmark_lifecycle_node)
+    target_link_libraries(benchmark_lifecycle_node ${PROJECT_NAME})
+    ament_target_dependencies(benchmark_lifecycle_node rclcpp)
+  endif()
+  add_performance_test(
+    benchmark_state
+    test/benchmark/benchmark_state.cpp)
+  if(TARGET benchmark_state)
+    target_link_libraries(benchmark_state ${PROJECT_NAME})
+  endif()
+  add_performance_test(
+    benchmark_transition
+    test/benchmark/benchmark_transition.cpp)
+  if(TARGET benchmark_transition)
+    target_link_libraries(benchmark_transition ${PROJECT_NAME})
+  endif()
+
   ament_add_gtest(test_lifecycle_node test/test_lifecycle_node.cpp TIMEOUT 120)
   if(TARGET test_lifecycle_node)
     ament_target_dependencies(test_lifecycle_node

rclcpp_lifecycle/QUALITY_DECLARATION.md

@@ -2,7 +2,7 @@ This document is a declaration of software quality for the `rclcpp_lifecycle` pa
 
 # rclcpp_lifecycle Quality Declaration
 
-The package `rclcpp_lifecycle` claims to be in the **Quality Level 2** category.
+The package `rclcpp_lifecycle` claims to be in the **Quality Level 1** category when used with a **Quality Level 1** middleware.
 
 Below are the rationales, notes, and caveats for this claim, organized by each requirement listed in the [Package Quality Categories in REP-2004](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#package-quality-categories) of the ROS2 developer guide.
 
@@ -80,13 +80,13 @@ The API is publicly available in its [ROS 2 API documentation](http://docs.ros2.
 
 The license for `rclcpp_lifecycle` is Apache 2.0, and a summary is in each source file, the type is declared in the [`package.xml`](./package.xml) manifest file, and a full copy of the license is in the [`LICENSE`](../LICENSE) file.
 
-There is an automated test which runs a linter that ensures each file has a license statement. [Here](http://build.ros2.org/view/Fpr/job/Fpr__rclcpp__ubuntu_focal_amd64/lastSuccessfulBuild/testReport/rclcpp_lifecycle/) can be found a list with the latest results of the various linters being run on the package.
+There is an automated test which runs a linter that ensures each file has a license statement. [Here](http://build.ros2.org/view/Rpr/job/Rpr__rclcpp__ubuntu_focal_amd64/lastCompletedBuild/testReport/rclcpp_lifecycle/) can be found a list with the latest results of the various linters being run on the package.
 
 ### Copyright Statements [3.iv]
 
 The copyright holders each provide a statement of copyright in each source code file in `rclcpp_lifecycle`.
 
-There is an automated test which runs a linter that ensures each file has at least one copyright statement. Latest linter result report can be seen [here](http://build.ros2.org/view/Fpr/job/Fpr__rclcpp__ubuntu_focal_amd64/lastSuccessfulBuild/testReport/rclcpp_lifecycle/copyright/).
+There is an automated test which runs a linter that ensures each file has at least one copyright statement. Latest linter result report can be seen [here](http://build.ros2.org/view/Rpr/job/Rpr__rclcpp__ubuntu_focal_amd64/lastCompletedBuild/testReport/rclcpp_lifecycle/copyright/).
 
 ## Testing [4]
 
@@ -119,11 +119,19 @@ This includes:
 
 Changes are required to make a best effort to keep or increase coverage before being accepted, but decreases are allowed if properly justified and accepted by reviewers.
 
-Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastSuccessfulBuild/cobertura/src_ros2_rclcpp_rclcpp_lifecycle_src/). A description of how coverage statistics are calculated is summarized in this page ["ROS 2 Onboarding Guide"](https://index.ros.org/doc/ros2/Contributing/ROS-2-On-boarding-Guide/#note-on-coverage-runs).
+Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastCompletedBuild/cobertura/src_ros2_rclcpp_rclcpp_lifecycle_src/). A description of how coverage statistics are calculated is summarized in this page ["ROS 2 Onboarding Guide"](https://index.ros.org/doc/ros2/Contributing/ROS-2-On-boarding-Guide/#note-on-coverage-runs).
 
 ### Performance [4.iv]
 
-It is not yet defined if this package requires performance testing and how addresses this topic.
+`rclcpp_lifecycle` follows the recommendations for performance testing of C/C++ code in the [ROS 2 Developer Guide](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#performance), and opts to do performance analysis on each release rather than each change.
+
+The performance tests of `rclcpp_lifecycle` are located in the [test/benchmark directory](https://github.com/ros2/rclcpp/tree/master/rclcpp_lifecycle/test/benchmark).
+
+Package and system level performance benchmarks that cover features of `rclcpp_lifecycle` can be found at:
+* [Benchmarks](http://build.ros2.org/view/Rci/job/Rci__benchmark_ubuntu_focal_amd64/BenchmarkTable/)
+* [Performance](http://build.ros2.org/view/Rci/job/Rci__nightly-performance_ubuntu_focal_amd64/lastCompletedBuild/)
+
+Changes that introduce regressions in performance must be adequately justified in order to be accepted and merged.
 
 ### Linters and Static Analysis [4.v]
 
@@ -151,31 +159,31 @@ It also has several test dependencies, which do not affect the resulting quality
 
 The `lifecycle_msgs` package contains message and service definitions for managing lifecycle nodes.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcl_interfaces/blob/master/lifecycle_msgs/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl_interfaces/blob/master/lifecycle_msgs/QUALITY_DECLARATION.md).
 
 #### `rclcpp`
 
 The `rclcpp` package provides the ROS client library in C++.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rclcpp/blob/master/rclcpp/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rclcpp/blob/master/rclcpp/QUALITY_DECLARATION.md).
 
 #### `rcl_lifecycle`
 
 The `rcl_lifecycle` package provides functionality for ROS 2 lifecycle nodes in C.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rcl/blob/master/rcl_lifecycle/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rcl/blob/master/rcl_lifecycle/QUALITY_DECLARATION.md).
 
 #### `rosidl_typesupport_cpp`
 
 The `rosidl_typesupport_cpp` package generates the type support for C++ messages.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rosidl_typesupport/blob/master/rosidl_typesupport_cpp/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rosidl_typesupport/blob/master/rosidl_typesupport_cpp/QUALITY_DECLARATION.md).
 
 #### `rmw`
 
 `rmw` is the ROS 2 middleware library.
 
-It is **Quality Level 2**, see its [Quality Declaration document](https://github.com/ros2/rmw/blob/master/rmw/QUALITY_DECLARATION.md).
+It is **Quality Level 1**, see its [Quality Declaration document](https://github.com/ros2/rmw/blob/master/rmw/QUALITY_DECLARATION.md).
 
 ### Direct Runtime non-ROS Dependency [5.iii]