-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Tutorial for writing a service with an asynchronous client node #4903
base: rolling
Are you sure you want to change the base?
Conversation
Signed-off-by: Santiago Tapia-Fernández <santiago.tapia@upm.es>
Signed-off-by: Santiago Tapia-Fernández <santiago.tapia@upm.es>
Signed-off-by: Santiago Tapia-Fernández <santiago.tapia@upm.es>
For information on how to write a basic client/server service see | ||
:doc:`checkout this tutorial <../../Beginner-Client-Libraries/Writing-A-Simple-Cpp-Service-And-Client>`. | ||
|
||
**All** service **clients** in ROS2 are **asynchronous**. In this tutorial, an example |
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.
**All** service **clients** in ROS2 are **asynchronous**. In this tutorial, an example | |
**All** service **clients** in ROS 2 are **asynchronous**. | |
In this tutorial, an example |
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 we usually do one sentence per line and ROS 2 is always stylized ROS 2
.
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.
I wouldn't assume all readers fully understand what is meant by "asynchronous." We get a lot of undergraduate students reading the docs who may not have reached that point in their education. It would help readability to quickly define your terms and then move on to discussing everything else.
the response, meaning its thread is not running but blocked until the | ||
response is returned. | ||
|
||
In contrast, the following diagram illustrates an asynchronous ROS2 client: |
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.
In contrast, the following diagram illustrates an asynchronous ROS2 client: | |
In contrast, the following diagram illustrates an asynchronous ROS 2 client: |
:alt: A sequence diagram that show how a sync-client waits for the response | ||
|
||
It shows a **synchronous** client. The client makes a request and waits for | ||
the response, meaning its thread is not running but blocked until the |
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.
the response, meaning its thread is not running but blocked until the | |
the response, meaning its thread is blocked (i.e. not running) until the |
Of course, it could be stopped by invoking a waiting function or another | ||
blocking function, but this diagram shows a ROS2 client, and in ROS2, to receive |
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.
Of course, it could be stopped by invoking a waiting function or another | |
blocking function, but this diagram shows a ROS2 client, and in ROS2, to receive | |
Of course, it could be stopped by invoking a waiting function or another | |
blocking function, but this diagram shows a ROS 2 client, and in ROS 2, to receive |
ll 50-53 is a bit of a run-on sentence. I suggest breaking this up into two sentences.
(the client is *activated*), and afterward, the response is received, and the | ||
client executes the callback for the response. | ||
|
||
Since any service client in ROS2 needs to be spinning to receive the server's |
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.
Since any service client in ROS2 needs to be spinning to receive the server's | |
Since a service client in ROS 2 needs to be spinning to receive the server's |
client executes the callback for the response. | ||
|
||
Since any service client in ROS2 needs to be spinning to receive the server's | ||
response, all clients in ROS2 are asynchronous by design (they cannot simply |
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.
response, all clients in ROS2 are asynchronous by design (they cannot simply | |
response, all clients in ROS 2 are asynchronous by design (they cannot simply |
In :doc:`Writing a Simple cpp Service and Client <../../Beginner-Client-Libraries/Writing-A-Simple-Cpp-Service-And-Client>` you | ||
learned how to define a custom service that adds two ints, we will use that service again. | ||
|
||
The source code in this tutorial is at `rclcpp examples <https://github.com/ros2/examples/tree/{REPOS_FILE_BRANCH}/rclcpp/services>`, |
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.
The source code in this tutorial is at `rclcpp examples <https://github.com/ros2/examples/tree/{REPOS_FILE_BRANCH}/rclcpp/services>`, | |
The source code for this tutorial is located at `rclcpp examples <https://github.com/ros2/examples/tree/{REPOS_FILE_BRANCH}/rclcpp/services>`. |
learned how to define a custom service that adds two ints, we will use that service again. | ||
|
||
The source code in this tutorial is at `rclcpp examples <https://github.com/ros2/examples/tree/{REPOS_FILE_BRANCH}/rclcpp/services>`, | ||
you might get the code there directly. |
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.
you might get the code there directly. |
delay in responding to requests. Nevertheless, it could | ||
serve as an example if the delay is removed. | ||
|
||
Actually, there are no particularly relevant elements here. The main |
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.
I think you could simplify this quite a bit. Just say that you've created a server with an arbitrary and artificial delay..
of a service client is analyzed to provide insight into this fact, explaining | ||
the reasons for its asynchronous nature and some of the timing-related | ||
consequences. | ||
|
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.
There's a saying in technical writing that you should, "tell people what you are going to do, tell them what to do, and then tell them what they did."
It may help readability if you preface this tutorial with that you are going to do. Perhaps something like, "In this tutorials we are going to create A and B, and use it evaluate the impact of C on D."
also be used to track the *state* of the request if necessary. | ||
|
||
The second callback is for receiving the server response. Note that, | ||
being a callback, it will be executed at the spinning thread. The code is |
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.
"at the spinning thread"
This phrasing is a bit unclear to me. Perhaps something like, "Note that since this function uses a callback, it will be executed promptly by the node's main thread while still allowing the node to accomplish other tasks."
|
||
.. note:: | ||
|
||
Compared to the code of a hypothetical synchronous client, the key |
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.
This block, 412-416 could use a bit of revision. It might help to state plainly what the code does, and then contrast with the alternative version.
Discover available components | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
To see what packages that contains *examples_* are registered and available |
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.
To see what packages that contains *examples_* are registered and available | |
To see what packages contain the term *examples_* are available |
|
||
ros2 pkg list | grep examples_ | ||
|
||
The terminal will show a list of packages from ros2_examples, actually, |
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.
This could be shortened. Try to keep it simple for the new users. 😉
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 sending us this pull request! I think this contribution could be very helpful for new ROS users learning how to build their first asynchronous client / server pair.
I am going to need you to do a few things so we can help you get this approved.
First, by convention, we request that each sentence gets it own line in your pull request. The reason we request this is that it makes it much easier for us to suggest changes in the file. There were a few places I wanted to make a suggestion but it was difficult to do because of the line breaks. Also, by convention, we stylize it "ROS 2" when written in the docs.
In terms of code I didn't see anything wrong with the pull request. However, the tutorial starts out a little weak, but finishes strong (see my notes at the start of the file). Please keep in mind the audience for the ROS docs are students and developers across the globe, so the shorter and simpler you can make your sentences the better.
If you could address the line break issue in the PR I can make another pass. Once you address those suggestions and we get a second review we can you merged into the docs. Thanks!
@kscottz, Thank very much for your comments. I am going to try to solve the issues you mentioned. Just to confirm:
I was wondering if it could be interesting to make a "Concept" article about *asynchronism" and other related concepts. This way the explanations here could be shorter since the reader could find more details somewhere else. It will take me a while because I have other urgent matters, but I am going to do it as soon as possible. |
Yep. Generally let your editor deal with line wrapping. Using this approach makes suggestions a whole lot easier on our side.
That sounds about right. Don't work too hard on shortening sentences, I can lend a hand with editing once we have the one sentence per line fixed. One other thing to keep in mind is that our linter is really picky. It tends to complain about trailing whitespace at the end of a line, so I would recommend making whitespace visible in your editor if you can. I can spot fix a few things but it really sucks if every line has an extra space or two at the end.
No worries! It is the holidays. We really appreciate your contribution to the documentation!
I would be open to it but I think it would help if we were all on the same page before you put a lot of effort into it. What I would recommend is that you file an issue and describe what you want to do with a detailed outline. We can discuss the outline before you sink a lot of effort into writing. Thanks for being flexible about the idiosyncrasies in our docs contribution process. |
.. redirect-from:: | ||
|
||
Async-Service | ||
Tutorials/Async-Service |
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.
this is whole new section, why we need this redirection?
it specifies one or more alternate URLs from which the page can be redirected. It's useful for maintaining backward compatibility when documentation pages are reorganized or renamed.
For information on how to write a basic client/server service see | ||
:doc:`checkout this tutorial <../../Beginner-Client-Libraries/Writing-A-Simple-Cpp-Service-And-Client>`. |
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.
For information on how to write a basic client/server service see | |
:doc:`checkout this tutorial <../../Beginner-Client-Libraries/Writing-A-Simple-Cpp-Service-And-Client>`. | |
For information on how to write a basic client/server service see :doc:`Writing a Simple Service and Client <../../Beginner-Client-Libraries/Writing-A-Simple-Cpp-Service-And-Client>`. |
Server --> Client : Return Response | ||
deactivate Server | ||
|
||
@enduml |
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.
line feed is required in the last?
@enduml | |
@enduml | |
Signed-off-by: Santiago Tapia-Fernández <santiago.tapia@upm.es>
…ntation into async_service
Hi again, I have reviewed the whole tutorial according to the comments from @kscottz and @fujitatomoya. I am afraid it must be very difficult to track the changes between my previous commit and this last commit because there are lots of changes. That is because I have written every sentence in a line, and, besides, I have reviewed almost every sentence to try to simplify the statements (just, please, remember that English is not my native language and that is not my fault...). I have also reworked the introduction to say what the reader is going to do and to make a short explanation about asynchronism. That means I have moved some paragraphs and rewritten some others. I have also corrected some other minor details like code indentation. Of course I have replaced ROS2 with ROS 2. This is my last commit, there is another commit but it is just a merge commit due to a fork synchronization, since this tutorial is completely new, it is not relevant. Regards, Santiago |
This PR is connected to another PR at example repository: Async Service Example.
This PR includes an intermediate tutorial to write an asynchronous client that sends a request inside a callback. The key feature about the example is that the node execution is not waiting in any where. It is just executing callbacks or spinning, all time.
Besides, it includes some experiments on executing the client in combination with a delayed server, those experiments are designed to help in the understanding of the concept of asynchronism.