moveit2
moveit2 copied to clipboard
Use sensor data QOS profile for sensor_msgs::Image and sensor_msgs::PointCloud
Description
I have been working with a Realsense lately and suspect there are some network traffic issues. I'm starting to read up on how we should be using DDS effectively. This first PR will handle updating point clouds/depth images publishers to use BEST_EFFORT for instead of RELIABLE. Some explanation of the differences are available here.
Currently the Realsense drivers use the system_default QOS, which has reliable settings, but the launch files take in parameters to change it for various topics. Testing this with a Realsense, you would need to launch it with the following parameters:
pointcloud_qos:=SENSOR_DATA color_qos:=SENSOR_DATA depth_qos:=SENSOR_DATA infra_qos:=SENSOR_DATA
Similarly, Rviz plugins for visualizing image/pointcloud data should use Reliability Policy: Best Effort - I have not yet tested if it will autodetect the QOS settings from the publisher and match them.
When testing this PR with applications using streaming images, users should run ros2 doctor -r and look for warnings about mismatched QOS settings between pub and sub.
More info about some of the defined QOS policies:
"sensor_data": what we should use for streaming large data constructs
static const rmw_qos_profile_t rmw_qos_profile_sensor_data =
{
RMW_QOS_POLICY_HISTORY_KEEP_LAST,
5,
RMW_QOS_POLICY_RELIABILITY_BEST_EFFORT,
RMW_QOS_POLICY_DURABILITY_VOLATILE,
RMW_QOS_DEADLINE_DEFAULT,
RMW_QOS_LIFESPAN_DEFAULT,
RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT,
RMW_QOS_LIVELINESS_LEASE_DURATION_DEFAULT,
false
};
"default" - what is currently used by MoveIt and by default by realsense
static const rmw_qos_profile_t rmw_qos_profile_default =
{
RMW_QOS_POLICY_HISTORY_KEEP_LAST,
10,
RMW_QOS_POLICY_RELIABILITY_RELIABLE,
RMW_QOS_POLICY_DURABILITY_VOLATILE,
RMW_QOS_DEADLINE_DEFAULT,
RMW_QOS_LIFESPAN_DEFAULT,
RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT,
RMW_QOS_LIVELINESS_LEASE_DURATION_DEFAULT,
false
};
"system_default" - this is different than "default" and is defined by the ros middleware. I'm not sure what these values are for cyclone. This is just a note for people to know that in the context of ROS2 DDS "default" != "system_default"
static const rmw_qos_profile_t rmw_qos_profile_system_default =
{
RMW_QOS_POLICY_HISTORY_SYSTEM_DEFAULT,
RMW_QOS_POLICY_DEPTH_SYSTEM_DEFAULT,
RMW_QOS_POLICY_RELIABILITY_SYSTEM_DEFAULT,
RMW_QOS_POLICY_DURABILITY_SYSTEM_DEFAULT,
RMW_QOS_DEADLINE_DEFAULT,
RMW_QOS_LIFESPAN_DEFAULT,
RMW_QOS_POLICY_LIVELINESS_SYSTEM_DEFAULT,
RMW_QOS_LIVELINESS_LEASE_DURATION_DEFAULT,
false
};
Best-effort settings reduce the amount of network traffic since the DDS implementation does not have to incur the overhead of reliable communications, where publishers require acknowledgements for messages sent to subscribers and must resend samples that have not been properly received.
Work to be done:
- [] Discuss adding the QOS to be used as a parameter versus just changing the hardcoded policy
- [] Perform some characterization of if/when this has a significant impact on network performance
Checklist
- [ ] Required by CI: Code is auto formatted using clang-format
- [ ] Extend the tutorials / documentation reference
- [ ] Document API changes relevant to the user in the MIGRATION.md notes
- [ ] Create tests, which fail without this PR reference
- [ ] Include a screenshot if changing a GUI
- [ ] While waiting for someone to review your request, please help review another open pull request to support the maintainers
Codecov Report
Base: 50.92% // Head: 50.95% // Increases project coverage by +0.04% :tada:
Coverage data is based on head (
1af8c7c) compared to base (8dcbe40). Patch has no changes to coverable lines.
Additional details and impacted files
@@ Coverage Diff @@
## main #664 +/- ##
==========================================
+ Coverage 50.92% 50.95% +0.04%
==========================================
Files 378 378
Lines 31671 31671
==========================================
+ Hits 16125 16135 +10
+ Misses 15546 15536 -10
| Impacted Files | Coverage Δ | |
|---|---|---|
| moveit_ros/moveit_servo/src/pose_tracking.cpp | 77.26% <0.00%> (+0.48%) |
:arrow_up: |
| moveit_ros/moveit_servo/src/servo_calcs.cpp | 67.43% <0.00%> (+1.46%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
This pull request is in conflict. Could you fix it @nbbrooks?
@henningkayser @jliukkonen I made some changes based on what you said, give it a look and let me know if anything else needs changing. I wasn't quite sure about the changes in depth_self_filter_nodelet so let me know if those aren't what you meant.
I rebased this, if it passes CI I'm going to merge this. Speak now if I should not do that.
This pull request is in conflict. Could you fix it @nbbrooks?