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

Subscribers always using qos depth of 1 with fastDDS #208

Closed
eschembor-irobot opened this issue Apr 4, 2023 · 3 comments · Fixed by #211
Closed

Subscribers always using qos depth of 1 with fastDDS #208

eschembor-irobot opened this issue Apr 4, 2023 · 3 comments · Fixed by #211
Labels
bug Something isn't working

Comments

@eschembor-irobot
Copy link

Description
Subscribers created by the bridge always using qos depth of 1 with fastDDS

  • Version: ca7d0c3
  • Platform: Ubuntu 20.04

Steps To Reproduce
Currently, get_publishers_info_by_topic will report all publishers as having a qos depth of 0 with fastDDS. I opened a ticket for that here ros2/rmw_fastrtps#684
This leads to foxglove_bridge using a qos depth of 1 for all subscribers when using fastDDS - which causes lots of dropped messages (a real headache for /tf visualization specifically). Hopefully, that issue with fastDDS will be resolved, but in the interim I suggest increasing the minimum qos depth for subscribers to alleviate this issue (internally, we're going to be patching foxglove_bridge for the time being to increase the minimum depth to 10).

Expected Behavior
Expect foxglove_bridge to use reasonable qos depth on subscribers when using fastDDS.

@eschembor-irobot eschembor-irobot added the bug Something isn't working label Apr 4, 2023
@foxhubber
Copy link

foxhubber bot commented Apr 4, 2023

Internal tracking ticket: FG-2767

@achim-k
Copy link
Collaborator

achim-k commented Apr 6, 2023

I guess a new parameter min_qos_depth (parameter max_qos_depth already exists) would be sufficient to solve this. Would that be an OK solution for you?

@eschembor-irobot
Copy link
Author

Yes, I think that's a good solution

achim-k added a commit that referenced this issue Apr 11, 2023
### Public-Facing Changes
- Add parameter for minimum subscription QoS depth [ROS2]

### Description
Adds an additional parameter for specifying the minimum QoS depth for
subscriptions. Fixes #208
achim-k added a commit that referenced this issue Jun 30, 2023
…epth

Some RMWs do not retrieve history information of the publisher endpoint
in which case the history depth is 0. We use a lower limit of 1 here,
so that the history depth is at least equal to the number of publishers.
This covers the case where there are multiple transient_local publishers
with a depth of 1 (e.g. multiple tf_static transform broadcasters).
See also
#238 and
#208
achim-k added a commit that referenced this issue Jul 5, 2023
…tory as unknown (#239)

### Public-Facing Changes

Assume publisher qos depth of 1 if the middleware reports the qos
history as unknown

### Description
Some RMWs do not report history information of a publisher endpoint in
which case the history depth is reported as 0. We internally use this
depth to calculate an appropriate depth for the subscription, which uses
the `KeepLast(depth)` history policy.
In case a publisher's history depth is reported as 0, we assume a depth
of 1 so that the final history depth is at least equal to the number of
publishers. This covers the case where there are multiple
transient_local publishers with a depth of 1 (e.g. multiple tf_static
transform broadcasters). Before this PR, a user would have to manually
specify a `min_qos_depth` of N when having N tf_static broadcasters.

This PR also increases the max qos depth default value, 10 seemed very
low. I'm not sure anymore why we added an upper limit in the first
place. Additionally, a warning is logged when the upper limit is
reached.

Fixes #238 
See also #208
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants