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

Propagate launch arguments in included launch descriptions #313

Open
ivanpauno opened this issue Aug 16, 2019 · 7 comments
Open

Propagate launch arguments in included launch descriptions #313

ivanpauno opened this issue Aug 16, 2019 · 7 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@ivanpauno
Copy link
Member

ivanpauno commented Aug 16, 2019

Feature request

Feature description

There have been some previous discussion in #252 #249 too.
I've been discussing this a little with @hidmic, here are some of the items we discussed:

  • We're currently doing a best effort checking of arguments when visiting IncludeLaunchDescription action. Launch arguments of nested launch descriptions are ignored in the process. We're also only showing those arguments when executing ros2 launch -s <PACKAGE> <LAUNCH_FILE>. But we're not checking in ros2 launch if the passed arguments were declared by the launch description or not. So currently, the launch arguments are "automatically propagated" to nested launch files, though they aren't showed to the user.
  • Checking if all the launch arguments are satisfied when visiting a IncludeLaunchDescription action, including the arguments in nested launch descriptions, is hard, because some information may not be available at that moment:
    • launch file name is maybe not known.
    • name of passed launch arguments to nested IncludeLaunchDescription may not be known.
    • A SetLaunchConfiguration action may satisfy the requirement before the nested IncludeLaunchDescritpion is visited. If we do the verification anticipated, we can't be aware of that.
  • If IncludeLaunchDescription actions aren't scoped (as is currently happening), we can run in the following problem:
    def get_launch_description():
      return LaunchDescription([
        IncludeLaunchDescription(
          <LAUNCH_DESCRIPTION_SOURCE_1>,
          launch_arguments={'<ARG1_NAME>': '<ARG1_VALUE>'}.items()
        ),
        IncludeLaunchDescription(
          <LAUNCH_DESCRIPTION_SOURCE_1>
        )
      ])
    The intention is that the first include action should use the passed arguments, and the second the defaults (the included file is the same). But the second one will use the same arguments as the first one, because of the lack of scoping.
    There are a lot of others similar issues. In general, considering that the included launch description will possible be maintained by another person, defaulting to scoped would be better (and only using non-scoped include actions when the user consciously ask for it).

I think that one possible solution to this is just showing the arguments in a different way, but keeping everything else as-is:

  • List all arguments in the "first level" when doing ros2 launch -s <PACKAGE> <LAUNCH_FILE> (not showing arguments of nested launch descriptions). Add a ... at the end, or something similar, meaning that the arguments of included launch descriptions weren't showed.
  • List included launch descriptions when doing ros2 launch -s <PACKAGE> <LAUNCH_FILE>. In case a path can't be resolved, show a message notifying that to the user.
  • Each include launch description action is responsible of checking the satisfaction of it's arguments, but it's not responsible of checking if launch arguments of nested launch descriptions are satisfied.

The idea of this, is that all arguments that aren't re-declared, aren't directly visible to the user without digging in the documentation. But though not visible, they can still be specified by the user when running the launch file.

@ivanpauno ivanpauno added the enhancement New feature or request label Aug 16, 2019
@ivanpauno
Copy link
Member Author

@wjwwood What do you think about this?

@wjwwood
Copy link
Member

wjwwood commented Aug 23, 2019

Based on this issue, @ivanpauno, @hidmic, and I had an offline discussion. This is a summary of our conclusions:


We think that the ros2 launch command should:

  • Only show arguments from the "root" launch file (e.g. the one that is specified when doing ros2 launch ...), when using the ros2 launch --show-args ... option.
  • Not check that the arguments passed by the user will be used by the launch file, nor that all the arguments have been satisfied.

The first point basically means we don't need to automatically propagate declared arguments from launch files included in the "root" launch file.

However, we still will allow declared arguments in the "root" launch file to be "non-deterministic" or "dynamic", meaning that they might not be resolvable without running the launch file.
For example, they may contain a substitution in their name, or maybe they have a conditional associated with them (declaring arguments is an action and therefore may have a condition), or they may be a child of a conditional Action, like the GroupAction.

Therefore, we cannot check that the arguments given by the user match, exceed, or fall short of what is declared, as the second point says, without actually running the launch file.


Arguments given by the user will continue to be set as if the SetLaunchConfiguration action were used at the beginning of the "root" launch file.

For example, ros2 launch some.launch.py foo:=bar will always result in the launch configuration foo being set to bar, even if the launch file never defines a foo argument.


After discussing the ramifications of this, we wanted to address some use cases that were impossible or inconvenient.

First, since we're only considering the arguments declared in the "root" launch file, it becomes inconvenient if you need to include a complex launch file with many arguments in a simple "wrapper" launch file.
This would mean that you need to re-declare all the arguments in the "wrapper" launch file, and if you need to do this a few times it becomes very repetitive.

Therefore we'd like to have a way to "promote" any arguments in a launch file to the current launch file when including it.
This is not a required feature, but it would be much more convenient for some common cases where complex launch files are reused.
This deserves its own issue.


The second under served use case is having control over which launch configurations are available to an included launch file.
In ROS 1, the default behavior is that an included launch file does not have access to an "arg" unless you pass it explicitly when including it, and there is a way to opt-in to having all the args in the current launch file forwarded to the included one (see: ros/ros_comm#710).
We would like to get back to this with launch. Currently an included launch description has access to all configurations of the launch file that included it.
In order to have control over this behavior the include launch description action will need an option to not forward any launch configurations to the included launch file.

This also deserves its own issue.


Because we're still allowing substitutions in the name of declared arguments and conditionals, we still need some of the behaviors @ivanpauno proposed as part of the "best-effort" nature of --show-args.

This will also be needed when trying to implement the first convenience feature described above (promoting arguments from one launch file to the next).

@ivanpauno and @hidmic please add to this in case I missed anything.

@nuclearsandwich
Copy link
Member

Just to leave a breadcrumb, @ivanpauno removed this from the patch release queue since it's a feature request without anyone allocated to fulfill it. If work is done toward this feature we can still consider it for backport.

@ivanpauno ivanpauno added the help wanted Extra attention is needed label Jan 30, 2020
@v-lopez
Copy link
Contributor

v-lopez commented Jan 25, 2021

I apologise for bringing back an old issue, but I believe this would greatly improve usability.

In our use case we'd like to expose an easy to use launch file to spawn a robot in the gazebo simulator, while being able to specify all the launch arguments for gazebo as well as for the robot configuration. An example in ROS1 would be this; https://github.com/pal-robotics/tiago_simulation/blob/kinetic-devel/tiago_gazebo/launch/tiago_gazebo.launch#L4

The solution in ROS1 as discussed here is pass_all_args, but if you want to inspect the possible options you need to navigate manually the list of included launch files. To keep things modular we have MANY launch files, and it gets unmanageable fast.

I was under the impression that the describe/execute duality of the new launch system would be ideal for these situations.

The idea of this, is that all arguments that aren't re-declared, aren't directly visible to the user without digging in the documentation. But though not visible, they can still be specified by the user when running the launch file.

I would ask that there's a version of --show-args that also shows the non-root promoted launch file args.

If I understand correctly, there are some launch files that may not be known until execution? Could we have a best effort implementation ignoring those?

I'd like to contribute in any way possible, although I just have user level experience with the new launch system.

@hidmic
Copy link
Contributor

hidmic commented Jan 25, 2021

I'd like to contribute in any way possible, although I just have user level experience with the new launch system.

@v-lopez That's great! I'll forewarn you that this isn't trivial to solve though. Yes, launch allows one to introspect a description, but the latter may not be known in full until runtime (think of a launch file that includes another launch file based on a substitution). There's also something to be said about arguments' scope.

I'd suggest you submit a design document that outlines how you intend to solve which problem (the first ramification discussed by @wjwwood, if I'm not mistaken). There we can discuss the approach.

@wjwwood
Copy link
Member

wjwwood commented Jan 25, 2021

I agree with @hidmic, specifically this is the ramification he was talking about:

After discussing the ramifications of this, we wanted to address some use cases that were impossible or inconvenient.

First, since we're only considering the arguments declared in the "root" launch file, it becomes inconvenient if you need to include a complex launch file with many arguments in a simple "wrapper" launch file.
This would mean that you need to re-declare all the arguments in the "wrapper" launch file, and if you need to do this a few times it becomes very repetitive.

Therefore we'd like to have a way to "promote" any arguments in a launch file to the current launch file when including it.
This is not a required feature, but it would be much more convenient for some common cases where complex launch files are reused.
This deserves its own issue.

So the idea would be to have you (or anyone interested) describe how that promoting mechanism would work and how to handle "dynamic" include actions (I think a best effort solution is better than none).

@wjwwood
Copy link
Member

wjwwood commented Nov 20, 2021

I merged #556 which I don't think closes this, but it's worth mentioning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants