API review

Proposer: Wim Meeussen

Present at review:

  • List reviewers

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.


  • Does AccelerometerState have enough information to figure out what the accelerometer data means? How do you know the timing of the samples?

    • We don't have timing data from the accelerometer. We just know that the 2-4 samples arrived during our realtime loop interval.

  • ActuatorCommand has both effort and current. Can you actually command both of these? How does it tell which one you're commanding, and which is junk?

    • current has been removed

  • The Projector API is completely impossible for me to decipher. That's fine if there's good documentation on what each field means.
    • The API basically exposes the hardware registers. This was sufficient for Blaise to write the driver


I made some changes to the doc in r26394 and r26395. Now the documentation of the actuator interface and the actuator statistics message are compatible.


  • Actuator
    • Can we give a better indication of the sign of zero_offset_? Is it actual_reading - calibrated_position or the other way around?
      • The reported position is actual_reading - zero_offset, I've updated the doxygen

    • Change the description to "take the zero offset into account when *reporting* the position values"
      • see above

  • Accelerometer
    • The state has a vector of samples. What's the process for figuring out exactly when a particular sample was taken? If it's difficult, maybe the state should have a vector of timestamps as well.
      • We don't get the timestamps from the hardware. We just know that the samples are coming at a regular interval (3KHz), so you should receive ~3 samples per realtime-loop update


  • For the top class, why add functions? Is this just a "general-purpose" repository where start up functions add what they detect and the user can then find it? Then why distinguish the five types? Why not define one device that has a command, status, and name? Yes - this is my non-programmer background...
  • What is current_time_? Is this the time the packet was received? I'm assuming all data is set simultaneously at that time?
    • The current_time_ is the system time just prior to the commands being sent to the EtherCAT hardware. I've updated doxygen

  • For actuators:
    • Why lump actuator and encoder? Ultimately, if we want to be general, it would be really nice to have just encoders or just motors.
    • Are the last_commanded, last_executed, last_measure current/effort directly related by torque constant? I assume so. Are the safety limits both the joint safety limits and the motor current limits? I would guess these are only motor current limits. If so, how are they set?
      • There are joint safety limits and motor current limits. The joint safety limits come from the URDF. The motor current limits are programmed into the MCBs during manufacturing.

    • Is there support to temporarily exceed the max. continuous torque?
      • No

    • Can you elaborate on motor voltage? Is this back-EMF during the off duty cycle? Or average voltage?
    • For the offset, why is a write-only in state? Shouldn't this be in command? What is the calibrated position?
      • This is a valid point. I probably would move zero_offset into command, but I think we will hopefully get rid of it when we start using the new RAM on the MCBs

  • For accelerometer:
    • How does the accelerometer know the frame_id?
      • It is a new frame created just for this purpose

    • What units are the samples in (g's? I'm hoping m/s^2? Or Volts?)
      • It is in g's. I've updated doxygen

        • Eric, just checked in a change to make it m/s/s

    • Yes, it would be helpful to see a little more info on the 3KHz vs. 1kHz issue.
  • For digital I/O:
    • why 8 bits?
      • Digital I/Os were a single bit WG005 and WG006, but Blaise wanted a digital I/O interface to the projector, in addition to the normal projector interface. This would have resulted in an explosion of digital I/O's. The compromise was to create multi-bit digital I/O's. Only the lsb is valid for non-projector digital I/O's

  • For pressure sensor:
    • What are the readings? Units, range?
      • Good question. They are 16-bit values of unknown unit units

  • For projector:
    • I don't know what A, B, I, L0, etc mean? Guess I don't understand the projector.
      • This is on a need to know basis :) It is not particularly well documented, but there is a schematic on the pr2hardware wiki that somewhat explains it.

Meeting agenda

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


Package status change mark change manifest)

  • /!\ Action items that need to be taken.

  • {X} Major issues that need to be resolved

Wiki: pr2_hardware_interface/Reviews/2009-10-14_API_Review (last edited 2010-01-20 16:09:45 by RobWheeler)