roslaunch/Reviews/2010-01-10_Doc_Review

Reviewer: Ethan Dreyfuss

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, though making it clearer on the package page that the command-line tools are what most users want might be helpful (or add an example to the package page as suggested in the concerns/issues section).
2. Are all of these APIs documented?
   • Yes, everything has documentation coverage.
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?
   • Yes, there are tutorials both in the form of common roslaunch invocations and for structuring launch files in larger projects
4. Is it clear to an outside user what the roadmap is for the Package?
   • Yes, some planned features with no definite timeline are listed, and the ROS roadmap is linked (presumably where any features with definite timelines will show up)
5. Is it clear to an outside user what the stability is for the Package?
   • Yes, stable except for the code API, the usage of which is warned against.
6. Are concepts introduced by the Package well illustrated?
   • Yes, there are good tutorials and example commands throughout

For each launch file in a Package

1. Is it clear how to run that launch file?
   • kwc: note to reviewer, 'roscore' is effectively a launch file
   • Yes, the documentation explains the usage of roslaunch, and none of the launch files require much beyond "roslaunch [filename.launch]". roscore.xml is a bit of a special case, but the documentation explains this. Also, it's not really necessary to launch any of these launch files manually, they're really intended as code examples rather than functional units.
2. Does the launch file start up with no errors when run correctly?
   • Yes.
3. Do the Nodes in that launch file correctly use ROS_ERROR/ROS_WARN/ROS_INFO logging levels?
   • Yes or N/A

Concerns / issues

===Minor typos (already fixed):===

• Architecture page: you can group together a collection of nodes to given them the same name -> give

• Commandline Tools page: Environment Variables section: This is a dangerous option as introduces a security hole and should only be used if you understand the consequences. -> This is a dangerous option as it introduces a security hole and should only be used if you understand the consequences.

• XML/node: machine="machine-name(optional) -> machine="machine-name"(optional)

• Tutorials/Roslaunch Tips for Larger Projects: "wait till" -> "wait until"

• "actually includes the exact same yaml file." -> "This actually includes the exact same yaml file as the line before it"

Additional comments / suggestions / concerns

TODO:

• First sentence suggestion:
  • roslaunch is a tool for launching ROS nodes locally and remotely via SSH, as well as setting parameters on the Parameter Server.

    -> roslaunch is a tool for easily launching multiple ROS nodes locally and remotely (via SSH), as well as setting parameters on the Parameter Server. Rationale: The distinguishing feature of roslaunch is not that it can launch nodes, you can do this by running the binaries directly, rather the distinguishing feature is that you can launch multiple nodes easily.

    • kwc: FIXED
• Commandline tools: -p option: different port than what? The default? Does it not use the ROS_MASTER_URI? I'm unclear on when I would actually need this option.
• The --args documentation is slightly confusing. "Display the command-line arguments that roslaunch uses when launching the node" would be better if it said something more like "Display the command-line arguments that roslaunch uses when launching the node in file.launch named node_name". Also the use case for substitution args is unclear. This seems to be thrown in there as a note for someone that had trouble with substitution args in the past because of a single vs double quote issue in bash. It might also be nice to document the behavior of listing nodes when a non-existent node name is passed in. It might be even nicer for this list to come up in addition to the usage instructions when no node name is passed in (assume a single argument is intended as a launch file argument).
• An example for --find would be nice. (And it's a great addition, I've needed exactly this tool before).
  • kwc: FIXED

DONE:

• It could be good to get a basic usage example onto the package page. I could see a new user being confused about whether they want the command line tools or something else.
  • kwc: FIXED
• roslaunch/XML: $(optenv ENVIRONMENT_VARIABLE) behavior should be documented, so perhaps change:
  • substitute the value of an environment variable if it is set. If default is provided, it will be used if environment variable is not set.

    -> substitute the value of an environment variable if it is set. If default is provided, it will be used if environment variable is not set otherwise the empty string wil/xmlparl be substituted.

  • kwc: FIXED
• The $(anon name) example is confusing. Why use name="anon foo" and not just foo? It makes it seem like it's intended to be a correct example but is missing $(). In fact, why not make it a correct usage example?
  • kwc: this was the XML formatter

• Substitution args are currently resolved on the local machine. In otherwords, environment variables and ROS package paths will be set to their values in your current environment. -> Substitution args are currently resolved on the local machine. In otherwords, environment variables and ROS package paths will be set to their values in your current environment even for remotely launched processes.

  • kwc: FIXED
• 4.2: is localhost being remapped to localhost? This is confusing, it should probably be something different. Does this remapping change where everything is launched because everything is launched on "localhost" by default?
  • kwc: changed name to "local_alt"
• 4.3: reference to xmlparam in example should be removed I believe, unless this is something that is still on the roadmap (I think it's just out of date, but I'm not sure).
  • kwc: FIXED

• http://www.ros.org/wiki/roslaunch/XML/node : "The main roslaunch page covers some examples of how to fully utilize the <node> tag, such as configuring to launch a Node in gdb." is no longer a true statement. The correct link is now: http://www.ros.org/wiki/roslaunch/Tutorials/Roslaunch%20Nodes%20in%20Valgrind%20or%20GDB

  • kwc: FIXED
• 1.1 (first example): Should the args="test" be args="--test"? Otherwise the sentence below wouldn't seem to be true (unless there's some prepend -- behavior of which I am unaware). (EDIT, this seems to be a wiki formatting issue, the source is correct), ditto for the args in the second example missing a $().
  • kwc: removed #!xml, which is clearly buggy

• non doc review note: I always have trouble remembering the launch-prefix attribute. I keep ending up looking it up. Maybe it could become just prefix at some point?

  • kwc: noted
• XML/machine: default="true|false|never" (optional): is the machine really chosen arbitrarily if no default machine is specified? I always assumed it was localhost by default. This should probably be made more prominent if it's true.
  • kwc: you are correct. FIXED

• "it is generally a symptom that your overall ROS graph will have communication issues." -> "needing to change this parameter is generally a symptom that your overall ROS graph will have communication issues."

• XML/remap: "in a more structured manner" -> "in a more structured manner than using the args attribute of <node>"

  • kwc: DONE

• XML/param: "You can also set private parameter across a group of nodes by specifying using the ~param syntax in a <param> tag." ~param syntax should probably be linked to relevant documentation.

  • kwc: DONE
• A sentence or two about how the type is automatically determined might be useful.
  • kwc: DONE

• the command= should make clear that the &quot; stuff is due to XML formatting requirements.

  • kwc: DONE

For bhaskara (DONE):

• Tutorials/XML: 1.3: large block of text from pre.machine: This should be updated to reflect a more recent pre.machine, which among other things removes the ugly "HACK" stuff.
• References to machine three/four should probably be removed

Conclusion

There are a passel of minor issues but nothing that should require much work to fix. Overall the documentation is in quite good shape. It does a particularly good job of linking internally when applicable.