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

separate callback_group from subscription options #492

Conversation

fujitatomoya
Copy link

aligned with ros2/rclcpp#1833

Signed-off-by: Tomoya Fujita Tomoya.Fujita@sony.com

@fujitatomoya
Copy link
Author

@iuhilnehc-ynos requesting review.

Copy link
Contributor

@iuhilnehc-ynos iuhilnehc-ynos left a comment

Choose a reason for hiding this comment

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

LGTM

@fujitatomoya
Copy link
Author

@clalancette @ahcorde could you take a look?

Copy link
Contributor

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Since we now have the ability to pass the callback group in directly to the callback function. The local copies of the options injecting the options can be removed too.

This should also be reviewed for how far it can be backported since it's using the new API.

tf2_ros/include/tf2_ros/transform_listener.h Outdated Show resolved Hide resolved
tf2_ros/include/tf2_ros/transform_listener.h Outdated Show resolved Hide resolved
tf2_ros/include/tf2_ros/transform_listener.h Outdated Show resolved Hide resolved
@fujitatomoya
Copy link
Author

@ahcorde @clalancette requesting maintainer review.

@fujitatomoya
Copy link
Author

@ahcorde @clalancette friendly ping.

fujitatomoya and others added 2 commits May 9, 2022 16:56
aligned with ros2/rclcpp#1833

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: Tully Foote <tfoote@osrfoundation.org>
@fujitatomoya fujitatomoya force-pushed the topic-subscription-not-hold-callback-group branch from 5b4a184 to 0e7121e Compare May 9, 2022 23:56
@fujitatomoya
Copy link
Author

closing because of ros2/rclcpp#1833 (comment)

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