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

Fix routing loop of Service request when client and server are server by the same bridge #43

Merged
merged 1 commit into from
Dec 29, 2023

Conversation

JEnoch
Copy link
Member

@JEnoch JEnoch commented Dec 29, 2023

In case a Service Server and a Client for the same Service are running in the same domain and discovered by the same bridge, the Client can directly communicate with the Server. Nevertheless, the bridge is also routing the Client's request to the Server, leading to a duplicate request received by the Server.

It seems to not cause an issue for lot of ROS Services Server (the Client will drop the 2nd reply anyway).
But for Lifecycle Nodes that are based on Services for state transitions, this leads to some error messages

E.g. to reproduce with lifecycle demo on a single host:

  • ros2 run lifecycle lifecycle_talker
  • ros2 run lifecycle lifecycle_listener
  • zenoh-bridge-ros2dds
  • ros2 run lifecycle lifecycle_service_client

On Transition 3 this error message shows up for lifecycle_talker:

[WARN] [1703856397.595288020] [rcl_lifecycle]: No transition matching 3 found for current state active
[ERROR] [1703856397.595358561] []: Unable to start transition 3 from current state active: Transition is not registered., at /Users/runner/mambaforge/conda-bld/ros-humble-rcl-lifecycle-0_1675746578761/work/ros-humble-rcl-lifecycle/src/work/src/rcl_lifecycle.c:356

This PR fixes that making sure that the Client Request when routed by the bridge as a Zenoh query doesn't loop back to the same bridge (using .allowed_destination(Locality::Remote))
The PR also improve requests/replies routing logs and operation names for clarity.

@DariusIMP
Copy link
Member

LGTM

@JEnoch JEnoch merged commit a856e06 into main Dec 29, 2023
7 checks passed
@JEnoch JEnoch deleted the fix_srv_req_loop branch December 29, 2023 17:21
JEnoch added a commit that referenced this pull request Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants