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

Make ServiceData thread-safe #292

Merged
merged 5 commits into from
Oct 10, 2024
Merged

Make ServiceData thread-safe #292

merged 5 commits into from
Oct 10, 2024

Conversation

Yadunund
Copy link
Member

@Yadunund Yadunund commented Oct 9, 2024

Thread-safe access to the rmw_service_t handler. Followed the exact same pattern as in #288

@ahcorde
Copy link
Contributor

ahcorde commented Oct 9, 2024

ros2.repos

rmw_zenoh/yadu/raii-service

ROS Package Failing test name Failing test output
rcl None None
rcl_action None None
rcl_lifecycle None None
rcl_yaml_param None None
rclcpp test_intra_process_manager Failed (Check logs)
rclcpp test_node_interfaces__node_graph Failed (Check logs)
rclcpp test_publisher Failed (Check logs)
rclcpp test_events_executor Timeout (Check logs)
rclcpp test_wait_set Failed (Check logs)
rclcpp_action None None
rclcpp_components None None
rclcpp_lifecycle None -
rmw_zenoh_cpp None None
test_cli None None
test_cli_remap test_cli_remapping TBD

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Just some minor comments. Tests are in the same status, I think it's safe to merge it

rmw_zenoh_cpp/src/detail/rmw_data_types.cpp Show resolved Hide resolved
rmw_zenoh_cpp/src/detail/rmw_node_data.hpp Outdated Show resolved Hide resolved
rmw_zenoh_cpp/src/detail/rmw_node_data.hpp Outdated Show resolved Hide resolved
@ahcorde
Copy link
Contributor

ahcorde commented Oct 9, 2024

This PR ros2/rclcpp#2648 removed the wait_set failure

Signed-off-by: Yadunund <[email protected]>
Signed-off-by: Yadunund <[email protected]>
Signed-off-by: Yadunund <[email protected]>
Copy link
Collaborator

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I also tested this with the Nav2 test, and it seems to work pretty well.

@Yadunund Yadunund merged commit ca899ac into rolling Oct 10, 2024
8 checks passed
@Yadunund Yadunund deleted the yadu/raii-service branch October 10, 2024 21:32
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