Add in the ability to start timers paused. (#1138)

* Add in the ability to start timers paused.

This was possible in the past by creating a timer
and then immediately canceling it.  But that could
lead to a potential race where the timer fired once
before it was canceled.  By allowing for the user
to create it paused, we avoid that race.

This is the rclpy version of ros2/rclcpp#2005

Signed-off-by: Chris Lalancette <[email protected]>

rclpy/rclpy/node.py

@@ -1676,25 +1676,32 @@ def create_timer(
         callback: Callable,
         callback_group: Optional[CallbackGroup] = None,
         clock: Optional[Clock] = None,
+        autostart: bool = True,
     ) -> Timer:
         """
         Create a new timer.
 
-        The timer will be started and every ``timer_period_sec`` number of seconds the provided
-        callback function will be called.
+        If autostart is ``True`` (the default), the timer will be started and every
+        ``timer_period_sec`` number of seconds the provided callback function will be called.
+        If autostart is ``False``, the timer will be created but not started; it can then be
+        started by calling ``reset()`` on the timer object.
 
-        :param timer_period_sec: The period (s) of the timer.
+        :param timer_period_sec: The period (in seconds) of the timer.
         :param callback: A user-defined callback function that is called when the timer expires.
         :param callback_group: The callback group for the timer. If ``None``, then the
             default callback group for the node is used.
         :param clock: The clock which the timer gets time from.
+        :param autostart: Whether to automatically start the timer after creation; defaults to
+            ``True``.
         """
         timer_period_nsec = int(float(timer_period_sec) * S_TO_NS)
         if callback_group is None:
             callback_group = self.default_callback_group
         if clock is None:
             clock = self._clock
-        timer = Timer(callback, callback_group, timer_period_nsec, clock, context=self.context)
+        timer = Timer(
+            callback, callback_group, timer_period_nsec, clock, context=self.context,
+            autostart=autostart)
 
         callback_group.add_entity(timer)
         self._timers.append(timer)

rclpy/rclpy/timer.py

@@ -14,19 +14,51 @@
 
 import threading
 
+from typing import Callable
+from typing import Optional
+
+from rclpy.callback_groups import CallbackGroup
+from rclpy.clock import Clock
+from rclpy.context import Context
 from rclpy.exceptions import InvalidHandle, ROSInterruptException
 from rclpy.impl.implementation_singleton import rclpy_implementation as _rclpy
 from rclpy.utilities import get_default_context
 
 
 class Timer:
 
-    def __init__(self, callback, callback_group, timer_period_ns, clock, *, context=None):
+    def __init__(
+        self,
+        callback: Callable,
+        callback_group: CallbackGroup,
+        timer_period_ns: int,
+        clock: Clock,
+        *,
+        context: Optional[Context] = None,
+        autostart: bool = True
+    ):
+        """
+        Create a Timer.
+
+        If autostart is ``True`` (the default), the timer will be started and every
+        ``timer_period_sec`` number of seconds the provided callback function will be called.
+        If autostart is ``False``, the timer will be created but not started; it can then be
+        started by calling ``reset()`` on the timer object.
+
+        :param callback: A user-defined callback function that is called when the timer expires.
+        :param callback_group: The callback group for the timer. If ``None``, then the
+            default callback group for the node is used.
+        :param timer_period_ns: The period (in nanoseconds) of the timer.
+        :param clock: The clock which the timer gets time from.
+        :param context: The context to be associated with.
+        :param autostart: Whether to automatically start the timer after creation; defaults to
+            ``True``.
+        """
         self._context = get_default_context() if context is None else context
         self._clock = clock
         with self._clock.handle, self._context.handle:
             self.__timer = _rclpy.Timer(
-                self._clock.handle, self._context.handle, timer_period_ns)
+                self._clock.handle, self._context.handle, timer_period_ns, autostart)
         self.timer_period_ns = timer_period_ns
         self.callback = callback
         self.callback_group = callback_group

rclpy/src/rclpy/timer.cpp

@@ -36,7 +36,7 @@ Timer::destroy()
 }
 
 Timer::Timer(
-  Clock & clock, Context & context, int64_t period_nsec)
+  Clock & clock, Context & context, int64_t period_nsec, bool autostart)
 : context_(context), clock_(clock)
 {
   // Create a client
@@ -61,7 +61,7 @@ Timer::Timer(
 
   rcl_ret_t ret = rcl_timer_init2(
     rcl_timer_.get(), clock_.rcl_ptr(), context.rcl_ptr(),
-    period_nsec, NULL, allocator, true);
+    period_nsec, NULL, allocator, autostart);
 
   if (RCL_RET_OK != ret) {
     throw RCLError("failed to create timer");
@@ -159,7 +159,7 @@ void
 define_timer(py::object module)
 {
   py::class_<Timer, Destroyable, std::shared_ptr<Timer>>(module, "Timer")
-  .def(py::init<Clock &, Context &, int64_t>())
+  .def(py::init<Clock &, Context &, int64_t, bool>())
   .def_property_readonly(
     "pointer", [](const Timer & timer) {
       return reinterpret_cast<size_t>(timer.rcl_ptr());

rclpy/src/rclpy/timer.hpp

@@ -46,9 +46,10 @@ class Timer : public Destroyable, public std::enable_shared_from_this<Timer>
    * \param[in] clock pycapsule containing an rcl_clock_t
    * \param[in] context Capsule for an rcl_timer_t
    * \param[in] period_nsec The period of the timer in nanoseconds
+   * \param[in] autostart Whether to automatically start the timer
    * \return a timer capsule
    */
-  Timer(Clock & clock, Context & context, int64_t period_nsec);
+  Timer(Clock & clock, Context & context, int64_t period_nsec, bool autostart);
 
   ~Timer() = default;
 

rclpy/test/test_timer.py

@@ -180,3 +180,25 @@ def test_time_until_next_call():
         if node is not None:
             node.destroy_node()
         rclpy.shutdown(context=context)
+
+
+def test_timer_without_autostart():
+    node = None
+    timer = None
+    rclpy.init()
+    try:
+        node = rclpy.create_node('test_timer_without_autostart')
+        timer = node.create_timer(1, lambda: None, autostart=False)
+        assert timer.is_canceled()
+
+        timer.reset()
+        assert not timer.is_canceled()
+
+        timer.cancel()
+        assert timer.is_canceled()
+    finally:
+        if timer is not None:
+            node.destroy_timer(timer)
+        if node is not None:
+            node.destroy_node()
+        rclpy.shutdown()

rclpy/test/test_waitable.py

@@ -131,7 +131,7 @@ def __init__(self, node):
         period_nanoseconds = 10000
         with self._clock.handle, node.context.handle:
             self.timer = _rclpy.Timer(
-                self._clock.handle, node.context.handle, period_nanoseconds)
+                self._clock.handle, node.context.handle, period_nanoseconds, True)
         self.timer_index = None
         self.timer_is_ready = False