API review

Proposer: Wim Meeussen

Present at review:

  • List reviewers

Question / concerns / comments

Sachin

  • In general, it seems like we should converge on one model for everything - right now it feels like a mix and match of KDL elements and our own
  • E.g. model_->robot_model_ where the pointer to a kdl tree is stored inside the ROBOT class as robot_model_. Shouldn't model_ just have all the information we require?

  • joint limits come from the joints inside the model whereas all other joint information comes from the KDL tree. It would be good to have one point - Joint - where we could get everything from.
  • It should be easier to find "passive" joints in the model.
  • Currently, there is no way to figure out if a joint is not actuated but still has an encoder on it
  • The list of joints should not contain fixed joints - fixed joints can be accessible in a different way
  • It should be easier to initialize Robot from URDF instead of from the robot model
  • An accessor method to get a chain based on a "group" name ("right_arm") would be great. Otherwise, you have to remember both the root and tip names every time you need a chain.
  • Are we still using these? If not, they should be taken out
  • void    propagateEffort ()
    void    propagateEffortBackwards ()
    void    propagateState ()
    void    propagateStateBackwards ()
  • It might also be worthwhile to think about representing some of the elements here using messages instead of internal classes to make them easier to pass around, e.g. use the manipulation_msgs/Limits.msg for joint limit specification.

Wim

urdf::Model

pr2_mechanism_model::Robot

KDL::Tree

Limits

Limits

-

Safety

Safety

-

Calibration

Calibration

-

-

Transmissions

-

-

Actuator

-

Type

Type

Type

Axis

-

Axis

Intertia

-

Inertia

Damping, friction

-

Damping, stiffness, inertia, scale

Visual

-

-

Collision

-

-

Meeting agenda

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

Minutes

  • Two proposals:
    • Simplify Robot Model and have URDF on the side
    • Extend Robot Model to include URDF
  • We can remove Type from robot_model.
  • KDL tree duplicated in URDF remove from interface. (It can be duplicated from URDF in one line.)
  • Most controllers (currently only the calibration controllers) do not need actuators and calibration info. Thus don't need pr2_mechanism_model.
  • PR2RobotState
    • Joint State
    • Actuator State
    • PR2RobotModel
  • In the future, consider removing abstraction of PR2RobotModel to keep things from getting too nested.
  • Access to joint state and model should be convenient but not necessarily efficient, (aka can be looked up at init, but should be cached to be used in realtime)
  • Encoders only, can have zero effort limits, and complain if effort is set on this actuator.
  • No good solution for unmeasured and unactuated joints.
  • Remove fixed joints from JointState.

    • Sachin come back to see if iterating a chain with fixed joints still an issue.
  • Do grouping outside URDF on param server. It should be discussed for new controllers design.
  • document propagate* methods as internal methods. (Rename to avoid ambiguity with chain propagation, like propagateActuatorStatesToJoint)

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_model/Reviews/2009-10-21_API_Review (last edited 2009-10-26 16:23:50 by wim)