API review

Proposer: Blaise Gassend

Present at review:

• List reviewers

This review is for the ROS API only.

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.

Gunter

• What is the 3x3 orientation co-variance? Why 3x3?
  • BG: It is in rotation around the x, y and z axis. Which x, y and z axis is currently unspecified, and there is a broader debate going on on this topic for the various pose messages. This is not currently being used in any detailed way by the node, and is not really part of this package, so I am not going to improve for now. Once things settle in geometry_messages, I'll make any necessary changes, but they aren't really going to change the API of the node.
• Is is_calibrated a service or topic?
  • BG: A latched topic

• Can't find any doc for DiagnosticMessage - is that DiagnosticArray

  • BG: It is. I have corrected the documentation.
• How is timing handled? Does the user have any control of this?
  • BG: Not currently. Given that the communication protocol's streaming does not allow you to set the update rate, and given that the amount of data (and computation overhead) is small, I think that streaming in the data at the IMU's maximum data rate is the right thing to do.
• This seems pretty basic. I admittedly have edited/replaced imu_node because I wanted more access to the hardware (different outputs/frequency). Is there any point in making imu_node more flexible?
  • BG: I don't want to add features for M3, but am willing to make changes now so that we can add features later without breaking compatibility. Let's talk about what features you would like to see added.

• Also, silly and maybe wrong place, but why are imu_node and 3dmgx2_driver 2 separate packages? I guess I appreciate 3dmgx2 having a full-featured (though is was missing the key hardware interface options I wanted library and then imu_node providing a striped-down interface to ROS. But imu_node doesn't really look like it's designed to (eventually) handle multiple devices?

  • BG: Originally, I believe it was for licensing issues. I am currently merging them into a single driver: microstrain_3dmgx2_imu

Wim

• Is the diagnostics (robot_msgs/DiagnosticMessage) actually a diagnostic_msgs/DiagnosticStatus?
  • No, the documentation was wrong. It is a diagnostic_msgs/DiagnosticArray.
• When does the calibrate service return? When the imu is calibrated, or when the calibration command is received? How do you know when the calibration is finished (meaning, how long do you have to listen to the is_calibrated topic to know if calibration failed or not)?
  • BG: Good questions. Let's discuss what the right option is.
• Could the calibrate service return success or failure?
  • BG: After discussion with Wim, I am going to make it so that the calibrate service only returns once the IMU is calibrated (this update is awaiting an upcoming release of diagnostics). At that time one could check whether it succeeded with the brand new wait_for_message (or whatever it's called) call. I am doing things this way to avoid having to create a message, and get it pushed down to where it can also be seen by Wiimote (also satisfies the IMU API).
• Could the self test service return success or failure?

  • BG: It does diagnostic_msgs/SelfTest

• The orientation covariance needs to be documented. This is a covariance on which representation? I assume rpy around fixed axis. See the /geometry_msgs/PoseWithCovariance as an example.
  • BG: It is actually x,y,z (though indistinguishable from r,p,y for small rotations). See response to Gunter above.
• Why is "is_calibrated" published in the ~ namespace, and "imu_data" not?
  • BG: Good question (recall the discussion we had on this question). I have moved "is_calibrated" to "imu_data/is_calibrated" and "calibrate" to "imu_data/calibrate" because they are services that all IMUs will need. I am leaving "add_offset" in the "~" namespace because it is specific to this driver.

Blaise

• Standardized topics/services should be in the imu_data namespace.
  • BG: Done.
• Should the node name stay imu_node?

Jeremy

• is_calibrated should be specified as being latched.

  • BG: Done
• Does the imu node support (or need to support) dynamic reconfigure?
  • BG: Probably should at some point. But not a pressing need because there is nothing here that really needs dynamic reconfiguring except the time offset, and it is already being handled in a way that works for now.

• imu_node is claiming a huge (and not met) space of possible devices. I would put both the node and library in a single package called 3dmgx2_imu or microstrain_imu since the driver can probably be extended to other microstrain devices in the future without too much trouble.

  • BG: In the process of doing that.

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