API review

Proposer: Sachin Chitta/Gil Jones

Present at review:

• List reviewers

What to review

This is the ROS API for the planning_environment package. The set of services and topics exposed here will let you check trajectories for collisions, add objects into the environment, clear objects from the environment and create collision maps for collision checking. The ROS API and messages and services are documented on the wiki page for this package.

Two dependencies are also up for review along with this package:

• The geometric_shapes_msgs package that contains the messages to represent geometric shapes.

• The mapping_msgs package that contains the messages to represent 3D collision information.

Note that

• GetStateCost will be removed from this package.

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.

Matei

• AllowedCollisionEntry / AllowedCollisionMatrix

  • these seem to imply that collisions can only be enabled / disabled for robot links, and against the entire world at a time. Are these still used, or just obsolete? I am also confused by the fact that SetAllowedCollisions uses the new OrderedCollisionOperations as input, but returns an AllowedCollisionMatrix. I guess I just don't understand how these messages work...

• MakeStableCollisionMap

  • looks good. The disregard_first_message seems a bit hacky, maybe there is a workaround at the source of the scans?

• GetEnvironmentSafety

  • are we moving to a convention where a service returns false only if the call itself fails? If so, this guy should follow.

• GetObjectsInCollisionMap

  • this depends on ObjectsInMap - have we reviewed mapping_msgs?

• GetRobotState

  • message is not commented. What does the error code refer to? Does it tell you if the state that it is returning is in some sort of error? This would imply something like CheckGetRobotState

• GetStateCost

  • unlike GetStateValidity, does not contain ordered collision ops or allowable contacts. Are those irrelevant for costs (what if cost is distance from obstacles, would we not want to enable/disable obstacles?)

• SetAllowedCollisions

  • many of our current message contain OrderedCollisionOperations themselves. When should this option here be used? If this has been used, what is the default for a message that contains an OrderedCollisionOperations itself?

Wim

• AllowedCollisionEntry: it is unclear why there is an array of bools, and what the size of that array should be

• AllowedCollisionMatrix: why do you need the indices, if the AllowedCollisionEntry also contains the link name

• MakeStableCollisionMapGoal: the field disregard_first_message seems like a hack to solve a problem with the laser. such a problem should get solved in the laser code or in the code that uses the laser, not in the message api

• GetEnvironmentSafety: in general when a service returns false, this means something went wrong in the communication. It is not good to reuse the return value of the service as response data, instead you can put the service response in the service specification itself.

• GetJointTrajectoryValidity and GetRobotTrajectoryValidity: instead of using start_index and end_index, pass only the part of the trajectory to check into the message

• GetObjectsInCollisionMap: why are points treated differently than objects and attached_objects. should there also be an option to enable the inclusion of objects and attached_objects

• GetRobotState: the multi_dof_joint_state is defined by a pose (actually almost a posestamped, but the header is pulled apart), but how do you know how many dof that joint has.

Tully

• AllowedCollisionEntry

  • array of bools is undocumented
  • What does the message as a whole represent? How should it be used? Is it required?

• AllowedCollisionMatrix

  • What does the message as a whole represent? How should it be used? Is it required?
  • why are the entries not sorted in link order?

• MakeStableCollisionMapFeedback is empty??

• MakeStableCollisionMapGoal

  • what is the cloud_source?? topic frame etc
  • what is a stable map?

• MakeStableCollisionMapResult

  • empty? is there really no information wanted?

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

• Remove extra link_name from AllowedCollisionEntries and remove indexes field (DONE)

• In MakeStableCollisionMap = remove disregard first message (DONE)

• Change comment for error_code - not a bool + "this is the first thing that went wrong" (DONE)
• Remove indices from the jointtrajectoryvalidity, etc. functions (DONE)
• Change conditions to a set of bools? All constants will go away (DONE)
• Wim says GET Robot state is fine!!
• Fix getRobotTrajectoryValidity, etc. in the same manner (DONE)

• Tag SetCollisionOperations and RevertCollisionOperationsToDefault as only intended for internal testing/debugging. (DONE)