-
Notifications
You must be signed in to change notification settings - Fork 5
Safe <-> Normal Mode switch #168
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…-core-reference into payload-mode
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
This PR implements safe <-> normal mode management with automatic entry/exit based on voltage thresholds, unintended reboot detection via a clean shutdown flag, and explicit SafeModeReason tracking. It updates the ModeManager interface and behavior, integrates ResetManager for clean reboot signaling, adjusts telemetry/packets, and adds integration tests and documentation to reflect the new logic.
- Add SafeModeReason enum and reason-aware forceSafeMode port; track CurrentSafeModeReason telemetry
- Implement voltage-based auto safe-mode entry (6.7V, 10s) and auto-exit (8.0V, 10s) with hysteresis and debounce
- Detect unintended reboots and enter safe mode with SYSTEM_FAULT; integrate ResetManager.prepareForReboot for clean shutdown marking
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds instructions to run a single integration test via TEST parameter for developer ease-of-use. |
| Makefile | Enhances test-integration target to accept TEST=<name |
| FprimeZephyrReference/test/long/mode_manager_test.py | Aligns tests with new enum values and idempotent FORCE_SAFE_MODE behavior; minor doc/assert updates needed to remove payload mode mention. |
| FprimeZephyrReference/test/int/safe_mode_test.py | New integration tests validating safe mode reasons, manual commands, and manual scenarios for voltage-based auto-transitions and reboot detection. |
| FprimeZephyrReference/ReferenceDeployment/Top/topology.fpp | Wires ResetManager.prepareForReboot to ModeManager.prepareForReboot to support clean shutdown signaling. |
| FprimeZephyrReference/ReferenceDeployment/Top/ReferenceDeploymentPackets.fppi | Adds CurrentSafeModeReason to health packet for GDS visibility. |
| FprimeZephyrReference/Components/ResetManager/ResetManager.fpp | Introduces prepareForReboot output port to notify ModeManager before reboot. |
| FprimeZephyrReference/Components/ResetManager/ResetManager.cpp | Calls prepareForReboot before sys_reboot to set clean shutdown flag when connected. |
| FprimeZephyrReference/Components/ModeManager/docs/sdd.md | Updates SDD: reasons, thresholds, debounce, auto-recovery behavior, prepareForReboot flow, and load switch behavior. |
| FprimeZephyrReference/Components/ModeManager/ModeManager.hpp | Updates interface: reason-aware forceSafeMode, prepareForReboot handler, new constants and state; changes SystemMode mapping to SAFE_MODE=1, NORMAL=2. |
| FprimeZephyrReference/Components/ModeManager/ModeManager.fpp | Defines SafeModeReason enum, updates ports (ForceSafeModeWithReason), events (auto entry/exit, reboot detection), and CurrentSafeModeReason telemetry. |
| FprimeZephyrReference/Components/ModeManager/ModeManager.cpp | Implements voltage-based transitions, unintended reboot detection via persisted state, reason tracking, idempotent FORCE_SAFE_MODE, and updated load switch sequencing and telemetry. |
ineskhou
left a comment
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.
Looks great! Just a few comments. Will run stuff with the voltage and command and stuff ASAP but might be after vibe test
| m_mode(SystemMode::NORMAL), | ||
| m_safeModeEntryCount(0), | ||
| m_runCounter(0), | ||
| m_safeModeReason(Components::SafeModeReason::NONE), |
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.
Is safemodereason available across boots? Assuming that we reboot after safe mode is enabled, and later want to know why we are in safe mode, I think it could be useful to write to a file and with an event or telemetry read from the file.
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.
Yes, SafeModeReason is already persisted across reboots. The implementation stores it in /mode_state.bin as part of the PersistentState structure
| Os::File::Status status = file.open(STATE_FILE_PATH, Os::File::OPEN_CREATE); | ||
|
|
||
| if (status != Os::File::OP_OK) { | ||
| // Log failure - next boot will be misclassified as unintended reboot |
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.
Is the file isn't writable the next boot will be unintended? What does that mean?
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.
- Logs
StatePersistenceFailurewarning - Continues without setting
cleanShutdownflag - Next boot will be misclassified as unintended reboot → enters safe mode with
SYSTEM_FAULT
Pull Request Title (e.g., Feature: Add user authentication)
Description
###Summary
###Test plan
Related Issues/Tickets
#106
This is part of the Mode Manager
How Has This Been Tested?
Screenshots / Recordings (if applicable)
Checklist
Further Notes / Considerations