Add a warning to ros2 node info when there is more than one node with the same name

[emersonknapp]

Second half of #453
Second half of https://github.com/ros-tooling/aws-roadmap/issues/196

Add a simple warning for ros2 node info when there is more than one node with the same name. This does not change functionality at all, just informs users of a potentially confusing situation.

Signed-off-by: Emerson Knapp emerson.b.knapp@gmail.com

[emersonknapp]

@chapulina ping for routing to the correct reviewers

[hidmic]

LGTM, but for a few minor nits. It'd be nice to have a test for it though :)

[emersonknapp]

@hidmic I've added a test to expect this warning line output in a launch scenario with this node. I was thinking of adding an inverse test, "this line is not present when running against a normal node" - but I couldn't think of how to do that "expectation of absence" - any ideas? Also, maybe it's not necessary because the existing tests handle the regular-case scenario. What do you think?

[emersonknapp]

@hidmic can you review this?

[hidmic]

LGTM pending green CI ! Apologies for the rather long round-trip.

On testing for absence, considering it's a single text line you want to check for, simply negating expect_output(..., strict=False) outcome will do the trick.

[emersonknapp]

@ros2/aws-oncall - please run this CI job
Gist: https://gist.githubusercontent.com/emersonknapp/3f56000a8f88a272334b7c4c2e53828d/raw/cecff37a502392f6efb96f404c5bdd96b31a2ad2/ros2.repos
BUILD args: --packages-up-to ros2node
TEST args: --packages-select ros2node
Job: ci_launcher

[emersonknapp]

• Linux
• Linux-aarch64
• macOS
• Windows

[emersonknapp]

Restarting after updating branch

• Linux
• Linux-aarch64
• macOS
• Windows

[emersonknapp]

I've just learned that rmw_cyclonedds_cpp uses a std::set<> under the hood to return node names, so it will not return a list with multiple nodes of the same name, therefore these checks don't help us in CycloneDDS - @hidmic do you have thoughts on this? I'm wondering if I should go in and change the rmw_cyclonedds_cpp implementation to use a std::vector instead, or if we should disable these tests for Cylone, or a third option (don't print this warning?)

[emersonknapp]

After updating rmw_cyclonedds to provide nodes with duplicate names

• Linux
• Linux-aarch64
• macOS
• Windows

[wjwwood]

one nitpick, otherwise lgtm

[hidmic]

Alright, looking good! Thanks for your contribution @emersonknapp, merging.