From 9d5481a0b220d680ab468f31ed7fd8b49d8172b1 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 08:18:33 +0530 Subject: [PATCH 01/33] gate the r10 decoders on the packet type decodeRecord, realtimeRr and frameAccel took b[1]==10 as an R10 record on any packet type once the frame was long enough. On console-log and command response frames b[1] is the sequence byte, so a 700-byte log frame with seq 10 decoded as an R10 sample with an hr read out of the log text and motion built from ASCII. They now accept only a live 0x2B frame or an archived 0x2F frame of R10 length, the same gate parseR24 already has. --- lib/src/live.dart | 21 ++++++++++---- test/decode_guards_test.dart | 55 ++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/lib/src/live.dart b/lib/src/live.dart index 520c689..1d27894 100644 --- a/lib/src/live.dart +++ b/lib/src/live.dart @@ -21,6 +21,17 @@ import 'records.dart'; /// unrecognized version. const int kR10MinLength = 685; +/// Whether a frame with packet type [pkt] and length [len] can carry an R10 +/// body. Only a live raw (0x2B) frame, at any length, or an archived +/// historical (0x2F) frame of at least [kR10MinLength] qualifies. On every +/// other packet type b[1] is the SEQUENCE byte, not a record type, so a +/// console-log (0x32, up to 2 KB) or long command-response frame whose seq +/// happened to be 10 would otherwise decode as R10: an "hr" read out of log +/// text, plus R-R and IMU motion built from the payload bytes. +bool _isR10Frame(int pkt, int len) => + pkt == PacketType.realtimeRawData || + (pkt == PacketType.historicalData && len >= kR10MinLength); + /// A decoded HR/activity sample. class DecodedSample { final int ts; // unix seconds @@ -141,7 +152,9 @@ ImuFrame? frameAccel(String hex) { return ts > 0 ? ImuFrame(ts, idx, mags, xs, ys, zs) : null; } // R10: rec 0x0A, ts@7, accel X@85/Y@285/Z@485 (100 int16 each). - if (rec == 0x0a && b.length >= kR10MinLength) { + if (rec == 0x0a && + b.length >= kR10MinLength && + _isR10Frame(pkt, b.length)) { final ts = view.getUint32(7, Endian.little); final mags = []; final xs = []; @@ -234,8 +247,7 @@ RealtimeRrResult? realtimeRr(String hex) { // between the wearing flag and live HRV/breathing compute. Four slots // exist, so four is the ceiling. maxRr = 4; - } else if (rec == 10 && - (pkt == PacketType.realtimeRawData || b.length >= kR10MinLength)) { + } else if (rec == 10 && _isR10Frame(pkt, b.length)) { tsOff = 7; cntOff = 18; // R10 declares its count at [18] and carries the values from [19], inside @@ -493,8 +505,7 @@ DecodedSample? decodeRecord(String hex) { // the same byte offset, fabricated IMU/motion) instead of falling through // to `return null` below and getting archived like every other // unrecognized version. - if (recType == 10 && - (pktType == PacketType.realtimeRawData || b.length >= kR10MinLength)) { + if (recType == 10 && _isR10Frame(pktType, b.length)) { final ts = view.getUint32(7, Endian.little); final hr = b[17]; // Null motion = the frame carried no readable IMU window; propagate that diff --git a/test/decode_guards_test.dart b/test/decode_guards_test.dart index b0e9cd9..7f19804 100644 --- a/test/decode_guards_test.dart +++ b/test/decode_guards_test.dart @@ -403,6 +403,61 @@ void main() { }); }); + group('R10 decoders only accept 0x2B / 0x2F frames', () { + // A long non-data frame whose b[1] (the SEQUENCE byte on control frames) + // happens to be 10. Console logs run up to 2 KB, so length alone does not + // tell it apart from an archived R10. + String controlFrameWithSeq10(int pkt) { + final b = Uint8List(700); + for (int i = 2; i < b.length; i++) { + b[i] = 0x61 + (i % 26); // ASCII log text + } + b[0] = pkt; + b[1] = 10; + b[17] = 0x65; // 'e' = 101 at the R10 hr offset + return _hex(b); + } + + // Binary command response with seq 10 and a plausible R-R layout at + // [18..], which ASCII text cannot satisfy. + String responseWithRrLayout() { + final b = Uint8List(700); + final v = ByteData.sublistView(b); + b[0] = PacketType.commandResponse; + b[1] = 10; + v.setUint32(7, 1775395266, Endian.little); + b[17] = 60; + b[18] = 2; + v.setInt16(19, 900, Endian.little); + v.setInt16(21, 950, Endian.little); + return _hex(b); + } + + for (final pkt in [PacketType.consoleLogs, PacketType.commandResponse]) { + test('0x${pkt.toRadixString(16)} seq 10 is not an R10 sample', () { + expect(decodeRecord(controlFrameWithSeq10(pkt)), isNull); + expect(frameAccel(controlFrameWithSeq10(pkt)), isNull); + }); + } + + test('realtimeRr ignores a command response with seq 10', () { + expect(realtimeRr(responseWithRrLayout()), isNull); + }); + + test('the same bytes framed as 0x2B / 0x2F still decode as R10', () { + for (final pkt in [ + PacketType.realtimeRawData, + PacketType.historicalData, + ]) { + final hex = '${pkt.toRadixString(16).padLeft(2, '0')}' + '${responseWithRrLayout().substring(2)}'; + expect(decodeRecord(hex)?.recType, 10); + expect(frameAccel(hex), isNotNull); + expect(realtimeRr(hex), isNotNull); + } + }); + }); + // ── 7. Battery percentages are bounded to 0..100 ───────────────────────── group('battery level is bounded', () { test('GET_BATTERY_LEVEL with 0xffff emits nothing, not 6553.5%', () { From f94859776b2f101171049ee4f917957d36eec6a1 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 08:20:40 +0530 Subject: [PATCH 02/33] parseR10Lite now refuses short historical records with layout byte 10 parseR10Lite only checked for 18 bytes, so a short 0x2F record whose layout byte happened to be 10 had unrelated bytes read as HR and R-R. every other R10 path already refuses that (0x2B at any length, or 0x2F of at least kR10MinLength), but the historical ingest calls parseR10Lite directly, so such a record would commit as a decoded sample, skip the undecodable archive, and get acked for trim. the gate now lives in parseR10Lite itself so every caller gets the same rule. --- lib/src/control.dart | 11 ++++++++ test/whoop_protocol_update_test.dart | 40 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/lib/src/control.dart b/lib/src/control.dart index ae2aff7..d6fe41c 100644 --- a/lib/src/control.dart +++ b/lib/src/control.dart @@ -165,6 +165,17 @@ class R10Lite { R10Lite? parseR10Lite(Uint8List inner) { if (inner.length < 18) return null; + // Same gate as every other R10 path: a live raw (0x2B) frame at any length, + // or a historical (0x2F) frame of at least [kR10MinLength]. 10 is not a + // known historical layout version, so a short 0x2F record whose layout byte + // happens to be 10 would otherwise have unrelated bytes read as HR@17 and + // R-R@19 — and on the historical ingest path that commits as decoded, skips + // the undecodable archive, and gets acked for trim. + final pkt = inner[0]; + if (pkt != PacketType.realtimeRawData && + !(pkt == PacketType.historicalData && inner.length >= kR10MinLength)) { + return null; + } return R10Lite(u32(inner, 7), inner[17], u32(inner, 3), rrIntervalsMs: _r10Rr(inner)); } diff --git a/test/whoop_protocol_update_test.dart b/test/whoop_protocol_update_test.dart index 4ff6174..580858d 100644 --- a/test/whoop_protocol_update_test.dart +++ b/test/whoop_protocol_update_test.dart @@ -1,6 +1,7 @@ import 'dart:typed_data'; import 'package:openstrap_protocol/openstrap_protocol.dart'; +import 'package:openstrap_protocol/src/live.dart' show kR10MinLength; import 'package:test/test.dart'; void main() { @@ -690,4 +691,43 @@ void main() { expect(decoded.fields['wearing'], isFalse); }); }); + + group('parseR10Lite applies the R10 packet/length gate', () { + Uint8List r10Body(int pkt, int len) { + final b = Uint8List(len); + final bd = b.buffer.asByteData(); + b[0] = pkt; + b[1] = Record.r10; + bd.setUint32(3, 42, Endian.little); // counter + bd.setUint32(7, 1790000000, Endian.little); // ts + b[17] = 0x48; // hr 72 + b[18] = 2; // rr count + bd.setInt16(19, 0x0320, Endian.little); + bd.setInt16(21, 0x0330, Endian.little); + return b; + } + + test('a short historical (0x2F) record with layout byte 10 is refused', () { + expect(parseR10Lite(r10Body(0x2F, 96)), isNull); + expect(parseR10Lite(r10Body(0x2F, kR10MinLength - 1)), isNull); + }); + + test('a full-length historical R10 still decodes', () { + final r = parseR10Lite(r10Body(0x2F, kR10MinLength))!; + expect(r.hr, 72); + expect(r.tsEpoch, 1790000000); + expect(r.rrIntervalsMs, [800, 816]); + }); + + test('a live raw (0x2B) R10 decodes at any length', () { + final r = parseR10Lite(r10Body(0x2B, 96))!; + expect(r.hr, 72); + expect(r.rrIntervalsMs, [800, 816]); + }); + + test('other packet types whose seq byte is 10 are refused', () { + expect(parseR10Lite(r10Body(0x32, 2000)), isNull); + expect(parseR10Lite(r10Body(0x24, 96)), isNull); + }); + }); } From 747e6eab80bab91698d31a07e1cfa3fb4aefa438 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 08:22:32 +0530 Subject: [PATCH 03/33] reassembler no longer holds a history_end behind a truncated frame when a notification inside the last data frame of a burst went missing, the reassembler kept waiting for the rest of that frame and pulled the following history_end marker into it. the band waits for the ack at that point, so the offload stalled until the band re-offered the marker or a watchdog fired, and the old marker then came out in the middle of the next exchange. while a frame is incomplete, the reassembler now checks whether a complete frame passing both crcs already sits inside the pending bytes. if one does, the short frame is given up (emitted as a crc failure so the loss still counts) and parsing resumes at the intact frame. --- lib/src/framing.dart | 36 ++++++++++++++++++++++++++++++++- test/framing_test.dart | 46 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/lib/src/framing.dart b/lib/src/framing.dart index e4cc58b..d7fc26d 100644 --- a/lib/src/framing.dart +++ b/lib/src/framing.dart @@ -176,7 +176,23 @@ class FrameReassembler { if (!resync()) break; continue; } - if (_buf.length < total) break; // wait for the rest of this frame + if (_buf.length < total) { + // A lost notification leaves this frame short, and the band sends + // HISTORY_END and then waits for the ACK — so no further bytes come + // to complete it, and the END sits inside the pending span until a + // watchdog aborts the burst. A complete frame passing both CRCs + // inside the span means this one was truncated: give it up and + // resume there. Emitted as a CRC failure so the loss is still + // counted. A payload that happened to contain a whole valid frame + // would need both an 8-bit and a 32-bit CRC to match by chance. + final next = _completeFrameWithin(headerLen); + if (next < 0) break; // wait for the rest of this frame + out.add(Frame( + Uint8List.fromList(_buf.sublist(headerLen, next)), true, false)); + _resyncs++; + _buf.removeRange(0, next); + continue; + } final frame = parseFrame(Uint8List.fromList(_buf.sublist(0, total)), profile: profile); if (frame != null) out.add(frame); @@ -206,6 +222,24 @@ class FrameReassembler { return out; } + /// Offset of the first SOF after index 0 that starts a frame which is + /// fully buffered and passes both CRCs, or -1. + int _completeFrameWithin(int headerLen) { + for (int k = 1; k + headerLen + 4 <= _buf.length; k++) { + if (_buf[k] != sof) continue; + final header = _buf.sublist(k, k + headerLen); + final declared = profile.declaredLen(header); + if (declared < 4 || headerLen + declared > 4096) continue; + if (!profile.headerCrcValid(header)) continue; + final end = k + headerLen + declared; + if (end > _buf.length) continue; + final f = parseFrame(Uint8List.fromList(_buf.sublist(k, end)), + profile: profile); + if (f != null && f.valid) return k; + } + return -1; + } + void reset() { _buf.clear(); _resyncs = 0; diff --git a/test/framing_test.dart b/test/framing_test.dart index ea7d1f4..8db7788 100644 --- a/test/framing_test.dart +++ b/test/framing_test.dart @@ -177,6 +177,52 @@ void main() { expect(frames.map((f) => f.seq), contains(6)); expect(frames.every((f) => f.valid), isTrue); }); + + // The band sends HISTORY_END and then waits for the ACK, so nothing + // arrives behind it to complete a truncated frame ahead of it. + List historyEnd(int seq) { + final inner = List.filled(21, 0) + ..[0] = PacketType.metadata + ..[1] = seq + ..[2] = SyncMeta.historyEnd; + for (int i = 0; i < 8; i++) { + inner[13 + i] = 0x50 + i; + } + return buildFrame(inner); + } + + test('a lost notification does not hold the HISTORY_END behind it', () { + final data = buildFrame( + [PacketType.historicalData, 1, 0x18, ...List.filled(93, 0x33)]); + expect(data.length, 104); + final end = historyEnd(2); + + final re = FrameReassembler(); + expect(re.feed(data).single.valid, isTrue); + // Only the first notification of the next data frame arrives. + expect(re.feed(data.sublist(0, 44)), isEmpty); + final frames = re.feed(end); + + expect(frames.length, 2); + expect(frames.first.valid, isFalse, + reason: 'the truncated frame must still count as a lost frame'); + expect(frames.last.valid, isTrue); + expect(frames.last.packetType, PacketType.metadata); + expect(parseMetadata(frames.last.inner)!.token, + [0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57]); + expect(re.feed(historyEnd(3)).single.seq, 3, + reason: 'nothing stale is left in the buffer'); + }); + + test('a partial frame still waits when nothing complete sits inside it', () { + final data = buildFrame( + [PacketType.historicalData, 1, 0x18, ...List.filled(93, 0xAA)]); + final re = FrameReassembler(); + expect(re.feed(data.sublist(0, 60)), isEmpty); + final frames = re.feed(data.sublist(60)); + expect(frames.single.valid, isTrue); + expect(re.resyncs, 0); + }); }); group('dangerousCmds (enforced never-send set)', () { From e69502f9020bde9eb739c23c2e6f8ce978a7e093 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 08:24:45 +0530 Subject: [PATCH 04/33] hello battery reads past the field when the band is below 1% parseHello scanned u16 offsets 3..9 for the first value in 10..1000. a band reporting 0.0-0.9% failed that test at [3], so the scan walked the u32's zero high bytes and took the counter at [8..11] as the battery, turning a nearly flat band into something like 25.6% or 30.0%. when the body carries the 0x04 layout marker I now read the u32 at [3] directly (0..1000, so sub-1% counts) and leave the battery null if it's out of range instead of scanning into the counter. bodies without the marker still use the scan. --- lib/src/control.dart | 17 +++++++++++++++- test/decode_guards_test.dart | 38 ++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/lib/src/control.dart b/lib/src/control.dart index d6fe41c..fd566bd 100644 --- a/lib/src/control.dart +++ b/lib/src/control.dart @@ -519,7 +519,22 @@ HelloInfo parseHello(Uint8List payload) { // right before the battery u32 at [3]. Starting at 2 read 0x04 | lo << 8, // which for a battery low byte of 1..3 lands in range (26.0/51.6/77.2%) and // stopped the scan before the real field. - for (int off = 3; off < 10; off++) { + // + // When the 0x04 marker is there the layout is known, so the u32 at [3] is + // read directly and 0..9 (0.0-0.9%) is a real reading. The scan rejects + // anything below 10, walks over the u32's zero high bytes and lands on the + // next header field (payload[8..11] is a changing counter), so a band that + // is nearly flat could report 25.6% or 30.0%. A marked body whose u32 is + // out of range leaves batteryPct null rather than falling back to a scan + // over those same counter bytes. + if (payload[2] == 0x04) { + final v = u32(payload, 3); + if (v <= 1000) { + final pct = _round(v / 10.0, 1); + if (_validBatteryPct(pct)) info.batteryPct = pct; + } + } + for (int off = 3; payload[2] != 0x04 && off < 10; off++) { if (off + 2 <= payload.length) { final v = u16(payload, off); if (v >= 10 && v <= 1000) { diff --git a/test/decode_guards_test.dart b/test/decode_guards_test.dart index 7f19804..23a5345 100644 --- a/test/decode_guards_test.dart +++ b/test/decode_guards_test.dart @@ -514,6 +514,44 @@ void main() { expect(h.batteryPct!, inInclusiveRange(0.0, 100.0)); expect(h.batteryPct, closeTo(89.9, 1e-9)); }); + + test('parseHello reads a sub-1% battery instead of the next header field', + () { + // Below 1.0% the u16 at [3] is 0..9. The old scan rejected it, walked + // the u32's zero high bytes and took the counter at [8..11] instead. + const body = + 'a001048303000000b196e201b0020000344332323438303932003865323738326237' + '346634303238346333663437363138623062613234373663356431366363303533' + '313862373532316431353635650600000002000000100000002900000011000000'; + final cases = , double>{ + [5, 0x01, 0x96]: 0.5, // counter byte 1 used to read as 25.6% + [0, 0x2c, 0x01]: 0.0, // 0x012c at [8] used to read as 30.0% + [9, 0x00, 0x02]: 0.9, // [9]=0, [10]=2 used to read as 51.2% + }; + cases.forEach((c, want) { + final b = hexToBytes(body); + b[3] = c[0]; + b[4] = 0; + b[8] = c[1]; + b[9] = c[2]; + b[10] = 0x02; + expect(parseHello(b).batteryPct, closeTo(want, 1e-9), + reason: 'raw ${c[0]}'); + }); + }); + + test('parseHello leaves an out-of-range marked battery null', () { + // With the 0x04 marker present an impossible u32 at [3] is not + // rescued by scanning into the counter bytes that follow it. + const body = + 'a001048303000000b196e201b0020000344332323438303932003865323738326237'; + final b = hexToBytes(body); + b[3] = 0xed; // 0x03ed = 1005 → 100.5% + b[4] = 0x03; + b[8] = 0x2c; + b[9] = 0x01; + expect(parseHello(b).batteryPct, isNull); + }); }); // ── 8. cmdBuzz rejects an out-of-range pattern ─────────────────────────── From 6568ddfd7d597ab8986209b0d16f034c34ddca57 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 08:26:08 +0530 Subject: [PATCH 05/33] huami clock: send sunday as 7, not 0 the mi band 2/3 clock write put day of week through weekday % 7, so every sunday went out as 0, which the current time characteristic defines as unknown. it now sends 1 = monday through 7 = sunday, same as the other current time encoder, and the test that had pinned sunday to 0 expects 7. --- lib/src/huami_legacy.dart | 5 +++-- test/huami_legacy_test.dart | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/src/huami_legacy.dart b/lib/src/huami_legacy.dart index 336540b..925ca0c 100644 --- a/lib/src/huami_legacy.dart +++ b/lib/src/huami_legacy.dart @@ -3,7 +3,8 @@ // band's AES auth (which lives with the edge adapter). // // CLOCK — Current Time (0x2A2B), 11 bytes: year u16 LE, month, day, hour, -// minute, second, day of week (0 = Sunday … 6 = Saturday), milliseconds / 4, +// minute, second, day of week (1 = Monday … 7 = Sunday; 0 means unknown, so +// Sunday must not wrap to 0), milliseconds / 4, // adjust reason (8 while daylight saving time is in effect, else 0), then the // UTC offset (DST included) as a signed byte in 15-minute units. // @@ -48,7 +49,7 @@ List huamiTimeValue(DateTime local) => [ local.hour, local.minute, local.second, - local.weekday % 7, + local.weekday, local.millisecond ~/ 4, _inDst(local) ? 8 : 0, _tzQuarters(local), diff --git a/test/huami_legacy_test.dart b/test/huami_legacy_test.dart index 5e9aa7e..6bb342b 100644 --- a/test/huami_legacy_test.dart +++ b/test/huami_legacy_test.dart @@ -2,13 +2,14 @@ import 'package:openstrap_protocol/openstrap_protocol.dart'; import 'package:test/test.dart'; void main() { - test('clock value: Sunday is 0, ms/4, then the UTC offset in quarters', () { + test('clock value: Sunday is 7, ms/4, then the UTC offset in quarters', () { final t = DateTime(2026, 10, 4, 7, 5, 9, 500); // a Sunday final v = huamiTimeValue(t); - expect(v.sublist(0, 9), [0xea, 0x07, 10, 4, 7, 5, 9, 0, 125]); + expect(v.sublist(0, 9), [0xea, 0x07, 10, 4, 7, 5, 9, 7, 125]); expect(v[9], anyOf(0, 8)); expect(v.last, (t.timeZoneOffset.inMinutes ~/ 15) & 0xff); expect(huamiTimeValue(DateTime(2026, 10, 10))[7], 6, reason: 'Saturday'); + expect(huamiTimeValue(DateTime(2026, 10, 5))[7], 1, reason: 'Monday'); }); test('fetch start: type 1, minute-resolution since, 0, tz', () { From f4ae06ae9183089f25f6a84bb22b11533c66af62 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 08:27:07 +0530 Subject: [PATCH 06/33] frame oura extended events correctly in parseOuraFrame parseOuraFrame read byte 1 of every frame as the payload length, but an extended (0xfd) event carries its crc8 there and its length in byte 3. When the crc was larger than the rest of the buffer the frame came back null and the notification never reached raw_archive; when it was smaller the payload was a crc-sized slice. It now frames 0xfd the same way parseOuraFrames does. --- lib/src/oura.dart | 10 ++++++++++ test/oura_test.dart | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/lib/src/oura.dart b/lib/src/oura.dart index e415a3f..e16bdc2 100644 --- a/lib/src/oura.dart +++ b/lib/src/oura.dart @@ -60,8 +60,18 @@ class OuraFrame { /// length LONGER than the buffer is a truncated frame, and this returns null /// rather than handing back a short payload that every downstream length check /// would then treat as a real, complete record. +/// +/// [kOuraEvtExtended] is framed as in [parseOuraFrames]: byte 1 is its CRC, +/// not a length. Reading it as one dropped most extended events before they +/// could be archived and sliced the rest to a CRC-dependent length. OuraFrame? parseOuraFrame(List value) { if (value.length < 2) return null; + if (value[0] == kOuraEvtExtended) { + if (value.length < 4) return null; + final end = 4 + value[3]; + if (value.length < end) return null; + return OuraFrame(value[0], Uint8List.fromList(value.sublist(1, end))); + } final len = value[1]; if (value.length - 2 < len) return null; return OuraFrame(value[0], Uint8List.fromList(value.sublist(2, 2 + len))); diff --git a/test/oura_test.dart b/test/oura_test.dart index 6676559..3acd778 100644 --- a/test/oura_test.dart +++ b/test/oura_test.dart @@ -361,6 +361,24 @@ void main() { expect(parseOuraExtendedEvent(parseOuraFrames(bytes).single), isNull); }); + test('parseOuraFrame frames an extended event by its own length byte', () { + // Byte 1 is a CRC. 0x5f read as a length would exceed the buffer and + // the notification would be dropped before reaching the archive. + final bytes = [0xfd, 0x5f, 0x50, 8, 1, 2, 3, 4, 0, 0, 0, 0]; + final f = parseOuraFrame(bytes)!; + expect(f.tag, kOuraEvtExtended); + expect(f.payload, bytes.sublist(1)); + expect(f.payload, parseOuraFrames(bytes).single.payload); + // Trailing bytes are the next bundled frame, not this one's payload. + expect(parseOuraFrame([...bytes, 0x61, 0x00])!.payload, bytes.sublist(1)); + // A small CRC no longer decides the slice length either. + final small = [0xfd, 0x02, 0x50, 8, 1, 2, 3, 4, 0, 0, 0, 0]; + expect(parseOuraFrame(small)!.payload, small.sublist(1)); + // Truncated header or body is null, never a short payload. + expect(parseOuraFrame(const [0xfd, 0x00, 0x50]), isNull); + expect(parseOuraFrame(bytes.sublist(0, 11)), isNull); + }); + test('the ring rejecting a command as unsupported is told apart', () { expect(ouraIsUnsupported(parseOuraFrame(_hex('300110'))!, 0x10), isTrue); expect(ouraIsUnsupported(parseOuraFrame(_hex('300110'))!, 0x2f), isFalse); From b8e8cc63afb8d2d0c4040ccd36d9417add385952 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 08:28:32 +0530 Subject: [PATCH 07/33] stop the garmin protobuf walker throwing on a bad length varint an overlong or 10-byte length varint decoded negative, slipped past the bounds check and threw a RangeError out of sublistView. that escaped garminParseBatteryResponseProto, which the gfdi frame handler calls inline, and wedged the whole garmin session. the varint reader now gives up after 10 bytes and a negative or oversized length ends the walk like any other malformed field. --- lib/src/garmin.dart | 10 +++++++--- test/garmin_test.dart | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/src/garmin.dart b/lib/src/garmin.dart index 6ee4c4a..a9b4187 100644 --- a/lib/src/garmin.dart +++ b/lib/src/garmin.dart @@ -689,13 +689,15 @@ Iterable<_ProtoField> _protoFields(Uint8List data) sync* { var i = 0; int readVarint() { var result = 0, shift = 0; - while (i < data.length) { + // A varint is at most 10 bytes; past that the shift wraps and the value + // is garbage, so treat it as malformed rather than keep OR-ing. + while (i < data.length && shift < 64) { final b = data[i++]; result |= (b & 0x7f) << shift; if (b & 0x80 == 0) return result; shift += 7; } - throw const FormatException('truncated varint'); + throw const FormatException('truncated or overlong varint'); } try { @@ -711,7 +713,9 @@ Iterable<_ProtoField> _protoFields(Uint8List data) sync* { i += 8; case 2: final len = readVarint(); - if (i + len > data.length) return; + // A 10-byte varint can come back negative, which would slip past + // an `i + len > length` check and throw out of sublistView. + if (len < 0 || len > data.length - i) return; yield _ProtoField(number, wireType, bytes: Uint8List.sublistView(data, i, i + len)); i += len; diff --git a/test/garmin_test.dart b/test/garmin_test.dart index eca60ff..eb4e80f 100644 --- a/test/garmin_test.dart +++ b/test/garmin_test.dart @@ -313,6 +313,35 @@ void main() { test('a response with no battery field parses to null', () { expect(garminParseBatteryResponseProto(Uint8List.fromList([])), isNull); }); + + test('a negative length varint stops the walk instead of throwing', () { + final ff9 = List.filled(9, 0xff); + // ff×9 01 decodes to -1 as a 64-bit varint. + final top = [0x42, ...ff9, 0x01, 0, 0, 0]; + expect(garminParseBatteryResponseProto(Uint8List.fromList(top)), isNull); + // Same garbage one level down, inside the service sub-message. + final nested = [0x42, 0x0d, 0x1a, 0x0b, 0x12, ...ff9, 0x01]; + expect( + garminParseBatteryResponseProto(Uint8List.fromList(nested)), isNull); + }); + + test('an overlong varint stops the walk instead of throwing', () { + final overlong = [0x42, ...List.filled(12, 0xff), 0x01, 0, 0]; + expect(garminParseBatteryResponseProto(Uint8List.fromList(overlong)), + isNull); + }); + + test('random bytes never throw', () { + // Fixed seed so a failure reproduces. + var s = 0x2545f491; + int next() => s = (s * 1103515245 + 12345) & 0x7fffffff; + for (var n = 0; n < 20000; n++) { + final len = next() % 24; + final bytes = Uint8List.fromList( + List.generate(len, (i) => i == 0 ? 0x42 : (next() >> 8) & 0xff)); + expect(() => garminParseBatteryResponseProto(bytes), returnsNormally); + } + }); }); group('protobuf request/response GFDI envelope', () { From b7639f0c72b0aa88684ed10e9ae75fbb5089aada Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 08:30:01 +0530 Subject: [PATCH 08/33] colmi big-data reassembler restarts on a new reply instead of splicing it in once a reply was pending, every later notification was appended until the declared length was reached, even one that clearly started the next reply. a sleep reply that lost its last notification got filled with the nap reply that followed it, so the nap's header and records were read as stage/minute pairs in the last night and the nap itself was lost. a chunk starting with 0xbc and a known type now drops the truncated reply and starts the new one. --- lib/src/colmi.dart | 15 +++++++++++++++ test/colmi_test.dart | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/lib/src/colmi.dart b/lib/src/colmi.dart index 915842f..2567a38 100644 --- a/lib/src/colmi.dart +++ b/lib/src/colmi.dart @@ -143,11 +143,26 @@ List colmiTemperatureRequest([int mode = 6]) => /// complete reply (header included) once `len + 6` bytes have arrived, else /// null. A notification that does not start with 0xbc while nothing is /// pending is not a reply this decoder understands and is dropped. +/// +/// A notification that starts a new reply (0xbc plus a known type) while one +/// is still pending means the pending reply lost a notification. It is +/// dropped and the new reply started instead: appending would fill the gap +/// with the next reply's header and records (a nap reply read as the last +/// night's stage/minute pairs) and lose the next reply entirely. class ColmiBigDataReassembler { List? _pending; int _want = 0; + static bool _startsReply(List chunk) => + chunk.length >= kColmiBigHeaderLength && + chunk[0] == kColmiCmdBigData && + (chunk[1] == kColmiBigSleep || + chunk[1] == kColmiBigNap || + chunk[1] == kColmiBigSpo2 || + chunk[1] == kColmiBigTemperature); + List? add(List chunk) { + if (_pending != null && _startsReply(chunk)) _pending = null; if (_pending == null) { if (chunk.length < kColmiBigHeaderLength || chunk[0] != kColmiCmdBigData) { diff --git a/test/colmi_test.dart b/test/colmi_test.dart index e41b56c..c11926f 100644 --- a/test/colmi_test.dart +++ b/test/colmi_test.dart @@ -79,6 +79,28 @@ void main() { expect(colmiBigDataCrcOk(whole), isTrue); }); + test('a new reply while one is pending drops the truncated one', () { + // Sleep reply whose last notification never arrived, then the nap + // reply the ring sends right after it. + final sleep = big(kColmiBigSleep, List.generate(60, (i) => 2 + i % 4)); + final nap = big(kColmiBigNap, [1, 2, 3, 4, 5]); + final r = ColmiBigDataReassembler(); + expect(r.add(sleep.sublist(0, 30)), isNull); + expect(r.add(sleep.sublist(30, 50)), isNull); + expect(r.add(nap), nap); + expect(r.pending, isFalse); + expect(colmiBigDataCrcOk(nap), isTrue); + }); + + test('a new reply split over notifications after a truncated one', () { + final spo2 = big(kColmiBigSpo2, List.generate(40, (i) => 90 + i % 8)); + final temp = big(kColmiBigTemperature, List.generate(30, (i) => i)); + final r = ColmiBigDataReassembler(); + expect(r.add(spo2.sublist(0, 20)), isNull); + expect(r.add(temp.sublist(0, 20)), isNull); + expect(r.add(temp.sublist(20)), temp); + }); + test('a non-0xbc chunk with nothing pending is dropped', () { expect(ColmiBigDataReassembler().add([0x01, 2, 3, 4, 5, 6, 7]), isNull); }); From 3223b219ee173b24f3e72b14148e5149c826051a Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 12:18:49 +0530 Subject: [PATCH 09/33] garmin fit: a timestamp that isn't a plain integer now fails as a FormatException a timestamp or timestamp_16 field declared as an array, a string or a float decoded to a List, String or NaN double, and the unchecked cast threw a TypeError or UnsupportedError instead. the edge only banks a file raw on FormatException, so one such file stopped the garmin download queue at that file on every sync. (cherry picked from commit 2d667645c845ebeaf509a3b933b1c3e78f44cf7d) --- lib/src/garmin_fit.dart | 16 +++++++++++++--- test/garmin_fit_test.dart | 17 +++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/src/garmin_fit.dart b/lib/src/garmin_fit.dart index 78d44c3..f1b57cd 100644 --- a/lib/src/garmin_fit.dart +++ b/lib/src/garmin_fit.dart @@ -182,6 +182,16 @@ List parseFit(List bytes) { return out; } +// A time field declared as an array, a string or a float decodes to a List, +// String or double (NaN included). The caller only banks the file raw on a +// FormatException, so any other error type here would stall the download +// queue on this file for good. +int? _timeField(Map fields, int num) { + final v = fields[num]; + if (v == null || v is int) return v as int?; + throw FormatException('FIT: field $num is not an integer timestamp'); +} + (FitMessage, int, int?) _data(ByteData d, int at, _Def def, int? lastTs) { final fields = {}; var i = at; @@ -210,12 +220,12 @@ List parseFit(List bytes) { i += f.size; } i += def.devBytes; - int? ts = (fields[253] as num?)?.toInt(); + int? ts = _timeField(fields, 253); // timestamp_16 (monitoring field 26): low 16 bits relative to the last // full timestamp. - final ts16 = def.global == kFitMsgMonitoring ? fields[26] as num? : null; + final ts16 = def.global == kFitMsgMonitoring ? _timeField(fields, 26) : null; if (ts == null && ts16 != null && lastTs != null) { - ts = lastTs + ((ts16.toInt() - (lastTs & 0xffff)) & 0xffff); + ts = lastTs + ((ts16 - (lastTs & 0xffff)) & 0xffff); } final unix = (ts ?? lastTs) == null ? null : (ts ?? lastTs)! + kFitEpochOffset; return (FitMessage(def.global, fields, unix), i, ts); diff --git a/test/garmin_fit_test.dart b/test/garmin_fit_test.dart index e802628..4e3aa68 100644 --- a/test/garmin_fit_test.dart +++ b/test/garmin_fit_test.dart @@ -112,4 +112,21 @@ void main() { final bad = Uint8List.fromList(good)..[4] = good[4] - 2; expect(() => parseFit(bad), throwsFormatException); }); + + test('a timestamp that is not a single integer is a FormatException', () { + List fit(int global, int field, int size, int base, List bytes) => + (FitWriter() + ..define(0, global, [(field, size, base)]) + ..data(0, bytes)) + .build(); + // 253 declared as uint32[2], float32 NaN, and a string; 26 as uint16[2]. + for (final f in [ + fit(kFitMsgMonitoring, 253, 8, 0x86, [...u32(fitTs(t0)), ...u32(1)]), + fit(kFitMsgMonitoring, 253, 4, 0x88, [0x00, 0x00, 0xc0, 0x7f]), + fit(kFitMsgMonitoring, 253, 4, 7, 'abc\x00'.codeUnits), + fit(kFitMsgMonitoring, 26, 4, 0x84, [...u16(1), ...u16(2)]), + ]) { + expect(() => parseFit(f), throwsFormatException); + } + }); } From fbe95e79fb1894ba77088e6cb4ac5d3cef05cd46 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 12:21:16 +0530 Subject: [PATCH 10/33] drop the deprecated gen5 compat getters nothing calls anymore layoutMarker, opticalBaseline, opticalAmp, spo2Candidate, onWristRaw and wakeQualityRaw were kept only so old callers compiled, and none are left. several were worse than stale names: spo2Candidate surfaced a plausible 95-99 only during band-declared sleep with no validity signal, onWristRaw read a flags bit as wear state, and opticalAmp glued two independent i8s into a u16. the underlying fields stay, so no decoded value changes; the not-wear-state caveat now lives on primaryFlagsBit8Raw itself. (cherry picked from commit 9b3b71cc5eb17a60abd3e72e4833f319ccf376a8) --- lib/src/gen5_records.dart | 56 +++++-------------------------- test/gen5_historical_test.dart | 9 ++--- test/gen5_record_fields_test.dart | 9 ++--- 3 files changed, 13 insertions(+), 61 deletions(-) diff --git a/lib/src/gen5_records.dart b/lib/src/gen5_records.dart index fd6edd0..00b88a7 100644 --- a/lib/src/gen5_records.dart +++ b/lib/src/gen5_records.dart @@ -96,9 +96,6 @@ class Gen5HistoricalHeader { required this.tsSubsec, }); - @Deprecated('inner[2] is a flags byte, not an opaque marker — use flags') - int get layoutMarker => flags; - /// Optical sample rate this record was captured at, in Hz — bit 7 of /// [flags]. int get ppgSampleRateHz => (flags & 0x80) != 0 ? 25 : 50; @@ -145,9 +142,6 @@ abstract class Gen5HistoricalRecord { required this.tsSubsec, }); - @Deprecated('inner[2] is a flags byte, not an opaque marker — use flags') - int get layoutMarker => flags; - /// Optical sample rate this record was captured at, in Hz — bit 7 of /// [flags] (set = 25 Hz, clear = 50 Hz). int get ppgSampleRateHz => (flags & 0x80) != 0 ? 25 : 50; @@ -407,15 +401,6 @@ class Gen5HistorySample extends Gen5HistoricalRecord { bool get psnrBAvailable => psnrB != -128; bool get psnrAAvailable => psnrA != -128; - /// COMPAT (deprecated): the old big-endian-u16 reads of these byte pairs. - /// Both conflated two independent fields; kept only so existing callers still - /// compile. `opticalBaseline` was `(pdMeanB<<8)|pdMeanA` and `opticalAmp` the - /// raw `(psnrB<<8)|psnrA` — the latter is the `0x8080` both-unavailable case. - @Deprecated('two independent u8 PD means, not a u16 — use pdMeanB/pdMeanA') - int get opticalBaseline => (pdMeanB << 8) | pdMeanA; - @Deprecated('two independent i8 pSNR values, not a u16 — use psnrB/psnrA') - int get opticalAmp => ((psnrB & 0xFF) << 8) | (psnrA & 0xFF); - /// @ inner[105:109] f32 LE (frame-abs 113). A per-second signal-quality /// metric the band's own optical processing computes: the log-variance of /// the signal it measured this second. Range ~-5.3..0 on the reference @@ -477,44 +462,21 @@ class Gen5HistorySample extends Gen5HistoricalRecord { /// heart-rate bytes, not a substitute HR — read [heartRate] for the value. int? get trustedHeartRateAlt => hrRrValidThisSecond ? heartRateAlt : null; - /// **Do not use.** The byte is zero in 99% of records and nonzero *only* - /// while [sleepState] is [Gen5SleepState.sleep], so this getter surfaces a - /// plausible-looking 95-99 on 0.6% of records and null on the rest — with no - /// validity signal to say which are real. See [spo2CandidateRaw]. - @Deprecated( - 'unsafe to surface: zero in 99% of records, nonzero only during ' - 'band-declared sleep, and with no validity flag. Read spo2CandidateRaw ' - 'if you need the byte.', - ) - int? get spo2Candidate => (spo2CandidateRaw >= 70 && spo2CandidateRaw <= 100) - ? spo2CandidateRaw - : null; - /// True when BOTH per-detector pSNR values read the -128 unavailable - /// sentinel — i.e. the old `opticalAmp == 0x8080` case, which was never a - /// real amplitude, just both detectors reporting unavailable at once. + /// sentinel — the `0x8080` that reading the pair as one big-endian u16 used + /// to report, which was never a real amplitude, just both detectors reporting unavailable at once. bool get isOpticalAmpSentinel => !psnrBAvailable && !psnrAAvailable; - /// bits 0-1 of [sleepStateByte] — the primary-flags bit-8 snapshot. + /// bits 0-1 of [sleepStateByte] — the primary-flags bit-8 snapshot. NOT a + /// wear indicator: nothing supports reading these bits as on-wrist. Wear + /// state comes from the HELLO body, the wrist on/off events, or the band + /// no longer emitting type-40 off-wrist. int get primaryFlagsBit8Raw => sleepStateByte & 0x03; - /// bits 2-3 of [sleepStateByte] — the passive strap-fit classifier state. + /// bits 2-3 of [sleepStateByte] — the passive strap-fit classifier state, + /// not a wake-quality score. int get strapFitStateRaw => (sleepStateByte >> 2) & 0x03; - /// **Deprecated: not a wear indicator.** Bits 0-1 are the primary-flags - /// bit-8 snapshot; nothing supports reading - /// them as on-wrist. Wear state comes from the HELLO body, the wrist - /// on/off events, or the fact that the band stops emitting type-40 off-wrist. - @Deprecated('bits 0-1 are the primary-flags bit-8 snapshot, not wear state — ' - 'use primaryFlagsBit8Raw (and do not treat it as on-wrist)') - int get onWristRaw => primaryFlagsBit8Raw; - - /// **Deprecated: misnamed.** These bits are the passive strap-fit classifier - /// state, not a wake-quality score. - @Deprecated('bits 2-3 are the passive strap-fit classifier state — ' - 'use strapFitStateRaw') - int get wakeQualityRaw => strapFitStateRaw; - /// bits 4-5 of [sleepStateByte], raw. Prefer [sleepState]. int get sleepStateRawNibble => (sleepStateByte >> 4) & 0x03; @@ -1290,7 +1252,7 @@ class Gen5PpgWaveform extends Gen5HistoricalRecord { /// record. Kept whole; [primaryFlagsBit8Raw] is the pinned two bits. /// /// NOTE the same caveat R18 carries: these bits are NOT a wear/on-wrist - /// reading — see [Gen5HistorySample.onWristRaw]'s deprecation. + /// reading — see [Gen5HistorySample.primaryFlagsBit8Raw]. final int primaryFlagsByte; /// The low two bits of [primaryFlagsByte] — the part pinned to R18. diff --git a/test/gen5_historical_test.dart b/test/gen5_historical_test.dart index 60eec2d..3602d6b 100644 --- a/test/gen5_historical_test.dart +++ b/test/gen5_historical_test.dart @@ -141,9 +141,6 @@ void main() { expect(sample.psnrBAvailable, isTrue); expect(sample.psnrAAvailable, isTrue); expect(sample.isOpticalAmpSentinel, isFalse); // not both-unavailable - // One compat assertion so the deprecated u16 view stays wired correctly. - // ignore: deprecated_member_use_from_same_package - expect(sample.opticalBaseline, 0x656F); }); test('experimental fields exposed raw, not fabricated', () { @@ -154,8 +151,6 @@ void main() { // frame-abs 82 is not SpO2 — zero in 99% of records and nonzero only // during band-declared sleep. This fixture is awake, so it reads 0. expect(sample.spo2CandidateRaw, 0); - // ignore: deprecated_member_use_from_same_package - expect(sample.spo2Candidate, isNull); }); test('band sleep state: this fixture is awake', () { @@ -205,8 +200,8 @@ void main() { final s = parseGen5Historical(inner) as Gen5HistorySample; expect(s.sleepStateRawNibble, (b >> 4) & 0x03); expect(s.sleepState, Gen5SleepState.values[(b >> 4) & 0x03]); - expect(s.onWristRaw, b & 0x03); - expect(s.wakeQualityRaw, (b >> 2) & 0x03); + expect(s.primaryFlagsBit8Raw, b & 0x03); + expect(s.strapFitStateRaw, (b >> 2) & 0x03); } }); }); diff --git a/test/gen5_record_fields_test.dart b/test/gen5_record_fields_test.dart index 690dc41..7e1b451 100644 --- a/test/gen5_record_fields_test.dart +++ b/test/gen5_record_fields_test.dart @@ -118,11 +118,6 @@ void main() { expect(s.psnrA, 0x1E); expect(s.psnrBAvailable, isTrue); expect(s.psnrAAvailable, isTrue); - // Compat: the deprecated u16 views reconstruct the old values exactly. - // ignore: deprecated_member_use_from_same_package - expect(s.opticalBaseline, 0x656F); - // ignore: deprecated_member_use_from_same_package - expect(s.opticalAmp, 0x1E1E); }); test('0x80 is the per-detector -128 unavailable sentinel, not a value', () { @@ -189,8 +184,8 @@ void main() { for (int b = 0; b <= 255; b++) { inner[73] = b; final s = parseGen5Historical(inner) as Gen5HistorySample; - expect(s.onWristRaw, b & 0x03); - expect(s.wakeQualityRaw, (b >> 2) & 0x03); + expect(s.primaryFlagsBit8Raw, b & 0x03); + expect(s.strapFitStateRaw, (b >> 2) & 0x03); expect(s.sleepStateRawNibble, (b >> 4) & 0x03); expect(s.bits67Raw, (b >> 6) & 0x03, reason: 'byte=$b'); } From e2fbda381236e6835421d518307f72cbd1b942df Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 12:19:09 +0530 Subject: [PATCH 11/33] pebble reassembler can drop a partial frame on a transport reset the pebble reassembler only ever appended bytes, so a ppogatt reset in the middle of an inner frame left the cut frame at the head of the buffer and every frame after it was read at the wrong length and endpoint, for the rest of the sync. re-sent packets were appended twice the same way. it now has reset() to throw the partial frame away, and addPacket() which takes the whole ppogatt packet, drops a repeated serial and discards the partial frame when serials skip. (cherry picked from commit 41c3ae26877fbbba7665dc734e5b53747536bc57) --- lib/src/pebble.dart | 31 ++++++++++++++++++++++++++++ test/pebble_test.dart | 47 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/lib/src/pebble.dart b/lib/src/pebble.dart index 7938e6d..8911c2b 100644 --- a/lib/src/pebble.dart +++ b/lib/src/pebble.dart @@ -74,8 +74,39 @@ List> pebblePpogattPackets(List frame, int firstSerial, } /// Reassembles inner frames from consecutive PPoGATT data payloads. +/// +/// The inner framing has no sync marker: the only boundary is the length at +/// the head of the buffer. A partial frame left behind by a transport reset, +/// a re-sent packet appended twice, or a skipped packet shifts every later +/// length and endpoint, and the rest of the session misframes (or waits +/// forever on a garbage length). [reset] and [addPacket] exist so that state +/// can be thrown away instead. class PebbleFrameReassembler { final List _buf = []; + int? _lastSerial; + + /// Drops any partial frame and forgets the serial sequence. Call it when + /// the PPoGATT transport resets. + void reset() { + _buf.clear(); + _lastSerial = null; + } + + /// Feeds one whole PPoGATT data packet (header byte included). A repeat of + /// the last serial is a re-send and is dropped. Any other serial that is + /// not the next one means packets were lost, so the partial frame is + /// discarded and this packet starts a fresh buffer. + List<(int, Uint8List)> addPacket(List packet) { + if (packet.isEmpty) return const []; + final serial = (packet[0] >> 3) & 0x1f; + final last = _lastSerial; + if (last != null) { + if (serial == last) return const []; + if (serial != (last + 1) & 0x1f) _buf.clear(); + } + _lastSerial = serial; + return add(packet.sublist(1)); + } /// Feeds one data payload; returns every complete (endpoint, payload). List<(int, Uint8List)> add(List payload) { diff --git a/test/pebble_test.dart b/test/pebble_test.dart index 58f0622..2a91293 100644 --- a/test/pebble_test.dart +++ b/test/pebble_test.dart @@ -17,6 +17,53 @@ void main() { expect(out.single.$2, List.generate(40, (i) => i)); }); + group('reassembler recovery', () { + // A 66-byte datalog data frame split into 20-byte packets, and two small + // whole frames that follow it. + final a = pebbleFrame(kPebbleEndpointDatalog, + [2, 1, ...List.filled(60, 0x5a)]); + final b = pebbleFrame(kPebbleEndpointDatalog, [2, 1, ...List.filled(30, 1)]); + final c = pebbleFrame(kPebbleEndpointDatalog, [2, 2, ...List.filled(30, 2)]); + + test('reset drops a partial frame cut by a transport reset', () { + final r = PebbleFrameReassembler(); + expect(r.addPacket(pebblePpogattPackets(a, 0).first), isEmpty); + r.reset(); + final out = [ + for (final p in [ + ...pebblePpogattPackets(b, 0), + ...pebblePpogattPackets(c, 2), + ]) + ...r.addPacket(p), + ]; + expect(out.map((f) => f.$2[1]), [1, 2]); + expect(out.map((f) => f.$2.length), [32, 32]); + }); + + test('a repeated serial is dropped, not appended twice', () { + final r = PebbleFrameReassembler(); + final ps = pebblePpogattPackets(b, 30); + final out = [ + ...r.addPacket(ps[0]), + ...r.addPacket(ps[0]), + for (final p in ps.skip(1)) ...r.addPacket(p), + ]; + expect(out.single.$2, [2, 1, ...List.filled(30, 1)]); + }); + + test('a serial gap discards the partial frame', () { + final r = PebbleFrameReassembler(); + final ps = pebblePpogattPackets(a, 0); + r.addPacket(ps[0]); + // ps[1] lost; the rest of the cut frame then a fresh frame on serial 9. + for (final p in ps.skip(2)) { + r.addPacket(p); + } + final out = [for (final p in pebblePpogattPackets(c, 9)) ...r.addPacket(p)]; + expect(out.single.$2, [2, 2, ...List.filled(30, 2)]); + }); + }); + test('phone version reply: 25 bytes, Android, LE capability mask', () { final v = pebblePhoneVersionReply(); expect(v, hasLength(25)); From 8993693d4fb5c095c9a22df4a75470a00365fc18 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 12:24:56 +0530 Subject: [PATCH 12/33] pebble reassembler only takes the next serial instead of clearing on a gap clearing the buffer on any out-of-sequence serial broke the normal go-back-n re-send: after a lost ack the watch re-sends from behind the last serial, the buffer was cleared and a mid-frame payload became its head, so every later frame misframed. addPacket now drops anything that is not the next serial without touching the buffer and returns null for it, and the re-send fills the gap in order. (cherry picked from commit 48a630e8b19fd2507f7dcab9011bf41d3a2cd300) --- lib/src/pebble.dart | 26 +++++++++++---------- test/pebble_test.dart | 54 ++++++++++++++++++++++++++----------------- 2 files changed, 47 insertions(+), 33 deletions(-) diff --git a/lib/src/pebble.dart b/lib/src/pebble.dart index 8911c2b..0afd8b5 100644 --- a/lib/src/pebble.dart +++ b/lib/src/pebble.dart @@ -79,8 +79,8 @@ List> pebblePpogattPackets(List frame, int firstSerial, /// the head of the buffer. A partial frame left behind by a transport reset, /// a re-sent packet appended twice, or a skipped packet shifts every later /// length and endpoint, and the rest of the session misframes (or waits -/// forever on a garbage length). [reset] and [addPacket] exist so that state -/// can be thrown away instead. +/// forever on a garbage length). [reset] throws a cut frame away, and +/// [addPacket] keeps re-sent and out-of-order packets out of the buffer. class PebbleFrameReassembler { final List _buf = []; int? _lastSerial; @@ -92,18 +92,20 @@ class PebbleFrameReassembler { _lastSerial = null; } - /// Feeds one whole PPoGATT data packet (header byte included). A repeat of - /// the last serial is a re-send and is dropped. Any other serial that is - /// not the next one means packets were lost, so the partial frame is - /// discarded and this packet starts a fresh buffer. - List<(int, Uint8List)> addPacket(List packet) { - if (packet.isEmpty) return const []; + /// Feeds one whole PPoGATT data packet (header byte included). Only the + /// next serial in sequence is taken; anything else (a repeat, or a packet + /// that arrived after one was lost or skipped) is dropped without touching + /// the buffer. PPoGATT is go-back-N: the watch re-sends every unacked + /// packet in order from the oldest, so the missing one comes back and the + /// frame completes. Clearing on a gap instead would make a mid-frame + /// payload the head of the buffer and misframe everything after it. + /// + /// Returns null when the packet was dropped, so the caller does not ack it. + List<(int, Uint8List)>? addPacket(List packet) { + if (packet.isEmpty) return null; final serial = (packet[0] >> 3) & 0x1f; final last = _lastSerial; - if (last != null) { - if (serial == last) return const []; - if (serial != (last + 1) & 0x1f) _buf.clear(); - } + if (last != null && serial != (last + 1) & 0x1f) return null; _lastSerial = serial; return add(packet.sublist(1)); } diff --git a/test/pebble_test.dart b/test/pebble_test.dart index 2a91293..8b9a3fb 100644 --- a/test/pebble_test.dart +++ b/test/pebble_test.dart @@ -1,3 +1,5 @@ +import 'dart:typed_data'; + import 'package:openstrap_protocol/openstrap_protocol.dart'; import 'package:test/test.dart'; @@ -25,17 +27,17 @@ void main() { final b = pebbleFrame(kPebbleEndpointDatalog, [2, 1, ...List.filled(30, 1)]); final c = pebbleFrame(kPebbleEndpointDatalog, [2, 2, ...List.filled(30, 2)]); + List<(int, Uint8List)> feed(PebbleFrameReassembler r, List> ps) => + [for (final p in ps) ...?r.addPacket(p)]; + test('reset drops a partial frame cut by a transport reset', () { final r = PebbleFrameReassembler(); expect(r.addPacket(pebblePpogattPackets(a, 0).first), isEmpty); r.reset(); - final out = [ - for (final p in [ - ...pebblePpogattPackets(b, 0), - ...pebblePpogattPackets(c, 2), - ]) - ...r.addPacket(p), - ]; + final out = feed(r, [ + ...pebblePpogattPackets(b, 0), + ...pebblePpogattPackets(c, 2), + ]); expect(out.map((f) => f.$2[1]), [1, 2]); expect(out.map((f) => f.$2.length), [32, 32]); }); @@ -43,24 +45,34 @@ void main() { test('a repeated serial is dropped, not appended twice', () { final r = PebbleFrameReassembler(); final ps = pebblePpogattPackets(b, 30); - final out = [ - ...r.addPacket(ps[0]), - ...r.addPacket(ps[0]), - for (final p in ps.skip(1)) ...r.addPacket(p), - ]; + expect(r.addPacket(ps[0]), isEmpty); + expect(r.addPacket(ps[0]), isNull); + final out = feed(r, ps.skip(1).toList()); expect(out.single.$2, [2, 1, ...List.filled(30, 1)]); }); - test('a serial gap discards the partial frame', () { + test('go-back-N re-send after lost acks keeps the frame intact', () { + final r = PebbleFrameReassembler(); + final pa = pebblePpogattPackets(a, 0); // serials 0..3 + final pc = pebblePpogattPackets(c, pa.length); + // 0,1,2 arrive; the acks for 1 and 2 are lost, so the watch re-sends + // from 1 onward, then the next frame. + final out = feed(r, [pa[0], pa[1], pa[2], pa[1], pa[2], pa[3], ...pc]); + expect(out.map((f) => f.$2.length), [62, 32]); + expect(out.first.$2, [2, 1, ...List.filled(60, 0x5a)]); + expect(out.last.$2[1], 2); + }); + + test('a skipped packet is dropped until it is re-sent in order', () { final r = PebbleFrameReassembler(); - final ps = pebblePpogattPackets(a, 0); - r.addPacket(ps[0]); - // ps[1] lost; the rest of the cut frame then a fresh frame on serial 9. - for (final p in ps.skip(2)) { - r.addPacket(p); - } - final out = [for (final p in pebblePpogattPackets(c, 9)) ...r.addPacket(p)]; - expect(out.single.$2, [2, 2, ...List.filled(30, 2)]); + final pa = pebblePpogattPackets(a, 0); + // packet 1 is skipped; 2 and 3 arrive out of order and are dropped, + // then go-back-N re-sends 1, 2, 3. + expect(r.addPacket(pa[0]), isEmpty); + expect(r.addPacket(pa[2]), isNull); + expect(r.addPacket(pa[3]), isNull); + final out = feed(r, [pa[1], pa[2], pa[3]]); + expect(out.single.$2, [2, 1, ...List.filled(60, 0x5a)]); }); }); From 779ec2a5c112b6e4902f6c4e279280731c262ee0 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 19:24:46 +0530 Subject: [PATCH 13/33] garmin fit views skip a NaN or Inf sample instead of throwing a field the profile types as an integer can still be declared float in a file, and only the all-ones pattern was treated as invalid, so NaN and Inf reached .toInt() in the typed views. that threw UnsupportedError, which the garmin adapter does not catch, so the file was never marked read and sync stalled on it every time. the views now read through an accessor that only yields finite whole numbers, and hrv/respiration require a finite value. (cherry picked from commit fdd5351d185a99a0a3b6ce7f537cec44e5d16027) --- lib/src/garmin_fit.dart | 52 ++++++++++++++++++++++++++------------- test/garmin_fit_test.dart | 25 +++++++++++++++++++ 2 files changed, 60 insertions(+), 17 deletions(-) diff --git a/lib/src/garmin_fit.dart b/lib/src/garmin_fit.dart index f1b57cd..6b7db3b 100644 --- a/lib/src/garmin_fit.dart +++ b/lib/src/garmin_fit.dart @@ -43,6 +43,24 @@ class FitMessage { final v = fields[field]; return v is num ? v : null; } + + // A field the profile types as an integer can still arrive declared as a + // float, and only the all-ones pattern counts as invalid there, so NaN and + // Inf get through. `.toInt()` on those throws UnsupportedError, which the + // caller does not catch, and the download queue would stall on that file. + // Such a sample is skipped instead. + int? int_(int field) { + final v = fields[field]; + if (v is int) return v; + if (v is double && v.isFinite && v == v.truncateToDouble()) return v.toInt(); + return null; + } + + /// [num_] restricted to finite values, so NaN/Inf never reach a reading. + num? finite(int field) { + final v = num_(field); + return v != null && v.isFinite ? v : null; + } } class _Field { @@ -238,8 +256,8 @@ List<(int, int)> fitMonitoringHr(List m) => [ for (final x in m) if (x.global == kFitMsgMonitoring && x.timestamp != null && - (x.num_(27) ?? 0) > 0) - (x.timestamp!, x.num_(27)!.toInt()), + (x.int_(27) ?? 0) > 0) + (x.timestamp!, x.int_(27)!), ]; /// Steps per local day: monitoring `cycles` (field 3) is a running count per @@ -250,9 +268,9 @@ Map fitDailySteps(List m) { final maxBy = <(DateTime, int), int>{}; for (final x in m) { if (x.global != kFitMsgMonitoring || x.timestamp == null) continue; - final packed = x.num_(24)?.toInt(); - final type = x.num_(5)?.toInt() ?? (packed == null ? null : packed & 0x1f); - final cycles = x.num_(3)?.toInt(); + final packed = x.int_(24); + final type = x.int_(5) ?? (packed == null ? null : packed & 0x1f); + final cycles = x.int_(3); if (cycles == null || (type != 6 && type != 1)) continue; final t = DateTime.fromMillisecondsSinceEpoch(x.timestamp! * 1000); final key = (DateTime(t.year, t.month, t.day), type!); @@ -273,7 +291,7 @@ List<(int, int, String)> fitSleepStages(List m, final levels = [ for (final x in m) if (x.global == kFitMsgSleepLevel && x.timestamp != null) - (x.timestamp!, x.num_(0)?.toInt() ?? -1), + (x.timestamp!, x.int_(0) ?? -1), ]..sort((a, b) => a.$1.compareTo(b.$1)); final out = <(int, int, String)>[]; for (var k = 0; k < levels.length; k++) { @@ -297,8 +315,8 @@ List<(int, double)> fitHrvLastNight(List m) => [ for (final x in m) if (x.global == kFitMsgHrvStatusSummary && x.timestamp != null && - x.num_(1) != null) - (x.timestamp!, x.num_(1)! / 128), + x.finite(1) != null) + (x.timestamp!, x.finite(1)! / 128), ]; /// The watch's resting HR (field 1 current-day, else field 0). @@ -306,8 +324,8 @@ List<(int, int)> fitRestingHr(List m) => [ for (final x in m) if (x.global == kFitMsgMonitoringHrData && x.timestamp != null && - (x.num_(1) ?? x.num_(0)) != null) - (x.timestamp!, (x.num_(1) ?? x.num_(0))!.toInt()), + (x.int_(1) ?? x.int_(0)) != null) + (x.timestamp!, (x.int_(1) ?? x.int_(0))!), ]; /// SpO2 readings in percent (field 0), positive only. @@ -315,8 +333,8 @@ List<(int, int)> fitSpo2(List m) => [ for (final x in m) if (x.global == kFitMsgSpo2Data && x.timestamp != null && - (x.num_(0) ?? 0) > 0) - (x.timestamp!, x.num_(0)!.toInt()), + (x.int_(0) ?? 0) > 0) + (x.timestamp!, x.int_(0)!), ]; /// Respiration rate in breaths/min (field 0, stored /100), positive only. @@ -324,15 +342,15 @@ List<(int, double)> fitRespiration(List m) => [ for (final x in m) if (x.global == kFitMsgRespirationRate && x.timestamp != null && - (x.num_(0) ?? 0) > 0) - (x.timestamp!, x.num_(0)! / 100), + (x.finite(0) ?? 0) > 0) + (x.timestamp!, x.finite(0)! / 100), ]; /// The watch's stress score 0-100 (field 0); negative values (no reading, /// activity) dropped. Its own time is field 1 when present. List<(int, int)> fitStress(List m) => [ for (final x in m) - if (x.global == kFitMsgStressLevel && (x.num_(0) ?? -1) >= 0) - if ((x.num_(1)?.toInt() ?? x.timestamp) case final t?) - (x.num_(1) != null ? t + kFitEpochOffset : t, x.num_(0)!.toInt()), + if (x.global == kFitMsgStressLevel && (x.int_(0) ?? -1) >= 0) + if ((x.int_(1) ?? x.timestamp) case final t?) + (x.int_(1) != null ? t + kFitEpochOffset : t, x.int_(0)!), ]; diff --git a/test/garmin_fit_test.dart b/test/garmin_fit_test.dart index 4e3aa68..920d2e9 100644 --- a/test/garmin_fit_test.dart +++ b/test/garmin_fit_test.dart @@ -129,4 +129,29 @@ void main() { expect(() => parseFit(f), throwsFormatException); } }); + + test('a float NaN or Inf in an integer field skips the sample, never throws', + () { + const nan = [0x00, 0x00, 0xc0, 0x7f], inf = [0x00, 0x00, 0x80, 0x7f]; + List fit(int global, int field, List bits) => + parseFit((FitWriter() + ..define(0, global, [(253, 4, 0x86), (field, 4, 0x88)]) + ..data(0, [...u32(fitTs(t0)), ...bits])) + .build()); + for (final bits in [nan, inf]) { + expect(fitMonitoringHr(fit(kFitMsgMonitoring, 27, bits)), isEmpty); + expect(fitDailySteps(fit(kFitMsgMonitoring, 3, bits)), isEmpty); + expect(fitDailySteps(fit(kFitMsgMonitoring, 5, bits)), isEmpty); + expect(fitSleepStages(fit(kFitMsgSleepLevel, 0, bits)), isEmpty); + expect(fitRestingHr(fit(kFitMsgMonitoringHrData, 1, bits)), isEmpty); + expect(fitSpo2(fit(kFitMsgSpo2Data, 0, bits)), isEmpty); + expect(fitRespiration(fit(kFitMsgRespirationRate, 0, bits)), isEmpty); + expect(fitHrvLastNight(fit(kFitMsgHrvStatusSummary, 1, bits)), isEmpty); + expect(fitStress(fit(kFitMsgStressLevel, 0, bits)), isEmpty); + expect(fitStress(fit(kFitMsgStressLevel, 1, bits)), isEmpty); + } + // A float that holds a whole number still reads (72.0 = 0x42900000). + expect(fitRestingHr(fit(kFitMsgMonitoringHrData, 1, [0, 0, 0x90, 0x42])), + [(t0, 72)]); + }); } From 04fa1d8dff49655043474d60942a3d5a516f8804 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 19:29:58 +0530 Subject: [PATCH 14/33] drop the deprecated gen5 clock builders from the public api cmdSetClockGen5 and cmdGetClockGen5 sent opcodes 146/147, which are not established commands, yet the barrel still exported them under names that read as the gen5 version of cmdSetClock. a set the strap ignores leaves the rtc unlatched with no error. nothing called them, so I removed the builders and their tests. the Cmd constants stay: 147 is still decoded in command responses and 146 is still classed as forbidden. (cherry picked from commit 089fdc33333ccaafd3414b429ea15eb013d8233a) --- lib/openstrap_protocol.dart | 2 -- lib/src/commands.dart | 55 ++----------------------------------- test/gen5_test.dart | 36 ------------------------ 3 files changed, 3 insertions(+), 90 deletions(-) diff --git a/lib/openstrap_protocol.dart b/lib/openstrap_protocol.dart index 72885d0..831b3a5 100644 --- a/lib/openstrap_protocol.dart +++ b/lib/openstrap_protocol.dart @@ -141,8 +141,6 @@ export 'src/commands.dart' cmdDisableAlarm, kDefaultAlarmHaptics, gen5ClientHello, - cmdSetClockGen5, - cmdGetClockGen5, cmdBuzzGen5Maverick, cmdSetConfigGen5, cmdSetDeviceConfigValueGen5, diff --git a/lib/src/commands.dart b/lib/src/commands.dart index e511605..314972d 100644 --- a/lib/src/commands.dart +++ b/lib/src/commands.dart @@ -146,8 +146,9 @@ Uint8List cmdGetClock(int seq, {BandProfile profile = BandProfile.gen4}) => /// SHARED ACROSS GENERATIONS — this is the correct builder for WHOOP 5 too: /// gen5 takes `SET_CLOCK(10)` carrying /// ``, and a real gen5 strap answers -/// `SUCCESS` for exactly this 8-byte form. Prefer this over -/// [cmdSetClockGen5], whose opcode 146 is not an established WHOOP opcode. +/// `SUCCESS` for exactly this 8-byte form. There is no separate gen5 clock +/// builder: opcode 146 ("SET_CLOCK_MAVERICK") is not an established WHOOP +/// opcode, and a set the strap ignores fails silently with the RTC unlatched. /// /// [now] defaults to `DateTime.now()`; pass a fixed instant for tests. Uint8List cmdSetClock(int seq, @@ -616,56 +617,6 @@ Uint8List cmdDisableAlarm(int seq, Uint8List gen5ClientHello({int seq = 1}) => buildCommand(seq, Cmd.getHello, const [0x01], BandProfile.gen5); -// ── gen5 clock (SET_CLOCK_MAVERICK=146 / GET_CLOCK_GEN5=147) ─────────────── -// -// gen5 replaces gen4's SET_CLOCK(0x0A)/GET_CLOCK(0x0B) with distinct opcode -// VALUES (§1.4). The bodies are NOT gen4's 8-byte two-u32 form — see each -// builder's own doc for the shape it sends. Still unverified against a real -// strap end to end: the bytes are right, but nothing here has watched an RTC -// actually latch. - -/// gen5 SET_CLOCK_MAVERICK (146) — **UNVERIFIED; prefer [cmdSetClock] (10).** -/// -/// Body is `[0x01][u32 epoch][u16 subsec]`. -/// -/// Opcode **146 is not an established WHOOP opcode**, and no "Maverick clock" -/// command is known to exist. What IS established for gen5: `SET_CLOCK(10)` -/// with ``, for which a real gen5 strap answers -/// `SUCCESS` while `GET_CLOCK(11)` reads the time back. -/// -/// So this builder sends a guess where a confirmed command exists. That is -/// especially dangerous for the clock: a rejected or no-op'd set is silent — -/// the RTC simply never latches, and every absolute timestamp afterwards -/// (alarms above all) is armed against a clock that was never set. -/// -/// Kept only for source compatibility. Do NOT send it to "find out what it -/// does": a clock write is a mutation, not a read. -@Deprecated('opcode 146 is not an established opcode; gen5 ' - 'uses SET_CLOCK(10) — use cmdSetClock(seq, profile: BandProfile.gen5)') -Uint8List cmdSetClockGen5(int seq, {DateTime? now}) { - final ms = (now ?? DateTime.now()).millisecondsSinceEpoch; - final sec = ms ~/ 1000; - final subsec = ((ms % 1000) * 32768) ~/ 1000; - final payload = [ - revision1, - ..._leTimestamp(sec, subsec), - 0, - 0, - ]; - return buildCommand(seq, Cmd.setClockMaverick, payload, BandProfile.gen5); -} - -/// gen5 GET_CLOCK_GEN5 (147) — **UNVERIFIED; prefer [cmdGetClock] (11).** -/// -/// Takes a `[0x01]` revision body. Same problem as [cmdSetClockGen5]: opcode -/// 147 is not an established WHOOP opcode, while `GET_CLOCK(11)` with an -/// EMPTY body is hardware-confirmed to answer on a real gen5 strap. Kept for -/// source compatibility only. -@Deprecated('opcode 147 is not an established opcode; gen5 ' - 'uses GET_CLOCK(11) — use cmdGetClock(seq, profile: BandProfile.gen5)') -Uint8List cmdGetClockGen5(int seq) => - buildCommand(seq, Cmd.getClockGen5, const [revision1], BandProfile.gen5); - // ── gen5 Maverick haptics (RUN_HAPTIC_PATTERN_MAVERICK = 0x13/19) ────────── /// gen5's Maverick haptic-buzz command — a DIFFERENT opcode from gen4's diff --git a/test/gen5_test.dart b/test/gen5_test.dart index 8a09ecf..5848f5c 100644 --- a/test/gen5_test.dart +++ b/test/gen5_test.dart @@ -264,42 +264,6 @@ void main() { reason: 'status $bad must not publish a clock'); } }); - - test( - 'the deprecated Maverick clock builders still emit their documented ' - 'bytes (kept only for a guarded experiment)', - () { - // ignore: deprecated_member_use_from_same_package - final setFrame = cmdSetClockGen5(1, now: DateTime.utc(2026, 1, 1)); - final setParsed = parseFrame(setFrame, profile: BandProfile.gen5)!; - expect(setParsed.valid, isTrue); - expect(setParsed.inner[2], Cmd.setClockMaverick); - expect(Cmd.setClockMaverick, 146); - - // ignore: deprecated_member_use_from_same_package - final getFrame = cmdGetClockGen5(1); - final getParsed = parseFrame(getFrame, profile: BandProfile.gen5)!; - expect(getParsed.valid, isTrue); - expect(getParsed.inner[2], Cmd.getClockGen5); - expect(Cmd.getClockGen5, 147); - }); - - test('the gen5 clock bodies carry the leading revision byte', () { - // Without it the strap reads the epoch's low byte as the revision and - // refuses the command, so the RTC never latches. - final setParsed = parseFrame( - cmdSetClockGen5(1, now: DateTime.utc(2026, 1, 1)), - profile: BandProfile.gen5, - )!; - expect(setParsed.inner[3], revision1); - final epoch = - ByteData.sublistView(setParsed.inner).getUint32(4, Endian.little); - expect(epoch, DateTime.utc(2026, 1, 1).millisecondsSinceEpoch ~/ 1000); - - final getParsed = - parseFrame(cmdGetClockGen5(1), profile: BandProfile.gen5)!; - expect(getParsed.inner[3], revision1); - }); }); group('OpcodeSafety', () { From cfff47c763974422473e7fba52f20b1b950ab0ad Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 19:31:24 +0530 Subject: [PATCH 15/33] drop the rest of the deprecated gen5 v26 compat surface the last cleanup removed only half of it. Gen5PpgWaveform still had burstIndex, frontEndMetaRaw, subChannel, subChannelKnown, signalMetric, gainSetting, gainIndex, flagA, flagB, ppgWaveform and the rawByte19 field, plus the kGen5V18MinInnerLen / kGen5V26MinInnerLen / kGen5V26MinInnerLenWithMeta aliases. nothing calls them, and several return something other than what the name says: subChannel is byte 2 of a sign-extended adc code (0xF8-0xFF whenever the first sample is negative), signalMetric is accel motion, and gainSetting/gainIndex are halves of a processing-state word. the real fields stay, so no decoded value changes. (cherry picked from commit 129bd1fc9eb97fd7f3cbfc12de8b2ced4fa79448) --- lib/openstrap_protocol.dart | 5 -- lib/src/gen5_records.dart | 92 +------------------------------ test/gen5_historical_test.dart | 32 ++--------- test/gen5_record_fields_test.dart | 3 - 4 files changed, 7 insertions(+), 125 deletions(-) diff --git a/lib/openstrap_protocol.dart b/lib/openstrap_protocol.dart index 831b3a5..3a13f3c 100644 --- a/lib/openstrap_protocol.dart +++ b/lib/openstrap_protocol.dart @@ -64,15 +64,10 @@ export 'src/gen5_records.dart' kGen5HistoricalDecoders, kGen5V22KnownTags, kGen5V22InnerLen, - // The exact lengths are the live names; the *MinInnerLen* aliases below - // are deprecated and exported only so existing callers still resolve. kGen5V18InnerLen, kGen5V26InnerLen, - kGen5V18MinInnerLen, kGen5V20InnerLen, kGen5V21InnerLen, - kGen5V26MinInnerLen, - kGen5V26MinInnerLenWithMeta, kGen5AccelScaleG, kGen5GyroScaleDps, isGen5ImuBuffer, diff --git a/lib/src/gen5_records.dart b/lib/src/gen5_records.dart index 00b88a7..564d5fc 100644 --- a/lib/src/gen5_records.dart +++ b/lib/src/gen5_records.dart @@ -517,10 +517,6 @@ const int kGen5V18InnerLen = 112; /// A caller that wants the signal can compare against [kGen5V18InnerLen]. const int kGen5V18MinReadableLen = 109; -@Deprecated( - 'v18 inner is exactly 112 bytes, not a floor — use kGen5V18InnerLen') -const int kGen5V18MinInnerLen = kGen5V18InnerLen; - class Gen5V18Decoder implements Gen5RecordDecoder { const Gen5V18Decoder(); @@ -1027,8 +1023,8 @@ class Gen5V21Decoder implements Gen5RecordDecoder { // This replaces an earlier reading of the same bytes (`burstIndex`@13, // `frontEndMetaRaw`@15:17, `subChannel`@17, 24 "AC samples", `signalMetric` // f32@67, `gainSetting`@71, `gainIndex`@72, `flagA`@73, `flagB`@74) which was -// REFUTED on hardware. Every deprecated member below says which real field its -// bytes actually belong to. +// REFUTED on hardware. Those names are gone rather than kept as aliases: each +// one returned a real field under a name that said something else. // // The pinning facts behind this layout, all exact (every R26 record has an // R18 twin on the shared (record_index, unix) key — zero orphans): @@ -1145,13 +1141,6 @@ Gen5PpgReconstruction reconstructSaturatedDeltaWindow( } class Gen5PpgWaveform extends Gen5HistoricalRecord { - /// Low byte of the record's sub-second @ inner[11] (frame-abs 19). - /// - /// This offset is the shared u16 sub-second, not a byte: reading one byte - /// here discards its high half (nonzero on 99% of real records). - @Deprecated('frame-abs 19 is the u16 sub-second — use tsSubsec / subSecond') - final int rawByte19; - /// u16 LE @ inner[11:13] (frame-abs 19) — the SAME field as [tsSubsec], the /// record's Q15 sub-second, kept under its historical name. /// @@ -1281,81 +1270,12 @@ class Gen5PpgWaveform extends Gen5HistoricalRecord { /// so nobody has to guess whether it was decoded; it carries no field. final int alignedTailByte; - /// COMPAT (deprecated): inner[13] read as a standalone "per-burst counter". - /// It is the LOW BYTE of the u16 PIP state/segment counter, so it wraps every - /// 256 records and drops the high half. - @Deprecated('inner[13] is the low byte of the u16 PIP state/segment counter ' - '— use pipStateCounter') - int get burstIndex => pipStateCounter & 0xFF; - - /// COMPAT (deprecated): inner[15:17] read as a u16 of unestablished meaning. - /// Those two bytes are the LOW HALF of [firstSampleAdc], the window's first - /// optical ADC code — half of a number, not a field. - @Deprecated('inner[15:17] is the low half of the i32 first optical ADC code ' - '— use firstSampleAdc') - int get frontEndMetaRaw => firstSampleAdc & 0xFFFF; - - /// COMPAT (deprecated): inner[17] read as a 0..7 "acquisition channel index". - /// It is byte 2 of [firstSampleAdc]'s sign-extended i32 — 0x00..0x07 for a - /// positive first sample, 0xF8..0xFF for a negative one. The "~0.2% outliers - /// at 0xFD..0xFF" this field's old doc comment reported were negative codes, - /// and the band does NOT multiplex channels through this byte. - @Deprecated('inner[17] is byte 2 of the sign-extended i32 first optical ADC ' - 'code, not a channel index — use firstSampleAdc') - int get subChannel => (firstSampleAdc >> 16) & 0xFF; - - /// COMPAT (deprecated): the 0..7 gate over [subChannel]. There is no channel - /// here to gate — it only ever answered "is the first sample positive?". - @Deprecated('there is no sub-channel in a v26 record — inner[17] is part of ' - 'the i32 first optical ADC code; use firstSampleAdcOrNull') - int? get subChannelKnown { - // ignore: deprecated_member_use_from_same_package - final b = subChannel; - return (b >= 0 && b <= 7) ? b : null; - } - - /// COMPAT (deprecated): the f32 at inner[67:71] read as a signal-quality - /// metric. It is the max adjacent accel-magnitude delta in g — motion. For a - /// real per-second quality weight use R18's - /// [Gen5HistorySample.signalQualityLogVariance]. - @Deprecated('inner[67:71] is the max adjacent accel-magnitude delta in g ' - '(== R18 dynamicAccelerationG), not a quality metric — use accelDeltaG') - double? get signalMetric => accelDeltaG; - - /// COMPAT (deprecated): the two halves of [channelStateWord] read as a gain - /// configuration. There is no gain field in a v26 record. - @Deprecated('inner[71] is the low byte of the u16 channel-0 processing-state ' - 'word (== R18 statusWord), not a gain — use channelStateWord') - int get gainSetting => channelStateWord & 0xFF; - @Deprecated( - 'inner[72] is the high byte of the u16 channel-0 processing-state ' - 'word (== R18 statusWord), not a gain — use channelStateWord') - int get gainIndex => (channelStateWord >> 8) & 0xFF; - - /// COMPAT (deprecated): the two trailing bytes read as unnamed flags. - @Deprecated('inner[73] is the primary-flags bit-8 snapshot — use ' - 'primaryFlagsByte / primaryFlagsBit8Raw') - int get flagA => primaryFlagsByte; - @Deprecated('inner[74] is the binary waveform-morphology acceptance result ' - '(NOT a heartbeat) — use morphologyPass / morphologyByte') - int get flagB => morphologyByte; - - /// COMPAT (deprecated): the 24 i16s read as AC-coupled SAMPLES. They are - /// DELTAS over a 25-sample window, so summing is required before anything - /// treats them as a waveform — which is also why their per-record mean sat - /// near zero and looked "DC-removed". - @Deprecated('inner[19:67] holds 24 saturated i16 DELTAS, not samples — use ' - 'opticalDeltas, or reconstructWindow() for absolute codes') - List get ppgWaveform => opticalDeltas; - const Gen5PpgWaveform({ required super.histVersion, required super.flags, required super.recordIndex, required super.unix, required super.tsSubsec, - @Deprecated('frame-abs 19 is the u16 sub-second — use tsSubsec / subSecond') - required this.rawByte19, required this.segmentId, required this.pipStateCounter, required this.firstSampleAdc, @@ -1380,12 +1300,6 @@ const int _kV26DeltasStart = 19; // frame-abs 27, body 6 /// otherwise decode out of whatever bytes happened to follow. const int kGen5V26InnerLen = 76; -@Deprecated('v26 inner is exactly 76 bytes, not a floor — use kGen5V26InnerLen') -const int kGen5V26MinInnerLen = kGen5V26InnerLen; - -@Deprecated('v26 inner is exactly 76 bytes, not a floor — use kGen5V26InnerLen') -const int kGen5V26MinInnerLenWithMeta = kGen5V26InnerLen; - class Gen5V26Decoder implements Gen5RecordDecoder { const Gen5V26Decoder(); @@ -1420,8 +1334,6 @@ class Gen5V26Decoder implements Gen5RecordDecoder { unix: hdr.unix, tsSubsec: v.getUint16(11, Endian.little), flags: hdr.flags, - // ignore: deprecated_member_use_from_same_package - rawByte19: inner[11], segmentId: v.getUint16(11, Endian.little), // Body offsets + 13. The trailing block (67/71/73) is byte-identical // to the twin R18 record's 33/67/73; see this section's header comment. diff --git a/test/gen5_historical_test.dart b/test/gen5_historical_test.dart index 3602d6b..18295b3 100644 --- a/test/gen5_historical_test.dart +++ b/test/gen5_historical_test.dart @@ -229,8 +229,7 @@ void main() { // a separate field. expect(wf.recordIndex, 25444781); expect(wf.unix, 1780917232); - // ignore: deprecated_member_use_from_same_package - expect(wf.rawByte19, 174); + expect(wf.segmentId & 0xFF, 174); expect(wf.pipStateCounter, 1); // These 24 i16s are DELTAS over a 25-sample window (body 6), not // samples — see `reconstructs a 25-sample window` below for the @@ -286,12 +285,9 @@ void main() { () { final wf = decode(frame); expect(wf.segmentId, 18350); - // The deprecated byte read returns 18350 & 0xFF — the high byte (71) - // is discarded. On real records that byte is nonzero 99% of the time. - // ignore: deprecated_member_use_from_same_package - expect(wf.rawByte19, 174); - // ignore: deprecated_member_use_from_same_package - expect(wf.rawByte19, wf.segmentId & 0xFF); + // A byte read would return 18350 & 0xFF = 174 and discard the high + // byte (71). On real records that byte is nonzero 99% of the time. + expect(wf.segmentId & 0xFF, 174); expect(wf.segmentId >> 8, isNonZero); }); @@ -348,8 +344,7 @@ void main() { final wf = parseGen5Historical(inner) as Gen5PpgWaveform; expect(wf.firstSampleAdc, -148726); expect(wf.firstSampleAdcInRange, isTrue); - // ignore: deprecated_member_use_from_same_package - expect(wf.subChannel, 0xFD); // the "outlier" the old reading saw + expect(inner[17], 0xFD); // the "outlier" the old reading saw }); test('an impossible first sample is exposed raw, not fabricated', () { @@ -433,23 +428,6 @@ void main() { expect(r.hasSaturatedDelta, isFalse); // independent of the clamp }); - test('the deprecated members map onto the real fields they misread', () { - // Every name below is deprecated; the package's analysis_options mutes - // deprecated_member_use_from_same_package precisely so these stay - // testable. See the class for what each byte really is. - final wf = decode(frame); - expect(wf.burstIndex, wf.pipStateCounter & 0xFF); - expect(wf.frontEndMetaRaw, wf.firstSampleAdc & 0xFFFF); // 50627 - expect(wf.subChannel, (wf.firstSampleAdc >> 16) & 0xFF); // 5 - expect(wf.subChannelKnown, 5); // "inside 0..7" only ever meant positive - expect(wf.signalMetric, wf.accelDeltaG); - expect(wf.gainSetting, wf.channelStateWord & 0xFF); // 80 - expect(wf.gainIndex, (wf.channelStateWord >> 8) & 0xFF); // 8 - expect(wf.flagA, wf.primaryFlagsByte); - expect(wf.flagB, wf.morphologyByte); - expect(wf.ppgWaveform, wf.opticalDeltas); // deltas, never samples - }); - test('a record short of the exact 76-byte length is rejected outright', () { // v26's inner is exactly 76 bytes. A truncated one used to decode its // waveform with the trailing metadata nulled out; it is now rejected, diff --git a/test/gen5_record_fields_test.dart b/test/gen5_record_fields_test.dart index 7e1b451..8d021d9 100644 --- a/test/gen5_record_fields_test.dart +++ b/test/gen5_record_fields_test.dart @@ -81,9 +81,6 @@ void main() { // the sub-second in hundredths. expect(w.segmentId, w.tsSubsec); expect(w.segmentIndex, 56); - // The deprecated byte read keeps only the low half. - // ignore: deprecated_member_use_from_same_package - expect(w.rawByte19, w.tsSubsec & 0xFF); }); test('the Q15 scale holds for every verified raw value', () { From 30d7acec9f49657cb4ec8ff6fddc893a0fd57a3f Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 19:31:18 +0530 Subject: [PATCH 16/33] flag frames the reassembler gives up as truncated giving up a short frame lets the HISTORY_END queued behind it through, and that END covers the record that never arrived. it came out as a plain crc failure, indistinguishable from noise, so a caller would ack the burst and the band would trim a record that exists nowhere. the given-up frame now carries Frame.truncated and the reassembler counts them in truncations, so the burst can be left untrimmed and re-offered. (cherry picked from commit 3f0108bc52285e14d39fb9809c97d41beba18f59) --- lib/src/framing.dart | 30 +++++++++++++++++++++++++++--- test/framing_test.dart | 17 +++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/lib/src/framing.dart b/lib/src/framing.dart index d7fc26d..4e6355e 100644 --- a/lib/src/framing.dart +++ b/lib/src/framing.dart @@ -36,7 +36,19 @@ class Frame { /// whose revision byte is not rev-1 — surfacing it instead of mis-decoding. final bool frameRevOk; - Frame(this.inner, this.crc8Ok, this.crc32Ok, {this.frameRevOk = true}); + /// Set only on a frame [FrameReassembler] gave up on because a lost + /// notification left it short (see [FrameReassembler.truncations]). Its + /// `inner` is just the bytes that did arrive, so [valid] is false. + /// + /// This is a lost record, not just a corrupt one: giving it up is what lets + /// the HISTORY_END queued behind it through, and that END covers the record + /// that never arrived. ACKing it trims that record from the band's flash + /// while it exists nowhere else, so the burst it belongs to must not be + /// trimmed. + final bool truncated; + + Frame(this.inner, this.crc8Ok, this.crc32Ok, + {this.frameRevOk = true, this.truncated = false}); /// Band-neutral alias for the header-integrity result. bool get headerCrcOk => crc8Ok; @@ -121,6 +133,14 @@ class FrameReassembler { final List _buf = []; final BandProfile profile; int _resyncs = 0; + int _truncations = 0; + + /// Number of frames given up as truncated (each emitted with + /// [Frame.truncated]). A caller that saw this move during an offload burst + /// must not ACK that burst's HISTORY_END: before the give-up existed the END + /// stayed stuck behind the short frame and the burst was abandoned unACKed, + /// so the band offered it again. + int get truncations => _truncations; /// Number of times the reassembler skipped a byte because the envelope did /// not hold up (bad SOF, implausible length, or a length field whose crc8 @@ -183,13 +203,16 @@ class FrameReassembler { // watchdog aborts the burst. A complete frame passing both CRCs // inside the span means this one was truncated: give it up and // resume there. Emitted as a CRC failure so the loss is still - // counted. A payload that happened to contain a whole valid frame + // counted, and flagged truncated so a caller can refuse to trim the + // burst the END closes. A payload that happened to contain a whole valid frame // would need both an 8-bit and a 32-bit CRC to match by chance. final next = _completeFrameWithin(headerLen); if (next < 0) break; // wait for the rest of this frame out.add(Frame( - Uint8List.fromList(_buf.sublist(headerLen, next)), true, false)); + Uint8List.fromList(_buf.sublist(headerLen, next)), true, false, + truncated: true)); _resyncs++; + _truncations++; _buf.removeRange(0, next); continue; } @@ -243,5 +266,6 @@ class FrameReassembler { void reset() { _buf.clear(); _resyncs = 0; + _truncations = 0; } } diff --git a/test/framing_test.dart b/test/framing_test.dart index 8db7788..0655dd8 100644 --- a/test/framing_test.dart +++ b/test/framing_test.dart @@ -206,7 +206,12 @@ void main() { expect(frames.length, 2); expect(frames.first.valid, isFalse, reason: 'the truncated frame must still count as a lost frame'); + expect(frames.first.truncated, isTrue, + reason: 'the END that follows covers a record that never arrived, ' + 'so the caller has to be able to tell this apart from noise'); + expect(re.truncations, 1); expect(frames.last.valid, isTrue); + expect(frames.last.truncated, isFalse); expect(frames.last.packetType, PacketType.metadata); expect(parseMetadata(frames.last.inner)!.token, [0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57]); @@ -222,6 +227,18 @@ void main() { final frames = re.feed(data.sublist(60)); expect(frames.single.valid, isTrue); expect(re.resyncs, 0); + expect(re.truncations, 0); + }); + + test('a frame with a corrupt payload is not reported as truncated', () { + final data = buildFrame( + [PacketType.historicalData, 1, 0x18, ...List.filled(93, 0x33)]); + data[20] ^= 0xFF; + final re = FrameReassembler(); + final f = re.feed(data).single; + expect(f.valid, isFalse); + expect(f.truncated, isFalse); + expect(re.truncations, 0); }); }); From efc524eb21b1066c9596f4391c8953a47bb61f26 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 19:35:28 +0530 Subject: [PATCH 17/33] keep a history_end held behind a truncated frame unless the caller opts in giving up a truncated frame let the history_end queued behind it through, and on a whoop 4 that end gets acked, so the band trimmed a record that never arrived. before that change the end stayed stuck, the burst was abandoned unacked and the band sent it again. the give-up is now off by default and only turns on with giveUpTruncatedFrames, so a repin alone does not change what gets trimmed. the truncated flag also read as if only truncated frames were lost records. a notification lost inside an earlier frame shows up as a plain crc failure and is just as lost, so there is now a lostFrames counter covering every frame that is not valid, and the docs point the trim decision at that. (cherry picked from commit 59a4f8fdd58f6d5a20803c62203aa6f7e7268e1a) --- lib/src/framing.dart | 66 ++++++++++++++++++++++++++++++------------ test/framing_test.dart | 43 +++++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 21 deletions(-) diff --git a/lib/src/framing.dart b/lib/src/framing.dart index 4e6355e..0011df4 100644 --- a/lib/src/framing.dart +++ b/lib/src/framing.dart @@ -37,14 +37,15 @@ class Frame { final bool frameRevOk; /// Set only on a frame [FrameReassembler] gave up on because a lost - /// notification left it short (see [FrameReassembler.truncations]). Its - /// `inner` is just the bytes that did arrive, so [valid] is false. + /// notification left it short and a complete frame was already buffered + /// behind it (see [FrameReassembler.giveUpTruncatedFrames]). Its `inner` is + /// just the bytes that did arrive, so [valid] is false. /// - /// This is a lost record, not just a corrupt one: giving it up is what lets - /// the HISTORY_END queued behind it through, and that END covers the record - /// that never arrived. ACKing it trims that record from the band's flash - /// while it exists nowhere else, so the burst it belongs to must not be - /// trimmed. + /// This says how the frame was lost, not whether it was. Every frame that is + /// not [valid] is a record that did not arrive intact — a notification lost + /// in the middle of a frame shows up as a plain CRC failure with this flag + /// false — so the trim decision must key on [valid] (or + /// [FrameReassembler.lostFrames]), never on this flag alone. final bool truncated; Frame(this.inner, this.crc8Ok, this.crc32Ok, @@ -134,21 +135,42 @@ class FrameReassembler { final BandProfile profile; int _resyncs = 0; int _truncations = 0; + int _lostFrames = 0; + + /// Whether a frame left short by a lost notification is given up as soon as + /// a complete frame passing both CRCs is buffered behind it. + /// + /// Off by default, and only safe to turn on for a caller that refuses to + /// trim any offload burst in which [lostFrames] moved. With it off, the + /// HISTORY_END the band sends after a burst whose last frame lost a + /// notification stays stuck behind that frame: nothing is ACKed, the idle + /// watchdog abandons the burst, and the band offers the record again. With + /// it on, that END is delivered — and a caller that ACKs it trims a record + /// that exists nowhere else. + final bool giveUpTruncatedFrames; /// Number of frames given up as truncated (each emitted with - /// [Frame.truncated]). A caller that saw this move during an offload burst - /// must not ACK that burst's HISTORY_END: before the give-up existed the END - /// stayed stuck behind the short frame and the burst was abandoned unACKed, - /// so the band offered it again. + /// [Frame.truncated]). Always 0 unless [giveUpTruncatedFrames] is on. A + /// subset of [lostFrames]; use that for the trim decision. int get truncations => _truncations; + /// Number of frames emitted that are not [Frame.valid]: a payload CRC + /// failure (including a notification lost in the middle of a frame) or a + /// frame given up as truncated. Each one is a record that did not arrive + /// intact. A caller that saw this move during an offload burst must not ACK + /// that burst's HISTORY_END, whichever way the frame was lost, or the band + /// trims records that were never banked. Bytes skipped by a resync never + /// become a frame at all, so watch [resyncs] alongside this. + int get lostFrames => _lostFrames; + /// Number of times the reassembler skipped a byte because the envelope did /// not hold up (bad SOF, implausible length, or a length field whose crc8 /// did not match). Callers use this to detect a degraded link — a bad /// length is discarded here, so it never reaches [Frame.valid]. int get resyncs => _resyncs; - FrameReassembler({this.profile = BandProfile.gen4}); + FrameReassembler( + {this.profile = BandProfile.gen4, this.giveUpTruncatedFrames = false}); List feed(List chunk) { final out = []; @@ -200,12 +222,17 @@ class FrameReassembler { // A lost notification leaves this frame short, and the band sends // HISTORY_END and then waits for the ACK — so no further bytes come // to complete it, and the END sits inside the pending span until a - // watchdog aborts the burst. A complete frame passing both CRCs - // inside the span means this one was truncated: give it up and - // resume there. Emitted as a CRC failure so the loss is still - // counted, and flagged truncated so a caller can refuse to trim the - // burst the END closes. A payload that happened to contain a whole valid frame - // would need both an 8-bit and a 32-bit CRC to match by chance. + // watchdog aborts the burst. Holding it there is what keeps the band + // from trimming the lost record, so the give-up below is opt-in (see + // [giveUpTruncatedFrames]). + // + // A complete frame passing both CRCs inside the span means this one + // was truncated: give it up and resume there. Emitted as a CRC failure + // and counted in [lostFrames] so the caller can refuse to trim the + // burst the END closes. A payload that happened to contain a whole + // valid frame would need both an 8-bit and a 32-bit CRC to match by + // chance. + if (!giveUpTruncatedFrames) break; // wait for the rest of this frame final next = _completeFrameWithin(headerLen); if (next < 0) break; // wait for the rest of this frame out.add(Frame( @@ -213,12 +240,14 @@ class FrameReassembler { truncated: true)); _resyncs++; _truncations++; + _lostFrames++; _buf.removeRange(0, next); continue; } final frame = parseFrame(Uint8List.fromList(_buf.sublist(0, total)), profile: profile); if (frame != null) out.add(frame); + if (frame != null && !frame.valid) _lostFrames++; if (frame != null && !frame.crc32Ok) { // The length field is covered only by the header check — 8 bits on // gen4 — so roughly 1 in 254 corrupt length pairs still passes it. A @@ -267,5 +296,6 @@ class FrameReassembler { _buf.clear(); _resyncs = 0; _truncations = 0; + _lostFrames = 0; } } diff --git a/test/framing_test.dart b/test/framing_test.dart index 0655dd8..f2e0417 100644 --- a/test/framing_test.dart +++ b/test/framing_test.dart @@ -191,13 +191,26 @@ void main() { return buildFrame(inner); } - test('a lost notification does not hold the HISTORY_END behind it', () { + test('by default a HISTORY_END behind a truncated frame stays held', () { + final data = buildFrame( + [PacketType.historicalData, 1, 0x18, ...List.filled(93, 0x33)]); + final re = FrameReassembler(); + expect(re.feed(data).single.valid, isTrue); + expect(re.feed(data.sublist(0, 44)), isEmpty); + expect(re.feed(historyEnd(2)), isEmpty, + reason: 'delivering the END would let it be ACKed, and the band ' + 'would trim the record that never arrived'); + expect(re.truncations, 0); + }); + + test('a lost notification does not hold the HISTORY_END behind it ' + 'when give-up is on', () { final data = buildFrame( [PacketType.historicalData, 1, 0x18, ...List.filled(93, 0x33)]); expect(data.length, 104); final end = historyEnd(2); - final re = FrameReassembler(); + final re = FrameReassembler(giveUpTruncatedFrames: true); expect(re.feed(data).single.valid, isTrue); // Only the first notification of the next data frame arrives. expect(re.feed(data.sublist(0, 44)), isEmpty); @@ -210,6 +223,7 @@ void main() { reason: 'the END that follows covers a record that never arrived, ' 'so the caller has to be able to tell this apart from noise'); expect(re.truncations, 1); + expect(re.lostFrames, 1); expect(frames.last.valid, isTrue); expect(frames.last.truncated, isFalse); expect(frames.last.packetType, PacketType.metadata); @@ -222,7 +236,7 @@ void main() { test('a partial frame still waits when nothing complete sits inside it', () { final data = buildFrame( [PacketType.historicalData, 1, 0x18, ...List.filled(93, 0xAA)]); - final re = FrameReassembler(); + final re = FrameReassembler(giveUpTruncatedFrames: true); expect(re.feed(data.sublist(0, 60)), isEmpty); final frames = re.feed(data.sublist(60)); expect(frames.single.valid, isTrue); @@ -239,6 +253,29 @@ void main() { expect(f.valid, isFalse); expect(f.truncated, isFalse); expect(re.truncations, 0); + expect(re.lostFrames, 1, + reason: 'a corrupt record is just as lost as a truncated one'); + }); + + test('a notification lost inside a non-final frame counts as lost', () { + List rec(int seq) => buildFrame([ + PacketType.historicalData, + seq, + 0x18, + ...List.filled(93, 0x30 + seq) + ]); + final a = rec(1), b = rec(2), c = rec(3); + final stream = [...a, ...b.sublist(0, 44), ...b.sublist(64), ...c, + ...historyEnd(4)]; + final re = FrameReassembler(); + final frames = re.feed(stream); + expect(frames.where((f) => !f.valid), isNotEmpty); + expect(frames.any((f) => f.truncated), isFalse); + expect(re.lostFrames, greaterThan(0), + reason: 'the END after this burst covers a record that never ' + 'arrived, so the caller must be able to see the loss'); + re.reset(); + expect(re.lostFrames, 0); }); }); From b15a6cca4c3f5b77650b3823963adbddd0b33d73 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 23:05:01 +0530 Subject: [PATCH 18/33] stop trusting battery_level millivolts and charging from a garbage body battery_pct was range-checked but battery_mv and charging were emitted for any body of 12+ bytes, so a body whose soc was already rejected could still report 60000 mV. hosts bank that value and a single bad row pins a lifetime max. now mv and charging only come out when soc is plausible, and mv must also fall in a li-ion cell range (2500-4500). (cherry picked from commit 07fa12f70b187cfa5d42f3254cc3bcc0f5fbbe42) --- lib/src/control.dart | 9 +++++++-- test/control_plane_offsets_test.dart | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/src/control.dart b/lib/src/control.dart index fd566bd..d0475bd 100644 --- a/lib/src/control.dart +++ b/lib/src/control.dart @@ -698,11 +698,16 @@ EventInfo? parseEvent( // [10], the byte that is always zero, so it was permanently false. if (body.length >= 12) { final soc = _round(u16(body, 1) / 10.0, 1); + // An out-of-range SoC means this body is not the layout being read, + // so the other fields from it are not trusted either. A millivolt + // reading outside a Li-ion cell's range is dropped on its own: hosts + // bank it, and one garbage value can pin a lifetime MAX. if (soc.isFinite && soc >= 0.0 && soc <= 100.0) { dec['battery_pct'] = soc; + final mv = u16(body, 5); + if (mv >= 2500 && mv <= 4500) dec['battery_mv'] = mv; + dec['charging'] = body[9] != 0; } - dec['battery_mv'] = u16(body, 5); - dec['charging'] = body[9] != 0; } break; case EventId.strapConditionReport: diff --git a/test/control_plane_offsets_test.dart b/test/control_plane_offsets_test.dart index 1f881fe..9d9ac8a 100644 --- a/test/control_plane_offsets_test.dart +++ b/test/control_plane_offsets_test.dart @@ -454,6 +454,31 @@ void main() { envelopePacket(0x30, EventId.batteryLevel, batteryBody(0)))!; expect(off.decoded['charging'], isFalse); }); + + test('BATTERY_LEVEL with a garbage body emits no millivolts or charging', + () { + List body({required int soc, required int mv}) => [ + 9, // not the documented revision + ...le32(soc), + ...le32(mv), + 1, // [9] + 0, + ...List.filled(8, 0), + ]; + // SoC rejected: nothing else from the same body is trusted either. + final junk = parseEvent(envelopePacket( + 0x30, EventId.batteryLevel, body(soc: 0xFFFF, mv: 60000)))!; + expect(junk.decoded.containsKey('battery_pct'), isFalse); + expect(junk.decoded.containsKey('battery_mv'), isFalse); + expect(junk.decoded.containsKey('charging'), isFalse); + + // Plausible SoC but a voltage no Li-ion cell reports: drop only mV. + final badMv = parseEvent(envelopePacket( + 0x30, EventId.batteryLevel, body(soc: 855, mv: 60000)))!; + expect(badMv.decoded['battery_pct'], closeTo(85.5, 1e-9)); + expect(badMv.decoded.containsKey('battery_mv'), isFalse); + expect(badMv.decoded['charging'], isTrue); + }); }); // Field-level bodies exist for four event ids the From f8ee613de8bf7a7741824bd8a9090b448de8628a Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 23:04:57 +0530 Subject: [PATCH 19/33] drop the deprecated channel0/channel1AdcOffset getters on the gen5 optical block they masked the signed tia offset current to an unsigned u16, so a -1000 offset read back as 64536. nothing outside one test called them, but they were still public through the barrel. tia1/2OffsetCurrentRaw and the nanoamp getters are the only view of that field now. (cherry picked from commit 9f2a4ec4816e8ecc361e78724c10efda63db8fb3) --- lib/src/gen5_records.dart | 14 -------------- test/gen5_record_fields_test.dart | 5 ----- 2 files changed, 19 deletions(-) diff --git a/lib/src/gen5_records.dart b/lib/src/gen5_records.dart index 564d5fc..a585ff6 100644 --- a/lib/src/gen5_records.dart +++ b/lib/src/gen5_records.dart @@ -706,13 +706,6 @@ class Gen5OpticalBlock { _view(channel0MetaRaw).getInt16(5, Endian.little); int get tia1OffsetCurrentNanoamps => tia1OffsetCurrentRaw * 10; - @Deprecated( - 'This field is the TIA 1 offset CURRENT: a signed i16 in 10 nA/LSB, not an ' - 'unsigned ADC offset. Use tia1OffsetCurrentRaw / ' - 'tia1OffsetCurrentNanoamps.', - ) - int get channel0AdcOffset => tia1OffsetCurrentRaw & 0xFFFF; - /// TIA 2's, same three fields at the same relative offsets (descriptor /// relative 13/14/18). int get channel1Source => channel1MetaRaw[0]; @@ -724,13 +717,6 @@ class Gen5OpticalBlock { int get tia2OffsetCurrentRaw => _view(channel1MetaRaw).getInt16(5, Endian.little); int get tia2OffsetCurrentNanoamps => tia2OffsetCurrentRaw * 10; - - @Deprecated( - 'This field is the TIA 2 offset CURRENT: a signed i16 in 10 nA/LSB, not an ' - 'unsigned ADC offset. Use tia2OffsetCurrentRaw / ' - 'tia2OffsetCurrentNanoamps.', - ) - int get channel1AdcOffset => tia2OffsetCurrentRaw & 0xFFFF; } /// The raw optical deep buffer. Layout is confirmed: the body starts at inner diff --git a/test/gen5_record_fields_test.dart b/test/gen5_record_fields_test.dart index 8d021d9..ad7e62b 100644 --- a/test/gen5_record_fields_test.dart +++ b/test/gen5_record_fields_test.dart @@ -306,11 +306,6 @@ void main() { expect(blk.tia1OffsetCurrentNanoamps, -10000); expect(blk.tia2OffsetCurrentRaw, 2400); expect(blk.tia2OffsetCurrentNanoamps, 24000); // 24 µA - // The deprecated getters keep their old unsigned-u16 behaviour. - // ignore: deprecated_member_use_from_same_package - expect(blk.channel0AdcOffset, 64536); - // ignore: deprecated_member_use_from_same_package - expect(blk.channel1AdcOffset, 2400); } }); }); From a44ac471931bc2290d74a75289b652eca3f26f87 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 23:05:33 +0530 Subject: [PATCH 20/33] pebble reassembler tells the caller to re-ack a repeated packet addPacket returned null for a re-send of the packet it had just taken, and its doc said null means don't ack. the watch only re-sends a packet when the ack for it was lost, so a caller following that contract would never ack it and the watch would retry until it reset the transport. a repeat of the last serial now returns an empty list (re-ack it, nothing new); null is kept for packets that are out of sequence. (cherry picked from commit 6ffc9f7e94a22961575ebf5df03b877bae94808f) --- lib/src/pebble.dart | 7 ++++++- test/pebble_test.dart | 16 +++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/src/pebble.dart b/lib/src/pebble.dart index 0afd8b5..a690d6d 100644 --- a/lib/src/pebble.dart +++ b/lib/src/pebble.dart @@ -100,11 +100,16 @@ class PebbleFrameReassembler { /// frame completes. Clearing on a gap instead would make a mid-frame /// payload the head of the buffer and misframe everything after it. /// - /// Returns null when the packet was dropped, so the caller does not ack it. + /// A repeat of the packet taken last returns an empty list, and the caller + /// must ack it again: the watch only re-sends a packet whose ack it never + /// saw, and without a fresh ack it keeps re-sending until its retry limit + /// resets the transport. Returns null only for a packet that is not the + /// next serial and not that repeat; the caller does not ack it. List<(int, Uint8List)>? addPacket(List packet) { if (packet.isEmpty) return null; final serial = (packet[0] >> 3) & 0x1f; final last = _lastSerial; + if (last != null && serial == last) return const []; if (last != null && serial != (last + 1) & 0x1f) return null; _lastSerial = serial; return add(packet.sublist(1)); diff --git a/test/pebble_test.dart b/test/pebble_test.dart index 8b9a3fb..0147037 100644 --- a/test/pebble_test.dart +++ b/test/pebble_test.dart @@ -46,11 +46,25 @@ void main() { final r = PebbleFrameReassembler(); final ps = pebblePpogattPackets(b, 30); expect(r.addPacket(ps[0]), isEmpty); - expect(r.addPacket(ps[0]), isNull); + // Not null: the repeat still has to be acked. + expect(r.addPacket(ps[0]), isEmpty); final out = feed(r, ps.skip(1).toList()); expect(out.single.$2, [2, 1, ...List.filled(30, 1)]); }); + test('a re-send of the frame-completing packet is acked, not re-emitted', + () { + final r = PebbleFrameReassembler(); + final ps = pebblePpogattPackets(b, 6); + expect(feed(r, ps).single.$2, [2, 1, ...List.filled(30, 1)]); + // The ack for the last packet was lost, so the watch re-sends it. + for (var i = 0; i < 3; i++) { + expect(r.addPacket(ps.last), isEmpty); + } + final next = feed(r, pebblePpogattPackets(c, 6 + ps.length)); + expect(next.single.$2[1], 2); + }); + test('go-back-N re-send after lost acks keeps the frame intact', () { final r = PebbleFrameReassembler(); final pa = pebblePpogattPackets(a, 0); // serials 0..3 From e0eb92db65f8ca9b02fc3f2791a0fc3097ffc4fa Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 23:05:44 +0530 Subject: [PATCH 21/33] gen5 hello battery dropped the tenths and let raw 1001..1009 through as 100% the hello integer-divided the deci-percent value and range-checked the quotient, so a whoop 5 at 19.9% read 19 on connect and 19.9 once the first battery event arrived, and a raw 1005 came out as a confident 100. it now rounds to 0.1% and refuses raw values past 1000, same as the gen4 hello and battery event decoders. batteryPct is a double now. (cherry picked from commit e2a259a4e8f6aecc84c78970d547d99f1d1614e7) --- lib/src/control.dart | 18 +++++++++++------- test/gen5_historical_test.dart | 27 +++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/lib/src/control.dart b/lib/src/control.dart index d0475bd..8b053c4 100644 --- a/lib/src/control.dart +++ b/lib/src/control.dart @@ -346,11 +346,15 @@ class Gen5HelloInfo { final int helloRevision; // body[0] - /// body[1..4] u32 LE, integer-divided by 10. **Null when the raw value is - /// outside 0..100** — a percentage is 0..100 by definition, so anything else - /// is a mis-read field, and this package's rule is to omit it rather than - /// report or clamp a number the bytes do not support. - final int? batteryPct; + /// body[1..4] u32 LE in tenths of a percent, rounded to 0.1% — the same + /// scale and rounding as the gen4 HELLO and BATTERY_LEVEL decoders, so the + /// value doesn't jump when the first battery event follows the hello. + /// **Null when the raw value is outside 0..1000** — a percentage is 0..100 + /// by definition, so anything else is a mis-read field, and this package's + /// rule is to omit it rather than report or clamp a number the bytes do not + /// support. The raw value is checked, not a quotient: integer division let + /// 1001..1009 through as 100. + final double? batteryPct; final bool charging; // body[5] bit0 final int tsSeconds; // body[6..9] u32 LE — whole seconds final int tsSubseconds; // body[10..13] u32 LE — 32768 units/s @@ -453,10 +457,10 @@ class Gen5HelloInfo { return sb.toString(); } - final batteryRaw = u32(body, 1) ~/ 10; + final batteryRaw = u32(body, 1); return Gen5HelloInfo( helloRevision: body[0], - batteryPct: batteryRaw <= 100 ? batteryRaw : null, + batteryPct: batteryRaw <= 1000 ? _round(batteryRaw / 10.0, 1) : null, charging: (body[5] & 0x01) != 0, tsSeconds: u32(body, 6), tsSubseconds: u32(body, 10), diff --git a/test/gen5_historical_test.dart b/test/gen5_historical_test.dart index 18295b3..2bbbb1b 100644 --- a/test/gen5_historical_test.dart +++ b/test/gen5_historical_test.dart @@ -709,7 +709,7 @@ void main() { test('Gen5HelloInfo.parse decodes the revision-1 body map', () { final h = Gen5HelloInfo.parse(gen5HelloBody())!; expect(h.helloRevision, 1); - expect(h.batteryPct, 90); // 901 ~/ 10 + expect(h.batteryPct, closeTo(90.1, 1e-9)); // raw 901 = 90.1% expect(h.charging, isFalse); expect(h.tsSeconds, 1780000000); expect(h.tsSubseconds, 28835); @@ -778,7 +778,7 @@ void main() { body[0] = 2; // some future revision final h = Gen5HelloInfo.parse(body)!; expect(h.helloRevision, 2); - expect(h.batteryPct, 90); + expect(h.batteryPct, closeTo(90.1, 1e-9)); expect(h.serial, '5AG0000001'); expect(h.firmwareVersion, '50.40.1.0'); expect(h.wristOn, isTrue); @@ -792,6 +792,29 @@ void main() { expect(h.serial, '5AG0000001'); // the rest of the record still decodes }); + test('battery keeps tenths and refuses raw values past 1000', () { + // Same scale and cap as the gen4 BATTERY_LEVEL event, so a WHOOP 5 at + // 19.9% doesn't read 19% on connect and 19.9% a few seconds later. + for (final e in { + 199: 19.9, + 99: 9.9, + 9: 0.9, + 0: 0.0, + 1000: 100.0, + }.entries) { + final body = gen5HelloBody(); + ByteData.sublistView(body).setUint32(1, e.key, Endian.little); + expect(Gen5HelloInfo.parse(body)!.batteryPct, closeTo(e.value, 1e-9), + reason: 'raw ${e.key}'); + } + for (final raw in [1001, 1005, 1009, 1010]) { + final body = gen5HelloBody(); + ByteData.sublistView(body).setUint32(1, raw, Endian.little); + expect(Gen5HelloInfo.parse(body)!.batteryPct, isNull, + reason: 'raw $raw'); + } + }); + test('gen5 GET_HELLO omits the hello when the body is too short', () { final inner = Uint8List(5 + 80); // < 104-byte semantic body inner[0] = PacketType.commandResponse; From d734064d4aa2b3c0761d536e6bd79a95c4c1be37 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 23:05:52 +0530 Subject: [PATCH 22/33] drop the deprecated ecg opcode aliases and builders the barrel kept cmdEcgControl and cmdEcgSendRaw unexported so a guessed opcode wasn't offered as a capability, but the Cmd.ecg* aliases behind them were still public through the constants export, including 0x7E and 0x7F, which aren't established opcodes at all. the other three aliases just duplicated the toggleLabrador* constants, and the builders had no callers outside their own test (cmdEcgControl's 'arm' sent the stop operation). I removed the five aliases, the three builders and that test. (cherry picked from commit c31871b944f6495fb025537a271186821edbad0e) --- lib/openstrap_protocol.dart | 4 +-- lib/src/commands.dart | 50 ++--------------------------- lib/src/constants.dart | 24 -------------- test/gen5_command_surface_test.dart | 32 ------------------ 4 files changed, 3 insertions(+), 107 deletions(-) diff --git a/lib/openstrap_protocol.dart b/lib/openstrap_protocol.dart index 3a13f3c..1da9bb9 100644 --- a/lib/openstrap_protocol.dart +++ b/lib/openstrap_protocol.dart @@ -163,9 +163,7 @@ export 'src/commands.dart' cmdRawDataStart, // 81 cmdRawDataStop, // 82 cmdStopHaptics, // 122 - // Filtered reading ("Labrador", R17). The wrong-bodied cmdEcgControl - // (124 as a bool) and the opcode-126 cmdEcgSendRaw stay deprecated - // and unexported. + // Filtered reading ("Labrador", R17). cmdLabradorDataGeneration, // 124 — R17 lifecycle cmdLabradorRawSave, // 125 — R17 lifecycle cmdLabradorFiltered, // 139 — R17 lifecycle diff --git a/lib/src/commands.dart b/lib/src/commands.dart index 314972d..d9cae8c 100644 --- a/lib/src/commands.dart +++ b/lib/src/commands.dart @@ -941,10 +941,8 @@ Uint8List cmdStopHaptics(int seq, {BandProfile profile = BandProfile.gen4}) => // 125 TOGGLE_LABRADOR_RAW_SAVE 00=disable 01=enable // 139 TOGGLE_LABRADOR_FILTERED 00=disable 01=enable // -// The old `cmdEcg*` builders below modelled 124 as a boolean arm/disarm. That -// is wrong at the byte level, not just in naming: `01 01` — what the old -// builder sent to "arm" — is the STOP operation, and `01 00` is not an -// operation the strap defines at all. +// The bytes matter: sending `01 01` to 124 as an "arm" flag is the STOP +// operation, and `01 00` is not an operation the strap defines at all. /// The operation byte of TOGGLE_LABRADOR_DATA_GENERATION (124) — the /// filtered-reading (R17) lifecycle. It is an operation selector, not a @@ -1015,47 +1013,3 @@ Uint8List cmdLabradorFiltered(int seq, bool on, buildCommand(seq, Cmd.toggleLabradorFiltered, [revision1, on ? 0x01 : 0x00], profile); -/// Arm / disarm an ECG reading (ECG main control, 0x7C) — `[0x01][0|1]`. -/// -/// **Wrong bytes, kept only for source compatibility.** Opcode 124 is -/// TOGGLE_LABRADOR_DATA_GENERATION and its second byte is an operation, not a -/// flag: `cmdEcgControl(seq, -/// true)` emits `01 01`, which is the **STOP** operation, and -/// `cmdEcgControl(seq, false)` emits `01 00`, which is not a defined -/// operation at all. Use [cmdLabradorDataGeneration]. -@Deprecated('opcode 124 takes an OPERATION byte, not a bool: this builder\'s ' - '`true` sends 01 01 = the STOP operation, and `false` sends the undefined ' - '01 00 — use cmdLabradorDataGeneration(seq, LabradorOperation.start)') -Uint8List cmdEcgControl(int seq, bool on, - {BandProfile profile = BandProfile.gen4}) => - buildCommand(seq, Cmd.toggleLabradorDataGeneration, - [revision1, on ? 0x01 : 0x00], profile); - -/// Start/stop the raw ECG trace (0x7E) — `[0x01][on]`. -/// -/// **Wrong opcode, kept only for source compatibility.** 0x7E (126) is not -/// an established WHOOP 5 opcode — origin unknown, most likely gen4 or -/// third-party lore. The raw save of a filtered -/// reading is TOGGLE_LABRADOR_RAW_SAVE (125): use [cmdLabradorRawSave], which -/// emits the identical `01 00` / `01 01` body on the opcode the strap -/// actually implements. -@Deprecated('opcode 126 (0x7E) is not an established WHOOP 5 opcode ' - '— the raw save is TOGGLE_LABRADOR_RAW_SAVE (125): use ' - 'cmdLabradorRawSave(seq, on)') -Uint8List cmdEcgSendRaw(int seq, bool on, - {BandProfile profile = BandProfile.gen4}) => - // ignore: deprecated_member_use_from_same_package - buildCommand( - seq, Cmd.ecgSendRawData, [revision1, on ? 0x01 : 0x00], profile); - -/// Start/stop the filtered ECG trace (0x8B) — `[0x01][on]`. -/// -/// Body-correct: this is TOGGLE_LABRADOR_FILTERED (139) under its old name, -/// and the bytes are unchanged. Renamed only, so [cmdLabradorFiltered] is a -/// drop-in replacement. -@Deprecated('renamed: opcode 139 is TOGGLE_LABRADOR_FILTERED and the trace is ' - 'a filtered reading, not an ECG — use cmdLabradorFiltered(seq, on) ' - '(identical bytes)') -Uint8List cmdEcgSendFiltered(int seq, bool on, - {BandProfile profile = BandProfile.gen4}) => - cmdLabradorFiltered(seq, on, profile: profile); diff --git a/lib/src/constants.dart b/lib/src/constants.dart index 2d11c14..71b31eb 100644 --- a/lib/src/constants.dart +++ b/lib/src/constants.dart @@ -163,30 +163,6 @@ class Cmd { static const int toggleLabradorDataGeneration = 0x7C; // 124 static const int toggleLabradorRawSave = 0x7D; // 125 static const int toggleLabradorFiltered = 0x8B; // 139 - // ── Deprecated "ECG" spellings of the three above ───────────────────────── - // The feature was previously modelled as an ECG arm/save/send matrix. The - // opcode NUMBERS 124/125/139 are right; the names and the arm/disarm mental - // model are not (see the operation table above). - @Deprecated('this is TOGGLE_LABRADOR_DATA_GENERATION (124): its body is an ' - 'operation byte (01=stop, 02=start, 03=restart), not an on/off flag — ' - 'use Cmd.toggleLabradorDataGeneration') - static const int ecgMainControl = 0x7C; - @Deprecated('this is TOGGLE_LABRADOR_RAW_SAVE (125), 01 00 / 01 01 — ' - 'use Cmd.toggleLabradorRawSave') - static const int ecgSaveRawData = 0x7D; - @Deprecated('opcode 126 (0x7E) is NOT an established WHOOP 5 opcode ' - '— origin unknown, most likely gen4 ' - 'or third-party lore. The raw trace is TOGGLE_LABRADOR_RAW_SAVE (125): ' - 'use Cmd.toggleLabradorRawSave') - static const int ecgSendRawData = 0x7E; - @Deprecated('opcode 127 (0x7F) is NOT an established WHOOP 5 opcode ' - '— origin unknown, most likely gen4 ' - 'or third-party lore. The filtered trace is TOGGLE_LABRADOR_FILTERED ' - '(139): use Cmd.toggleLabradorFiltered') - static const int ecgSaveFilteredData = 0x7F; - @Deprecated('this is TOGGLE_LABRADOR_FILTERED (139), 01 00 / 01 01 — ' - 'use Cmd.toggleLabradorFiltered') - static const int ecgSendFilteredData = 0x8B; // Signal-processing configuration. Advanced/diagnostic: it retunes the // on-strap algorithms, so it can change what the strap itself reports. static const int setSignalConfig = 0x8A; diff --git a/test/gen5_command_surface_test.dart b/test/gen5_command_surface_test.dart index a49a6ea..3c2e851 100644 --- a/test/gen5_command_surface_test.dart +++ b/test/gen5_command_surface_test.dart @@ -540,38 +540,6 @@ void main() { expect(enabled.inner.sublist(3, 5), [0x01, 0x01]); } }); - - // The deprecated builders keep their exact old bytes: existing callers are - // flagged, not silently re-pointed. Two of the three are wrong on the wire - // — that is what the deprecation says, and this pins it. - test('deprecated ECG builders still emit their old (wrong) bytes', () { - // ignore: deprecated_member_use_from_same_package - final armed = parseFrame(cmdEcgControl(1, true, profile: gen5), - profile: gen5)!; - expect(armed.opcode, 0x7C); - expect(armed.inner.sublist(3, 5), [0x01, 0x01], - reason: 'the old "arm" is the STOP operation'); - // ignore: deprecated_member_use_from_same_package - final disarmed = parseFrame(cmdEcgControl(1, false, profile: gen5), - profile: gen5)!; - expect(disarmed.inner.sublist(3, 5), [0x01, 0x00], - reason: 'the old "disarm" is an undefined operation byte'); - - // ignore: deprecated_member_use_from_same_package - final raw = - parseFrame(cmdEcgSendRaw(1, true, profile: gen5), profile: gen5)!; - expect(raw.opcode, 0x7E, - reason: '126 is not an established WHOOP opcode'); - expect(raw.inner.sublist(3, 5), [0x01, 0x01]); - - // 139 was only misnamed — same bytes as the replacement. - // ignore: deprecated_member_use_from_same_package - final filtered = cmdEcgSendFiltered(1, true, profile: gen5); - expect(filtered, cmdLabradorFiltered(1, true, profile: gen5)); - final f = parseFrame(filtered, profile: gen5)!; - expect(f.opcode, 0x8B); - expect(f.inner.sublist(3, 5), [0x01, 0x01]); - }); }); group('constants', () { From 0d56c90baa38c701b63dd32d694bf2e1c70ef6dc Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sat, 17 Oct 2026 23:05:54 +0530 Subject: [PATCH 23/33] drop the unused decodeBatch helper decodeBatch was exported but nothing called it, and it silently threw away every record decodeRecord refused without saying how many. every real ingest path decodes bytes through parseR24, so I removed it rather than keep untested public surface around. (cherry picked from commit 3d08a6149c806a2fe9bb5ccdf36474d0d3756948) --- lib/openstrap_protocol.dart | 3 +-- lib/src/live.dart | 10 ---------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/lib/openstrap_protocol.dart b/lib/openstrap_protocol.dart index 1da9bb9..d1ed937 100644 --- a/lib/openstrap_protocol.dart +++ b/lib/openstrap_protocol.dart @@ -91,8 +91,7 @@ export 'src/live.dart' realtimeRr, decodeR10Imu, decodeR11Raw, - decodeRecord, - decodeBatch; + decodeRecord; // Source 2 — CRC, constants, framing, commands. export 'src/crc.dart' show crc8, crc32, crc16Modbus; diff --git a/lib/src/live.dart b/lib/src/live.dart index 1d27894..0c1b03f 100644 --- a/lib/src/live.dart +++ b/lib/src/live.dart @@ -633,13 +633,3 @@ R11Raw? decodeR11Raw(String hex) { channelB: channel(236), ); } - -/// Decode a batch of hex records, returning all surfaceable samples. -List decodeBatch(List records) { - final out = []; - for (final hex in records) { - final s = decodeRecord(hex); - if (s != null) out.add(s); - } - return out; -} From bf203cfbbc251c838e2d0a380df55a32d1870636 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Sun, 18 Oct 2026 15:08:13 +0530 Subject: [PATCH 24/33] truncated-frame give-up could resume inside the frame's own header with giveUpTruncatedFrames on, the scan for a complete frame inside a short one started at offset 1. when the header's crc8 byte is 0xAA and a valid frame starts there, the payload slice ran backwards and feed() threw a RangeError. the scan now starts past the header. (cherry picked from commit 87a07e505bdc70ca71ab52cd51b3771db144cdd7) --- lib/src/framing.dart | 11 ++++++++--- test/framing_test.dart | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/src/framing.dart b/lib/src/framing.dart index 0011df4..8e7ab74 100644 --- a/lib/src/framing.dart +++ b/lib/src/framing.dart @@ -274,10 +274,15 @@ class FrameReassembler { return out; } - /// Offset of the first SOF after index 0 that starts a frame which is - /// fully buffered and passes both CRCs, or -1. + /// Offset of the first SOF after the pending frame's header that starts a + /// frame which is fully buffered and passes both CRCs, or -1. + /// + /// The scan starts past the header: that header already passed its check, + /// and its crc8 byte is 0xAA for about 1 in 256 lengths, so starting inside + /// it can match a frame at an offset shorter than the header and the caller's + /// payload slice runs backwards. int _completeFrameWithin(int headerLen) { - for (int k = 1; k + headerLen + 4 <= _buf.length; k++) { + for (int k = headerLen; k + headerLen + 4 <= _buf.length; k++) { if (_buf[k] != sof) continue; final header = _buf.sublist(k, k + headerLen); final declared = profile.declaredLen(header); diff --git a/test/framing_test.dart b/test/framing_test.dart index f2e0417..68b3763 100644 --- a/test/framing_test.dart +++ b/test/framing_test.dart @@ -244,6 +244,28 @@ void main() { expect(re.truncations, 0); }); + test('give-up does not resume inside the truncated frame\'s own header', + () { + // A length pair whose crc8 is itself 0xAA, so the header's last byte + // reads as an SOF and a valid frame can start there. + late int lo, hi; + search: + for (hi = 0; hi < 16; hi++) { + for (lo = 0; lo < 256; lo++) { + if ((lo | hi << 8) >= 64 && crc8([lo, hi]) == 0xAA) break search; + } + } + final inner = buildFrame([PacketType.event, 1, 0x05, 0, 0, 0, 0, 0]); + expect(inner.first, 0xAA); + final re = FrameReassembler(giveUpTruncatedFrames: true); + expect(re.feed([0xAA, lo, hi, ...inner]), isEmpty, + reason: 'nothing complete sits after the header, so this waits'); + final frames = re.feed(historyEnd(2)); + expect(frames.first.truncated, isTrue); + expect(frames.last.packetType, PacketType.metadata); + expect(re.truncations, 1); + }); + test('a frame with a corrupt payload is not reported as truncated', () { final data = buildFrame( [PacketType.historicalData, 1, 0x18, ...List.filled(93, 0x33)]); From d7a6ca34111fd60f8d3790ea59ace24a78dd08ba Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Thu, 29 Oct 2026 00:39:48 +0530 Subject: [PATCH 25/33] colmi: don't drop a pending reply on a continuation that looks like a header a continuation whose payload happened to start with bc and a known type threw away a valid pending reply. header-shaped chunks are appended now and their offsets remembered; the pending reply is given up only when it completes with a bad crc, or a later reply completes with a good one before it does. --- lib/src/colmi.dart | 72 ++++++++++++++++++++++++++++++++++++-------- test/colmi_test.dart | 13 ++++++++ 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/lib/src/colmi.dart b/lib/src/colmi.dart index 2567a38..0bd937a 100644 --- a/lib/src/colmi.dart +++ b/lib/src/colmi.dart @@ -144,14 +144,21 @@ List colmiTemperatureRequest([int mode = 6]) => /// null. A notification that does not start with 0xbc while nothing is /// pending is not a reply this decoder understands and is dropped. /// -/// A notification that starts a new reply (0xbc plus a known type) while one -/// is still pending means the pending reply lost a notification. It is -/// dropped and the new reply started instead: appending would fill the gap -/// with the next reply's header and records (a nap reply read as the last -/// night's stage/minute pairs) and lose the next reply entirely. +/// A notification that looks like the start of a new reply (0xbc plus a known +/// type) while one is still pending is ambiguous: either the pending reply +/// lost a notification and this is the next reply, or the pending reply's own +/// payload happens to start with those bytes. It is appended either way and +/// its offset remembered. The pending reply is given up for the one starting +/// there only when the evidence says so: the pending reply completes and fails +/// its CRC, or the later reply completes on its own with a valid CRC before +/// the pending one does. Restarting on the header shape alone discarded valid +/// replies whose payload resembled a header; appending blindly filled a lost +/// notification's gap with the next reply and lost that reply too. class ColmiBigDataReassembler { List? _pending; int _want = 0; + // Offsets inside [_pending] where a header-shaped notification began. + List _restarts = []; static bool _startsReply(List chunk) => chunk.length >= kColmiBigHeaderLength && @@ -161,21 +168,60 @@ class ColmiBigDataReassembler { chunk[1] == kColmiBigSpo2 || chunk[1] == kColmiBigTemperature); + static int _wantOf(List header) => + kColmiBigHeaderLength + (header[2] | (header[3] << 8)); + + void _start(List bytes, List restarts) { + _want = _wantOf(bytes); + _pending = [...bytes]; + _restarts = restarts; + } + + /// Restart from the header at [_restarts]`[idx]`, keeping later offsets. + void _restartAt(int idx) { + final p = _pending!; + final from = _restarts[idx]; + _start(p.sublist(from), + [for (final o in _restarts.skip(idx + 1)) o - from]); + } + List? add(List chunk) { - if (_pending != null && _startsReply(chunk)) _pending = null; if (_pending == null) { if (chunk.length < kColmiBigHeaderLength || chunk[0] != kColmiCmdBigData) { return null; } - _want = kColmiBigHeaderLength + (chunk[2] | (chunk[3] << 8)); - _pending = []; + _start(chunk, []); + } else { + if (_startsReply(chunk)) _restarts.add(_pending!.length); + _pending!.addAll(chunk); + } + while (true) { + final p = _pending!; + if (p.length >= _want) { + final whole = p.sublist(0, _want); + if (_restarts.isEmpty || colmiBigDataCrcOk(whole)) { + _pending = null; + _restarts = []; + return whole; + } + _restartAt(0); + continue; + } + // Still short: a later reply that is already whole and checks out means + // the pending one lost a notification and will never complete on its + // own bytes. + var done = -1; + for (var k = 0; k < _restarts.length && done < 0; k++) { + final o = _restarts[k]; + final w = _wantOf(p.sublist(o, o + kColmiBigHeaderLength)); + if (p.length - o >= w && colmiBigDataCrcOk(p.sublist(o, o + w))) { + done = k; + } + } + if (done < 0) return null; + _restartAt(done); } - _pending!.addAll(chunk); - if (_pending!.length < _want) return null; - final whole = _pending!.sublist(0, _want); - _pending = null; - return whole; } bool get pending => _pending != null; diff --git a/test/colmi_test.dart b/test/colmi_test.dart index c11926f..12e0939 100644 --- a/test/colmi_test.dart +++ b/test/colmi_test.dart @@ -101,6 +101,19 @@ void main() { expect(r.add(temp.sublist(20)), temp); }); + test('a continuation that looks like a header does not drop the reply', () { + // The second notification's payload bytes happen to read bc 27 len len, + // a sleep header. Restarting on that shape discarded a valid reply. + final payload = List.generate(40, (i) => i); + payload.setRange(14, 18, [0xbc, kColmiBigSleep, 0x05, 0x00]); + final whole = big(kColmiBigSleep, payload); + final r = ColmiBigDataReassembler(); + expect(r.add(whole.sublist(0, 20)), isNull); + expect(r.add(whole.sublist(20, 30)), isNull); + expect(r.add(whole.sublist(30)), whole); + expect(colmiBigDataCrcOk(whole), isTrue); + }); + test('a non-0xbc chunk with nothing pending is dropped', () { expect(ColmiBigDataReassembler().add([0x01, 2, 3, 4, 5, 6, 7]), isNull); }); From b49a288841624a43a1f5aaaf6fcdc5d50e1575e5 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Thu, 29 Oct 2026 00:39:49 +0530 Subject: [PATCH 26/33] garmin: reject a 10-byte varint with bits past 63 the 10th byte only carries bit 63. anything above it wrapped into a small value, so a malformed length could frame a valid-looking message. --- lib/src/garmin.dart | 5 +++++ test/garmin_test.dart | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/lib/src/garmin.dart b/lib/src/garmin.dart index a9b4187..79976e5 100644 --- a/lib/src/garmin.dart +++ b/lib/src/garmin.dart @@ -693,6 +693,11 @@ Iterable<_ProtoField> _protoFields(Uint8List data) sync* { // is garbage, so treat it as malformed rather than keep OR-ing. while (i < data.length && shift < 64) { final b = data[i++]; + // The 10th byte holds only bit 63; anything above it would wrap into a + // small value instead of being rejected. + if (shift == 63 && (b & 0x7e) != 0) { + throw const FormatException('overlong varint'); + } result |= (b & 0x7f) << shift; if (b & 0x80 == 0) return result; shift += 7; diff --git a/test/garmin_test.dart b/test/garmin_test.dart index eb4e80f..8d3bc0f 100644 --- a/test/garmin_test.dart +++ b/test/garmin_test.dart @@ -331,6 +331,17 @@ void main() { isNull); }); + test('a 10-byte varint with bits past 63 is rejected, not wrapped', () { + // The length 0x06 | (0x02 << 63) wraps to 6 in 64 bits and used to frame + // a valid-looking battery message out of a malformed length. + final inner = [8, 1, 16, 73]; + final service = [26, inner.length, ...inner]; + final len = [0x80 | service.length, ...List.filled(8, 0x80), 0x02]; + final smart = [66, ...len, ...service]; + expect(garminParseBatteryResponseProto(Uint8List.fromList(smart)), + isNull); + }); + test('random bytes never throw', () { // Fixed seed so a failure reproduces. var s = 0x2545f491; From c9f78a0dd89ca1af8c0ad8aebd04b22a500d7858 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Thu, 29 Oct 2026 00:39:49 +0530 Subject: [PATCH 27/33] battery level: read soc and millivolts as the u32s they are only the low half was read, so a body with a nonzero high half passed the range checks. --- lib/src/control.dart | 10 ++++++---- test/control_plane_offsets_test.dart | 10 ++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/src/control.dart b/lib/src/control.dart index 8b053c4..0a860cd 100644 --- a/lib/src/control.dart +++ b/lib/src/control.dart @@ -689,10 +689,10 @@ EventInfo? parseEvent( dec['double_tap'] = true; // no payload beyond event+timestamp, confirmed break; case EventId.batteryLevel: - // Byte-verified on a real BATTERY_LEVEL fixture: soc @ body[1] (u16, + // Byte-verified on a real BATTERY_LEVEL fixture: soc @ body[1] (u32, // DECI-percent — divide by 10; this is a DIFFERENT convention from // COMMAND_RESPONSE's GET_BATTERY_LEVEL, which is direct-percent on - // gen5. Don't conflate the two.), battery_mV @ body[5] (u16), + // gen5. Don't conflate the two.), battery_mV @ body[5] (u32), // charging @ body[9]. Shared across gen4/gen5 — these are // inner-relative offsets, identical across generations. // @@ -701,14 +701,16 @@ EventInfo? parseEvent( // packed pairs at [11:15] and [15:19]. `charging` used to be read from // [10], the byte that is always zero, so it was permanently false. if (body.length >= 12) { - final soc = _round(u16(body, 1) / 10.0, 1); + // Read as the u32s they are: a nonzero high half is a different + // layout, and reading only the low half let it pass the range gates. + final soc = _round(u32(body, 1) / 10.0, 1); // An out-of-range SoC means this body is not the layout being read, // so the other fields from it are not trusted either. A millivolt // reading outside a Li-ion cell's range is dropped on its own: hosts // bank it, and one garbage value can pin a lifetime MAX. if (soc.isFinite && soc >= 0.0 && soc <= 100.0) { dec['battery_pct'] = soc; - final mv = u16(body, 5); + final mv = u32(body, 5); if (mv >= 2500 && mv <= 4500) dec['battery_mv'] = mv; dec['charging'] = body[9] != 0; } diff --git a/test/control_plane_offsets_test.dart b/test/control_plane_offsets_test.dart index 9d9ac8a..4c4af65 100644 --- a/test/control_plane_offsets_test.dart +++ b/test/control_plane_offsets_test.dart @@ -478,6 +478,16 @@ void main() { expect(badMv.decoded['battery_pct'], closeTo(85.5, 1e-9)); expect(badMv.decoded.containsKey('battery_mv'), isFalse); expect(badMv.decoded['charging'], isTrue); + + // The fields are u32s: a nonzero high half is out of range even when the + // low half alone would pass. + final highSoc = parseEvent(envelopePacket( + 0x30, EventId.batteryLevel, body(soc: 0x10000 + 855, mv: 3900)))!; + expect(highSoc.decoded.containsKey('battery_pct'), isFalse); + final highMv = parseEvent(envelopePacket( + 0x30, EventId.batteryLevel, body(soc: 855, mv: 0x10000 + 3900)))!; + expect(highMv.decoded['battery_pct'], closeTo(85.5, 1e-9)); + expect(highMv.decoded.containsKey('battery_mv'), isFalse); }); }); From a1a7ce86e357c4bd52ac1f1bc0c2750a3fb34ed0 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Thu, 29 Oct 2026 03:31:42 +0530 Subject: [PATCH 28/33] reject non-finite values before the int check in the fit reader on the web double.infinity is int, so an infinite float in an integer or timestamp field could get through. --- lib/src/garmin_fit.dart | 7 ++++++- test/garmin_fit_test.dart | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/src/garmin_fit.dart b/lib/src/garmin_fit.dart index 6b7db3b..51c92bd 100644 --- a/lib/src/garmin_fit.dart +++ b/lib/src/garmin_fit.dart @@ -49,8 +49,11 @@ class FitMessage { // Inf get through. `.toInt()` on those throws UnsupportedError, which the // caller does not catch, and the download queue would stall on that file. // Such a sample is skipped instead. + // On the web `double.infinity is int` holds, so finiteness is checked + // before the int test too. int? int_(int field) { final v = fields[field]; + if (v is! num || !v.isFinite) return null; if (v is int) return v; if (v is double && v.isFinite && v == v.truncateToDouble()) return v.toInt(); return null; @@ -206,7 +209,9 @@ List parseFit(List bytes) { // queue on this file for good. int? _timeField(Map fields, int num) { final v = fields[num]; - if (v == null || v is int) return v as int?; + if (v == null) return null; + // `double.infinity is int` holds on the web. + if (v is int && v.isFinite) return v; throw FormatException('FIT: field $num is not an integer timestamp'); } diff --git a/test/garmin_fit_test.dart b/test/garmin_fit_test.dart index 920d2e9..346c04a 100644 --- a/test/garmin_fit_test.dart +++ b/test/garmin_fit_test.dart @@ -123,6 +123,7 @@ void main() { for (final f in [ fit(kFitMsgMonitoring, 253, 8, 0x86, [...u32(fitTs(t0)), ...u32(1)]), fit(kFitMsgMonitoring, 253, 4, 0x88, [0x00, 0x00, 0xc0, 0x7f]), + fit(kFitMsgMonitoring, 253, 4, 0x88, [0x00, 0x00, 0x80, 0x7f]), fit(kFitMsgMonitoring, 253, 4, 7, 'abc\x00'.codeUnits), fit(kFitMsgMonitoring, 26, 4, 0x84, [...u16(1), ...u16(2)]), ]) { @@ -150,6 +151,10 @@ void main() { expect(fitStress(fit(kFitMsgStressLevel, 0, bits)), isEmpty); expect(fitStress(fit(kFitMsgStressLevel, 1, bits)), isEmpty); } + // Directly, since on the web `double.infinity is int` holds. + for (final v in [double.infinity, double.negativeInfinity, double.nan]) { + expect(FitMessage(0, {1: v}, null).int_(1), isNull); + } // A float that holds a whole number still reads (72.0 = 0x42900000). expect(fitRestingHr(fit(kFitMsgMonitoringHrData, 1, [0, 0, 0x90, 0x42])), [(t0, 72)]); From 58eaf8769897f988f9a98d26f387dd56bd7860b8 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Thu, 29 Oct 2026 03:31:42 +0530 Subject: [PATCH 29/33] ack every re-sent pebble packet that was already taken after 0, 1, 2 with the acks for 1 and 2 lost, the re-sent 1 came back null, so it never got acked and the watch kept re-sending it. --- lib/src/pebble.dart | 26 +++++++++++++++++++++----- test/pebble_test.dart | 16 +++++++++++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/lib/src/pebble.dart b/lib/src/pebble.dart index a690d6d..2e9035a 100644 --- a/lib/src/pebble.dart +++ b/lib/src/pebble.dart @@ -85,11 +85,17 @@ class PebbleFrameReassembler { final List _buf = []; int? _lastSerial; + // Packets taken since the last reset, capped at [_repeatWindow]: a serial + // behind the last one only counts as a repeat if it was actually taken. + int _taken = 0; + static const int _repeatWindow = 16; + /// Drops any partial frame and forgets the serial sequence. Call it when /// the PPoGATT transport resets. void reset() { _buf.clear(); _lastSerial = null; + _taken = 0; } /// Feeds one whole PPoGATT data packet (header byte included). Only the @@ -100,18 +106,28 @@ class PebbleFrameReassembler { /// frame completes. Clearing on a gap instead would make a mid-frame /// payload the head of the buffer and misframe everything after it. /// - /// A repeat of the packet taken last returns an empty list, and the caller + /// A repeat of a packet already taken returns an empty list, and the caller /// must ack it again: the watch only re-sends a packet whose ack it never /// saw, and without a fresh ack it keeps re-sending until its retry limit - /// resets the transport. Returns null only for a packet that is not the - /// next serial and not that repeat; the caller does not ack it. + /// resets the transport. After 0, 1, 2 with the acks for 1 and 2 lost, the + /// re-sent 1 is such a repeat even though 2 was taken last. Returns null + /// only for a packet that has not been taken and is not the next serial; + /// the caller does not ack it. + /// + /// Serials are 5-bit, so "behind" and "ahead" share one space: anything up + /// to [_repeatWindow] - 1 behind the last taken serial (and actually taken + /// since the last reset) is a repeat, anything else is unseen. List<(int, Uint8List)>? addPacket(List packet) { if (packet.isEmpty) return null; final serial = (packet[0] >> 3) & 0x1f; final last = _lastSerial; - if (last != null && serial == last) return const []; - if (last != null && serial != (last + 1) & 0x1f) return null; + if (last != null) { + final behind = (last - serial) & 0x1f; + if (behind < _repeatWindow && behind < _taken) return const []; + if (serial != (last + 1) & 0x1f) return null; + } _lastSerial = serial; + if (_taken < _repeatWindow) _taken++; return add(packet.sublist(1)); } diff --git a/test/pebble_test.dart b/test/pebble_test.dart index 0147037..85d090d 100644 --- a/test/pebble_test.dart +++ b/test/pebble_test.dart @@ -71,12 +71,26 @@ void main() { final pc = pebblePpogattPackets(c, pa.length); // 0,1,2 arrive; the acks for 1 and 2 are lost, so the watch re-sends // from 1 onward, then the next frame. - final out = feed(r, [pa[0], pa[1], pa[2], pa[1], pa[2], pa[3], ...pc]); + for (final p in [pa[0], pa[1], pa[2]]) { + expect(r.addPacket(p), isEmpty); + } + // Both re-sends were already taken: acked (not null), not re-appended. + expect(r.addPacket(pa[1]), isEmpty); + expect(r.addPacket(pa[2]), isEmpty); + final out = feed(r, [pa[3], ...pc]); expect(out.map((f) => f.$2.length), [62, 32]); expect(out.first.$2, [2, 1, ...List.filled(60, 0x5a)]); expect(out.last.$2[1], 2); }); + test('a serial behind the last one is not acked unless it was taken', () { + final r = PebbleFrameReassembler(); + final pa = pebblePpogattPackets(a, 5); + expect(r.addPacket(pa[0]), isEmpty); + // Serial 4 was never taken in this session, so it is not a repeat. + expect(r.addPacket(pebblePpogattPackets(b, 4).first), isNull); + }); + test('a skipped packet is dropped until it is re-sent in order', () { final r = PebbleFrameReassembler(); final pa = pebblePpogattPackets(a, 0); From 887771f8f69f8ecbae4a68ccc4ecaed13aced988 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Thu, 29 Oct 2026 08:47:18 +0530 Subject: [PATCH 30/33] refuse FIT timestamps declared as floats a whole-number float32 timestamp passed the integer check on the web after rounding, which could move readings by tens of seconds. --- lib/src/garmin_fit.dart | 16 ++++++++++++---- test/garmin_fit_test.dart | 4 +++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/src/garmin_fit.dart b/lib/src/garmin_fit.dart index 51c92bd..d59d738 100644 --- a/lib/src/garmin_fit.dart +++ b/lib/src/garmin_fit.dart @@ -207,11 +207,17 @@ List parseFit(List bytes) { // String or double (NaN included). The caller only banks the file raw on a // FormatException, so any other error type here would stall the download // queue on this file for good. -int? _timeField(Map fields, int num) { +// +// The declared base type is checked too: on the web an integral double is an +// `int`, so a float32 timestamp would pass the value check after rounding to +// float precision (tens of seconds at current FIT dates). +int? _timeField(Map fields, List<_Field> definition, int num) { final v = fields[num]; if (v == null) return null; + final declaredFloat = definition + .any((f) => f.num == num && f.base < _bases.length && _bases[f.base].$3); // `double.infinity is int` holds on the web. - if (v is int && v.isFinite) return v; + if (!declaredFloat && v is int && v.isFinite) return v; throw FormatException('FIT: field $num is not an integer timestamp'); } @@ -243,10 +249,12 @@ int? _timeField(Map fields, int num) { i += f.size; } i += def.devBytes; - int? ts = _timeField(fields, 253); + int? ts = _timeField(fields, def.fields, 253); // timestamp_16 (monitoring field 26): low 16 bits relative to the last // full timestamp. - final ts16 = def.global == kFitMsgMonitoring ? _timeField(fields, 26) : null; + final ts16 = def.global == kFitMsgMonitoring + ? _timeField(fields, def.fields, 26) + : null; if (ts == null && ts16 != null && lastTs != null) { ts = lastTs + ((ts16 - (lastTs & 0xffff)) & 0xffff); } diff --git a/test/garmin_fit_test.dart b/test/garmin_fit_test.dart index 346c04a..b8844f8 100644 --- a/test/garmin_fit_test.dart +++ b/test/garmin_fit_test.dart @@ -119,10 +119,12 @@ void main() { ..define(0, global, [(field, size, base)]) ..data(0, bytes)) .build(); - // 253 declared as uint32[2], float32 NaN, and a string; 26 as uint16[2]. + // 253 declared as uint32[2], float32 NaN, a whole float32 (1e9), and a + // string; 26 as uint16[2]. for (final f in [ fit(kFitMsgMonitoring, 253, 8, 0x86, [...u32(fitTs(t0)), ...u32(1)]), fit(kFitMsgMonitoring, 253, 4, 0x88, [0x00, 0x00, 0xc0, 0x7f]), + fit(kFitMsgMonitoring, 253, 4, 0x88, [0x28, 0x6b, 0x6e, 0x4e]), fit(kFitMsgMonitoring, 253, 4, 0x88, [0x00, 0x00, 0x80, 0x7f]), fit(kFitMsgMonitoring, 253, 4, 7, 'abc\x00'.codeUnits), fit(kFitMsgMonitoring, 26, 4, 0x84, [...u16(1), ...u16(2)]), From 9155f995422b61ae66763f73ae83479ead938ec7 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Thu, 29 Oct 2026 10:10:50 +0530 Subject: [PATCH 31/33] garmin stress: drop a sample whose own time is unreadable a stress record with a broken time field fell back to the running timestamp, which can belong to another record, so the score was filed at the wrong time. --- lib/src/garmin_fit.dart | 9 +++++++-- test/garmin_fit_test.dart | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/src/garmin_fit.dart b/lib/src/garmin_fit.dart index d59d738..b16a6fc 100644 --- a/lib/src/garmin_fit.dart +++ b/lib/src/garmin_fit.dart @@ -360,10 +360,15 @@ List<(int, double)> fitRespiration(List m) => [ ]; /// The watch's stress score 0-100 (field 0); negative values (no reading, -/// activity) dropped. Its own time is field 1 when present. +/// activity) dropped. Its own time is field 1 when present. A field 1 that is +/// present but unreadable (NaN, a fraction, an array) drops the sample: the +/// running timestamp may belong to another record, so falling back to it +/// would file the score at the wrong time. List<(int, int)> fitStress(List m) => [ for (final x in m) - if (x.global == kFitMsgStressLevel && (x.int_(0) ?? -1) >= 0) + if (x.global == kFitMsgStressLevel && + (x.int_(0) ?? -1) >= 0 && + (!x.fields.containsKey(1) || x.int_(1) != null)) if ((x.int_(1) ?? x.timestamp) case final t?) (x.int_(1) != null ? t + kFitEpochOffset : t, x.int_(0)!), ]; diff --git a/test/garmin_fit_test.dart b/test/garmin_fit_test.dart index b8844f8..6c30329 100644 --- a/test/garmin_fit_test.dart +++ b/test/garmin_fit_test.dart @@ -161,4 +161,23 @@ void main() { expect(fitRestingHr(fit(kFitMsgMonitoringHrData, 1, [0, 0, 0x90, 0x42])), [(t0, 72)]); }); + + test('a stress sample whose own time is unreadable is dropped, not refiled', + () { + List fit(List<(int, int, int)> f, List bytes) => + parseFit((FitWriter() + ..define(0, kFitMsgStressLevel, f) + ..data(0, bytes)) + .build()); + // Field 1 NaN next to a good running timestamp and score. + expect( + fitStress(fit([(253, 4, 0x86), (0, 2, 0x83), (1, 4, 0x88)], + [...u32(fitTs(t0)), ...u16(40), 0x00, 0x00, 0xc0, 0x7f])), + isEmpty); + // No field 1 at all: the running timestamp is the sample's time. + expect( + fitStress(fit([(253, 4, 0x86), (0, 2, 0x83)], + [...u32(fitTs(t0)), ...u16(40)])), + [(t0, 40)]); + }); } From 17854ab7b23e768854e1710d9c2324cf8395b94f Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Thu, 29 Oct 2026 11:54:39 +0530 Subject: [PATCH 32/33] garmin stress: a sent-but-invalid or float time field also drops the sample the invalid value was indistinguishable from a field that was never sent, so it still fell back to the running timestamp, and a time declared as a float was read after rounding. messages now keep each field's declared type. --- lib/src/garmin_fit.dart | 44 ++++++++++++++++++++++++++++++--------- test/garmin_fit_test.dart | 15 +++++++++++++ 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/lib/src/garmin_fit.dart b/lib/src/garmin_fit.dart index b16a6fc..c93ec45 100644 --- a/lib/src/garmin_fit.dart +++ b/lib/src/garmin_fit.dart @@ -32,12 +32,25 @@ const int kFitMsgHrvStatusSummary = 370; /// One decoded data message: its global number, its fields by field number /// (scalars as num, arrays as List, strings as String; invalid values are -/// absent), and its timestamp in Unix seconds when it has one. +/// absent), and its timestamp in Unix seconds when it has one. [bases] is the +/// declared base type of every field in the message's definition, invalid or +/// not, so a reader can tell "not sent" from "sent but unusable". class FitMessage { final int global; final Map fields; final int? timestamp; - const FitMessage(this.global, this.fields, this.timestamp); + final Map bases; + const FitMessage(this.global, this.fields, this.timestamp, + {this.bases = const {}}); + + /// [int_] for a field the profile types as a date_time: null when the + /// field was declared as a float, since a float32 time is rounded by tens + /// of seconds at current dates. + int? time_(int field) { + final b = bases[field]; + if (b != null && b < _bases.length && _bases[b].$3) return null; + return int_(field); + } num? num_(int field) { final v = fields[field]; @@ -259,7 +272,12 @@ int? _timeField(Map fields, List<_Field> definition, int num) { ts = lastTs + ((ts16 - (lastTs & 0xffff)) & 0xffff); } final unix = (ts ?? lastTs) == null ? null : (ts ?? lastTs)! + kFitEpochOffset; - return (FitMessage(def.global, fields, unix), i, ts); + return ( + FitMessage(def.global, fields, unix, + bases: {for (final f in def.fields) f.num: f.base}), + i, + ts + ); } // ── typed views ───────────────────────────────────────────────────────── @@ -360,15 +378,21 @@ List<(int, double)> fitRespiration(List m) => [ ]; /// The watch's stress score 0-100 (field 0); negative values (no reading, -/// activity) dropped. Its own time is field 1 when present. A field 1 that is -/// present but unreadable (NaN, a fraction, an array) drops the sample: the -/// running timestamp may belong to another record, so falling back to it -/// would file the score at the wrong time. +/// activity) dropped. Its own time is field 1 when the definition carries +/// it. A field 1 that is declared but unusable (the invalid value, NaN, a +/// fraction, an array, a float) drops the sample: the running timestamp may +/// belong to another record, so falling back to it would file the score at +/// the wrong time. List<(int, int)> fitStress(List m) => [ for (final x in m) if (x.global == kFitMsgStressLevel && (x.int_(0) ?? -1) >= 0 && - (!x.fields.containsKey(1) || x.int_(1) != null)) - if ((x.int_(1) ?? x.timestamp) case final t?) - (x.int_(1) != null ? t + kFitEpochOffset : t, x.int_(0)!), + (!_declares(x, 1) || x.time_(1) != null)) + if ((x.time_(1) ?? x.timestamp) case final t?) + (x.time_(1) != null ? t + kFitEpochOffset : t, x.int_(0)!), ]; + +// A message built without a definition (bases empty) falls back to whether +// the field holds a value at all. +bool _declares(FitMessage x, int field) => + x.bases.isEmpty ? x.fields.containsKey(field) : x.bases.containsKey(field); diff --git a/test/garmin_fit_test.dart b/test/garmin_fit_test.dart index 6c30329..40aa772 100644 --- a/test/garmin_fit_test.dart +++ b/test/garmin_fit_test.dart @@ -174,6 +174,21 @@ void main() { fitStress(fit([(253, 4, 0x86), (0, 2, 0x83), (1, 4, 0x88)], [...u32(fitTs(t0)), ...u16(40), 0x00, 0x00, 0xc0, 0x7f])), isEmpty); + // Field 1 sent as the uint32 invalid value. + expect( + fitStress(fit([(253, 4, 0x86), (0, 2, 0x83), (1, 4, 0x86)], + [...u32(fitTs(t0)), ...u16(40), 0xff, 0xff, 0xff, 0xff])), + isEmpty); + // Field 1 declared float32 holding a whole number (1e9). + expect( + fitStress(fit([(253, 4, 0x86), (0, 2, 0x83), (1, 4, 0x88)], + [...u32(fitTs(t0)), ...u16(40), 0x28, 0x6b, 0x6e, 0x4e])), + isEmpty); + // A good field 1 is the sample's own time. + expect( + fitStress(fit([(253, 4, 0x86), (0, 2, 0x83), (1, 4, 0x86)], + [...u32(fitTs(t0)), ...u16(40), ...u32(fitTs(t0 + 60))])), + [(t0 + 60, 40)]); // No field 1 at all: the running timestamp is the sample's time. expect( fitStress(fit([(253, 4, 0x86), (0, 2, 0x83)], From 0d3cb738712cd0efb18c7e09593db5bbf02b434f Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Thu, 29 Oct 2026 13:51:21 +0530 Subject: [PATCH 33/33] colmi test: a header-shaped continuation that completes with a bad crc --- test/colmi_test.dart | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/colmi_test.dart b/test/colmi_test.dart index 12e0939..0c11fe4 100644 --- a/test/colmi_test.dart +++ b/test/colmi_test.dart @@ -114,6 +114,21 @@ void main() { expect(colmiBigDataCrcOk(whole), isTrue); }); + test('a header-shaped continuation that completes with a bad crc is kept', + () { + // Same embedded header, but this notification carries all 11 bytes the + // header claims, so it completes as a candidate reply and fails its crc. + // The pending reply still finishes on the last chunk. + final payload = List.generate(40, (i) => i); + payload.setRange(14, 18, [0xbc, kColmiBigSleep, 0x05, 0x00]); + final whole = big(kColmiBigSleep, payload); + expect(colmiBigDataCrcOk(whole.sublist(20, 31)), isFalse); + final r = ColmiBigDataReassembler(); + expect(r.add(whole.sublist(0, 20)), isNull); + expect(r.add(whole.sublist(20, 31)), isNull); + expect(r.add(whole.sublist(31)), whole); + }); + test('a non-0xbc chunk with nothing pending is dropped', () { expect(ColmiBigDataReassembler().add([0x01, 2, 3, 4, 5, 6, 7]), isNull); });