[depth_image_proc] using sensor data qos profile everywhere #1081
Conversation
| // TODO(ros2) Implement when SubscriberStatusCallback is available | ||
| // pub_depth_ = it_->advertise("image", 1, connect_cb, connect_cb); | ||
| pub_depth_ = image_transport::create_publisher(this, "image"); | ||
| pub_depth_ = image_transport::create_publisher(this, "image", rmw_qos_profile_sensor_data); |
There was a problem hiding this comment.
This line should be reverted - the strategy throughout image_pipeline is supposed to be:
- All publishers should be SystemDefaultQOS with allowable overrides
- All subscribers should be SensorDataQOS with allowable overrides
This follows REP-2003 and deals with the fact that a DefaultQOS publisher will publish best effort when a SensorQOS subscribes to it - but is entirely incompatible with a DefaultQOS subscriber.
| // TODO(ros2) Implement when SubscriberStatusCallback is available | ||
| // pub_depth_ = it_->advertise("image", 1, connect_cb, connect_cb); | ||
| pub_depth_ = image_transport::create_publisher(this, "image"); | ||
| pub_depth_ = image_transport::create_publisher(this, "image", rmw_qos_profile_sensor_data); |
| pub_registered_ = image_transport::create_camera_publisher(this, "depth_registered/image_rect"); | ||
| pub_registered_ = image_transport::create_camera_publisher( | ||
| this, "depth_registered/image_rect", | ||
| rmw_qos_profile_sensor_data); |
| registered_msg->width = resolution.width; | ||
| // step and data set in convert(), depend on depth data type | ||
|
|
||
|
|
There was a problem hiding this comment.
we can revert this extra newline
|
Wait - I just noticed this was targeted at humble (as I was trying to figure out how these updates got missed when I did a big pass over QoS updates). I'm not sure we actually want to change the default behavior in Humble - as that is a stable release, as Humble as been out nearly 3 years, people likely expect it's behavior not to change. |
I understand. I also had this doubt while creating the PR, however I also thought that setting the subscribers to use a data sensor qos profile would have been no issue, as it is anyway compatible with the default qos profile that people might have been using for their camera publishers so far in humble |
Yeah, I'm gonna leave this open and see if there is any other community feedback |
2 participants
In
depth_image_proc, some publishers are already set to use sensor data QoS; however, this is not in place for all of them, nor is for any subscriber.This PR aims at changing the QoS profile for those publishers and subscribers that are still using the default one, in order to have all of them set to use a sensor data QoS.