[launch] unexpected behaviour setting node_name on a multi-node process

[stonier]

Bug report

Required Info:

• Operating System:
  • Ubuntu 18.04
• Installation type:
  • binaries
• Version or commit hash:
  • crystal
• DDS implementation:
  • default
• Client library (if applicable):
  • rclpy

Steps to reproduce issue

Multi-node rclpy process (installs ros2 runnable testies):

#!/usr/bin/env python3
# -*- coding: utf-8 -*-

import rclpy
import rclpy.executors

def foo():
    print("foo")

def main():
    rclpy.init()
    foo_node = rclpy.create_node(node_name="foo")
    bar_node = rclpy.create_node(node_name="bar")
    timer = foo_node.create_timer(timer_period_sec=0.5, callback=foo)
    executor = rclpy.executors.SingleThreadedExecutor()
    executor.add_node(foo_node)
    executor.add_node(bar_node)
    try:
        executor.spin()
    except KeyboardInterrupt:
        print("Keyboard Interrupt")
    timer.cancel()
    foo_node.destroy_timer(timer)
    foo_node.destroy_node()
    bar_node.destroy_node()
    rclpy.shutdown()

And launcher:

#!/usr/bin/env python3
# -*- coding: utf-8 -*-

import launch
import launch_ros.actions

def main():
    launch_description = launch.LaunchDescription()
    launch_description.add_action(
        launch_ros.actions.Node(
            package='py_trees_ros_tutorials',
            node_name="one",
            node_executable="testies",
            output='screen',
        )
    )
    ls = launch.LaunchService()
    ls.include_launch_description(
        launch_ros.get_default_launch_description(
            prefix_output_with_name=False
        )
    )
    ls.include_launch_description(launch_description)
    return ls.run()

Expected behavior

With node_name="one" commented out in the launcher, two nodes appear: foo and bar. This works fine.

With node_name="one" uncommented, then I'd expect termination, some meaningful error message (not warning), even better, a hint directing the user to the correct way to override node names in a multi-node process.

Actual behavior

In the latter case, surprise! Everything launches, but both nodes foo and bar get set to one

$ ros2 node list
/launch_ros
/one
/one

with the following warning message.

[WARN] [rcl.logging_rosout]: Publisher already registered for provided node name. If this is due to 
multiple nodes with the same name then all logs for that logger name will go out over the existing 
publisher. As soon as any node with that name is destructed it will unregister the publisher, preventing 
any further logs for that name from being published on the rosout topic.

Additional Behaviour

Is there a case for setting all node names in a process to the launch defined override? If so, then this is probably not the path to take regardless.

[stonier]

FYI @IanTheEngineer

[sloretz]

linking ros2/rcl#375 for context

[wjwwood]

I believe this is the expected behavior when overriding the node name using the command line in a multi-node process.

Launch cannot know if the executable you're running is multi-node or not, and therefore cannot do anything about it.

See also: https://answers.ros.org/question/316870/ros2-composition-and-node-names-with-launch-files/?answer=316925#post-id-316925

[stonier]

Nice informative article on ros answers, thanks.

Couldn't launch do something about it in the same way that the command line overrides do? e.g. an api like:

launch_description.add_action(
        launch_ros.actions.MultiNode(
            package='my_package',
            node_name_remappings={"my_node1": "foo", "my_node2": "bar" },
            node_executable="testies",
            output='screen',
        )
    )

MultiNode could be for all intensive purposes, the same as Node sans the way it creates the command line arguments for the process.

[wjwwood]

Having an action for multi node processes is exactly what is needed, but it also relies on separating node description and arguments (remapping, parameters, etc) and the process to be executed.

See:

• Separate "executable" from "execute": #114
• Introduce idea of "Composable Node", probably should inherit from "Node" which can be given to "ExecuteNode": ros2/launch_ros#9

[stonier]

Aye, lots more than just node name remapping. I cherry-picked that just to keep the example simple.

My use case actually doesn't lend itself to Composable Nodes. I have independently built classes that are instantiated and required to work together via their programming (not middleware) api in an application. The nodes are there for communicating to external processes.

[wjwwood]

My use case actually doesn't lend itself to Composable Nodes.

Sure, I just meant that like ComposableNode's we need to separate the description of a node from the execution of them. Which composable nodes inherently do, but currently the existing "Node actions" (multi- node or not) do not.

I talk about this in my most recent ROSCon talk:

https://roscon.ros.org/2018/presentations/ROSCon2018_launch.pdf (slide number 7)

But it has unfortunately yet to make it to reality.

[wjwwood]

Since there is #114 (separate the idea of Nodes and Processes) and ros2/launch_ros#7, and also because we have the linked ROS answer, I'm going to close this.