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

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

Open
surfertas opened this issue Aug 13, 2020 · 8 comments
Open

Comments

@surfertas
Copy link

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
Copy link

BrettRD commented Feb 1, 2021

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: ros-visualization/rqt#187
edit:
also related: ros-simulation/gazebo_ros_pkgs#926

@jacobperron
Copy link

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
Copy link

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 ros-visualization/rqt#187 first but I think is the right place.

@jacobperron
Copy link

@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
Copy link

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

@jacobperron
Copy link

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

@karl-schulz
Copy link

karl-schulz commented Mar 8, 2022

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:
ros-perception/image_pipeline#158 (comment)

Corresponding PR:
#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.

karl-schulz added a commit to karl-schulz/rqt_image_view that referenced this issue Mar 8, 2022
karl-schulz added a commit to karl-schulz/rqt_image_view that referenced this issue Mar 8, 2022
@jacobperron
Copy link

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).

jacobperron pushed a commit to karl-schulz/rqt_image_view that referenced this issue Mar 21, 2022
jacobperron added a commit that referenced this issue Mar 21, 2022
* Fixing #41 - using sensor data QoS profile with BEST_EFFORT

* Using SubscriptionOptions for QoS overrides

Co-authored-by: Jacob Perron <[email protected]>

* Disable QoS overrides until they will actually work

We need to wait until ROS command-line arguments are properly parsed upstream.

Signed-off-by: Jacob Perron <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
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

No branches or pull requests

4 participants