ros2 / launch

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

Bug or feature? PushRosNamespace is not forwarded to managed actions #743

Closed oKermorgant closed 10 months ago

oKermorgant commented 10 months ago

Hi,

It appears that actions that are somehow managed (not added to the LaunchDescription explicitly but through some event or composition) do not inherit properties of their manager.

Here is an example, where

The first listener gets the topic fine while the other one is run outside the namespace of its own EventHandler.

It may not be a bug but the logic is somehow hard to process.

from launch_ros.actions import Node

from launch import LaunchDescription
from launch.actions import RegisterEventHandler
from launch.event_handlers import OnProcessStart
from launch.actions import GroupAction
from launch_ros.actions import PushRosNamespace

def generate_launch_description():

    talker_ns = 'some_ns'

    # runs in namespace
    talker = Node(package='demo_nodes_cpp', executable='talker', name='talker',
                  namespace = talker_ns)

    event = OnProcessStart(target_action=talker,
                           on_start=[Node(package='demo_nodes_cpp', executable='listener',
                                          name = 'handled_listener')])

    handler = RegisterEventHandler(event)

    # handler runs in namespace but resulting listener will not
    handled_listener = GroupAction([PushRosNamespace(talker_ns), handler])

    # runs in namespace
    base_listener = GroupAction([PushRosNamespace(talker_ns), Node(package='demo_nodes_cpp', executable='listener',name='base_listener')])

    return LaunchDescription([talker, handled_listener, base_listener])
ros-discourse commented 10 months ago

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/simple-launch-approaching-event-handling/34715/1

moriarty commented 10 months ago

This was discussed in a meeting and is believed to be working as expected, even if not ergonomic. The group is being lazily evaluated and closed by time launch happens.

As a quick work around you could:

from launch_ros.actions import Node

from launch import LaunchDescription
from launch.actions import RegisterEventHandler
from launch.event_handlers import OnProcessStart
from launch.actions import GroupAction
from launch_ros.actions import PushRosNamespace

def generate_launch_description():

    talker_ns = 'some_ns'

    # runs in namespace
    talker = Node(package='demo_nodes_cpp', executable='talker', name='talker',
                  namespace = talker_ns)

    event = OnProcessStart(target_action=talker,
                           on_start=[Node(package='demo_nodes_cpp', executable='listener',
-                                          name = 'handled_listener')])
+                                          name = 'handled_listener', namespace = talker_ns)])

    handler = RegisterEventHandler(event)

    # handler runs in namespace but resulting listener will not
    handled_listener = GroupAction([PushRosNamespace(talker_ns), handler])

    # runs in namespace
    base_listener = GroupAction([PushRosNamespace(talker_ns), Node(package='demo_nodes_cpp', executable='listener',name='base_listener')])

    return LaunchDescription([talker, handled_listener, base_listener])

Original code example:

alex@alex-servalws:~/ros/r/examples$ ros2 node list
/handled_listener
/some_ns/base_listener
/some_ns/talker

With diff adding namepace arg to second handled listener

alex@alex-servalws:~/ros/r/examples$ ros2 node list
/some_ns/base_listener
/some_ns/handled_listener
/some_ns/talker

But also if this is for classroom use for simple cases of launching nodes you could try the XML launch file support, as I mentioned in the discourse thread.

clalancette commented 10 months ago

This was discussed in a meeting and is believed to be working as intended.

Yes, agreed.

Just to add a bit more context, these groups are evaluated lazily, but at the time they are needed. So in the original example, the GroupAction used in handled_listener is only open for as long as it takes to register the event. The event might be handled much, much later, at which point the group isn't around any more.

Besides explicitly adding in the namespace as @moriarty showed, you could also use the GroupAction in the on_start parameter to the OnProcessStart handler.

oKermorgant commented 10 months ago

Thanks, I get that the namespace of the handled node has to be explicit, either through a namespace argument or a GroupAction.