roswtf/Reviews/2009-12-30_Doc_Review

Reviewers:

• Tully
• Kevin

API Review

Tully: Please also do an API review of the plugin API.

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. Is it clear to an outside user what the roadmap is for the Package?
5. Is it clear to an outside user what the stability is for the Package?
6. Are concepts introduced by the Package well illustrated?

Concerns / issues

Kevin

Usage

• Should be a very comprehensive lists of checks that roswtf runs. Environment checks, file system checks, etc. Online checks should also be spelled out

  • kwc: I avoided this on purpose. I don't believe that such a list would be maintainable.
• The "Cannot communicate with master, ignoring graph checks" message should be changed.

  • "ROS Master isn't running. ROS_MASTER_URI=http://localhost:11311. If the ROS is running, check the Master URI."

    • kwc: fixed
• It should try "ps aux | grep ros" to see if any ROS processes are running if the core is down.
  • kwc: this is a feature request, please ticket instead

Plugins

• Agree with Tully on the plugin doc. Could also explain why a plugin is useful to developers and users ("Using roswtf, it's easy to warn users when a critical part of the system isn't working. For example, tf uses roswtf to check for transform loops.")

  • kwc: I made some minor tweaks, but I'm not sure how the suggestion above is that different from what is already written.

• Tutorial would be super useful. Tutorial should cover the basic plugin, making the package, and writing a unit test for the plugin. It can be everything I want my diagnostics/Tutorials/Creating a Diagnostic Analyzer tutorial to be.

• The sentence for plugins: "Keeping in mind that creating a plugin affects everyone who installs your package, here the necessary steps." is confusing.
  • kwc: fixed
  • Plugins should be loaded and run as safely as possible. Plugins that abort, etc, should report an error. I think this might already happen
    • kwc: it does already happen
  • Plugins could be given a timeout to run their checks to avoid hanging plugins
    • kwc: this is outside the scope of a doc review, and it's also intractable. Even if the plugin is run in a separate thread, there is no way to guarantee it's termination.
  • They could be loaded in separate threads, in case one is really slow.
  • I wrote up a few similar thoughts in #2270

Other docs

• manifest.xml description brief needs to be filled out.
  • kwc: done
• Thoughts on topics #2267. This is already "wontfix".
  • I think we could look for topics that aren't hooked up, ("Warning: No publishers for 'joint_states'", "Warning: Topic 'joint_state' isn't being published") and display them alphabetically.
  • Misnamed topics is really annoying. Telling users that two topics with the same type aren't hooked up is very useful. ("Warning: Topic 'joint_state' isn't being published. Topic type 'sensor_msgs/JointStates' matches 'joint_states'.")
  • roswtf could work with rxgraph to show attempted subscriptions, etc. I'm not sure this already exists (#2271)
    • kwc: it already does

Tully

Usage

• There are no tutorials to get the user running
  • Something like "Go to your ros environment and type 'roswtf'
  • Also a tutorial where you can set something bad and catch it would be good.
    • maybe one warning and one error condition, instructed to be induced by the tutorial, and then fixed. So the user can see the roswtf when returning all's good and when warning about something.
      • kwc: DONE. Could probably do more examples, but the issue is how many assumptions we can make about the user's actual configuration (and ability to write files).

Plugin API

• The API itself is pretty good. It's just knowing what the critical interfaces are is not well documented.
• Document what is ctx? Why is it there? Can it be useful to me as a plugin writer?
• What are the return codes/values? When I did it I copied from tfwtf

    for r in rospack_warnings:
        warning_rule(r, r[0](ctx), ctx)
    for r in rospack_errors:
        error_rule(r, r[0](ctx), ctx)

• similarly, error_rule and warning_rule are not documented on the wiki, though you can drill down and find them in the autodocs
• basically a simple tutorial of what the minimum elements are for creating a plugin would be very helpful. I can only really understand what I did now that I've looked back on it and looked up the specific functions in the API docs.

Conclusion

TODO:

• work on epydoc of roswtf and link into plugin documentation
• beef up plugin documentation and move to separate page
• DONE: add in basic tutorial that shows roswtf working/failing

Based on above comments, roswtf end user API is doc reviewed. More work remains to be done for plugin API. As plugin API is still very advanced and not well exercised, leaving that for a later date.

→ status=Doc reviewed, notes=end-user docs passed. plugin API requires more work.