SteveMacenski / slam_toolbox

Slam Toolbox for lifelong mapping and localization in potentially massive maps with ROS
GNU Lesser General Public License v2.1
1.6k stars 513 forks source link

Raster map is updated and published even with no external subscribers #632

Closed james-ward closed 1 year ago

james-ward commented 1 year ago

Required Info:

Steps to reproduce issue

ros2 topic info -v /map

Expected behavior

No subscribers to /map topic

Actual behavior

In our application updating the raster map takes significant time, and stops incoming scans being processed when it happens. We don't actually use the map, so we don't want to do it.

The slam_toolbox node is self-subscribed. This means that the short-circuit to only update the map if there is a subscriber never fires: https://github.com/SteveMacenski/slam_toolbox/blob/912703c43b7a640303b1b5fc62f05d53fae9cf57/src/slam_toolbox_common.cpp#L390 The culprit is the map_saver helper which is creating a subscription at initialisation: https://github.com/SteveMacenski/slam_toolbox/blob/912703c43b7a640303b1b5fc62f05d53fae9cf57/src/map_saver.cpp#L41

Additional information

I'm happy so submit a PR, but want to know what approach is preferred. I can think of three off the top of my head:

  1. Change the check to be >1 rather than >0. Not nice for maintainability, and means the map saver only works if there is another subscriber.
  2. Lazy initialisation of the subscriber. Means a delay when calling the saver service to check that the topic is published.
  3. A config entry to specify whether to load the map saver at runtime.
  4. ??? something else entirely???
SteveMacenski commented 1 year ago

A config entry to specify whether to load the map saver at runtime.

That would be my first instinct, so lets do that! Just a use_map_saver and if false then it just doesn't create the object on on_configure. That seems like an easy enough PR! Just make sure to update the readme / example yamls with the new param