diff --git a/PROVESFlightControllerReference/Components/StartupManager/StartupManager.cpp b/PROVESFlightControllerReference/Components/StartupManager/StartupManager.cpp index 22609ff2..128f92bd 100644 --- a/PROVESFlightControllerReference/Components/StartupManager/StartupManager.cpp +++ b/PROVESFlightControllerReference/Components/StartupManager/StartupManager.cpp @@ -7,6 +7,7 @@ #include "PROVESFlightControllerReference/Components/StartupManager/StartupManager.hpp" #include "Os/File.hpp" +#include "Os/FileSystem.hpp" #include "PROVESFlightControllerReference/Components/StartupManager/HardCodedStartup.h" #include @@ -94,7 +95,10 @@ StartupManager::Status write(const Fw::StringBase& file_path, const T& value) { if (status == Os::File::OP_OK) { FwSizeType size = sizeof(data_buffer); status = file.write(data_buffer, size); - if (status == Os::File::OP_OK && size == sizeof(data_buffer)) { + // Flush before close so the data has reached storage before callers (e.g. the atomic + // rename in persist_boot_count) treat the write as durable. close() returns void and + // cannot report a flush failure. + if (status == Os::File::OP_OK && size == sizeof(data_buffer) && file.flush() == Os::File::OP_OK) { return_status = StartupManager::SUCCESS; } } @@ -102,6 +106,11 @@ StartupManager::Status write(const Fw::StringBase& file_path, const T& value) { return return_status; } +// Boot counts beyond this are treated as file corruption rather than real history: a hard reset +// (e.g. the watchdog power cycle used for command-loss recovery) can tear the flash write and leave +// a well-formed file full of junk, which would otherwise be incremented and persisted forever. +static constexpr FwSizeType MAX_PLAUSIBLE_BOOT_COUNT = 1000000; + FwSizeType StartupManager ::get_boot_count(bool increment) { // Read the boot count file path from parameter and assert that it is either valid or the default value FwSizeType boot_count = 0; @@ -109,20 +118,45 @@ FwSizeType StartupManager ::get_boot_count(bool increment) { auto boot_count_file = this->paramGet_BOOT_COUNT_FILE(is_valid); FW_ASSERT(is_valid == Fw::ParamValid::VALID || is_valid == Fw::ParamValid::DEFAULT); - // Open the boot count file and add one to the current boot count ensuring a minimum of 1 in the case - // of read failure. Since read will retain the `0` initial value on read failure, we can ignore the error - // status returned by the read. + // Read the current count ensuring a minimum of 1 after increment in the case of read failure. + // Since read will retain the `0` initial value on read failure, we can ignore the error status. (void)read(boot_count_file, boot_count); + if (boot_count > MAX_PLAUSIBLE_BOOT_COUNT) { + this->log_WARNING_HI_BootCountCorrupted(static_cast(boot_count)); + boot_count = 0; + } boot_count = FW_MAX(1, increment ? boot_count + 1 : boot_count); - // Rewrite the updated boot count back to the file, and on failure emit a warning about the inability to - // persist the boot count. - StartupManager::Status status = write(boot_count_file, boot_count); - if (status != StartupManager::SUCCESS) { - this->log_WARNING_LO_BootCountUpdateFailure(); + + // Only the once-per-boot increment writes the file; plain reads (GET_BOOT_COUNT) must not. + // Every write is a window for a reset to tear the file, and the count is queried far more + // often than it changes. On failure, run_handler retries on subsequent ticks. + if (increment) { + StartupManager::Status status = this->persist_boot_count(boot_count_file, boot_count); + this->m_boot_count_persisted = (status == StartupManager::SUCCESS); + if (!this->m_boot_count_persisted && !this->m_boot_count_write_logged) { + this->log_WARNING_LO_BootCountUpdateFailure(); + this->m_boot_count_write_logged = true; + } } return boot_count; } +StartupManager::Status StartupManager ::persist_boot_count(const Fw::StringBase& file_path, FwSizeType value) { + // Write to a temp file, then rename over the target. The flight FS is FAT (ELM FatFs), whose + // rename is not guaranteed power-cut atomic - but the new data is fully written and flushed + // before it replaces the old file, so a reset can no longer tear the value mid-write (the + // observed failure). Worst case during the rename window is a missing file, which reads as a + // failed read and re-initializes the count - detectable, unlike silent garbage. + Fw::String temp_path(file_path); + temp_path += ".tmp"; + StartupManager::Status status = write(temp_path, value); + if (status == StartupManager::SUCCESS && + Os::FileSystem::rename(temp_path.toChar(), file_path.toChar()) != Os::FileSystem::OP_OK) { + status = StartupManager::FAILURE; + } + return status; +} + Fw::Time StartupManager ::update_quiescence_start() { Fw::ParamValid is_valid; auto time_file = this->paramGet_QUIESCENCE_START_FILE(is_valid); @@ -260,6 +294,16 @@ void StartupManager ::run_handler(FwIndexType portNum, U32 context) { this->runSequence_out(0, first_sequence); this->m_transmit_enable_ticks = this->paramGet_TRANSMIT_ENABLE_TICKS(is_valid); FW_ASSERT(is_valid == Fw::ParamValid::VALID || is_valid == Fw::ParamValid::DEFAULT); + } else if (!this->m_boot_count_persisted) { + // The first-tick write can fail transiently (e.g. filesystem not ready yet). Re-attempt each + // tick until the count is durably stored, so the increment is delayed rather than lost. + auto boot_count_file = this->paramGet_BOOT_COUNT_FILE(is_valid); + FW_ASSERT(is_valid == Fw::ParamValid::VALID || is_valid == Fw::ParamValid::DEFAULT); + this->m_boot_count_persisted = + (this->persist_boot_count(boot_count_file, this->m_boot_count) == StartupManager::SUCCESS); + if (this->m_boot_count_persisted) { + this->m_boot_count_write_logged = false; + } } #if DEFAULT_STARTUP_VALUE == 1 diff --git a/PROVESFlightControllerReference/Components/StartupManager/StartupManager.fpp b/PROVESFlightControllerReference/Components/StartupManager/StartupManager.fpp index ccb96a99..2d746732 100644 --- a/PROVESFlightControllerReference/Components/StartupManager/StartupManager.fpp +++ b/PROVESFlightControllerReference/Components/StartupManager/StartupManager.fpp @@ -54,6 +54,10 @@ module Components { event BootCountUpdateFailure() severity warning low \ format "Failed to update boot count file" + @ Event emitted when the boot count file holds an implausible value (torn or corrupt write) + event BootCountCorrupted(raw: I64) severity warning high \ + format "Boot count file corrupt (raw value {}) - treating as unreadable" + @ Event emitted when the quiescence file was not updated event QuiescenceFileInitFailure() severity warning low \ format "Failed to initialize quiescence start time file" diff --git a/PROVESFlightControllerReference/Components/StartupManager/StartupManager.hpp b/PROVESFlightControllerReference/Components/StartupManager/StartupManager.hpp index 25b56e6b..e0185cbc 100644 --- a/PROVESFlightControllerReference/Components/StartupManager/StartupManager.hpp +++ b/PROVESFlightControllerReference/Components/StartupManager/StartupManager.hpp @@ -30,16 +30,27 @@ class StartupManager final : public StartupManagerComponentBase { //! Destroy StartupManager object ~StartupManager(); - //! \brief read and increment the boot count + //! \brief read and optionally increment the boot count //! - //! Reads the boot count from the boot count file, increments it, and writes it back to the file. If the read - //! fails, the boot count will be initialized to 1. If the write fails, a warning will be emitted. + //! Reads the boot count from the boot count file. If the read fails or the stored value is implausible + //! (torn/corrupt write), the count is treated as unread and initializes to 1 on increment. Only the + //! increment path writes the file; plain reads leave it untouched. If the write fails, a warning is + //! emitted and the write is retried on subsequent run ticks. //! - //! \warning this function will modify the boot count file on disk. + //! \warning this function will modify the boot count file on disk when increment is true. //! //! \return The updated boot count FwSizeType get_boot_count(bool increment); + //! \brief durably persist the boot count via write-to-temp + rename + //! + //! The new value is fully written and flushed before it replaces the old file, so a reset + //! cannot tear the value mid-write. FAT's rename is not power-cut atomic; the residual worst + //! case is a missing file, which reads as a failed read rather than silent garbage. + //! + //! \return Status of the persist operation + Status persist_boot_count(const Fw::StringBase& file_path, FwSizeType value); + //! \brief get and possibly initialize the quiescence start time //! //! Reads the quiescence start time from the quiescence start time file. If the read fails, the current time is @@ -141,7 +152,9 @@ class StartupManager final : public StartupManagerComponentBase { private: Fw::Time m_quiescence_start; //!< Time of the start of the quiescence wait FwOpcodeType m_stored_opcode; //!< Stored opcode for delayed response - FwSizeType m_boot_count; //!< Current boot count + FwSizeType m_boot_count = 0; //!< Current boot count (0 = first run tick not yet processed) + bool m_boot_count_persisted = false; //!< Whether the incremented boot count has reached the file + bool m_boot_count_write_logged = false; //!< Warning already emitted for the current persist-failure streak U32 m_stored_sequence; //!< Stored sequence number for delayed response std::atomic m_waiting; //!< Indicates if waiting for quiescence Fw::String m_sequence_file; //!< The filepath for the sequence last initiated diff --git a/PROVESFlightControllerReference/Components/StartupManager/docs/sdd.md b/PROVESFlightControllerReference/Components/StartupManager/docs/sdd.md index 3ef0dce3..fa87289a 100644 --- a/PROVESFlightControllerReference/Components/StartupManager/docs/sdd.md +++ b/PROVESFlightControllerReference/Components/StartupManager/docs/sdd.md @@ -8,7 +8,7 @@ The StartupManager component manages boot counting, quiescence waiting periods, The StartupManager serves four primary functions: -1. Boot Counting: Tracks the number of system boots persistently across power cycles +1. Boot Counting: Tracks the number of system boots persistently across power cycles, hardened against file corruption from hard resets (see [Boot count persistence](#boot-count-persistence)) 2. Quiescence Wait: Implements a configurable waiting period (default 45 minutes) before allowing full system startup, useful for missions requiring initial stabilization 3. Startup Sequence: Automatically dispatches and monitors the execution of startup command sequences 4. Hard-coded Radio Enable: When enabled at compile time (`DEFAULT_STARTUP_VALUE == 1` in `HardCodedStartup.h`), after `TRANSMIT_ENABLE_TICKS` 1 Hz run ticks, asserts `enableTransmit` to enable LoRa transmission independently of the startup sequence file. When disabled (`DEFAULT_STARTUP_VALUE == 0`), the countdown does not run and transmit is not asserted automatically — preferred for ground testing so the radio does not turn on accidentally. @@ -48,6 +48,15 @@ On each 1 Hz `run`, when `DEFAULT_STARTUP_VALUE == 1` and `m_transmit_enable_tic `HardCodedStartup.h` defines `DEFAULT_STARTUP_VALUE`: +### Boot count persistence + +The boot count lives in `BOOT_COUNT_FILE` on the flight filesystem (FAT — ELM FatFs, see `prj.conf`) and is incremented lazily on the first 1 Hz `run` tick of each boot. Hard resets (e.g. the watchdog power cycle used for command-loss recovery) can land while a write is in flight, so the persistence path is hardened in four ways: + +1. **Corruption guard**: a read that returns a value above 1,000,000 is treated as a failed read (the file contained torn/junk data) and reported via `BootCountCorrupted` with the raw value. The count then re-initializes on the next increment instead of propagating garbage. Observed on HWIL: a torn write left the file reading `0x02FE191005000001`, and the next boot persisted exactly garbage+1. +2. **Read-only queries**: `GET_BOOT_COUNT` never writes the file. Only the once-per-boot increment does, minimizing the window in which a reset can tear a write. +3. **Increment retry**: if the first-tick persist fails (e.g. filesystem not ready), `run` re-attempts it each tick until it succeeds — the increment is delayed, not lost. `BootCountUpdateFailure` is emitted once per failure streak. +4. **Write-then-rename persist**: the value is written to `.tmp`, flushed, and renamed over the target. FAT's rename is not guaranteed power-cut atomic, but the new data is fully on storage before it replaces the old file, closing the torn-in-place-write window that produced the observed garbage. The residual worst case during the rename window is a missing file, which reads as a failed read (count re-initializes, visibly) rather than silent corruption. + ## Port Descriptions @@ -65,6 +74,8 @@ On each 1 Hz `run`, when `DEFAULT_STARTUP_VALUE == 1` and `m_transmit_enable_tic | State Variable | Type | Description | |----------------|------|-------------| | `m_boot_count` | `FwSizeType` | Current boot count. Zero indicates uninitialized state | +| `m_boot_count_persisted` | `bool` | Whether the incremented boot count has durably reached the file; drives the per-tick retry | +| `m_boot_count_write_logged` | `bool` | `BootCountUpdateFailure` already emitted for the current persist-failure streak | | `m_quiescence_start` | `Fw::Time` | Time when quiescence period started (mission epoch) | | `m_waiting` | `std::atomic` | True when waiting for quiescence period to elapse | | `m_stored_opcode` | `FwOpcodeType` | Opcode of pending `WAIT_FOR_QUIESCENCE` command | @@ -95,7 +106,8 @@ On each 1 Hz `run`, when `DEFAULT_STARTUP_VALUE == 1` and `m_transmit_enable_tic | Name | Severity | Arguments | Description | |------|----------|-----------|-------------| | `CurrentBootCount` | ACTIVITY_LO | `i: I64` | Emitted by `GET_BOOT_COUNT` with the current boot count | -| `BootCountUpdateFailure` | WARNING_LO | None | Emitted when the boot count file cannot be updated. Boot count was incremented in memory but not persisted | +| `BootCountUpdateFailure` | WARNING_LO | None | Emitted once per failure streak when the boot count file cannot be updated. The increment is retried on each subsequent `run` tick until it persists | +| `BootCountCorrupted` | WARNING_HI | `raw: I64` | Emitted when the boot count file holds an implausible value (> 1,000,000), indicating a torn or corrupt write. The value is treated as unreadable | | `QuiescenceFileInitFailure` | WARNING_LO | None | Emitted when the quiescence start time file cannot be initialized. System will use current time but cannot persist it | | `StartupSequenceFinished` | ACTIVITY_LO | None | Emitted when the startup sequence completes successfully | | `StartupSequenceFailed` | WARNING_LO | `response: Fw.CmdResponse` | Emitted when the startup sequence fails, includes the failure response code | @@ -121,3 +133,5 @@ On each 1 Hz `run`, when `DEFAULT_STARTUP_VALUE == 1` and `m_transmit_enable_tic | REQ-SM-007 | StartupManager shall handle file I/O errors gracefully | Verification: Remove file permissions and verify warning events are emitted | | REQ-SM-008 | When `DEFAULT_STARTUP_VALUE == 1`, StartupManager shall enable LoRa transmit after `TRANSMIT_ENABLE_TICKS` 1 Hz ticks | Verification: Confirm `HardcodedRadioEnable` and RF transmit after the configured delay | | REQ-SM-009 | When `DEFAULT_STARTUP_VALUE == 0`, StartupManager shall not assert `enableTransmit` from the hard-coded countdown | Verification: Build with gate disabled and confirm no `HardcodedRadioEnable` / automatic TX after boot | +| REQ-SM-010 | StartupManager shall not propagate an implausible boot count read from a corrupt file | Verification: Write junk to `BOOT_COUNT_FILE`, reboot, confirm `BootCountCorrupted` and a re-initialized count | +| REQ-SM-011 | StartupManager shall persist the boot count atomically and retry a failed increment until it is durably stored | Verification: HWIL `test_safe_09` asserts boot count == initial+1 across a watchdog hard reset | diff --git a/docs-site/components/StartupManager.md b/docs-site/components/StartupManager.md index 3ef0dce3..fa87289a 100644 --- a/docs-site/components/StartupManager.md +++ b/docs-site/components/StartupManager.md @@ -8,7 +8,7 @@ The StartupManager component manages boot counting, quiescence waiting periods, The StartupManager serves four primary functions: -1. Boot Counting: Tracks the number of system boots persistently across power cycles +1. Boot Counting: Tracks the number of system boots persistently across power cycles, hardened against file corruption from hard resets (see [Boot count persistence](#boot-count-persistence)) 2. Quiescence Wait: Implements a configurable waiting period (default 45 minutes) before allowing full system startup, useful for missions requiring initial stabilization 3. Startup Sequence: Automatically dispatches and monitors the execution of startup command sequences 4. Hard-coded Radio Enable: When enabled at compile time (`DEFAULT_STARTUP_VALUE == 1` in `HardCodedStartup.h`), after `TRANSMIT_ENABLE_TICKS` 1 Hz run ticks, asserts `enableTransmit` to enable LoRa transmission independently of the startup sequence file. When disabled (`DEFAULT_STARTUP_VALUE == 0`), the countdown does not run and transmit is not asserted automatically — preferred for ground testing so the radio does not turn on accidentally. @@ -48,6 +48,15 @@ On each 1 Hz `run`, when `DEFAULT_STARTUP_VALUE == 1` and `m_transmit_enable_tic `HardCodedStartup.h` defines `DEFAULT_STARTUP_VALUE`: +### Boot count persistence + +The boot count lives in `BOOT_COUNT_FILE` on the flight filesystem (FAT — ELM FatFs, see `prj.conf`) and is incremented lazily on the first 1 Hz `run` tick of each boot. Hard resets (e.g. the watchdog power cycle used for command-loss recovery) can land while a write is in flight, so the persistence path is hardened in four ways: + +1. **Corruption guard**: a read that returns a value above 1,000,000 is treated as a failed read (the file contained torn/junk data) and reported via `BootCountCorrupted` with the raw value. The count then re-initializes on the next increment instead of propagating garbage. Observed on HWIL: a torn write left the file reading `0x02FE191005000001`, and the next boot persisted exactly garbage+1. +2. **Read-only queries**: `GET_BOOT_COUNT` never writes the file. Only the once-per-boot increment does, minimizing the window in which a reset can tear a write. +3. **Increment retry**: if the first-tick persist fails (e.g. filesystem not ready), `run` re-attempts it each tick until it succeeds — the increment is delayed, not lost. `BootCountUpdateFailure` is emitted once per failure streak. +4. **Write-then-rename persist**: the value is written to `.tmp`, flushed, and renamed over the target. FAT's rename is not guaranteed power-cut atomic, but the new data is fully on storage before it replaces the old file, closing the torn-in-place-write window that produced the observed garbage. The residual worst case during the rename window is a missing file, which reads as a failed read (count re-initializes, visibly) rather than silent corruption. + ## Port Descriptions @@ -65,6 +74,8 @@ On each 1 Hz `run`, when `DEFAULT_STARTUP_VALUE == 1` and `m_transmit_enable_tic | State Variable | Type | Description | |----------------|------|-------------| | `m_boot_count` | `FwSizeType` | Current boot count. Zero indicates uninitialized state | +| `m_boot_count_persisted` | `bool` | Whether the incremented boot count has durably reached the file; drives the per-tick retry | +| `m_boot_count_write_logged` | `bool` | `BootCountUpdateFailure` already emitted for the current persist-failure streak | | `m_quiescence_start` | `Fw::Time` | Time when quiescence period started (mission epoch) | | `m_waiting` | `std::atomic` | True when waiting for quiescence period to elapse | | `m_stored_opcode` | `FwOpcodeType` | Opcode of pending `WAIT_FOR_QUIESCENCE` command | @@ -95,7 +106,8 @@ On each 1 Hz `run`, when `DEFAULT_STARTUP_VALUE == 1` and `m_transmit_enable_tic | Name | Severity | Arguments | Description | |------|----------|-----------|-------------| | `CurrentBootCount` | ACTIVITY_LO | `i: I64` | Emitted by `GET_BOOT_COUNT` with the current boot count | -| `BootCountUpdateFailure` | WARNING_LO | None | Emitted when the boot count file cannot be updated. Boot count was incremented in memory but not persisted | +| `BootCountUpdateFailure` | WARNING_LO | None | Emitted once per failure streak when the boot count file cannot be updated. The increment is retried on each subsequent `run` tick until it persists | +| `BootCountCorrupted` | WARNING_HI | `raw: I64` | Emitted when the boot count file holds an implausible value (> 1,000,000), indicating a torn or corrupt write. The value is treated as unreadable | | `QuiescenceFileInitFailure` | WARNING_LO | None | Emitted when the quiescence start time file cannot be initialized. System will use current time but cannot persist it | | `StartupSequenceFinished` | ACTIVITY_LO | None | Emitted when the startup sequence completes successfully | | `StartupSequenceFailed` | WARNING_LO | `response: Fw.CmdResponse` | Emitted when the startup sequence fails, includes the failure response code | @@ -121,3 +133,5 @@ On each 1 Hz `run`, when `DEFAULT_STARTUP_VALUE == 1` and `m_transmit_enable_tic | REQ-SM-007 | StartupManager shall handle file I/O errors gracefully | Verification: Remove file permissions and verify warning events are emitted | | REQ-SM-008 | When `DEFAULT_STARTUP_VALUE == 1`, StartupManager shall enable LoRa transmit after `TRANSMIT_ENABLE_TICKS` 1 Hz ticks | Verification: Confirm `HardcodedRadioEnable` and RF transmit after the configured delay | | REQ-SM-009 | When `DEFAULT_STARTUP_VALUE == 0`, StartupManager shall not assert `enableTransmit` from the hard-coded countdown | Verification: Build with gate disabled and confirm no `HardcodedRadioEnable` / automatic TX after boot | +| REQ-SM-010 | StartupManager shall not propagate an implausible boot count read from a corrupt file | Verification: Write junk to `BOOT_COUNT_FILE`, reboot, confirm `BootCountCorrupted` and a re-initialized count | +| REQ-SM-011 | StartupManager shall persist the boot count atomically and retry a failed increment until it is durably stored | Verification: HWIL `test_safe_09` asserts boot count == initial+1 across a watchdog hard reset |