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

Implement Clock::sleep_for and Clock::sleep_until #1730

Closed
Tracked by #696
emersonknapp opened this issue Jul 27, 2021 · 6 comments · Fixed by #1828
Closed
Tracked by #696

Implement Clock::sleep_for and Clock::sleep_until #1730

emersonknapp opened this issue Jul 27, 2021 · 6 comments · Fixed by #1828
Assignees
Labels
enhancement New feature or request

Comments

@emersonknapp
Copy link
Collaborator

Feature request

Feature description

Implement the sleep_for and sleep_until functions specified in the "Clock and Time" design document for ROS 2 https://design.ros2.org/articles/clock_and_time.html#public-api

Implementation considerations

This will probably be best implemented using a condition_variable's wait_until and wait_for functions.

For stead/system time, these can be used, but the CV will need to be interrupted if time source type changes.

For ROS time, the CV can be awakened on receiving new clock samples, in order to check if the latest /clock time is >= the requested sleep_until time.

@emersonknapp emersonknapp changed the title Clock sleep_for and sleep_until functionality Implement Clock::sleep_for and Clock::sleep_until Jul 27, 2021
@emersonknapp
Copy link
Collaborator Author

I'm planning to work on this right now, wanted to open this issue to track work and hold discussion.

I think it makes sense to mostly implement in C++ so that we can take advantage of std::condition_variable. To implement in rcl in pure C this would require using pthread API which wouldn't port to Windows easily. Downside to this is that we'll need to implement in rclpy separately.

@emersonknapp
Copy link
Collaborator Author

emersonknapp commented Jul 29, 2021

Wow, I forgot that I opened #1730 back in March. Related/identical issue

@clalancette
Copy link
Contributor

(assigned to Emerson since he is actively working on it)

@emersonknapp
Copy link
Collaborator Author

emersonknapp commented Dec 11, 2021

I'm having some trouble using this feature in practice given the API that we defined.

The can sleep forever part is the troublesome bit. If there is no /clock signal, or something else is misconfigured, there's no way that I know of to get back the program execution without shutting down the entire context. I wonder if it would be a better API to return a Future, or have a real_time_timeout optional parameter, such that there is a guaranteed return from the function, so that a caller can still do processing, while knowing that timing is not coming through. What do you think @sloretz ?

@sloretz
Copy link
Contributor

sloretz commented Dec 11, 2021

The can sleep forever part is the troublesome bit. If there is no /clock signal, or something else is misconfigured, there's no way that I know of to get back the program execution without shutting down the entire context. I wonder if it would be a better API to return a Future, or have a real_time_timeout optional parameter, such that there is a guaranteed return from the function, so that a caller can still do processing, while knowing that timing is not coming through.

That's interesting. I would have thought sleepimg forever is a desirable feature, like when running nodes against slower-than-real-time simulations. What you describe kind of sounds like "sleep until either of these times are reached on these two clocks". Maybe that use case would be better served with two timers, one using each clock, whose callbacks set a single condition variable?

@emersonknapp
Copy link
Collaborator Author

Alternatively - instead of "sleep until either of these times are reached" - perhaps exposing a "cancel/interrupt" of some sort would work. Not sure if we would want to expose the condition_variable that is used in sleeping, but a notify call to that could be used to interrupt sleep (as long as it's not in a while loop).

I think I have a reasonable way to move forward with the behavior I want to implement in rosbag2, but it's still worth possibly thinking about an interrupt mechanism here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants