From 5675b2fc58ab5aa7455cb7b0f7837fe5391e8291 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sun, 26 Jul 2026 17:39:24 +0530 Subject: [PATCH] fix: bound, dispatch and abstain across the record/response decoders Eight decode-layer defects, all found by audit and each covered by a regression test that fails against the previous behaviour. records.dart - rr_count from inner[18] was used verbatim with no cap and no value bound, so a stray 0x28 read 40 int16s across offsets 19..98 and turned ppg, accel, spo2, skin-temp and ambient bytes into fabricated R-R beats feeding RMSSD. Now capped at 8 (matching live.dart, which already rejected "large count = wrong offset") and each interval gated to 200-2500 ms (matching control.dart). Applies on the trusted v24/v12 path too, and rr_count now reports what was accepted. - _round/_jsRound laundered NaN/Infinity into a finite value. A non-finite accel vector now rejects the record instead of reporting a perfectly still wrist. live.dart - decodeRecord routed only v24/v25/v10, so v7/v9/v12/v18 returned null and lost their own timestamp. All known versions now route through parseR24, whose per-version offsets and plausibility gate are unchanged. - _r10Motion returned Motion(0,0) for a short frame, which published as a measured zero-motion sample. activity/stepsInc are now nullable so "not read" is distinguishable from "measured zero". control.dart - parseRealtimeHr declared n intervals but only ever extracted two, disagreeing with live.dart's realtimeRr on identical bytes. All declared slots are read, bounded, and an implausible count is refused. - getAlarmTime decoded the epoch at a fixed offset valid only for the 7-byte simple form, misreading the 20-byte rich form this package itself writes. Now dispatches on the leading form byte and emits nothing for an unknown form. - battery had no upper bound: 0xffff decoded as 6553.5%. Out-of-range values are now omitted rather than clamped. commands.dart - cmdBuzz did not range-check pattern, so cmdBuzz(0, 300) silently shipped effect 44 with a valid CRC. Now throws. The 2934-case parity oracle is unchanged and still passes. --- lib/src/commands.dart | 17 +- lib/src/control.dart | 73 +++++-- lib/src/live.dart | 43 +++- lib/src/records.dart | 80 ++++++- test/decode_guards_test.dart | 391 +++++++++++++++++++++++++++++++++++ 5 files changed, 574 insertions(+), 30 deletions(-) create mode 100644 test/decode_guards_test.dart diff --git a/lib/src/commands.dart b/lib/src/commands.dart index 6b2587f..5f595ff 100644 --- a/lib/src/commands.dart +++ b/lib/src/commands.dart @@ -158,8 +158,21 @@ Uint8List cmdToggleImu(int seq, bool on) => buildCommand(seq, Cmd.toggleImuMode, [on ? 0x01 : 0x00]); Uint8List cmdEnableOptical(int seq, bool on) => buildCommand(seq, Cmd.enableOpticalData, [revision1, on ? 0x01 : 0x00]); -Uint8List cmdBuzz(int seq, [int pattern = hapticShortPulse]) => - buildCommand(seq, Cmd.runHapticsPattern, [pattern, 0, 0, 0, 0]); +/// Play a haptic waveform effect (RUN_HAPTICS_PATTERN = 0x4F). +/// +/// [pattern] is a single u8 waveform-effect id. It is RANGE-CHECKED rather than +/// masked: `buildFrame` would happily wrap `cmdBuzz(0, 300)` to effect 44 and +/// hand the strap a perfectly CRC-valid packet playing the wrong effect, with +/// nothing to tell the caller. A value that does not fit a u8 is a caller bug, +/// so throw. (Contrast [cmdSetAlarm], which masks because its payload is a +/// pattern LIST already validated for length.) +Uint8List cmdBuzz(int seq, [int pattern = hapticShortPulse]) { + if (pattern < 0 || pattern > 0xff) { + throw ArgumentError.value( + pattern, 'pattern', 'haptic waveform effect must fit in a u8 (0-255)'); + } + return buildCommand(seq, Cmd.runHapticsPattern, [pattern, 0, 0, 0, 0]); +} // ── On-device haptic alarm (SET_ALARM_TIME = 0x42) ───────────────────────── // diff --git a/lib/src/control.dart b/lib/src/control.dart index e5f2784..b57b8e8 100644 --- a/lib/src/control.dart +++ b/lib/src/control.dart @@ -163,6 +163,11 @@ R10Lite? parseR10Lite(Uint8List inner) { } // ── Compact realtime HR (small 0x28 packet body) ───────────────────────────── + +/// RR slots the compact 0x28 form can hold: [10] [12] [14] [16], stopping +/// before the `wearing` byte at [18]. +const int _maxRealtimeRr = 4; + class RealtimeHr { final int hrBpm; final double hrPrecise; @@ -188,13 +193,23 @@ RealtimeHr? parseRealtimeHr(Uint8List inner) { // be one byte out of bounds. no rr_count byte just means no RR intervals, // not "reject this decode" (copilot review caught this, real bug). final n = inner.length > 9 ? inner[9] : 0; - if (n > 0 && inner.length >= 12) { - final v = u16(inner, 10); - if (v >= 200 && v <= 2500) rr.add(v); - } - if (n > 1 && inner.length >= 14) { - final v = u16(inner, 12); - if (v >= 200 && v <= 2500) rr.add(v); + // Read ALL declared intervals, not just the first two — this used to stop + // after slots 1 and 2, silently discarding beats 3 and 4 that live.dart's + // realtimeRr returns from the same bytes (fewer beats = a different RMSSD). + // + // The wire form has room for exactly four slots, at [10] [12] [14] [16], + // bounded by the `wearing` byte at [18]. A declared count above that cannot + // fit the layout, so — like live.dart, which rejects a large count as + // "wrong offset" — we emit NO intervals rather than reading `wearing` (or + // whatever follows) as a heartbeat. Values are gated to the same + // physiological range records.dart uses. + if (n > 0 && n <= _maxRealtimeRr) { + for (int i = 0; i < n; i++) { + final off = 10 + 2 * i; + if (off + 2 > inner.length) break; + final v = u16(inner, off); + if (v >= kMinRrMs && v <= kMaxRrMs) rr.add(v); + } } final wearing = inner.length > 18 ? inner[18] == 1 : true; return RealtimeHr(hr, hr.toDouble(), rr, wearing, ts); @@ -232,6 +247,12 @@ class HelloInfo { }); } +/// A battery percentage is 0..100. Anything else is a mis-read field, not a +/// battery level — callers must omit it, never clamp it (a clamp would report +/// a confident 100% for garbage bytes). +bool _validBatteryPct(double pct) => + pct.isFinite && pct >= 0.0 && pct <= 100.0; + List _asciiRuns(Uint8List data, int start, int minlen) { final runs = []; final cur = StringBuffer(); @@ -254,12 +275,20 @@ HelloInfo parseHello(Uint8List payload) { final info = HelloInfo(rawHex: _hex(payload)); if (payload.length < 10) return info; + // Battery is a u16 in tenths of a percent whose offset drifts across + // firmware, so we scan for the first field that could BE one. The upper + // bound used to be 1009 (= 100.9%), which let an impossible reading through; + // a percentage is 0..100 by definition, so 1000 is the ceiling. Out of + // range = not the battery field, keep scanning / leave batteryPct null. for (int off = 1; off < 10; off++) { if (off + 2 <= payload.length) { final v = u16(payload, off); - if (v >= 10 && v <= 1009) { - info.batteryPct = _round(v / 10.0, 1); - break; + if (v >= 10 && v <= 1000) { + final pct = _round(v / 10.0, 1); + if (_validBatteryPct(pct)) { + info.batteryPct = pct; + break; + } } } } @@ -397,12 +426,30 @@ CmdResponse? parseCommandResponse(Uint8List inner) { final payload = Uint8List.sublistView(inner, 3); final dec = {}; if (op == Cmd.getBatteryLevel && inner.length >= 7) { - dec['battery_pct'] = _round(u16(inner, 5) / 10.0, 1); // u16 LE @[5:7] / 10 + // u16 LE @[5:7] in tenths of a percent. A battery percentage outside + // 0..100 is not a battery percentage — `ff ff` here used to surface as + // 6553.5%. Emit nothing rather than a number the UI would render. + final pct = _round(u16(inner, 5) / 10.0, 1); + if (_validBatteryPct(pct)) dec['battery_pct'] = pct; } else if (op == Cmd.getHelloHarvard) { final h = parseHello(payload); dec['hello'] = h; - } else if (op == Cmd.getAlarmTime && payload.length >= 5) { - dec['alarm_epoch'] = u32(payload, 1); + } else if (op == Cmd.getAlarmTime && payload.isNotEmpty) { + // GET_ALARM_TIME echoes whichever alarm form the strap holds, and the + // epoch offset DIFFERS between them (this package writes both — see + // cmdSetAlarmSimple / cmdSetAlarm): + // 0x01 simple, 7 B: [0x01][u32 epoch][u16 subsec] → epoch @1 + // 0x04 rich, 20 B: [0x04][u8 index][u32 epoch][u16 subsec][12 B haptics] + // → epoch @2 + // Reading @1 unconditionally decoded the rich form's [index][epoch:3] as + // the epoch. An unrecognised leading byte is an unknown form: emit no + // alarm_epoch rather than guessing an offset. + final form = payload[0]; + if (form == 0x01 && payload.length >= 5) { + dec['alarm_epoch'] = u32(payload, 1); + } else if (form == 0x04 && payload.length >= 6) { + dec['alarm_epoch'] = u32(payload, 2); + } } else if (op == Cmd.getAdvertisingNameHarvard) { dec['strap_name'] = _decodeAdvName(payload); } else if (op == Cmd.getClock) { diff --git a/lib/src/live.dart b/lib/src/live.dart index 3883dd8..d21864b 100644 --- a/lib/src/live.dart +++ b/lib/src/live.dart @@ -12,10 +12,21 @@ import 'records.dart'; class DecodedSample { final int ts; // unix seconds final int hr; // bpm (0 = off-wrist / no reading) - final double activity; // motion magnitude (stddev of |accel(g)|), 0 if no IMU - final int stepsInc; // steps detected in this record's IMU window (R10 only) + + /// Motion magnitude (stddev of |accel(g)|) over this record's IMU window. + /// + /// NULL means "this record carried no usable IMU data" — an R10 frame that + /// was truncated before the accel arrays. That is NOT the same claim as + /// `0.0`, which means "the IMU was read and the wrist was still". The old + /// code returned 0.0 for both, so a truncated frame was indistinguishable + /// from a genuine zero-motion reading downstream. + final double? activity; + + /// Steps detected in this record's IMU window (R10 only). Null when the IMU + /// window was unreadable (see [activity]); `0` means "read, no gait found". + final int? stepsInc; final bool wristOn; // worn proxy (hr>0) - final int recType; // 10 | 24 | 28 + final int recType; // 7 | 9 | 10 | 12 | 18 | 24 | 25 | 28 DecodedSample({ required this.ts, @@ -170,8 +181,13 @@ class _Motion { } // Decode the R10 IMU arrays into (activity, steps) over the 100-sample window. -_Motion _r10Motion(ByteData view, int len) { - if (len < 685) return _Motion(0, 0); +// Returns NULL when the frame is too short to contain the accel arrays at all +// — "we could not measure motion", which the caller surfaces as a null +// activity/steps rather than a fabricated 0.0/0 that reads like a real +// zero-motion sample. A non-null result with steps==0 IS a measurement: the +// window was read and no gait rhythm was found. +_Motion? _r10Motion(ByteData view, int len) { + if (len < 685) return null; const acc = 1 / 4096; List arr(int off) { final out = []; @@ -184,7 +200,7 @@ _Motion _r10Motion(ByteData view, int len) { final ax = arr(85), ay = arr(285), az = arr(485); final n = math.min(ax.length, math.min(ay.length, az.length)); - if (n == 0) return _Motion(0, 0); + if (n == 0) return null; // nothing measured — same absence as a short frame. final mags = []; for (int i = 0; i < n; i++) { final x = ax[i] * acc, y = ay[i] * acc, z = az[i] * acc; @@ -283,8 +299,13 @@ DecodedSample? decodeRecord(String hex) { if (b.length < 18) return null; - // WHOOP 4 historical telemetry (v24 / v25 auto-routed by parseR24). - if (recType == 24 || recType == 25) { + // WHOOP 4 historical telemetry — EVERY layout version parseR24 has a field + // map for, not just v24/v25. v12 in particular is real, shipping firmware + // (Record.r12); routing it to null here cost the caller the record's own + // timestamp, so a multi-day backfill collapsed onto the capture time. + // parseR24 owns the per-version HR offset and the plausibility gate, so the + // versions it cannot decode honestly still come back null. + if (kKnownRecordVersions.contains(recType)) { final d = parseR24(b); if (d == null) return null; return DecodedSample( @@ -301,12 +322,14 @@ DecodedSample? decodeRecord(String hex) { if (recType == 10) { final ts = view.getUint32(7, Endian.little); final hr = b[17]; + // Null motion = the frame carried no readable IMU window; propagate that + // absence instead of reporting a measured zero. final m = _r10Motion(view, b.length); return DecodedSample( ts: ts, hr: hr, - activity: m.activity, - stepsInc: m.steps, + activity: m?.activity, + stepsInc: m?.steps, wristOn: hr > 0, recType: 10, ); diff --git a/lib/src/records.dart b/lib/src/records.dart index 3d1123b..2c354ce 100644 --- a/lib/src/records.dart +++ b/lib/src/records.dart @@ -13,6 +13,12 @@ class R24 { final int hr; // heart rate bpm @ inner[17] (frame[21], v24/v12) /// Beat-to-beat (R-R) intervals in ms for this 1 s record, 0–4 of them. + /// + /// [rrCount] is the number of intervals we ACCEPTED, i.e. always + /// `rrIntervalsMs.length` — not the raw declared count byte. A record whose + /// declared count is implausible (see [kMaxRrPerRecord]) or whose interval + /// values fall outside [kMinRrMs]..[kMaxRrMs] contributes nothing here rather + /// than handing HRV a fabricated beat. final int rrCount; final List rrIntervalsMs; @@ -91,14 +97,55 @@ class R24 { // Replicate JavaScript Math.round semantics: round half toward +Infinity // (Math.round(2.5)=3, Math.round(-2.5)=-2). Dart's roundToDouble rounds half // away from zero, so we implement the JS rule explicitly. -double _jsRound(double v) => (v + 0.5).floorToDouble(); +// +// NaN / ±Infinity are passed through UNCHANGED rather than being folded to 0.0 +// (which is what control.dart's `_round` does). Rounding cannot manufacture a +// real number out of a non-finite one: a NaN accel component means the four +// bytes we read are not a float32 field, and silently emitting 0.0 would turn +// "these bytes are not accel" into "the wrist was perfectly still" — a +// fabricated measurement that then propagates through every downstream +// mean/std. control.dart can afford 0.0 because its callers only use it for +// display scalars; here the honest answer is to keep the non-finite value +// visible so [_parseV24Layout] can REJECT the whole record (see the +// `isFinite` gate there), which is an absence the API can already express +// (parseR24 returns null). +double _jsRound(double v) { + if (!v.isFinite) return v; + return (v + 0.5).floorToDouble(); +} /// Round `v` to `decimals` places using JS `Math.round(v*p)/p` semantics. +/// Non-finite input is returned unchanged — see [_jsRound]. double _round(double v, int decimals) { + if (!v.isFinite) return v; final p = _pow10(decimals); return _jsRound(v * p) / p; } +/// Largest R-R interval count a single 1 s historical record can plausibly +/// declare. The sibling realtime decoder (live.dart `realtimeRr`) applies the +/// same ceiling for the same reason: the wire form carries 0–4 beats, so a +/// larger count byte means we are reading the wrong offset — not a second +/// containing dozens of heartbeats. A record declaring more than this yields +/// NO intervals at all (absence), never a truncated read of whatever bytes +/// happen to follow (ppg, accel float32s, skin contact, spo2, temp, ambient). +const int kMaxRrPerRecord = 8; + +/// Physiologically possible beat-to-beat interval bounds, ms — 2500 ms = 24 bpm, +/// 200 ms = 300 bpm. Identical to the bound control.dart's `parseRealtimeHr` +/// applies to the realtime RR slots, so both decoders agree on what a beat is. +const int kMinRrMs = 200; +const int kMaxRrMs = 2500; + +/// The historical-record layout versions this package has a field map for. +/// v25 has its own dedicated layout ([_parseV25]); the rest share the v24 field +/// map with a per-version HR offset ([_hrOffsetByVersion]). Used by live.dart's +/// `decodeRecord` so every version [parseR24] can decode is actually routed to +/// it (v12 in particular is real firmware and used to fall through to null, +/// which cost callers the record's own timestamp). +final Set kKnownRecordVersions = + Set.unmodifiable({..._hrOffsetByVersion.keys, 25}); + double _pow10(int n) { double p = 1; for (int i = 0; i < n; i++) { @@ -264,12 +311,25 @@ R24? _parseV24Layout( ); // R-R intervals: rr_count @ [18], then rr_count signed int16 LE from [19]. - final rrCount = inner[18]; + // + // The declared count is UNTRUSTED. Taken raw it addresses up to 255 int16s + // starting at [19], which walks straight through ppg@29/31, the accel + // float32s@36/40/44, skin contact@51, spo2@64/66, skin temp@68 and + // ambient@70 — reinterpreting all of them as "beats" that then feed + // RMSSD/HRV. So: reject an implausible count outright, and accept only + // values inside the physiological interval range. Both guards run on EVERY + // path, including the "trusted" v24/v12 one that skips + // [_physiologicallyPlausible]. + final declaredRrCount = inner[18]; final rrIntervalsMs = []; - for (int i = 0; i < rrCount && 19 + 2 * i + 2 <= inner.length; i++) { - final v = view.getInt16(19 + 2 * i, Endian.little); - if (v > 0) rrIntervalsMs.add(v); + if (declaredRrCount <= kMaxRrPerRecord) { + for (int i = 0; i < declaredRrCount && 19 + 2 * i + 2 <= inner.length; i++) { + final v = view.getInt16(19 + 2 * i, Endian.little); + if (v >= kMinRrMs && v <= kMaxRrMs) rrIntervalsMs.add(v); + } } + // rrCount reports what we accepted, never what the byte claimed. + final rrCount = rrIntervalsMs.length; final hr = inner[hrOffset]; final accelG = [ @@ -278,6 +338,16 @@ R24? _parseV24Layout( _round(view.getFloat32(44, Endian.little), 4), ]; + // A NaN / ±Infinity accel component means bytes [36:48] are not the float32 + // vector this field map claims. Emitting 0.0 (or the raw NaN) would poison + // every downstream mean/std with a value that reads as a real measurement, + // so we reject the record instead — the caller already handles a null decode + // by archiving the raw bytes. This runs BEFORE the validate gate so it also + // covers the trusted v24/v12 path, which skips [_physiologicallyPlausible]. + for (final c in accelG) { + if (!c.isFinite) return null; + } + if (validate && !_physiologicallyPlausible(accelG, hr)) { return null; } diff --git a/test/decode_guards_test.dart b/test/decode_guards_test.dart new file mode 100644 index 0000000..4dd8e13 --- /dev/null +++ b/test/decode_guards_test.dart @@ -0,0 +1,391 @@ +// Regression tests for the decode guards that keep the package's "never +// fabricate" contract: a decoder must emit ABSENCE, not a plausible-looking +// default, when the bytes do not actually carry the field. +// +// Every test here fails against the pre-guard behavior. + +import 'dart:io'; +import 'dart:typed_data'; + +import 'package:openstrap_protocol/openstrap_protocol.dart'; +import 'package:test/test.dart'; + +/// Record 0 of the golden capture — a real, CRC-valid v24 historical record +/// (inner starts at packet type 0x2f). 96 bytes. Every mutation below starts +/// from this known-good record so the ONLY thing under test is the guard. +const String _goodV24 = + '2f1805f13ced01c261d2698848805454016202d802f5010000000000609f0fff80de23' + '3d3dda19be5c87a9be7b14803f0020dc463dda19be5c87a9be7b14803f16028402bc02' + '86025e01000b010c020c0000000000370001510c000000000000'; + +String _hex(Uint8List b) => + b.map((x) => x.toRadixString(16).padLeft(2, '0')).join(); + +/// A copy of [hex] with [patch]'s byte values written at their offsets. +String _patched(String hex, Map patch) { + final b = Uint8List.fromList(hexToBytes(hex)); + patch.forEach((off, v) => b[off] = v); + return _hex(b); +} + +/// Build a compact 0x28 realtime-HR inner frame: +/// `[0x28][rec][ts u32 @2][..][hr @8][rr_count @9][rr @10,12,14,16][wearing @18]` +Uint8List _realtimeHrFrame({ + required int ts, + required int hr, + required int declaredCount, + required List rrMs, +}) { + final b = Uint8List(19); + final v = ByteData.sublistView(b); + b[0] = 0x28; + b[1] = 0x01; + v.setUint32(2, ts, Endian.little); + b[8] = hr; + b[9] = declaredCount; + for (int i = 0; i < rrMs.length && i < 4; i++) { + v.setUint16(10 + 2 * i, rrMs[i], Endian.little); + } + b[18] = 1; // wearing + return b; +} + +/// Wrap a COMMAND_RESPONSE body: `[0x24][seq][opcode] + payload`. +Uint8List _cmdResponse(int opcode, List payload) => + Uint8List.fromList([0x24, 0x11, opcode, ...payload]); + +List _u32le(int v) => + [v & 0xff, (v >> 8) & 0xff, (v >> 16) & 0xff, (v >> 24) & 0xff]; + +void main() { + // ── 1. R-R count / value bounds in the historical record decoder ────────── + group('R24 R-R intervals are bounded (count + physiological value)', () { + test('baseline: the unmodified record still decodes its 2 real beats', () { + final r = parseR24(hexToBytes(_goodV24))!; + expect(r.rrCount, 2); + expect(r.rrIntervalsMs, [728, 501]); + }); + + test('an implausible declared count yields NO intervals, not 38 of them', + () { + // inner[18] = 0x28 (40). Taken raw this reads 40 int16s from [19:99], + // turning ppg@29/31, the accel float32s@36/40/44, skin contact@51, + // spo2@64/66, skin temp@68 and ambient@70 into "beats" that feed HRV. + final r = parseR24(hexToBytes(_patched(_goodV24, {18: 0x28})))!; + expect(r.rrCount, 0, reason: 'count above the ceiling must be rejected'); + expect(r.rrIntervalsMs, isEmpty); + }); + + test('the count guard applies on the TRUSTED v24/v12 path too', () { + // v24 and v12 skip the physiological-plausibility gate entirely, so the + // R-R guard has to be independent of it. + for (final version in [24, 12]) { + final r = parseR24( + hexToBytes(_patched(_goodV24, {1: version, 18: 0xff})))!; + expect(r.rrCount, 0, reason: 'v$version must still bound the count'); + expect(r.rrIntervalsMs, isEmpty, reason: 'v$version'); + } + }); + + test('an R-R value below the physiological floor is dropped', () { + // count=1, interval = 5 ms (12000 bpm). The old filter was only `v > 0`. + final r = parseR24( + hexToBytes(_patched(_goodV24, {18: 1, 19: 0x05, 20: 0x00})))!; + expect(r.rrIntervalsMs, isEmpty); + expect(r.rrCount, 0); + }); + + test('an R-R value above the physiological ceiling is dropped', () { + // count=1, interval = 30000 ms as int16 → 30000 is > 2500 ms (24 bpm). + final r = parseR24( + hexToBytes(_patched(_goodV24, {18: 1, 19: 0x30, 20: 0x75})))!; + expect(r.rrIntervalsMs, isEmpty); + expect(r.rrCount, 0); + }); + + test('rrCount reports what was accepted, never the raw declared byte', () { + // count=2, first beat valid (800 ms), second beat 1 ms → 1 accepted. + final r = parseR24(hexToBytes(_patched(_goodV24, { + 18: 2, + 19: 0x20, 20: 0x03, // 800 + 21: 0x01, 22: 0x00, // 1 + })))!; + expect(r.rrIntervalsMs, [800]); + expect(r.rrCount, r.rrIntervalsMs.length); + }); + + test('FirmwareAwareR24Decoder inherits the same bounds', () { + final r = FirmwareAwareR24Decoder() + .decode(hexToBytes(_patched(_goodV24, {18: 0x28})))!; + expect(r.rrCount, 0); + expect(r.rrIntervalsMs, isEmpty); + }); + }); + + // ── 2. Non-finite accel is rejected, never rounded to 0.0 ──────────────── + group('R24 rejects a non-finite accel vector', () { + // float32 quiet-NaN / +Infinity little-endian byte patterns. + const nan = [0x00, 0x00, 0xc0, 0x7f]; + const inf = [0x00, 0x00, 0x80, 0x7f]; + + Map at(int off, List bytes) => + {for (int i = 0; i < bytes.length; i++) off + i: bytes[i]}; + + test('a CRC-valid v24 record with NaN accel decodes to null', () { + for (final off in [36, 40, 44]) { + final r = parseR24(hexToBytes(_patched(_goodV24, at(off, nan)))); + expect(r, isNull, reason: 'NaN at inner[$off] must reject the record'); + } + }); + + test('an Infinity accel component decodes to null', () { + for (final off in [36, 40, 44]) { + final r = parseR24(hexToBytes(_patched(_goodV24, at(off, inf)))); + expect(r, isNull, reason: '+Inf at inner[$off] must reject the record'); + } + }); + + test('the guard covers the trusted v12 path, not just the validated ones', + () { + final r = parseR24( + hexToBytes(_patched(_goodV24, {1: 12, ...at(36, nan)}))); + expect(r, isNull); + }); + + test('FirmwareAwareR24Decoder also rejects it (no fallback laundering)', + () { + final r = FirmwareAwareR24Decoder() + .decode(hexToBytes(_patched(_goodV24, at(40, nan)))); + expect(r, isNull); + }); + + test('a finite accel record is unaffected', () { + final r = parseR24(hexToBytes(_goodV24))!; + expect(r.accelG.every((c) => c.isFinite), isTrue); + expect(r.accelG[2], closeTo(1.0006, 1e-4)); + }); + }); + + // ── 3. decodeRecord routes every known historical version ──────────────── + group('decodeRecord routes all historical record versions', () { + test('v12 carries its OWN timestamp instead of decoding to null', () { + final s = decodeRecord(_patched(_goodV24, {1: 12})); + expect(s, isNotNull, + reason: 'Record.r12 is real firmware and must not fall through'); + expect(s!.ts, 1775395266); + expect(s.hr, 98); + expect(s.recType, 12); + }); + + test('v9 and v18 route through parseR24 as well', () { + for (final version in [9, 18]) { + final s = decodeRecord(_patched(_goodV24, {1: version})); + expect(s, isNotNull, reason: 'v$version must be routed'); + expect(s!.ts, 1775395266, reason: 'v$version ts'); + expect(s.recType, version); + } + }); + + test('routing does NOT bypass the plausibility gate', () { + // v7 reads HR at inner[27], which is 0 in this record → implausible, so + // parseR24 declines and decodeRecord must surface that as null rather + // than a 0-bpm sample. + expect(decodeRecord(_patched(_goodV24, {1: 7})), isNull); + }); + + test('v24 / v25 behavior is unchanged', () { + final s = decodeRecord(_goodV24)!; + expect(s.recType, 24); + expect(s.ts, 1775395266); + }); + + test('an unknown version is still not routed here', () { + // Only versions with a field map are routed; anything else stays null so + // decodeRecord never invents a timestamp for a layout we cannot read. + expect(decodeRecord(_patched(_goodV24, {1: 11})), isNull); + }); + }); + + // ── 4. parseRealtimeHr reads every declared R-R slot ───────────────────── + group('parseRealtimeHr R-R extraction', () { + test('all four declared intervals are returned, not just the first two', + () { + final f = _realtimeHrFrame( + ts: 1775395266, + hr: 62, + declaredCount: 4, + rrMs: [800, 810, 820, 830], + ); + expect(parseRealtimeHr(f)!.rrMs, [800, 810, 820, 830]); + }); + + test('it agrees with live.dart realtimeRr on the same bytes', () { + final f = _realtimeHrFrame( + ts: 1775395266, + hr: 62, + declaredCount: 3, + rrMs: [900, 910, 920], + ); + expect(parseRealtimeHr(f)!.rrMs, realtimeRr(_hex(f))!.rrMs); + }); + + test('an implausible declared count yields no intervals at all', () { + // 40 cannot fit the 4-slot wire form → we are reading the wrong offset. + final f = _realtimeHrFrame( + ts: 1775395266, + hr: 62, + declaredCount: 40, + rrMs: [800, 810, 820, 830], + ); + expect(parseRealtimeHr(f)!.rrMs, isEmpty); + }); + + test('out-of-range slot values are dropped, in-range ones kept', () { + final f = _realtimeHrFrame( + ts: 1775395266, + hr: 62, + declaredCount: 4, + rrMs: [800, 5, 3000, 830], + ); + expect(parseRealtimeHr(f)!.rrMs, [800, 830]); + }); + }); + + // ── 5. GET_ALARM_TIME dispatches on the on-wire alarm form ─────────────── + group('GET_ALARM_TIME form dispatch', () { + const epoch = 1775395266; + + test('the RICH (0x04) form reads the epoch at offset 2', () { + // [0x04][u8 index][u32 epoch][u16 subsec][12-byte haptic pattern] + final payload = [ + 0x04, 0x00, ..._u32le(epoch), 0x00, 0x40, + ...kDefaultAlarmHaptics, + ]; + expect(payload.length, 20); + final r = parseCommandResponse(_cmdResponse(0x43, payload))!; + expect(r.decoded['alarm_epoch'], epoch); + }); + + test('the rich form written by cmdSetAlarm round-trips', () { + final when = DateTime.fromMillisecondsSinceEpoch(epoch * 1000); + final frame = cmdSetAlarm(0, when); + final inner = parseFrame(frame)!.inner; + // inner = [0x23][seq][0x42] + payload (+ pad4); the response echoes the + // same payload under opcode 0x43. + final payload = inner.sublist(3); + final r = parseCommandResponse(_cmdResponse(0x43, payload))!; + expect(r.decoded['alarm_epoch'], epoch); + }); + + test('the SIMPLE (0x01) form still reads the epoch at offset 1', () { + final payload = [0x01, ..._u32le(epoch), 0x00, 0x40]; + expect(payload.length, 7); + final r = parseCommandResponse(_cmdResponse(0x43, payload))!; + expect(r.decoded['alarm_epoch'], epoch); + }); + + test('an unknown form emits no alarm_epoch rather than a guessed offset', + () { + final payload = [0x07, 0x00, ..._u32le(epoch), 0x00, 0x40]; + final r = parseCommandResponse(_cmdResponse(0x43, payload))!; + expect(r.decoded.containsKey('alarm_epoch'), isFalse); + }); + }); + + // ── 6. R10 motion: "not measured" is not "measured zero" ───────────────── + group('R10 motion absence is distinguishable from a measured zero', () { + // Truncated R10: header + HR present, but the accel arrays (which start at + // [85] and run to [685]) are not. + String shortR10() { + final b = Uint8List(200); + final v = ByteData.sublistView(b); + b[0] = 0x2b; + b[1] = 0x0a; + v.setUint32(7, 1775395266, Endian.little); + b[17] = 61; + return _hex(b); + } + + test('a truncated R10 reports null activity/steps, not 0.0/0', () { + final s = decodeRecord(shortR10())!; + expect(s.recType, 10); + expect(s.ts, 1775395266); + expect(s.hr, 61); + expect(s.activity, isNull, + reason: 'no IMU window was read — that is not a zero-motion reading'); + expect(s.stepsInc, isNull); + }); + + test('a full R10 still reports a measured activity', () { + final full = File('test/r10_fixture.hex').readAsStringSync().trim(); + final s = decodeRecord(full)!; + expect(s.activity, isNotNull); + expect(s.activity, greaterThanOrEqualTo(0.0)); + expect(s.stepsInc, isNotNull); + }); + + test('toMap carries the absence through as null', () { + expect(decodeRecord(shortR10())!.toMap()['activity'], isNull); + expect(decodeRecord(shortR10())!.toMap()['steps_inc'], isNull); + }); + }); + + // ── 7. Battery percentages are bounded to 0..100 ───────────────────────── + group('battery level is bounded', () { + test('GET_BATTERY_LEVEL with 0xffff emits nothing, not 6553.5%', () { + final r = parseCommandResponse( + Uint8List.fromList([0x24, 0x11, 0x1a, 0x00, 0x00, 0xff, 0xff]))!; + expect(r.decoded.containsKey('battery_pct'), isFalse); + }); + + test('an in-range battery level still decodes', () { + final r = parseCommandResponse( + Uint8List.fromList([0x24, 0x11, 0x1a, 0x00, 0x00, 0x2c, 0x01]))!; + expect(r.decoded['battery_pct'], closeTo(30.0, 1e-9)); + }); + + test('exactly 100.0% is accepted', () { + final r = parseCommandResponse( + Uint8List.fromList([0x24, 0x11, 0x1a, 0x00, 0x00, 0xe8, 0x03]))!; + expect(r.decoded['battery_pct'], closeTo(100.0, 1e-9)); + }); + + test('parseHello rejects an above-100% candidate instead of reporting it', + () { + // u16 @1 = 0x03ed = 1005 → 100.5%, which is not a battery percentage. + // Every later offset is 0xffff, so nothing else qualifies → null. + final payload = Uint8List.fromList( + [0x00, 0xed, 0x03, ...List.filled(9, 0xff)]); + expect(parseHello(payload).batteryPct, isNull); + }); + + test('parseHello still reads a real battery field', () { + // The real captured HELLO body: battery is 89.9% at offset 3. + const body = + 'a001048303000000b196e201b0020000344332323438303932003865323738326237' + '346634303238346333663437363138623062613234373663356431366363303533' + '313862373532316431353635650600000002000000100000002900000011000000'; + final h = parseHello(hexToBytes(body)); + expect(h.batteryPct, isNotNull); + expect(h.batteryPct!, inInclusiveRange(0.0, 100.0)); + expect(h.batteryPct, closeTo(89.9, 1e-9)); + }); + }); + + // ── 8. cmdBuzz rejects an out-of-range pattern ─────────────────────────── + group('cmdBuzz pattern is range-checked', () { + test('an out-of-u8 pattern throws instead of silently wrapping', () { + // 300 & 0xff == 44: the old builder emitted a CRC-valid packet playing + // waveform effect 44 with no indication anything was wrong. + expect(() => cmdBuzz(0, 300), throwsA(isA())); + expect(() => cmdBuzz(0, 256), throwsA(isA())); + expect(() => cmdBuzz(0, -1), throwsA(isA())); + }); + + test('valid patterns still build the same bytes', () { + expect(cmdBuzz(0), cmdBuzz(0, 2)); + expect(() => cmdBuzz(0, 255), returnsNormally); + expect(() => cmdBuzz(0, 0), returnsNormally); + }); + }); +}