rclcpp
rclcpp copied to clipboard
Extend Time implementation into Timer and Rate objects
The Timer and Rate need to use a clock, with a coupled time source to accurately sleep.
- [ ] Remove internal Rate sleep method, and externalize it with a valid clock or pass in the clock.
- [ ] Provide a sleep method for the Timer class.
@tfoote Is there any progress on this issue? or Anyone who working on this issue?
In my opinion, to solve this issue, it seems to implement clock.sleep_for() and clock.sleep_until() such as https://github.com/ros2/ros2/issues/530#issue-339636054 and https://design.ros2.org/articles/clock_and_time.html#public-api.
To implement clock.sleep, Clock would be stuck in a while loop until the target time is coming. In the case of the simulation time, Clock doesn't subscribe clock_msg by itself. The solution would be the followings.
- TimeSource class has it's own thread to spin clock_msg callback function likewise ROS1
- Clock has to know It's parent TimeSource at executing sleep function, and spin TimeSource's clock_msg callback until the target time is coming.
However, TimeSource can manage multiple Clocks. So, the first one is more preferred. Does it makes sense?
Yes, a clock.sleep_for() method and clock.sleep_until() method are the next steps.
For the threading, this is something that I think that there's better solutions than just having each time source spin it's own thread. We don't yet have callback groups implemented either. I'd suggest starting an implementation without worrying about the extra thread and we can make that more robust in a second round. The developer facing API is the first thing to think about. If we don't find a better solution than adding a thread to the TimeSource it can be added to the implementation. But you're right that the Clock shouldn't know anything about subscribing. In the case that simulated time is active the clock can block waiting for a set_clock call from the time source. (And likely create an error if a TimeSource is not attached.)
There's also a proposal to add common functionality to rcl: https://github.com/ros2/rcl/issues/898