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

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

Closed
oKermorgant opened this issue Nov 18, 2023 · 4 comments
Closed

Comments

@oKermorgant
Copy link

oKermorgant commented Nov 18, 2023

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

  • a talker is explicitly run in a namespace
  • a listener is also run in a namespace, through a GroupAction
  • another listener is run through an OnProcessStart event which is itself inside a GroupAction pushing the correct namespace

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
Copy link

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
Copy link

moriarty commented Nov 30, 2023

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
Copy link
Contributor

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
Copy link
Author

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

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

4 participants