search

swri-robotics / mapviz

Modular ROS visualization tool for 2D data.
BSD 3-Clause "New" or "Revised" License
371 stars 144 forks source link

Update GPS subscrption to use sensor qos #750

Closed shrijitsingh99 closed 2 years ago

shrijitsingh99 commented 2 years ago

All sensors are generally expected to use the sensor QoS. There is even a REP proposal to standardise it https://github.com/ros-infrastructure/rep/pull/212. Besides that one of the most commonly use packages for localization i.e. robot_localization also uses that QoS, so it would make sensr for Mapviz to also support it.

danthony06 commented 2 years ago

@daniel-dsouza I think we need a pretty good review of this. Are you interested in taking a look at it?

shrijitsingh99 commented 2 years ago

Sorry, about that. I pushed the wrong commit before. This PR is just a minor change. Though it will require changes at multiple places since many of the plugins subscribe to sensor data.

danthony06 commented 2 years ago

Ah, I see now. Let me read through that REP and we should get it merged shortly.

shrijitsingh99 commented 2 years ago

Friendly ping Just adding more information sensor data qos just makes a smaller queue size and set reliability to best effort. Having best efforts reliability give maximum compatibility since then it works with both reliable and best effort publishers. https://docs.ros.org/en/rolling/Concepts/About-Quality-of-Service-Settings.html

matt-attack commented 2 years ago

Since this is for visualization, there is actually value in having reliable transport unless the network doesn't permit it. Generally I would like to see all the messages and not just whatever happens to make it through for debugging purposes. Perhaps adding an option to mapviz to let the user specify what reliability they would like on a global basis would be a good idea.

danthony06 commented 2 years ago

We also should merge this into the ros2-devel branch.