Use multi-threaded component container, do not use intraprocess comms in Servo#723
Conversation
883fb8f to
c0e531a
Compare
Codecov Report
@@ Coverage Diff @@
## main #723 +/- ##
=======================================
Coverage 54.23% 54.23%
=======================================
Files 192 192
Lines 20224 20224
=======================================
Hits 10966 10966
Misses 9258 9258 Continue to review full report at Codecov.
|
henningkayser
left a comment
There was a problem hiding this comment.
Huh, I wasn't aware about this... Good improvement
AndyZe
left a comment
There was a problem hiding this comment.
I think this type of change should at least be tested with the tutorial to verify that publications to the controller have the right rate. (Which I would normally do but I guess it will have to wait a week.)
schornakj
left a comment
There was a problem hiding this comment.
For the projects I work on that use Servo, I have not encountered any bugs, regressions, or weird behavior since switching to running the ServoNode component through the multithreaded component container.
|
@AndyZe @henningkayser @JafarAbdi this PR depends on a separate PR: #722 Should I separate these so we can merge this one separately, or do you all approve the other PR as well? |
|
I`m fine merging this and closing the other PR |
I think it's fine merging both. With #722 merged we should still review the QOS settings everywhere and see if we can or should enable intra process comm again. |
|
Jafar is doing the release right now so I'm going to wait on merging any of these until he finishes. |
Based off:
Description
As the multi-threaded component container (and executor) are now usable in ROS Rolling we should switch to using them with servo and its tests.
Checklist