API review

Proposer: Jack O'Quin

Present at review:

• Piyush Khandelwal
• Kevin Sun
• Wim Meeussen
• Stuart Glaser
• Ryan Gariepy

The goal is to reach consensus among those using various Velodyne LIDAR models about the direction this stack should take. Topics to consider include (but are not limited to):

• Repackaging the driver as a separate stack with minimal dependencies
• Adding HDL-32E support
• A common calibration file format for all models

• Migration from velodyne_common/RawScan to velodyne_msgs/VelodyneScan interface

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.

• (Jack) General ideas for repackaging (open for discussion):

  • velodyne stack containing only:

    • velodyne_msgs like today

    • velodyne_driver (minimal version of velodyne_common). Only publishes the new velodyne_msgs/VelodyneScan and velodyne_msgs/VelodynePacket message formats. No longer contains the velodyne::Data classes.

      • (Piyush) +1; what are the issues that other users may face with backward compatibility?

      • (Jack) We'll need to work out the details before we have a complete answer. At a minimum, users of the velodyne::Data classes will need significant changes. I believe most can drop access to the raw data and use a custom PointCloud2 instead. Doing that in a nodelet should be efficient. The main purpose of the raw VelodyneScan messages will be for creating bag files, that device-dependent format is at least four times smaller than an equivalent PointCloud2.

      • (Jack) Also, the new driver will exclusively use the new calibration file format. So, people will need to convert their calibration files to YAML (or whatever we decide to use). That was wrong: the driver does not require calibration, the velodyne_pointcloud conversion package will need it.

        • (Piyush) Could you elaborate this point? Is it not possible for the driver to use the calibration file directly?

        • (Jack) The calibration is needed when translating raw data into point clouds. The velodyne_driver package merely publishes raw data, so no calibration is done there.

    • velodyne_pointcloud package for converting raw velodyne_msgs/VelodynePacket data to sensor_msgs/PointCloud2. We might want to move this to a separate unary stack to avoid pulling in the entire PCL library as a dependency. I am currently assuming the dependency will shrink to pcl_common and pcl_ros in Fuerte.

    • velodyne_common deprecated, but still provided for a while

    • velodyne_pcl deprecated, moved to velodyne_experimental stack in projects branch.

    • velodyne_viz deleted

  • velodyne_utils stack containing obstacle detection and calibration packages:

    • velodyne_calibration when ready

    • velodyne_height_map

    • velodyne_ringcomp when ready

    • velodyne_image when ready (used by velodyne_calibration)

• (Jack) We need to comply with REP-0117.

• (Jack) Piyush has been working on a ROS calibration package independent of the factory settings. We had discussed saving the data in YAML format, like ROS camera calibration files.

  • Advantages:
    • Easy for humans to read or edit
    • More expressive than the older HDL-64E format, as powerful as the newer HDL-32E XML files
    • Good ROS support, familiarity
    • Independent of whatever changes Velodyne may introduce in future models
    • Can add additional fields with backwards compatibility
  • Disadvantages:
    • Requires us to write and maintain conversion scripts from the existing device formats to YAML

  • (Jack) This has now been implemented. The conversion script is gen_calibration.py.

• (Jack) Currently, velodyne_pcl creates an "unorganized" point cloud, containing one long row of points. PCL supports two-dimensional point clouds for devices like the Kinect, which produce a rectangular array of points. That seems somewhat attractive for Velodyne data, with points for 64 or 32 different lasers, depending on the model. Some algorithms might be able to take advantage of that. Unfortunately, the Velodyne does not transmit data in compact rectangles. We should investigate the advantages and disadvantages of organizing Velodyne point clouds by laser number.

• (Jack) We need to add diagnostics support to the driver, so the diagnostic aggregator can report whether the device is providing packets at the expected rate.

• (Ryan) We should also add support for the built-in IMU at some point.

• (Jack) I suspect the lack of comments so far may be due to insufficient detail about the proposed changes. I am creating experimental versions of the new packages so we can resolve those details without breaking current users.

• (Wim) I'd +1 Jack's concern about the dependency on the pcl library for the velodyne_pointcloud package. Converting this package to a unary stack seems like a reasonable solution.

  • (Jack) I wanted to mention this issue explicitly, but I don't plan to do anything about it right away. My hope is that we can just eat the overhead in Electric, waiting for Fuerte build changes intended to address the problem by enabling dependencies on a minimal pcl_common library.

    • (Jack) UPDATE: Fuerte does not provide a minimal pcl_common dependency, that is now planned for Groovy Galapagos. I intend to try very hard to make it happen in that release. I am still reluctant to split the velodyne_pointcloud package off into a separate stack. Without that package, the driver is virtually useless by itself. If anyone has a use case for installing the driver on a limited-capacity outboard controller, we can revisit this decision. Right now, I don't think it likely.

      • (Ryan) +1 on this

• (Wim) Our own YAML format for calibration will allow us to push our own calibration beyond what is supported in the factory format, and potentially calibrate more/different parameters of the scanner.

  • (Piyush) I am in the middle of revamping the calibration code. The calibrator should be able to determine the parameters below.

    • Roll and Pitch - The Velodyne may not be exactly level on the robot. By doing plane fitting in a large open area, the roll and pitch can be estimated. These values can then be used to fix the tf tree (by hand atm), or fix the point cloud being generated to match the tf tree in velodyne_pointcloud.

    • Intensity Correction - In our Velodyne, all lasers return different intensity values when they hit the same object. I currently fit a degree 3 polynomial to match all of them to a given reference laser.
    • Laser Angle Correction - Estimate the deviations in angles from the specifications for each laser. Plane fitting can be used to fix this as well.

  • (Piyush) Please let me know if you guys have other parameters in mind as well.

• (Stu) It would be great if the fields of struct correction_angles were described in more detail.

• (Stu) I expected to see public functions like packet2scans and scan2xyz. The code structure of velodyne_common expects me to have a file containing velodyne scans, but doesn't help me out if I just have a velodyne packet in memory. Exposing functions that operate on the raw data would give users more flexibility in how to process and store velodyne data.

  • (Piyush) +1. If the Data classes are being done away with, a scan2xyz function could be in a library inside velodyne_pointcloud that converts a velodyne_msgs/VelodyneScan to a pcl::PointCloud<PointT>. Nodes/Nodelets in velodyne_pointcloud could be a wrapper around this function.

• (Stu) In velodyne_common, all the data_*.h files seem to contain very similar code, but in slightly different namespaces.

  • (Jack) Various velodyne_common interfaces have been evolving gradually. To eliminate this confusion, I intend to deprecate that stack, and no longer support the data_*.h interfaces. That assumes we can now do what we need using something like the custom velodyne_pcl::PointXYZIR point type (with X, Y, Z, intensity and laser_ring). I also plan to provide the standard pcl::PointXYZI, which is all most consumers of the data require.

    • Update: velodyne_common is now deprecated and has been moved out of SVN trunk and into the projects branch.

  • (Jack) To get a clearer view of the much simplified raw data interfaces, see the velodyne_rawdata::RawData class API in the velodyne_pointcloud package. The doc indexer is a few days behind right now, but that version is fairly close. It should be up to date some day soon.

• (Jack) Putting every topic in the "/velodyne" namespace was a mistake. This was one of the first ROS stacks I created (two or three years ago), and I did not understand ROS topic naming very well. Changing topic names is disruptive to existing users, but now is the right time to do it. Those who wish can still run their nodes in the "/velodyne" namespace.

  • (Jack) The prototype implementations of velodyne_driver and velodyne_pointcloud use the following topic names:

    • "/velodyne/packets" -> "/velodyne_packets"

    • "/velodyne/pointcloud2" -> "/velodyne_points"

Meeting agenda

The meeting was carried out via extensive e-mail discussions. The API has been approved.

Thanks to all who participated.

Conclusions

• Remove old messages and interfaces.

• HDL-32E device works.

• Calibration packages need to be converted to latest interfaces.

• YAML calibration file handles all supported devices.

• Keep PCL dependency for now, can repackage later, if needed.

Documentation issues

• Add "How-to" Tutorial to wiki documentation.

• Document YAML calibration file contents.

Future work

• Publish velodyne_msgs/VelodyneInfo with calibration information.

• Provide a driver for the GPS and IMU packets.