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

YAML config: Substitution happens for comments #1317

Open
akorb opened this issue Aug 19, 2022 · 5 comments · May be fixed by ros2/launch_ros#322
Open

YAML config: Substitution happens for comments #1317

akorb opened this issue Aug 19, 2022 · 5 comments · May be fixed by ros2/launch_ros#322
Assignees
Labels
bug Something isn't working

Comments

@akorb
Copy link

akorb commented Aug 19, 2022

Bug report

Required Info:

  • Operating System:
    • Ubuntu 20.04 LTS
  • Installation type:
    • Binary
  • Version or commit hash:
    • galactic
  • DDS implementation:
    • Eclipse Cyclone DDS

Steps to reproduce issue

Add a comment like this to any configuration yaml file that is used:

    # $(env NON_EXISTING_ENV_VAR)

and add allow_subst="true" to its referencing param tag:

    <param from="my_config.yaml" allow_substs="true"/>

Expected behavior

The comment should be ignored and it should work as usually.

Actual behavior

Ros tries to resolve the environment variable and crashes if it does not exist, even though it is commented out.

@clalancette
Copy link
Contributor

Can you give a complete example of the problem?

From the short snippet you've provided, it looks like you are using XML launch files. In XML # is not the comment character; <!-- is. So it's not commented out.

@clalancette clalancette added the more-information-needed Further information is required label Aug 19, 2022
@akorb
Copy link
Author

akorb commented Aug 19, 2022

I see. The comment is meant to be put into a yaml configuration file (my_config.yaml). For example:

/**:
  ros__parameters:
    parameterA: 5

    # $(env NON_EXISTING_ENV_VAR)

    parameterB: true

And in the launch file allow_substs has to be set to true, otherwise the $(env ...) is not resolved:

<launch>
  <node pkg="my_node" exec="my_node" name="my_node">
    <param from="my_config.yaml" allow_substs="true"/>
  </node>
</launch>

@clalancette
Copy link
Contributor

OK, I see. Thanks for the updated example, that helps a lot.

This is indeed a bug, and I can reproduce it locally on Rolling with the following:

turtlesim.yaml:

/turtlesim:
  ros__parameters:

    # $(env NON_EXISTING_ENV_VAR)

    background_b: 0
    background_g: 86
    background_r: 69
    holonomic: false
    qos_overrides:
      /parameter_events:
        publisher:
          depth: 1000
          durability: volatile
          history: keep_last
          reliability: reliable
    use_sim_time: false

turtlesim-launch.xml

<launch>
  <node pkg="turtlesim" exec="turtlesim_node" name="turtlesim">
    <param from="turtlesim.yaml" allow_substs="true"/>
  </node>
</launch>

@ivanpauno Since you did the initial substitution work, can you take a look?

@clalancette clalancette added bug Something isn't working and removed more-information-needed Further information is required labels Aug 19, 2022
@ivanpauno
Copy link
Member

Yes, I can take a look, it seems a bit tricky ...

@ivanpauno
Copy link
Member

I'm thinking than loading the file as yaml and dumping it will solve the issue, but it may not work if the original file is not a valid yaml file (which currently was possible).
Anyway, I think it should work for most use cases.
@hidmic do you have any other alternative?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants