Add a `create_timer` method to `Node` and `LifecycleNode` classes #1975

asymingt · 2022-07-22T19:19:07Z

Problem

Right now the Node and LifecycleNode classes only provide a method for creating a wall timer, even though the underlying functions for creating a generic timer (which respects ROS time via /clock) are available. This is a problem because those that write node implementations -- and especially those that come from a ROS1 background -- default to using {Node, LifecycleNode}::create_wall_timer not understanding the implications this decision has on timer behavior in replay, and especially when ros2 bag play --rate is used. The existing method of creating the correct timer type is opaque: it requires knowledge of free-functions in rclcpp.

Relates to issues:

Relates to pull requests:

Add clock type to node_options #1982

Fix

This PR adds the piping to allow a timer that respects /clock when use_sim_time: True node parameter is set. While it is possible to do this manually with rclcpp::create_timer it's much less convenient. An analogous function already exists in rclpy.

Progress

Add the create_timer method to Node class.
Take care of edge cases in chrono::duration casts.
Add unit tests to create_timer, timer_source and timer.
Debug TestTimeSource.check_sim_time_updated_in_callback_if_use_clock_thread.
UpdateLifecycleNode to also support create_timer.
Refactor create_timer to drop raw pointers and have common error checks.

jacobperron · 2022-07-22T19:52:45Z

Make sure to also sign-off your commits to satisfy the DCO check.

jacobperron

Looks good to me. Please add a test exercising the new method, then I'll trigger CI.

asymingt · 2022-07-22T21:50:36Z

Looks good to me. Please add a test exercising the new method, then I'll trigger CI.

Done. Please take a look at the unit tests, and particularly the EXPECT_FALSE(timer->is_steady() for the GENERIC_TIMER implementation. To me it seems logical that being driven by /clock might end up making the time source non-monotonic. Does this violate any assumption made by the timer class?

Unit tests pass locally:

83: [==========] Running 16 tests from 1 test suite.
83: [----------] Global test environment set-up.
83: [----------] 16 tests from PerTimerType/TestTimer
83: [ RUN      ] PerTimerType/TestTimer.test_simple_cancel/wall_timer
83: [       OK ] PerTimerType/TestTimer.test_simple_cancel/wall_timer (17 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_simple_cancel/generic_timer
83: [       OK ] PerTimerType/TestTimer.test_simple_cancel/generic_timer (5 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_is_canceled_reset/wall_timer
83: [       OK ] PerTimerType/TestTimer.test_is_canceled_reset/wall_timer (5 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_is_canceled_reset/generic_timer
83: [       OK ] PerTimerType/TestTimer.test_is_canceled_reset/generic_timer (5 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_run_cancel_executor/wall_timer
83: [       OK ] PerTimerType/TestTimer.test_run_cancel_executor/wall_timer (105 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_run_cancel_executor/generic_timer
83: [       OK ] PerTimerType/TestTimer.test_run_cancel_executor/generic_timer (105 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_run_cancel_timer/wall_timer
83: [       OK ] PerTimerType/TestTimer.test_run_cancel_timer/wall_timer (105 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_run_cancel_timer/generic_timer
83: [       OK ] PerTimerType/TestTimer.test_run_cancel_timer/generic_timer (105 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_bad_arguments/wall_timer
83: [ERROR] [1658525415.784172078] []: Failed to fini rcl clock.
83: [       OK ] PerTimerType/TestTimer.test_bad_arguments/wall_timer (5 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_bad_arguments/generic_timer
83: [ERROR] [1658525415.788818007] []: Failed to fini rcl clock.
83: [       OK ] PerTimerType/TestTimer.test_bad_arguments/generic_timer (5 ms)
83: [ RUN      ] PerTimerType/TestTimer.callback_with_timer/wall_timer
83: [       OK ] PerTimerType/TestTimer.callback_with_timer/wall_timer (7 ms)
83: [ RUN      ] PerTimerType/TestTimer.callback_with_timer/generic_timer
83: [       OK ] PerTimerType/TestTimer.callback_with_timer/generic_timer (6 ms)
83: [ RUN      ] PerTimerType/TestTimer.callback_with_period_zero/wall_timer
83: [       OK ] PerTimerType/TestTimer.callback_with_period_zero/wall_timer (5 ms)
83: [ RUN      ] PerTimerType/TestTimer.callback_with_period_zero/generic_timer
83: [       OK ] PerTimerType/TestTimer.callback_with_period_zero/generic_timer (4 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_failures_with_exceptions/wall_timer
83: [ERROR] [1658525415.815430083] [rclcpp]: Failed to clean up rcl timer handle: error not set
83: [       OK ] PerTimerType/TestTimer.test_failures_with_exceptions/wall_timer (4 ms)
83: [ RUN      ] PerTimerType/TestTimer.test_failures_with_exceptions/generic_timer
83: [ERROR] [1658525415.820125821] [rclcpp]: Failed to clean up rcl timer handle: error not set
83: [       OK ] PerTimerType/TestTimer.test_failures_with_exceptions/generic_timer (5 ms)
83: [----------] 16 tests from PerTimerType/TestTimer (493 ms total)
83: 
83: [----------] Global test environment tear-down
83: [==========] 16 tests from 1 test suite ran. (493 ms total)
83: [  PASSED  ] 16 tests.
83: -- run_test.py: return code 0    
83: -- run_test.py: inject classname prefix into gtest result file 
The following tests passed:
        test_timer

...

100% tests passed, 0 tests failed out of 1

Label Time Summary:
gtest    =   0.68 sec*proc (1 test)

Total Test time (real) =   0.68 sec
Finished <<< rclcpp [0.94s]

Summary: 1 package finished [1.76s]

jacobperron · 2022-07-22T22:08:56Z

To me it seems logical that being driven by /clock might end up making the time source non-monotonic. Does this violate any assumption made by the timer class?

Correct, if we are using a ROS clock (ie. potentially relying on the /clock topic) then we can't assume time will be be monotonically increasing. I don't think this violates any assumptions of the timer class, but I can double-check.

fujitatomoya

lgtm with one question.

rclcpp/include/rclcpp/create_timer.hpp

Blast545 · 2022-08-03T14:55:57Z

@ros-pull-request-builder retest this please

asymingt · 2022-08-03T16:31:32Z

rclcpp/test/rclcpp/test_time_source.cpp

@@ -695,7 +694,7 @@ class ClockThreadTestingNode : public rclcpp::Node
    this->set_parameter(rclcpp::Parameter("use_sim_time", true));

    // Create a 100ms timer
-    timer_ = this->create_wall_timer(
+    timer_ = this->create_timer(


We should consider reverting this, even though the reversion should be a no-op. I get that this timer sets up the periodic publishing of /clock to drive peer nodes' perception of time (which we call "ROS time"). For this reason a wall timer makes more sense. However, my understanding is that since the parameter use_sim_time is not set on this node, create_timer should simply fall back to a wall timer. Thus, both create_timer and create_wall_timer are equivalent here.

If I understand the tests correctly, we want this to be a wall timer so I would just revert it to be safe.

yes, using create_wall_timer() here makes more sense I think

Can you double check this is what you want!

We should consider reverting this, even though the reversion should be a no-op. I get that this timer sets up the periodic publishing of /clock to drive peer nodes' perception of time (which we call "ROS time"). For this reason a wall timer makes more sense. However, my understanding is that since the parameter use_sim_time is not set on this node, create_timer should simply fall back to a wall timer. Thus, both create_timer and create_wall_timer are equivalent here.

The above message I sent was wrong, as I incorrectly thought this change was to the SimClockPublisherNode code, which it is not. If you look at how this code is used in the unit test , we have a SimClockPublisherNode that publishes time on /clock and a ClockThreadTestingNode that consumes this time.

I'm fairly sure that we want a create_timer here to prove that a /clock driven sense of time has a callback within which you can make subsequent calls to this->now(). If we really wanted to test a wall timer, there would be no need to spin up a SimClockPublisherNode instance.

Yes, that seems to be the case.
I'm not sure what was the test doing before as create_wall_timer() never used sim time.

The comments also say that this test should be running with use_clock_thread=true, and that's not the case.

When I modify the test to do what's supposed to do, it's super flaky.
IMO, it's fair to comment out the test and create a new issue for it.
@jacobperron any opinion?

I commented out the test as I think the problem is the test itself, see 0093121

I thought the test was checking that the wall timer and simulation time updates were not interfering with each other. But, I'm not sure. I'm okay with disabling the flaky test. Maybe we can create a GitHub ticket to look into it (maybe it is just not a well-defined test and could be removed).

Based on the comments, the test name, and the code; it seems that the test is supposed to be checking that the clock is updated when use_clock_thread=True even if you're blocked in a callback.
The part that's missing is that use_clock_thread was never actually enabled.
Enabling that doesn't solve the issue, as it seems something is not working well.

@anaelle-sw I think you added this feature, could you take a look at the issue?

asymingt · 2022-08-04T19:13:50Z

@clalancette and @jacobperron -- I suspect that the nondeterminism in this PR has to do with discovery of the /clock topic periodically failing for some reason. If you consider the /clock topic is produced by some ClockSourceNode for consumption by ClockSinkNode (which has use_sim_time: True set as a local parameter). Internally if ClockSinkNode uses this->create_timer the resulting timer will only be advanced when /clock is received). However, it seems like the DDS discovery mechanism sometimes fails to find /clock and the timer spins indefinitely at ros::Time(0). I'm not sure why this only happens on some machines and not others, but I have seen it happen on my local machine.

asymingt · 2022-08-04T19:48:43Z

@clalancette and @jacobperron -- I suspect that the nondeterminism in this PR has to do with discovery of the /clock topic periodically failing for some reason. If you consider the /clock topic is produced by some ClockSourceNode for consumption by ClockSinkNode (which has use_sim_time: True set as a local parameter). Internally if ClockSinkNode uses this->create_timer the resulting timer will only be advanced when /clock is received). However, it seems like the DDS discovery mechanism sometimes fails to find /clock and the timer spins indefinitely at ros::Time(0). I'm not sure why this only happens on some machines and not others, but I have seen it happen on my local machine.

Update: the problem is completely fixed locally when I set rclcpp::NodeOptions().use_clock_thread(false) when instantiating my ClockSinkNode. Perhaps there is a race condition with the clock thread implementation.

ivanpauno

I haven't looked at the tests yet, but code looks good.
I only have a minor comment.

rclcpp/include/rclcpp/create_timer.hpp

rclcpp/test/rclcpp/test_timer.cpp

ivanpauno · 2022-08-11T17:48:44Z

Update: the problem is completely fixed locally when I set rclcpp::NodeOptions().use_clock_thread(false) when instantiating my ClockSinkNode. Perhaps there is a race condition with the clock thread implementation.

Could you link what the test that was causing issues is?
I went through the previous CI jobs and I see different tests failing, but I'm not sure what's the one you're referring here.

asymingt · 2022-08-12T22:32:16Z

Update: the problem is completely fixed locally when I set rclcpp::NodeOptions().use_clock_thread(false) when instantiating my ClockSinkNode. Perhaps there is a race condition with the clock thread implementation.

Could you link what the test that was causing issues is? I went through the previous CI jobs and I see different tests failing, but I'm not sure what's the one you're referring here.

The last five commits are all empty, and in theory they should all pass. However, passing happens on only some CI workers (look at the CI build history). Locally, things pass every time. I can occasionally replicate the issue with test TestTimeSource.check_sim_time_updated_in_callback_if_use_clock_thread, but there's no single test I can show you repeatedly fails locally.

ivanpauno · 2022-08-17T16:08:20Z

The last five commits are all empty, and in theory they should all pass. However, passing happens on only some CI workers (look at the CI build history). Locally, things pass every time. I can occasionally replicate the issue with test TestTimeSource.check_sim_time_updated_in_callback_if_use_clock_thread, but there's no single test I can show you repeatedly fails locally.

Okay, that's strange.
I will take a look and see if I can reproduce locally.

ivanpauno · 2022-08-17T16:21:42Z

I'm taking a look to all the failures in https://build.ros2.org/job/Rpr__rclcpp__ubuntu_jammy_amd64/227/#showFailuresLink and https://build.ros2.org/job/Rpr__rclcpp__ubuntu_jammy_amd64/228/#showFailuresLink, and they don't seem to be related to your changes at all.

It seems that for some reason they're happening in the Rpr job but not in the nightlies.

About test_time_source, the tests are written in a way that can easily fail.
Increasing 1s here to a longer duration may fix the issue.

asymingt · 2022-08-17T23:40:36Z

I'm taking a look to all the failures in https://build.ros2.org/job/Rpr__rclcpp__ubuntu_jammy_amd64/227/#showFailuresLink and https://build.ros2.org/job/Rpr__rclcpp__ubuntu_jammy_amd64/228/#showFailuresLink, and they don't seem to be related to your changes at all.

It seems that for some reason they're happening in the Rpr job but not in the nightlies.

About test_time_source, the tests are written in a way that can easily fail. Increasing 1s here to a longer duration may fix the issue.

I initially tried 10s and that was too large, so I dropped it down to 2s and this seems more stable.

asymingt · 2022-08-18T06:22:55Z

@jacobperron and @ivanpauno - Please take a look at the last two commits and let me know if you are happy. I think I addressed all review comments, even though this one was quite tricky. I am of the opinion that this PR is now ready to merge.

asymingt · 2022-08-19T06:30:11Z

@ros-pull-request-builder retest this please

asymingt · 2022-08-19T17:47:02Z

@ros-pull-request-builder retest this please

ivanpauno · 2022-08-19T20:25:18Z

Something I noticed while reading the tests, this line should say >= and not ==:

rclcpp/rclcpp/test/rclcpp/test_time_source.cpp

Line 84 in 6e0ec4a

if (clock->now().nanoseconds() == end_time.count()) {

asymingt · 2022-08-19T23:33:47Z

Something I noticed while reading the tests, this line should say >= and not ==:

rclcpp/rclcpp/test/rclcpp/test_time_source.cpp

Line 84 in 6e0ec4a

if (clock->now().nanoseconds() == end_time.count()) {

Fixed in c7acee7

Signed-off-by: Andrew Symington <andrew.c.symington@gmail.com>

… sequence Signed-off-by: Andrew Symington <andrew.c.symington@gmail.com>

Signed-off-by: Andrew Symington <andrew.c.symington@gmail.com>

ivanpauno

LGTM!
I will wait for @jacobperron approval as I made some minor changes.

Thanks @asymingt !!

…ble call sequence" This reverts commit 3de6312. Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

asymingt · 2022-08-26T22:17:15Z

LGTM! I will wait for @jacobperron approval as I made some minor changes.

Thanks @asymingt !!

Thanks for getting this one over the line, Ivan 👍

jacobperron

LGTM, pending CI. One tiny nitpick.

rclcpp/include/rclcpp/node_interfaces/node_topics.hpp

jacobperron · 2022-08-29T18:09:06Z

rclcpp/test/rclcpp/test_time_source.cpp

@@ -695,7 +694,7 @@ class ClockThreadTestingNode : public rclcpp::Node
    this->set_parameter(rclcpp::Parameter("use_sim_time", true));

    // Create a 100ms timer
-    timer_ = this->create_wall_timer(
+    timer_ = this->create_timer(


I thought the test was checking that the wall timer and simulation time updates were not interfering with each other. But, I'm not sure. I'm okay with disabling the flaky test. Maybe we can create a GitHub ticket to look into it (maybe it is just not a well-defined test and could be removed).

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno · 2022-08-29T19:15:03Z

CI:

Linux
Linux-aarch64
Windows

ivanpauno · 2022-08-29T20:01:32Z

I forgot to include rclcpp_lifecycle and test_rclcpp, running CI again:

Linux
Linux-aarch64
Windows

sloretz · 2022-08-30T17:02:58Z

I'm seeing a build failure in building tf2_ros that might be related to this change: https://ci.ros2.org/job/ci_linux-aarch64/11736/console#console-section-12

--- stderr: tf2_ros
09:46:40 In file included from �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_subscription.hpp�[K,
09:46:40                  from �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/node_impl.hpp�[K,
09:46:40                  from �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/node.hpp:1�[K,
09:46:40                  from �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/executors/single_threaded_executor.hpp�[K,
09:46:40                  from �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/executors.hpp�[K,
09:46:40                  from �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/rclcpp.hpp:�[K,
09:46:40                  from �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/src/ros2/geometry2/tf2_ros/include/tf2_ros/create_timer_interface.h�[K,
09:46:40                  from �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/src/ros2/geometry2/tf2_ros/include/tf2_ros/create_timer_ros.h�[K,
09:46:40                  from �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/src/ros2/geometry2/tf2_ros/src/create_timer_ros.cpp�[K:
09:46:40 /home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp: In instantiation of ‘�[Krclcpp::TimerBase::SharedPtr rclcpp::create_timer(std::shared_ptr<rclcpp::node_interfaces::NodeBaseInterface>, std::shared_ptr<rclcpp::node_interfaces::NodeTimersInterface>, rclcpp::Clock::SharedPtr, rclcpp::Duration, CallbackT&&, rclcpp::CallbackGroup::SharedPtr) [with CallbackT = std::function<void()>; rclcpp::TimerBase::SharedPtr = std::shared_ptr<rclcpp::TimerBase>; rclcpp::Clock::SharedPtr = std::shared_ptr<rclcpp::Clock>; rclcpp::CallbackGroup::SharedPtr = std::shared_ptr<rclcpp::CallbackGrou�[K’:
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/src/ros2/geometry2/tf2_ros/src/create_timer_ros.cpp:61:�[K   required from here
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp:95:��[Kerro�[Kno matching function for call t�[Kcreate_timer(std::__shared_ptr<rclcpp::node_interfaces::NodeBaseInterface, __gnu_cxx::_S_atomic>::element_type*, std::__shared_ptr<rclcpp::node_interfaces::NodeTimersInterface, __gnu_cxx::_S_atomic>::element_type*, rclcpp::Clock::SharedPtr&, std::chrono::duration<long int, std::ratio<1, 1000000000> >, std::function<void()>, rclcpp::CallbackGroup::SharedPt�[K’
09:46:40    95 |   return �[Kcreate_tim�[K
09:46:40       |          �[K~~~~~~~~~~�[K
09:46:40    96 | �[K    node_base.get�[K
09:46:40       |     �[K~~~~~~~~~~~~~�[K  
09:46:40    97 | �[K    node_timers.get�[K
09:46:40       |     �[K~~~~~~~~~~~~~~~�[K
09:46:40    98 | �[K    clo�[K
09:46:40       |     �[K~~~�[K            
09:46:40    99 | �[K    period.to_chrono<std::chrono::nanoseconds>�[K
09:46:40       |     �[K~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~�[K
09:46:40   100 | �[K    std::forward<CallbackT>(callbac�[K
09:46:40       |     �[K~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~�[K
09:46:40   101 | �[K    gro�[K;
09:46:40       |     �[K~~~�[K            
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp:87��[Knot�[Kcandidate�[Ktemplate<class CallbackT> rclcpp::TimerBase::SharedPtr rclcpp::create_timer(std::shared_ptr<rclcpp::node_interfaces::NodeBaseInterface>, std::shared_ptr<rclcpp::node_interfaces::NodeTimersInterface>, rclcpp::Clock::SharedPtr, rclcpp::Duration, CallbackT&&, rclcpp::CallbackGroup::SharedP�[K’
09:46:40    87 | �[Kcreate_ti�[K(
09:46:40       | �[K^~~~~~~~~�[K
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp:87��[Knot�[K  template argument deduction/substitution failed:
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp:96:��[Knot�[K  cannot conver�[Knode_base.std::shared_ptr<rclcpp::node_interfaces::NodeBaseInterface>::<anonymous>.std::__shared_ptr<rclcpp::node_interfaces::NodeBaseInterface, __gnu_cxx::_S_atomic>::ge�[K’ (typ�[Kstd::__shared_ptr<rclcpp::node_interfaces::NodeBaseInterface, __gnu_cxx::_S_atomic>::element_ty�[K’ {ak�[Krclcpp::node_interfaces::NodeBaseInterfa�[K’}) to typ�[Kstd::shared_ptr<rclcpp::node_interfaces::NodeBaseInterfa�[K’
09:46:40    96 |     �[Knode_base.ge�[K,
09:46:40       |     �[K~~~~~~~~~~~~�[K
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp:141��[Knot�[Kcandidate�[Ktemplate<class DurationRepT, class DurationT, class CallbackT> typename rclcpp::GenericTimer<CallbackT>::SharedPtr rclcpp::create_timer(rclcpp::Clock::SharedPtr, std::chrono::duration<_Rep1, _Period1>, CallbackT, rclcpp::CallbackGroup::SharedPtr, rclcpp::node_interfaces::NodeBaseInterface*, rclcpp::node_interfaces::NodeTimersInterfac�[K’
09:46:40   141 | �[Kcreate_ti�[K(
09:46:40       | �[K^~~~~~~~~�[K
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp:141��[Knot�[K  template argument deduction/substitution failed:
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp:95:��[Knot�[K  mismatched type�[Kstd::chrono::duration<_Rep1, _Perio�[K’ an�[Kstd::__shared_ptr<rclcpp::node_interfaces::NodeTimersInterface, __gnu_cxx::_S_atomic>::element_ty�[K’ {ak�[Krclcpp::node_interfaces::NodeTimersInterfa�[K’}
09:46:40    95 |   return �[Kcreate_tim�[K
09:46:40       |          �[K~~~~~~~~~~�[K
09:46:40    96 | �[K    node_base.get�[K
09:46:40       |     �[K~~~~~~~~~~~~~�[K  
09:46:40    97 | �[K    node_timers.get�[K
09:46:40       |     �[K~~~~~~~~~~~~~~~�[K
09:46:40    98 | �[K    clo�[K
09:46:40       |     �[K~~~�[K            
09:46:40    99 | �[K    period.to_chrono<std::chrono::nanoseconds>�[K
09:46:40       |     �[K~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~�[K
09:46:40   100 | �[K    std::forward<CallbackT>(callbac�[K
09:46:40       |     �[K~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~�[K
09:46:40   101 | �[K    gro�[K;
09:46:40       |     �[K~~~�[K            
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp:107��[Knot�[Kcandidate�[Ktemplate<class NodeT, class CallbackT> rclcpp::TimerBase::SharedPtr rclcpp::create_timer(NodeT, rclcpp::Clock::SharedPtr, rclcpp::Duration, CallbackT&&, rclcpp::CallbackGroup::SharedP�[K’
09:46:40   107 | �[Kcreate_ti�[K(
09:46:40       | �[K^~~~~~~~~�[K
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp:107��[Knot�[K  template argument deduction/substitution failed:
09:46:40 �[K/home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/rclcpp/include/rclcpp/rclcpp/create_timer.hpp:95:��[Knot�[K  candidate expects 5 arguments, 6 provided
09:46:40    95 |   return �[Kcreate_tim�[K
09:46:40       |          �[K~~~~~~~~~~�[K
09:46:40    96 | �[K    node_base.get�[K
09:46:40       |     �[K~~~~~~~~~~~~~�[K  
09:46:40    97 | �[K    node_timers.get�[K
09:46:40       |     �[K~~~~~~~~~~~~~~~�[K
09:46:40    98 | �[K    clo�[K
09:46:40       |     �[K~~~�[K            
09:46:40    99 | �[K    period.to_chrono<std::chrono::nanoseconds>�[K
09:46:40       |     �[K~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~�[K
09:46:40   100 | �[K    std::forward<CallbackT>(callbac�[K
09:46:40       |     �[K~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~�[K
09:46:40   101 | �[K    gro�[K;
09:46:40       |     �[K~~~�[K

jacobperron · 2022-08-30T17:08:24Z

We probably should have run with test args --package-above rclcpp rclcpp_lifecycle

sloretz · 2022-08-30T17:24:03Z

Checking if it's this PR

build: --packags-up-to tf2_ros, test: --packages-select tf2_ros linux aarch64

With this PR:

WIthout this PR repos file

…sses (#1975)" This reverts commit 6167a57. Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

…sses (#1975)" (#2009) This reverts commit 6167a57. Signed-off-by: Shane Loretz <sloretz@openrobotics.org> Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

…ode` classes (#1975)" (#2009)" This reverts commit 11b5f8d.

…ode` classes (#1975)" (#2009)" Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com> This reverts commit 11b5f8d.

… classes (#1975)" (#2009) (#2010) Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

jacobperron added the enhancement New feature or request label Jul 22, 2022

asymingt force-pushed the asymingt/add-create-timer-function-to-node-class branch from b852aa2 to a828be8 Compare July 22, 2022 20:01

asymingt marked this pull request as ready for review July 22, 2022 20:01

asymingt force-pushed the asymingt/add-create-timer-function-to-node-class branch from a828be8 to cf4d644 Compare July 22, 2022 20:06

jacobperron reviewed Jul 22, 2022

View reviewed changes

fujitatomoya approved these changes Jul 29, 2022

View reviewed changes

rclcpp/include/rclcpp/create_timer.hpp Outdated Show resolved Hide resolved

asymingt changed the title ~~Add a create_timer method to Node class~~ Add a create_timer method to Node and LifecycleNode classes Aug 2, 2022

asymingt commented Aug 3, 2022

View reviewed changes

jacobperron requested a review from ivanpauno August 10, 2022 20:28

ivanpauno reviewed Aug 10, 2022

View reviewed changes

rclcpp/include/rclcpp/create_timer.hpp Outdated Show resolved Hide resolved

ivanpauno reviewed Aug 10, 2022

View reviewed changes

rclcpp/test/rclcpp/test_timer.cpp Outdated Show resolved Hide resolved

asymingt force-pushed the asymingt/add-create-timer-function-to-node-class branch from 329ec52 to 3de6312 Compare August 17, 2022 23:58

asymingt mentioned this pull request Aug 20, 2022

Add clock type to node_options #1982

Merged

Add a create_timer method to the Node class

efc08b7

Signed-off-by: Andrew Symington <andrew.c.symington@gmail.com>

asymingt added 3 commits August 25, 2022 17:31

Refactor create_timer to use shared_ptr and follow a predictable call…

af24d74

… sequence Signed-off-by: Andrew Symington <andrew.c.symington@gmail.com>

Include a missing header file

b8338e8

Signed-off-by: Andrew Symington <andrew.c.symington@gmail.com>

Correct a time comparison in spin_until_time

95afb7e

Signed-off-by: Andrew Symington <andrew.c.symington@gmail.com>

ivanpauno force-pushed the asymingt/add-create-timer-function-to-node-class branch from c7acee7 to 43adf6e Compare August 26, 2022 18:53

ivanpauno approved these changes Aug 26, 2022

View reviewed changes

ivanpauno added 4 commits August 26, 2022 16:00

Revert "Refactor create_timer to use shared_ptr and follow a predicta…

f05c81c

…ble call sequence" This reverts commit 3de6312. Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Adjust create_timer() overloads, reuse code

a5c956c

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Comment out problematic test

2e6e4ff

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Please linters

1c16100

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno force-pushed the asymingt/add-create-timer-function-to-node-class branch from 43adf6e to 1c16100 Compare August 26, 2022 19:00

Fix rclcpp_lifecycle

2a9e6f6

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno requested review from clalancette and jacobperron August 26, 2022 20:22

jacobperron approved these changes Aug 29, 2022

View reviewed changes

nit

7e24c8d

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno force-pushed the asymingt/add-create-timer-function-to-node-class branch from 7e2e277 to 7e24c8d Compare August 29, 2022 19:08

ivanpauno merged commit 6167a57 into ros2:rolling Aug 30, 2022

sloretz added a commit that referenced this pull request Aug 30, 2022

Revert "Add a create_timer method to Node and LifecycleNode cla…

cd1a4e8

…sses (#1975)" This reverts commit 6167a57. Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

sloretz mentioned this pull request Aug 30, 2022

Revert "Add a create_timer method to Node and LifecycleNode classes (#1975)" #2009

Merged

ivanpauno mentioned this pull request Aug 30, 2022

Revert "Revert "Add a create_timer method to Node and LifecycleNode classes (#1975)" (#2009) #2010

Merged

ivanpauno added a commit that referenced this pull request Aug 30, 2022

Revert "Revert "Add a create_timer method to Node and `LifecycleN…

13a47fe

…ode` classes (#1975)" (#2009)" This reverts commit 11b5f8d.

ivanpauno added a commit that referenced this pull request Aug 30, 2022

Revert "Revert "Add a create_timer method to Node and `LifecycleN…

e1efffb

…ode` classes (#1975)" (#2009)" Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com> This reverts commit 11b5f8d.

ivanpauno added a commit that referenced this pull request Aug 31, 2022

Revert "Revert "Add a create_timer method to Node and LifecycleNode…

df994e4

… classes (#1975)" (#2009) (#2010) Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Add a create_timer method to Node and LifecycleNode classes #1975

Add a create_timer method to Node and LifecycleNode classes #1975

Conversation

asymingt commented Jul 22, 2022 • edited Loading

Problem

Fix

Progress

jacobperron commented Jul 22, 2022

jacobperron left a comment

Choose a reason for hiding this comment

asymingt commented Jul 22, 2022 • edited Loading

jacobperron commented Jul 22, 2022

fujitatomoya left a comment

Choose a reason for hiding this comment

Blast545 commented Aug 3, 2022

asymingt Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

jacobperron Aug 4, 2022

Choose a reason for hiding this comment

ivanpauno Aug 10, 2022

Choose a reason for hiding this comment

asymingt Aug 12, 2022 • edited Loading

Choose a reason for hiding this comment

ivanpauno Aug 17, 2022

Choose a reason for hiding this comment

ivanpauno Aug 17, 2022

Choose a reason for hiding this comment

ivanpauno Aug 26, 2022

Choose a reason for hiding this comment

jacobperron Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

ivanpauno Aug 29, 2022

Choose a reason for hiding this comment

asymingt commented Aug 4, 2022 • edited Loading

asymingt commented Aug 4, 2022 • edited Loading

ivanpauno left a comment

Choose a reason for hiding this comment

ivanpauno commented Aug 11, 2022 • edited Loading

asymingt commented Aug 12, 2022

ivanpauno commented Aug 17, 2022

ivanpauno commented Aug 17, 2022

asymingt commented Aug 17, 2022

asymingt commented Aug 18, 2022

asymingt commented Aug 19, 2022

asymingt commented Aug 19, 2022

ivanpauno commented Aug 19, 2022

asymingt commented Aug 19, 2022

ivanpauno left a comment

Choose a reason for hiding this comment

asymingt commented Aug 26, 2022

jacobperron left a comment

Choose a reason for hiding this comment

jacobperron Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

ivanpauno commented Aug 29, 2022

ivanpauno commented Aug 29, 2022

sloretz commented Aug 30, 2022

jacobperron commented Aug 30, 2022

sloretz commented Aug 30, 2022

Add a `create_timer` method to `Node` and `LifecycleNode` classes #1975

Add a `create_timer` method to `Node` and `LifecycleNode` classes #1975

asymingt commented Jul 22, 2022 •

edited

Loading

asymingt commented Jul 22, 2022 •

edited

Loading

asymingt Aug 3, 2022 •

edited

Loading

asymingt Aug 12, 2022 •

edited

Loading

jacobperron Aug 29, 2022 •

edited

Loading

asymingt commented Aug 4, 2022 •

edited

Loading

asymingt commented Aug 4, 2022 •

edited

Loading

ivanpauno commented Aug 11, 2022 •

edited

Loading

jacobperron Aug 29, 2022 •

edited

Loading