talker_listener_launch_test example is broken about launch_ros

[jacobperron]

See #112 for a fix of the example.

Longer term, I think we need to remove the injection from the CLI tool, or be able to detect that the preamble has already been included to avoid including it more than once.

[teutobod]

Any updates here? I'm stuck to the same issue.

[jacobperron]

I can reproduce the issue.

Interestingly, if you run the test with the CLI tool ros2 test, then there is no error:

ros2 test src/ros2/launch_ros/launch_testing_ros/test/examples/talker_listener_launch_test.py

[jacobperron]

I guess it kind of makes sense, since the ROS node is specific to launch_ros and not launch; the node is started by the default launch_ros description here:

launch_ros/launch_ros/launch_ros/default_launch_description.py

Lines 80 to 92 in c37340d

	def get_default_launch_description(*, rclpy_context=None):
	"""
	Return a LaunchDescription to be included before user descriptions.
	:param: rclpy_context Provide a context other than the default rclpy context
	to pass down to rclpy.init.
	The context is expected to have already been initialized by the caller
	using rclpy.init().
	"""
	default_ros_launch_description = launch.LaunchDescription([
	# ROS initialization (create node and other stuff).
	ROSSpecificLaunchStartup(rclpy_context=rclpy_context)
	])

I think either the README should be updated to use ros2 test, or the example launch file should be updated to use the get_default_launch_description.

[jacobperron]

Updating the launch file fixes the issue for launching with launch_test, e.g.:

    ld = launch_ros.get_default_launch_description()
    ld.add_action(talker_node)
    ld.add_action(listener_node)
    # Start tests right away - no need to wait for anything
    ld.add_action(launch_testing.actions.ReadyToTest())

But, then two nodes are created if we run with ros2 test and we get the following warning:

[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.

Seems like a different approach is required.

[hidmic]

There's no good solution here really, besides not mixing up launch_test and ros2 test. The former just runs the test, the latter injects launch_ros default launch description. There was a discussion about this in ros2/launch#208. I still think tools should not inject anything, but consensus back then was to reduce boilerplate as much as possible.

[stonier]

Got burned by this last night as well.

+1 for removing the injection. Reasoning:

• Prefer to have one launch test file format, not two (e.g. the current launch_test file is broken with launch_test, which makes it something else).
• Prefer not to depend on user friendly tooling (i.e. ros2 test). There already is an execution framework (i.e. launch_test) so ros2 test should be there to provide conveniences, not dependencies.

[jacobperron]

I've fixed the example in #121 and opened a ticket to discuss removing the injected launch description: #120