Add ForEach action to repeat entities using iteration-specific values#802
Conversation
fb823a2 to
769f819
Compare
769f819 to
391f8af
Compare
391f8af to
b9cd085
Compare
|
After some quick initial feedback, I changed the implementation to instead take a callback function, similar to |
|
Are you planning to support xml and yaml syntax as well? |
b9cd085 to
a28eb0c
Compare
|
Yeah, I think XML and YAML support are very important. I managed to get a working hacky implementation, but it could probably be improved. |
50a7b35 to
65dfa86
Compare
|
Well, with frontend support, this feels complete enough, so I think this is ready for a real review. |
@Timple feel free to provide feedback and/or just say whether you think this would be useful |
|
I don't know how this would look like in xml. So an example or test for that would be nice to show that. Having the ability to support N lidar nodes is nice 👍 |
|
I will work on adding tests, but there are XML and YAML examples in the PR description above. |
|
Tjeez... I totally overlooked those. That works great! |
593554a to
6b35a08
Compare
|
I've now added tests (Python, XML, YAML). |
6b35a08 to
3c84d51
Compare
|
We discussed this PR at a high level in the November 12th ROS PMC meeting. In short, this is pushing us towards less declarative launch files, which makes writing potential linters/checkers harder. However, it was still possible to do this before using Now the PR just needs to be reviewed. |
|
Pulls: #802 |
ahcorde
left a comment
There was a problem hiding this comment.
Not really sure if we should update the copyroght to 2025,
I will be great to add some documentation to https://docs.ros.org/en/jazzy/Tutorials/Intermediate/Launch/Launch-Main.html
I wrote this in 2024, but I don't mind changing the year to 2025.
Yeah, I will document this. Also, if it's preferable, I could post on Discourse before this is merged just to get feedback from users. |
We can wait a little bit more in case you want to get some more feedback, but I'm happy to take this one as it's right now |
|
This pull request has been mentioned on ROS Discourse. There might be relevant details there: |
|
Thank you. I posted on Discourse. Let's just wait a bit. |
b091600 to
009c67b
Compare
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
009c67b to
2056b70
Compare
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
2056b70 to
d97347f
Compare
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
14c1d3c to
027e560
Compare
|
I generalized the action into a |
|
This pull request has been mentioned on ROS Discourse. There might be relevant details there: |
|
There's a seemingly-unrelated build failure with |
|
Test failures on Windows are unrelated. |
Relates #499
Being able to repeat entities (e.g., nodes) N times based on the value of a launch argument is a pretty commonly-requested feature. Example: #499 and https://discourse.ros.org/t/poll-interest-in-ros2-launch-action-to-support-for-loops-e-g-includenlaunchdescriptions/20026. Some current solutions/workarounds include:
OpaqueFunction: Launch Loop Support? [Feature Request] #499 (comment)While these work, they involve some boilerplate code that has to be repeated in each launch file, and they make launch files more complex.
This PR aims to simplify this by adding a new
ForEachaction. It takes in sets of input values provided as a semicolon-separated list of string YAML structures, which can be a launch argument (LaunchConfiguration) or any other substitution. It also takes in a function that gets called with each set of input values. This function should return a list of entities using the input values to differentiate entities between each for-loop iteration.This is basically a nicer alternative to using an
OpaqueFunctiondirectly in Python, and supports frontends, whichOpaqueFunctiondoesn't/can't.Example:
Frontends are also supported, which isn't possible with
OpaqueFunction. In this case, the callback function is created internally by the parser using the child entities. In the launch file, entities can use a$(for-var)substitution referencing a variable name:If any of these launch files were launched without any launch arguments, they would output the following log messages:
With
robots:="{name: 'robotC', id: 3};{name: 'robotD', id: 4};{name: 'robotE', id: 5}", they would output:In Python, default values can be defined through default values of callback function parameters, in which case they may be omitted from a set of values in the YAML string. For frontends, a default value can be provided to the
$(for-var)substitution, e.g.,$(for-var name 'default').There's also a
ForLoopaction, which is a simpler version ofForEachwith just an index value. It takes in something that defines the number of for-loop iterations and a function that gets called with each for-loop index value (0 to N, exclusive). For frontends, the callback function is created internally by the parser using the child entities and an$(index)substitution referencing the (index) name of the for-loop:Examples:
Click to expand: Python
Click to expand: XML
Click to expand: YAML
If any of these launch files were launched without any launch arguments, they would create 2 pairs of
ping&pongnodes, each pair in its own namespace. Withnum_robots:=5, they would create 5 pairs.Bonus: you can even nest
<for>loops:Click to expand
Output:
Open questions:
launch? I think so given that this is a common feature request (and undoubtedly a common Google search), but I'm open to other opinions.ForEachVar.get_local_arg_name()) makes it pretty robust.Also, once it looks good enough, I'd like to post on Discourse to request feedback from users.I posted here: https://discourse.ros.org/t/requesting-feedback-new-for-loop-action-for-ros-2-launch-files-to-repeat-entities/42021