-
Notifications
You must be signed in to change notification settings - Fork 44
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 overloaded Request for non-blocking calls using abstract types #386
base: gz-transport12
Are you sure you want to change the base?
Add overloaded Request for non-blocking calls using abstract types #386
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## gz-transport12 #386 +/- ##
==================================================
- Coverage 87.23% 87.06% -0.18%
==================================================
Files 60 60
Lines 5211 5227 +16
==================================================
+ Hits 4546 4551 +5
- Misses 665 676 +11 ☔ View full report in Codecov by Sentry. |
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 the patch @srmainwaring ! Could you also add a C++ example under example/
that exercises the new Request()
API?
Thanks for the review @caguero - yes will add a test + example. |
330a61e
to
a86e517
Compare
@srmainwaring friendly ping on the test + example so we can get this in before code freeze. |
…stract types. - Add overloaded method to Node. - Update specialisation of ReqHandler to handle callbacks using google::protobuf::Message. Signed-off-by: Rhys Mainwaring <[email protected]>
f5c8bf9
to
73ef858
Compare
@srmainwaring , friendly reminder. I can help you with a test if you commit an example with the new functionality. |
🎉 New feature
Summary
Add an overloaded Request method to the class Node that accepts references to abstract request and response types, specifically the type
google::protobuf::Message
.The addition is to support a non-blocking version of the service call used in
cmdServiceReq
(gz-transport/src/cmd/gz.hh). The existing Request methods accepting a callback function all require a concrete type as they callRequest().GetTypeName()
andResponse().GetTypeName()
. This does not work when the request or response types are the abstract base classgoogle::protobuf::Message
. The instance calls_request->GetTypeName()
and_response->GetTypeName()
on the other hand are valid, and this is what the new overload function uses.There is also a change to the template specialisation of
ReqHandler
to support callbacks.Test it
Tasks
main
because of ABI changes.gz-transport
The current use case is in a downstream project providing python bindings for gz-msgs and gz-transport: https://github.com/srmainwaring/gz-python/tree/srmainwaring/service-request
At present only blocking service requests are supported. The Python bindings use the same approach as the
gz service
command line tools. This PR allows asynchronous callbacks as well. The example below is for a service that allows the PID settings of a joint controller to be updated in Python at runtime. Both blocking and non-blocking versions of the call are demonstrated.Expected console output:
% ~/Code/osrf/gz_garden_ws/src/gz-python/python/pid_service.py Blocking service call Non-blocking service call Result: True Response: p_gain_optional { data: 0.3 } i_gain_optional { } d_gain_optional { } i_max_optional { data: 1.0 } i_min_optional { data: -1.0 } limit_optional { data: 10.0 } Done
Expected
gz sim
output (using a modified version of the ArduPilot plugin: srmainwaring/ardupilot_gazebo-1@c938fd5):Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.