-
Notifications
You must be signed in to change notification settings - Fork 310
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
Use singleton approach to store and reuse the service clients (backport #1949) #1953
Conversation
Cherry-pick of b039baa has failed:
To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
Backport fails because #1703 hasn't been backported. @saikishor could you fix that pls? |
ce9d65f
to
6dbd5d3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## humble #1953 +/- ##
==========================================
+ Coverage 62.87% 62.92% +0.05%
==========================================
Files 109 109
Lines 12494 12505 +11
Branches 8481 8479 -2
==========================================
+ Hits 7855 7869 +14
+ Misses 854 852 -2
+ Partials 3785 3784 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
The namespacing support "silently" is added now? |
Yes! It always existed in the name of the argument, but with this even the cli should be happy. Moreover, this only applies if the service is relative and not absolute If you prefer, not to have it. I can fix it |
Fixes: #1934
the node.destroy_node() at the end does the same thing by destroying clients and every subscribers, publishers etc (https://github.com/ros2/rclpy/blob/8f1f16f16090184062f1993728d063ff2680f958/rclpy/rclpy/node.py#L2017-L2053)
This is an automatic backport of pull request #1949 done by Mergify.