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

Reduce overhead for inheriting from rclcpp::Executor when base functionality is not reused #2506

Merged
merged 13 commits into from
Apr 16, 2024

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented Apr 16, 2024

refactor of "API Changes for Multihtreaded Events Executor": #2466

Janosch Machowinski and others added 10 commits April 14, 2024 14:32
This is useful in case a second thread needs to wake up another
thread, that is sleeping using a clock.

Signed-off-by: Janosch Machowinski <[email protected]>
This adds support for multiple threads waiting on the same
clock, while an shutdown is invoked.

Signed-off-by: Janosch Machowinski <[email protected]>
This commit makes every public funciton virtual, and adds virtual impl
function for the existing template functions.

The goal of this commit is to be able to fully control the everything from
a derived class.

Signed-off-by: Janosch Machowinski <[email protected]>
Co-authored-by: William Woodall <[email protected]>
Signed-off-by: jmachowinski <[email protected]>
Co-authored-by: William Woodall <[email protected]>
Signed-off-by: jmachowinski <[email protected]>
Co-authored-by: William Woodall <[email protected]>
Signed-off-by: jmachowinski <[email protected]>
Co-authored-by: William Woodall <[email protected]>
Signed-off-by: jmachowinski <[email protected]>
Signed-off-by: Janosch Machowinski <[email protected]>
This change allows it to use a second thread to wait for
the future to become ready.

Signed-off-by: Janosch Machowinski <[email protected]>
@wjwwood wjwwood self-assigned this Apr 16, 2024
Copy link
Contributor

mergify bot commented Apr 16, 2024

⚠️ The sha of the head commit of this PR conflicts with #2466. Mergify cannot evaluate rules on this PR. ⚠️

Signed-off-by: William Woodall <[email protected]>
Signed-off-by: William Woodall <[email protected]>
@wjwwood
Copy link
Member Author

wjwwood commented Apr 16, 2024

Tests are passing for me locally on rclcpp and rclcpp_lifecycle, running CI now:

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

@wjwwood
Copy link
Member Author

wjwwood commented Apr 16, 2024

Well, CI passed finally, but Windows took way too long, so we're past the deadline. I'm going to ask for an exception for merging this one in tomorrow's meeting.

@wjwwood
Copy link
Member Author

wjwwood commented Apr 16, 2024

We got approval in the ROS 2 meeting, we'll announce this exception on discourse too. Merging it now. 🎉

@wjwwood wjwwood merged commit 6bb9407 into rolling Apr 16, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the wjwwood/executor_features branch April 16, 2024 15:36
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

already merged, lgtm as history.

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/freeze-of-ros-2-base-packages-upcoming-branch-and-tutorial-party-for-jazzy-jalisco/37191/2

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.

5 participants