Publish the command result code on Apple, as Android already does (#894)

Packages/WhoopProtocol/Sources/WhoopProtocol/Interpreter.swift

@@ -771,6 +771,13 @@ private func decodeWhoop5CommandResponse(_ frame: [UInt8], fb: FieldBuilder, sch
     let name = schema.enumName("CommandNumber", respCmd)   // e.g. "GET_BATTERY_LEVEL(26)"
     let pay = Array(frame[11..<payloadEnd])
     fb.region(11, payloadEnd, "response payload", "cmd")
+    // Origin-seq echo + result code, the 4.0 offsets shifted by the usual +4 (#894). The Kotlin twin has
+    // always published both here; this decoder published neither. Bounded by `pay` so a short reply
+    // decodes nothing rather than reading the CRC32 trailer as a result.
+    if pay.count >= 1 { fb.add(11, 1, "resp_seq", "cmd", value: .int(Int(pay[0]))) }
+    if pay.count >= 2 {
+        fb.add(12, 1, "result", "cmd", value: .string(schema.enumName("CommandResult", Int(pay[1]))))
+    }
     if name.hasPrefix("GET_BATTERY_LEVEL"), pay.count >= 3 {
         // Direct percent at pay[2] (47% confirmed against the app) — the 4.0 deci-percent ÷10 is gone.
         fb.add(11 + 2, 1, "battery_pct", "battery", value: .double(Double(pay[2])), note: "%")

Packages/WhoopProtocol/Sources/WhoopProtocol/PostHooks.swift

@@ -161,6 +161,18 @@ func registerPostHooks() {
         guard 7 <= payEnd else { return }
         let pay = Array(frame[7..<payEnd])
         fb.region(7, length, "response payload", "cmd")
+        // The origin-seq echo and the result code (#894). Kotlin has published both on this family since
+        // #791 and on 5/MG since the port; Swift published neither, so an Apple strap log could not say
+        // whether a command the app sent succeeded — and every probe that needed the answer re-derived
+        // this byte privately (FeatureFlagProbe.resultLabel, DeviceConfigReadProbe, BodyLocationProbe).
+        //
+        // Read from `pay`, not the raw frame, so a response too short to carry them decodes nothing
+        // rather than reporting CRC32 bytes as a result code. Same offsets and the same
+        // `"NAME(raw)"` rendering as the Kotlin twin, so the two platforms' logs are comparable.
+        if pay.count >= 1 { fb.add(7, 1, "resp_seq", "cmd", value: .int(Int(pay[0]))) }
+        if pay.count >= 2 {
+            fb.add(8, 1, "result", "cmd", value: .string(schema.enumName("CommandResult", Int(pay[1]))))
+        }
         let cmd = frame.count > 6 ? Int(frame[6]) : nil
         let name = cmd.flatMap { schema.enums["CommandNumber"]?[String($0)] }
         switch name {

Packages/WhoopProtocol/Sources/WhoopProtocol/Resources/whoop_protocol.json

@@ -57,7 +57,8 @@
       "140": "SET_ADVERTISING_NAME", "141": "GET_ADVERTISING_NAME",
       "142": "START_FIRMWARE_LOAD_NEW", "143": "LOAD_FIRMWARE_DATA_NEW",
       "144": "PROCESS_FIRMWARE_IMAGE_NEW", "145": "GET_HELLO", "151": "GET_BATTERY_PACK_INFO",
-      "153": "TOGGLE_PERSISTENT_R20", "154": "TOGGLE_PERSISTENT_R21"}
+      "153": "TOGGLE_PERSISTENT_R20", "154": "TOGGLE_PERSISTENT_R21"},
+    "CommandResult": {"0": "FAILURE", "1": "SUCCESS", "2": "PENDING", "3": "UNSUPPORTED"}
   },
   "envelope": [
     {"off": 0, "len": 1, "name": "SOF", "cat": "frame"},

Packages/WhoopProtocol/Tests/WhoopProtocolTests/Resources/golden.json

@@ -3213,6 +3213,24 @@
 "note": null
 },
 {
+"off": 7,
+"len": 1,
+"name": "resp_seq",
+"cat": "cmd",
+"value": 0,
+"raw": "00",
+"note": null
+},
+{
+"off": 8,
+"len": 1,
+"name": "result",
+"cat": "cmd",
+"value": "FAILURE(0)",
+"raw": "00",
+"note": null
+},
+{
 "off": 15,
 "len": 4,
 "name": "crc32",
@@ -3225,7 +3243,9 @@
 "parsed": {
 "resp_cmd": "GET_BATTERY_LEVEL(26)",
 "response payload": "[8 bytes]",
-"battery_pct": 42.5
+"battery_pct": 42.5,
+"resp_seq": 0,
+"result": "FAILURE(0)"
 }
 },
 {

Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop4ResponseResultTests.swift

@@ -0,0 +1,128 @@
+import XCTest
+@testable import WhoopProtocol
+
+/// COMMAND_RESPONSE origin-seq echo + result code — the Swift twin of Android's `Whoop4ResponseResultTest`
+/// (#894, closing the gap #791 closed on the other platform).
+///
+/// Kotlin has published `resp_seq` and `result` on 5/MG since the port and on 4.0 since #791. Swift published
+/// neither, on either family: an Apple strap log named the command a response was for and then said nothing
+/// about whether it had succeeded. The consequence was not only cosmetic — every probe that needed the answer
+/// re-derived the byte privately (`FeatureFlagProbe.resultLabel`, `DeviceConfigReadProbe`,
+/// `BodyLocationProbe`'s "Result code @12"), three copies of one two-line decode.
+///
+/// The frames below are the **same real captures the Kotlin test uses**, byte for byte, so the two suites now
+/// assert the same decode of the same bytes — which is the only thing that actually holds two independent
+/// reimplementations together. They come from the #791 report (WHOOP 4.0, Galaxy S24 Ultra).
+final class Whoop4ResponseResultTests: XCTestCase {
+
+    private func bytes(_ s: String) -> [UInt8] {
+        var out = [UInt8](); var i = s.startIndex
+        while i < s.endIndex {
+            let j = s.index(i, offsetBy: 2)
+            out.append(UInt8(s[i..<j], radix: 16)!); i = j
+        }
+        return out
+    }
+
+    /// A `GET_DATA_RANGE` the strap actually answered: result `0x01` = SUCCESS, origin-seq echo `0x0A`.
+    private let answeredDataRangeHex =
+        "aa4c00a7247e220a01010000000050070100a307010050070100000000000000020035030000" +
+        "c2fc13008046394b00000000bbe1636aa02d0000bbe1636aa02d0000c6e4636ac07c000000000447ea04"
+
+    func testAnsweredDataRangeReportsSuccessAndItsOriginSeq() {
+        let f = parseFrame(bytes(answeredDataRangeHex))
+        XCTAssertEqual(f.crcOK, true)
+        XCTAssertEqual(f.parsed["resp_cmd"]?.stringValue, "GET_DATA_RANGE(34)")
+        XCTAssertEqual(f.parsed["resp_seq"]?.intValue, 0x0A)
+        XCTAssertEqual(f.parsed["result"]?.stringValue, "SUCCESS(1)")
+    }
+
+    /// The empty extended-battery reply at the heart of #791: acknowledged, correct echo, result FAILURE with
+    /// an all-zero payload. Without the result byte this is indistinguishable from a success carrying zeros —
+    /// a materially different conclusion, and the one the reporter had to hand-decode from raw hex.
+    private var emptyExtendedBatteryHex: String {
+        "aa2400fa2495622200" + String(repeating: "00", count: 27) + "0ac33306"
+    }
+
+    func testEmptyExtendedBatteryReplyReportsFailureNotEmptySuccess() {
+        let f = parseFrame(bytes(emptyExtendedBatteryHex))
+        XCTAssertEqual(f.crcOK, true)
+        XCTAssertEqual(f.parsed["resp_cmd"]?.stringValue, "GET_EXTENDED_BATTERY_INFO(98)")
+        XCTAssertEqual(f.parsed["resp_seq"]?.intValue, 0x22)
+        XCTAssertEqual(f.parsed["result"]?.stringValue, "FAILURE(0)")
+    }
+
+    /// The duplicate-write symptom the echo makes legible. One `GET_DATA_RANGE` send produced three CRC-valid
+    /// responses whose strap-side seq advanced (0x7E/0x7F/0x80) while the origin-seq echo stayed at 0x0A — so
+    /// the strap really did receive the command three times, visible in a log instead of by frame archaeology.
+    func testDuplicateResponsesShareOneOriginSeqAcrossAdvancingStrapSeq() {
+        let bodyAfterCmd = "0a01010000000050070100a307010050070100000000000000020035030000" +
+            "c2fc13008046394b00000000bbe1636aa02d0000bbe1636aa02d0000c6e4636ac07c000000000447ea04"
+        let echoes = ["7e", "7f", "80"].map { strapSeq -> Int? in
+            let f = parseFrame(bytes("aa4c00a724\(strapSeq)22" + bodyAfterCmd))
+            XCTAssertEqual(f.parsed["resp_cmd"]?.stringValue, "GET_DATA_RANGE(34)")
+            return f.parsed["resp_seq"]?.intValue
+        }
+        XCTAssertEqual(echoes, [0x0A, 0x0A, 0x0A] as [Int?])
+    }
+
+    /// Fails closed. A response whose declared payload stops before the result byte must decode neither field
+    /// rather than read the CRC32 trailer as a result code — the reason both fields are read from the bounded
+    /// payload slice and not from a raw frame offset.
+    func testShortResponseDecodesNoResultRatherThanReadingTheCrc() {
+        // Inner record is [type][seq][cmd] only: declared length 7, so the payload is empty and the four
+        // bytes at offsets 7..10 are the CRC32 trailer.
+        let inner: [UInt8] = [36, 0x11, 34]
+        var frame: [UInt8] = [0xAA, 7, 0, 0]
+        frame[3] = crc8(Array(frame[1..<3]))
+        frame += inner
+        let c32 = crc32(inner)
+        frame += [UInt8(c32 & 0xFF), UInt8((c32 >> 8) & 0xFF),
+                  UInt8((c32 >> 16) & 0xFF), UInt8((c32 >> 24) & 0xFF)]
+
+        let f = parseFrame(frame)
+        XCTAssertEqual(f.crcOK, true)
+        XCTAssertEqual(f.parsed["resp_cmd"]?.stringValue, "GET_DATA_RANGE(34)")
+        XCTAssertNil(f.parsed["resp_seq"])
+        XCTAssertNil(f.parsed["result"])
+    }
+
+    /// A counterexample to the result-byte reading itself, pinned rather than left to surprise someone.
+    ///
+    /// This is a real 4.0 `GET_BATTERY_LEVEL` reply carrying a valid 42.5% — it plainly succeeded — and its
+    /// `pay[1]` is `0x00`, which the #791 mapping renders `FAILURE(0)`. The parity corpus frame below and the
+    /// Kotlin `FramingTest` battery fixture (91.2%) both have that shape, so it is not a one-off.
+    ///
+    /// The #791 evidence for `pay[1] == result` was an answered `GET_DATA_RANGE` (`0x01`) against an empty
+    /// `GET_EXTENDED_BATTERY_INFO` (`0x00`), which those two frames fit and this one does not.
+    ///
+    /// The telling detail is that `resp_seq` is `0` here too — the whole two-byte prefix is zeroed, where
+    /// every other real capture in the tree (both families) carries a plausible non-zero echo. So the likely
+    /// reading is not that `0x00` means something other than FAILURE, but that a 4.0 battery reply does not
+    /// carry a `[seq][result]` prefix at all, its value simply sitting at a fixed `pay[2..4]`. Android has
+    /// rendered this same frame as FAILURE since #791, so the question is upstream of this change rather
+    /// than introduced by it — matching the twin is the point here. Tracked in #900; asserted meanwhile so
+    /// the behaviour is deliberate and a future correction has to come past this test.
+    func testSuccessfulBatteryReadStillReportsResultZero() {
+        let f = parseFrame(bytes("aa0f00c324141a0000a9010000000052cd1a49"))
+        XCTAssertEqual(f.crcOK, true)
+        XCTAssertEqual(f.parsed["battery_pct"]?.doubleValue, 42.5)   // it succeeded
+        XCTAssertEqual(f.parsed["resp_seq"]?.intValue, 0)            // echo zeroed too, not just the result
+        XCTAssertEqual(f.parsed["result"]?.stringValue, "FAILURE(0)")
+    }
+
+    /// An undocumented result code stays a number rather than becoming an invented name, and renders exactly
+    /// as the Kotlin twin's `hexLabel` does.
+    func testUnknownResultCodeRendersAsHexNotAName() {
+        let inner: [UInt8] = [36, 0x11, 34, 0x0A, 0x7F]
+        var frame: [UInt8] = [0xAA, 9, 0, 0]
+        frame[3] = crc8(Array(frame[1..<3]))
+        frame += inner
+        let c32 = crc32(inner)
+        frame += [UInt8(c32 & 0xFF), UInt8((c32 >> 8) & 0xFF),
+                  UInt8((c32 >> 16) & 0xFF), UInt8((c32 >> 24) & 0xFF)]
+
+        let f = parseFrame(frame)
+        XCTAssertEqual(f.parsed["result"]?.stringValue, "0x7F(127)")
+    }
+}

Packages/WhoopProtocol/Tests/WhoopProtocolTests/Whoop5CommandResponseTests.swift

@@ -82,15 +82,13 @@ final class Whoop5CommandResponseTests: XCTestCase {
     /// Decode only. No claim is made here about what the strap did with the commands — all three answered
     /// SUCCESS and produced no ECG data, which is the open question in #891, not something a test settles.
     ///
-    /// Asserts `resp_cmd` and the envelope, which is what this decoder exposes. The Kotlin twin also
-    /// asserts `resp_seq` and `result` on these same bytes, because the Kotlin decoder publishes both and
-    /// this one publishes neither — on either family. That gap is real and pre-existing (every Swift probe
-    /// re-derives the result byte itself rather than reading a parsed field), but it is the opposite
-    /// divergence from the one this change fixes, so it is filed rather than folded in here.
-    private let mgToggleReplies: [(hex: String, cmd: String)] = [
-        ("aa010c000100271124148b2f01010000845a1705", "TOGGLE_LABRADOR_FILTERED(139)"),
-        ("aa010c000100271124157d300101000067ab1b82", "TOGGLE_LABRADOR_RAW_SAVE(125)"),
-        ("aa010c000100271124167c3101010000ef4bcf45", "TOGGLE_LABRADOR_DATA_GENERATION(124)"),
+    /// Now asserts `resp_seq` and `result` alongside `resp_cmd`, matching the Kotlin twin field for field.
+    /// When these fixtures landed this decoder published neither, which is how that divergence was found;
+    /// #894 closed it, so the two suites finally agree on every field of the same bytes.
+    private let mgToggleReplies: [(hex: String, cmd: String, seq: Int)] = [
+        ("aa010c000100271124148b2f01010000845a1705", "TOGGLE_LABRADOR_FILTERED(139)", 47),
+        ("aa010c000100271124157d300101000067ab1b82", "TOGGLE_LABRADOR_RAW_SAVE(125)", 48),
+        ("aa010c000100271124167c3101010000ef4bcf45", "TOGGLE_LABRADOR_DATA_GENERATION(124)", 49),
     ]
 
     func testMgToggleRepliesDecode() {
@@ -99,9 +97,21 @@ final class Whoop5CommandResponseTests: XCTestCase {
             XCTAssertEqual(f.typeName, "COMMAND_RESPONSE", reply.cmd)
             XCTAssertEqual(f.crcOK, true, reply.cmd)
             XCTAssertEqual(f.parsed["resp_cmd"]?.stringValue, reply.cmd)
+            XCTAssertEqual(f.parsed["resp_seq"]?.intValue, reply.seq)
+            XCTAssertEqual(f.parsed["result"]?.stringValue, "SUCCESS(1)")
         }
     }
 
+    /// The battery reply already in this file, now also asserting the two new fields — it is the one real
+    /// 5/MG fixture whose value byte is independently known (47%), so it pins that publishing `resp_seq`
+    /// and `result` did not shift the payload offsets the value decode reads from.
+    func testBatteryReplyAlsoReportsSeqAndResult() {
+        let f = parseFrame(bytes(batteryHex), family: .whoop5)
+        XCTAssertEqual(f.parsed["resp_seq"]?.intValue, 2)
+        XCTAssertEqual(f.parsed["result"]?.stringValue, "SUCCESS(1)")
+        XCTAssertEqual(f.parsed["battery_pct"]?.doubleValue, 47)
+    }
+
     /// Byte 13 — the first response-payload byte, where `GET_BATTERY_LEVEL` returns its percent — is `0x01`
     /// in all three replies. It is deliberately NOT decoded into a field: whether it echoes the value that
     /// was written or reads back stored state is precisely what #891 cannot yet distinguish, and naming it