Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Packages/WhoopProtocol/Sources/WhoopProtocol/DataRange.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,19 @@ public enum DataRange {
guard behind >= 0, behind <= t else { return nil }
return behind
}

/// True when a GET_DATA_RANGE COMMAND_RESPONSE is the `PENDING(2)` acknowledgement rather than the
/// answer. The strap replies twice: a short PENDING ack, then the payload with `SUCCESS(1)`. Framing's
/// own result-code table already states it — "2=PENDING precedes SUCCESS on GET_DATA_RANGE
/// (hardware-confirmed, #78 fork)" — but `pagesBehind` had no way to tell the two apart, so the ack
/// (which carries no ring pointers by construction) decoded to nil and logged as a decode FAILURE once
/// per sync.
///
/// The result byte sits at `cmdOff + 2`, after the echoed opcode and the origin sequence. A frame too
/// short to hold one is not a PENDING ack, so it stays false and the caller keeps its existing
/// too-short handling.
public static func isPendingResponse(_ frame: [UInt8], cmdOff: Int) -> Bool {
guard cmdOff >= 0, cmdOff + 2 < frame.count else { return false }
return frame[cmdOff + 2] == 2
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,39 @@ final class DataRangeTests: XCTestCase {
"pagesBehind for \(h) at cmdOff \(cmdOff) should be \(expected)")
}
}

// MARK: - #689 PENDING ack. Byte-parity twin of the Kotlin DataRangeScanTest cases.
//
// GET_DATA_RANGE answers twice and only the second carries the ring pointers. Both frames below are
// real, captured on a WHOOP MG (WS50_r00, fw 50.39.1.0) on 2026-08-28, and they are one request's
// two replies.

/// The PENDING(2) ack: 20 bytes, result byte at cmdOff+2 is 0x02, no ring pointers in it at all.
func testIsPendingResponse_trueForPendingAck() {
XCTAssertTrue(DataRange.isPendingResponse(hex("aa010c000100271124e8220402000000c391bc3d"), cmdOff: 10))
}

/// The SUCCESS(1) answer to that same request. Must NOT be skipped.
func testIsPendingResponse_falseForSuccessAnswer() {
let h = "aa014c00010032d124e92204010100bf0000b4be0000c1be0000b4be00000c000000000002008f00"
XCTAssertFalse(DataRange.isPendingResponse(hex(h), cmdOff: 10))
}

/// The reason this pair matters: the ack decodes to nil, so before the guard it logged as a decode
/// failure once per sync while the real answer decoded fine two lines later.
func testPendingAckIsExactlyTheFramePagesBehindCannotDecode() {
let pending = hex("aa010c000100271124e8220402000000c391bc3d")
XCTAssertNil(DataRange.pagesBehind(from: pending, cmdOff: 10))
XCTAssertTrue(DataRange.isPendingResponse(pending, cmdOff: 10))
}

/// A frame too short to hold a result byte is not a PENDING ack; the caller keeps its own handling.
func testIsPendingResponse_falseWhenTooShortForResultByte() {
XCTAssertFalse(DataRange.isPendingResponse([UInt8](repeating: 0, count: 11), cmdOff: 10))
}

/// A negative cmdOff is rejected rather than indexing backwards.
func testIsPendingResponse_falseForNegativeCmdOff() {
XCTAssertFalse(DataRange.isPendingResponse([UInt8](repeating: 0, count: 40), cmdOff: -1))
}
}
18 changes: 13 additions & 5 deletions Strand/BLE/BLEManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6161,11 +6161,19 @@ extension BLEManager: @preconcurrency CBPeripheralDelegate {
// log was indistinguishable from one where the strap never answered, which is why a broken decode
// survived unnoticed. The raw-frame dump above is unconditional for the same reason. If a firmware
// revision ever shifts these fields again, the rejection must be visible rather than silent.
if let pages = DataRange.pagesBehind(from: frame, cmdOff: cmdOff) {
log("Strap backlog pages behind: \(pages) (#689 — GET_DATA_RANGE ring backlog, diagnostic only)")
} else {
log("Strap backlog pages behind: not decodable from this frame (#689 — offsets may have moved; "
+ "the raw frame above is the input). Diagnostic only, sync is unaffected.")
// ...but NOT on the PENDING(2) ack. GET_DATA_RANGE answers twice — a short ack, then the payload —
// which Framing's result-code table already records ("2=PENDING precedes SUCCESS on
// GET_DATA_RANGE"). The ack carries no ring pointers at all, so decoding it failed every time and
// reported a decode problem that did not exist: one "offsets may have moved" per sync, on healthy
// hardware, pointing the reader at an alignment bug rather than at the ack. Skip it here; the
// SUCCESS frame still logs its failure loudly, which is the case the paragraph above protects.
if !DataRange.isPendingResponse(frame, cmdOff: cmdOff) {
if let pages = DataRange.pagesBehind(from: frame, cmdOff: cmdOff) {
log("Strap backlog pages behind: \(pages) (#689 — GET_DATA_RANGE ring backlog, diagnostic only)")
} else {
log("Strap backlog pages behind: not decodable from this frame (#689 — offsets may have moved; "
+ "the raw frame above is the input). Diagnostic only, sync is unaffected.")
}
}
if let newest = BLEManager.dataRangeNewestUnix(from: frame) {
// #695: the sync-affecting side-effects (strapNewestTs, backfill window, LiveState range) only
Expand Down
25 changes: 17 additions & 8 deletions android/app/src/main/java/com/noop/ble/WhoopBleClient.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6640,14 +6640,23 @@ class WhoopBleClient(
// logged NOTHING — a strap log was indistinguishable from one where the strap never
// answered, which is why a broken decode survived unnoticed. The raw-frame dump above
// is unconditional for the same reason. Twin of the Swift branch.
val pagesBehind = com.noop.protocol.DataRange.pagesBehind(frame, cmdOff)
if (pagesBehind != null) {
log("Strap backlog pages behind: $pagesBehind (#689 — GET_DATA_RANGE ring backlog, diagnostic only)")
} else {
log(
"Strap backlog pages behind: not decodable from this frame (#689 — offsets may " +
"have moved; the raw frame above is the input). Diagnostic only, sync is unaffected.",
)
// ...but NOT on the PENDING(2) ack. GET_DATA_RANGE answers twice — a short ack, then
// the payload — which Framing's result-code table already records ("2=PENDING
// precedes SUCCESS on GET_DATA_RANGE"). The ack carries no ring pointers at all, so
// decoding it failed every time and reported a decode problem that did not exist:
// one "offsets may have moved" per sync on healthy hardware, pointing the reader at
// an alignment bug rather than at the ack. Skip it here; a SUCCESS frame that will
// not decode still logs loudly, which is the case the paragraph above protects.
if (!com.noop.protocol.DataRange.isPendingResponse(frame, cmdOff)) {
val pagesBehind = com.noop.protocol.DataRange.pagesBehind(frame, cmdOff)
if (pagesBehind != null) {
log("Strap backlog pages behind: $pagesBehind (#689 — GET_DATA_RANGE ring backlog, diagnostic only)")
} else {
log(
"Strap backlog pages behind: not decodable from this frame (#689 — offsets may " +
"have moved; the raw frame above is the input). Diagnostic only, sync is unaffected.",
)
}
}
dataRangeNewestUnix(frame)?.let {
strapNewestTs = it
Expand Down
17 changes: 17 additions & 0 deletions android/app/src/main/java/com/noop/protocol/DataRange.kt
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,21 @@ object DataRange {
if (behind < 0L || behind > t) return null
return behind
}

/**
* True when a GET_DATA_RANGE COMMAND_RESPONSE is the `PENDING(2)` acknowledgement rather than the
* answer. The strap replies twice: a short PENDING ack, then the payload with `SUCCESS(1)`. Framing's
* own result-code table already states it — "2=PENDING precedes SUCCESS on GET_DATA_RANGE
* (hardware-confirmed, #78 fork)" — but [pagesBehind] had no way to tell the two apart, so the ack
* (which carries no ring pointers by construction) decoded to null and logged as a decode FAILURE once
* per sync.
*
* The result byte sits at `cmdOff + 2`, after the echoed opcode and the origin sequence. A frame too
* short to hold one is not a PENDING ack, so it stays false and the caller keeps its existing
* too-short handling. Twin of the Swift `DataRange.isPendingResponse`.
*/
fun isPendingResponse(frame: ByteArray, cmdOff: Int): Boolean {
if (cmdOff < 0 || cmdOff + 2 >= frame.size) return false
return (frame[cmdOff + 2].toInt() and 0xFF) == 2
}
}
40 changes: 40 additions & 0 deletions android/app/src/test/java/com/noop/ble/DataRangeScanTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package com.noop.ble

import com.noop.protocol.DataRange
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test

/**
Expand Down Expand Up @@ -156,4 +158,42 @@ class DataRangeScanTest {
)
}
}

// #689 PENDING ack: GET_DATA_RANGE answers twice and only the second carries the ring pointers.
// Byte-parity twin of the Swift DataRangeTests cases. Both frames are real, captured on a WHOOP MG
// (WS50_r00, fw 50.39.1.0) on 2026-08-28; the pair below is one request's two replies.

/** The PENDING(2) ack: 20 bytes, result byte at cmdOff+2 is 0x02, no ring pointers in it at all. */
@Test fun `isPendingResponse true for the PENDING ack`() =
assertTrue(DataRange.isPendingResponse(hex("aa010c000100271124e8220402000000c391bc3d"), 10))

/** The SUCCESS(1) answer to that same request. Must NOT be skipped. */
@Test fun `isPendingResponse false for the SUCCESS answer`() =
assertFalse(
DataRange.isPendingResponse(
hex(
"aa014c00010032d124e92204010100bf0000b4be0000c1be0000b4be00000c00000000" +
"0002008f00",
),
10,
),
)

/**
* The reason this pair matters: the ack decodes to null, so before the guard it logged as a decode
* failure once per sync while the real answer decoded fine two lines later.
*/
@Test fun `the PENDING ack is exactly the frame pagesBehind cannot decode`() {
val pending = hex("aa010c000100271124e8220402000000c391bc3d")
assertNull(DataRange.pagesBehind(pending, 10))
assertTrue(DataRange.isPendingResponse(pending, 10))
}

/** A frame too short to hold a result byte is not a PENDING ack; the caller keeps its own handling. */
@Test fun `isPendingResponse false when too short for a result byte`() =
assertFalse(DataRange.isPendingResponse(ByteArray(11), 10))

/** A negative cmdOff is rejected rather than indexing backwards. */
@Test fun `isPendingResponse false for a negative cmdOff`() =
assertFalse(DataRange.isPendingResponse(ByteArray(40), -1))
}
Loading