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

Added incompatible event support to ros2 topic echo and ros2 topic pub #410

Merged
merged 4 commits into from Apr 1, 2020

Conversation

jaisontj
Copy link
Contributor

@jaisontj jaisontj commented Nov 27, 2019

Depends on ros2/rclpy:459

Related to the feature discussed here. This PR handles the ON_REQUESTED_INCOMPATIBLE_QOS and ON_OFFERED_INCOMPATIBLE_QOS callback events on ros2 topic echo and ros2 topic pub commands respectively.

This way, users now will get a warning when they echo/pub to a topic with incompatible QoS Policies.

Signed-off-by: Jaison Titus jaisontj92@gmail.com

Signed-off-by: Jaison Titus <jaisontj92@gmail.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
@mm318
Copy link
Member

mm318 commented Mar 24, 2020

@jaisontj, @ivanpauno, @wjwwood, can any of you bring this pull request out of draft mode?

@ivanpauno ivanpauno marked this pull request as ready for review March 25, 2020 12:21
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

Left some minor comments, LGTM!

ros2topic/ros2topic/verb/echo.py Outdated Show resolved Hide resolved
ros2topic/ros2topic/verb/echo.py Outdated Show resolved Hide resolved
ros2topic/ros2topic/verb/info.py Outdated Show resolved Hide resolved
ros2topic/ros2topic/verb/pub.py Outdated Show resolved Hide resolved
ros2topic/ros2topic/verb/pub.py Outdated Show resolved Hide resolved
Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
@ivanpauno ivanpauno merged commit 02ae480 into ros2:master Apr 1, 2020
@ivanpauno ivanpauno changed the title [WIP] Added incompatible event support to ros2 topic echo and ros2 topic pub Added incompatible event support to ros2 topic echo and ros2 topic pub Apr 1, 2020
@Karsten1987
Copy link
Contributor

Karsten1987 commented Apr 28, 2020

This change calls rmw_create_publisher twice per topic on the same node. In the case of DDS (tested on FastRTPS only) this might work well, but other RMW implementations, i.e. iceoryx might not support this.

Is there a way that the publisher is called only once? Meaning, can the QoS event checked somewhat before creating a publisher?

@mm318
Copy link
Member

mm318 commented Apr 28, 2020

We could add an option to rclcpp's and rclpy's create_publisher()/create_subscription() to ignore registering callbacks of unsupported event types, instead of throwing and retrying to create_publisher()/create_subscription() at the client level.

mm318 added a commit to aws-ros-dev/ros2cli that referenced this pull request Apr 30, 2020
…topic pub (ros2#410)"

This reverts commit 02ae480.

Signed-off-by: Miaofei <miaofei@amazon.com>
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.

None yet

4 participants