API review

Proposer: Joe Romano

This API review is to cover the action servers present in the pr2_gripper_sensor_action package. This package relies heavily on the accompanying support packages pr2_gripper_sensor_msgs and pr2_gripper_sensor_controller. All of these packages have been documented. You can browse them at your convenience, and please signal anything that looks out of place, whether in an API, package structure, or documentation. Please put all comments on this review page.

The main content we will be reviewing (the public API) is located at the pr2_gripper_sensor_action wiki page.

Present at review:

• Joe Romano
• Kaijen
• Matei
• Sachin
• Gill
• Peter Brook
• Gunter Niemeyer

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.

Sachin

• gripper_action looks fine
• force_servo
  • goal - max_joint_effort - I am assuming this is the maximum commanded effort after the mechanical reduction (maybe make this clear in the documentation)
  • result - all constants should be encoded in the message itself
• find_contact
  • goal - again, put constants in the message
  • result - ditto, also why is controller state an int16
    • put in typical values for joint position for open and closed positions of the gripper
• slip_servo
  • goal - put in typical numbers in the comments for the PR2 grippers
• event_detector
  • goal - all constants should go into the message itself
    • put in typical numbers in the comments
    • if the magnitudes don't change with every request, they should be ROS parameters
  • result - does the timestamp in the header correspond to the timestamp of the event?
    • what does the frame_id in the result header indicate

  • we need to figure out how much of this can be made more generic or if it is worthwhile to have a simple binary message so its not PR2 dependent

• grab
  • hardness_gain - can this be a ROS param instead of having to be in the message?
  • deformation_limit - ditto?
  • in the result and feedback why is realtime_controller_state a float64?

Matei

• great job documenting stuff
• gripper_action
  • looks good
• force_servo
  • why is there a header in the result message?
  • where are constants defined? They usually go into the message body
  • in the documentation, maybe say "Description: action node to provide control of the gripper joint based on the contact force reported by the tactile sensors"
• find_contact
  • what does it mean to "zero out the force"?
  • constants definition should go in the message
• slip_servo
  • "a force servoing algorithm similar to that above" - which one?
  • what does it mean that the object "recently slipped"?
  • how does the controller figure out if the gripper is empty or not?
  • constants definition should go in the message
• event_detector
  • constants definition should go in the message
  • define rigorously in the documentation all "events", such as "slip signal" and "acceleration signal"

Meeting agenda

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

Conclusion

Package status change mark change manifest)

• * remove all references to motor effort in documentation

  * always refer to virtual joint and distance between fingerpads

  * define all states within message with predefined values

  * put in comments

  * create 'release' command to compliment grab and use the low-level action.

  * make a 'beginner' and 'intermdiate' section in wiki

  * think about some code to handle when the sensors are not working. either report broken or handle this condition. report error state back to user for broken sensors.

  * slip servo deformation limit- add in comments about what happens when it is not filled in, and also what a good value is.

  * for all message variables indicate what happens when not filled in.

• add in some code to deal with joint effort indicating that contact is being made.

  * check headers to make sure time is correct. put a stamp in place of header.

  * push hardness_gain onto param server

• add in param server values that let you edit the various states that grab will report failure (cancelled) on.

  * make grab result not a float64.

  * be more clear on description of 'force servo'

  * for find_contact zero stuff be more descriptive in message and wiki about why you would use is and what happens.

  * slip servo wiki description - be more specifc about force controller.

  * add into wiki hard definitions about return states for slip, acceleration, etc. Add in DEFINITIONS section to wiki page.

---