SensorEgg: GATT link and pairing review fixes (plans 0017/0018)

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<n>, 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.

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<n>; drops in 3 consecutive
+// 1 s windows drop the link so the beacon path takes over.
+uint32_t sensoreggFrameDrops();

BirdsEye/sensoregg_gatt.cpp

@@ -3,6 +3,8 @@
 #include <math.h>
 #include <string.h>
 
+#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

BirdsEye/sensoregg_gatt.h

@@ -21,6 +21,8 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#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

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