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

improved rcl_wait in the area of timeout computation and spurious wakeups #1135

Merged
merged 4 commits into from
Mar 29, 2024

Conversation

jmachowinski
Copy link
Contributor

These patches fix unneeded wake ups in case timer override is active.
Also the improve the timeout computation in case of many registered timers.

Janosch Machowinski added 3 commits March 7, 2024 20:47
Signed-off-by: Janosch Machowinski <j.machowinski@nospam.org>
Added special handling for timers with a clock that has time override
enabled. For theses timer we should not compute a timeout, as the waitset
is waken up by the associated guard condition.
Before this change, the waitset could wait up, because of an expected ready
timer, that was acutally not ready, as the time update to the ROS_TIME had
not yet arrived.

Signed-off-by: Janosch Machowinski <j.machowinski@nospam.org>
Signed-off-by: Janosch Machowinski <j.machowinski@nospam.org>
@jmachowinski
Copy link
Contributor Author

@sloretz You added the TODO, perhaps you want to have a look at this.

@fujitatomoya fujitatomoya added the enhancement New feature or request label Mar 14, 2024
@mjcarroll
Copy link
Member

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@mjcarroll
Copy link
Member

@jmachowinski some linting as well as test failures on this one, can you take a look?

@jmachowinski jmachowinski force-pushed the timer_improvements branch 2 times, most recently from ab72207 to 6fac0d8 Compare March 25, 2024 12:37
This commit changes the computation of the timer timeout, to be more
precise, in the case, of many registered timers.

Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com>
@jmachowinski
Copy link
Contributor Author

jmachowinski commented Mar 25, 2024

@mjcarroll fixed

@mjcarroll
Copy link
Member

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@mjcarroll
Copy link
Member

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@mjcarroll mjcarroll merged commit 3c6c5dc into ros2:rolling Mar 29, 2024
2 of 3 checks passed
clalancette added a commit that referenced this pull request Mar 29, 2024
wjwwood pushed a commit that referenced this pull request Mar 30, 2024
@wjwwood
Copy link
Member

wjwwood commented Mar 30, 2024

Note that this was reverted in #1142 (comment), and we'll be working on re-applying this change with a fix to avoid broken tests asap.

@jmachowinski
Copy link
Contributor Author

Found the bug, and also some bugs in the test. I will push a PR tomorrow.

@jmachowinski
Copy link
Contributor Author

ros2/rclcpp#2469
#1146

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 this pull request may close these issues.

None yet

4 participants