fix(cie): always re-apply thread configuration and keep running#49
Merged
fix(cie): always re-apply thread configuration and keep running#49
Conversation
Port the "always re-apply" behavior from agnocast's fix/thread-configurator-always-reapply branch. The configurator now: - Always re-applies scheduling configuration when a CallbackGroupInfo or NonRosThreadInfo message is received, even if the callback group was already configured. This handles the case where the OS reuses thread IDs after an application restarts. - Keeps running via executor->spin() instead of exiting after all configurations are applied, enabling automatic re-configuration when target applications restart. - Tracks configured_at_least_once_ to report unapplied threads only when the configurator is terminated before any complete round. Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the ROS 2 thread configurator to continuously run and to always re-apply thread scheduling/affinity configuration when thread info is received, so configurations are re-enforced after target applications restart (including cases where the OS reuses thread IDs).
Changes:
- Switch the main executable to
executor->spin()(continuous operation) and only report unapplied configs on shutdown if nothing was ever fully configured. - Always re-apply configuration even when a callback group / non-ROS thread was previously marked as applied; add “all configured” notification +
has_configured_once()tracking. - Update the top-level README to document continuous operation and
SCHED_DEADLINEbehavior (SCHED_FLAG_RESET_ON_FORK).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cie_thread_configurator/src/thread_configurator_node_main.cpp | Runs executor continuously; changes shutdown reporting behavior. |
| cie_thread_configurator/src/thread_configurator_node.cpp | Removes “skip if applied” logic and adds “all configured” tracking/logging. |
| cie_thread_configurator/include/cie_thread_configurator/thread_configurator_node.hpp | Updates public API (removes all_applied, adds has_configured_once/state). |
| README.md | Documents continuous-running configurator and SCHED_DEADLINE semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
After the first full configuration cycle, unapplied_num_ stays at 0 and all configs have applied=true. When a target app restarts and re-sends callback info, on_all_configured() was called for every single re-apply message because unapplied_num_==0 was always true. Guard the call with !configured_at_least_once_ so the "all configured" log fires only once during the initial configuration round. Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
When the YAML contains zero configured threads, call on_all_configured() during construction so has_configured_once() returns true and the shutdown path does not misleadingly warn about unapplied configs. Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
Move the unapplied check into print_all_unapplied() itself so main always calls it unconditionally, and remove the now-unused has_configured_once() method. Upstream: autowarefoundation/agnocast#1262 Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
sykwer
approved these changes
Apr 22, 2026
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
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
Port the "always re-apply" behavior from agnocast's
fix/thread-configurator-always-reapplybranch.executor->spin()instead of exiting after all configurations are applied, enabling automatic re-configuration when target applications restarthas_configured_once()and moved the unapplied guard intoprint_all_unapplied()itself, somainalways calls it unconditionallyRelated links
How was this PR tested?
Notes for reviewers
The key behavioral changes are:
callback_group_callbackandnon_ros_thread_callbackno longer early-return whenconfig->appliedis true — they always re-apply the configurationon_all_configured()fires only once (guarded byconfigured_at_least_once_), andunapplied_num_is only decremented on the first applyexecutor->spin()instead ofspin_once()in a while loop, so the configurator node stays alive indefinitelyhas_configured_once()/all_applied()removed;print_all_unapplied()now self-guards with an early return whenunapplied_num_ == 0