API review

Proposer: Eitan Marder-Eppstein

Present at review:

• List reviewers

We'll discuss the ROS level API for the move_base package: move_base documentation

Question / concerns / comments

Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting.

• Josh
  • Why does move_base use private topics?
    • It doesn't anymore
  • The action API section uses "action_name/...". I assume action_name is a placeholder?
    • I screwed up with the action_name stuff.... its really just ~topic. I've changed to docs to reflect this.
• Stu:
  • On the main wiki page, some topics/parameters have tildes and some don't
    • Will be fixed with the action items below
  • "action_name" should just be set to "move_base" (or to nothing).
    • A screwup in my docs as mentioned above.
  • Interesting that the position is published under "current_position" and not as a "feedback" nor a "result"

    • Originally, I was going to move current_position to be feedback, but I wanted rviz to be able to display it which meant I needed to publish it as a PoseStamped. Makes you think a bit about feedback. It would be nice if you could subscribe to a topic/field or something to get around this.

    • With my new subtopic_forwarder tool... I can now publish this as feedback
  • How do I figure out what the other possibilities are for base_global_planner and base_local_planner? Are these plugins?
    • They are plugins... which means you can rospack plugins --attrib=plugin nav_core, I can also list all known versions in the documentation
    • Documentation updated to reflect the fact that they are indeed plugins
• Bhaskara:
  • External API includes how node will behave in world, and this should be described in the documentation. In particular:
    • What is the recovery behavior.
    • Any guarantees a user can expect, e.g., "if there are no dynamic obstacles, move base will eventually get within the tolerance of its goal, or signal failure".
      • I'll add a section to the documentation discussing things like this.
  • Since the particular recovery strategy is somewhat arbitrary, I suggest making executeCycle virtual, and the functions it calls protected.
    • Eventually, the recovery strategy will be a plugin as Ethan ticketed below, however, this will be after M3. For this round, the user won't be able to modify the recovery behavior besides stoppinf the robot from rotating in place or executing the recovery behavior.
• Ethan
  • Corrected a few minor typos in wiki

  • Ultimately a pluggable recovery behavior would be valuable, I have opened a ticket against this with more detail: https://code.ros.org/trac/ros-pkg/ticket/2883

    • We'll get to this eventually, but probably not for M3 so it won't happen with this move_base API

  • Adding a boolean parameter to disable spinning in the recovery behavior would be a useful stopgap measure: https://code.ros.org/trac/ros-pkg/ticket/2882

    • Done

Meeting agenda

To be filled out by proposer based on comments gathered during API review period

Conclusion

Package status change mark change manifest)

• Move private topics to the global namespace of the node. They can be pushed into namespaces in launch files and topics can be remapped appropriately.

• Add a section to the documentation as Bhaskara suggested for how the node should behave in the world

---