Add support for timers on reset callback (#1979)

Signed-off-by: Mauro Passerino <[email protected]>
Co-authored-by: Alberto Soragna <[email protected]>

Author: mauropasse

rclcpp/include/rclcpp/timer.hpp

@@ -149,11 +149,48 @@ class TimerBase
   bool
   exchange_in_use_by_wait_set_state(bool in_use_state);
 
+  /// Set a callback to be called when the timer is reset
+  /**
+   * You should aim to make this callback fast and not blocking.
+   * If you need to do a lot of work or wait for some other event, you should
+   * spin it off to another thread.
+   *
+   * Calling it again will override any previously set callback.
+   * An exception will be thrown if the callback is not callable.
+   *
+   * This function is thread-safe.
+   *
+   * If you want more information available in the callback,
+   * you may use a lambda with captures or std::bind.
+   *
+   * \param[in] callback functor to be called whenever timer is reset
+   */
+  RCLCPP_PUBLIC
+  void
+  set_on_reset_callback(std::function<void(size_t)> callback);
+
+  /// Unset the callback registered for reset timer
+  RCLCPP_PUBLIC
+  void
+  clear_on_reset_callback();
+
 protected:
+  std::recursive_mutex callback_mutex_;
+  // Declare callback before timer_handle_, so on destruction
+  // the callback is destroyed last. Otherwise, the rcl timer
+  // callback would point briefly to a destroyed function.
+  // Clearing the callback on timer destructor also makes sure
+  // the rcl callback is cleared before on_reset_callback_.
+  std::function<void(size_t)> on_reset_callback_{nullptr};
+
   Clock::SharedPtr clock_;
   std::shared_ptr<rcl_timer_t> timer_handle_;
 
   std::atomic<bool> in_use_by_wait_set_{false};
+
+  RCLCPP_PUBLIC
+  void
+  set_on_reset_callback(rcl_event_callback_t callback, const void * user_data);
 };
 
 

rclcpp/src/rclcpp/timer.cpp

@@ -19,9 +19,12 @@
 #include <memory>
 #include <thread>
 
+#include "rmw/impl/cpp/demangle.hpp"
+
 #include "rclcpp/contexts/default_context.hpp"
+#include "rclcpp/detail/cpp_callback_trampoline.hpp"
 #include "rclcpp/exceptions.hpp"
-
+#include "rclcpp/logging.hpp"
 #include "rcutils/logging_macros.h"
 
 using rclcpp::TimerBase;
@@ -71,7 +74,9 @@ TimerBase::TimerBase(
 }
 
 TimerBase::~TimerBase()
-{}
+{
+  clear_on_reset_callback();
+}
 
 void
 TimerBase::cancel()
@@ -96,7 +101,11 @@ TimerBase::is_canceled()
 void
 TimerBase::reset()
 {
-  rcl_ret_t ret = rcl_timer_reset(timer_handle_.get());
+  rcl_ret_t ret = RCL_RET_OK;
+  {
+    std::lock_guard<std::recursive_mutex> lock(callback_mutex_);
+    ret = rcl_timer_reset(timer_handle_.get());
+  }
   if (ret != RCL_RET_OK) {
     rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't reset timer");
   }
@@ -138,3 +147,73 @@ TimerBase::exchange_in_use_by_wait_set_state(bool in_use_state)
 {
   return in_use_by_wait_set_.exchange(in_use_state);
 }
+
+void
+TimerBase::set_on_reset_callback(std::function<void(size_t)> callback)
+{
+  if (!callback) {
+    throw std::invalid_argument(
+            "The callback passed to set_on_reset_callback "
+            "is not callable.");
+  }
+
+  auto new_callback =
+    [callback, this](size_t reset_calls) {
+      try {
+        callback(reset_calls);
+      } catch (const std::exception & exception) {
+        RCLCPP_ERROR_STREAM(
+          rclcpp::get_logger("rclcpp"),
+          "rclcpp::TimerBase@" << this <<
+            " caught " << rmw::impl::cpp::demangle(exception) <<
+            " exception in user-provided callback for the 'on reset' callback: " <<
+            exception.what());
+      } catch (...) {
+        RCLCPP_ERROR_STREAM(
+          rclcpp::get_logger("rclcpp"),
+          "rclcpp::TimerBase@" << this <<
+            " caught unhandled exception in user-provided callback " <<
+            "for the 'on reset' callback");
+      }
+    };
+
+  std::lock_guard<std::recursive_mutex> lock(callback_mutex_);
+
+  // Set it temporarily to the new callback, while we replace the old one.
+  // This two-step setting, prevents a gap where the old std::function has
+  // been replaced but rcl hasn't been told about the new one yet.
+  set_on_reset_callback(
+    rclcpp::detail::cpp_callback_trampoline<
+      decltype(new_callback), const void *, size_t>,
+    static_cast<const void *>(&new_callback));
+
+  // Store the std::function to keep it in scope, also overwrites the existing one.
+  on_reset_callback_ = new_callback;
+
+  // Set it again, now using the permanent storage.
+  set_on_reset_callback(
+    rclcpp::detail::cpp_callback_trampoline<
+      decltype(on_reset_callback_), const void *, size_t>,
+    static_cast<const void *>(&on_reset_callback_));
+}
+
+void
+TimerBase::clear_on_reset_callback()
+{
+  std::lock_guard<std::recursive_mutex> lock(callback_mutex_);
+
+  if (on_reset_callback_) {
+    set_on_reset_callback(nullptr, nullptr);
+    on_reset_callback_ = nullptr;
+  }
+}
+
+void
+TimerBase::set_on_reset_callback(rcl_event_callback_t callback, const void * user_data)
+{
+  rcl_ret_t ret = rcl_timer_set_on_reset_callback(timer_handle_.get(), callback, user_data);
+
+  if (ret != RCL_RET_OK) {
+    rclcpp::exceptions::throw_from_rcl_error(ret, "Failed to set timer on reset callback");
+  }
+}