Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend Time implementation into Timer and Rate objects #465

Open
2 tasks
tfoote opened this issue Apr 26, 2018 · 3 comments
Open
2 tasks

Extend Time implementation into Timer and Rate objects #465

tfoote opened this issue Apr 26, 2018 · 3 comments
Labels
backlog enhancement New feature or request help wanted Extra attention is needed

Comments

@tfoote
Copy link
Contributor

tfoote commented Apr 26, 2018

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.
@vinnamkim
Copy link
Contributor

@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 ros2/ros2#530 (comment) 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.

  1. TimeSource class has it's own thread to spin clock_msg callback function likewise ROS1
  2. 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?

@tfoote
Copy link
Contributor Author

tfoote commented Apr 2, 2019

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.)

@jacobperron
Copy link
Member

There's also a proposal to add common functionality to rcl: ros2/rcl#898

@wjwwood wjwwood added help wanted Extra attention is needed backlog labels Jun 15, 2022
@wjwwood wjwwood removed their assignment Jun 15, 2022
nnmm pushed a commit to ApexAI/rclcpp that referenced this issue Jul 9, 2022
* Remove tests about FastRTPS not supporting MANUAL_BY_TOPIC liveliness

Signed-off-by: Emerson Knapp <[email protected]>

* Remove unused variable

Signed-off-by: Emerson Knapp <[email protected]>
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this issue Aug 5, 2022
* Add target-ros2-distro to action CI to allow it to run correctly

Signed-off-by: Emerson Knapp <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants