Use rclcpp::SystemDefaultsQoS in servo#721
Conversation
Codecov Report
@@ Coverage Diff @@
## main #721 +/- ##
==========================================
+ Coverage 54.24% 54.26% +0.02%
==========================================
Files 192 192
Lines 20224 20230 +6
==========================================
+ Hits 10969 10975 +6
Misses 9255 9255
Continue to review full report at Codecov.
|
henningkayser
left a comment
There was a problem hiding this comment.
This is so much cleaner than the magic numbers. What is the typical system default profile? With rmw_qos_profile_default we would at least make sure the settings are reasonable. I don't have too much insight what to use here, though.
|
I tried reading into the QoS settings and the code inside rclcpp for it and it seems this system default is what is recommended for most pub/sub. I created this issue because for a while I was trying to add one for rmw_qos_profile_default: https://answers.ros.org/question/387911/should-there-be-a-defaultqos-or-default-constructor-for-qos/ |
|
I found this comment:
I think this means that by using system default we are leaving it up to the default set by the middleware vendor or the XML file for configuring that middleware. I think this is probably more ideal for pub/sub interfaces as they are then whatever the dds implementers think is the best default config or they are what someone has configured for the default config for pub/sub on their system. |
vatanaksoytezer
left a comment
There was a problem hiding this comment.
I don't have much insight on Qos's either but looking at this I agree with Henning's comments in the sense that this is much better than using a magic number.
Description
This standardizes the QoS used by servo to be the default that the ROS documentation recommends for pub/sub interfaces.
Checklist