Skip to content

Add ForEach action to repeat entities using iteration-specific values#802

Merged
christophebedard merged 3 commits intoros2:rollingfrom
christophebedard:christophebedard/for-loop-action
Feb 25, 2025
Merged

Add ForEach action to repeat entities using iteration-specific values#802
christophebedard merged 3 commits intoros2:rollingfrom
christophebedard:christophebedard/for-loop-action

Conversation

@christophebedard
Copy link
Member

@christophebedard christophebedard commented Oct 13, 2024

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:

  1. OpaqueFunction: Launch Loop Support? [Feature Request] #499 (comment)
  2. Recursion: https://github.com/MetroRobots/rosetta_launch#12---recursion

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 ForEach action. 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 OpaqueFunction directly in Python, and supports frontends, which OpaqueFunction doesn't/can't.

Example:

import launch


def for_each(id: int, name: str):
    return [
        launch.actions.LogInfo(msg=f"'{name}' id={id}"),
    ]


def generate_launch_description():
    return launch.LaunchDescription([
        launch.actions.DeclareLaunchArgument(
            'robots', default_value="{name: 'robotA', id: 1};{name: 'robotB', id: 2}"),
        launch.actions.ForEach(launch.substitutions.LaunchConfiguration('robots'), function=for_each),
    ])

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:

<launch>
    <arg name="robots" default="{name: 'robotA', id: 1};{name: 'robotB', id: 2}" />
    <for_each values="$(var robots)" >
        <log message="'$(for-var name)' id=$(for-var id)" />
    </for_each>
</launch>
launch:
    - arg:
        name: robots
        default: "{name: 'robotA', id: 1};{name: 'robotB', id: 2}"
    - for_each:
        values: $(var robots)
        children:
            - log:
                message: "'$(for-var name)' id=$(for-var id)"

If any of these launch files were launched without any launch arguments, they would output the following log messages:

[INFO] [launch.user]: 'robotA' id=1
[INFO] [launch.user]: 'robotB' id=2

With robots:="{name: 'robotC', id: 3};{name: 'robotD', id: 4};{name: 'robotE', id: 5}", they would output:

[INFO] [launch.user]: 'robotC' id=3
[INFO] [launch.user]: 'robotD' id=4
[INFO] [launch.user]: 'robotE' id=5

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 ForLoop action, which is a simpler version of ForEach with 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
import launch
import launch_ros


def for_i(i: int):
    return [
        launch_ros.actions.Node(
            package='test_tracetools',
            executable='test_ping',
            output='screen',
            namespace=['/pingpong_', str(i)],
        ),
        launch_ros.actions.Node(
            package='test_tracetools',
            executable='test_pong',
            output='screen',
            namespace=['/pingpong_', str(i)],
        ),
    ]


def generate_launch_description():
    return launch.LaunchDescription([
        launch.actions.DeclareLaunchArgument('num_robots', default_value='2'),
        launch.actions.ForLoop(launch.substitutions.LaunchConfiguration('num_robots'), function=for_i),
    ])
Click to expand: XML
<launch>
    <arg name="num_robots" default="2" />
    <for len="$(var num_robots)" name="i" >
        <node pkg="test_tracetools" exec="test_ping" namespace="/pingpong_$(index i)" output="screen" />
        <node pkg="test_tracetools" exec="test_pong" namespace="/pingpong_$(index i)" output="screen" />
    </for>
</launch>
Click to expand: YAML
launch:
    - arg:
        name: num_robots
        default: '2'
    - for:
        len: $(var num_robots)
        name: i
        children:
            - node:
                pkg: test_tracetools
                exec: test_ping
                namespace: /pingpong_$(index i)
                output: screen
            - node:
                pkg: test_tracetools
                exec: test_pong
                namespace: /pingpong_$(index i)
                output: screen

If any of these launch files were launched without any launch arguments, they would create 2 pairs of ping & pong nodes, each pair in its own namespace. With num_robots:=5, they would create 5 pairs.

Bonus: you can even nest <for> loops:

Click to expand
<launch>
    <arg name="num_i" default="2" />
    <arg name="num_j" default="3" />
    <for len="$(var num_i)" name="i" >
        <for len="$(var num_j)" name="j" >
            <log message="i=$(index i), j=$(index j)" />
        </for>
    </for>
</launch>

Output:

[INFO] [launch.user]: i=0, j=0
[INFO] [launch.user]: i=0, j=1
[INFO] [launch.user]: i=0, j=2
[INFO] [launch.user]: i=1, j=0
[INFO] [launch.user]: i=1, j=1
[INFO] [launch.user]: i=1, j=2

Open questions:

  1. Is this worth having in 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.
    • We discussed this PR at a high level in the November 12th, 2024, 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 OpaqueFunction (only in Python), so this reduces boilerplate code, and it allows us to use this functionality from the XML & YAML frontends. Therefore, pragmatically, it was deemed to be acceptable.
  2. For frontends, is there another mechanism (other than locals) for defining and using the index variable in each loop iteration?
    • I don't think so. And adding a suffix to the local name (see ForEachVar.get_local_arg_name()) makes it pretty robust.
  3. Anything else? Any limitations or anything that would break this?

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

@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch from fb823a2 to 769f819 Compare October 13, 2024 22:09
@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch from 769f819 to 391f8af Compare November 2, 2024 14:21
@christophebedard christophebedard changed the title Add ForLoop action to repeat entities with a substitutable index Add ForLoop action to repeat entities with an index Nov 2, 2024
@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch from 391f8af to b9cd085 Compare November 2, 2024 14:28
@christophebedard
Copy link
Member Author

After some quick initial feedback, I changed the implementation to instead take a callback function, similar to OpaqueFunction.

@Timple
Copy link

Timple commented Nov 3, 2024

Are you planning to support xml and yaml syntax as well?

@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch from b9cd085 to a28eb0c Compare November 4, 2024 15:50
@christophebedard
Copy link
Member Author

Yeah, I think XML and YAML support are very important. I managed to get a working hacky implementation, but it could probably be improved.

@christophebedard christophebedard self-assigned this Nov 4, 2024
@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch 4 times, most recently from 50a7b35 to 65dfa86 Compare November 5, 2024 16:37
@christophebedard christophebedard marked this pull request as ready for review November 5, 2024 17:07
@christophebedard
Copy link
Member Author

Well, with frontend support, this feels complete enough, so I think this is ready for a real review.

@christophebedard
Copy link
Member Author

Are you planning to support xml and yaml syntax as well?

@Timple feel free to provide feedback and/or just say whether you think this would be useful

@Timple
Copy link

Timple commented Nov 5, 2024

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 👍

@christophebedard
Copy link
Member Author

I will work on adding tests, but there are XML and YAML examples in the PR description above.

@Timple
Copy link

Timple commented Nov 5, 2024

Tjeez... I totally overlooked those. That works great!

@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch 2 times, most recently from 593554a to 6b35a08 Compare November 6, 2024 18:57
@christophebedard
Copy link
Member Author

I've now added tests (Python, XML, YAML).

@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch from 6b35a08 to 3c84d51 Compare November 6, 2024 19:07
@christophebedard
Copy link
Member Author

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 OpaqueFunction (only in Python), so this reduces boilerplate code, and it allows us to use this functionality from the XML & YAML frontends. Therefore, pragmatically, it was deemed to be acceptable.

Now the PR just needs to be reviewed.

@ahcorde
Copy link
Contributor

ahcorde commented Feb 12, 2025

Pulls: #802
Gist: https://gist.githubusercontent.com/ahcorde/d49e11b1a4d043160708c62bc883309f/raw/6f5aa30c2c8087e2ac6fca1b311bda68d210de8c/ros2.repos
BUILD args: --packages-above-and-dependencies launch launch_xml launch_yaml --packages-above-and-dependencies launch launch_xml launch_yaml
TEST args: --packages-above launch launch_xml launch_yaml --packages-above launch launch_xml launch_yaml
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15181

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

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

@christophebedard
Copy link
Member Author

Not really sure if we should update the copyroght to 2025,

I wrote this in 2024, but I don't mind changing the year to 2025.

I will be great to add some documentation to https://docs.ros.org/en/jazzy/Tutorials/Intermediate/Launch/Launch-Main.html

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.

@ahcorde
Copy link
Contributor

ahcorde commented Feb 12, 2025

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

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/requesting-feedback-new-for-loop-action-for-ros-2-launch-files-to-repeat-entities/42021/1

@christophebedard
Copy link
Member Author

Thank you. I posted on Discourse. Let's just wait a bit.

@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch 2 times, most recently from b091600 to 009c67b Compare February 17, 2025 20:30
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch from 009c67b to 2056b70 Compare February 23, 2025 00:00
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch from 2056b70 to d97347f Compare February 23, 2025 00:28
@christophebedard christophebedard changed the title Add ForLoop action to repeat entities with an index Add ForEach action to repeat entities using iteration-specific values Feb 24, 2025
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
@christophebedard christophebedard force-pushed the christophebedard/for-loop-action branch from 14c1d3c to 027e560 Compare February 24, 2025 00:37
@christophebedard
Copy link
Member Author

I generalized the action into a ForEach action to make it more flexible (and much more powerful) based on feedback from https://discourse.ros.org/t/requesting-feedback-new-for-loop-action-for-ros-2-launch-files-to-repeat-entities/42021. It can now iterate over sets of parameters. I've kept the original index-based ForLoop action for simple use-cases. See the updated PR description.

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/requesting-feedback-new-for-loop-action-for-ros-2-launch-files-to-repeat-entities/42021/5

@ahcorde
Copy link
Contributor

ahcorde commented Feb 24, 2025

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@christophebedard
Copy link
Member Author

There's a seemingly-unrelated build failure with rviz_ogre_vendor in the Windows job. Can't really figure out what happened. I just re-triggered the job.

@christophebedard
Copy link
Member Author

christophebedard commented Feb 25, 2025

Test failures on Windows are unrelated.

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.

4 participants