API review

Proposer: Conor

Present at review:

• List reviewers

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.

Wim:

• The action interface should not implement a build-in polling method to block the execute method. Any variations on how to terminate the execute method should be implemented by the action implementation, not by the action interface.

  • Remove the ResultStatus waitForDeactivation(Feedback& feedback) method

  • Remove the void deactivate(const ResultStatus& result_status, const Feedback& feedback) method

• Why is bool isActive() not public?

• The void update(const Feedback& feedback) method is only used for debugging. Do we want that in the action interface?

• The void terminate() method permanently disables an action. It is better to destruct an action instead of making it sitting there forever. How should we do this? Do we still need a void terminate() method?

Bhaskara

• The status "undefined" sounds like an error situation. Something like "initial" might be clearer.
• What does the API specify about various race conditions, e.g., two activate messages from different clients received out of order. Or a preempt message received before its corresponding activate message.
• Is it possible to have goals uniquely identified? E.g. by timestamp and sender name? This would make it easier for an action client to determine in a generic way, e.g., that the succeeded status I just received refers to the goal I just sent, as opposed to a previous goal, or a goal that somebody else sent.
• What does isPreemptRequested mean? That a preempt was received in the last N seconds? Or since the last call to isPreemptRequested? This should be specified in the doxygen.
• Requiring the action to explicitly call update at a given rate makes certain things awkward, e.g. if the action internally calls some long blocking operation. Could this happen in a separate thread, such that the action writer just has to update the feedback member variable as and when changes happen?
• I can't tell what exactly waitForDeactivation does from the documentation.

Meeting agenda

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

Conclusion

can we find a way to enable passing a new goal w/o shutting down

• executive needs prempt feedback
• move base currently shuts down and restarts before controller times out is there a way to atomically get preemt/new goal?

would you want an unthreaded version of action base

• allow user to thread things
• this would make simple things harder for you'd need to spin your own thread

what happens if it is blocked with a bunch of new goals queued up

• might prempt multiply and then run goal
• or multiple goals then preempt might preempt the first one
• preempt should be in the same channel to keep sequencing
  • a little awkward to have goal inside preempt message
• alternative is to give it a topic/side channel
• it's improper to queue up many commands before you hear a preempt -- how to enforce this policy?
  • order on timestamps, ignore older commands(needs timestamps on everything)
• need way to atomically preempt knowing that you have a new goal
• proposal one composite message to get rid of ordering problem
  • command + status + goal + feedback --- semantics are encoded into topic name only fill out the valid fields
  • this will prevent the worry of out of date commands
  • generally agreed to do this
• all communications need to be reliable
• pull waitForDeactivate and dactiveate out into helper library which constructs with reference/pointer to action
• make isActive public?
• add diagnostics into action base to make easier debugging
• execute w/o terminate at destruction causes segfaults (action runner does it correctly but if an external user uses it they can shoot themselves in the foot)
  • Add a warning in destructor if terminate has not been called
• Change undefined to uninitialized
• add goal id into command
  • for now write in clients
  • type is uint64
  • add guid generator in ROS proper

• Action items that need to be taken.

• Major issues that need to be resolved

---