laser_assembler/Reviews/2009-9-29_Doc_Review

Reviewer: Ethan Dreyfuss

Instructions for doing a doc review

See DocReviewProcess for more instructions

1. Does the documentation define the Users of your Package, i.e. for the expected usages of your Stack, which APIs will users engage with?

   1. Yes, users of articulated laser rangefinders who want to assemble multiple scans over time into a single XYZ point cloud. These users engage with the laser_scan_assembler or the point_cloud_assembler API, depending on whether or not their scans have been converted to point clouds already (the former if they have not, the latter if they have).

2. Are all of these APIs documented?
   1. Yes, the parameters, input, and output are clearly specified for both APIs.
3. Do relevant usages have associated tutorials? (you can ignore this if a Stack-level tutorial covers the relevant usage), and are the indexed in the right places?

   1. Mostly. The http://www.ros.org/wiki/laser_pipeline/Tutorials/IntroductionToWorkingWithLaserScannerData tutorial covers one possible usage. I would like to see a snippet added to explain calling the build_cloud service from your own code rather than just using pr2_laser_snapshotter.

   2. Also the http://www.ros.org/wiki/laser_assembler/Tutorials page has a confusing "no results found". Either add headers specifying that this is for laser_assembler specific tutorials or remove the search until there are laser_assembler specific tutorials.

4. If there are hardware dependencies of the Package, are these documented?
   1. Yes, the documentation explains that you need a laser scanner. (It is implicit that you need a way to transform your measurements into your desired fixed_frame, but I think this is OK since this is both obvious and may involve all sorts of different hardware).
5. Is it clear to an outside user what the roadmap is for the Package?
   1. There is none at package level, there is at stack level. Verify that this should be at stack level not package level.
6. Is it clear to an outside user what the stability is for the Package?

   1. There is no explicit mention of stability at stack or package level. I'm unclear on what this documentation would look like.

7. Are concepts introduced by the Package well illustrated?

   1. Yes, the architecture is well illustrated. A little animated gif of 3 seconds worth of laser scans and the corresponding assembled point cloud would be a nice visual for the package, but is not really required to get the concepts across.

      • [Vijay] Good idea, but definitely punting this for later. #2815

8. Is the research related to the Package referenced properly? i.e. can users easily get to relevant papers?
   1. N/A
9. Are any mathematical formulas in the Package not covered by papers properly documented?
   1. N/A

For each launch file in a Package

No launch files in laser_assembler.

Concerns / issues

There were a few minor wording / grammar type issues in the documentation that I have corrected. Make sure that the changes are to your liking: http://www.ros.org/wiki/laser_assembler?action=diff&rev2=37&rev1=32
- [Vijay] Good catches. Thanks!

The biggest change was removing: "It will not wait until it has received all scans (or clouds) in the requested interval." which seemed somewhat redundant and was confusing because there is no way to know ever when all scans have been received.
- [Vijay] Sounds reasonable. I later mention that the service call is non-blocking, which is probably enough information

Conclusion

Almost there, add a service call snippet to the tutorial, fix the "no results found" issue, check on roadmap/stability, consider creating an illustration of the assembly process, and check on my editorial changes.