Conversation
Signed-off-by: sykwer <sykwer@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces support for reentrant callback groups in the CallbackIsolatedExecutor, enabling parallel execution of callbacks within the same callback group. The implementation creates a new MultiThreadedExecutorInternal class to handle reentrant callback groups with configurable parallelism (currently hardcoded to 4 threads), while maintaining single-threaded execution for mutually exclusive callback groups.
Changes:
- Added MultiThreadedExecutorInternal class to enable multi-threaded execution for reentrant callback groups
- Modified CallbackIsolatedExecutor to detect and handle reentrant callback groups differently from mutually exclusive ones
- Created a sample ReentrantNode application to demonstrate the reentrant callback group functionality
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| callback_isolated_executor/include/callback_isolated_executor/multi_threaded_executor_internal.hpp | Defines the MultiThreadedExecutorInternal class with thread synchronization primitives for reentrant callback group execution |
| callback_isolated_executor/src/multi_threaded_executor_internal.cpp | Implements thread pool management, synchronization logic, and executor lifecycle for reentrant callback groups |
| callback_isolated_executor/include/callback_isolated_executor/callback_isolated_executor.hpp | Adds reentrant parallelism configuration and methods to handle different callback group types |
| callback_isolated_executor/src/callback_isolated_executor.cpp | Refactors spin logic to route reentrant and mutually exclusive callback groups to appropriate executors |
| callback_isolated_executor/CMakeLists.txt | Updates build configuration to include new MultiThreadedExecutorInternal implementation |
| cie_sample_application/include/cie_sample_application/reentrant_node.hpp | Defines ReentrantNode header with reentrant callback group and timer declarations |
| cie_sample_application/src/reentrant_node.cpp | Implements ReentrantNode with two timers assigned to a reentrant callback group for testing |
| cie_sample_application/src/reentrant_node_main.cpp | Creates executable entry point for the ReentrantNode sample application |
| cie_sample_application/CMakeLists.txt | Adds build targets for the new reentrant_node component and executable |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cie_sample_application/include/cie_sample_application/reentrant_node.hpp
Outdated
Show resolved
Hide resolved
cie_sample_application/include/cie_sample_application/reentrant_node.hpp
Outdated
Show resolved
Hide resolved
...ck_isolated_executor/include/callback_isolated_executor/multi_threaded_executor_internal.hpp
Show resolved
Hide resolved
cie_sample_application/include/cie_sample_application/reentrant_node.hpp
Outdated
Show resolved
Hide resolved
…t_node.hpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…t_node.hpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
atsushi421
reviewed
Jan 19, 2026
callback_isolated_executor/src/multi_threaded_executor_internal.cpp
Outdated
Show resolved
Hide resolved
…l.cpp Co-authored-by: atsushi yano <55824710+atsushi421@users.noreply.github.com>
atsushi421
approved these changes
Jan 25, 2026
atsushi421
added a commit
that referenced
this pull request
Jan 25, 2026
…lback_isolated Add support for reentrant callback groups in ComponentManagerCallbackIsolated, matching the functionality already present in CallbackIsolatedExecutor (PR #21). - Use MultiThreadedExecutorInternal for reentrant callback groups with configurable parallelism (default 4 threads) - Use SingleThreadedExecutor for mutually exclusive callback groups - Unify ExecutorWrapper to use std::shared_ptr<rclcpp::Executor> instead of separate executor types - Update CMakeLists.txt to include multi_threaded_executor_internal.cpp and required include directories
atsushi421
added a commit
that referenced
this pull request
Jan 25, 2026
…lback_isolated Add support for reentrant callback groups in ComponentManagerCallbackIsolated, matching the functionality already present in CallbackIsolatedExecutor (PR #21). - Use MultiThreadedExecutorInternal for reentrant callback groups with configurable parallelism (default 4 threads) - Use SingleThreadedExecutor for mutually exclusive callback groups - Unify ExecutorWrapper to use std::shared_ptr<rclcpp::Executor> instead of separate executor types - Update CMakeLists.txt to include multi_threaded_executor_internal.cpp and required include directories
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
TODO
4for now (should be configurable parameter)Related links
How was this PR tested?
Notes for reviewers