API review

Proposer: Sachin

Present at review:

  • Brian
  • Eitan
  • Stu
  • Wim
  • Tully

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.

Meeting agenda

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


  • Subscribe to <name>/command, in addition to cmd_vel, passing inputs to the same callback. In the long term, we'll remove the subscription to cmd_vel.

  • Change a subscription to "/cmd_vel", if any, to "cmd_vel", using a NodeHandle with no namespace.

  • Combine max_vel/vx and max_vel/vy to max_translational_velocity. Same for accel. Change rotational limits to use the word "rotational"
  • Changes to BaseControllerStage.msg:

    • Provide joint_*_measured, joint_*_commanded, and joint_*_error for velocities and effort.
    • Convert command_vx, command_vy, and command_vw to a Twist
    • Remove joint_stall and joint_speed_filtered
  • Remove stall logic from the controller, along with all associated params. Let people compute stall state on the outside, using data from BaseControllerState

  • Remove kp_wheel_steer from documentation. There's some doubt as to it being a well-defined parameter, and it might be removed from the code.
  • Lower the default on the watchdog timeout parameter to something like what we're already setting in our launch files, in the 0.2-0.5 second range.
  • Change state_publish_time to state_publish_rate (or state_publish_frequency, if that's what used in other controllers), and interpret it as a rate instead of a time.
  • Check and fix types in parameters in docs; most of them say "string," when that's wrong.
  • The wheel radius is needed by both the base controller and the base odometry. We will encode the wheel radius in the urdf in a better way than using a collision element. e.g. by adding an extra link
  • Keep different gain for all individual wheels
  • Fix yaml file for base controller to remove duplication of gains (Sachin, ticket wim)
  • In the long term we want ability to remap topics of controllers
  • Too hard to get rid of base_footprint, so we keep it. It does not belong in the base odometry, so move it into the urdf. Offset currently is 5.1 cm (Sachin, talk to John)
  • In documentation fix types of parameters, a lot of them are copy-pasted wrongly
  • expected_publish_time: rename to odom_publish_rate
  • Make covariance parameters consistent with imu node. Discuss with Blaise on what the right way would be, and change imu and/or odom

Wiki: pr2_mechanism_controllers/Reviews/2009-18-12_API_Review (last edited 2010-01-21 03:06:26 by SachinChitta)