API review

Proposer: Kevin

Present at review:

• Kevin
• Eric
• Wim
• John
• Stu

Overview

I'd like to integrate a new system to the PR2 to help verify the transmission status of each joint. We've been using a similar system in the burn in room, and it has been remarkably effective at catching broken belts and miscalibrated joints.

For Diamondback, I've extended the pr2_mechanism_diagnostics stack to check the transmissions of each joint using the mechanism_statistics topic and the URDF. I've tested this system using the regression tests that we used for the burn in system, and I've been using it on physical hardware for the past few weeks.

I've documented the ROS API on the package wiki page (no code API supported).

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.

Eric

• Are you proposing changing the names from mechanism_diagnostics to mechanism_diagnostics_node? Why?
• The API uses "trans" in some places and "transmission" in others. We should standardize on one, and trans is not clear to non-experts, so I would change the parameter and topic names to all be "transmission"
• Naming - I propose the reset be called "reset_transmission_status", since the publishing topic is called transmission_status
• Naming - I propose the enable/disable parameter be called "check_transmission_status"
• Is it required to call the reset function to keep using the robot, or will it work until the next failure, but continue to publish False on the transmission topic?
• Where do the tolerances come from? I remember we had issues with this, especially on the wrist. Is there some aspect of this that should be exposed as a parameter?
• Probably just a feature for the roadmap, but would doing this checking within the transmission be appropriate? I'm asking partially because this only supports some transmission types, and each new transmission type will require modifying this code to support it.

Wim

• The ~transmission_status topic is a summary of all transmissions together. It might be useful to show the status of each transmission in a machine readable format (not diagnostics). The summary could still be part of the message.
• What are the tolerances for the checks? Are they configurable? How did we decide on the numbers?
• Do you need to call reset_trans_check before you can reset the motors?
• What does init_trans_check do? I assumes is enables/disables the checks of transmissions. If so, can we skip the 'init' prefix, it seems so have nothing to do with initialization.
• pr2_mechanism_diagnostics seems not appropriate as a name any more, since the node now 'monitors' the robot, and takes action when something goes wrong. This is much more than just producing diagnostics.

Stu

• Ditto Wim on "init_trans_check". I have no idea what that means.

• reset_trans_check is a PowerBoardCommand2? Looking at that service, I have no idea how to fill it in to reset the transmissions.

• How difficult is it for a user to get back to a working state when the transmission check fails? Do they have to restart the robot?

John

• Be more verbose about transmission_status? E.g. define constants for success, error, etc. Status of 1 or 0 seems a little bit ambiguous. Or rename the topic to transmission_success.

• I second renaming *trans_* to *transmission_*.

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