ros2 / launch

Tools for launching multiple processes and for writing tests involving multiple processes.
Apache License 2.0
126 stars 142 forks source link

LaunchConfigurationNotEquals() always returns `True` #509

Closed hmellor closed 3 years ago

hmellor commented 3 years ago

Bug report

Required Info:

Steps to reproduce issue

I am trying to include an external launch file in a new launch file, but only if an argument in the new launch file is set. I have tried to do this using the condition kwarg where the specified condition evaluates to False if the value of arg is the default "". However, the condition always evaluates to True.

package_dir = ament_index_python.packages.get_package_share_directory("package")

declare_ncom = launch.actions.DeclareLaunchArgument("arg", default_value="")
arg = launch.substitutions.LaunchConfiguration("arg", default="")

launch_description = launch.launch_description_sources.PythonLaunchDescriptionSource(f"{package_dir}/launch/launch.py")
package_launch = launch.actions.IncludeLaunchDescription(
    condition=launch.conditions.LaunchConfigurationNotEquals(arg, ""), 
    launch_description_source=launch_description,
    launch_arguments={"arg": arg}.items(),
)

Expected behavior

If the arg has not been set, then package_launch's condition should evaluate to False and the launch.py should not run from the new launch file.

Actual behavior

launch.py always runs.

hidmic commented 3 years ago

@HMellor LaunchConfigurationNotEquals takes the launch configuration name, not the object itself.

Having said that, I'm open to supporting that OR improving documentation to make it clearer. Would you be willing to do the latter?

hmellor commented 3 years ago

Ah ok, the type hinting for the argument was SomeSubstitutionsType, which is non-specific and open to being misinterpreted (such as I have done here). If it's just the docstring I'd be happy to make a pull request to clarify the argument. Have I understood you correctly @hidmic?

hidmic commented 3 years ago

If it's just the docstring I'd be happy to make a pull request to clarify the argument. Have I understood you correctly @hidmic?

Yes, exactly.