fix: clean up pending_clusters_ when fault cleared before min_count#211
Merged
mfaferek93 merged 2 commits intoselfpatch:mainfrom Feb 15, 2026
Merged
Conversation
selfpatch#127) When a fault was cleared before a pending cluster reached min_count, the cluster retained a phantom reference to the cleared fault code. This could cause incorrect cluster activation with stale fault data. - Remove cleared fault from pending cluster's fault_codes list - Reassign representative if cleared fault was the representative - Remove pending cluster entirely if all faults are cleared - Add 3 unit tests covering the fix Closes selfpatch#127 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #127 by properly cleaning up pending_clusters_ when a fault is cleared before its cluster reaches min_count. The fix ensures that cleared fault codes are removed from pending clusters, representatives are reassigned when necessary, and empty pending clusters are removed entirely.
Changes:
- Enhanced
process_clear()to iterate throughpending_clusters_and remove cleared faults from their respective clusters - Implemented representative reassignment logic when the cleared fault was the cluster representative, supporting FIRST, MOST_RECENT, and HIGHEST_SEVERITY policies
- Added logic to remove pending clusters entirely when all their faults are cleared
- Added three comprehensive tests covering partial clears, representative reassignment, and complete cluster removal
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/ros2_medkit_fault_manager/src/correlation/correlation_engine.cpp | Implements cleanup logic in process_clear() to remove cleared faults from pending clusters, reassign representatives, and remove empty clusters (lines 142-179) |
| src/ros2_medkit_fault_manager/test/test_correlation_engine.cpp | Adds three tests: ClearFaultRemovesFromPendingCluster, ClearRepresentativeReassignsPendingCluster, and ClearAllFaultsRemovesPendingCluster to verify the fix |
mfaferek93
reviewed
Feb 15, 2026
Address review feedback: convert the HIGHEST_SEVERITY fallback comment into a tracked TODO referencing issue selfpatch#213. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
pending_clusters_when a fault is cleared before the cluster reachesmin_countfault_codeslistTest plan
ClearFaultRemovesFromPendingCluster— clearing one fault from pending cluster prevents premature activationClearRepresentativeReassignsPendingCluster— representative is reassigned after clearClearAllFaultsRemovesPendingCluster— clearing all faults removes the pending clusterclang-formatandament_copyrightpassCloses #127
🤖 Generated with Claude Code