API review

Proposer: Wim Meeussen

Present at review:

  • List reviewers

Question / concerns / comments

Gunter

  • SwitchController Service: As mentioned in the controller interface, I would strongly argue that starting/stopping controllers should be a guaranteed behavior (no fail possible), which completely eliminates the need for strictness. As is, there is still the question of whether on failure some controllers have been stopped? Will they be restarted? So you can be left with no controllers running? This entire error handling process (which needs to occur in realtime..) is tricky and inconsistent with "realtime". So I think it's both better and easier to avoid entirely.

  • SwitchControllerResult: Why is it empty? As well as Feedback? I guess that really brings up the bigger question: What is the action and why is there an action? I thought actions were intended for things that take a finite amount of time, i.e. that are interruptible, etc. Switching controllers happens instantaneously. Either yes or no. So a service seems very appropriate. I guess I don't understand the need for an action (admittedly neither the details).

  • BufferedData: What is this? I don't understand...

  • The only messages that I understand are Joint/Actuator/Controller State....
  • ControllerState: How are times defined? Why float64? What is their unit? And over what time frame are mean/variance calculated?

  • ActuatorState: Why is time a float64? Why include encoder_velocity? I don't understand the calibration_reading and other calibration data? requested and commanded current/effort - naming doesn't make it very clear which is which (or what their difference is).

  • JointState: What is the publishing cycle for the min/max values? It wasn't immediately clear to me that these are the "statistics" on the actual values versus the imposed limits.

  • Consistent with ActuatorState (and other discussions) should JointState and ControllerState have a time field?

Meeting agenda

These comments were discussed as part of the pr2 controller manager api review

Conclusion

Package status change mark change manifest)

  • /!\ Action items that need to be taken.

  • {X} Major issues that need to be resolved


Wiki: pr2_mechanism_msgs/Reviews/2009-10-06_API_Review (last edited 2009-10-29 18:08:28 by wim)