Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ros2] Ignition Transport message duplication caused by bidirectional bridge #143

Closed
AndrejOrsula opened this issue Mar 26, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@AndrejOrsula
Copy link
Contributor

AndrejOrsula commented Mar 26, 2021

Environment

  • OS Version: Ubuntu 20.04
  • Ignition: Edifice, built from source
  • ROS 2: Foxy, binary install
  • ros_ign branch: ros2 (54e12e2), built from source

Note: Tested inside Docker container. I would appreciate someone to reproduce this issue to ensure it is not caused by my setup.

Description

  • Expected behaviour: Running a bidirectional parameter_bridge with Ignition publisher (separate) should generate only ROS 2 messages.
  • Actual behaviour: Running a bidirectional parameter_bridge with Ignition publisher (separate) generates both ROS 2 messages and duplicate versions of ignition-transport messages.
    • What could happening?:
      • Bidirectional bridge creates Node A: Ignition subscriber + ROS 2 publisher, and Node B: ROS 2 subscriber + Ignition publisher.
      • Once an ignition-transport message is published, Node A receives it and generates ROS 2 message. Subsequently, Node B receives this message generated by Node A and generates a duplicate ignition-transport message. (problem with ROS 2 subscriber configuration?)
      • This is just a speculation, I have not investigated the code. I am sure the original developers thought of this, otherwise there would be an infinite recursion happening if this issue occurred in both directions/multiple times.
  • Workaround: Use unidirectional bridge (99% of the use cases) to prevent message duplication that both decreases performance and could potentially break something. Updating examples to use unidirectional bridge instead of bidirectional bridge might also be an idea.

Note: Issue does not seem to occur for ROS 2 messages, i.e. bridge does not duplicate any ROS 2 messages.
Note: I have not tested this with ROS 1, so I am not sure if it experiences the same problem.

Steps to reproduce

Shell 1:
ros2 run ros_ign_bridge parameter_bridge /chatter@std_msgs/msg/String@ignition.msgs.StringMsg

Shell 2:
ign topic -e -t /chatter

Shell 3:
ign topic -t /chatter -m ignition.msgs.StringMsg -p 'data:"This message should be received only once"'

Output

root@P5550:~# ign topic -e -t /chatter
data: "This message should be received only once"

data: "This message should be received only once"

Expected output (same as without running any bridge)

root@P5550:~# ign topic -e -t /chatter
data: "This message should be received only once"
@AndrejOrsula AndrejOrsula added the bug Something isn't working label Mar 26, 2021
@chapulina
Copy link
Contributor

Interesting, I thought this had been fixed by #33. I see the code that's supposed to prevent it in the ros2 branch:

https://github.com/ignitionrobotics/ros_ign/blob/117e2b2efb2430b199779e82d40344386dd8a253/ros_ign_bridge/src/factory.hpp#L98-L100

Maybe the IntraProcess call is not doing the trick anymore.

@AndrejOrsula
Copy link
Contributor Author

AndrejOrsula commented Mar 27, 2021

I believe the issue originates solely in the ROS 2 subscriber as there is currently no mechanism preventing this to occur. Ignition IntraProcess seems to work fine (otherwise there would be the same issue in the opposite direction and ROS 2 messages would get duplicated).

I have proposed a solution in #146 that makes use of ignore_local_publications subscriber option. However, it does not fix the issue for all rmw implementations, e.g. Cyclone DDS works, but FastRTPS (ROS 2 Foxy default) still has the issue. I added more info into the PR itself.

@AndrejOrsula
Copy link
Contributor Author

This issue was resolved by #146 in this codebase. Furthermore, FastRTPS RMW also supports this functionality now (ros2/rmw_fastrtps#536).

Closing...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants