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

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

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.

  • {X} 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)


Wiki: planning_environment_msgs/Reviews/2010-02-22_API_Review (last edited 2010-03-06 00:13:26 by SachinChitta)