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

Add two example packages of service client and service server to understand asynchronism #398

Open
wants to merge 2 commits into
base: rolling
Choose a base branch
from

Conversation

santiago-tapia
Copy link

This PR add two new packages in C++ services examples.

  • The server example introduces a response delay in the service, configurable via a parameter. The purpose of this example is to bring the service response time to a user-perceivable scale, allowing the effects of such delays to be better observed.
  • The client example uses a callback to receive the server's response. By using a callback, the main program of the node can follow the usual implementation of the main function in any other node; the spin method can be used, and there is no synchronous blocking call to wait for the response.

Signed-off-by: Santiago Tapia-Fernández <[email protected]>
Signed-off-by: Santiago Tapia-Fernández <[email protected]>
Copy link

@MichaelOrlov MichaelOrlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@santiago-tapia Thanks for the PR.
We have discussed this PR on today's issue triage meeting, and we are curious how these proposed examples differ from the existent services tutorial https://docs.ros.org/en/rolling/Tutorials/Beginner-Client-Libraries/Writing-A-Simple-Cpp-Service-And-Client.html

We think that It would be better to change the existing tutorial rather than add new examples.

@santiago-tapia
Copy link
Author

@MichaelOrlov, Thanks for your comment.

Actually, I am working on an additional intermediate tutorial on services and a new article on "Concepts". I began working on the examples just for starting the whole task by coding. You might wait until they are all completed to get the general intention.

Regarding the novelty of the examples themselves, I think they add the following features to the examples already there:

  • The server includes a delay, that is not a real feature, but it helps in understanding that servers could take a significant time to respond, while adding two integers is instantaneous and, so, the user could not appreciate the impact of a slow response.

  • The existing example for an asynchronous client is not explained in a tutorial (as I said these ones will be explained in a tutorial), it prones requests and, well, it is very complex. My proposal for an asynchronous is simpler, it works only with callbacks and processes all the responses. I think it helps in understanding why services are asynchronous and what are the consequences of being asynchronous.

As I said, I think you might hold this PR until you could read the associated documentation and then decide on its relevance or novelty.

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@santiago-tapia

thanks for opening the PR and effort.

i have a couple of comments though.

The server includes a delay, that is not a real feature, but it helps in understanding that servers could take a significant time to respond, while adding two integers is instantaneous and, so, the user could not appreciate the impact of a slow response.

so are you trying to introduce the kind of anti-pattern in examples with adding the delay in the service server?

i am not sure about this...

  • it is obvious that if the service server processes huge task on service request, it will take time in the application callback.
  • i do not think this is the consequence, because application implements so?
  • and there is nothing we can do in the system to address this situation but application logic. this is application implementation.

The existing example for an asynchronous client is not explained in a tutorial (as I said these ones will be explained in a tutorial)

https://docs.ros.org/en/rolling/Tutorials/Beginner-Client-Libraries/Writing-A-Simple-Cpp-Service-And-Client.html#write-the-client-node uses async_send_request.
and,

auto result_future = client->async_send_request(request);
?

My proposal for an asynchronous is simpler, it works only with callbacks and processes all the responses.

above tutorial and async example are really simple for me. what could be the different from yours?

@santiago-tapia
Copy link
Author

@fujitatomoya

Thank you for your comments.

I understand your caution about the delay in the server, I also think it might be misunderstood, but I also think some potential readers would like to understand how services work and what are the effects of timing in their systems. I hope, on a proper explanation, they will not be confused. Providing an arbitrary delay to an, otherwise, fixed and very short delay would help in understanding the features of services in other, less optimal, circumstances. Obviously, this will not be a copy-paste example or at least, you will need to remove the delay after copy-paste.

With respect with the simplicity of the example, I am afraid I have not explained myself clearly, I was comparing my example with this one: https://github.com/ros2/examples/blob/rolling/rclcpp/services/async_client/main.cpp in terms of simplicity. I mean, that example is great, but I think its target is showing the feature about discarding requests (not an easy task as it seems)... 

And I think I do add something to the code in https://github.com/ros2/examples/blob/rolling/rclcpp/services/minimal_client/main.cpp, if you keep reading below line 39, next line is:

if (rclcpp::spin_until_future_complete(node, result_future) !=

That, spin_until_future_complete, is a problematic call if you try it inside another callback because you will be reentrying an spinning method inside your actual spin. Furthermore, in my opinion, the combination of an asynchronous call and an immediate call for waiting is an example of an asynchronous client without exposing the full picture of asynchronism. 

My example targets this situation: when a programmer would like to call a service on an incoming event, that is, inside a ROS2 callback and would also like to receive a callback when the work is done instead of waiting for it. Introducing the delay in the server will be useful to show that other things may happen while you get your response, including making other requests (and that doing so is not a problem if you know it). 

@fujitatomoya
Copy link
Collaborator

@santiago-tapia appreciate your explanation.

Furthermore, in my opinion, the combination of an asynchronous call and an immediate call for waiting is an example of an asynchronous client without exposing the full picture of asynchronism.
My example targets this situation: when a programmer would like to call a service on an incoming event, that is, inside a ROS2 callback and would also like to receive a callback when the work is done instead of waiting for it.

i believe this is good point, and would be useful for user to know how to do that with example. as far as i know we do not have this kind of examples. basically key part is already in https://github.com/ros2/examples/blob/rolling/rclcpp/services/async_client/main.cpp to use callback via async_send_request, but that is not the example for async service request in the callback.

@santiago-tapia
Copy link
Author

I have just created the PR for the tutorial: Tutorial PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-information-needed Further information is required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants