Skip to content

Remove use of get_callback_groups().#320

Merged
clalancette merged 1 commit intomasterfrom
clalancette/thread-safe-callback-groups
Jul 23, 2021
Merged

Remove use of get_callback_groups().#320
clalancette merged 1 commit intomasterfrom
clalancette/thread-safe-callback-groups

Conversation

@clalancette
Copy link
Copy Markdown
Contributor

This is not thread-safe, and we can do exactly the same
thing by getting the default callback group and storing
our custom callback group within the class.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

Goes along with ros2/rclcpp#1723

This is not thread-safe, and we can do exactly the same
thing by getting the default callback group and storing
our custom callback group within the class.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Copy Markdown
Contributor Author

CI for this is in #320

@clalancette clalancette merged commit 1c3d136 into master Jul 23, 2021
@delete-merged-branch delete-merged-branch bot deleted the clalancette/thread-safe-callback-groups branch July 23, 2021 20:55
@Karsten1987
Copy link
Copy Markdown
Contributor

@ralph-lange just fyi. Can you verify that the RT ping/pong behavior is still in tact here?

@ralph-lange
Copy link
Copy Markdown
Contributor

Thank you for pointing to this PR. Yes, the ping-pong demo works as before. I've already tested it on Saturday, while adding jitter measurement, cf. https://github.com/boschresearch/ros2_examples/tree/cbg_executor/jitter.

@ros-discourse
Copy link
Copy Markdown

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

https://discourse.ros.org/t/compilation-bloat-issue/30345/1

jefferyyjhsu pushed a commit to jefferyyjhsu/ros2-examples that referenced this pull request Apr 19, 2023
This is not thread-safe, and we can do exactly the same
thing by getting the default callback group and storing
our custom callback group within the class.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
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