API review

Proposer: Eitan Marder-Eppstein and Vijay Pradeep

Make sure you read this page before coming to the review: http://pr.willowgarage.com/wiki/actionlib

Also, see this page for Doxygen/Code level API: http://pr.willowgarage.com/pr-docs/ros-packages/actionlib/html/index.html

Present at review:

• Pretty much everybody

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.

• [Vijay] Having 6 template arguments for both the ActionServer and ActionClient is quite unwieldy. With the typedef generators in the ROS 0.7 release, we can now template on just a single type (ie. ActionClient<MoveBase>)

• [Roland] For the client-side "GoalHandle sendGoal()", is it possible to set a NULL completion_callback if I want just feedback_callback?

  • [Vijay] Yes, it is possible: my_client.sendGoal(my_goal, ActionClient::CompletionCallback(), &myFeebackCb); ... We can discuss whether this is a 'clean enough' API.

• [Eitan] Do we need a ROS Header in our messages, or is it unnecessary?

Meeting agenda

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

Conclusion

Package status change mark change manifest)

• Action items that need to be taken.

• Major issues that need to be resolved

• Perhaps in future, add waitForGoal(), which blocks on a condition variable that is signaled on receipt of a new goal. Allows the user to write a polling-style (instead of callback-style) action server.
• Should separate preempt and goal messages onto separate topics.
  • As a consequence, need to sort out logic for out-of-order preempt/goal receipt. E.g., if you get a preempt for a goal you don't know about, do you keep the preempt around? For how long?
    • Keep goal-specific cancellations around, up to some maximum number (just for memory management); recalling goals as they arrive.
    • Cancel requests with timestamp in the future sticks around until that time, recalling all goals that arrive in the meantime.

• Add a new state recalled. Transition there from pending if preempted prior to activation.

• Add a callback to ActionClient on activation

• Change documentation to lessen the presentation of the ActionClient state machine, which is a purely internal concept.

• Change completionCallback to generic transitionCallback

• Don't let preempt on a GoalHandle preempt other goals. Change GoalHandle::preemptGoal() to preempt(), taking no time arg.

• Throughout the system, change "preempt" to "cancel".

• Change ActionClient::preempt becomes cancelAllGoals and cancelGoalsBefore(time)

• Add a new state preempting. Transition from active, and to preempted, aborted, or succeeded.

• Change state machine to match what Eric drew on the board, which disregards some of the above comments about new states.

  • Same as the proposed state machine, with additional recalled state, plus cancel_requested field sent with status.

• If the server implementer attempts an invalid state transition, cause a hard failure, e.g., BREAK_ALWAYS()
• Add string "reason" to status message, to allow for more meaningful feedback for recalled / aborted messages.