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

Revert "improved rcl_wait in the area of timeout computation and spur… #1142

Merged
merged 1 commit into from
Mar 30, 2024

Conversation

clalancette
Copy link
Contributor

…ious wakeups (#1135)"

This reverts commit 3c6c5dc.

The combination of this and ros2/rclcpp#2142 seems to be causing regressions on the buildfarm; see https://ci.ros2.org/job/ci_linux/20668/#showFailuresLink for the results.

For now, let's revert this one to get CI back to green. @mjcarroll @wjwwood @jmachowinski FYI

@clalancette
Copy link
Contributor Author

Here's a full CI run on Linux on the current code: Build Status

And here is full CI with it reverted (i.e. this PR applied):

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

@jmachowinski
Copy link
Contributor

@clalancette can you run the CI with #1143 ?

@wjwwood
Copy link
Member

wjwwood commented Mar 30, 2024

I played around with #1143 a bit but was unable to fix all the tests with it. I think it's a good start, but in order to avoid these tests failing over the weekend, I'm going to go forward with this revert and then work on re-reverting it next week with a fix like #1143.

Note the tests that failed in #1142 (comment) are known flakes that are (in my opinion) unrelated to this change.

@wjwwood wjwwood marked this pull request as ready for review March 30, 2024 00:35
@wjwwood wjwwood merged commit 1c42509 into rolling Mar 30, 2024
2 of 3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the clalancette/revert-rcl-wait branch March 30, 2024 00:36
@wjwwood
Copy link
Member

wjwwood commented Mar 30, 2024

Follow up CI for rolling, to see that this was effective post merge:

  • Linux: Build Status

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

Successfully merging this pull request may close these issues.

3 participants