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

IfCondition is ignored in ComposableNode if LoadComposableNodes is used #335

Closed
hodnajit opened this issue Dec 2, 2022 · 5 comments
Closed

Comments

@hodnajit
Copy link

hodnajit commented Dec 2, 2022

Bug report

Required Info:

  • Operating System:
    • ubuntu 22.04
  • Installation type:
    • binaries
  • Version or commit hash:
    • ros-rolling-launch (1.3.0-1jammy.20221102.202147)
  • DDS implementation:
    • rmw_cyclonedds_cpp
  • Client library (if applicable):
    • launch_ros

Steps to reproduce issue

Run example launchfile.

#!/usr/bin/env python3

from launch import LaunchDescription
from launch_ros.actions import Node
from launch.conditions import IfCondition
from launch.actions import DeclareLaunchArgument, GroupAction
from launch.substitutions import LaunchConfiguration, PythonExpression
from launch_ros.actions import LoadComposableNodes
from launch_ros.descriptions import ComposableNode


def generate_launch_description():
    show_image = LaunchConfiguration('show_image')

    declare_show_image_cmd = DeclareLaunchArgument(
        'show_image', default_value='False',
        description='Show image if True')

    use_composition = LaunchConfiguration('use_composition')

    declare_use_composition_cmd = DeclareLaunchArgument(
        'use_composition', default_value='True',
        description='Launch GroupAction if True')

    container = Node(
        condition=IfCondition(use_composition),
        name='image_container',
        package='rclcpp_components',
        executable='component_container',
        output='both',
    )

    load_composable_nodes = LoadComposableNodes(
        condition=IfCondition(use_composition),
        target_container='image_container',
        composable_node_descriptions=[
            ComposableNode(
                package='image_tools',
                plugin='image_tools::Cam2Image',
                name='cam2image',
                remappings=[('/image', '/burgerimage')],
                parameters=[{'width': 320, 'height': 240, 'burger_mode': True, 'history': 'keep_last'}],
                extra_arguments=[{'use_intra_process_comms': True}],
            ),
            ComposableNode(
                package='image_tools',
                plugin='image_tools::ShowImage',
                name='showimage',
                condition=IfCondition(show_image),
                remappings=[('/image', '/burgerimage')],
                parameters=[{'history': 'keep_last'}],
                extra_arguments=[{'use_intra_process_comms': True},]
            ),
        ],
    )

    # Create the launch description and populate
    ld = LaunchDescription()

    # Declare the launch options
    ld.add_action(declare_show_image_cmd)
    ld.add_action(declare_use_composition_cmd)

    # Add composable container
    ld.add_action(container)

    # Add the actions to launch of the nodes
    ld.add_action(load_composable_nodes)
    return ld

Expected behavior

The node /showimage is not launched.

Actual behavior

The node /showimage is launched.

@hodnajit hodnajit closed this as completed Dec 2, 2022
@hodnajit hodnajit reopened this Dec 2, 2022
@hodnajit
Copy link
Author

hodnajit commented Dec 2, 2022

Maybe better example (similar to this test)

condition=IfCondition(LaunchConfiguration('flag'))

Again, despite the IfCondition, all nodes are launched

/test_component_container_node_name
/test_component_container_node_name_1
/test_component_container_node_name_2

`#!/usr/bin/env python3

from launch import LaunchDescription
from launch_ros.actions import Node
from launch.conditions import IfCondition, UnlessCondition
from launch.actions import DeclareLaunchArgument, GroupAction
from launch.substitutions import LaunchConfiguration, PythonExpression
from launch_ros.actions import LoadComposableNodes, ComposableNodeContainer
from launch_ros.descriptions import ComposableNode

TEST_CONTAINER_NAME = 'test_component_container_node_name'
TEST_NODE_NAME = 'test_composable_node_name'
TEST_NODE_NAMESPACE = 'test_composable_node_namespace'

def generate_launch_description():
flag = LaunchConfiguration('flag')

TEST_NODE_NAME_1 = 'test_component_container_node_name_1'
TEST_NODE_NAME_2 = 'test_component_container_node_name_2'

declare_flag_cmd = DeclareLaunchArgument(name='flag', default_value='False')

container = Node(
    name=TEST_CONTAINER_NAME,
    package='rclcpp_components',
    executable='component_container',
    output='both',
)

load_composable_nodes = LoadComposableNodes(
    target_container=TEST_CONTAINER_NAME,
    composable_node_descriptions=[
        ComposableNode(
            package='composition',
            plugin='composition::Listener',
            name=TEST_NODE_NAME_1,
            condition=UnlessCondition(flag)
        ),
        ComposableNode(
            package='composition',
            plugin='composition::Listener',
            name=TEST_NODE_NAME_2,
            condition=IfCondition(flag)
        )
    ],
)

# Create the launch description and populate
ld = LaunchDescription()

# Declare the launch options
ld.add_action(declare_flag_cmd)

# Add composable container
ld.add_action(container)

# Add the actions to launch of the nodes
ld.add_action(load_composable_nodes)

return ld

`

@hodnajit hodnajit changed the title IfCondition is ignored in ComposableNode IfCondition is ignored in ComposableNode if LoadComposableNodes is used Dec 2, 2022
@methylDragon
Copy link
Contributor

methylDragon commented Dec 7, 2022

Reformatting for convenience

#!/usr/bin/env python3

from launch import LaunchDescription
from launch_ros.actions import Node
from launch.conditions import IfCondition, UnlessCondition
from launch.actions import DeclareLaunchArgument, GroupAction
from launch.substitutions import LaunchConfiguration, PythonExpression
from launch_ros.actions import LoadComposableNodes, ComposableNodeContainer
from launch_ros.descriptions import ComposableNode

TEST_CONTAINER_NAME = 'test_component_container_node_name'
TEST_NODE_NAME = 'test_composable_node_name'
TEST_NODE_NAMESPACE = 'test_composable_node_namespace'

def generate_launch_description():
    flag = LaunchConfiguration('flag')

    TEST_NODE_NAME_1 = 'test_component_container_node_name_1'
    TEST_NODE_NAME_2 = 'test_component_container_node_name_2'

    declare_flag_cmd = DeclareLaunchArgument(name='flag', default_value='False')

    container = Node(
        name=TEST_CONTAINER_NAME,
        package='rclcpp_components',
        executable='component_container',
        output='both',
    )

    load_composable_nodes = LoadComposableNodes(
        target_container=TEST_CONTAINER_NAME,
        composable_node_descriptions=[
            ComposableNode(
                package='composition',
                plugin='composition::Listener',
                name=TEST_NODE_NAME_1,
                condition=UnlessCondition(flag)
            ),
            ComposableNode(
                package='composition',
                plugin='composition::Listener',
                name=TEST_NODE_NAME_2,
                condition=IfCondition(flag)
            )
        ],
    )

    # Create the launch description and populate
    ld = LaunchDescription()

    # Declare the launch options
    ld.add_action(declare_flag_cmd)

    # Add composable container
    ld.add_action(container)

    # Add the actions to launch of the nodes
    ld.add_action(load_composable_nodes)

    return ld

@methylDragon
Copy link
Contributor

Gotcha, I can reproduce this issue, now to look into it...

@methylDragon
Copy link
Contributor

methylDragon commented Dec 8, 2022

Hello! This PR should address the issue (:

Do note that it'll take a little while for this change to propagate through and get released (the PR will need to get merged, then the repo has to get released, and finally there has to be a rolling sync.) So I'd recommend building from source if you want the change immediately.
@hodnajit

@hodnajit
Copy link
Author

hodnajit commented Dec 8, 2022

Thank you :) great job!

@hodnajit hodnajit closed this as completed Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants