ros2 / rmw

The ROS Middleware (rmw) Interface.
Apache License 2.0
95 stars 67 forks source link

Document which QoS policies are correctly read by rmw_get_publishers/subscriptions_info_by_topic #308

Closed ivanpauno closed 3 years ago

ivanpauno commented 3 years ago

Based on https://github.com/ros2/rclcpp/pull/1626#issuecomment-816998500.

ivanpauno commented 3 years ago

LGTM

I guess some RMW implementations should probably be updated to match this documentation. In that case, this may be considered an API change and perhaps we should hold until after we branch for Galactic.

The rmw implementations already match this behavior, so this is only documentation.

I don't remember if they set the policies that are not updated to RMW_QOS_POLICY_*_UNKNOWN, so I can delete that line of the docs. But I think they should be doing that, if not it's hard/impossible to understand the output.

ivanpauno commented 3 years ago

I don't remember if they set the policies that are not updated to RMW_QOS_POLICY_*_UNKNOWN, so I can delete that line of the docs. But I think they should be doing that, if not it's hard/impossible to understand the output.

Fastrtps was already doing that https://github.com/ros2/rmw_fastrtps/blob/7bb3563bebd20c3cae03231c2e321bf132651449/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp#L149. Cyclonedds shares through discovery all qos policies.

I send a patch to connextdds to fix this https://github.com/ros2/rmw_connextdds/pull/28.

ivanpauno commented 3 years ago

PR checker is happy, this a docs only PR so no need of more CI.