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

Provide support for services #80

Closed
4 tasks
francocipollone opened this issue Dec 4, 2023 · 8 comments
Closed
4 tasks

Provide support for services #80

francocipollone opened this issue Dec 4, 2023 · 8 comments
Assignees

Comments

@francocipollone
Copy link
Collaborator

Tasks

  • Implement service side rmw methods
  • Implement client side rmw methods
  • Update graph cache accordingly
  • Implement methods related to services introspection
@francocipollone
Copy link
Collaborator Author

Some wip at rolling...francocipollone/service_support_via_queryable via using zenoh queryable.

It might change to use topics as the DDS-based implementations due to rmw api current behavior. Being discussed.

@francocipollone francocipollone self-assigned this Dec 4, 2023
@francocipollone
Copy link
Collaborator Author

Some comment related to zenoh queryables and ownerships:

Context

Following RMW implementation if using queryable:

  • The queryable is declared at rmw_create_service
  • A query is analized at rmw_take_request
  • A query is answered at rmw_send_response

Issue

In this example: https://github.com/eclipse-zenoh/zenoh-c/blob/master/examples/z_queryable.c#L28-L42

The callback for the queryable receives a z_query_t pointer.
The method that is used to send a reply is: z_query_reply (For what I saw from docstrings: "This function must be called inside of a Queryable callback passing the
query received as parameters of the callback function" )

After double checking with Zettascale folks: using the z_query_t outside of its callback is UB, as said query is destroyed. It is in the roadmap to support this API.

Alternative

We could move forward using publishers and subscriptions for service support, typically how it is done for the dds-based implementations, and upgrade to use Queryables later on when supported.

@clalancette
Copy link
Collaborator

We could move forward using publishers and subscriptions for service support, typically how it is done for the dds-based implementations, and upgrade to use Queryables later on when supported.

I don't love the 2 topic solution (it leads to races), but this is an OK workaround for now. Once we get the queryable-handle support later, we can switch to it.

The only thing I'll suggest is that we design the API so that we can easily switch to the queryable-based solution in the future.

@JEnoch
Copy link
Contributor

JEnoch commented Dec 4, 2023

It is in the roadmap to support this API.

Actually owned queries are coming very soon in zenoh-c: eclipse-zenoh/zenoh-c#213
And will come later for zenoh-pico.

@francocipollone
Copy link
Collaborator Author

It is in the roadmap to support this API.

Actually owned queries are coming very soon in zenoh-c: eclipse-zenoh/zenoh-c#213 And will come later for zenoh-pico.

Awesome, thanks for tackling this quickly

@nkoenig
Copy link
Contributor

nkoenig commented Jan 16, 2024

@Yadunund and @francocipollone , checking to see if services are now supported? I see #86 and #88, both of which seem promising.

@clalancette
Copy link
Collaborator

@Yadunund and @francocipollone , checking to see if services are now supported? I see #86 and #88, both of which seem promising.

Sort of. They work in their most basic form, but they are pretty buggy right now. #90 adds in graph introspection for services. I'm working on some additional fixes on top of #90 which improves the reliability of them. Hopefully after that they will be closer to "done".

@clalancette
Copy link
Collaborator

We've fixed all of the known issues with services right now, so closing this. Any other problems we can ticket separately.

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

No branches or pull requests

4 participants