Add component container for EventsCBGExecutor + Isolated component container for all events based executors#3123
Conversation
Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>
Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>
| if (executor_type == "events") { | ||
| using executor = rclcpp::experimental::executors::EventsExecutor; | ||
| using ComponentManagerIsolated = rclcpp_components::ComponentManagerIsolated<executor>; | ||
| exec = std::make_shared<executor>(); |
There was a problem hiding this comment.
Personally I don't think anything in the experimental namespace should be officially supported in a thing like this. Especially knowing that this is going to be depreciated in short order with the new one
There was a problem hiding this comment.
probably we can say EventExecutor is now graduated phase as official for next turtle-M release from the incubation experimental phase?
| } else { | ||
| using ComponentManagerIsolated = | ||
| rclcpp_components::ComponentManagerIsolated<rclcpp::executors::SingleThreadedExecutor>; | ||
| } else if (executor_type == "cbg-single-threaded") { |
There was a problem hiding this comment.
I think there might be an issue here. Taking the SingleThreadedExecutor and MultiThreadedExecutor as examples, we currently always create a SingleThreaded Executor to manage the components, and pass the executor type to the isolated component manager based on the cli args. The SingleThreadedExecutor is responsible for loading and unloading components
If I am understanding this correctly, with this PR we are creating a single/threaded event executor based on the cli args to manage the components, but then for each component we always create a multi-threaded event executor, correct?
I also wonder if we really need separate cbg-single-threaded vs cbg-multi-threaded. Instead, I think we could take the thread count from ros arguments (similar to the new executable in this PR), and if it is not specified, default to the multi-threaded event executor. What do you think?
There was a problem hiding this comment.
good callout. I'll make those changes on the other PR
4 participants
Description
Fixes # (issue)
Is this user-facing behavior change?
Yes, users will now be able to use the EventsCBGExecutor in a component container, and both the EventsExecutor and EventsCBGExecutor in an isolated component container.
Did you use Generative AI?
Nope
Additional Information
Depends on #3097
Partial re-implementation of #2541, but with additional options for the EventsCBGExecutor to be run in single threaded or multithreaded mode