Add XML/YAML launch file equivalents to Tf2 tutorials #6031
Conversation
- Add XML and YAML equivalents for all Python launch files - Update tutorial RST files with tabbed multi-format code examples - Use .xml as default in console commands with inline format comments Signed-off-by: Luke Sy <sylukewicent@gmail.com>
Signed-off-by: Luke Sy <sylukewicent@gmail.com>
- Update launch file format references (.xml -> .py) - Make launch file sections format-agnostic with tab-specific explanations - Correct YAML literalinclude line numbers in Adding-A-Frame tutorials Signed-off-by: Luke Sy <sylukewicent@gmail.com>
|
@lsy3 this is great and super helpful. I did a 10 minute read through on this PR and I didn't see any immediate problems. However, I also did not run the launch files in question. The PR touches a lot of files but the changes are fairly repetitive. @emersonknapp if you are happy with this I am willing to do a second approval. |
kscottz
left a comment
kscottz
left a comment
There was a problem hiding this comment.
I think this is acceptable as is. I'll go ahead and approve it.
We probably want to circle back and write a tutorial comparing and contrasting the three launch file formats and discussing why you might choose one over the other.
|
@SuperJappie08 really appreciate your help on #6021. We would really appreciate your help on this PR too. It would be awesome if we could get these into the docs for the holidays. |
| .. group-tab:: XML | ||
|
|
||
| .. literalinclude:: launch/py_turtle_tf2_demo_launch.xml | ||
| :language: xml |
There was a problem hiding this comment.
The file name is missing.
| :language: xml | |
| :language: xml | |
| :name: turtle_tf2_demo_launch.xml |
| .. group-tab:: YAML | ||
|
|
||
| .. literalinclude:: launch/py_turtle_tf2_demo_launch.yaml | ||
| :language: yaml |
There was a problem hiding this comment.
The name is present on the Python variant.
It should match with the other frontends (either all having a name or none having a name).
| :language: yaml | |
| :language: yaml | |
| :name: turtle_tf2_demo_launch.yaml |
| :name: turtle_tf2_demo_launch.py | ||
| .. tabs:: | ||
|
|
||
| .. group-tab:: Python |
There was a problem hiding this comment.
I think the other launch tutorials use the order
- XML
- YAML
- Python
Is there a reason to deviate from this, as using raw launch (without front-end, so most Python launchfiles) is being discouraged?
| .. code-block:: console | ||
| $ ros2 launch learning_tf2_py turtle_tf2_demo_launch.py | ||
| $ ros2 launch learning_tf2_py turtle_tf2_demo_launch.xml # .py or .yaml are also acceptable |
There was a problem hiding this comment.
Can this also be within tabs like all the other launch frontend specific sections.
|
|
||
| .. literalinclude:: launch/py_turtle_tf2_demo_launch.yaml | ||
| :language: yaml | ||
| :lines: 4-9 |
There was a problem hiding this comment.
| :lines: 4-9 | |
| :lines: 4-14 |
This should probably extend over the whole launchfile
|
|
||
| .. literalinclude:: launch/py_turtle_tf2_fixed_frame_demo_launch.xml | ||
| :language: xml | ||
| :lines: 3-4 |
|
|
||
| .. group-tab:: Python | ||
|
|
||
| .. literalinclude:: launch/py_turtle_tf2_dynamic_frame_demo_launch.py |
There was a problem hiding this comment.
| .. literalinclude:: launch/py_turtle_tf2_dynamic_frame_demo_launch.py | |
| .. literalinclude:: launch/py_turtle_tf2_dynamic_frame_demo_launch.py | |
| :language: python |
| .. group-tab:: Python | ||
|
|
||
| .. literalinclude:: launch/py_turtle_tf2_dynamic_frame_demo_launch.py | ||
| :name: turtle_tf2_dynamic_frame_demo_launch.py |
There was a problem hiding this comment.
Can the name be added to all tabs?
|
|
||
| .. literalinclude:: launch/turtle_tf2_fixed_frame_demo_launch.xml | ||
| :language: xml | ||
| :lines: 3-4 |
| .. code-block:: console | ||
| $ ros2 launch learning_tf2_cpp turtle_tf2_demo_launch.py | ||
| $ ros2 launch learning_tf2_cpp turtle_tf2_demo_launch.xml # .py or .yaml are also acceptable |
There was a problem hiding this comment.
Can this also be tabs per launch file type for consistency between all tutorials?
|
@lsy3 do you have some time to address @SuperJappie08 comments ? |
I can give it a look within the coming week! |
|
I really went overboard with the comments; I'm still figuring out a nice way to review large PRs. Any tips are greatly appreciated |
|
@SuperJappie08 great question. I think it helps to have the right perspective when reviewing. I've written out my sorta philosophy below. When it comes to community contributions you shouldn't be asking yourself, "Is this perfect?" you should be asking yourself, "Is this better than what we have now?" If the PR is better than we have now then we should probably merge it. The likelihood of the contributor returning to fix the PR drops with every change you request, so you want to be judicious. I would limit big change requests on the part of the author to the things that are absolutely critical (i.e. stuff that's actually broken and requires a big fix). For everything else I try to use the "suggest changes" features. By the time you go back and forth on requesting a change, having the author add it, and then reviewing it again you probably wasted a lot more time than having just suggested a change. I also think this feature is helpful because it sorta signals your expectations without forcing the author to spend more time actually implementing them. In my mind it makes the PR review process a bit less oppositional and much more conversant because you are helping the author get their work merged. |
4 participants
Description
Add XML/YAML launch file equivalents for intermediate Tf2 tutorials.
Fixes #6006
Did you use Generative AI?
Partially - Cursor AI Assistant for generating XML/YAML launch file equivalents and tutorial rephrasing. Each generated line reviewed.
Additional Information
Tutorials/Intermediate/Tf2/directory