fix(fault_manager): accurate HIGHEST_SEVERITY reassignment and stale fault_to_cluster_ cleanup#221
Merged
mfaferek93 merged 3 commits intoselfpatch:mainfrom Feb 18, 2026
Conversation
mfaferek93
requested review from
bburda,
Copilot and
mfaferek93
and removed request for
Copilot
Collaborator
|
Interesting, copilot seems to be dead 💀 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes two issues in the fault manager's correlation engine: (1) accurate representative reassignment for HIGHEST_SEVERITY policy when the current representative is cleared, and (2) cleanup of stale fault_to_cluster_ entries when pending clusters expire.
Changes:
- Added
fault_severitiesmap toPendingClusterto track per-fault severity for accurate representative selection - Implemented proper HIGHEST_SEVERITY representative reassignment using stored severities
- Added cleanup of stale
fault_to_cluster_entries incleanup_expired() - Added two new unit tests covering both fixes
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/correlation/correlation_engine.hpp | Added fault_severities map field to PendingCluster struct to store per-fault severity |
| src/ros2_medkit_fault_manager/src/correlation/correlation_engine.cpp | Implemented severity tracking, HIGHEST_SEVERITY representative reassignment logic, active cluster sync, and fault_to_cluster_ cleanup on expiration |
| src/ros2_medkit_fault_manager/test/test_correlation_engine.cpp | Added two new tests verifying HIGHEST_SEVERITY reassignment and fault_to_cluster_ cleanup on expiration |
src/ros2_medkit_fault_manager/src/correlation/correlation_engine.cpp
Outdated
Show resolved
Hide resolved
src/ros2_medkit_fault_manager/src/correlation/correlation_engine.cpp
Outdated
Show resolved
Hide resolved
mfaferek93
requested changes
Feb 17, 2026
src/ros2_medkit_fault_manager/src/correlation/correlation_engine.cpp
Outdated
Show resolved
Hide resolved
…fault_to_cluster_ cleanup (selfpatch#213, selfpatch#214) Store per-fault severity in PendingCluster so HIGHEST_SEVERITY representative reassignment selects the actual next-highest fault instead of falling back to codes.front(). Also clean up fault_to_cluster_ entries when pending clusters expire in cleanup_expired(). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
… clear reassignment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
eclipse0922
force-pushed
the
fix/213-214-correlation-cleanup
branch
from
f02ace0 to
d772703
Compare
…or active clusters cleanup_expired() was unconditionally erasing fault_to_cluster_ entries for all faults in expired pending clusters. Since activated clusters share these entries (pending is not removed on activation), this broke process_clear() for active clusters — creating ghost clusters that could never be cleaned up. Now only erase fault_to_cluster_ entries if the cluster was never activated (not present in active_clusters_). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mfaferek93
approved these changes
Feb 18, 2026
Collaborator
mfaferek93
left a comment
mfaferek93
left a comment
There was a problem hiding this comment.
Both issues fixed, test added, CI green. Ship it.
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.
Summary
PendingClustersoHIGHEST_SEVERITYrepresentative reassignment selects the actual next-highest severity fault instead of falling back tocodes.front()(Store per-fault severity in PendingCluster for accurate HIGHEST_SEVERITY representative reassignment #213)fault_to_cluster_entries when pending clusters expire incleanup_expired()(cleanup_expired() should remove stale fault_to_cluster_ entries for expired pending clusters #214)Issue
Closes #213
Closes #214
Type of Change
What Changed
correlation_engine.hpp: Addedfault_severitiesmap toPendingClusterstruct to track per-fault severitycorrelation_engine.cpp/try_auto_cluster(): Record each fault's severity when added to a pending clustercorrelation_engine.cpp/process_clear(): Use stored severities to find the highest-severity remaining fault when the representative is cleared; also sync active cluster representative from the updated pending clustercorrelation_engine.cpp/cleanup_expired(): Erasefault_to_cluster_entries for all faults in an expired pending cluster before removing ittest_correlation_engine.cpp: Added 2 new tests covering both fixesTesting
colcon buildsucceedsChecklist
clang-format-18)🤖 Generated with Claude Code