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

Support for conditional parameter import from yaml file in XML launch file #323

Open
Flova opened this issue Oct 30, 2021 · 11 comments
Open
Assignees
Labels
help wanted Extra attention is needed

Comments

@Flova
Copy link

Flova commented Oct 30, 2021

Bug report

Conditional loading of parameters using the <param> tag in the XML launch files is not possible at the moment, because the said tag does not support the if or unless argument as described in https://design.ros2.org/articles/roslaunch_xml.html.
In ROS1 the <rosparam> and <param> tags supported conditional loading using if="", this enabled e.g. overwriting of some parameters if the launch argument sim was set. Now that the loading was moved from <rosparam> to <param> it is missing this feature.

  • Operating System:
    • Ubuntu 20.04
  • Installation type:
    • binaries
  • Version or commit hash:
    • 0.10.6-1focal.20210901.033438 (foxy)
  • DDS implementation:
    • standard

Steps to reproduce issue

<arg name="dummy" default="false" />
<arg name="sim" default="false" />

 <node pkg="foo_bar" exec="foo" name="foo_bar" output="screen">
         <!-- Set default config -->
        <param from="$(find-pkg-share foo_bar)/config/params.yaml" />

        <!-- Start foo different setting-->
        <param name="asdf" value="dummy" if="$(var dummy)"/>

        <!-- Load special sim parameters-->
        <param from="$(find-pkg-share foo_bar)/config/simparams.yaml" if="$(var sim)"/>
    </node>

Launch a node with the launch file above and it will include all parameters even if sim and dummy are false.

Expected behavior

I would expect one of the following behaviors, even tho I am preferring the first one.

  1. The condition is recognized and the params is are not loaded
  2. The parser throws an error stating that the if argument is not supported in this context. The same happens for other similar errors (e.g. arg instead of var).

Actual behavior

  1. It ignores the if.
@adityapande-1995
Copy link
Contributor

adityapande-1995 commented Dec 8, 2021

Launch throws an error in rolling :
launch.invalid_launch_file_error.InvalidLaunchFileError: Caught exception when trying to load file of format [launch]: Unexpected attribute(s) found in `param`: {'if'}

To get this on foxy, need to backport this PR that added the validation to frontend : ros2/launch#468
and this one that adds the validation to node action in launch: #198

Opened backport PRs:
ros2/launch#567
#293

@Flova
Copy link
Author

Flova commented Dec 8, 2021

Okay it is definitely better to have this helpful error message.

This being said, what are the arguments against the a conditional parameter, as it was supported in ROS 1 and there is no way to do this in ROS 2 in an elegant way.
One could manually load and set the parameters inside the node depending on a launch parameter, but that would not be very clean, as each node would implement ist parsing and loading. Are there any architectural constraints that limit this functionality? Afaik there are conditions in the launch files for e.g. the set env or include tags.

@hidmic
Copy link
Contributor

hidmic commented Dec 8, 2021

@Flova conditions apply to Action instances (and to their markup equivalents). Both include and set_env tags (and even set_parameter tags) represent actions, whereas param tags nested in a node tag represent a dictionary.

For this feature to work, conditions would have to apply to all LaunchDescriptionEntity instances and parameters would have to become such entities (instead of plain types in a plain dictionary). It is doable, though I haven't thought through what the entire change would look like. Contributions are welcome.

@adityapande-1995
Copy link
Contributor

adityapande-1995 commented Jan 10, 2022

One backport PR is blocked here : #293 (comment). Working on it

@adityapande-1995
Copy link
Contributor

@Flova
Copy link
Author

Flova commented Jan 14, 2022

Could we leave this issue open as a feature request for conditional loading of parameters? Or should I create a new one?

@jacobperron
Copy link
Member

Re-opening since the feature isn't supported.

@jacobperron jacobperron reopened this Feb 22, 2022
@jacobperron
Copy link
Member

Note that the backport ros2/launch#567 inadvertently broke behavior of front-end launch files (reported in ros2/ros2#1244). I'm considering reverting the backports. Although the error messages are nice to have, I think the break in behavior was not worth it.

@adityakamath
Copy link

Is there no update on this yet? It still does not work, I'm trying it on Galactic?

It would also be nice to know the reasoning behind deprecating this feature for ROS2, there also seems to be nothing mentioned in the ROS2 design documentation.

@Flova
Copy link
Author

Flova commented Aug 19, 2022

Sadly, this is still missing in ros2. But I also didn't have the resources to implement it myself yet.

@adityakamath
Copy link

Thanks for the update @Flova. I eventually moved to using python launch files, where it is possible.

@clalancette clalancette added the help wanted Extra attention is needed label Aug 31, 2022
@jacobperron jacobperron transferred this issue from ros2/launch Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants