Add timer on reset callback

[mauropasse]

@wjwwood this PR is created after discussions originated from this PR: https://github.com/ros2/rcl/pull/966 Instead of adding an on_trigger callback to the rcl guard conditions, we add here an on_reset callback to the rcl timers.

The issue which originated this need, is wake up an events based executor when a timer is reset (which could also be caused by a time jump).

FYI @alsora

[timonegk]

@wjwwood @clalancette It would be very nice to merge this and https://github.com/ros2/rcl/pull/995 soon. We are trying to use https://github.com/irobot-ros/events-executor/, but it requires these pull requests, and manually rebuilding rcl and rclcpp with the pull requests merged is not sufficient because the rclcpp pull request includes ABI changes that require a rebuild of all packages using TimerBase. It also seems as if the comments on both pull requests have been addressed.

[Flova]

Another ping on this one due to the breaking ABI changes and the need to build nearly everything from source as a consequence if the events executor is used in a single node.

Also I think you mean https://github.com/ros2/rclcpp/pull/1979 @timonegk right?

[Flova]

As the maintainers were updated recently also a frindly ping at @audrow and @ivanpauno

[ivanpauno]

The issue which originated this need, is wake up an events based executor when a timer is reset (which could also be caused by a time jump).

Could you explain that in more detail?

[mauropasse]

Could you explain that in more detail?

There are two situations in which a wait-set based executor can be awaken when something changes related to the timer, by means of triggering its guard condition:

1. Timer is reset: https://github.com/ros2/rcl/blob/e4f7e1367dfda83d3e309397f9201642ebe38618/rcl/src/rcl/timer.c#L431
2. Timer detects a time jump: https://github.com/ros2/rcl/blob/e4f7e1367dfda83d3e309397f9201642ebe38618/rcl/src/rcl/timer.c#L113

Before these same situations, the on_reset_callback is called. Maybe a new on_time_jump_callback could be created, along the on_reset_callback. In the current state, the on_reset_callback is used for both purposes.

[ivanpauno]

Timer is reset:

I think we trigger the guard condition because the timeout used in rcl_wait() wasn't calculated considering that timer. A callback isn't trigger immediately when a timer is reset. Based on that, I'm not sure if you would need to trigger a callback in this case for the events executor. But if it's needed I think it's fine to add it.

Maybe a new on_time_jump_callback could be created, along the on_reset_callback.

IMO, that sounds better. It's weird to have a on_reset callback that's also triggered on jump. Could the clock jump callbacks be used for that though?

[mauropasse]

Could the clock jump callbacks be used for that though?

Yes, no need to add an extra callback for it. I removed the callback call on _rcl_timer_time_jump. That function is actually used as a clock jump callback you mentioned.