ros-visualization / rqt_image_view

http://wiki.ros.org/rqt_image_view
25 stars 59 forks source link

Allowing an input field to change the Quality of Service parameter. #41

Open surfertas opened 4 years ago

surfertas commented 4 years ago

Hi all, thanks for maintaining this package.

I was wondering with ROS2 and the introduction of QoS, if it makes sense to allow for an input field that allows the user to change the QoS of the subscriber in order to meet the QoS compatibility requirements.

If this is something that is worth considering, I can try and work on a PR.

BrettRD commented 3 years ago

This is a standing problem, and I would love to see a PR for it. The gazebo plugins seem to do their own thing, and cause a number of QoS issues. Automatic selection of QoS, would help a lot. related: https://github.com/ros-visualization/rqt/issues/187 edit: also related: https://github.com/ros-simulation/gazebo_ros_pkgs/pull/926

jacobperron commented 3 years ago

FWIW, as of ROS Galactic we will be able to change the QoS settings via ROS parameters. Note, this require enabling the feature in rqt_image_view (which probably makes sense to do).

karl-schulz commented 2 years ago

Would it be an option to set the default QoS for subscribers in rqt_image_view to BEST_EFFORT? Most image data is published via BEST_EFFORT anyways, and messages with RELIABLE will still be shown (it's compatible in that direction).

Obviously, having an option like in RVIZ would be ideal, but probably also more work.

I posted this comment in https://github.com/ros-visualization/rqt/issues/187 first but I think is the right place.

jacobperron commented 2 years ago

@karl-schulz I think defaulting to "best effort" QoS makes sense for subscriptions. I'm happy to take a look at a PR changing the defaults.

karl-schulz commented 2 years ago

I guess it should be targeted to the rolling-devel branch? Also, does the changelog have to modified in the PR?

jacobperron commented 2 years ago

Yes, please target rolling-devel. No need to touch the changelog file (it is updated during a release).

karl-schulz commented 2 years ago

Hi @jacobperron after some time I found the time to implement the change, but stumbled upon a different PR, using the QOS of a publisher on the given topic. I just wanted to make sure that we decide for the BEST_EFFORT default instead of this solution before creating a PR.

Issue: https://github.com/ros-perception/image_pipeline/issues/158#issuecomment-157524991

Corresponding PR: https://github.com/ros-visualization/rqt_image_view/pull/53

Alternatively we can also combine both and use BEST_EFFORT as a default when there is no publisher.

I think simply using BEST_EFFORT has no real downsides and is more predictable, as RQT probably never is dependent on reliability, I just wanted to double-check your opinion.

jacobperron commented 2 years ago

I think defaulting to best_effort (and providing the ability to override QoS options at start-up) is more straight-forward than adapting to the first discovered publisher (as in #53).