-
Notifications
You must be signed in to change notification settings - Fork 2
fix(fault_manager): allow CLEARED faults to be reactivated by FAILED … #145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Fixes fault reactivation behavior so that faults in CLEARED state can transition back to an active state when new FAILED events arrive (closing #144).
Changes:
- Allow
CLEARED→ (PREFAILED/CONFIRMED) transitions on newFAILEDevents (both in-memory and SQLite storage). - Reset debounce counter on reactivation and increment
occurrence_count, updating sources/description as appropriate. - Update/extend unit + integration tests to cover cleared-fault reactivation behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/ros2_medkit_fault_manager/src/fault_storage.cpp |
Enables reactivation of CLEARED faults in in-memory storage by handling FAILED events specially. |
src/ros2_medkit_fault_manager/src/sqlite_fault_storage.cpp |
Enables reactivation of CLEARED faults in SQLite storage by updating status/counters/sources on FAILED events. |
src/ros2_medkit_fault_manager/test/test_fault_manager.cpp |
Updates unit tests for in-memory storage to validate reactivation, PASSED-ignore behavior, and debounce restart. |
src/ros2_medkit_fault_manager/test/test_sqlite_storage.cpp |
Updates unit tests for SQLite storage to validate reactivation, PASSED-ignore behavior, and debounce restart. |
src/ros2_medkit_fault_manager/test/test_integration.test.py |
Adds an integration test ensuring a CLEARED fault becomes active again after a new FAILED report. |
18fa1d7 to
d2013b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
…events
Previously, report_fault_event() had an early return that silently
ignored all events for CLEARED faults. This caused faults to remain
CLEARED forever even when the underlying issue recurred.
Changes:
- FAILED events now reactivate CLEARED faults with debounce counter
reset to -1 (fresh start)
- PASSED events for CLEARED faults are still ignored (expected)
- occurrence_count increments on reactivation
- Return true to trigger EVENT_CONFIRMED and snapshot capture
Updated tests to reflect new behavior and added integration test.
d2013b8 to
1843cee
Compare
Pull Request
Summary
allow CLEARED faults to be reactivated by FAILED events
Previously, report_fault_event() had an early return that silently
ignored all events for CLEARED faults. This caused faults to remain
CLEARED forever even when the underlying issue recurred.
Changes:
reset to -1 (fresh start)
Updated tests to reflect new behavior and added integration test.
Issue
Link the related issue (required):
Type
Testing
How was this tested / how should reviewers verify it?
Checklist