Ensure rclcpp::TimerBase thread-safety.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

rclcpp/CHANGELOG.rst

@@ -2,6 +2,11 @@
 Changelog for package rclcpp
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+0.8.4 (2020-01-17)
+------------------
+* Intra-process subscriber should use RMW actual qos (ros2`#913 <https://github.com/ros2/rclcpp/issues/913>`_) (`#914 <https://github.com/ros2/rclcpp/issues/914>`_) (`#965 <https://github.com/ros2/rclcpp/issues/965>`_)
+* Contributors: Todd Malsbary
+
 0.8.3 (2019-11-19)
 ------------------
 

rclcpp/include/rclcpp/clock.hpp

@@ -16,6 +16,8 @@
 #define RCLCPP__CLOCK_HPP_
 
 #include <functional>
+#include <thread>
+#include <mutex>
 
 #include "rclcpp/macros.hpp"
 #include "rclcpp/time.hpp"
@@ -95,6 +97,12 @@ public:
   rcl_clock_type_t
   get_clock_type() const noexcept;
 
+  RCLCPP_PUBLIC
+  std::mutex &
+  get_clock_mutex() noexcept {
+    return clock_mutex_;
+  }
+
   // Add a callback to invoke if the jump threshold is exceeded.
   /**
    * These callback functions must remain valid as long as the
@@ -136,6 +144,7 @@ private:
   rcl_clock_t rcl_clock_;
   friend TimeSource;  /// Allow TimeSource to access the rcl_clock_ datatype.
   rcl_allocator_t allocator_;
+  std::mutex clock_mutex_;
 };
 
 }  // namespace rclcpp

rclcpp/include/rclcpp/subscription.hpp

@@ -124,15 +124,16 @@ public:
       using rclcpp::detail::resolve_intra_process_buffer_type;
 
       // Check if the QoS is compatible with intra-process.
-      if (qos.get_rmw_qos_profile().history == RMW_QOS_POLICY_HISTORY_KEEP_ALL) {
+      rmw_qos_profile_t qos_profile = get_actual_qos().get_rmw_qos_profile();
+      if (qos_profile.history == RMW_QOS_POLICY_HISTORY_KEEP_ALL) {
         throw std::invalid_argument(
                 "intraprocess communication is not allowed with keep all history qos policy");
       }
-      if (qos.get_rmw_qos_profile().depth == 0) {
+      if (qos_profile.depth == 0) {
         throw std::invalid_argument(
                 "intraprocess communication is not allowed with 0 depth qos policy");
       }
-      if (qos.get_rmw_qos_profile().durability != RMW_QOS_POLICY_DURABILITY_VOLATILE) {
+      if (qos_profile.durability != RMW_QOS_POLICY_DURABILITY_VOLATILE) {
         throw std::invalid_argument(
                 "intraprocess communication allowed only with volatile durability");
       }
@@ -149,7 +150,7 @@ public:
         options.get_allocator(),
         context,
         this->get_topic_name(),    // important to get like this, as it has the fully-qualified name
-        qos.get_rmw_qos_profile(),
+        qos_profile,
         resolve_intra_process_buffer_type(options.intra_process_buffer_type, callback)
         );
       TRACEPOINT(

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>0.8.3</version>
+  <version>0.8.4</version>
   <description>The ROS client library in C++.</description>
   <maintainer email="dthomas@osrfoundation.org">Dirk Thomas</maintainer>
   <license>Apache License 2.0</license>

rclcpp/resource/logging.hpp.em

@@ -128,9 +128,9 @@ def get_rclcpp_suffix_from_features(features):
       typename ::rclcpp::Logger>::value, \
       "First argument to logging macros must be an rclcpp::Logger"); \
 @[ if 'throttle' in feature_combination]@ \
-    auto get_time_point = [&clock](rcutils_time_point_value_t * time_point) -> rcutils_ret_t { \
+    auto get_time_point = [&c=clock](rcutils_time_point_value_t * time_point) -> rcutils_ret_t { \
       try { \
-        *time_point = clock.now().nanoseconds(); \
+        *time_point = c.now().nanoseconds(); \
       } catch (...) { \
         RCUTILS_SAFE_FWRITE_TO_STDERR( \
         "[rclcpp|logging.hpp] RCLCPP_@(severity)@(suffix) could not get current time stamp\n"); \

rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp

@@ -402,6 +402,8 @@ NodeParameters::declare_parameter(
 
   // Publish if events_publisher_ is not nullptr, which may be if disabled in the constructor.
   if (nullptr != events_publisher_) {
+    parameter_event.node = combined_name_;
+    parameter_event.stamp = node_clock_->get_clock()->now();
     events_publisher_->publish(parameter_event);
   }
 

rclcpp/src/rclcpp/timer.cpp

@@ -17,6 +17,7 @@
 #include <chrono>
 #include <string>
 #include <memory>
+#include <thread>
 
 #include "rclcpp/contexts/default_context.hpp"
 #include "rclcpp/exceptions.hpp"
@@ -40,11 +41,14 @@ TimerBase::TimerBase(
   timer_handle_ = std::shared_ptr<rcl_timer_t>(
     new rcl_timer_t, [ = ](rcl_timer_t * timer) mutable
     {
-      if (rcl_timer_fini(timer) != RCL_RET_OK) {
-        RCUTILS_LOG_ERROR_NAMED(
-          "rclcpp",
-          "Failed to clean up rcl timer handle: %s", rcl_get_error_string().str);
-        rcl_reset_error();
+      {
+        std::lock_guard<std::mutex> clock_guard(clock->get_clock_mutex());
+        if (rcl_timer_fini(timer) != RCL_RET_OK) {
+          RCUTILS_LOG_ERROR_NAMED(
+            "rclcpp",
+            "Failed to clean up rcl timer handle: %s", rcl_get_error_string().str);
+          rcl_reset_error();
+        }
       }
       delete timer;
       // Captured shared pointers by copy, reset to make sure timer is finalized before clock
@@ -54,16 +58,19 @@ TimerBase::TimerBase(
 
   *timer_handle_.get() = rcl_get_zero_initialized_timer();
 
-  rcl_clock_t * clock_handle = clock_->get_clock_handle();
-  if (
-    rcl_timer_init(
-      timer_handle_.get(), clock_handle, rcl_context.get(), period.count(), nullptr,
-      rcl_get_default_allocator()) != RCL_RET_OK)
   {
-    RCUTILS_LOG_ERROR_NAMED(
-      "rclcpp",
-      "Couldn't initialize rcl timer handle: %s\n", rcl_get_error_string().str);
-    rcl_reset_error();
+    std::lock_guard<std::mutex> clock_guard(clock_->get_clock_mutex());
+    rcl_clock_t * clock_handle = clock_->get_clock_handle();
+    if (
+      rcl_timer_init(
+        timer_handle_.get(), clock_handle, rcl_context.get(), period.count(), nullptr,
+        rcl_get_default_allocator()) != RCL_RET_OK)
+    {
+      RCUTILS_LOG_ERROR_NAMED(
+        "rclcpp",
+        "Couldn't initialize rcl timer handle: %s\n", rcl_get_error_string().str);
+      rcl_reset_error();
+    }
   }
 }
 

rclcpp/test/test_logging.cpp

@@ -77,16 +77,33 @@ public:
   }
 };
 
+class DummyNode
+{
+public:
+  DummyNode()
+  {
+    clock_ = rclcpp::Clock::make_shared(RCL_ROS_TIME);
+  }
+  rclcpp::Clock::SharedPtr get_clock()
+  {
+    return clock_;
+  }
+
+private:
+  rclcpp::Clock::SharedPtr clock_;
+};
+
 TEST_F(TestLoggingMacros, test_logging_named) {
   for (int i : {1, 2, 3}) {
     RCLCPP_DEBUG(g_logger, "message %d", i);
   }
+  size_t expected_location = __LINE__ - 2u;
   EXPECT_EQ(3u, g_log_calls);
   EXPECT_TRUE(g_last_log_event.location != NULL);
   if (g_last_log_event.location) {
     EXPECT_STREQ("TestBody", g_last_log_event.location->function_name);
     EXPECT_THAT(g_last_log_event.location->file_name, EndsWith("test_logging.cpp"));
-    EXPECT_EQ(82u, g_last_log_event.location->line_number);
+    EXPECT_EQ(expected_location, g_last_log_event.location->line_number);
   }
   EXPECT_EQ(RCUTILS_LOG_SEVERITY_DEBUG, g_last_log_event.level);
   EXPECT_EQ("name", g_last_log_event.name);
@@ -210,6 +227,26 @@ TEST_F(TestLoggingMacros, test_throttle) {
       EXPECT_EQ(6u, g_log_calls);
     }
   }
+  DummyNode node;
+  rcl_clock_t * node_clock = node.get_clock()->get_clock_handle();
+  ASSERT_TRUE(node_clock);
+  ASSERT_EQ(RCL_RET_OK, rcl_enable_ros_time_override(node_clock));
+  EXPECT_EQ(6u, g_log_calls);
+  EXPECT_EQ(RCL_RET_OK, rcl_set_ros_time_override(node_clock, RCUTILS_MS_TO_NS(10)));
+  for (uint64_t i = 0; i < 3; ++i) {
+    RCLCPP_DEBUG_THROTTLE(g_logger, *node.get_clock(), 10, "Throttling");
+    if (i == 0) {
+      EXPECT_EQ(7u, g_log_calls);
+      rcl_time_point_value_t clock_ns = node.get_clock()->now().nanoseconds() + RCUTILS_MS_TO_NS(5);
+      EXPECT_EQ(RCL_RET_OK, rcl_set_ros_time_override(node_clock, clock_ns));
+    } else if (i == 1) {
+      EXPECT_EQ(7u, g_log_calls);
+      rcl_time_point_value_t clock_ns = node.get_clock()->now().nanoseconds() + RCUTILS_MS_TO_NS(5);
+      EXPECT_EQ(RCL_RET_OK, rcl_set_ros_time_override(node_clock, clock_ns));
+    } else {
+      EXPECT_EQ(8u, g_log_calls);
+    }
+  }
 }
 
 bool log_function(rclcpp::Logger logger)

rclcpp/test/test_subscription.cpp

@@ -287,10 +287,6 @@ static std::vector<TestParameters> invalid_qos_profiles()
     TestParameters(
       rclcpp::QoS(rclcpp::KeepAll()),
       "keep_all_qos"));
-  parameters.push_back(
-    TestParameters(
-      rclcpp::QoS(rclcpp::KeepLast(0)),
-      "keep_last_zero_depth_qos"));
 
   return parameters;
 }

rclcpp_action/CHANGELOG.rst

@@ -3,6 +3,9 @@ Changelog for package rclcpp_action
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+0.8.4 (2020-01-17)
+------------------
+
 0.8.3 (2019-11-19)
 ------------------
 * issue-919 Fixed "memory leak" in action clients (`#920 <https://github.com/ros2/rclcpp/issues/920>`_)

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>0.8.3</version>
+  <version>0.8.4</version>
   <description>Adds action APIs for C++.</description>
   <maintainer email="dthomas@osrfoundation.org">Dirk Thomas</maintainer>
   <license>Apache License 2.0</license>

rclcpp_components/CHANGELOG.rst

@@ -2,6 +2,11 @@
 Changelog for package rclcpp_components
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+0.8.4 (2020-01-17)
+------------------
+* Remove absolute path from installed CMake code (`#948 <https://github.com/ros2/rclcpp/issues/948>`_) (`#950 <https://github.com/ros2/rclcpp/issues/950>`_)
+* Contributors: Jacob Perron
+
 0.8.3 (2019-11-19)
 ------------------
 

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>0.8.3</version>
+  <version>0.8.4</version>
   <description>Package containing tools for dynamically loadable components</description>
   <maintainer email="michael@openrobotics.org">Michael Carroll</maintainer>
   <license>Apache License 2.0</license>

rclcpp_components/rclcpp_components-extras.cmake.in

@@ -25,7 +25,8 @@ macro(_rclcpp_components_register_package_hook)
   endif()
 endmacro()
 
-set(@PROJECT_NAME@_NODE_TEMPLATE "@CMAKE_INSTALL_PREFIX@/@node_main_template_install_dir@/node_main.cpp.in")
+get_filename_component(@PROJECT_NAME@_SHARE_DIR "${@PROJECT_NAME@_DIR}" DIRECTORY)
+set(@PROJECT_NAME@_NODE_TEMPLATE "${@PROJECT_NAME@_SHARE_DIR}/node_main.cpp.in")
 
 include("${rclcpp_components_DIR}/rclcpp_components_register_nodes.cmake")
 include("${rclcpp_components_DIR}/rclcpp_components_register_node.cmake")

rclcpp_lifecycle/CHANGELOG.rst

@@ -3,6 +3,9 @@ Changelog for package rclcpp_lifecycle
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+0.8.4 (2020-01-17)
+------------------
+
 0.8.3 (2019-11-19)
 ------------------
 

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>0.8.3</version>
+  <version>0.8.4</version>
   <description>Package containing a prototype for lifecycle implementation</description>
   <maintainer email="karsten@osrfoundation.org">Karsten Knese</maintainer>
   <license>Apache License 2.0</license>