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.

Sign up for GitHub

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

Jump to bottom

Jazzy: Follow REP-2003 and allow overrides #847

Closed
6 tasks done
mikeferguson opened this issue Jan 17, 2024 · 3 comments
Closed
6 tasks done

Jazzy: Follow REP-2003 and allow overrides #847

mikeferguson opened this issue Jan 17, 2024 · 3 comments
Assignees
@mikeferguson
Labels
enhancement ros2

Comments

@mikeferguson
Copy link
Member

mikeferguson commented Jan 17, 2024 •
edited
Loading

  • All publishers should be SystemDefaultQOS with allowable overrides
  • All subscribers should be SensorDataQOS with allowable overrides

Notes on overrides: https://docs.ros.org/en/iron/How-To-Guides/Overriding-QoS-Policies-For-Recording-And-Playback.html

From point_cloud_node:

auto sub_opts = rclcpp::SubscriptionOptions();
sub_opts.qos_overriding_options = rclcpp::QosOverridingOptions::with_default_policies();
  • image_save should subscribe with SensorDataQOS (Problem with QoS #828)
  • image_proc
  • stereo_image_proc
  • depth_image_proc
  • image_view
  • Add tutorial to image_pipeline tutorials on how to use QoS (link to design doc above also)
@mikeferguson mikeferguson self-assigned this Jan 17, 2024
This was referenced Jan 17, 2024
Closed
Closed
@mikeferguson mikeferguson changed the title Jammy: Follow REP-2003 Jazzy: Follow REP-2003 Jan 17, 2024
@mikeferguson mikeferguson mentioned this issue Jan 17, 2024
Closed
@mikeferguson mikeferguson changed the title Jazzy: Follow REP-2003 Jazzy: Follow REP-2003 and allow overrides Jan 19, 2024
@mikeferguson mikeferguson mentioned this issue Jan 19, 2024
Closed
@mikeferguson mikeferguson mentioned this issue Feb 1, 2024
Merged
mikeferguson added a commit that referenced this issue Feb 4, 2024
@mikeferguson
QoS improvements for image_proc and stereo_image_proc (#922)
af7f154 
First part of #847 
* Add QoS overrides for all publishers (in the new, standard way)
* stereo_image_proc: Default subscriber QoS to SensorDataQoS
* Clean up some of the comments around lazy subscribers, make them more
consistent across nodes
@mikeferguson mikeferguson mentioned this issue Mar 19, 2024
Closed
@tcappellari-bdai
Copy link

Is this still being worked on? We are having issues with these nodes as the QoS for subscribers are not sensor defaults

Kotochleb pushed a commit to Kotochleb/image_pipeline that referenced this issue May 27, 2024
@mikeferguson
QoS improvements for image_proc and stereo_image_proc (ros-perception…
af0e5f8 
…#922)

First part of ros-perception#847 
* Add QoS overrides for all publishers (in the new, standard way)
* stereo_image_proc: Default subscriber QoS to SensorDataQoS
* Clean up some of the comments around lazy subscribers, make them more
consistent across nodes
@mikeferguson mikeferguson mentioned this issue Aug 6, 2024
Merged
@mikeferguson
Copy link
Member Author

mikeferguson commented Aug 6, 2024 •
edited
Loading

I finally got around to finishing this work - PR is here: #1019

There are a few nodes/components that are not updated due to API limitations in message_filters and/or image_transport - I'll file a separate ticket with those nodes listed and what upstream API stuff we should be pushing for K-turtle

mikeferguson added a commit that referenced this issue Aug 19, 2024
@mikeferguson
Finish QoS updates (#1019)
d272c4e 
This implements the remainder of #847:

 - Make sure publishers default to system defaults (reliable)
- Add QoS overriding where possible (some of the image_transport /
message_filters stuff doesn't really support that)
 - Use the matching heuristic for subscribers consistently
mergify bot pushed a commit that referenced this issue Aug 19, 2024
@mikeferguson @mergify
Finish QoS updates (#1019)
77eb881 
This implements the remainder of #847:

 - Make sure publishers default to system defaults (reliable)
- Add QoS overriding where possible (some of the image_transport /
message_filters stuff doesn't really support that)
 - Use the matching heuristic for subscribers consistently

(cherry picked from commit d272c4e)

# Conflicts:
#	depth_image_proc/src/point_cloud_xyzi_radial.cpp
@mergify mergify bot mentioned this issue Aug 19, 2024
Merged
mikeferguson added a commit that referenced this issue Aug 19, 2024
@mikeferguson
Finish QoS updates (#1019)
95d4769 
This implements the remainder of #847:

 - Make sure publishers default to system defaults (reliable)
- Add QoS overriding where possible (some of the image_transport /
message_filters stuff doesn't really support that)
 - Use the matching heuristic for subscribers consistently
mikeferguson pushed a commit that referenced this issue Aug 19, 2024
@mergify
Finish QoS updates (backport #1019) (#1024)
14a21c5 
This implements the remainder of #847:

 - Make sure publishers default to system defaults (reliable)
- Add QoS overriding where possible (some of the image_transport /
message_filters stuff doesn't really support that)
 - Use the matching heuristic for subscribers consistently
@mikeferguson
Copy link
Member Author

Merged into both Jazzy and Rolling - I'll cut a new release shortly

@mikeferguson mikeferguson mentioned this issue Nov 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mikeferguson mikeferguson

Labels
enhancement ros2
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mikeferguson @tcappellari-bdai