Improvements to ExePathFeedback (and MoveBaseFeedback)

[howardcochran]

I wish to use an external executive to control move_base_flex via the GetPathAction, ExePathAction, and RecoveryAction actions. One of the things my external executive needs to know during ExePathAction is the status of the last controller iteration. In other words, if the controller begins failing, the ExePathAction server will retry for awhile (until either controller_max_retries or controller_patience is exceeded).

Currently, during this time, the action server stops sending any feedback messages. I can improve this easily by modifying AbstractNavigationServer::callActionExePath() in abstract_navigation_server.cpp so that the ::NO_LOCAL_CMD case publishes feedback.

However, currently the feedback definition contains only distance and angle to goal, current pose, and current twist. I need it to also contain the result of the most recent invocation of the local planner. This is the same as the "outcome" field in the ExePathActionResult message and holds the return values for controllers (SUCCESS, COLLISION, ROBOT_STUCK, BLOCKED_PATH, etc.)

With this information, my high level controller can decide to take action based on why the controller is failing without having to always wait for it to give up. Actions might include switching controllers, a dynamic reconfigure, requesting a new global plan, executing a recovery behavior, etc.

It looks, to me, like adding this field and getting it populated is a pretty simple change that I can make but I want to know whether this proposal sounds acceptable for merging upstream, as I would like to avoid forking away from move_base_flex.

I can make the same change to the MoveBaseAction also.

One other note: The ROS move_base package always publishes feedback when it has a goal, regardless of whether local or planners are succeeding or failing, so it makes sense that mbf should also be able to do this.

[spuetz]

@howardcochran You are right, adding the outcome to the feedback could be useful. Thank you for your remark! I would suggest you to create a pull request after we finished #36. #36 will make a huge change to the code strcuture. Have a look at the branch https://github.com/magazino/move_base_flex/tree/feature/concurrency. We will merge that to master for version 0.2. as soon as it is stable, probably with in next week.

[howardcochran]

@spuetz, thank you much. I will rebase my proposed change after you merge the concurrency branch and then open a PR.

[corot]

Hi! As @spuetz proposed, I have merged the new concurrence branch, so feel free to try it and integrate your changes.
As it's a really massive change (and so pretty sure contains errors), please try it with your current configuration before doing any modification

[howardcochran]

OK. I have rebased on 0.2.0 (actually, current master, which is 2 commits ahead of that) and will make a PR for my proposed change momentarily.

[howardcochran]

I have made the changes requested in the review and rebased the branch on 0.2.1. It is ready for re-review or merging. Thanks.

[howardcochran]

This change was merged from PR #89 . Closing this issue.