diff --git a/BirdsEye/display_pages.ino b/BirdsEye/display_pages.ino index 79e6e19..c9e5c73 100644 --- a/BirdsEye/display_pages.ino +++ b/BirdsEye/display_pages.ino @@ -784,11 +784,18 @@ void displayPage_egg_test() { } display.println(); - // Row 4: live flags from the latest frame. + // Row 4: live flags from the latest frame, plus the GATT notify + // frames the logger has had to drop since boot (D, only once + // nonzero; clamped so the worst case "FLG PAIR FAULT D9999" is 20). display.print(F("FLG")); if (sensoreggPairingFlag()) display.print(F(" PAIR")); if (sensoreggTcFault()) display.print(F(" FAULT")); if (!sensoreggPairingFlag() && !sensoreggTcFault()) display.print(F(" -")); + const uint32_t eggDrops = sensoreggFrameDrops(); + if (eggDrops != 0) { + display.print(F(" D")); + display.print(eggDrops > 9999UL ? 9999UL : eggDrops); + } display.println(); // Row 5: which egg the filter accepts. diff --git a/BirdsEye/sensoregg.h b/BirdsEye/sensoregg.h index b396a42..d02a451 100644 --- a/BirdsEye/sensoregg.h +++ b/BirdsEye/sensoregg.h @@ -21,7 +21,10 @@ // SoftDevice. A PAIRED egg in range, while the egg radio is wanted, // gets a GATT connection and streams the PerchWerks Sensor Service // (self-describing channel table, per-channel batch frames stamped at -// acquisition, a Clock/boot_id epoch — sensoregg_gatt.{h,cpp}). +// acquisition, a Clock/boot_id epoch — sensoregg_gatt.{h,cpp}). The +// clock fit is anchored on every bring-up but only its epoch (boot_id) +// check is consumed; mapping pod time onto logger time is reserved for +// a future per-sample resampling plan — values here are latest-sample. // Everything else — unpaired pods, the pre-connect window, backoff, // and the pairing capture itself — rides the passive PW-ADV observer // exactly as before (no SCAN_REQ; sensoregg_protocol.{h,cpp}). The two @@ -33,11 +36,18 @@ // the link for transfers and shutdown. While streaming, the scanner's // 44% duty is not paid at all. // -// GATT DEGRADATIONS (documented, deliberate): Sample frames carry no -// MCP STATUS, so sensoreggTcFault() reads false while streaming (an -// open probe still shows as sentinel -> NaN -> '---', the same visible -// outcome); sensoreggProtoVersion()/sensoreggPairingFlag() hold their -// last beacon values. +// WHAT THE STREAM REPORTS (plan 0018 review fixes): committing the +// stream CLEARS every beacon-only field rather than holding it — +// temperatures NaN and battery 0xFF until their own channel's first +// frame, tcFault and the pairing flag false (Sample frames carry no MCP +// STATUS and no pairing bit; an open probe still shows as sentinel -> +// NaN -> '---', the same visible outcome). Each value is then live only +// while ITS OWN channel keeps arriving (per-role stamps, staleness = +// the channel's frame cadence with one frame of slack, floored at the +// 1 s rule) — so an unmapped role (a pod with no IAT) stays NaN, and an +// EGT channel that stops while CJ continues goes NaN. Only +// sensoreggProtoVersion() keeps its last beacon value (it names the +// egg's beacon firmware). // // PAIRING (plan 0017): runtime MAC filter, persisted as the // "sensoregg_mac" setting ("AA:BB:CC:DD:EE:FF"; empty = unpaired = @@ -119,7 +129,8 @@ bool sensoreggLinkUp(); bool sensoreggAppHung(); // Latest EGT / cold junction in degC. NaN when the link is stale OR the -// egg reported the invalid sentinel (open probe, sensor fault). +// egg reported the invalid sentinel (open probe, sensor fault), or — +// on the GATT stream — when that value's own channel has gone stale. float sensoreggEgtC(); float sensoreggJunctionC(); @@ -133,7 +144,9 @@ float sensoreggAuxC(); uint8_t sensoreggBatteryPct(); // True while fresh AND the egg flags a thermocouple fault (open / -// out-of-range probe, MCP9600 STATUS input-range bit). +// out-of-range probe, MCP9600 STATUS input-range bit). Beacon-only: +// always false while the GATT stream is the source (frames carry no +// STATUS; the fault shows as a NaN EGT instead). bool sensoreggTcFault(); // Free-running egg sequence counter from the latest payload (debug). @@ -204,3 +217,8 @@ uint8_t sensoreggLinkMode(); // Live ATT MTU of the egg link (0 when not connected). 247 after a // successful exchange; frames are sized to it on the egg side. uint16_t sensoreggGattMtu(); + +// Cumulative notify frames the logger could not keep since boot (ring +// full, or oversize). Shown on EGG TEST as D; drops in 3 consecutive +// 1 s windows drop the link so the beacon path takes over. +uint32_t sensoreggFrameDrops(); diff --git a/BirdsEye/sensoregg.ino b/BirdsEye/sensoregg.ino index 1d0550f..20f808c 100644 --- a/BirdsEye/sensoregg.ino +++ b/BirdsEye/sensoregg.ino @@ -66,6 +66,10 @@ static uint8_t eggMacFilter[6] = SENSOREGG_MAC; static volatile bool eggPairingActive = false; // capture window open static volatile bool eggTestActive = false; // EGG TEST bench latch static uint32_t eggPairStartMs = 0; // main loop only +// Capture persist throttle (main loop only): a failing settings write +// is retried at ~1 Hz, not at the beacon rate (plan 0017 review). +static bool eggPairPersistTried = false; +static uint32_t eggPairPersistLastMs = 0; // Per-slot advertiser address (raw LSB-first), captured alongside the // payload so the main-loop drain can pair without the callback ever // learning any protocol. Plain RAM like eggBuf; the ready flags below @@ -132,17 +136,15 @@ static BLEClientCharacteristic eggChrDesc{BLEUuid(kPwDescUuid)}; static BLEClientCharacteristic eggChrSamp{BLEUuid(kPwSampUuid)}; static BLEClientCharacteristic eggChrClk{BLEUuid(kPwClkUuid)}; -// Ordered so "engaged" (a connection exists or is being made) is a -// single >= compare: BACKOFF sits between the idle states and -// CONNECTING on purpose. -enum EggLinkState : uint8_t { - EGG_LINK_IDLE = 0, // link not wanted (unpaired / gate closed) - EGG_LINK_WAIT_ADV, // wanted — the scan callback fires the connect - EGG_LINK_BACKOFF, // cooling off after a failure/disconnect - EGG_LINK_CONNECTING, // sd_ble_gap_connect in flight - EGG_LINK_BRINGUP, // discovery/reads running in the callback task - EGG_LINK_STREAMING, // notify subscription live, surface fed by GATT -}; +// The link states live in the host-tested sensoregg_gatt unit (with +// the transition rules that race the callback task); short local names +// keep the glue readable. Ordered so "engaged" is a single >= compare. +static constexpr uint8_t EGG_LINK_IDLE = sensoregg_gatt::LINK_IDLE; +static constexpr uint8_t EGG_LINK_WAIT_ADV = sensoregg_gatt::LINK_WAIT_ADV; +static constexpr uint8_t EGG_LINK_BACKOFF = sensoregg_gatt::LINK_BACKOFF; +static constexpr uint8_t EGG_LINK_CONNECTING = sensoregg_gatt::LINK_CONNECTING; +static constexpr uint8_t EGG_LINK_BRINGUP = sensoregg_gatt::LINK_BRINGUP; +static constexpr uint8_t EGG_LINK_STREAMING = sensoregg_gatt::LINK_STREAMING; static volatile uint8_t eggLinkState = EGG_LINK_IDLE; static volatile uint16_t eggConnHandle = BLE_CONN_HANDLE_INVALID; static volatile uint32_t eggLinkEventMs = 0; // stamp of the last transition @@ -163,22 +165,44 @@ static volatile bool eggBringupReady = false; static volatile bool eggBringupFailed = false; // Notify frame ring (the data plane keeps the copy-only discipline): -// the notify callback memcpys, SENSOREGG_LOOP drains. 32-byte slots -// cover the egg's <=26-byte frames; a fatter future pod's frames are -// counted as drops rather than truncated. -static uint8_t eggFrameBuf[8][32]; +// the notify callback memcpys, SENSOREGG_LOOP drains. Slots are sized +// to kMaxFrameLen (244 = ATT_MTU 247 - 3, a full 117-sample frame), the +// largest notify the link can legally carry — the old 32-byte slots +// silently dropped any batch over 11 samples while the link stayed up +// looking healthy. 8 x 244 = ~1.9 KB (was 256 B): the depth is kept at +// 8 so a main-loop stall (SD GC, 100 ms-2 s) at the stream's ~7 Hz +// frame rate still loses at most a second of superseded values, and +// ~1.7 KB extra is noise next to the 8 KB track-JSON buffers on the +// 256 KB part. Drops (ring full, or an impossible oversize frame) are +// counted, shown on EGG TEST, and a sustained run of them drops the +// link (sensoregg_gatt::DropMonitor) so the beacon path takes over. +static uint8_t eggFrameBuf[8][sensoregg_gatt::kMaxFrameLen]; static volatile uint8_t eggFrameLen[8]; static volatile uint32_t eggFrameAtMs[8]; static volatile bool eggFrameReady[8] = {false}; static volatile uint8_t eggFrameW = 0; // notify callback writes static uint8_t eggFrameR = 0; // main loop reads -static uint32_t eggFrameDrops = 0; +static volatile uint32_t eggFrameDrops = 0; // callback writes, loop reads +static sensoregg_gatt::DropMonitor eggDropMon; // main loop only +static_assert(sensoregg_gatt::kMaxFrameLen <= 255, + "eggFrameLen is a uint8_t"); // Committed pod identity (main loop only, valid while STREAMING). static sensoregg_gatt::PodDescriptor eggPod; static int8_t eggRoleIdx[sensoregg_gatt::ROLE_COUNT] = {-1, -1, -1, -1}; static int8_t eggFastestIdx = 0; // whose frame seq feeds the zombie monitor +// Anchored on every bring-up; only its epoch (boot_id) is consumed today +// — pod->logger time mapping is reserved for per-sample resampling +// (plan 0018 follow-ups). static sensoregg_gatt::ClockFit eggClockFit; +// Stream surface bookkeeping (plan 0018 review, E3). eggReadingFromGatt +// is true while the stream was the last writer of eggReading — then +// every value accessor is additionally gated on ITS OWN role's receive +// stamp (eggRoleFresh), because the shared eggRxMs is kept fresh by any +// channel's frames. A beacon parse clears it (the beacon rewrites the +// whole Reading at once, so the plain 1 s rule is exact there). +static bool eggReadingFromGatt = false; +static sensoregg_gatt::RoleFreshness eggRoleFresh; // RACE-GATED SCANNER (plan 0012). The scanner's 40-of-every-90 ms radio // claim is only paid while a race session is running — the one time the EGT @@ -252,9 +276,21 @@ static void sensoreggScanCallback(ble_gap_evt_adv_report_t* report) { sensoregg_protocol::matchesMagic(buf, len) && sensoreggMacAccepted(report->peer_addr.addr) && sensoreggIsPaired()) { + // CONNECTING is posted BEFORE the request: the connect callback + // only adopts a link it finds in CONNECTING. eggLinkState = EGG_LINK_CONNECTING; eggLinkEventMs = millis(); - Bluefruit.Central.connect(report); + const bool accepted = Bluefruit.Central.connect(report); + if (!accepted) { + // Refused outright — no connect callback will ever come. Back off + // now and un-pause the scanner (it is still parked on this + // report) so the beacon keeps feeding the surface meanwhile. + eggLinkState = sensoregg_gatt::linkAfterConnectRequest(accepted); + eggLinkEventMs = millis(); + if (eggScanWanted()) { + Bluefruit.Scanner.resume(); + } + } return; } @@ -323,6 +359,28 @@ static void sensoreggSampleNotifyCb(BLEClientCharacteristic* chr, // staging block's comment; plan 0018). Stages raw bytes; the main loop // parses and commits. No Serial here — the commit path narrates. static void sensoreggCentralConnectCb(uint16_t connHandle) { + // A connection we no longer want (SENSOREGG_SLEEP() ran while the + // SoftDevice was already establishing it, or the connect timeout gave + // up first) is dropped on arrival — otherwise it would come up in a + // transfer session / the charging park where nothing reconciles it. + // The handle is never adopted (the disconnect callback ignores a + // handle that is not eggConnHandle); a CONNECTING state whose gate + // dropped is retired to BACKOFF rather than left for the 10 s timeout. + if (!sensoregg_gatt::linkAcceptCentralConnect( + (sensoregg_gatt::LinkState)eggLinkState, eggSleeping, + eggLinkWanted())) { + Bluefruit.disconnect(connHandle); + if (eggLinkState == EGG_LINK_CONNECTING) { + eggLinkState = EGG_LINK_BACKOFF; + eggLinkEventMs = millis(); + } + return; + } + // A ready/failed flag still up from an EARLIER bring-up (consumed by + // nobody — e.g. staged just before a sleep) must never pair with this + // link's staging buffers. Clear both before touching the buffers. + eggBringupReady = false; + eggBringupFailed = false; eggConnHandle = connHandle; eggLinkState = EGG_LINK_BRINGUP; eggLinkEventMs = millis(); @@ -362,8 +420,11 @@ static void sensoreggCentralConnectCb(uint16_t connHandle) { static void sensoreggCentralDisconnectCb(uint16_t connHandle, uint8_t reason) { - (void)connHandle; (void)reason; + // Only OUR link's drop moves the state machine. A connection the + // connect callback refused (above) was never adopted; its disconnect + // must not knock a sleeping/idle machine into BACKOFF. + if (connHandle != eggConnHandle) return; eggConnHandle = BLE_CONN_HANDLE_INVALID; eggLinkState = EGG_LINK_BACKOFF; eggLinkEventMs = millis(); @@ -457,7 +518,10 @@ void SENSOREGG_SLEEP() { // live link — this covers BLE transfer mode (BLE_SETUP calls this // first) and shutdown (bleShutdownQuiesce()'s bounded settle runs // after us and gives the disconnect airtime; its own disconnect only - // handles the peripheral handle). + // handles the peripheral handle). A link the SoftDevice already + // established but whose connect callback has not run yet is invisible + // here (no handle, connect_cancel() a no-op) — the callback itself + // refuses it on arrival (linkAcceptCentralConnect sees eggSleeping). if (eggLinkState == EGG_LINK_CONNECTING) { sd_ble_gap_connect_cancel(); } @@ -465,6 +529,10 @@ void SENSOREGG_SLEEP() { Bluefruit.disconnect(eggConnHandle); } eggLinkState = EGG_LINK_IDLE; + // A bring-up staged before the sleep is moot — don't let it be + // consumed on the wake side. + eggBringupReady = false; + eggBringupFailed = false; } void SENSOREGG_WAKE() { @@ -490,6 +558,7 @@ void SENSOREGG_LOOP() { sensoregg_protocol::Reading r; if (sensoregg_protocol::parsePayload(local, localLen, r)) { eggReading = r; + eggReadingFromGatt = false; eggRxMs = atMs; eggHaveReading = true; sensoregg_protocol::seqMonitorFeed(eggSeqMon, r.sequence, atMs); @@ -498,11 +567,16 @@ void SENSOREGG_LOOP() { // Pairing capture (plan 0017): first frame whose egg advertises // its own pairing-window flag wins. PERSIST FIRST (camera // precedent, camera_ble.ino) — a failed SD write leaves the - // window open so the next frame retries. The RAM filter is + // window open and a later frame retries, throttled to ~1 Hz + // (pairPersistDue) so a failing card isn't hammered at 10 Hz. The RAM filter is // updated while eggPairingActive still bypasses it in the // callback, so a torn filter read can never reject the captured // egg; the flag clears last. - if (eggPairingActive && r.pairingActive) { + if (eggPairingActive && r.pairingActive && + sensoregg_protocol::pairPersistDue(eggPairPersistTried, + eggPairPersistLastMs, atMs)) { + eggPairPersistTried = true; + eggPairPersistLastMs = atMs; uint8_t human[6]; char macStr[sensoregg_protocol::kMacStrLen]; sensoregg_protocol::macReverse(localPeer, human); @@ -573,13 +647,37 @@ void SENSOREGG_LOOP() { eggFastestIdx = sensoregg_gatt::fastestChannel(eggPod); sensoregg_gatt::clockFitAnchor(eggClockFit, clkBoot, clkPod, eggClkReqMs, eggClkRspMs); - eggLinkState = EGG_LINK_STREAMING; - debug(F("SensorEgg: GATT link up — fw ")); - debug(eggPod.fwMajor); - debug(F(".")); - debug(eggPod.fwMinor); - debug(F(", channels ")); - debugln(eggPod.channelCount); + // Commit only onto the bring-up we staged: the disconnect callback + // (higher-priority task) may already have written BACKOFF and + // dropped the handle. Check + set inside one critical section so + // it cannot land between them (BASEPRI-masked — the SoftDevice's + // radio interrupts are unaffected; sd_functions.ino precedent). + bool committed; + taskENTER_CRITICAL(); + committed = sensoregg_gatt::linkMayCommitStreaming( + (sensoregg_gatt::LinkState)eggLinkState, + eggConnHandle != BLE_CONN_HANDLE_INVALID); + if (committed) eggLinkState = EGG_LINK_STREAMING; + taskEXIT_CRITICAL(); + if (committed) { + // Start the stream from a clean surface: nothing a beacon left + // behind (tcFault, pairing flag, an aux/battery value the pod + // may never stream) may be held under a fresh frame stamp. + sensoregg_gatt::readingResetForStream(eggReading); + sensoregg_gatt::roleFreshnessReset(eggRoleFresh); + eggSeqMon = sensoregg_protocol::SeqMonitor(); + eggReadingFromGatt = true; + sensoregg_gatt::dropMonitorReset(eggDropMon, eggFrameDrops, + millis()); + debug(F("SensorEgg: GATT link up — fw ")); + debug(eggPod.fwMajor); + debug(F(".")); + debug(eggPod.fwMinor); + debug(F(", channels ")); + debugln(eggPod.channelCount); + } else { + debugln(F("SensorEgg: link gone before commit — not streaming")); + } } else { debugln(F("SensorEgg: descriptor/clock parse failed — dropping link")); if (eggConnHandle != BLE_CONN_HANDLE_INVALID) { @@ -626,18 +724,27 @@ void SENSOREGG_LOOP() { const int16_t rawLast = f.raw[f.n - 1]; const float real = sensoregg_gatt::sampleToReal(rawLast, eggPod.ch[chIdx]); + int8_t role = -1; if (chIdx == eggRoleIdx[sensoregg_gatt::ROLE_EGT]) { eggReading.egtC = real; + role = sensoregg_gatt::ROLE_EGT; } else if (chIdx == eggRoleIdx[sensoregg_gatt::ROLE_CJ]) { eggReading.junctionC = real; + role = sensoregg_gatt::ROLE_CJ; } else if (chIdx == eggRoleIdx[sensoregg_gatt::ROLE_AUX]) { eggReading.auxC = real; + role = sensoregg_gatt::ROLE_AUX; } else if (chIdx == eggRoleIdx[sensoregg_gatt::ROLE_BATT]) { - // Ratio channel is whole percent (scale 1.0) — route the raw, - // sentinel/out-of-range -> the surface's 0xFF "unknown". - eggReading.battery = (rawLast < 0 || rawLast > 100) - ? (uint8_t)0xFF - : (uint8_t)rawLast; + // Through the descriptor's scale/offset like every other channel + // (the EGT pod happens to declare 1.0/0, a future pod need not); + // sentinel -> 0xFF "unknown", otherwise clamped 0-100. + eggReading.battery = sensoregg_gatt::batteryPercent(real); + role = sensoregg_gatt::ROLE_BATT; + } + if (role >= 0) { + sensoregg_gatt::roleFreshnessStamp(eggRoleFresh, + (sensoregg_gatt::Role)role, atMs, + f.n, f.intervalMs); } eggReading.sequence = f.seq; // display: the stream's own counter // Zombie detection feeds on the FASTEST channel's frame seq only — @@ -652,11 +759,36 @@ void SENSOREGG_LOOP() { eggRateCount++; // the test page's Hz meter counts GATT frames too } + // Sustained frame drops (E5): the stream isn't being consumed — drop + // the link so the beacon path takes over; the reconnect after the + // backoff starts a fresh evaluation. + if (eggLinkState == EGG_LINK_STREAMING && + eggConnHandle != BLE_CONN_HANDLE_INVALID && + sensoregg_gatt::dropMonitorUpdate(eggDropMon, eggFrameDrops, + millis())) { + debugln(F("SensorEgg: sustained frame drops — dropping link")); + Bluefruit.disconnect(eggConnHandle); + } + // Link reconcile: post/withdraw the wanted state, time out a stuck // connect, expire the backoff. The scan callback does the actual // connecting (it holds the fresh adv report). { const uint32_t now = millis(); + // Orphan guard: BRINGUP/STREAMING with no handle held means a + // disconnect was lost in a race — fall back to BACKOFF so the + // scanner and the retry path take over instead of wedging. + taskENTER_CRITICAL(); + { + const uint8_t fixed = sensoregg_gatt::linkReconcileOrphan( + (sensoregg_gatt::LinkState)eggLinkState, + eggConnHandle != BLE_CONN_HANDLE_INVALID); + if (fixed != eggLinkState) { + eggLinkState = fixed; + eggLinkEventMs = now; + } + } + taskEXIT_CRITICAL(); switch (eggLinkState) { case EGG_LINK_IDLE: if (eggLinkWanted()) { @@ -746,6 +878,16 @@ void SENSOREGG_LOOP() { // DATA SURFACE /////////////////////////////////////////// +// Per-role gate on top of the link-level rule: while the stream was the +// last writer, a value is live only while its own channel is (E3). +// Takes a plain uint8_t: Arduino's auto-prototype generator places this +// signature above the includes, where sensoregg_gatt::Role is unknown. +static bool eggRoleLive(uint8_t role) { + return !eggReadingFromGatt || + sensoregg_gatt::roleFresh(eggRoleFresh, (sensoregg_gatt::Role)role, + millis()); +} + bool sensoreggLinkUp() { return eggHaveReading && sensoregg_protocol::isFresh(eggRxMs, millis()); } @@ -755,26 +897,38 @@ float sensoreggEgtC() { // link still yields NaN when the egg itself sent the invalid sentinel // OR when the egg's app has hung (radio beaconing a frozen payload — // a flat line must never masquerade as data). - if (!sensoreggLinkUp() || sensoreggAppHung()) return NAN; + if (!sensoreggLinkUp() || sensoreggAppHung() || + !eggRoleLive(sensoregg_gatt::ROLE_EGT)) { + return NAN; + } return eggReading.egtC; } float sensoreggJunctionC() { - if (!sensoreggLinkUp() || sensoreggAppHung()) return NAN; + if (!sensoreggLinkUp() || sensoreggAppHung() || + !eggRoleLive(sensoregg_gatt::ROLE_CJ)) { + return NAN; + } return eggReading.junctionC; } float sensoreggAuxC() { // Same gating as the EGT: stale link or hung app -> NaN. Also NaN when // the egg is v1 (no aux field) or its divider reported the sentinel. - if (!sensoreggLinkUp() || sensoreggAppHung()) return NAN; + if (!sensoreggLinkUp() || sensoreggAppHung() || + !eggRoleLive(sensoregg_gatt::ROLE_AUX)) { + return NAN; + } return eggReading.auxC; } uint8_t sensoreggBatteryPct() { // 0xFF = unknown: stale/hung link, v1 stub, or the egg's own // no-pack-fitted gate. Never report a stale percent as current. - if (!sensoreggLinkUp() || sensoreggAppHung()) return 0xFF; + if (!sensoreggLinkUp() || sensoreggAppHung() || + !eggRoleLive(sensoregg_gatt::ROLE_BATT)) { + return 0xFF; + } return eggReading.battery; } @@ -805,6 +959,7 @@ bool sensoreggIsPaired() { void sensoreggRequestPair() { // Idempotent; a re-request restarts the timeout clock. eggPairStartMs = millis(); + eggPairPersistTried = false; // a fresh window's first capture is immediate eggPairingActive = true; } @@ -868,6 +1023,10 @@ uint8_t sensoreggLinkMode() { return sensoreggLinkUp() ? 1 : 0; } +uint32_t sensoreggFrameDrops() { + return eggFrameDrops; +} + uint16_t sensoreggGattMtu() { if (eggConnHandle == BLE_CONN_HANDLE_INVALID) return 0; BLEConnection* conn = Bluefruit.Connection(eggConnHandle); @@ -922,5 +1081,6 @@ float sensoreggPacketHz() { return 0.0f; } void sensoreggGattClientInit() {} uint8_t sensoreggLinkMode() { return 0; } uint16_t sensoreggGattMtu() { return 0; } +uint32_t sensoreggFrameDrops() { return 0; } #endif // BIRDSEYE_ENABLE_SENSOREGG diff --git a/BirdsEye/sensoregg_gatt.cpp b/BirdsEye/sensoregg_gatt.cpp index bd80fb8..1af4ff0 100644 --- a/BirdsEye/sensoregg_gatt.cpp +++ b/BirdsEye/sensoregg_gatt.cpp @@ -3,6 +3,8 @@ #include #include +#include "nan_bits.h" + namespace sensoregg_gatt { namespace { @@ -87,6 +89,13 @@ float sampleToReal(int16_t raw, const ChannelInfo& c) { return (float)raw * c.scale + c.offset; } +uint8_t batteryPercent(float real) { + if (isNanF(real)) return 0xFF; + if (real <= 0.0f) return 0; + if (real >= 100.0f) return 100; + return (uint8_t)(real + 0.5f); +} + void mapChannels(const PodDescriptor& pd, int8_t outIdx[ROLE_COUNT]) { for (uint8_t r = 0; r < ROLE_COUNT; r++) outIdx[r] = -1; for (uint8_t i = 0; i < pd.channelCount; i++) { @@ -140,4 +149,85 @@ uint32_t clockFitPodToLogger(const ClockFit& f, uint32_t podMs) { return f.loggerMs0 + (uint32_t)delta; } +bool linkMayCommitStreaming(LinkState state, bool handleValid) { + return state == LINK_BRINGUP && handleValid; +} + +LinkState linkReconcileOrphan(LinkState state, bool handleValid) { + if (!handleValid && (state == LINK_BRINGUP || state == LINK_STREAMING)) { + return LINK_BACKOFF; + } + return state; +} + +bool linkAcceptCentralConnect(LinkState state, bool sleeping, bool wanted) { + return state == LINK_CONNECTING && !sleeping && wanted; +} + +LinkState linkAfterConnectRequest(bool accepted) { + return accepted ? LINK_CONNECTING : LINK_BACKOFF; +} + +void dropMonitorReset(DropMonitor& m, uint32_t totalDrops, uint32_t nowMs) { + m.started = true; + m.windowStartMs = nowMs; + m.dropsAtWindowStart = totalDrops; + m.badWindows = 0; +} + +bool dropMonitorUpdate(DropMonitor& m, uint32_t totalDrops, uint32_t nowMs) { + if (!m.started) { + dropMonitorReset(m, totalDrops, nowMs); + return false; + } + if ((uint32_t)(nowMs - m.windowStartMs) < kDropWindowMs) return false; + const bool bad = totalDrops != m.dropsAtWindowStart; + m.badWindows = bad ? (uint8_t)(m.badWindows + 1) : (uint8_t)0; + m.windowStartMs = nowMs; + m.dropsAtWindowStart = totalDrops; + if (m.badWindows >= kDropWindowsToFail) { + m.badWindows = 0; + return true; + } + return false; +} + +void readingResetForStream(sensoregg_protocol::Reading& r) { + r.egtC = NAN; + r.junctionC = NAN; + r.auxC = NAN; + r.flags = 0; + r.pairingActive = false; + r.tcFault = false; + r.status = 0; + r.battery = 0xFF; + r.sequence = 0; + // protoVersion deliberately kept (see header). +} + +uint32_t roleStaleAfterMs(uint8_t n, uint16_t intervalMs) { + const uint32_t span = (uint32_t)n * (uint32_t)intervalMs; // <= 117*65535 + uint32_t stale = 2u * span; // no overflow + if (stale < sensoregg_protocol::kStalenessMs) { + stale = sensoregg_protocol::kStalenessMs; + } + if (stale > kRoleStaleMaxMs) stale = kRoleStaleMaxMs; + return stale; +} + +void roleFreshnessReset(RoleFreshness& f) { f = RoleFreshness(); } + +void roleFreshnessStamp(RoleFreshness& f, Role role, uint32_t atMs, + uint8_t n, uint16_t intervalMs) { + if (role >= ROLE_COUNT) return; + f.have[role] = true; + f.atMs[role] = atMs; + f.staleAfterMs[role] = roleStaleAfterMs(n, intervalMs); +} + +bool roleFresh(const RoleFreshness& f, Role role, uint32_t nowMs) { + if (role >= ROLE_COUNT || !f.have[role]) return false; + return (uint32_t)(nowMs - f.atMs[role]) < f.staleAfterMs[role]; +} + } // namespace sensoregg_gatt diff --git a/BirdsEye/sensoregg_gatt.h b/BirdsEye/sensoregg_gatt.h index f6b13a1..31acb68 100644 --- a/BirdsEye/sensoregg_gatt.h +++ b/BirdsEye/sensoregg_gatt.h @@ -21,6 +21,8 @@ #include #include +#include "sensoregg_protocol.h" // Reading + kStalenessMs (the shared surface) + namespace sensoregg_gatt { constexpr uint8_t kMaxChannels = 21; // 8 + 21*24 = 512 = ATT max value @@ -30,6 +32,10 @@ constexpr uint8_t kMinRecordLen = 24; // schema v1; larger = newer schema, constexpr size_t kSampleHeaderLen = 10; constexpr size_t kClockLen = 6; constexpr uint8_t kMaxSamplesPerFrame = 117; // (247-3-10)/2, spec section 5 +// Largest Sample notify the link can carry: ATT_MTU 247 (the logger's +// configCentralConn cap) - 3 = 244 = a full 117-sample frame. Receive +// buffers sized to this take every legal frame at any negotiated MTU. +constexpr size_t kMaxFrameLen = 10 + 2 * (size_t)kMaxSamplesPerFrame; constexpr int16_t kInvalidSentinel = INT16_MIN; // 0x8000 = "no valid reading" // One parsed channel descriptor record (PW_CHANNEL_SCHEMA.md section 5). @@ -83,6 +89,14 @@ bool parseClock(const uint8_t* d, size_t len, uint8_t& bootId, // sentinel becomes NaN BEFORE the conversion (never scaled). float sampleToReal(int16_t raw, const ChannelInfo& c); +// The surface's battery percent from a battery-role channel's +// engineering value (i.e. AFTER sampleToReal — the descriptor's +// scale/offset apply to the ratio channel like any other): NaN (the +// sentinel) -> 0xFF "unknown"; otherwise clamped to 0-100 and rounded. +// NaN is detected by bit pattern (nan_bits.h) — the device builds with +// -Ofast, where isnan() folds to false and NaN comparisons are UB. +uint8_t batteryPercent(float real); + // Descriptor-driven role mapping — the consumer never hardcodes channel // ids. Temperatures route by the schema's normative names ("EGT", "CJ", // "IAT"); the battery routes by quantity 0x08 (ratio). outIdx[] holds @@ -100,6 +114,15 @@ int8_t fastestChannel(const PodDescriptor& pd); // anchors on the connect-time Clock read: logger time = the // request/response midpoint (tightest pair available), pod time = the // value read. boot_id inequality = the pod's millis restarted (epoch). +// +// FIRMWARE CONSUMPTION TODAY: the sketch anchors a fit on every +// bring-up and uses ONLY clockFitSameEpoch() (the pod-reboot check). +// clockFitPodToLogger() and halfRttMs have no firmware consumer yet — +// the surface is latest-value-only and DOVEX rows are stamped by the +// logger's GPS clock. They are kept, tested, deliberately: reserved for +// the follow-up plan that timestamps per-sample rows from the fit (plan +// 0018 "Status / follow-ups"). Do not read an unused-API warning as +// dead code to delete. struct ClockFit { bool valid = false; uint8_t bootId = 0; @@ -117,4 +140,128 @@ bool clockFitSameEpoch(const ClockFit& f, uint8_t frameBootId); // within +/- ~24.8 days of the anchor, i.e. always in practice. uint32_t clockFitPodToLogger(const ClockFit& f, uint32_t podMs); +// ---- Link state machine (review fixes 2026-09) --------------------------- +// The sketch's central-link states, owned here so the transition rules +// that race the Bluefruit callback task are host-tested. Ordered so +// "engaged" (a connection exists or is being made) is a single >= +// compare: BACKOFF sits between the idle states and CONNECTING on +// purpose. +enum LinkState : uint8_t { + LINK_IDLE = 0, // link not wanted (unpaired / gate closed) + LINK_WAIT_ADV, // wanted — the scan callback fires the connect + LINK_BACKOFF, // cooling off after a failure/disconnect + LINK_CONNECTING, // sd_ble_gap_connect in flight + LINK_BRINGUP, // discovery/reads running in the callback task + LINK_STREAMING, // notify subscription live, surface fed by GATT +}; + +// May the main loop promote a staged bring-up to STREAMING? Only while +// the bring-up it staged is still the live one: state still BRINGUP and +// a connection handle still held. The disconnect callback runs in a +// higher-priority task and can land between the bring-up's ready flag +// and the loop's commit — it has already written BACKOFF and dropped +// the handle, and an unconditional commit would overwrite that with +// STREAMING on a link that no longer exists (scanner held off, EGT NaN +// until shutdown). The caller evaluates this and writes the new state +// inside one critical section. +bool linkMayCommitStreaming(LinkState state, bool handleValid); + +// Reconcile guard: a state that claims a connection (BRINGUP or +// STREAMING) while no handle is held is an orphan left by a lost race — +// send it to BACKOFF so the normal retry path (and the scanner) takes +// over. Every other state is returned unchanged. +LinkState linkReconcileOrphan(LinkState state, bool handleValid); + +// Should the central connect callback keep the connection it was just +// handed? Only when it is the connect we asked for (state CONNECTING) +// and the link is still wanted and the radio is not asleep. The +// SoftDevice can raise CONNECTED after SENSOREGG_SLEEP() already ran +// (its connect_cancel() is a no-op once the link exists, and the +// handle was not yet known to disconnect) or after the connect timeout +// gave up; accepting then would bring the link up during a transfer +// session / the charging park, where nothing reconciles it. A false +// answer means: disconnect that handle without adopting it (a +// CONNECTING state is retired to BACKOFF; IDLE/BACKOFF stay put). +bool linkAcceptCentralConnect(LinkState state, bool sleeping, bool wanted); + +// State after the scan callback asked the SoftDevice to connect. A +// refused request (Central.connect() == false: radio busy, invalid +// params) never produces a connect callback, so staying CONNECTING +// would sit out the full 10 s connect timeout — plus the 5 s backoff — +// with the scanner paused on the report that triggered it. Refused -> +// BACKOFF immediately (and the caller resumes the scanner). +LinkState linkAfterConnectRequest(bool accepted); + +// Sustained-drop rule for the notify frame ring. A drop is a frame the +// logger could not keep (ring full — the main loop fell behind — or a +// frame over kMaxFrameLen). One bad window is normal: an SD garbage- +// collection stall parks the main loop for up to ~2 s and the ring +// overflows once, and latest-value semantics lose nothing but +// superseded samples. Drops in kDropWindowsToFail CONSECUTIVE +// kDropWindowMs windows mean the stream is not being consumed; the +// caller then drops the link so the beacon path (and the reconnect) +// takes over rather than streaming into the floor. Feed the cumulative +// drop counter every loop while streaming; a stall spanning several +// windows is judged as one window (windows tumble on loop time). +constexpr uint32_t kDropWindowMs = 1000; +constexpr uint8_t kDropWindowsToFail = 3; + +struct DropMonitor { + bool started = false; + uint32_t windowStartMs = 0; + uint32_t dropsAtWindowStart = 0; + uint8_t badWindows = 0; +}; + +// Start a fresh evaluation at the current cumulative count. +void dropMonitorReset(DropMonitor& m, uint32_t totalDrops, uint32_t nowMs); + +// Returns true once, when the kDropWindowsToFail-th consecutive bad +// window closes (and re-arms the count). +bool dropMonitorUpdate(DropMonitor& m, uint32_t totalDrops, uint32_t nowMs); + +// ---- Stream surface rules (review fixes 2026-09) ------------------------- +// The GATT stream feeds the same sensoregg_protocol::Reading the beacon +// does, but it only carries the channels the descriptor maps — nothing +// else in that struct is refreshed by a frame, while every frame keeps +// the link's arrival stamp fresh. Two rules keep that from holding a +// value (a held value draws a flat line indistinguishable from data): + +// On committing STREAMING, clear everything the stream will (or can +// never) rewrite: temperatures NaN, battery 0xFF (unknown), flags / +// status / tcFault / pairingActive cleared, sequence 0. Sample frames +// carry no MCP STATUS and no pairing bit, so while streaming those +// read false (a fault still reaches the page as the sentinel -> NaN -> +// '---'). protoVersion is left as the last beacon's: it names the +// egg's beacon firmware, which the stream does not contradict. +void readingResetForStream(sensoregg_protocol::Reading& r); + +// Per-role receive stamps: each mapped role is fresh only while ITS OWN +// channel keeps arriving, so an EGT channel that stops while CJ carries +// on goes NaN instead of being held by CJ's traffic. A role that was +// never stamped (e.g. a pod with no IAT channel) is never fresh. +// +// Staleness per role = the frame's own cadence with one frame of slack, +// 2 x (n x interval_ms), floored at sensoregg_protocol::kStalenessMs +// (so a fast channel keeps exactly the 1 s rule) and capped at +// kRoleStaleMaxMs (a corrupt interval can't hold a value for minutes). +// The floor alone would be wrong for slow channels: the EGT pod's IAT +// runs at 1 s and its battery at 30 s, so a flat 1 s rule would flap +// IAT and blank the battery forever. +constexpr uint32_t kRoleStaleMaxMs = 60000; + +uint32_t roleStaleAfterMs(uint8_t n, uint16_t intervalMs); + +struct RoleFreshness { + bool have[ROLE_COUNT] = {false, false, false, false}; + uint32_t atMs[ROLE_COUNT] = {0, 0, 0, 0}; + uint32_t staleAfterMs[ROLE_COUNT] = {0, 0, 0, 0}; +}; + +void roleFreshnessReset(RoleFreshness& f); +void roleFreshnessStamp(RoleFreshness& f, Role role, uint32_t atMs, + uint8_t n, uint16_t intervalMs); +// Wrap-safe: fresh while (now - at) < staleAfter. +bool roleFresh(const RoleFreshness& f, Role role, uint32_t nowMs); + } // namespace sensoregg_gatt diff --git a/BirdsEye/sensoregg_protocol.cpp b/BirdsEye/sensoregg_protocol.cpp index d8d11c4..5bdbd9f 100644 --- a/BirdsEye/sensoregg_protocol.cpp +++ b/BirdsEye/sensoregg_protocol.cpp @@ -148,4 +148,9 @@ bool macAccepts(const uint8_t filterHuman[6], const uint8_t peerLsbFirst[6]) { return true; } +bool pairPersistDue(bool attempted, uint32_t lastAttemptMs, uint32_t nowMs) { + if (!attempted) return true; + return (uint32_t)(nowMs - lastAttemptMs) >= kPairPersistRetryMs; +} + } // namespace sensoregg_protocol diff --git a/BirdsEye/sensoregg_protocol.h b/BirdsEye/sensoregg_protocol.h index 2a84527..f882974 100644 --- a/BirdsEye/sensoregg_protocol.h +++ b/BirdsEye/sensoregg_protocol.h @@ -169,6 +169,19 @@ bool seqMonitorLive(const SeqMonitor& m, uint32_t nowMs); // before giving up. Matches the camera's pairing timeout. constexpr uint32_t kPairingTimeoutMs = 120000; +// Capture persist retry throttle. A capture persists FIRST (an SD +// read-modify-write of /SETTINGS.json); when that write fails the +// window stays open so a later frame retries. Unthrottled, every +// matching beacon (~10 Hz) re-ran the whole SD read-modify-write for up +// to the 2-minute window against a card that is already failing. +// ~1 Hz is plenty for a human-scale pairing gesture. +constexpr uint32_t kPairPersistRetryMs = 1000; + +// May a capture persist be attempted now? Always for the window's first +// attempt (attempted == false); afterwards only once kPairPersistRetryMs +// has passed since the last attempt (wrap-safe). +bool pairPersistDue(bool attempted, uint32_t lastAttemptMs, uint32_t nowMs); + // "AA:BB:CC:DD:EE:FF" + NUL. constexpr size_t kMacStrLen = 18; diff --git a/BirdsEye/sim/stubs/module_stubs.cpp b/BirdsEye/sim/stubs/module_stubs.cpp index c375dc7..a61344c 100644 --- a/BirdsEye/sim/stubs/module_stubs.cpp +++ b/BirdsEye/sim/stubs/module_stubs.cpp @@ -147,6 +147,7 @@ float sensoreggPacketHz() { return 0.0f; } void sensoreggGattClientInit() {} uint8_t sensoreggLinkMode() { return 0; } uint16_t sensoreggGattMtu() { return 0; } +uint32_t sensoreggFrameDrops() { return 0; } // ---- neopixel.ino surface ---- diff --git a/CHANGELOG.md b/CHANGELOG.md index d8cd041..a8e19cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,60 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2 (FAULT, no erase offer), and a volume that mounts on the retry boots normally. The recovered-on-probe path also now records the active SPI clock it had been leaving at 0. +- **SensorEgg GATT link can no longer wedge "streaming" with no link** + (plan 0018 review). If the egg dropped in the instant between the + bring-up finishing and the main loop committing it, the disconnect was + overwritten and the logger sat in STREAMING with no connection — the + scanner held off (a link counted as engaged) and EGT read `---` until + shutdown or a transfer. The commit now happens only while the staged + bring-up is still the live one (checked and set atomically), a + "connected" state with no connection handle is reconciled back to the + retry path, and a ready flag left over from an earlier bring-up can't + be consumed by a later one. +- **Shutdown and BLE transfer mode can no longer leave a SensorEgg link + up** (plan 0018 review). If the egg connection was completing inside + the radio at the moment the logger went to sleep or entered transfer + mode, the pending-connect cancel was a no-op and the link finished + coming up afterwards — occupying the radio for the whole transfer + session or charging park. The logger now drops such a late connection + the moment it arrives, and ignores the disconnect of a connection it + never adopted. +- **SensorEgg GATT stream no longer holds stale values** (plan 0018 + review). Fields only the beacon carries were kept from the last beacon + while the stream's frames kept the link "fresh": a thermocouple fault + seen once could read `*TC FAULT*` for the whole session, and a pod + without an intake-air channel logged a flat line into `Temp2`. And + since frames from any channel kept the link alive, an EGT channel that + stopped while cold-junction frames continued left EGT frozen. Starting + the stream now clears those fields, and every value goes `nan`/`---` + as soon as its own channel stops arriving (the 1 s rule for EGT/CJ; a + slow channel gets one frame of slack at its own rate, e.g. 2 s for a + 1 s intake-air channel). +- **A refused SensorEgg connect no longer costs 15 s of silence** (plan + 0018 review). When the radio refused the connection request outright, + the logger still waited out the full 10 s connect timeout and 5 s + backoff with the scanner paused, so neither the GATT stream nor the + beacon fed the EGT readout. It now backs off immediately and resumes + the scanner. +- **SensorEgg GATT frames of any legal size are received, and lost frames + are visible** (plan 0018 review). The logger's receive slots held only + 32 bytes, so an egg batching more than 11 samples per frame (the spec + allows 117) had every such frame silently discarded while the link + looked healthy. Slots now take the largest frame the link can carry + (244 bytes), the running count of dropped frames shows on EGG TEST as + `D`, and drops in three consecutive seconds drop the link so the + beacon takes over instead of streaming into the floor. +- **SensorEgg battery over GATT honours the pod's declared scaling** + (plan 0018 review). The battery channel's raw value was used as a + percent directly, ignoring the scale/offset the pod's channel table + declares, so a pod reporting in tenths of a percent would have read as + "unknown". It now converts like every other channel and clamps to + 0–100. +- **A failing SD card is no longer hammered during egg pairing** (plan + 0017 review). When saving a newly captured egg failed, the logger + retried the settings write on every matching beacon — about ten full + read-modify-writes of `SETTINGS.json` a second for up to two minutes. + Retries are now paced at about once a second. ### Added - **SensorEgg GATT link** (plan 0018): a paired egg is no longer just diff --git a/CLAUDE.md b/CLAUDE.md index 513d644..284e970 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1371,9 +1371,14 @@ hardware needs no power switch. Wake = chip reset = fresh `setup()`. subsystem 1) and is not paid at all while streaming. GATT bring-up (blocking discovery/reads) runs in the Bluefruit callback task — the one documented deviation from "callbacks only copy"; the notify data - plane keeps the copy-only frame-ring discipline. Degradations while - streaming: `tcFault` reads false (frames carry no MCP STATUS), - `protoVersion`/`pairingFlag` hold their last beacon values. + plane keeps the copy-only frame-ring discipline. **Committing the + stream clears the beacon-only fields** (temps NaN, battery 0xFF, + `tcFault`/pairing flag false — frames carry no MCP STATUS or pairing + bit; only `protoVersion` keeps its beacon value), and each value is + live only while **its own channel** keeps arriving + (`sensoregg_gatt::RoleFreshness`: 2 × the frame's n × interval, + floored at the 1 s rule, capped at 60 s) — the shared arrival stamp + alone would let any channel's traffic hold another's value. - **Scanner robustness (bench-proven, do not remove)**: (1) `Scanner.filterMSD(0xFFFF)` rejects ambient packets INLINE — Bluefruit self-resumes filtered reports, while an accepted report pauses scanning @@ -1398,8 +1403,9 @@ hardware needs no power switch. Wake = chip reset = fresh `setup()`. filter (so a different egg can be captured while one is paired), and the first parsed frame advertising the egg's OWN pairing-window flag (egg-side long-press, flags bit0) wins — persist-first (a failed SD - write keeps the window open to retry), then the RAM filter, then the - window closes. Unpair = persist-first `""` → accept-any. Applied LIVE + write keeps the window open to retry, throttled to ~1 Hz by + `pairPersistDue` rather than the ~10 Hz beacon rate), then the RAM + filter, then the window closes. Unpair = persist-first `""` → accept-any. Applied LIVE on pair/unpair — no reboot, same exception as `camera_serial`. The scan callback filters length + magic + MAC, copies **up to the largest known layout** (`kPayloadLenMax` — the old fixed-14 copy diff --git a/docs/plans/0017-sensoregg-pairing-menu.md b/docs/plans/0017-sensoregg-pairing-menu.md index 25e88a8..9cd0aaa 100644 --- a/docs/plans/0017-sensoregg-pairing-menu.md +++ b/docs/plans/0017-sensoregg-pairing-menu.md @@ -115,3 +115,14 @@ read BLE task), same care as `eggSleeping`. - Status: shipped with this plan's PR into BETA. - Deferred: manual MAC entry page (B1 reserved); DOVEX egg-battery column; sim egg-frame injection + flag-1 golden variant. + +## Review fixes (2026-09) + +- **E7 — capture persist retried at the beacon rate.** Persist-first + keeps the window open when `setSetting()` fails, but the retry fired + on every matching beacon (~10 Hz): a full SD read-modify-write of + `/SETTINGS.json` per frame for up to the 2-minute window, against a + card that is already failing. Retries now go through + `sensoregg_protocol::pairPersistDue()` (first attempt of a window + immediate, then ≥ `kPairPersistRetryMs` = 1 s apart; host-tested); + `sensoreggRequestPair()` re-arms the immediate first attempt. diff --git a/docs/plans/0018-sensoregg-gatt-link.md b/docs/plans/0018-sensoregg-gatt-link.md index e8e32f5..2e1fbb9 100644 --- a/docs/plans/0018-sensoregg-gatt-link.md +++ b/docs/plans/0018-sensoregg-gatt-link.md @@ -133,6 +133,80 @@ dropping the link for transfers and shutdown. - Status: shipped with this plan's PR into BETA. - Follow-ups: per-sample DOVEX row timestamping using the clock fit - (its anchor + epoch machinery ships here, unused by rows yet); + (its anchor + epoch machinery ships here; only the epoch check is + consumed — `clockFitPodToLogger()` is reserved for this); LESC bonding when the egg's phase 4 lands; multi-pod (the state machine is single-link by design today). + +## Review fixes (2026-09) + +Findings from the post-merge review, one commit each. The link-state +rules that race the Bluefruit callback task moved into +`sensoregg_gatt` (`LinkState` + decision helpers) so they are +host-tested; the sketch keeps only the critical sections and radio calls. + +- **E1 — STREAMING with no link.** The disconnect callback (a + higher-priority task) could run between the bring-up's ready flag and + the main loop's commit; the unconditional `STREAMING` write then + resurrected a dead link and held the scanner off. Now: + `linkMayCommitStreaming()` (state still BRINGUP + handle held) is + evaluated and applied inside one `taskENTER_CRITICAL()` section; + `linkReconcileOrphan()` sends BRINGUP/STREAMING-without-a-handle to + BACKOFF every loop; the connect callback clears any stale + ready/failed flag before touching the staging buffers, and + `SENSOREGG_SLEEP()` clears them too. +- **E2 — a link completing across `SENSOREGG_SLEEP()`.** With the + SoftDevice's CONNECTED raised but the deferred connect callback not + yet run, sleep saw no handle and `sd_ble_gap_connect_cancel()` was a + no-op; the callback then brought the link up inside a transfer + session or the charging park. The connect callback now asks + `linkAcceptCentralConnect()` (state CONNECTING, not sleeping, still + wanted) and disconnects the handle otherwise — which also covers a + connect that lands after the 10 s timeout gave up. The disconnect + callback ignores handles other than `eggConnHandle`, so refusing a + connection cannot knock the machine into BACKOFF. +- **E3 — beacon-only fields held under GATT traffic.** `tcFault`, the + aux temperature and the battery kept their last beacon values while + every frame refreshed the shared `eggRxMs`: a stuck `*TC FAULT*`, and + an IAT-less pod logging a flat line into `Temp2` (the never-hold rule, + broken). And because zombie detection watches only the fastest + channel, an EGT channel that stopped while CJ carried on held EGT. + Now `readingResetForStream()` clears the surface on commit (temps NaN, + battery 0xFF, flags/status/tcFault/pairing false, seq 0; protoVersion + kept) and the zombie monitor restarts; `RoleFreshness` stamps each + mapped role on its own frames, and every value accessor also requires + its role fresh while the stream was the last writer. *Judgement + call:* the per-role window is the channel's own frame cadence with one + frame of slack (`2 × n × interval_ms`), **floored at the 1 s + `kStalenessMs` rule** and capped at 60 s — a flat 1 s rule would flap + the pod's 1 s IAT channel and permanently blank its 30 s battery, while + the EGT/CJ channels (250 ms) keep exactly the beacon's 1 s rule. + This supersedes the "graceful degradations" bullet above: nothing but + `protoVersion` holds a beacon value any more. +- **E4 — `Central.connect()` result ignored.** A refused request never + yields a connect callback, so the machine sat in CONNECTING for the + 10 s timeout (+5 s backoff) with the scanner paused on the triggering + report. Now `linkAfterConnectRequest(false)` → BACKOFF, stamped, and + the callback resumes the scanner (when the scan is wanted) so the + beacon keeps feeding the surface. +- **E5 — oversize frames dropped silently.** 32-byte ring slots took + only ≤11-sample frames; the spec allows 117 per frame. Slots are now + `kMaxFrameLen` = 244 (ATT_MTU 247 − 3: every legal frame at any MTU the + logger negotiates), 8 deep = ~1.9 KB (was 256 B; depth kept so a ~1 s + main-loop stall at the ~7 Hz frame rate loses only superseded values). + `eggFrameDrops` is surfaced (`sensoreggFrameDrops()`, EGG TEST row 4 + `D`), and `DropMonitor` drops the link after drops in 3 consecutive + 1 s windows — one SD-stall burst is judged as a single window and + never fires it. +- **E6 — battery ignored scale/offset.** Now `sampleToReal()` then + `batteryPercent()` (NaN → 0xFF, clamp 0–100, rounded; NaN tested by + bit pattern for `-Ofast`). Out-of-range values now clamp instead of + reading "unknown". +- **E8 — clock-fit API with no firmware consumer.** Stated plainly + rather than deleted: the sketch anchors `eggClockFit` on every + bring-up and consumes only `clockFitSameEpoch()` (pod-reboot + detection). `clockFitPodToLogger()` and `halfRttMs` are anchored but + **not yet consumed** — reserved, tested, for the per-sample row + timestamping follow-up below. Documented in `sensoregg_gatt.h`, + `sensoregg.h` and at the sketch's `eggClockFit`. No sketch state was + truly dead (the fit itself carries the epoch). diff --git a/tests/sensoregg_gatt_test.cpp b/tests/sensoregg_gatt_test.cpp index 326cc1c..799297e 100644 --- a/tests/sensoregg_gatt_test.cpp +++ b/tests/sensoregg_gatt_test.cpp @@ -206,3 +206,190 @@ TEST_CASE("sensoregg_gatt - clock fit survives the u32 millis wrap") { // And a frame slightly BEFORE the anchor maps backward, not 4 Gms off. CHECK(clockFitPodToLogger(f, 0xFFFFFE00UL) == 500010UL - 256UL); } + +// --------------------------------------------------------------------------- +// Link state machine (review fixes 2026-09) +// --------------------------------------------------------------------------- + +TEST_CASE("sensoregg_gatt - STREAMING commits only onto the live bring-up") { + CHECK(linkMayCommitStreaming(LINK_BRINGUP, true)); + // E1: the disconnect callback beat the loop's commit — it already + // wrote BACKOFF and dropped the handle. The commit must not + // resurrect a dead link as STREAMING. + CHECK(!linkMayCommitStreaming(LINK_BACKOFF, false)); + CHECK(!linkMayCommitStreaming(LINK_BRINGUP, false)); + // A stale ready flag consumed after sleep (IDLE) never commits. + CHECK(!linkMayCommitStreaming(LINK_IDLE, false)); + CHECK(!linkMayCommitStreaming(LINK_IDLE, true)); + CHECK(!linkMayCommitStreaming(LINK_STREAMING, true)); + CHECK(!linkMayCommitStreaming(LINK_CONNECTING, true)); +} + +TEST_CASE("sensoregg_gatt - an orphaned connected state falls back to BACKOFF") { + CHECK(linkReconcileOrphan(LINK_STREAMING, false) == LINK_BACKOFF); + CHECK(linkReconcileOrphan(LINK_BRINGUP, false) == LINK_BACKOFF); + // Live links are untouched. + CHECK(linkReconcileOrphan(LINK_STREAMING, true) == LINK_STREAMING); + CHECK(linkReconcileOrphan(LINK_BRINGUP, true) == LINK_BRINGUP); + // States that legitimately hold no handle are untouched. + CHECK(linkReconcileOrphan(LINK_IDLE, false) == LINK_IDLE); + CHECK(linkReconcileOrphan(LINK_WAIT_ADV, false) == LINK_WAIT_ADV); + CHECK(linkReconcileOrphan(LINK_BACKOFF, false) == LINK_BACKOFF); + CHECK(linkReconcileOrphan(LINK_CONNECTING, false) == LINK_CONNECTING); +} + +TEST_CASE("sensoregg_gatt - the connect callback keeps only a wanted connect") { + CHECK(linkAcceptCentralConnect(LINK_CONNECTING, false, true)); + // E2: SENSOREGG_SLEEP() ran while the SoftDevice was establishing + // the link (state already IDLE, sleep gate up). + CHECK(!linkAcceptCentralConnect(LINK_IDLE, true, false)); + CHECK(!linkAcceptCentralConnect(LINK_CONNECTING, true, true)); + // Gate dropped (race over / unpaired) while connecting. + CHECK(!linkAcceptCentralConnect(LINK_CONNECTING, false, false)); + // The connect timeout already gave up and moved to BACKOFF. + CHECK(!linkAcceptCentralConnect(LINK_BACKOFF, false, true)); + CHECK(!linkAcceptCentralConnect(LINK_WAIT_ADV, false, true)); +} + +// --------------------------------------------------------------------------- +// Stream surface rules (review fixes 2026-09) +// --------------------------------------------------------------------------- + +TEST_CASE("sensoregg_gatt - committing a stream clears every beacon-only field") { + sensoregg_protocol::Reading r; + r.egtC = 650.0f; + r.junctionC = 30.0f; + r.auxC = 40.0f; // an IAT-less pod would otherwise hold this forever + r.flags = 0x03; + r.pairingActive = true; + r.tcFault = true; // would otherwise read *TC FAULT* all session + r.status = 0x10; + r.battery = 87; + r.sequence = 1234; + r.protoVersion = 2; + readingResetForStream(r); + CHECK(std::isnan(r.egtC)); + CHECK(std::isnan(r.junctionC)); + CHECK(std::isnan(r.auxC)); + CHECK(r.flags == 0); + CHECK(!r.pairingActive); + CHECK(!r.tcFault); + CHECK(r.status == 0); + CHECK(r.battery == 0xFF); + CHECK(r.sequence == 0); + CHECK(r.protoVersion == 2); // kept: names the beacon firmware +} + +TEST_CASE("sensoregg_gatt - role staleness follows the channel's own cadence") { + // Fast channel (EGT, one 250 ms sample per frame): exactly the + // beacon's 1 s rule. + CHECK(roleStaleAfterMs(1, 250) == sensoregg_protocol::kStalenessMs); + // Batched fast channel: 4 x 250 ms = 1 s per frame -> 2 s. + CHECK(roleStaleAfterMs(4, 250) == 2000UL); + // IAT at 1 s: 2 s, not a 1 s rule that would flap every frame. + CHECK(roleStaleAfterMs(1, 1000) == 2000UL); + // Battery at 30 s: 60 s (the cap). + CHECK(roleStaleAfterMs(1, 30000) == kRoleStaleMaxMs); + // Corrupt / extreme interval: capped, never minutes. + CHECK(roleStaleAfterMs(117, 65535) == kRoleStaleMaxMs); + // Aperiodic / zero: the floor. + CHECK(roleStaleAfterMs(1, 0) == sensoregg_protocol::kStalenessMs); +} + +TEST_CASE("sensoregg_gatt - each role goes stale on its own channel") { + RoleFreshness f; + // Never stamped (a pod without that channel) -> never fresh. + CHECK(!roleFresh(f, ROLE_AUX, 0)); + CHECK(!roleFresh(f, ROLE_EGT, 100)); + + roleFreshnessStamp(f, ROLE_EGT, 1000, 1, 250); + roleFreshnessStamp(f, ROLE_CJ, 1000, 1, 250); + CHECK(roleFresh(f, ROLE_EGT, 1999)); + // EGT's frames stop; CJ's keep coming. EGT must go NaN on its own. + roleFreshnessStamp(f, ROLE_CJ, 1900, 1, 250); + CHECK(!roleFresh(f, ROLE_EGT, 2000)); + CHECK(roleFresh(f, ROLE_CJ, 2000)); + CHECK(!roleFresh(f, ROLE_AUX, 2000)); + + // Wrap-safe across the u32 millis wrap. + roleFreshnessStamp(f, ROLE_BATT, 0xFFFFFF00UL, 1, 30000); + CHECK(roleFresh(f, ROLE_BATT, 0x00000100UL)); + + // Out-of-range role is ignored, never fresh. + roleFreshnessStamp(f, ROLE_COUNT, 0, 1, 250); + CHECK(!roleFresh(f, ROLE_COUNT, 0)); + + roleFreshnessReset(f); + CHECK(!roleFresh(f, ROLE_CJ, 2000)); +} + +TEST_CASE("sensoregg_gatt - a refused connect request backs off at once") { + CHECK(linkAfterConnectRequest(true) == LINK_CONNECTING); + // E4: no connect callback will follow a refusal — don't sit out the + // 10 s connect timeout with the scanner paused. + CHECK(linkAfterConnectRequest(false) == LINK_BACKOFF); +} + +// --------------------------------------------------------------------------- +// Frame ring sizing + sustained-drop rule (review fixes 2026-09) +// --------------------------------------------------------------------------- + +TEST_CASE("sensoregg_gatt - the largest legal frame fits the receive slot") { + CHECK(kMaxFrameLen == 244); // ATT_MTU 247 - 3 + CHECK(kMaxFrameLen == 247 - 3); + // And a full-size frame parses. + uint8_t buf[kMaxFrameLen] = {0}; + buf[9] = kMaxSamplesPerFrame; + SampleFrame f; + CHECK(parseSampleFrame(buf, sizeof(buf), f)); + CHECK(f.n == kMaxSamplesPerFrame); +} + +TEST_CASE("sensoregg_gatt - one stall's burst of drops never drops the link") { + DropMonitor m; + CHECK(!dropMonitorUpdate(m, 0, 0)); // first call arms + // A 2 s SD stall: the loop comes back once, 10 frames lost. + CHECK(!dropMonitorUpdate(m, 10, 2000)); + // Healthy afterwards: the bad streak resets. + CHECK(!dropMonitorUpdate(m, 10, 3000)); + CHECK(!dropMonitorUpdate(m, 12, 4000)); // one more bad window + CHECK(!dropMonitorUpdate(m, 12, 5000)); // clean again + CHECK(!dropMonitorUpdate(m, 12, 5999)); // mid-window: no verdict +} + +TEST_CASE("sensoregg_gatt - drops in consecutive windows drop the link once") { + DropMonitor m; + dropMonitorReset(m, 100, 0); + CHECK(!dropMonitorUpdate(m, 101, 1000)); // bad 1 + CHECK(!dropMonitorUpdate(m, 101, 1500)); // mid-window + CHECK(!dropMonitorUpdate(m, 105, 2000)); // bad 2 + CHECK(dropMonitorUpdate(m, 106, 3000)); // bad 3 -> fail + // Re-armed: it takes another full streak to fire again. + CHECK(!dropMonitorUpdate(m, 107, 4000)); + CHECK(!dropMonitorUpdate(m, 108, 5000)); + CHECK(dropMonitorUpdate(m, 109, 6000)); + // Wrap-safe window timing across the millis wrap. + dropMonitorReset(m, 0, 0xFFFFFF00UL); + CHECK(!dropMonitorUpdate(m, 1, 0x00000100UL)); // only 512 ms elapsed + CHECK(!dropMonitorUpdate(m, 1, 0x00000400UL)); // window closes, bad 1 +} + +TEST_CASE("sensoregg_gatt - battery percent honours the descriptor scaling") { + ChannelInfo c; + c.scale = 1.0f; + c.offset = 0.0f; + CHECK(batteryPercent(sampleToReal(87, c)) == 87); + CHECK(batteryPercent(sampleToReal(INT16_MIN, c)) == 0xFF); // sentinel + // E6: a pod declaring deci-percent must not read 870 -> "unknown". + c.scale = 0.1f; + CHECK(batteryPercent(sampleToReal(870, c)) == 87); + CHECK(batteryPercent(sampleToReal(875, c)) == 88); // rounds + // An offset applies too. + c.scale = 1.0f; + c.offset = -10.0f; + CHECK(batteryPercent(sampleToReal(60, c)) == 50); + // Out of range clamps rather than wrapping a uint8_t. + c.offset = 0.0f; + CHECK(batteryPercent(sampleToReal(250, c)) == 100); + CHECK(batteryPercent(sampleToReal(-5, c)) == 0); +} diff --git a/tests/sensoregg_protocol_test.cpp b/tests/sensoregg_protocol_test.cpp index fa4321c..835626d 100644 --- a/tests/sensoregg_protocol_test.cpp +++ b/tests/sensoregg_protocol_test.cpp @@ -397,3 +397,18 @@ TEST_CASE("sensoregg_protocol - macAccepts pins the LSB-first convention") { // exact bug class the reversal exists to prevent. CHECK(!macAccepts(filterHuman, filterHuman)); } + +TEST_CASE("sensoregg_protocol - pairing persist retries are throttled to ~1 Hz") { + using namespace sensoregg_protocol; + // The window's first capture attempt is immediate. + CHECK(pairPersistDue(false, 0, 0)); + CHECK(pairPersistDue(false, 5000, 5000)); + // After a failed write, beacons at ~10 Hz must not re-run the SD + // read-modify-write each time. + CHECK(!pairPersistDue(true, 5000, 5100)); + CHECK(!pairPersistDue(true, 5000, 5999)); + CHECK(pairPersistDue(true, 5000, 6000)); + // Wrap-safe. + CHECK(!pairPersistDue(true, 0xFFFFFF00UL, 0x00000100UL)); // 512 ms + CHECK(pairPersistDue(true, 0xFFFFFF00UL, 0x00000300UL)); // 1024 ms +}