diff --git a/PROVESFlightControllerReference/Components/CMakeLists.txt b/PROVESFlightControllerReference/Components/CMakeLists.txt index 8c7565f14..f2ddc6601 100644 --- a/PROVESFlightControllerReference/Components/CMakeLists.txt +++ b/PROVESFlightControllerReference/Components/CMakeLists.txt @@ -23,6 +23,7 @@ add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/PayloadCom/") add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/PowerMonitor/") add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/ResetManager/") #add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/SBand/") +add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/StackMonitor/") add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/StartupManager/") add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/ThermalManager/") add_fprime_subdirectory("${CMAKE_CURRENT_LIST_DIR}/Watchdog") diff --git a/PROVESFlightControllerReference/Components/StackMonitor/CMakeLists.txt b/PROVESFlightControllerReference/Components/StackMonitor/CMakeLists.txt new file mode 100644 index 000000000..a44764519 --- /dev/null +++ b/PROVESFlightControllerReference/Components/StackMonitor/CMakeLists.txt @@ -0,0 +1,25 @@ +#### +# F Prime CMakeLists.txt: +# +# SOURCES: list of source files (to be compiled) +# AUTOCODER_INPUTS: list of files to be passed to the autocoders +# DEPENDS: list of libraries that this module depends on +# +# More information in the F´ CMake API documentation: +# https://fprime.jpl.nasa.gov/latest/docs/reference/api/cmake/API/ +# +#### + +# Module names are derived from the path from the nearest project/library/framework +# root when not specifically overridden by the developer. i.e. The module defined by +# `Ref/SignalGen/CMakeLists.txt` will be named `Ref_SignalGen`. + +register_fprime_library( + AUTOCODER_INPUTS + "${CMAKE_CURRENT_LIST_DIR}/StackMonitor.fpp" + SOURCES + "${CMAKE_CURRENT_LIST_DIR}/StackMonitor.cpp" + "${CMAKE_CURRENT_LIST_DIR}/StackMonitorCore.cpp" +# DEPENDS +# MyPackage_MyOtherModule +) diff --git a/PROVESFlightControllerReference/Components/StackMonitor/StackMonitor.cpp b/PROVESFlightControllerReference/Components/StackMonitor/StackMonitor.cpp new file mode 100644 index 000000000..58d8ace40 --- /dev/null +++ b/PROVESFlightControllerReference/Components/StackMonitor/StackMonitor.cpp @@ -0,0 +1,81 @@ +// ====================================================================== +// \title StackMonitor.cpp +// \brief cpp file for StackMonitor component implementation class +// ====================================================================== + +#include "PROVESFlightControllerReference/Components/StackMonitor/StackMonitor.hpp" + +#include + +namespace Components { + +namespace { + +//! k_thread_foreach_unlocked callback: append this thread's stack usage to +//! the ThreadStackSampleSet pointed to by userData. Called without the +//! thread-monitor spinlock held, so it must not allocate or block: it only +//! reads the stack high-water mark, does a bounded string copy of the name, +//! and stores integers into a fixed slot (ThreadStackSampleSet::add). Keeps +//! all Zephyr calls out of StackMonitorCore, which stays pure. +void appendThreadSample(const struct k_thread* thread, void* userData) { + auto* samples = static_cast(userData); + + std::size_t unusedBytes = 0; + if (k_thread_stack_space_get(thread, &unusedBytes) != 0) { + // Couldn't read this thread's stack info (e.g. mid-teardown); skip + // it rather than report a bogus sample. The next tick will pick it + // back up if it's still alive. + return; + } + + const char* name = "unknown"; +#if defined(CONFIG_THREAD_NAME) + // thread is logically read-only here, but k_thread_name_get takes a + // non-const k_tid_t; the callback signature is fixed by + // k_thread_foreach_unlocked and always hands us a live thread object. + const char* threadName = k_thread_name_get(const_cast(thread)); + if (threadName != nullptr) { + name = threadName; + } +#endif + + (void)samples->add(name, static_cast(thread->stack_info.size), + static_cast(unusedBytes)); +} + +} // namespace + +// ---------------------------------------------------------------------- +// Component construction and destruction +// ---------------------------------------------------------------------- + +StackMonitor ::StackMonitor(const char* const compName) + : StackMonitorComponentBase(compName), m_core(WARN_THRESHOLD_PERCENT), m_samples(), m_result() {} + +StackMonitor ::~StackMonitor() {} + +// ---------------------------------------------------------------------- +// Handler implementations for user-defined typed input ports +// ---------------------------------------------------------------------- + +void StackMonitor ::run_handler(FwIndexType portNum, U32 context) { + this->m_samples.clear(); + k_thread_foreach_unlocked(&appendThreadSample, &this->m_samples); + + this->m_core.tick(this->m_samples, this->m_result); + + this->tlmWrite_MinFreeBytes(this->m_result.summary.worstThreadFreeBytes); + this->tlmWrite_WorstThread(Fw::TlmString(this->m_result.summary.worstThreadName)); + this->tlmWrite_ThreadsBelowThreshold(this->m_result.summary.threadsBelowThreshold); + this->tlmWrite_SampleOverflow(this->m_result.summary.overflowed); + + for (std::uint32_t i = 0; i < this->m_result.warningCount; i++) { + const StackWarning& warning = this->m_result.newWarnings[i]; + this->log_WARNING_HI_StackLow(Fw::LogStringArg(warning.name), warning.freeBytes, warning.sizeBytes); + } + for (std::uint32_t i = 0; i < this->m_result.recoveryCount; i++) { + this->log_ACTIVITY_HI_StackRecovered(Fw::LogStringArg(this->m_result.newRecoveries[i].name)); + } +} + +} // namespace Components diff --git a/PROVESFlightControllerReference/Components/StackMonitor/StackMonitor.fpp b/PROVESFlightControllerReference/Components/StackMonitor/StackMonitor.fpp new file mode 100644 index 000000000..8b68e6088 --- /dev/null +++ b/PROVESFlightControllerReference/Components/StackMonitor/StackMonitor.fpp @@ -0,0 +1,67 @@ +module Components { + @ Program-wide stack-usage watchdog. Walks all live Zephyr threads once per + @ tick (k_thread_foreach) and reports per-thread stack high-water + @ telemetry, warning when any thread's free stack drops below a configurable + @ percent of its own size and clearing on recovery. Fixed-capacity storage + @ throughout: zero heap allocation after construction. + @ S-Band reintegration plan, PR 1 / Slice 1.1 (D6). + passive component StackMonitor { + + @ Port receiving calls from the rate group + sync input port run: Svc.Sched + + @ Free bytes remaining on the thread under the most stack pressure this tick + telemetry MinFreeBytes: U32 + + @ Name of the thread under the most stack pressure this tick + telemetry WorstThread: string size 32 + + @ Count of threads currently below the warn threshold + telemetry ThreadsBelowThreshold: U32 + + @ True when a tick saw more live threads than the monitor's fixed + @ capacity (extra threads went unsampled that tick -- not silent) + telemetry SampleOverflow: bool + + @ Event logged when a thread's free stack drops below its warn threshold + event StackLow( + thread: string size 32 @< Name of the thread + freeBytes: U32 @< Free bytes remaining on the thread's stack + sizeBytes: U32 @< Total size of the thread's stack + ) \ + severity warning high \ + format "Thread {} stack low: {} of {} bytes free" + + @ Event logged when a previously-low thread recovers above its warn threshold + event StackRecovered( + thread: string size 32 @< Name of the thread + ) \ + severity activity high \ + format "Thread {} stack recovered" + + ############################################################################### + # Standard AC Ports: Required for Channels, Events, Commands, and Parameters # + ############################################################################### + @ Port for requesting the current time + time get port timeCaller + + @ Port for sending command registrations + command reg port cmdRegOut + + @ Port for receiving commands + command recv port cmdIn + + @ Port for sending command responses + command resp port cmdResponseOut + + @ Port for sending textual representation of events + text event port logTextOut + + @ Port for sending events to downlink + event port logOut + + @ Port for sending telemetry channels to downlink + telemetry port tlmOut + + } +} diff --git a/PROVESFlightControllerReference/Components/StackMonitor/StackMonitor.hpp b/PROVESFlightControllerReference/Components/StackMonitor/StackMonitor.hpp new file mode 100644 index 000000000..73e6bd4c2 --- /dev/null +++ b/PROVESFlightControllerReference/Components/StackMonitor/StackMonitor.hpp @@ -0,0 +1,57 @@ +// ====================================================================== +// \title StackMonitor.hpp +// \brief hpp file for StackMonitor component implementation class +// ====================================================================== + +#ifndef Components_StackMonitor_HPP +#define Components_StackMonitor_HPP + +#include "PROVESFlightControllerReference/Components/StackMonitor/StackMonitorComponentAc.hpp" +#include "PROVESFlightControllerReference/Components/StackMonitor/StackMonitorCore.hpp" + +namespace Components { + +class StackMonitor final : public StackMonitorComponentBase { + public: + // ---------------------------------------------------------------------- + // Component construction and destruction + // ---------------------------------------------------------------------- + + //! Construct StackMonitor object + StackMonitor(const char* const compName //!< The component name + ); + + //! Destroy StackMonitor object + ~StackMonitor(); + + private: + // ---------------------------------------------------------------------- + // Handler implementations for user-defined typed input ports + // ---------------------------------------------------------------------- + + //! Handler implementation for run + //! + //! Port receiving calls from the rate group + void run_handler(FwIndexType portNum, //!< The port number + U32 context //!< The call order + ) override; + + //! Warn when a thread's free stack drops below this percent of its own size. + static constexpr std::uint32_t WARN_THRESHOLD_PERCENT = 20; + + //! Pure-logic core: turns a snapshot of per-thread stack usage into a + //! summary and warn/clear decisions. Host-testable in isolation; see + //! test/unit-tests/test_StackMonitor_Core.cpp. + StackMonitorCore m_core; + + //! Per-tick sample and result storage. Fixed-size and kept as member + //! state (not run_handler locals): together they are a few KB, which + //! would not be comfortable on the 4 KB rate-group thread stack, and + //! keeping them here guarantees zero heap allocation after boot. + ThreadStackSampleSet m_samples; + StackMonitorTickResult m_result; +}; + +} // namespace Components + +#endif diff --git a/PROVESFlightControllerReference/Components/StackMonitor/StackMonitorCore.cpp b/PROVESFlightControllerReference/Components/StackMonitor/StackMonitorCore.cpp new file mode 100644 index 000000000..60b5ad5e3 --- /dev/null +++ b/PROVESFlightControllerReference/Components/StackMonitor/StackMonitorCore.cpp @@ -0,0 +1,143 @@ +// ====================================================================== +// \title StackMonitorCore.cpp +// \brief cpp file for StackMonitorCore pure-logic class +// ====================================================================== + +#include "PROVESFlightControllerReference/Components/StackMonitor/StackMonitorCore.hpp" + +#include + +namespace Components { + +namespace { + +//! Bounded copy of a thread name into a fixed-size slot, always +//! null-terminated. No allocation. +void copyName(char (&dst)[STACK_MONITOR_MAX_NAME_LEN], const char* src) { + (void)std::strncpy(dst, (src != nullptr) ? src : "", STACK_MONITOR_MAX_NAME_LEN - 1); + dst[STACK_MONITOR_MAX_NAME_LEN - 1] = '\0'; +} + +//! Free bytes, clamped so a corrupt/odd sample can never read as more free +//! than the thread's total stack size. +std::uint32_t clampedFreeBytes(const ThreadStackSample& sample) { + return (sample.freeBytes > sample.sizeBytes) ? sample.sizeBytes : sample.freeBytes; +} + +//! Percent of the thread's stack currently free, 0-100. +std::uint32_t freePercent(const ThreadStackSample& sample) { + if (sample.sizeBytes == 0) { + return 0; + } + return (clampedFreeBytes(sample) * 100) / sample.sizeBytes; +} + +} // namespace + +// ---------------------------------------------------------------------- +// ThreadStackSampleSet +// ---------------------------------------------------------------------- + +void ThreadStackSampleSet::clear() { + this->count = 0; + this->overflowed = false; +} + +bool ThreadStackSampleSet::add(const char* name, std::uint32_t sizeBytes, std::uint32_t freeBytes) { + if (this->count >= STACK_MONITOR_MAX_THREADS) { + this->overflowed = true; + return false; + } + ThreadStackSample& slot = this->samples[this->count]; + copyName(slot.name, name); + slot.sizeBytes = sizeBytes; + slot.freeBytes = freeBytes; + this->count++; + return true; +} + +// ---------------------------------------------------------------------- +// StackMonitorCore +// ---------------------------------------------------------------------- + +StackMonitorCore::StackMonitorCore(std::uint32_t warnThresholdPercent) + : m_warnThresholdPercent(warnThresholdPercent), m_warned() {} + +void StackMonitorCore::tick(const ThreadStackSampleSet& sampleSet, StackMonitorTickResult& result) { + result.summary = StackMonitorSummary(); + result.summary.overflowed = sampleSet.overflowed; + result.warningCount = 0; + result.recoveryCount = 0; + + bool haveWorst = false; + std::uint32_t worstFreePercent = 0; + + for (std::uint32_t i = 0; i < sampleSet.count; i++) { + const ThreadStackSample& sample = sampleSet.samples[i]; + std::uint32_t fPercent = freePercent(sample); + + if (!haveWorst || fPercent < worstFreePercent) { + haveWorst = true; + worstFreePercent = fPercent; + copyName(result.summary.worstThreadName, sample.name); + result.summary.worstThreadFreeBytes = sample.freeBytes; + result.summary.worstThreadUsedPercent = 100 - fPercent; + } + + bool isBelowThreshold = fPercent < m_warnThresholdPercent; + if (isBelowThreshold) { + result.summary.threadsBelowThreshold++; + } + + bool wasWarned = (this->findWarned(sample.name) >= 0); + if (isBelowThreshold && !wasWarned) { + if (result.warningCount < STACK_MONITOR_MAX_THREADS) { + StackWarning& warning = result.newWarnings[result.warningCount]; + copyName(warning.name, sample.name); + warning.freeBytes = sample.freeBytes; + warning.sizeBytes = sample.sizeBytes; + result.warningCount++; + } + this->setWarned(sample.name); + } else if (!isBelowThreshold && wasWarned) { + if (result.recoveryCount < STACK_MONITOR_MAX_THREADS) { + copyName(result.newRecoveries[result.recoveryCount].name, sample.name); + result.recoveryCount++; + } + this->clearWarned(sample.name); + } + } +} + +std::int32_t StackMonitorCore::findWarned(const char* name) const { + for (std::uint32_t i = 0; i < STACK_MONITOR_MAX_THREADS; i++) { + if (this->m_warned[i].used && (std::strncmp(this->m_warned[i].name, name, STACK_MONITOR_MAX_NAME_LEN) == 0)) { + return static_cast(i); + } + } + return -1; +} + +void StackMonitorCore::setWarned(const char* name) { + if (this->findWarned(name) >= 0) { + return; + } + for (std::uint32_t i = 0; i < STACK_MONITOR_MAX_THREADS; i++) { + if (!this->m_warned[i].used) { + copyName(this->m_warned[i].name, name); + this->m_warned[i].used = true; + return; + } + } + // Table full: the warning event is still emitted by tick(); this + // thread just isn't latched (it may re-warn on a later tick). +} + +void StackMonitorCore::clearWarned(const char* name) { + std::int32_t index = this->findWarned(name); + if (index >= 0) { + this->m_warned[index].used = false; + } +} + +} // namespace Components diff --git a/PROVESFlightControllerReference/Components/StackMonitor/StackMonitorCore.hpp b/PROVESFlightControllerReference/Components/StackMonitor/StackMonitorCore.hpp new file mode 100644 index 000000000..e0f822512 --- /dev/null +++ b/PROVESFlightControllerReference/Components/StackMonitor/StackMonitorCore.hpp @@ -0,0 +1,124 @@ +// ====================================================================== +// \title StackMonitorCore.hpp +// \brief hpp file for StackMonitorCore pure-logic class +// ====================================================================== + +#pragma once + +#include + +namespace Components { + +//! Fixed capacities: no heap allocation anywhere in the stack-monitoring +//! path (per-tick heap churn is not flight-safe; see issue #122 and the +//! S-Band reintegration plan). MAX_NAME_LEN matches Zephyr's default +//! CONFIG_THREAD_MAX_NAME_LEN; MAX_THREADS covers the 25-slot dynamic +//! thread pool plus static threads with margin. +static constexpr std::uint32_t STACK_MONITOR_MAX_NAME_LEN = 32; +static constexpr std::uint32_t STACK_MONITOR_MAX_THREADS = 32; + +//! One thread's stack usage as sampled for a single tick. POD with a +//! fixed-size name buffer: safe to fill inside k_thread_foreach's +//! spinlocked callback (integer stores + bounded string copy only). +struct ThreadStackSample { + char name[STACK_MONITOR_MAX_NAME_LEN]; + std::uint32_t sizeBytes; + std::uint32_t freeBytes; +}; + +//! Caller-owned, fixed-capacity set of samples for one tick. +//! add() performs only a bounds check, a bounded string copy, and integer +//! stores -- no allocation of any kind -- so it is safe to call from the +//! k_thread_foreach callback, which Zephyr runs with the thread-monitor +//! spinlock held and IRQs locked. +struct ThreadStackSampleSet { + ThreadStackSample samples[STACK_MONITOR_MAX_THREADS]; + std::uint32_t count = 0; + bool overflowed = false; + + //! Reset to empty (count/overflowed only; sample slots are overwritten on add). + void clear(); + + //! Append a sample. When the set is full the sample is dropped and the + //! overflowed flag is set (no silent truncation); returns false in that case. + bool add(const char* name, std::uint32_t sizeBytes, std::uint32_t freeBytes); +}; + +//! Per-tick summary across all sampled threads. +struct StackMonitorSummary { + char worstThreadName[STACK_MONITOR_MAX_NAME_LEN] = {0}; + std::uint32_t worstThreadFreeBytes = 0; + std::uint32_t worstThreadUsedPercent = 0; + std::uint32_t threadsBelowThreshold = 0; + bool overflowed = false; //!< True when this tick saw more threads than capacity +}; + +//! A thread that just crossed below its warn threshold this tick. +struct StackWarning { + char name[STACK_MONITOR_MAX_NAME_LEN]; + std::uint32_t freeBytes; + std::uint32_t sizeBytes; +}; + +//! A thread that just crossed back above its warn threshold this tick. +struct StackRecovery { + char name[STACK_MONITOR_MAX_NAME_LEN]; +}; + +//! Result of processing one tick of thread stack samples. Fixed-size; +//! intended to live as long-lived (member) storage, not on a small +//! thread's stack. +struct StackMonitorTickResult { + StackMonitorSummary summary; + StackWarning newWarnings[STACK_MONITOR_MAX_THREADS]; + std::uint32_t warningCount = 0; + StackRecovery newRecoveries[STACK_MONITOR_MAX_THREADS]; + std::uint32_t recoveryCount = 0; +}; + +//! Pure logic core for the StackMonitor component. +//! +//! Given a snapshot of per-thread stack usage taken once per tick, computes a +//! summary of the thread under the most stack pressure and warn/clear +//! decisions per thread with hysteresis. Host-compilable: no Zephyr or +//! F Prime autocode dependencies, matching the pattern used by +//! DetumbleManager::BDot. All state is fixed-size at construction; the +//! class performs zero heap allocation. +class StackMonitorCore { + public: + //! \param warnThresholdPercent warn when a thread's free stack drops + //! below this percent of its own size. + explicit StackMonitorCore(std::uint32_t warnThresholdPercent); + + //! Process one tick's worth of thread stack samples, writing into the + //! caller-provided result (cleared first). No allocation. + void tick(const ThreadStackSampleSet& sampleSet, StackMonitorTickResult& result); + + private: + //! Latched warn state, keyed by fixed-size thread name. An entry is + //! occupied while its thread is below threshold and hasn't yet + //! recovered; it is freed on recovery. Kept across ticks so a thread + //! that stays below threshold isn't re-warned every tick, and a thread + //! that disappears from the sample set simply keeps its entry (no + //! assert, no crash, no re-warn if it reappears still below). + //! Linear scan over <=MAX_THREADS entries at 1 Hz. + struct WarnEntry { + char name[STACK_MONITOR_MAX_NAME_LEN]; + bool used; + }; + + //! Find the warn-table index for a name, or -1 if absent. + std::int32_t findWarned(const char* name) const; + + //! Mark a name warned (no-op if the table is unexpectedly full; the + //! warning event itself is still emitted by tick()). + void setWarned(const char* name); + + //! Clear a name's warned entry, if present. + void clearWarned(const char* name); + + std::uint32_t m_warnThresholdPercent; + WarnEntry m_warned[STACK_MONITOR_MAX_THREADS]; +}; + +} // namespace Components diff --git a/PROVESFlightControllerReference/Components/StackMonitor/docs/sdd.md b/PROVESFlightControllerReference/Components/StackMonitor/docs/sdd.md new file mode 100644 index 000000000..c90383e21 --- /dev/null +++ b/PROVESFlightControllerReference/Components/StackMonitor/docs/sdd.md @@ -0,0 +1,44 @@ +# Components::StackMonitor + +Program-wide stack-usage watchdog. Once per tick, walks every live Zephyr +thread (`k_thread_foreach`) and reports per-thread stack high-water telemetry, +warning when any thread's free stack drops below a configurable percent of its +own size and clearing on recovery. + +Part of the S-Band reintegration plan, PR 1 / Slice 1.1 (decision D6): a +prerequisite for safely re-enabling the S-Band radio thread with a larger, +dedicated stack, by making per-thread stack pressure observable in YAMCS. + +The stack-usage decision logic lives in `StackMonitorCore`, a pure C++ class +with no Zephyr or F Prime dependencies, so it is host-testable in isolation +(see `test/unit-tests/test_StackMonitor_Core.cpp`). `StackMonitor.cpp` is a +thin adapter: it walks Zephyr threads, builds a sample list, and feeds it to +the core each tick. + +All storage is fixed-capacity POD (32 threads x 32-char names) kept as +component member state: the `k_thread_foreach` callback runs under the kernel +thread-monitor spinlock with IRQs locked, so it performs only a bounds check, +a bounded string copy, and integer stores. Zero heap allocation anywhere in +the monitoring path after boot (per-tick heap churn is the defect class that +got the S-Band radio removed; see issue #122). If a tick sees more live +threads than capacity, the extras are dropped and the `SampleOverflow` +telemetry channel reports it (no silent truncation). + +## Telemetry +| Name | Description | +|---|---| +| MinFreeBytes | Free bytes remaining on the thread under the most stack pressure this tick | +| WorstThread | Name of the thread under the most stack pressure this tick | +| ThreadsBelowThreshold | Count of threads currently below the warn threshold | +| SampleOverflow | True when a tick saw more live threads than the monitor's fixed capacity | + +## Events +| Name | Description | +|---|---| +| StackLow | A thread's free stack dropped below its warn threshold | +| StackRecovered | A previously-low thread recovered above its warn threshold | + +## Change Log +| Date | Description | +|---|---| +|---| Initial Draft | diff --git a/PROVESFlightControllerReference/ReferenceDeployment/Top/ReferenceDeploymentPackets.fppi b/PROVESFlightControllerReference/ReferenceDeployment/Top/ReferenceDeploymentPackets.fppi index f107909f9..eb1de9aa3 100644 --- a/PROVESFlightControllerReference/ReferenceDeployment/Top/ReferenceDeploymentPackets.fppi +++ b/PROVESFlightControllerReference/ReferenceDeployment/Top/ReferenceDeploymentPackets.fppi @@ -130,6 +130,10 @@ telemetry packets ReferenceDeploymentPackets { ReferenceDeployment.modeManager.CurrentMode ReferenceDeployment.modeManager.SafeModeEntryCount ReferenceDeployment.modeManager.CurrentSafeModeReason + ReferenceDeployment.stackMonitor.MinFreeBytes + ReferenceDeployment.stackMonitor.WorstThread + ReferenceDeployment.stackMonitor.ThreadsBelowThreshold + ReferenceDeployment.stackMonitor.SampleOverflow } packet HealthWarnings id 3 group 5 { diff --git a/PROVESFlightControllerReference/ReferenceDeployment/Top/instances.fpp b/PROVESFlightControllerReference/ReferenceDeployment/Top/instances.fpp index c1a541ebc..7fbfddd8c 100644 --- a/PROVESFlightControllerReference/ReferenceDeployment/Top/instances.fpp +++ b/PROVESFlightControllerReference/ReferenceDeployment/Top/instances.fpp @@ -242,4 +242,6 @@ module ReferenceDeployment { instance picoTempManager: Drv.PicoTempManager base id 0x10079000 + instance stackMonitor: Components.StackMonitor base id 0x1007A000 + } diff --git a/PROVESFlightControllerReference/ReferenceDeployment/Top/topology.fpp b/PROVESFlightControllerReference/ReferenceDeployment/Top/topology.fpp index d7ab78d9d..0e475adb6 100644 --- a/PROVESFlightControllerReference/ReferenceDeployment/Top/topology.fpp +++ b/PROVESFlightControllerReference/ReferenceDeployment/Top/topology.fpp @@ -117,6 +117,7 @@ module ReferenceDeployment { instance dropDetector instance picoTempManager + instance stackMonitor # ---------------------------------------------------------------------- # Pattern graph specifiers @@ -284,6 +285,7 @@ module ReferenceDeployment { rateGroup1Hz.RateGroupMemberOut[17] -> adcs.run rateGroup1Hz.RateGroupMemberOut[18] -> thermalManager.run rateGroup1Hz.RateGroupMemberOut[19] -> ComCcsdsLora.authenticationRouter.run + rateGroup1Hz.RateGroupMemberOut[12] -> stackMonitor.run } diff --git a/PROVESFlightControllerReference/test/unit-tests/CMakeLists.txt b/PROVESFlightControllerReference/test/unit-tests/CMakeLists.txt index 0a0475502..4302a6045 100644 --- a/PROVESFlightControllerReference/test/unit-tests/CMakeLists.txt +++ b/PROVESFlightControllerReference/test/unit-tests/CMakeLists.txt @@ -39,6 +39,14 @@ target_include_directories(detumble_manager_bdot PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/../../.. ) +# StackMonitor Core +add_library(stack_monitor_core STATIC + ${CMAKE_CURRENT_SOURCE_DIR}/../../../PROVESFlightControllerReference/Components/StackMonitor/StackMonitorCore.cpp +) +target_include_directories(stack_monitor_core PUBLIC + ${CMAKE_CURRENT_SOURCE_DIR}/../../.. +) + # --- Auto-discover and build tests --- file(GLOB TEST_SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/test_*.cpp") @@ -53,6 +61,7 @@ foreach(test_src ${TEST_SOURCES}) detumble_manager_strategy_selector detumble_manager_bdot rtc_manager_rtc_helper + stack_monitor_core ) add_test(NAME ${test_name} COMMAND ${test_name}) diff --git a/PROVESFlightControllerReference/test/unit-tests/test_StackMonitor_Core.cpp b/PROVESFlightControllerReference/test/unit-tests/test_StackMonitor_Core.cpp new file mode 100644 index 000000000..d32bd6fed --- /dev/null +++ b/PROVESFlightControllerReference/test/unit-tests/test_StackMonitor_Core.cpp @@ -0,0 +1,173 @@ +#include + +#include + +#include "PROVESFlightControllerReference/Components/StackMonitor/StackMonitorCore.hpp" + +using Components::STACK_MONITOR_MAX_THREADS; +using Components::StackMonitorCore; +using Components::StackMonitorTickResult; +using Components::ThreadStackSampleSet; + +// Warn when a thread's free stack drops below 20% of its own size. +const std::uint32_t WARN_THRESHOLD_PERCENT = 20; + +TEST(StackMonitorCoreTest, SummarizesWorstThreadFromSampleSet) { + StackMonitorCore core(WARN_THRESHOLD_PERCENT); + + ThreadStackSampleSet samples; + samples.clear(); + samples.add("idle", 1024, 900); // 12% used, healthy + samples.add("sband", 4096, 1024); // 75% used, worst of the three + samples.add("watchdog", 2048, 1800); // ~12% used, healthy + + StackMonitorTickResult result; + core.tick(samples, result); + + EXPECT_STREQ(result.summary.worstThreadName, "sband"); + EXPECT_EQ(result.summary.worstThreadFreeBytes, 1024u); + EXPECT_EQ(result.summary.worstThreadUsedPercent, 75u); + EXPECT_EQ(result.summary.threadsBelowThreshold, 0u); +} + +TEST(StackMonitorCoreTest, WarnsWhenThreadCrossesBelowThreshold) { + StackMonitorCore core(WARN_THRESHOLD_PERCENT); + + // sband: 4096 total, 700 free => ~17% free, below the 20% threshold. + ThreadStackSampleSet samples; + samples.clear(); + samples.add("sband", 4096, 700); + + StackMonitorTickResult result; + core.tick(samples, result); + + EXPECT_EQ(result.summary.threadsBelowThreshold, 1u); + ASSERT_EQ(result.warningCount, 1u); + EXPECT_STREQ(result.newWarnings[0].name, "sband"); + EXPECT_EQ(result.newWarnings[0].freeBytes, 700u); + EXPECT_EQ(result.newWarnings[0].sizeBytes, 4096u); + EXPECT_EQ(result.recoveryCount, 0u); +} + +TEST(StackMonitorCoreTest, DoesNotReWarnWhileStillBelowThreshold) { + StackMonitorCore core(WARN_THRESHOLD_PERCENT); + + ThreadStackSampleSet samples; + samples.clear(); + samples.add("sband", 4096, 700); // below threshold + + StackMonitorTickResult result; + core.tick(samples, result); + ASSERT_EQ(result.warningCount, 1u); + + // Still below threshold on the next tick: no new warning (hysteresis). + core.tick(samples, result); + EXPECT_EQ(result.warningCount, 0u); + EXPECT_EQ(result.summary.threadsBelowThreshold, 1u); +} + +TEST(StackMonitorCoreTest, ClearsWhenThreadRecoversAboveThreshold) { + StackMonitorCore core(WARN_THRESHOLD_PERCENT); + + ThreadStackSampleSet low; + low.clear(); + low.add("sband", 4096, 700); // ~17% free, below threshold + + StackMonitorTickResult result; + core.tick(low, result); + ASSERT_EQ(result.warningCount, 1u); + + ThreadStackSampleSet recovered; + recovered.clear(); + recovered.add("sband", 4096, 2048); // 50% free, back above threshold + + core.tick(recovered, result); + + EXPECT_EQ(result.warningCount, 0u); + ASSERT_EQ(result.recoveryCount, 1u); + EXPECT_STREQ(result.newRecoveries[0].name, "sband"); + EXPECT_EQ(result.summary.threadsBelowThreshold, 0u); + + // Crossing below threshold again after recovery re-arms the warning. + core.tick(low, result); + ASSERT_EQ(result.warningCount, 1u); + EXPECT_STREQ(result.newWarnings[0].name, "sband"); +} + +TEST(StackMonitorCoreTest, EmptySampleSetProducesNoCrashAndNeutralSummary) { + StackMonitorCore core(WARN_THRESHOLD_PERCENT); + + ThreadStackSampleSet samples; + samples.clear(); + + StackMonitorTickResult result; + core.tick(samples, result); + + EXPECT_STREQ(result.summary.worstThreadName, ""); + EXPECT_EQ(result.summary.worstThreadFreeBytes, 0u); + EXPECT_EQ(result.summary.threadsBelowThreshold, 0u); + EXPECT_EQ(result.warningCount, 0u); + EXPECT_EQ(result.recoveryCount, 0u); +} + +TEST(StackMonitorCoreTest, ThreadDisappearingBetweenTicksDoesNotAssert) { + StackMonitorCore core(WARN_THRESHOLD_PERCENT); + + ThreadStackSampleSet withSband; + withSband.clear(); + withSband.add("sband", 4096, 700); // below threshold, warns + withSband.add("idle", 1024, 900); + + StackMonitorTickResult result; + core.tick(withSband, result); + ASSERT_EQ(result.warningCount, 1u); + + // sband thread has exited; only idle remains this tick. + ThreadStackSampleSet withoutSband; + withoutSband.clear(); + withoutSband.add("idle", 1024, 900); + + core.tick(withoutSband, result); + + EXPECT_EQ(result.warningCount, 0u); + EXPECT_EQ(result.recoveryCount, 0u); + EXPECT_STREQ(result.summary.worstThreadName, "idle"); + EXPECT_EQ(result.summary.threadsBelowThreshold, 0u); + + // sband reappears still below threshold: no re-warn (hysteresis survives the gap). + core.tick(withSband, result); + EXPECT_EQ(result.warningCount, 0u); + EXPECT_EQ(result.summary.threadsBelowThreshold, 1u); +} + +TEST(StackMonitorCoreTest, MoreThreadsThanCapacitySetsOverflowFlag) { + StackMonitorCore core(WARN_THRESHOLD_PERCENT); + + ThreadStackSampleSet samples; + samples.clear(); + + // Fill to capacity: every add succeeds, no overflow yet. + char name[16]; + for (std::uint32_t i = 0; i < STACK_MONITOR_MAX_THREADS; i++) { + (void)snprintf(name, sizeof(name), "thread%u", i); + EXPECT_TRUE(samples.add(name, 4096, 2048)); + } + EXPECT_EQ(samples.count, STACK_MONITOR_MAX_THREADS); + EXPECT_FALSE(samples.overflowed); + + // One past capacity: the sample is dropped and the flag is raised. + EXPECT_FALSE(samples.add("straw", 4096, 2048)); + EXPECT_EQ(samples.count, STACK_MONITOR_MAX_THREADS); + EXPECT_TRUE(samples.overflowed); + + // The overflow is surfaced in the tick summary (no silent truncation). + StackMonitorTickResult result; + core.tick(samples, result); + EXPECT_TRUE(result.summary.overflowed); + + // A subsequent in-capacity tick reports clean again. + samples.clear(); + samples.add("idle", 1024, 900); + core.tick(samples, result); + EXPECT_FALSE(result.summary.overflowed); +} diff --git a/S-BAND-REINTEGRATION-PLAN.md b/S-BAND-REINTEGRATION-PLAN.md new file mode 100644 index 000000000..3b4f10d69 --- /dev/null +++ b/S-BAND-REINTEGRATION-PLAN.md @@ -0,0 +1,200 @@ +# S-Band Reintegration TDD Plan + +**Goal:** Bring the S-Band (SX1280/RadioLib) radio back into `main`, fixing the defects +that got it removed in PR #256 ("Remove SBand to protect RAM"), with every fix pinned +by a test written first. + +**Related:** issue #122 (library can lock up / RAM concerns), issue #299 (stack +overflow analysis), PR #175 (original MVP), PR #256 (removal), PR #109 (radio fault +management / nRST reset), branch `s-band-speedup` (post-removal correctness fixes). + +> Naming note: #122 says "RadioHead" but the flight code uses **RadioLib** +> (`lib/RadioLib`, jgromes). Correct this when updating the issues. + +--- + +## Root causes (as-investigated, 2026-07-13) + +1. **Stack overflow on RX** (#299): `deferredRxHandler` runs on the SBand thread with a + 256-byte local array, then calls `dataOut_out` — a *synchronous* call chain through + `ComCcsdsSband` (frameAccumulator → deframer → router) — overflowing the 4096-byte + stack. +2. **Why "just raise the stack" failed:** `CONFIG_DYNAMIC_THREAD_STACK_SIZE=4096` is a + single global for the 25-slot Zephyr thread pool (`DYNAMIC_THREAD_ALLOC=n`). Raising + it costs ~100 KB across all threads on a 520 KB part → no boot. + **Never tried:** `CONFIG_DYNAMIC_THREAD_ALLOC=y` lets one oversized request fall back + to a single boot-time `k_malloc` (~8 KB total cost); all other threads keep pool stacks. +3. **In-flight heap churn** (#122 comment): RadioLib defaults to dynamic allocation; + `RADIOLIB_STATIC_ONLY` is not set. +4. **Unbounded waits** (#122): RadioLib internal BUSY/status polling can spin forever if + SPI/BUSY wedges (the BUSY line wasn't even wired until `s-band-speedup` 7c473f4). +5. **No tests:** the SBand UT block is commented out; the component has no seam + (`FprimeHal` → `Module` → `SX1280` held by value). + +## Decisions (grilled 2026-07-12/13) + +| # | Decision | +|---|----------| +| D1 | Resurrect the existing RadioLib driver in place (no Zephyr-native rewrite, no com-chain async rework). Cherry-pick only *correctness* commits from `s-band-speedup`. | +| D2 | SBand thread gets a dedicated 8 KB stack via `CONFIG_DYNAMIC_THREAD_ALLOC=y` fallback; boot-failure is loud (assert/FATAL); stack usage is *measured*, not assumed. | +| D3 | Failure semantics: bounded call → N consecutive errors/timeouts → auto nRST reset (reuse #109 path) → M failed resets → **FAULTED, latched until ground command**. Invariant: S-Band failure degrades to "no S-Band", never to backpressure, hang, or spacecraft reset. UHF unaffected. | +| D4 | Test seam: new `SBandRadioIf` abstract interface over the 13 SX1280 calls the component uses; production impl wraps SX1280 and owns the bounded-timeout logic; F´ UTs drive the component against a scripted fake. | +| D5 | Three staged PRs (infra → component+UTs → topology re-enable). PR 3 gated on ≥24 h dual-radio HWIL soak + end-to-end functional pass. | +| D6 | Program-wide `StackMonitor` component (all threads, 1 Hz, `k_thread_foreach_unlocked` + `k_thread_stack_space_get`, needs `CONFIG_THREAD_MONITOR=y` and `CONFIG_INIT_STACKS=y`). | +| D7 | Out of scope: throughput tuning (passthrough chunk/cooldown), async com-chain rework (documented fallback only), static stack-analysis tooling, stale `s-band-*` branch cleanup, fprime-zephyr changes (already merged via #326). | + +**Fallback trigger:** if soak shows SBand stack high-water > 70 % of 8 KB, revisit +option C from #299 (async hop in the com chain) before flying. + +--- + +## PR 1 — Infrastructure (SBand stays commented out; zero flight-behavior change) + +### Slice 1.1 — StackMonitor component (tracer bullet) + +New passive component `Components/StackMonitor`. No provider seam: the Zephyr +adapter (`StackMonitor.cpp`) calls `k_thread_foreach_unlocked` + +`k_thread_stack_space_get` directly and stays a thin, heap-free translation +into fixed-capacity samples; all decision logic (summary, warning/recovery +transitions, overflow) lives in the pure, host-testable `StackMonitorCore`. +One RED→GREEN cycle per behavior in `StackMonitorCore`, in order: + +1. On each `tick`, computes the worst thread's free bytes (`MinFreeBytes`/ + `WorstThread`), the count of threads below the warning threshold, and + whether the fixed-capacity sample buffer overflowed — this is what + `StackMonitor` publishes as telemetry. +2. Emits a WARNING_HI EVR when any thread's free stack drops below a threshold + (percentage of its own size), throttled; clears on recovery. +3. Handles thread count changing between ticks without asserting. + +The Zephyr adapter itself (the `k_thread_foreach_unlocked` callback, real +telemetry/EVR wiring) has no host build target and is validated via +integration/HWIL in PR 3, not component-level UTs. + +### Slice 1.2 — Loud boot failure on task-start error + +Verify what `ActiveComponentBase`/`Os::Task` does today when +`k_thread_stack_alloc` returns null (`ERROR_RESOURCES`). If it can silently limp, +add an assert/FATAL. Behavior to pin (host UT where the Os layer permits, else +HWIL check in PR 3): a deployment whose thread can't get its stack never runs +half-alive. + +### Slice 1.3 — Kconfig flips + +`CONFIG_DYNAMIC_THREAD_ALLOC=y`, `CONFIG_THREAD_MONITOR=y`, `CONFIG_INIT_STACKS=y` +(required for `k_thread_stack_space_get`). No new tests (config only); gate = +full existing suite + CI build for v5c/v5d/v5e + a short UHF bench sanity to +prove no regression while SBand is still off. + +**PR 1 exit:** main builds and soaks exactly as before, now with worst-thread +stack telemetry (`MinFreeBytes`/`WorstThread`/`ThreadsBelowThreshold`/ +`SampleOverflow`) visible in YAMCS's Health packet. + +--- + +## PR 2 — Component rework + UTs (built, tested, still not in the topology) + +Un-comment `add_fprime_subdirectory(.../SBand/)` and the UT registration; the +library and its tests build without any topology change. + +### Slice 2.1 — Seam + first light (tracer bullet) + +RED: component UT constructs SBand with a fake `SBandRadioIf`; asserts that startup +configures the radio and arms receive. GREEN: introduce `SBandRadioIf`, refactor +SBand to hold a reference (production default = RadioLib-backed impl), pass. +This cycle proves the whole UT harness path works. + +### Slice 2.2 — RX happy path + +Fake presents a received packet → component emits `dataOut` with the exact bytes, +re-arms receive, updates RSSI/SNR telemetry. (Implementation note, not test +subject: read directly into the allocated `Fw::Buffer` — the 256-byte stack array +goes away here.) + +### Slice 2.3 — RX allocation failure + +Buffer manager returns invalid buffer → WARNING EVR, receive re-armed, no leak, +no crash. + +### Slice 2.4 — TX path + transmit gate + +`dataIn` while ENABLED → radio transmit called with the frame, buffer returned, +comStatus emitted. While DISABLED → no radio call, buffer still returned, +comStatus still emitted (the com queue must never starve). + +### Slice 2.5 — Errors are bounded and counted + +Fake returns an error / simulated timeout → call completes promptly, EVR emitted, +consecutive-error counter advances; one success resets the counter. + +### Slice 2.6 — Auto-reset escalation + +N consecutive failures → exactly one nRST reset request via the #109 path, EVR, +counters observable in telemetry. + +### Slice 2.7 — FAULTED latch + +M resets without recovery → FAULTED: telemetry flag set; no further radio-interface +calls; `dataIn` buffers returned immediately; `run` ticks are no-ops. Latched — +further errors can't re-trigger resets. + +### Slice 2.8 — Ground recovery + +`RESET_RADIO` (existing command) while FAULTED → full re-init through the fake, +FAULTED cleared on success, re-latched if re-init fails. + +### Slice 2.9 — Cherry-pick `s-band-speedup` correctness fixes + +BUSY-GPIO wiring into RadioLib (7c473f6/363101e), `dataOut_out` after all SPI ops +(8aeee2f), atomic inter-thread flag (9b240d0), enableRx/enableTx return types +(17df3ec). Where a fix has observable behavior (ordering, flag races), pin it with +a UT first; pure wiring is covered by PR 3 bench work. + +### Slice 2.10 — `RADIOLIB_STATIC_ONLY=1` + +Set in SBand's CMake (where RadioLib is added). Gate = build + full UT suite; +grep the map file to confirm no RadioLib heap symbols remain on hot paths. + +**PR 2 exit:** SBand component fully unit-tested against the D3 invariant; zero +topology/flight change. + +--- + +## PR 3 — Topology re-enable (soak-gated) + +### Slice 3.1 — Wire it back + +Un-comment instances, `ComCcsdsSband` subtopology, connections, rate-group slots. +New `SBAND_STACK_SIZE = 8 * 1024` for the sband instance only (comment explaining +the pool-fallback mechanism). CI builds for v5c/v5d/v5e; existing integration + +day-in-the-life suites green. + +### Slice 3.2 — HWIL functional pass (bench, v5e) + +- Downlink: TM frames received over the S-Band link end-to-end. +- Uplink: command through `ComCcsdsSband.authenticationRouter` → `cmdDisp` executes. +- HWIL console/debug view (not telemetry — StackMonitor only reports the + worst thread) confirms the sband thread is running with an 8 KB stack. + +### Slice 3.3 — HWIL fault injection (the #122 repro, now with an expected answer) + +Physically disconnect SPI/BUSY mid-operation → expect: bounded EVRs → auto-reset +attempts → FAULTED. **No watchdog reset, no queue overflow, UHF link still up.** +Then `RESET_RADIO` with hardware restored → link recovers. + +### Slice 3.4 — Soak gate (merge gate for PR 3) + +≥24 h continuous, both radios active with periodic S-Band TX/RX traffic: +- zero watchdog/unexpected resets; +- sband stack high-water < 70 % of 8 KB, observed via HWIL console/debug (not + StackMonitor telemetry, which only reports the worst thread) — else trigger + the option-C fallback review; +- StackMonitor's worst-thread watermark doesn't regress vs. the PR 1 baseline, + cross-checked against the HWIL per-thread console view for the sband thread + specifically; +- heap watermark flat after init (STATIC_ONLY doing its job); +- RX/TX counters advance the whole window. + +**Post-merge:** update #122 and #299 with results (and the RadioLib naming +correction); file the follow-up issue for throughput tuning (deferred +`s-band-speedup` perf commits + passthrough work). diff --git a/prj.conf b/prj.conf index a375729a0..bcce13855 100644 --- a/prj.conf +++ b/prj.conf @@ -43,7 +43,12 @@ CONFIG_I2C_TCA954X_CHANNEL_INIT_PRIO=71 CONFIG_DYNAMIC_THREAD=y CONFIG_KERNEL_MEM_POOL=y -CONFIG_DYNAMIC_THREAD_ALLOC=n +# S-Band reintegration plan (PR 1, Slice 1.3): the SBand thread needs a dedicated +# 8 KB stack (see PR 3), which is larger than the shared 4096-byte pool slot size. +# CONFIG_DYNAMIC_THREAD_ALLOC=y lets a single oversized k_thread_create() request +# fall back to a boot-time k_malloc() instead of failing; CONFIG_DYNAMIC_THREAD_PREFER_POOL=y +# is unchanged, so every other thread (<=4096 bytes) still gets a pool stack as before. +CONFIG_DYNAMIC_THREAD_ALLOC=y CONFIG_DYNAMIC_THREAD_PREFER_POOL=y CONFIG_DYNAMIC_THREAD_POOL_SIZE=25 # Num threads in the thread pool @@ -53,6 +58,17 @@ CONFIG_DYNAMIC_THREAD_STACK_SIZE=4096 CONFIG_THREAD_STACK_INFO=y CONFIG_RING_BUFFER=y +# S-Band reintegration plan (PR 1, Slice 1.1 / D6): needed by the StackMonitor +# component to enumerate live threads (k_thread_foreach) and read their names +# (k_thread_name_get) for per-thread stack telemetry. +CONFIG_THREAD_MONITOR=y +CONFIG_THREAD_NAME=y +# k_thread_stack_space_get() (used to read each thread's free-stack high-water +# mark) is only compiled in when CONFIG_INIT_STACKS is also set: it relies on +# stacks being pre-filled with a known 0xaa pattern at creation time so the +# unused portion can be measured later. +CONFIG_INIT_STACKS=y + CONFIG_COMMON_LIBC_MALLOC=y CONFIG_SENSOR=y