message_filters/Reviews/2010-01-20 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?
   • Yes.
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?
   • No tutorials, but the code examples on the wiki page cover normal usage.
4. If there are hardware dependencies of the Package, are these documented?
   • N/A
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?
   • Not stated explicitly anywhere. Since it's in ROS core with two language implementations, can probably assume stability.
7. Are concepts introduced by the Package well illustrated?
   • Yes.
8. Is the research related to the Package referenced properly? i.e. can users easily get to relevant papers?
   • N/A
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

• Wiki

  • Made some minor formatting edits (http://www.ros.org/wiki/message_filters?action=diff&rev1=24&rev2=25).

  • Edited the TimeSynchronizer example slightly and added a note about image_transport::CameraSubscriber (http://www.ros.org/wiki/message_filters?action=diff&rev1=25&rev2=26). The example is good, I just don't want vision people stumbling across it and blindly copy-pasting instead of using image_transport.

  • Next to the C++ class API links you could also link to the Python versions.
• C++ code API

  • Would be nice if member functions for Cache had doxygen briefs.

    • Josh: FIXED

  • TimeSynchronizer description refers to sensor_msg::CamInfo instead of sensor_msgs::CameraInfo

    • Josh: FIXED

  • Auto-generated docs for TimeSynchronizer are a mess but I don't really know what to do about it. Probably it's OK as long as people are mainly looking at the example code.

    • Josh: Not sure what to do about this either. I think I'm going to leave as-is for now, and hope to clean it up in the future through a custom doxygen config.

  • TimeSequencer::add() is not documented. Is there a reason for that to be public?

    • Josh: FIXED. Documented it. It's public because we found in the original MessageNotifier that it's sometimes useful to be able to manually add messages.

  • Somehow it wasn't obvious to me from the various documentation that you can register multiple callbacks with registerCallback(). Would be nice to state that explicitly.

    • Josh: FIXED. I added some new information to the pattern section of the wiki

  • It's a little confusing that registerCallback() isn't listed in filters that derive from SimpleFilter. Maybe mention on the doxy main page that all filters provide registerCallback(), for simpler filters this is provided by the SimpleFilter base class.

    • Josh: FIXED
• Python code API

  • message_filters.TimeSequencer is listed as implemented but not documented, and there's no Python code example on the main page. What's the deal there?

Conclusion

Looks good, I only saw minor issues.