Skip to content

Porting RewrittenYaml from Nav2#518

Open
DLu wants to merge 3 commits intoros2:rollingfrom
DLu:rewritten_yaml
Open

Porting RewrittenYaml from Nav2#518
DLu wants to merge 3 commits intoros2:rollingfrom
DLu:rewritten_yaml

Conversation

@DLu
Copy link
Contributor

@DLu DLu commented Feb 17, 2026

Description

Straight port of RewrittenYaml from Nav2

Fixes #258

Is this user-facing behavior change?

Yes, adds a new substitution.

Did you use Generative AI?

No

Additional Information

This is the exact implementation and test code from Nav2, with the exception of modifying the imports in the second two commits. No other improvements were attempted.

@DLu
Copy link
Contributor Author

DLu commented Feb 17, 2026

TBH: I feel like the multiple types of rewrites available here chafes a bit against the "Distinct substitutions that can be composed make the most sense to me." that @mjcarroll articulated here, but I also haven't written a lot of uses for RewrittenYaml

@izzaaaatanishq
Copy link
Contributor

Related: ros-teleop/twist_mux#65

@emersonknapp
Copy link
Contributor

emersonknapp commented Feb 26, 2026

This is sometimes useful, for sure. But I've noticed in the wild that a lot of uses of it aren't actually necessary, resulting in a no-op, and it's only there because the user copy-pasted a Nav2 python launchfile as their starting point.

We could consider having it raise a Warning when the result is a no-op just to make it clear to users that they've cargo-culted something unnecessary.

Also we should try really hard to make sure that all Actions and Substitutions are exposed to launch frontend (so they can be used in YAML/XML). I had started on this one before parental leave but don't think I finished it. I think the interface is complex enough that the $(rewritten-yaml ARGS ARGS ARGS) gets complex enough it probably warrants some new patterns or architecture to support high-complexity substitutions. OR, we may be better off taking a closer look at the exact use cases this is smoothing over in the majority of uses and see how the core tooling can support that better.

Nothing I've said is a blocker, but food for thought.

@peci1
Copy link

peci1 commented Feb 28, 2026

Any particular reason why this goes to launch_ros and not to launch? I don't see anything ROS-specific here...

@mjcarroll
Copy link
Member

@DLu when you get a moment, do you have any thoughts on these two comments?

@DLu
Copy link
Contributor Author

DLu commented Mar 9, 2026

This is sometimes useful, for sure. But I've noticed in the wild that a lot of uses of it aren't actually necessary, resulting in a no-op, and it's only there because the user copy-pasted a Nav2 python launchfile as their starting point.

This is a bigger question, and maybe something that people with more Nav2 experience than I should address. I fall into the category of people who mostly copy-paste it.

Also we should try really hard to make sure that all Actions and Substitutions are exposed to launch frontend (so they can be used in YAML/XML).

I concur, but was starting by just porting what exists in the Nav2 repo.

I think the interface is complex enough that the $(rewritten-yaml ARGS ARGS ARGS) gets complex enough it probably warrants some new patterns or architecture to support high-complexity substitutions

That's basically what I was getting at with the "Yaml Operations" patterns I discussed in my original post here: #517

Any particular reason why this goes to launch_ros and not to launch? I don't see anything ROS-specific here...

A) The original ticket was here #258 in this repo.
B) ros2/launch/launch has a fairly sparing use of yaml (except in the for-loops(?!)), whereas there's a lot here.
C) All the parameter descriptions and utilities are in this repo.

I'm happy to change it as needed.

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.

consider porting RewrittenYaml from Nav2 to allow param overrides to YAML from launch

5 participants