rostopic/Reviews/2009-11 Doc Review_Doc_Review

Reviewer:

  • Melonee
  • Tim
  • Tully

To review:

http://www.ros.org/wiki/rostopic

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?
  2. Are all of these APIs documented?
  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?
  4. If there are hardware dependencies of the Package, are these documented?
  5. Is it clear to an outside user what the roadmap is for the Package?
  6. Is it clear to an outside user what the stability is for the Package?
  7. Are concepts introduced by the Package well illustrated?
  8. Is the research related to the Package referenced properly? i.e. can users easily get to relevant papers?
  9. Are any mathematical formulas in the Package not covered by papers properly documented?

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

Melonee

  • It's not immediately clear that the window_size is the number of samples. My first thought was the window_size was a time period to same over i.e. 5 seconds.
    • kwc: done
  • more examples of using filter might be helpful, I had a hard time parsing the tf topic based on the example, but eventually got it to work.
    • kwc: Do you have an example of a useful example?
  • you should mention the escape needed for negative numbers with rostopic pub

    • kwc: I streamlined the rostopic pub documentation so that the "YAML Command Line" documentation is more prominent. This is discussed there.
  • I went through the documentation option by option and tried them, the documentation was a good reference.

Stu

Tim

  • The package summary states "This library is for internal-use only as its API is not stable" yet the roadmap says "rostopic is a stable command-line tool."
    • kwc: I updated the text to try and clarify that the roadmap refers to the command-line API. The internal code API is not considered support/stable.
  • Consistency of argument order: all but one example are of the form "rostopic COMMAND [OPTION...] TOPIC", except for --offset (where the option appears after the topic)
    • kwc: fixed
  • Typo: the package summary begins "rostopic is contains"
    • kwc: fixed
  • It's good that the YAML format on the command line has its own page, but the "rostopic pub" subsection might still benefit from an example more complex than a single string
    • kwc: this is a hard one. I updated the example to be a more generic example with a integer and float. The original text had more examples, but then it was clear people then weren't going to the YAML page. Basically, the more examples I put on the page, the less likely people are to read the other page, which is really necessary.

Tully

  • "header: auto" is very bad, how does it fill in frame_id?
    • kwc: I documented that it fills in an empty frame_id. This is useful for the small subset of messages that have Headers but don't use the frame_id, e.g. roslib/Log.
  • I cannot figure out how to fill a header.stamp in a PointStamped from the documentation.

  • document -p and -c are mutually exclusive
    • kwc: fixed
  • the line "rostopic type /topic_name | rosmsg" doesn't work. rosmsg needs an argument
    • kwc: fixed

Conclusion

Implemented rostopic info, addressed concerns above.

Wiki: rostopic/Reviews/2009-11 Doc Review_Doc_Review (last edited 2009-11-05 22:51:59 by KenConley)