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

RouteServiceCli: create DDS Reader/Writer only if a remote Server is announced (fix #62) #63

Merged
merged 4 commits into from
Jan 23, 2024

Conversation

JEnoch
Copy link
Member

@JEnoch JEnoch commented Jan 22, 2024

fix #62.

The route for a Service Client (RouteServiceCli) was creating a DDS Reader for requests and a DDS Writer for reply as soon as created, following the discovery of a ROS Service Client over DDS. This lead the Service Client to think a Server was existing, but it wasn't! The Client was sending the request to the bridge that routed it as a query going nowhere, and thus no reply ever came to the Client that kept hanging.

This PR fixes this making sure the route is activated (i.e. DDS Reader/Writer created) only when a remote bridge announces the discovery and the routing of a corresponding Service Server.

This behaviour (announcement from remote bridge) relies on the Liveliness Tokens declared to announce the discovery and the routing of each ROS entity (pub/sub, client/server for service/action).
Ideally, we would like a non-ROS pure-Zenoh Queryable to be detected as matching the RouteServiceCli without the need of a Liveliness Token, and thus having the route activated when detecting this Queryable. This requires the introduction of a Querier object in Zenoh, and the implementation of a MatchingStatus listener in the same way than for the Publisher (see eclipse-zenoh/zenoh#565).
This will come later.

@JEnoch
Copy link
Member Author

JEnoch commented Jan 22, 2024

Note to reviewers:
this PR also includes an internal renaming of activate/deactivate functions into announce_route/retire_route functions.
It's a first step for a reorganisation of the internal design of each route, making the distinction between 3 states:

  • Created: the route is initialised with relevant information, but doing nothing yet
  • Announced: the route existence is announced to remote bridges via a Liveliness Token. It implies the announcement of the ROS entity it serves.
  • Active: the route has it's DDS Reader and/or Writer created and is ready to route data

Such distinction allows to:

  1. announce a route but not activate it. For instance when a ROS Publisher is discovered, we need to announce it via its RoutePublisher object, but it shall not be active unless a matching subscriber is detected. Otherwise, it will receive DDS data for nothing, creating a burden that might impact the perfs.
  2. activate a route but not announce it. For instance when a remote bridge announces a ROS Service Server (via its RouteServiceSrv object), the local bridge has to be activated and to create the corresponding DDS Reader on Request topic and DDS Writer on Reply topic, such as the ROS graph is valid and the Service can be discovered by the ROS Nodes.

@JEnoch JEnoch requested a review from OlivierHecart January 22, 2024 14:30
@JEnoch JEnoch changed the title Fix 62 RouteServiceCli: create DDS Reader/Writer only if a remote Server is announced (fix #62) Jan 22, 2024
@JEnoch JEnoch merged commit ca44eb4 into main Jan 23, 2024
9 checks passed
JEnoch added a commit that referenced this pull request Jan 23, 2024
@JEnoch JEnoch deleted the fix_62 branch April 29, 2024 14:29
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.

[Bug] ROS Service Client hanging when the bridge has not discovered a Server
2 participants