Move the monolithic move_base action out of mbf_abstract_nav

[corot]

It's obviously anything but "abstract". Most simple change is to put it on mbf_costmap_2d_nav, but as @reinzor suggested, putting it in a separated node that calls the other actions would extend this feature to whatever navigation system we use, not only costmap-based navigation.

[reinzor]

If this state machine code is going to be moved, I would suggest moving it to another node to keep the mbf_costmap_2d_nav server clean:

• mbf_costmap_2d_nav_server (exposing the https://github.com/magazino/move_base_flex/tree/advanced_naming/mbf_msgs interfaces except for the MoveBaseAction)
• mbf_costmap_2d_move_base_node (containing the state machine logic and merge this with the python node functionality present in https://github.com/magazino/move_base_flex/blob/advanced_naming/mbf_costmap_2d_nav/scripts/move_base_legacy_relay.py)

This way, the MoveBase action can be deprecated: https://github.com/magazino/move_base_flex/blob/advanced_naming/mbf_msgs/action/MoveBase.action

[corot]

mmm.... 🤔 I don't want to deprecate mbf_msgs/MoveBase action! The first idea behind MBF is to allow external executives to manage navigation, but we also want to overcome the limitations of current move_base, by providing a much more detailed MoveBase action.

A different thing is the move_base_msgs/MoveBase action provided by the move_base_legacy_relay.py scripts, that it's there to allow us to claim that MBF is fully backward-compatible with current move_base, and yes, is deprecated (though not clearly stated... will fix that in the wiki)

[reinzor]

Ok. If I understand this correctly: you want to provide one action to move the base right? Why not make a separate node then (or even package) that exposes this functionality and calls the other actions of the server? This way, this node can be used for all servers and you make a clear separation between the server and the statemachine logic.

Basically this is transferring the statemachine logic (that is now in the astract server), to a separate node. In order to be backwards compatible and provide a richer move interface. This can then also be used as an example for others to implement there own statemachine.

[corot]

Very good idea!
Indeed, we must remake this action server to also implement #1 and #18. We are thinking now what to do exactly, probably a separated node as you propose.

[corot]

🤔
I'm rethinking this a bit... why move_base action cannot be on the abstract server? Once renamed to something with less connotations as Navigate.action, or MoveTo.action, it makes perfect sense to provide this action server as part of any navigation framework, implemented by just calling the other actions.

[spuetz]

I've divided the actions into separated classes with in the new structure I implemented for the mbf_abstract_nav, which goes along with the plugin concurrency issue #36. So #36 will solve this issue too, more or less.

[corot]

Right, this actions get implemented as a MoveBaseAction object, same way as Controller/Planner/RecoveryAction are. Not sure if it's the cleanest option... but makes sense to me, and so I'll let it as it is by now. @reinzor suggestion of a external node makes sense too, though, so I'll keep the idea in mind