rostopic/Reviews/2009-11 Doc Review_Doc_Review
Reviewer:
- Melonee
- Tim
- Tully
To review:
Instructions for doing a doc review
See DocReviewProcess for more instructions
- Does the documentation define the Users of your Package, i.e. for the expected usages of your Stack, which APIs will users engage with?
- Are all of these APIs documented?
- 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?
- If there are hardware dependencies of the Package, are these documented?
- Is it clear to an outside user what the roadmap is for the Package?
- Is it clear to an outside user what the stability is for the Package?
- Are concepts introduced by the Package well illustrated?
- Is the research related to the Package referenced properly? i.e. can users easily get to relevant papers?
- Are any mathematical formulas in the Package not covered by papers properly documented?
For each launch file in a Package
- Is it clear how to run that launch file?
- Does the launch file start up with no errors when run correctly?
- 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
Add rostopic info as an alias to rostopic list <topic> to mirror rosnode
kwc: ticketed https://code.ros.org/trac/ros/ticket/1961
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.
kwc: added some examples to ROS/YAMLCommandLine
- 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.