-
Notifications
You must be signed in to change notification settings - Fork 316
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
base: rolling
Are you sure you want to change the base?
Conversation
Signed-off-by: Santiago Tapia-Fernández <[email protected]>
Signed-off-by: Santiago Tapia-Fernández <[email protected]>
There was a problem hiding this 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.
@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:
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
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:
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). |
@santiago-tapia appreciate your explanation.
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 |
I have just created the PR for the tutorial: Tutorial PR |
This PR add two new packages in C++ services examples.