Remap two nodes of one executable file with same node name in different namespaces

[iuhilnehc-ynos]

Bug report

Required Info:

• Operating System:
  • Ubuntu 20.04
• Installation type:
  • source
• Version or commit hash:
  • 8bcada7b884693644d7ba2e3bd2c22fd9541eba7
• DDS implementation:
  • Fast-RTPS
• Client library (if applicable):
  • rclcpp

Steps to reproduce issue

• use Order of Applying Remapping Rules

$ ros2 run examples_rclcpp_minimal_composition composition_composed \
    --ros-args \
    -r publisher_node:__node:=aaa \
    -r aaa:__ns:=/ns1 \                                     // Must use aaa because of current `Order of Applying Remapping Rules`
    -r subscriber_node:__node:=aaa \
    -r aaa:__ns:=/ns2                                       // bad

Expected behavior

$ ros2 run examples_rclcpp_minimal_composition composition_composed \
    --ros-args \
    -r publisher_node:__ns:=/ns1 \
    -r publisher_node:__node:=aaa \
    -r subscriber_node:__ns:=/ns2 \
    -r subscriber_node:__node:=aaa

$ ros2 node list
/ns1/aaa
/ns2/aaa

Actual behavior

$ ros2 node list
/ns1/aaa
/ns1/aaa

Additional information

Remapping rules are applied in the following order:

• 1. Node name remapping
• 2. Namespace remapping // Should namespace remapping be the first one?
• 3. All other rules

[iuhilnehc-ynos]

I know the old usage in user applications must be updated after this fix. such as updating from

    -r publisher_node:__node:=aaa \
    -r aaa:__ns:=/ns1                                     // aaa is not effective.

to

    -r publisher_node:__node:=aaa \
    -r publisher_node:__ns:=/ns1

but I think using this order of remapping rules that remapping namespace before node name seems no limitation.

[fujitatomoya]

@iuhilnehc-ynos

Actual behavior

I have the following result, it is slightly different from yours. (with https://github.com/ros2/ros2/commit/ca09b31ce2851773cf722e1fe1e64b70ee651fdd)

$ ros2 run examples_rclcpp_minimal_composition composition_composed \
    --ros-args \
    -r publisher_node:__ns:=/ns1 \
    -r publisher_node:__node:=aaa \
    -r subscriber_node:__ns:=/ns2 \
    -r subscriber_node:__node:=aaa
[WARN] [1600843894.399257225] [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.
[INFO] [1600843894.899459042] [aaa]: Publisher: 'Hello, world! 0'
[INFO] [1600843894.899907305] [aaa]: Subscriber: 'Hello, world! 0'

$ ros2 node list
WARNING: Be aware that are nodes in the graph that share an exact name, this can have unintended side effects.
/aaa
/aaa

could you confirm the problem?

[fujitatomoya]

okay, now i see it. sorry i was too early to ask you.

[iuhilnehc-ynos]

Never mind. Do you think if we should reorder the remapping of node name and namespace?

[ivanpauno]

what happens if you do:

$ ros2 run examples_rclcpp_minimal_composition composition_composed \
    --ros-args \
    -r publisher_node:__ns:=/ns1 \
    -r publisher_node:__node:=aaa \
    -r subscriber_node:__ns:=/ns2 \
    -r subscriber_node:__node:=aaa \                  

does thus work?

Based on the design doc, I think it should be used like that.

[iuhilnehc-ynos]

does thus work?

No, it doesn't work. This's my expectation, so I think we need to update the design document of Order of Applying Remapping Rules

[ivanpauno]

No, it doesn't work. This's my expectation, so I think we need to update the design document of Order of Applying Remapping Rules

Ah, ok. Yeah reading the docs again that doesn't work.

Remapping two nodes that originally have a different name to a same new name but with a different namespace doesn't seem possible. I'm wondering if that use case is important, why would someone want to do that? I mean, picking a different names for both doesn't seem to be bad, and it avoids the issue.

The only downside of the current approach is that some examples can be counter-intuitive, but I'm not sure if that can completely be avoided.

[ivanpauno]

I will left this open and marked as backlog. For pushing the modification further, a non-breaking alternative should be implemented.

See https://github.com/ros2/design/pull/299#issuecomment-718931539 for details.