Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve launch documentation #3950

Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from

Conversation

mhidalgo-bdai
Copy link

@mhidalgo-bdai mhidalgo-bdai commented Oct 3, 2023

This patch improves upon existing documentation on launch to make it easier on newcomers to use it. In particular, it:

  • reworks the About Launch page under Concepts to be a bit more thorough, and
  • extends the guide on Using Python, XML, and YAML for ROS 2 Launch files with more examples.

Contributed by The AI Institute.

* Rework launch concepts page
* Augment launch file examples

Contributed by The AI Institute

Signed-off-by: Michel Hidalgo <mhidalgo@theaiinstitute.com>
@mhidalgo-bdai
Copy link
Author

mhidalgo-bdai commented Oct 3, 2023

FYI @jbarry-bdai @baxelrod-bdai

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+100 for improving this. As a high-level comment: as someone with a CS background, I appreciate the terminology used to explain concepts in launch.

If I were to show this to any of my non-CS colleagues, they would probably be more confused about launch than if they hadn't read this page.

Not sure how to improve upon this though. Perhaps it should not be the only description, or be put somewhere else than Concepts/Basic?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! I guess we could move this modified page under Intermediate or Advanced, but I will say that I wouldn't know how to explain launch in simpler terms. One could argue that a newcomer doesn't really need to understand launch to use launch files, and that's fair, but a primer on how to run ros2 launch will only take them so far.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could move these modified page under Intermediate or Advanced, but I will say that I wouldn't know how to explain launch in simpler terms.

I should perhaps have posted my comment on specific sections :)

As an example, the following section does actually nicely explain how things work, but is not something I would give to non-CS people:

In a way, launch descriptions are analogous to programs in a domain specific language tailored for process orchestration, and, in particular, ROS 2 system orchestration. When composed using the building blocks available in its native Python implementation, these descriptions resemble ASTs in procedural programming languages. The analogy has its limits, however: context is not implicitly restricted to syntactical boundaries like it would for typical variable scopes, and action execution is naturally concurrent as opposed to sequential, to name a few. However, it does bring about an important distinction that is easy to miss when writing launch files in Python: no action nor condition nor substitution carries out a computation upon instantiation but simply specifies a computation to be carried out in runtime.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the following section does actually nicely explain how things work, but is not something I would give to non-CS people

Yeah, I get it, it presumes a CS background and then reaches out to it to make a point. I could just state it without explanation. Or we could take this to an advanced concepts page entirely.

a primer on how to run ros2 launch will only take them so far.

On a second thought, maybe we could split the difference by keeping the basic notions that are necessary to use XML launch files here, and deferring the rest along with the introduction to Python launch files. It's a detour from usual practice (Python launch files came first, Python launch files have access to all features and more), but much of the complexity that launch has to offer does not surface when using comparably simpler XML (or YAML) launch files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a second thought, maybe we could split the difference by keeping the basic notions that are necessary to use XML launch files here, and deferring the rest along with the introduction to Python launch files.

personally, I would be very much in favour of this.

The current documentation seems to suggest that Python launch files are the main entry point, which I'm not sure is a desirable situation.

Similar to my comment above about explanations for users with CS backgrounds, Python launch files are considered complex and 'verbose' by many I discuss this with (surprisingly almost, as that's typically a complaint people have about XML).

Going over the XML launch files first and then gradually building up to the Python backend -- perhaps while making references to it already from the XML-focused content -- would seem like a great way to flatten the learning curve a bit.

The XML frontend would then be for "simple" launch files, while complex things like iteration, event handling, etc, would be where it would make sense for users to 'graduate' to Python launch files.

(example: showing a simple .xacro in RViz does not absolutely need a Python launch file, but composing nodes in a container with life-cycle management and other event handling definitely does at the moment)


Having written that: I'm just some guy who randomly decided to comment on your PR, so take it for what it is: a single bit of input.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having written that: I'm just some guy who randomly decided to comment on your PR, so take it for what it is: a single bit of input.

Haha, I know, I know. It doesn't make your arguments less valid though. I'll wait for @clalancette or @audrow to chime in.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a second thought, maybe we could split the difference by keeping the basic notions that are necessary to use XML launch files here, and deferring the rest along with the introduction to Python launch files. It's a detour from usual practice (Python launch files came first, Python launch files have access to all features and more), but much of the complexity that launch has to offer does not surface when using comparably simpler XML (or YAML) launch files.

What do you think about this @clalancette ?

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @gavanderhoorn suggests, the documentation around launch could really use some improvements. So thanks for starting the conversation. I'm going to actually leave some general thoughts here, rather than doing a detailed review of what you've done here.

General thoughts

I think the biggest thing we need to think about is how to present launch to users holistically. Right now, there are ~10 places we discuss launch:

There is good information in each of those places, but the overall impression I have from looking at it is that the documentation is scattered. I think it would be difficult for anyone to come along and successfully learn launch from that set of pages.

So my suggestion here is that we take a step back and figure out how we would reduce that set and make it more coherent.

Here is a straw-man argument for what the final state should be (happy to iterate on this):

And that's it. All of the content of the pages I did not mention would be migrated to one of these places as appropriate. And we would add in lots of interlinks between the concepts, tutorials, and API documentation.

This cuts us down to ~7 places where we discuss launch, but I also think that it is far more logical this way. We have conceptual things under "Concepts", and "How to use" things under "Tutorials".

Thoughts on the overall idea, and of the specifics of what I've laid out above?

@mhidalgo-bdai
Copy link
Author

mhidalgo-bdai commented Oct 5, 2023

Thoughts on the overall idea, and of the specifics of what I've laid out above?

I'm a bit fuzzy on what would happen to guides. Guide != tutorial IMHO. Otherwise, the layout looks reasonable to me. I agree it doesn't help to have bits of launch documentation all over the place. I would personally build up launch and launch_ros documentation to include conceptual and architectural deep dives, not just an API reference. I even have a proposal for a layout I can share.

That said, I can't stretch the scope of this PR to cover a complete overhaul. I can reorganize this contribution into basic and intermediate concepts. I can relocate the guide.

@clalancette
Copy link
Contributor

I'm a bit fuzzy on what would happen to guides. Guide != tutorial IMHO

Yes, completely agreed. Guides are meant to answer a specific question, while Tutorials walk through step-by-step, showing the user how to do something.

That said, in my taxonomy above there are only two guides:

  • Launching composable nodes
  • Launch file different formats

I think both of those could be reasonably incorporated into Tutorials and removed from the Guides. Though again, I'm willing to hear other opinions and change it up.

I would personally build up launch and launch_ros documentation to include conceptual and architectural deep dives, not just an API reference. I even have a proposal for a layout I can share.

This is a hard one to gauge for me. The ability to have detailed documentation in the individual packages is definitely a feature. But I also think that we've been most successful (in terms of people finding it) with documentation that is here in ros2_documentation. Can you share your layout proposal as a starting point?

That said, I can't stretch the scope of this PR to cover a complete overhaul.

Yeah, that's totally fair. I think it is worthwhile for us to figure out what we want the final product to look like, and write it down somewhere (likely an issue in this repository). Then as people have time to work on it, at least we have a framework to fit it into.

@mhidalgo-bdai
Copy link
Author

I think both of those could be reasonably incorporated into Tutorials and removed from the Guides.

I see. That's fair.

Can you share your layout proposal as a starting point?

I had something like this in mind for a launch documentation TOC:

  • Motivation (why launch?)
    • FAQ (e.g. why not a shell script?)
  • Concepts (what is launch?)
    • Context
      • Configurations
      • Events
      • Scope
    • Statements
      • Actions
    • Expressions
      • Conditions
      • Substitutions
    • Control flow
      • Event handling
  • Architecture (launch details)
    • Entities
    • Descriptions
      • Frontend (parsers)
    • Service
  • User guide (how to launch?)
    • Cheatsheet (reference on actions, substitutions, etc.)
    • Extensions
      • Implementing an action
      • Implementing a event
      • Implementing a condition
      • Implementing a substitution
      • Implementing an event handler
      • Adding a new frontend
  • API Reference (polished)
  • Glossary

IMHO this is too detailed to be hosted in https://docs.ros.org. For the front page, the launch concepts page in this PR already covers enough of the basics (and internals for CS savvy folks).

I think it is worthwhile for us to figure out what we want the final product to look like, and write it down somewhere (likely an issue in this repository)

Sounds reasonable. I can adapt this PR to cover concepts and a tutorial on writing launch files.

@mhidalgo-bdai
Copy link
Author

@clalancette friendly ping

@mhidalgo-bdai
Copy link
Author

@clalancette another friendly ping (now that ROSCON is over :))

@clalancette
Copy link
Contributor

@mhidalgo-bdai So sorry for the very delayed response. I'm finally coming back around to this.

I like the topics covered in your table of contents. That is exactly the type of thing I think we should cover.

What I'm wondering is whether it is worthwhile to split it up, though. In particular, I'm kind of thinking we can take your Concepts section and move it into Concepts/Intermediate here on https://docs.ros.org . And then we can take the User Guide portion and move it into Tutorials/Intermediate here on https://docs.ros.org. That would leave only "Architecture" and "API" reference in the individual packages, where I think they belong. Incorporating that idea back into what I mentioned before, we would end up with something like this:

Thoughts on that?

@mhidalgo-bdai
Copy link
Author

This looks reasonable. I can try to rework this patch into both concepts and user guide pages. It may take me a while though (internal re-prioritization). I'll ping you when I get to circle back.

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is really good enhancement for launch, thanks 👍 besides, restructure the sections previously discussed, i have a couple of minor comments.

All of the above is specified in a launch file, which can be written in Python, XML, or YAML.
This launch file can then be run using the ``ros2 launch`` command, and all of the nodes specified will be run.
``launch`` provides the means to orchestrate the execution of ROS 2 systems, in ROS 2 terms.
_Launch files_ describe the orchestration procedure, including but not limited to which nodes to run, how to run them, and which arguments to use for them.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a comment, this does not block the PR.

i would expect by orchestrated that auto-healing like restarting the node on different physical host system if anything happens, replication of nodes and so on. AFAIK, those orchestration features are not supported ROS 2 launch?

including but not limited to which nodes to run, how to run them, and which arguments to use for them.

IMO, those are just configuration and setting for the ROS 2 node?

Moving parts
------------

``launch`` works with [_actions_](https://docs.ros.org/en/rolling/p/launch/launch.html#launch.Action), abstract representations of computational procedures with side effects on its execution environment.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same fix can be applied to other places as well.

Suggested change
``launch`` works with [_actions_](https://docs.ros.org/en/rolling/p/launch/launch.html#launch.Action), abstract representations of computational procedures with side effects on its execution environment.
``launch`` works with [_actions_](https://docs.ros.org/en/{DISTRO}/p/launch/launch.html#launch.Action), abstract representations of computational procedures with side effects on its execution environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants