search

ros / common_msgs

Commonly used messages in ROS. Includes messages for actions (actionlib_msgs), diagnostics (diagnostic_msgs), geometric primitives (geometry_msgs), robot navigation (nav_msgs), and common sensors (sensor_msgs), such as laser range finders, cameras, point clouds.
http://wiki.ros.org/common_msgs
186 stars 190 forks source link

Add feedback to GetMap.action #154

Closed JaccovdS closed 4 years ago

JaccovdS commented 4 years ago

The GetMap.action now only contains a result. This PR extends the action with feedback which is useful for getting progress updates while doing mapping.

tfoote commented 4 years ago

Can you please explain what a progress_map is and how does it indicate progress on a getter type of call?

JaccovdS commented 4 years ago

The progress map is basically a partial map that has been build up during the mapping process until now. I.e. if you are mapping a big space you also might want to track the progress visually. Some packages now provide a topic to which you can subscribe to see the map created until now. However since mapping can also be considered as a process which takes a long time it makes sense to provide an action interface which not only provides the resulting map but also the 'in between' versions.

tfoote commented 4 years ago

As I understand this action it's a non-blocking analog to the service GetMap.srv and should return the current map as soon as practical. I'm not familiar with any general concept of when a map is complete. And this action does not therefore expect to wait for "completeness". There is some point when a human often says "good enough" and stops building/updating the map. But that's more a practical thing that we don't have standard implementations of life long mapping where maps are continuously updating with changes to the map. Which again comes back to the lack of concept of completeness. Similarly if this is a feedback message for the sense of progress it should convey some information which provides feedback on the process of the action which is GetMap.

It sounds more like you're implementing an action which I would classify as exploring the environment and presumably implementing an exploration algorithm which will explore until you have coverage of all the frontiers are explore or some other metric. To get to that end you should create a specific action to cover that semantic use case. And then you can add in meaningful progress indicators such as a hypothetical frontier coverage ratio and if you want it could also stream back the in progress map. But you seem to be adding a lot more semantic meaning and that should be caputred into your action definition and not try to shoe horn it into an existing generic action like GetMap.

JaccovdS commented 4 years ago

I agree with you that there is no completeness in a mapping process in general. And that is the reason that I think it would be beneficial to provide the map i.e. the OccupancyGrid that has been built up until this point in time as feedback. The user can use for example rviz to judge if exploration should be terminated by monitoring this feedback.

I agree with you that there might be situations where you want to add additional or different information to cover different use-cases and these should be captured in a different action message. However I don't see why that is a reason to not add feedback to this action. The action does not become less generic by adding this feedback.

JaccovdS commented 4 years ago

Change doesn't seem to be inline with the philosophy. Closing PR.