ros-perception / depthimage_to_laserscan

Converts a depth image to a laser scan for use with navigation and localization.
253 stars 170 forks source link

[ROS2 Humble] Error while using IPC in composition #75

Open Myzhar opened 10 months ago

Myzhar commented 10 months ago

When I try to enable IPC in composition I get this error: Component constructor threw an exception: intraprocess communication is not allowed with a zero qos history depth value

The default QoS settings for the depthimage_to_laserscan component are not good for using IPC.

Example:

# ZED Wrapper component
    zed_wrapper_component = ComposableNode(
        package='zed_components',
        namespace=camera_name_val,
        plugin='stereolabs::ZedCamera',
        name=zed_node_name,
        parameters=[
            # YAML files
            config_common_path,  # Common parameters
            config_camera_path,  # Camera related parameters
            # Overriding
            {
                'general.camera_name': camera_name_val,
                'general.camera_model': camera_model_val,
                'general.svo_file': svo_path,
                'general.serial_number': serial_number
            }
        ],
        extra_arguments=[{'use_intra_process_comms': True}]
    )

    # Depth to Laser scan component
    zed_cvt_component = ComposableNode(
        package='depthimage_to_laserscan',
        namespace=camera_name_val,
        plugin='depthimage_to_laserscan::DepthImageToLaserScanROS',
        name='depthimage_to_laserscan',
        parameters=[
            config_path_cvt,
            # Overriding
            {
                'output_frame': camera_depth_frame,
                'qos_overrides./parameter_events.publisher.depth': 5
            }],
        remappings=[
                ('depth', zed_node_name_val + '/depth/depth_registered'),
                ('depth_camera_info', zed_node_name_val + '/depth/camera_info')
            ],
        extra_arguments=[{'use_intra_process_comms': True}]
    )

    # ROS 2 Component Container
    container = ComposableNodeContainer(
            name='zed_depth_to_laserscan',
            namespace=camera_name_val,
            package='rclcpp_components',
            executable='component_container',
            composable_node_descriptions=[
                zed_wrapper_component,
                zed_cvt_component
            ],
            output='screen',
    )
Myzhar commented 10 months ago

Read mode on Stack Exchange.

clalancette commented 10 months ago

Based on some internal discussion as well as the Stack Exchange thread, I think the solution here is to switch away from SystemDefaultsQoS here in depthimage_to_laserscan, and to enable qos_overrides. We still have a bug down in rclcpp where intraprocess comms should not throw this error, but depthimage_to_laserscan should also not be using SystemDefaultsQoS.

@Myzhar If you'd like to open a PR to do that, I'd be happy to review it.

Myzhar commented 10 months ago

@clalancette I will try to work on this during the weekend. :+1:

clalancette commented 10 months ago

@clalancette I will try to work on this during the weekend. 👍

Fantastic, thank you!

Myzhar commented 10 months ago

@clalancette I tested the fix with my launch file and I confirm that now I can enable the IPC option also for the depthimage_to_laserscan node component by uncommenting the launch file line #extra_arguments=[{'use_intra_process_comms': True}] # Uncomment when supported by the package

I am not sure if there is a way to effectively test whether IPC is enabled and being used correctly.

clalancette commented 10 months ago

I am not sure if there is a way to effectively test whether IPC is enabled and being used correctly.

Yeah, we should be able to add launch tests to the tests. I'll comment on the PR.

Myzhar commented 10 months ago

Is there a command to get the status of IPC while a node is running? I'd like to add it to diagnostic.

clalancette commented 10 months ago

Is there a command to get the status of IPC while a node is running? I'd like to add it to diagnostic.

I'm not sure, to be honest.