camera_calibration/Reviews/2010-01-12_mihelich_Doc_Review

Reviewer: Patrick

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?

   • From the main page it looks like calibrationnode's ROS API is the only thing users should be concerned about. Then the C++ code API page says the package has no code API, but the Python code API page lists a couple of classes with no disclaimer.

2. Are all of these APIs documented?
   • Yes.
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?
   • There are tutorials for both mono and stereo calibration. Please put direct links to them on the main page.
4. If there are hardware dependencies of the Package, are these documented?

   • A "Supported hardware" section would be nice, basically saying you require the standard ROS camera interface (image_raw & camera_info topics, set_camera_info service) and pointing to camera_drivers for examples.

5. Is it clear to an outside user what the roadmap is for the Package?
   • No.
6. Is it clear to an outside user what the stability is for the Package?
   • No.
7. Are concepts introduced by the Package well illustrated?
   • N/A
8. Is the research related to the Package referenced properly? i.e. can users easily get to relevant papers?
   • The main page should link to the OpenCV calibration page. OpenCV is linked in a couple other random places (stereo tutorial, Python docs).
9. Are any mathematical formulas in the Package not covered by papers properly documented?
   • N/A

For each launch file in a Package

1. Is it clear how to run that launch file?
2. Does the launch file start up with no errors when run correctly?
3. Do the Nodes in that launch file correctly use ROS_ERROR/ROS_WARN/ROS_INFO logging levels?

Concerns / issues

• The board dimensions being hard-coded is a big problem. rows/cols/square size need to be configurable.

• The main page calls the node calibrationnode when it's actually cal.py. These need to match.

• The node docs need to be more explicit about what the user needs to remap. Example usage for the mono and stereo cases would work.
• The C++ code API page ideally should disappear entirely; there's no C++ API and the ROS API shouldn't be repeated there. I've just removed all the old C++ stuff and messages on trunk.
• Fixed "Services" to "Services Called".
• Tutorials
  • Mono tutorial does not remap "camera", is that needed?

  • They don't say what to do when the camera has bayer output, which cal.py chokes on (need better error reporting there). Should give an example image_proc or stereo_image_proc command and point to those packages.

  • Are the screenshots all up to date? The buttons are different from mono to stereo.
  • Need to explain how to upload the calibration to the camera (even if it's just a button push) and that this overwrites what was there before.
  • Either take out the dumps of calibration parameters at the end or say that cal.py outputs the calibration so you can maintain an off-camera copy.
• API
  • It would have been clearer to the user to split cal.py into separate mono_cal.py and stereo_cal.py nodes.

  • If you must keep it one node, use groups to separate the mono/stereo topics and services, see for example how the stereo_image_proc docs group topics.

  • The remappings for stereo calibration seem really awkward; you have to repeat yourself a lot. Maybe mimic stereo_view, e.g.: cal.py stereo:=wide_stereo image:=image_mono.

• I was unable to run cal.py against the prosilica camera on my desk, with image_proc running:

  $ rosrun camera_calibration cal.py image:=/prosilica/image_mono
  OpenCV Error: Incorrect size of input array () in cvGetSubRect, file /u/mihelich/ros/vision_opencv/opencv2/build/opencv-svn/src/cxcore/cxarray.cpp, line 1251
  Exception in thread Thread-5:
  Traceback (most recent call last):
    File "/usr/lib/python2.5/threading.py", line 486, in __bootstrap_inner
      self.run()
    File "/u/mihelich/ros/image_pipeline/camera_calibration/nodes/cal.py", line 59, in run
      self.function(m)
    File "/u/mihelich/ros/image_pipeline/camera_calibration/nodes/cal.py", line 177, in handle_monocular
      self.redraw_monocular(scrib, rgb)
    File "/u/mihelich/ros/image_pipeline/camera_calibration/nodes/cal.py", line 395, in redraw_monocular
      self.buttons(display)
    File "/u/mihelich/ros/image_pipeline/camera_calibration/nodes/cal.py", line 380, in buttons
      self.button(cv.GetSubRect(display, (x,180,100,100)), "CALIBRATE", self.goodenough)
  error

Conclusion

The GUI is quite nice but both it and the docs need some more polish before we can tell people "use this for your calibration needs."