Avoid using ros_testing, and use launch_testing instead

[ivanpauno]

I figured out why I have some unexpected errors in ros2/rmw_fastrtps#312.
launch_testing_ros default launch description, creates a node that we weren't used. That node was created without specifying an rmw implementation, so fastrtps was used, and created problems when running connext tests.

Some of the failures were because:

• Discovery works different in the mentioned PR, and were expected to fail if nodes of different vendors were running and the same time.
• Connext printing

  PRESCstReaderCollator_storeSampleData:deserialize sample error in topic 'DISCParticipant' with type 'DISCParticipantParameter'
  

  That was only reproducible when using Fast-RTPS 1.9.x, and not 1.9.3.

I will merge this PR independently of https://github.com/ros2/rmw_fastrtps/pull/312, as the tests should continue working in master too.

[ivanpauno]

CI, up to affected ros2* packages:

• Linux

[hidmic]

This is fine as a workaround. BUT if you use launch_ros on a launch test, you should probably be tagging it as a rostest. Whether the default launch description is necessary or not is an implementation detail that could change in the future.

Ideally, I'd like ros2/launch_ros#120 to be addressed.

[wjwwood]

Discovery works different in the mentioned PR, and were expected to fail if nodes of different vendors were running and the same time.

That really sucks, ideally we'd avoid any issues when there just happens to be nodes from other vendors on the network. This goes beyond cross-vendor not working and strays into cross-vendor interferes with one another, which is way worse, imo.

Ideally, I'd like ros2/launch_ros#120 to be addressed.

But that doesn't resolve this right? It changes how the or when the internal node is created for launch, but it doesn't prevent it from happening. So you'd still have an internal launch node that would not be using the right rmw implementation, no?

[hidmic]

That really sucks, ideally we'd avoid any issues when there just happens to be nodes from other vendors on the network.

Agreed. FWIW interaction is mild though, stdout spam only.

So you'd still have an internal launch node that would not be using the right rmw implementation, no?

Correct, though not in this case as these tests do not make use of it. As to whether having launch_ros starting an internal node using a potentially different RMW implementation is a good idea, though I agree it's not, I think it's a separate issue entirely -- which IMHO is actually a launch_ros gotcha e.g. you shouldn't change the RMW_IMPLEMENTATION envvar for a lifecycle node you're about to launch either.

[ivanpauno]

That really sucks, ideally we'd avoid any issues when there just happens to be nodes from other vendors on the network. This goes beyond cross-vendor not working and strays into cross-vendor interferes with one another, which is way worse, imo.

I will try to solve the problem. The main issue is that we're using the participant name when we can't parse the userData qos:

https://github.com/ros2/rmw_connext/blob/ae7117ec9942953d159b6a25feb2bb0a0e17c857/rmw_connext_shared_cpp/src/node_names.cpp#L120-L135

The ros2node cli tests were ignoring the presence of launch_ros node and the ros2cli daemon node:

ros2cli/ros2node/test/test_cli.py

Line 103 in 96cf5b9

filtered_patterns=['.*launch_ros.*', '.*ros2cli.*'],

But now, the Participant running in the modified rmw_fastrtps_cpp launch_ros node doesn't have the same userData qos, and then it uses the participant name, which is different from the node name.
I think we can drop taking the participant name as node name in all the rmw implementations. That will solve the problem.

[wjwwood]

Correct, though not in this case as these tests do not make use of it.

I see, so it would workaround it in this case, but if this test used lifecycle nodes (or we were testing ros2lifecycle) it would still be an issue?

[ivanpauno]

I think this doesn't make more sense after ros2/launch_ros#128. Closing!