Conditional includes

[chapulina]

Bug report

Required Info:

• Operating System:
  • Ubuntu Bionic
• Installation type:
  • debs

Steps to reproduce issue

While working on ros-simulation/gazebo_ros_pkgs#962, we wanted to conditionally add the IncludeLaunchDescription action using the condition keyword. But that action is not accepting the keyword.

Here's a simple example with 2 files:

main.launch.py

from launch import Condition
from launch import LaunchDescription
from launch.actions import DeclareLaunchArgument
from launch.actions import ExecuteProcess
from launch.actions import IncludeLaunchDescription
from launch.conditions import IfCondition
from launch.launch_description_sources import PythonLaunchDescriptionSource
from launch.substitutions import LaunchConfiguration
from launch.substitutions import ThisLaunchFileDir

def generate_launch_description():
    return LaunchDescription([
        DeclareLaunchArgument('do', default_value='true',
                              description='false == dont'),
        ExecuteProcess(
            cmd=['echo',  'main'], output='screen',
            condition=IfCondition(LaunchConfiguration('do'))),

        IncludeLaunchDescription(
            PythonLaunchDescriptionSource([ThisLaunchFileDir(), '/included.launch.py']),
            condition=IfCondition(LaunchConfiguration('do'))
        ),
    ])

included.launch.py

from launch import LaunchDescription
from launch.actions import ExecuteProcess

def generate_launch_description():
    return LaunchDescription([
        ExecuteProcess(
            cmd=['echo',  'included'], output='screen')
    ])

Expected behavior

main.launch.py prints

[echo-1] main
[echo-2] included

main.launch.py do:=false prints nothing

Actual behavior

[ERROR] [launch]: Caught exception in launch (see debug for traceback): __init__() got an unexpected keyword argument 'condition'

Additional information

Commenting out the condition inside IncludeLaunchDescription lets the program run, but included is always printed.

[hidmic]

Yeap, that's a bug. Current implementation is not forwarding (nor taking) additional kwargs to its base class constructor.

[chapulina]

I haven't checked, but this may be also true for other actions. It may be a good idea to go over the arguments for all actions.

[sloretz]

Yeap, that's a bug. Current implementation is not forwarding (nor taking) additional kwargs to its base class constructor.

@chapulina and I tried making a subclass of IncludeLaunchDescription that forwarded args to the Action constructor, but more needs to be done. IncludeLaunchDescription overrides visit(...), which is what evaluates the condition in the base class.

[hidmic]

True that, though I don't see why it cannot override execute(...) instead. I wonder if this class was written before Action had any features. On a second look, it looks it uses visit() to dodge mypy... In any case, yes, it needs to be fixed at the source (as opposed to in a subclass).