Implemented Multithreaded Executor example#251
Conversation
Release Version of Multithreaded Executor example Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
| // You MUST use the MultiThreadedExecutor to use, well, multiple threads | ||
| rclcpp::executors::MultiThreadedExecutor executor; | ||
| auto pubnode = std::make_shared<PublisherNode>(); | ||
| auto subnode = std::make_shared<DualThreadedNode>(); // This contains BOTH subscriber nodes. They will still run on different threads |
There was a problem hiding this comment.
Maybe clarify which parts of the node are being run on multiple threads?
I also think that "subscriber nodes" should be rephrased to "subscription callbacks" (since it's still just one node).
There was a problem hiding this comment.
I have taken your suggestion and renamed "subscriber nodes" to "subscription callbacks". Definitely makes more sense.
I was hoping that the constructor would make the two threads clear enough. Should I add a few more comments around subscription1_ and subscription2_ to make it completely unambiguous?
clalancette
left a comment
There was a problem hiding this comment.
Thanks so much for implementing this; it's a long-standing missing example.
I have a few different things to fix inline, but they are mostly easy. Once those are in, we can run CI and then get this merged.
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A; OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
clalancette
left a comment
There was a problem hiding this comment.
Looking pretty good. A few more comments, then we can give this a run in CI.
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
|
Hopefully the latest version should take care of the remaining issues. Unless you see something else that can be improved? |
clalancette
left a comment
There was a problem hiding this comment.
Still a few more minor things.
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
clalancette
left a comment
There was a problem hiding this comment.
A couple more small things, then I'll run CI.
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
clalancette
left a comment
There was a problem hiding this comment.
Looking good to me. I'll fire up CI on this now.
Release Version of Multithreaded Executor example Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A; OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
|
FYI: I renamed this to |
|
CI:
|
Distro A, OPSEC #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
Distro A, Opsec #2893 Signed-off-by: Jacob Hassold <jhassold@dcscorp.com>
clalancette
left a comment
There was a problem hiding this comment.
Code looks good, CI looks good, I'm happy with this. @allenh1 I'm planning on merging this later today unless you have any objections.
|
@clalancette Looks fine to me. @jhdcs thanks for iterating! |
|
@allenh1, @clalancette, You are quite welcome. Happy to help. |
Release Version of Multithreaded Executor example
Pertains to issue #208
Signed-off-by: Jacob Hassold jhassold@dcscorp.com
Distro A; OPSEC #2893