8.2.0

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

rclcpp/CHANGELOG.rst

@@ -2,6 +2,15 @@
 Changelog for package rclcpp
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+8.2.0 (2021-03-31)
+------------------
+* Initialize integers in test_parameter_event_handler.cpp to avoid undefined behavior (`#1609 <https://github.com/ros2/rclcpp/issues/1609>`_)
+* Namespace tracetools C++ functions (`#1608 <https://github.com/ros2/rclcpp/issues/1608>`_)
+* Revert "Namespace tracetools C++ functions (`#1603 <https://github.com/ros2/rclcpp/issues/1603>`_)" (`#1607 <https://github.com/ros2/rclcpp/issues/1607>`_)
+* Namespace tracetools C++ functions (`#1603 <https://github.com/ros2/rclcpp/issues/1603>`_)
+* Clock subscription callback group spins in its own thread (`#1556 <https://github.com/ros2/rclcpp/issues/1556>`_)
+* Contributors: Chris Lalancette, Christophe Bedard, Ivan Santiago Paunovic, anaelle-sw
+
 8.1.0 (2021-03-25)
 ------------------
 * Remove rmw_connext_cpp references. (`#1595 <https://github.com/ros2/rclcpp/issues/1595>`_)

rclcpp/include/rclcpp/any_service_callback.hpp

@@ -107,12 +107,12 @@ public:
       TRACEPOINT(
         rclcpp_callback_register,
         static_cast<const void *>(this),
-        get_symbol(shared_ptr_callback_));
+        tracetools::get_symbol(shared_ptr_callback_));
     } else if (shared_ptr_with_request_header_callback_) {
       TRACEPOINT(
         rclcpp_callback_register,
         static_cast<const void *>(this),
-        get_symbol(shared_ptr_with_request_header_callback_));
+        tracetools::get_symbol(shared_ptr_with_request_header_callback_));
     }
 #endif  // TRACETOOLS_DISABLED
   }

rclcpp/include/rclcpp/any_subscription_callback.hpp

@@ -243,22 +243,22 @@ public:
       TRACEPOINT(
         rclcpp_callback_register,
         static_cast<const void *>(this),
-        get_symbol(shared_ptr_callback_));
+        tracetools::get_symbol(shared_ptr_callback_));
     } else if (shared_ptr_with_info_callback_) {
       TRACEPOINT(
         rclcpp_callback_register,
         static_cast<const void *>(this),
-        get_symbol(shared_ptr_with_info_callback_));
+        tracetools::get_symbol(shared_ptr_with_info_callback_));
     } else if (unique_ptr_callback_) {
       TRACEPOINT(
         rclcpp_callback_register,
         static_cast<const void *>(this),
-        get_symbol(unique_ptr_callback_));
+        tracetools::get_symbol(unique_ptr_callback_));
     } else if (unique_ptr_with_info_callback_) {
       TRACEPOINT(
         rclcpp_callback_register,
         static_cast<const void *>(this),
-        get_symbol(unique_ptr_with_info_callback_));
+        tracetools::get_symbol(unique_ptr_with_info_callback_));
     }
 #endif  // TRACETOOLS_DISABLED
   }

rclcpp/include/rclcpp/node_interfaces/node_time_source.hpp

@@ -48,7 +48,8 @@ public:
     rclcpp::node_interfaces::NodeLoggingInterface::SharedPtr node_logging,
     rclcpp::node_interfaces::NodeClockInterface::SharedPtr node_clock,
     rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters,
-    const rclcpp::QoS & qos = rclcpp::RosoutQoS()
+    const rclcpp::QoS & qos = rclcpp::RosoutQoS(),
+    bool use_clock_thread = true
   );
 
   RCLCPP_PUBLIC

rclcpp/include/rclcpp/node_options.hpp

@@ -47,6 +47,7 @@ public:
    *   - start_parameter_services = true
    *   - start_parameter_event_publisher = true
    *   - clock_qos = rclcpp::ClockQoS()
+   *   - use_clock_thread = true
    *   - rosout_qos = rclcpp::RosoutQoS()
    *   - parameter_event_qos = rclcpp::ParameterEventQoS
    *     - with history setting and depth from rmw_qos_profile_parameter_events
@@ -258,6 +259,20 @@ public:
   NodeOptions &
   clock_qos(const rclcpp::QoS & clock_qos);
 
+
+  /// Return the use_clock_thread flag.
+  RCLCPP_PUBLIC
+  bool
+  use_clock_thread() const;
+
+  /// Set the use_clock_thread flag, return this for parameter idiom.
+  /**
+   * If true, a dedicated thread will be used to subscribe to "/clock" topic.
+   */
+  RCLCPP_PUBLIC
+  NodeOptions &
+  use_clock_thread(bool use_clock_thread);
+
   /// Return a reference to the parameter_event_qos QoS.
   RCLCPP_PUBLIC
   const rclcpp::QoS &
@@ -384,6 +399,8 @@ private:
 
   rclcpp::QoS clock_qos_ = rclcpp::ClockQoS();
 
+  bool use_clock_thread_ {true};
+
   rclcpp::QoS parameter_event_qos_ = rclcpp::ParameterEventsQoS(
     rclcpp::QoSInitialization::from_rmw(rmw_qos_profile_parameter_events)
   );

rclcpp/include/rclcpp/time_source.hpp

@@ -25,6 +25,7 @@
 #include "rcl_interfaces/msg/parameter_event.hpp"
 
 #include "rclcpp/node.hpp"
+#include "rclcpp/executors.hpp"
 #include "rclcpp/node_interfaces/node_parameters_interface.hpp"
 
 
@@ -57,7 +58,10 @@ public:
    * \param qos QoS that will be used when creating a `/clock` subscription.
    */
   RCLCPP_PUBLIC
-  explicit TimeSource(rclcpp::Node::SharedPtr node, const rclcpp::QoS & qos = rclcpp::ClockQoS());
+  explicit TimeSource(
+    rclcpp::Node::SharedPtr node,
+    const rclcpp::QoS & qos = rclcpp::ClockQoS(),
+    bool use_clock_thread = true);
 
   /// Empty constructor
   /**
@@ -66,7 +70,9 @@ public:
    * \param qos QoS that will be used when creating a `/clock` subscription.
    */
   RCLCPP_PUBLIC
-  explicit TimeSource(const rclcpp::QoS & qos = rclcpp::ClockQoS());
+  explicit TimeSource(
+    const rclcpp::QoS & qos = rclcpp::ClockQoS(),
+    bool use_clock_thread = true);
 
   /// Attack node to the time source.
   /**
@@ -118,6 +124,11 @@ public:
   RCLCPP_PUBLIC
   ~TimeSource();
 
+protected:
+  // Dedicated thread for clock subscription.
+  bool use_clock_thread_;
+  std::thread clock_executor_thread_;
+
 private:
   // Preserve the node reference
   rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base_;
@@ -140,6 +151,9 @@ private:
   using SubscriptionT = rclcpp::Subscription<MessageT, Alloc>;
   std::shared_ptr<SubscriptionT> clock_subscription_{nullptr};
   std::mutex clock_sub_lock_;
+  rclcpp::CallbackGroup::SharedPtr clock_callback_group_;
+  rclcpp::executors::SingleThreadedExecutor clock_executor_;
+  std::promise<void> cancel_clock_executor_promise_;
 
   // The clock callback itself
   void clock_cb(const rosgraph_msgs::msg::Clock::SharedPtr msg);

rclcpp/include/rclcpp/timer.hpp

@@ -181,7 +181,7 @@ public:
     TRACEPOINT(
       rclcpp_callback_register,
       static_cast<const void *>(&callback_),
-      get_symbol(callback_));
+      tracetools::get_symbol(callback_));
   }
 
   /// Default destructor.

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>8.1.0</version>
+  <version>8.2.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/src/rclcpp/node.cpp

@@ -187,7 +187,8 @@ Node::Node(
       node_logging_,
       node_clock_,
       node_parameters_,
-      options.clock_qos()
+      options.clock_qos(),
+      options.use_clock_thread()
     )),
   node_waitables_(new rclcpp::node_interfaces::NodeWaitables(node_base_.get())),
   node_options_(options),

rclcpp/src/rclcpp/node_interfaces/node_time_source.cpp

@@ -27,7 +27,8 @@ NodeTimeSource::NodeTimeSource(
   rclcpp::node_interfaces::NodeLoggingInterface::SharedPtr node_logging,
   rclcpp::node_interfaces::NodeClockInterface::SharedPtr node_clock,
   rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters,
-  const rclcpp::QoS & qos)
+  const rclcpp::QoS & qos,
+  bool use_clock_thread)
 : node_base_(node_base),
   node_topics_(node_topics),
   node_graph_(node_graph),
@@ -35,7 +36,7 @@ NodeTimeSource::NodeTimeSource(
   node_logging_(node_logging),
   node_clock_(node_clock),
   node_parameters_(node_parameters),
-  time_source_(qos)
+  time_source_(qos, use_clock_thread)
 {
   time_source_.attachNode(
     node_base_,

rclcpp/src/rclcpp/node_options.cpp

@@ -78,6 +78,7 @@ NodeOptions::operator=(const NodeOptions & other)
     this->start_parameter_services_ = other.start_parameter_services_;
     this->start_parameter_event_publisher_ = other.start_parameter_event_publisher_;
     this->clock_qos_ = other.clock_qos_;
+    this->use_clock_thread_ = other.use_clock_thread_;
     this->parameter_event_qos_ = other.parameter_event_qos_;
     this->rosout_qos_ = other.rosout_qos_;
     this->parameter_event_publisher_options_ = other.parameter_event_publisher_options_;
@@ -272,6 +273,19 @@ NodeOptions::clock_qos(const rclcpp::QoS & clock_qos)
   return *this;
 }
 
+bool
+NodeOptions::use_clock_thread() const
+{
+  return this->use_clock_thread_;
+}
+
+NodeOptions &
+NodeOptions::use_clock_thread(bool use_clock_thread)
+{
+  this->use_clock_thread_ = use_clock_thread;
+  return *this;
+}
+
 const rclcpp::QoS &
 NodeOptions::parameter_event_qos() const
 {

rclcpp/src/rclcpp/time_source.cpp

@@ -33,21 +33,29 @@
 namespace rclcpp
 {
 
-TimeSource::TimeSource(std::shared_ptr<rclcpp::Node> node, const rclcpp::QoS & qos)
-: logger_(rclcpp::get_logger("rclcpp")),
+TimeSource::TimeSource(
+  std::shared_ptr<rclcpp::Node> node,
+  const rclcpp::QoS & qos,
+  bool use_clock_thread)
+: use_clock_thread_(use_clock_thread),
+  logger_(rclcpp::get_logger("rclcpp")),
   qos_(qos)
 {
   this->attachNode(node);
 }
 
-TimeSource::TimeSource(const rclcpp::QoS & qos)
-: logger_(rclcpp::get_logger("rclcpp")),
+TimeSource::TimeSource(
+  const rclcpp::QoS & qos,
+  bool use_clock_thread)
+: use_clock_thread_(use_clock_thread),
+  logger_(rclcpp::get_logger("rclcpp")),
   qos_(qos)
 {
 }
 
 void TimeSource::attachNode(rclcpp::Node::SharedPtr node)
 {
+  use_clock_thread_ = node->get_node_options().use_clock_thread();
   attachNode(
     node->get_node_base_interface(),
     node->get_node_topics_interface(),
@@ -127,7 +135,7 @@ void TimeSource::attachNode(
 void TimeSource::detachNode()
 {
   this->ros_time_active_ = false;
-  clock_subscription_.reset();
+  destroy_clock_sub();
   parameter_subscription_.reset();
   node_base_.reset();
   node_topics_.reset();
@@ -242,6 +250,24 @@ void TimeSource::create_clock_sub()
       rclcpp::QosPolicyKind::Reliability,
     });
 
+  if (use_clock_thread_) {
+    clock_callback_group_ = node_base_->create_callback_group(
+      rclcpp::CallbackGroupType::MutuallyExclusive,
+      false
+    );
+    options.callback_group = clock_callback_group_;
+    if (!clock_executor_thread_.joinable()) {
+      clock_executor_thread_ = std::thread(
+        [this]() {
+          cancel_clock_executor_promise_ = std::promise<void>{};
+          auto future = cancel_clock_executor_promise_.get_future();
+          clock_executor_.add_callback_group(clock_callback_group_, node_base_);
+          clock_executor_.spin_until_future_complete(future);
+        }
+      );
+    }
+  }
+
   clock_subscription_ = rclcpp::create_subscription<rosgraph_msgs::msg::Clock>(
     node_parameters_,
     node_topics_,
@@ -255,6 +281,12 @@ void TimeSource::create_clock_sub()
 void TimeSource::destroy_clock_sub()
 {
   std::lock_guard<std::mutex> guard(clock_sub_lock_);
+  if (clock_executor_thread_.joinable()) {
+    cancel_clock_executor_promise_.set_value();
+    clock_executor_.cancel();
+    clock_executor_thread_.join();
+    clock_executor_.remove_callback_group(clock_callback_group_);
+  }
   clock_subscription_.reset();
 }
 

rclcpp/test/rclcpp/test_parameter_event_handler.cpp

@@ -149,8 +149,8 @@ TEST_F(TestNode, RegisterParameterCallback)
 
 TEST_F(TestNode, SameParameterDifferentNode)
 {
-  int64_t int_param_node1;
-  int64_t int_param_node2;
+  int64_t int_param_node1{0};
+  int64_t int_param_node2{0};
 
   auto cb1 = [&int_param_node1](const rclcpp::Parameter & p) {
       int_param_node1 = p.get_value<int64_t>();

rclcpp/test/rclcpp/test_time_source.cpp

@@ -516,3 +516,229 @@ TEST_F(TestTimeSource, no_pre_jump_callback) {
   EXPECT_EQ(1, cbo.last_postcallback_id_);
   EXPECT_EQ(1, cbo.post_callback_calls_);
 }
+
+// A TimeSource-inheriting class
+// that allows access to TimeSource protected attributes
+// use_clock_thread_ and clock_executor_thread_
+class ClockThreadTestingTimeSource : public rclcpp::TimeSource
+{
+public:
+  ClockThreadTestingTimeSource()
+  : rclcpp::TimeSource()
+  {
+  }
+
+  bool GetUseClockThreadOption()
+  {
+    return this->use_clock_thread_;
+  }
+
+  bool IsClockThreadJoinable()
+  {
+    return this->clock_executor_thread_.joinable();
+  }
+};
+
+TEST_F(TestTimeSource, check_use_clock_thread_value) {
+  // Create three nodes, with use_clock_thread option
+  // respectively set to default, true, and false
+
+  auto default_node_ = std::make_shared<rclcpp::Node>(
+    "default_option_node");
+
+  auto clock_thread_node_ = std::make_shared<rclcpp::Node>(
+    "clock_thread_node",
+    rclcpp::NodeOptions().use_clock_thread(true));
+
+  auto no_clock_thread_node_ = std::make_shared<rclcpp::Node>(
+    "no_clock_thread_node",
+    rclcpp::NodeOptions().use_clock_thread(false));
+
+  // Test value of use_clock_thread_ TimeSource attribute
+  // when the different nodes are attached
+
+  ClockThreadTestingTimeSource ts;
+
+  ts.attachNode(default_node_);
+  ASSERT_TRUE(ts.GetUseClockThreadOption());
+  ts.detachNode();
+
+  ts.attachNode(clock_thread_node_);
+  ASSERT_TRUE(ts.GetUseClockThreadOption());
+  ts.detachNode();
+
+  ts.attachNode(no_clock_thread_node_);
+  ASSERT_FALSE(ts.GetUseClockThreadOption());
+  ts.detachNode();
+}
+
+TEST_F(TestTimeSource, check_clock_thread_status) {
+  // Test if TimeSource clock-dedicated thread is running
+  // according to the use_sim_time parameter
+  // and to the options of the attached node
+  ClockThreadTestingTimeSource ts;
+
+  // Tests for default options node
+  auto default_node_ = std::make_shared<rclcpp::Node>(
+    "default_option_node");
+
+  default_node_->set_parameter(rclcpp::Parameter("use_sim_time", true));
+  ts.attachNode(default_node_);
+  ASSERT_TRUE(ts.IsClockThreadJoinable());
+  ts.detachNode();
+
+  default_node_->set_parameter(rclcpp::Parameter("use_sim_time", false));
+  ts.attachNode(default_node_);
+  ASSERT_FALSE(ts.IsClockThreadJoinable());
+  ts.detachNode();
+
+  // Tests for node with use_clock_thread option forced to false
+  auto no_clock_thread_node_ = std::make_shared<rclcpp::Node>(
+    "no_clock_thread_node",
+    rclcpp::NodeOptions().use_clock_thread(false));
+
+  no_clock_thread_node_->set_parameter(rclcpp::Parameter("use_sim_time", true));
+  ts.attachNode(no_clock_thread_node_);
+  ASSERT_FALSE(ts.IsClockThreadJoinable());
+  ts.detachNode();
+
+  no_clock_thread_node_->set_parameter(rclcpp::Parameter("use_sim_time", false));
+  ts.attachNode(no_clock_thread_node_);
+  ASSERT_FALSE(ts.IsClockThreadJoinable());
+  ts.detachNode();
+}
+
+// A Node-inheriting class
+// that regularly publishes a incremented Clock msg on topic `/clock'
+class SimClockPublisherNode : public rclcpp::Node
+{
+public:
+  SimClockPublisherNode()
+  : rclcpp::Node("sim_clock_publisher_node")
+  {
+    // Create a clock publisher
+    clock_pub_ = this->create_publisher<rosgraph_msgs::msg::Clock>(
+      "/clock",
+      rclcpp::QoS(1)
+    );
+
+    // Create a 1ms timer
+    pub_timer_ = this->create_wall_timer(
+      std::chrono::milliseconds(1),
+      std::bind(
+        &SimClockPublisherNode::timer_callback,
+        this)
+    );
+
+    // Init clock msg to zero
+    clock_msg_.clock.sec = 0;
+    clock_msg_.clock.nanosec = 0;
+  }
+
+  ~SimClockPublisherNode()
+  {
+    // Cleanly stop executor and thread
+    node_executor.cancel();
+    node_thread_.join();
+  }
+
+  void SpinNode()
+  {
+    // Spin node in its own dedicated thread
+    node_thread_ = std::thread(
+      [this]() {
+        node_executor.add_node(this->get_node_base_interface());
+        node_executor.spin();
+      });
+  }
+
+private:
+  void timer_callback()
+  {
+    // Increment clock msg and publish it
+    clock_msg_.clock.nanosec += 1000000;
+    clock_pub_->publish(clock_msg_);
+  }
+
+  rclcpp::Publisher<rosgraph_msgs::msg::Clock>::SharedPtr clock_pub_;
+  rclcpp::TimerBase::SharedPtr pub_timer_;
+  rosgraph_msgs::msg::Clock clock_msg_;
+  std::thread node_thread_;
+  rclcpp::executors::SingleThreadedExecutor node_executor;
+};
+
+// A Node-inheriting class
+// that check its clock time within a timer callback
+class ClockThreadTestingNode : public rclcpp::Node
+{
+public:
+  ClockThreadTestingNode()
+  : rclcpp::Node("clock_thread_testing_node")
+  {
+    // Set use_sim_time parameter to true to subscribe to `/clock` topic
+    this->set_parameter(rclcpp::Parameter("use_sim_time", true));
+
+    // Create a 100ms timer
+    timer_ = this->create_wall_timer(
+      std::chrono::milliseconds(100),
+      std::bind(
+        &ClockThreadTestingNode::timer_callback,
+        this)
+    );
+  }
+
+  bool GetIsCallbackFrozen()
+  {
+    return is_callback_frozen_;
+  }
+
+private:
+  void timer_callback()
+  {
+    rclcpp::Time start_time = this->now();
+    bool is_time_out = false;
+
+    // While loop condition tests
+    // if the node's clock time is incremented
+    while (rclcpp::ok() &&
+      !is_time_out)
+    {
+      rclcpp::sleep_for(std::chrono::milliseconds(100));
+      rclcpp::Time time_now = this->now();
+      rclcpp::Duration time_spent = time_now - start_time;
+      is_time_out = time_spent.seconds() > 1.0;
+    }
+
+    // If out of while loop, set variable to false
+    // and cancel timer to avoid to enter the callback again
+    is_callback_frozen_ = false;
+    timer_->cancel();
+  }
+
+  rclcpp::TimerBase::SharedPtr timer_;
+  bool is_callback_frozen_ = true;
+};
+
+TEST_F(TestTimeSource, check_sim_time_updated_in_callback_if_use_clock_thread) {
+  // Test if clock time of a node with
+  // parameter use_sim_time = true and option use_clock_thread = true
+  // is updated while node is not spinning
+  // (in a timer callback)
+
+  // Create a "sim time" publisher and spin it
+  SimClockPublisherNode pub_node;
+  pub_node.SpinNode();
+
+  // Spin node for 2 seconds
+  ClockThreadTestingNode clock_thread_testing_node;
+  auto steady_clock = rclcpp::Clock(RCL_STEADY_TIME);
+  auto start_time = steady_clock.now();
+  while (rclcpp::ok() &&
+    (steady_clock.now() - start_time).seconds() < 2.0)
+  {
+    rclcpp::spin_some(clock_thread_testing_node.get_node_base_interface());
+  }
+
+  // Node should have get out of timer callback
+  ASSERT_FALSE(clock_thread_testing_node.GetIsCallbackFrozen());
+}

rclcpp_action/CHANGELOG.rst

@@ -3,6 +3,9 @@ Changelog for package rclcpp_action
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+8.2.0 (2021-03-31)
+------------------
+
 8.1.0 (2021-03-25)
 ------------------
 

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>8.1.0</version>
+  <version>8.2.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_components/CHANGELOG.rst

@@ -2,6 +2,9 @@
 Changelog for package rclcpp_components
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+8.2.0 (2021-03-31)
+------------------
+
 8.1.0 (2021-03-25)
 ------------------
 

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>8.1.0</version>
+  <version>8.2.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>

rclcpp_lifecycle/CHANGELOG.rst

@@ -3,6 +3,14 @@ Changelog for package rclcpp_lifecycle
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+8.2.0 (2021-03-31)
+------------------
+* Fix flaky lifecycle node tests (`#1606 <https://github.com/ros2/rclcpp/issues/1606>`_)
+* Clock subscription callback group spins in its own thread (`#1556 <https://github.com/ros2/rclcpp/issues/1556>`_)
+* Delete debug messages (`#1602 <https://github.com/ros2/rclcpp/issues/1602>`_)
+* add automatically_add_executor_with_node option (`#1594 <https://github.com/ros2/rclcpp/issues/1594>`_)
+* Contributors: BriceRenaudeau, Ivan Santiago Paunovic, Jacob Perron, anaelle-sw
+
 8.1.0 (2021-03-25)
 ------------------
 

rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp

@@ -179,11 +179,16 @@ public:
   /// Create and return a callback group.
   /**
    * \param[in] group_type callback group type to create by this method.
+   * \param[in] automatically_add_to_executor_with_node A boolean that
+   *   determines whether a callback group is automatically added to an executor
+   *   with the node with which it is associated.
    * \return a callback group
    */
   RCLCPP_LIFECYCLE_PUBLIC
   rclcpp::CallbackGroup::SharedPtr
-  create_callback_group(rclcpp::CallbackGroupType group_type);
+  create_callback_group(
+    rclcpp::CallbackGroupType group_type,
+    bool automatically_add_to_executor_with_node = true);
 
   /// Return the list of callback groups in the node.
   /**

rclcpp_lifecycle/package.xml

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

rclcpp_lifecycle/src/lifecycle_node.cpp

@@ -102,7 +102,8 @@ LifecycleNode::LifecycleNode(
       node_logging_,
       node_clock_,
       node_parameters_,
-      options.clock_qos()
+      options.clock_qos(),
+      options.use_clock_thread()
     )),
   node_waitables_(new rclcpp::node_interfaces::NodeWaitables(node_base_.get())),
   node_options_(options),
@@ -164,9 +165,10 @@ LifecycleNode::get_logger() const
 
 rclcpp::CallbackGroup::SharedPtr
 LifecycleNode::create_callback_group(
-  rclcpp::CallbackGroupType group_type)
+  rclcpp::CallbackGroupType group_type,
+  bool automatically_add_to_executor_with_node)
 {
-  return node_base_->create_callback_group(group_type);
+  return node_base_->create_callback_group(group_type, automatically_add_to_executor_with_node);
 }
 
 const rclcpp::ParameterValue &

rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp

@@ -63,11 +63,8 @@ public:
 
   ~LifecycleNodeInterfaceImpl()
   {
-    fprintf(stderr, "oh yeah destructuro\n");
     rcl_node_t * node_handle = node_base_interface_->get_rcl_node_handle();
-    fprintf(stderr, "oh yeah end destructuro\n");
     auto ret = rcl_lifecycle_state_machine_fini(&state_machine_, node_handle);
-    fprintf(stderr, "oh yeah end destructuro222\n");
     if (ret != RCL_RET_OK) {
       RCUTILS_LOG_FATAL_NAMED(
         "rclcpp_lifecycle",
@@ -103,7 +100,6 @@ public:
       rosidl_typesupport_cpp::get_service_type_support_handle<GetAvailableTransitionsSrv>(),
       &state_machine_options);
     if (ret != RCL_RET_OK) {
-      fprintf(stderr, "oh yeah %s\n", node_base_interface_->get_name());
       throw std::runtime_error(
               std::string("Couldn't initialize state machine for node ") +
               node_base_interface_->get_name());

rclcpp_lifecycle/test/test_lifecycle_node.cpp

@@ -34,6 +34,88 @@
 using lifecycle_msgs::msg::State;
 using lifecycle_msgs::msg::Transition;
 
+static const std::chrono::nanoseconds DEFAULT_EVENT_TIMEOUT = std::chrono::seconds(3);
+static const std::chrono::nanoseconds DEFAULT_EVENT_SLEEP_PERIOD = std::chrono::milliseconds(100);
+
+static
+bool wait_for_event(
+  std::shared_ptr<rclcpp_lifecycle::LifecycleNode> node,
+  std::function<bool()> predicate,
+  std::chrono::nanoseconds timeout,
+  std::chrono::nanoseconds sleep_period)
+{
+  auto start = std::chrono::steady_clock::now();
+  std::chrono::microseconds time_slept(0);
+
+  bool predicate_result;
+  while (!(predicate_result = predicate()) &&
+    time_slept < std::chrono::duration_cast<std::chrono::microseconds>(timeout))
+  {
+    rclcpp::Event::SharedPtr graph_event = node->get_graph_event();
+    node->wait_for_graph_change(graph_event, sleep_period);
+    time_slept = std::chrono::duration_cast<std::chrono::microseconds>(
+      std::chrono::steady_clock::now() - start);
+  }
+  return predicate_result;
+}
+
+static
+bool wait_for_topic(
+  std::shared_ptr<rclcpp_lifecycle::LifecycleNode> node,
+  const std::string & topic,
+  std::chrono::nanoseconds timeout = DEFAULT_EVENT_TIMEOUT,
+  std::chrono::nanoseconds sleep_period = DEFAULT_EVENT_SLEEP_PERIOD)
+{
+  return wait_for_event(
+    node,
+    [node, topic]()
+    {
+      auto topic_names_and_types = node->get_topic_names_and_types();
+      return topic_names_and_types.end() != topic_names_and_types.find(topic);
+    },
+    timeout,
+    sleep_period);
+}
+
+static
+bool wait_for_service(
+  std::shared_ptr<rclcpp_lifecycle::LifecycleNode> node,
+  const std::string & service,
+  std::chrono::nanoseconds timeout = DEFAULT_EVENT_TIMEOUT,
+  std::chrono::nanoseconds sleep_period = DEFAULT_EVENT_SLEEP_PERIOD)
+{
+  return wait_for_event(
+    node,
+    [node, service]()
+    {
+      auto service_names_and_types = node->get_service_names_and_types();
+      return service_names_and_types.end() != service_names_and_types.find(service);
+    },
+    timeout,
+    sleep_period);
+}
+
+static
+bool wait_for_service_by_node(
+  std::shared_ptr<rclcpp_lifecycle::LifecycleNode> node,
+  const std::string & node_name,
+  const std::string & service,
+  std::chrono::nanoseconds timeout = DEFAULT_EVENT_TIMEOUT,
+  std::chrono::nanoseconds sleep_period = DEFAULT_EVENT_SLEEP_PERIOD)
+{
+  return wait_for_event(
+    node,
+    [node, node_name, service]()
+    {
+      auto service_names_and_types_by_node = node->get_service_names_and_types_by_node(
+        node_name, "");
+      return service_names_and_types_by_node.end() != service_names_and_types_by_node.find(
+        service);
+    },
+    timeout,
+    sleep_period);
+}
+
 class TestDefaultStateMachine : public ::testing::Test
 {
 protected:
@@ -559,10 +641,8 @@ TEST_F(TestDefaultStateMachine, test_graph_topics) {
   ASSERT_NE(names.end(), std::find(names.begin(), names.end(), std::string("/testnode")));
 
   // Other topics may exist for an rclcpp::Node, but just checking the lifecycle one exists
+  ASSERT_TRUE(wait_for_topic(test_node, "/testnode/transition_event"));
   auto topic_names_and_types = test_node->get_topic_names_and_types();
-  ASSERT_NE(
-    topic_names_and_types.end(),
-    topic_names_and_types.find(std::string("/testnode/transition_event")));
   EXPECT_STREQ(
     topic_names_and_types["/testnode/transition_event"][0].c_str(),
     "lifecycle_msgs/msg/TransitionEvent");
@@ -580,39 +660,26 @@ TEST_F(TestDefaultStateMachine, test_graph_topics) {
 TEST_F(TestDefaultStateMachine, test_graph_services) {
   auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
 
-  auto service_names_and_types = test_node->get_service_names_and_types();
   // These are specific to lifecycle nodes, other services are provided by rclcpp::Node
-  ASSERT_NE(
-    service_names_and_types.end(),
-    service_names_and_types.find(std::string("/testnode/change_state")));
+  ASSERT_TRUE(wait_for_service(test_node, "/testnode/change_state"));
+  ASSERT_TRUE(wait_for_service(test_node, "/testnode/get_available_states"));
+  ASSERT_TRUE(wait_for_service(test_node, "/testnode/get_available_transitions"));
+  ASSERT_TRUE(wait_for_service(test_node, "/testnode/get_state"));
+  ASSERT_TRUE(wait_for_service(test_node, "/testnode/get_transition_graph"));
+
+  auto service_names_and_types = test_node->get_service_names_and_types();
   EXPECT_STREQ(
     service_names_and_types["/testnode/change_state"][0].c_str(),
     "lifecycle_msgs/srv/ChangeState");
-
-  ASSERT_NE(
-    service_names_and_types.end(),
-    service_names_and_types.find(std::string("/testnode/get_available_states")));
   EXPECT_STREQ(
     service_names_and_types["/testnode/get_available_states"][0].c_str(),
     "lifecycle_msgs/srv/GetAvailableStates");
-
-  ASSERT_NE(
-    service_names_and_types.end(),
-    service_names_and_types.find(std::string("/testnode/get_available_transitions")));
   EXPECT_STREQ(
     service_names_and_types["/testnode/get_available_transitions"][0].c_str(),
     "lifecycle_msgs/srv/GetAvailableTransitions");
-
-  ASSERT_NE(
-    service_names_and_types.end(),
-    service_names_and_types.find(std::string("/testnode/get_state")));
   EXPECT_STREQ(
     service_names_and_types["/testnode/get_state"][0].c_str(),
     "lifecycle_msgs/srv/GetState");
-
-  ASSERT_NE(
-    service_names_and_types.end(),
-    service_names_and_types.find(std::string("/testnode/get_transition_graph")));
   EXPECT_STREQ(
     service_names_and_types["/testnode/get_transition_graph"][0].c_str(),
     "lifecycle_msgs/srv/GetAvailableTransitions");
@@ -621,40 +688,28 @@ TEST_F(TestDefaultStateMachine, test_graph_services) {
 TEST_F(TestDefaultStateMachine, test_graph_services_by_node) {
   auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
 
+  // These are specific to lifecycle nodes, other services are provided by rclcpp::Node
+  ASSERT_TRUE(wait_for_service_by_node(test_node, "testnode", "/testnode/change_state"));
+  ASSERT_TRUE(wait_for_service_by_node(test_node, "testnode", "/testnode/get_available_states"));
+  ASSERT_TRUE(
+    wait_for_service_by_node(test_node, "testnode", "/testnode/get_available_transitions"));
+  ASSERT_TRUE(wait_for_service_by_node(test_node, "testnode", "/testnode/get_state"));
+  ASSERT_TRUE(wait_for_service_by_node(test_node, "testnode", "/testnode/get_transition_graph"));
+
   auto service_names_and_types_by_node =
     test_node->get_service_names_and_types_by_node("testnode", "");
-  // These are specific to lifecycle nodes, other services are provided by rclcpp::Node
-  ASSERT_NE(
-    service_names_and_types_by_node.end(),
-    service_names_and_types_by_node.find(std::string("/testnode/change_state")));
   EXPECT_STREQ(
     service_names_and_types_by_node["/testnode/change_state"][0].c_str(),
     "lifecycle_msgs/srv/ChangeState");
-
-  ASSERT_NE(
-    service_names_and_types_by_node.end(),
-    service_names_and_types_by_node.find(std::string("/testnode/get_available_states")));
   EXPECT_STREQ(
     service_names_and_types_by_node["/testnode/get_available_states"][0].c_str(),
     "lifecycle_msgs/srv/GetAvailableStates");
-
-  ASSERT_NE(
-    service_names_and_types_by_node.end(),
-    service_names_and_types_by_node.find(std::string("/testnode/get_available_transitions")));
   EXPECT_STREQ(
     service_names_and_types_by_node["/testnode/get_available_transitions"][0].c_str(),
     "lifecycle_msgs/srv/GetAvailableTransitions");
-
-  ASSERT_NE(
-    service_names_and_types_by_node.end(),
-    service_names_and_types_by_node.find(std::string("/testnode/get_state")));
   EXPECT_STREQ(
     service_names_and_types_by_node["/testnode/get_state"][0].c_str(),
     "lifecycle_msgs/srv/GetState");
-
-  ASSERT_NE(
-    service_names_and_types_by_node.end(),
-    service_names_and_types_by_node.find(std::string("/testnode/get_transition_graph")));
   EXPECT_STREQ(
     service_names_and_types_by_node["/testnode/get_transition_graph"][0].c_str(),
     "lifecycle_msgs/srv/GetAvailableTransitions");
@@ -666,7 +721,7 @@ TEST_F(TestDefaultStateMachine, test_callback_groups) {
   EXPECT_EQ(groups.size(), 1u);
 
   auto group = test_node->create_callback_group(
-    rclcpp::CallbackGroupType::MutuallyExclusive);
+    rclcpp::CallbackGroupType::MutuallyExclusive, true);
   EXPECT_NE(nullptr, group);
 
   groups = test_node->get_callback_groups();