control_toolbox/Reviews/2010-01-12_Doc_Review
Reviewers: Wim, John
I've generated the most up-to-date doxygen locally (pending release):
Instructions for doing a doc review
See DocReviewProcess for more instructions
Wim
- PID
Init from a RosNode is not documented
- Providing the derivative of the error as an optional argument is not documented. It would be good to document the purpose of this extra argument: it allows you to override the computation of the derivative.
- ramp
- the doxygen comments seem to be copied from the sinesweep, and do not match the ramop.
- dither
- it seems strange that the dither can only be initialized from a ros node, while all other tools can also(/only) take their arguments in the init method.
- pid gain setter
- the ros api of the gain setter is not documented. When multiple pid's are added, how do you distinguish between them in the ros api?
Described now.
- the ros api of the gain setter is not documented. When multiple pid's are added, how do you distinguish between them in the ros api?
- 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
John (file:///wg/stor1a/sglaser/tmp/temprosdoc/control_toolbox/html/index.html)
file:///wg/stor1a/sglaser/tmp/temprosdoc/control_toolbox/html/index.html missing descriptions for Ramp Output and Sine Sweep.
should mention how i_clamp truncates i_error_ or i_term_ in file:///wg/stor1a/sglaser/tmp/temprosdoc/control_toolbox/html/classcontrol__toolbox_1_1Pid.html.
- Make existing text-based equations in Pid Class description into equations.
- Not important, but just noting that equations in sinesweep contains slightly fuzzy font.
- Dither needs more description. Not clear what the amplitude/bandwidth of the white noise is.