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

LoadComposableNodes action blocks previous actions #107

Closed
jacobperron opened this issue Dec 14, 2019 · 0 comments · Fixed by #113
Closed

LoadComposableNodes action blocks previous actions #107

jacobperron opened this issue Dec 14, 2019 · 0 comments · Fixed by #113
Assignees
Labels
bug Something isn't working
Projects

Comments

@jacobperron
Copy link
Member

jacobperron commented Dec 14, 2019

Bug report

In particular, it would be really convenient to be able to start a node container just before some actions to load components into it. Maybe not a bug, but certainly a big usability improvement.

Required Info:

  • Operating System:
    • Ubuntu 18.04
  • Installation type:
    • source
  • Version or commit hash:
  • DDS implementation:
    • Fast-RTPS

Steps to reproduce issue

Launch the following launch file:

from launch import LaunchDescription
from launch_ros.actions import ComposableNodeContainer
from launch_ros.actions import LoadComposableNodes
from launch_ros.descriptions import ComposableNode


def generate_launch_description():
    return LaunchDescription([
        ComposableNodeContainer(
           package='rclcpp_components', node_executable='component_container',
           node_name='my_container', node_namespace=''
        ),  
        LoadComposableNodes(
            composable_node_descriptions=[
                ComposableNode(
                    package='composition',
                    node_plugin='composition::Talker'
                )
            ],  
            target_container='my_container'
        ),  
    ])

Expected behavior

A container node starts and the composition talker component is loaded.

Actual behavior

The container does not start and so the LoadComposableNodes

Additional information

I understand that the LoadComposableNodes action is blocking until the service is available. But my intuition says that since the ComposableNodeContainer action appears first, it should be started before the loading action starts.

After talking with @wjwwood, it sounds like we should update the implementation of LoadComposableNodes so it waits for the service in a separate thread so that it doesn't block the asyncio loop.

@wjwwood wjwwood added the bug Something isn't working label Jan 2, 2020
@wjwwood wjwwood added this to To do in Foxy via automation Jan 2, 2020
jacobperron added a commit that referenced this issue Jan 16, 2020
Fixes #107.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron self-assigned this Jan 16, 2020
Foxy automation moved this from To do to Done Jan 30, 2020
jacobperron added a commit that referenced this issue Jan 30, 2020
…#113)

Fixes #107.

Before, the first call to _load_in_sequence was not asynchronous, unlike it's recursive calls.
This change makes the first call asynchronous.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this issue Mar 12, 2020
…#113)

Fixes #107.

Before, the first call to _load_in_sequence was not asynchronous, unlike it's recursive calls.
This change makes the first call asynchronous.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this issue Mar 12, 2020
…#113)

Fixes #107.

Before, the first call to _load_in_sequence was not asynchronous, unlike it's recursive calls.
This change makes the first call asynchronous.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
nuclearsandwich pushed a commit that referenced this issue Mar 13, 2020
Fixes #107.

Before, the first call to _load_in_sequence was not asynchronous, unlike it's recursive calls.
This change makes the first call asynchronous.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this issue Mar 17, 2020
…#113) (#132)

Fixes #107.

Before, the first call to _load_in_sequence was not asynchronous, unlike it's recursive calls.
This change makes the first call asynchronous.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
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
No open projects
Foxy
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants