Use singleton approach to store and reuse the service clients #1949
Conversation
github-actions
bot
requested review from
ARK3r,
bmagyar,
destogl,
fmauch,
harderthan,
moriarty and
Raivias
|
@bijoua29 Check this approach to handle the same situations |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1949 +/- ##
==========================================
- Coverage 87.73% 87.71% -0.02%
==========================================
Files 122 122
Lines 13010 13020 +10
Branches 1165 1166 +1
==========================================
+ Hits 11414 11421 +7
- Misses 1165 1167 +2
- Partials 431 432 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@saikishor Looks good to me but let me test this in the actual application |
Sure. Go ahead. Please let us know if this helps in such situations. I've only manually verified in the tests that if it is reusing it or not. This fix along with the multiple controllers spawning should be more robust |
|
@saikishor I tested the PR in our application and replicated the same results as before i.e. improvement in controller startup success. So I think we can merge this in. I approved it. BTW, I wouldn't say it completely fixes #1934 but it definitely improves it. I will close #1947 in lieu of this PR. |
saikishor
added
the
backport-humble
There was a problem hiding this comment.
What do we have to change in ros2_control_demo_testing to make this work?
Thanks for bringing it to our attention @christophfroehlich. Now it should be all good. I verified locally |
christophfroehlich
merged commit b039baa
into
ros-controls:master
(cherry picked from commit b039baa) # Conflicts: # controller_manager/controller_manager/controller_manager_services.py
…#1953) Co-authored-by: Sai Kishor Kothakota <[email protected]>
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)