#103 probe: stop reporting silence, and one verb's refusal, as a verdict about the firmware

Packages/WhoopProtocol/Sources/WhoopProtocol/DeviceConfigReadProbe.swift

@@ -23,8 +23,16 @@ import Foundation
 /// (`Resources/whoop_protocol.json`, `CommandNumber`) names 121 `GET_DEVICE_CONFIG_VALUE` and 128
 /// `GET_FF_VALUE`, but a name in a table is not a served verb: opcode 96 (`enterHighFreqHistoricalMode`)
 /// is a standing example of a number the table carries that nothing in the wild sends. So the probe's
-/// PRIMARY deliverable is a clean verdict per verb — **answered**, **rejected as UNSUPPORTED**, or
-/// **silent** — and "both verbs are unimplemented" is a useful, publishable result, not a failure.
+/// PRIMARY deliverable is a clean verdict per verb — **answered**, **rejected as UNSUPPORTED**,
+/// **silent**, or **undecodable** — and "both verbs are unimplemented" is a useful, publishable result,
+/// not a failure.
+///
+/// Those four are not interchangeable, and the verdict never treats them as such. **Only UNSUPPORTED is
+/// the firmware's own answer**, so only UNSUPPORTED supports the sentence "this firmware does not serve
+/// that verb". A timeout is a fact about one run and not even proof a frame reached the strap —
+/// `BLEManager.send` returns without transmitting when there is no `cmdCharacteristic`, and again when
+/// the 5/MG allowlist does not carry the opcode — while an undecodable reply is affirmative evidence the
+/// strap DID transmit, which is the opposite of "not served". Both are reported as **unconfirmed**.
 ///
 /// Only if a verb answers does the probe go on to read values: first the sixteen key names NOOP already
 /// has (`Whoop5Config.enableR22Sequence` — their VALUES on a real strap have never been read, only
@@ -353,6 +361,9 @@ public struct DeviceConfigReadProbeReport: Equatable, Sendable {
     public private(set) var steps = 0
     /// Set once the walk stopped for a reason worth naming beyond "the plan ran out".
     public private(set) var stopReason: String?
+    /// Per-verb reply window, in seconds, recorded by `noteTimeout`. The verdict names the window a verb
+    /// went silent through instead of converting that silence into a claim about the firmware.
+    private var silenceWindow: [UInt8: Int] = [:]
 
     // MARK: Plan cursors
 
@@ -468,7 +479,12 @@ public struct DeviceConfigReadProbeReport: Equatable, Sendable {
 
     /// Record the strap answering nothing at all within the per-step window. The verb is marked silent,
     /// which retires it — one no-reply must not cost another twenty timeouts.
+    ///
+    /// The window is kept, not just printed, because the verdict has to be able to say how long nothing
+    /// came back for. Silence is not the firmware refusing; it does not even establish that a frame was
+    /// transmitted (see the header), so it can never be reported as what the firmware serves.
     public mutating func noteTimeout(for step: Step, seconds: Int) {
+        silenceWindow[step.opcode] = seconds
         setStatus(.silent, for: step.opcode)
         trace.append("\(DeviceConfigReadProbeReport.opcodeLabel(step.opcode)) key=\"\(step.key)\" → no COMMAND_RESPONSE within \(seconds)s")
     }
@@ -491,18 +507,37 @@ public struct DeviceConfigReadProbeReport: Equatable, Sendable {
             ? "GET_DEVICE_CONFIG_VALUE(121)" : "GET_FF_VALUE(128)"
     }
 
+    /// How one verb ended, worded to exactly the evidence behind it. See the file header for why the four
+    /// statuses are not interchangeable; the short version is that only `unsupported` is the firmware
+    /// answering, so only `unsupported` speaks for the firmware.
+    private func outcome(_ status: VerbStatus, for opcode: UInt8) -> String {
+        switch status {
+        case .untried:     return "not asked"
+        case .answered:    return "answered"
+        case .unsupported: return "refused by firmware (UNSUPPORTED)"
+        case .silent:
+            guard let seconds = silenceWindow[opcode] else { return "served no reply — unconfirmed" }
+            return "served no reply in \(seconds)s — unconfirmed"
+        case .undecodable: return "replied but the frame did not decode — unconfirmed"
+        }
+    }
+
     /// One-line summary of what the probe established.
+    ///
+    /// "not served by this firmware" is a claim about the firmware, so it is made in exactly one case: the
+    /// firmware refused BOTH verbs itself. Every other run in which nothing answered reports each verb
+    /// against the evidence that verb produced, because two timeouts, one refusal plus one timeout, and a
+    /// reply that failed to decode are three different findings that used to print as the same sentence.
     public var verdict: String {
         let answered = [featureFlagVerb, deviceConfigVerb].filter { $0 == .answered }.count
         if answered == 0 {
-            let both = "neither GET_FF_VALUE(128) nor GET_DEVICE_CONFIG_VALUE(121) is served by this firmware"
-            if featureFlagVerb == .unsupported || deviceConfigVerb == .unsupported {
-                return "\(both) — rejected as UNSUPPORTED"
-            }
-            if featureFlagVerb == .silent && deviceConfigVerb == .silent {
-                return "\(both) — no reply to either"
+            if featureFlagVerb == .unsupported && deviceConfigVerb == .unsupported {
+                return "neither GET_FF_VALUE(128) nor GET_DEVICE_CONFIG_VALUE(121) is served by this "
+                    + "firmware — rejected as UNSUPPORTED"
             }
-            return both
+            let ff = outcome(featureFlagVerb, for: DeviceConfigReadProbe.getFeatureFlagValueCmd)
+            let dc = outcome(deviceConfigVerb, for: DeviceConfigReadProbe.getDeviceConfigValueCmd)
+            return "no read verb answered — GET_FF_VALUE(128) \(ff); GET_DEVICE_CONFIG_VALUE(121) \(dc)"
         }
         let named = readings.filter { $0.value != nil }.count
         if named == 0 {

Packages/WhoopProtocol/Tests/WhoopProtocolTests/DeviceConfigReadProbeTests.swift

@@ -246,10 +246,16 @@ final class DeviceConfigReadProbeTests: XCTestCase {
         XCTAssertEqual(report.steps, 2)
         XCTAssertEqual(report.featureFlagVerb, .unsupported)
         XCTAssertEqual(report.deviceConfigVerb, .unsupported)
-        XCTAssertTrue(report.verdict.contains("rejected as UNSUPPORTED"))
+        // The one run that supports the strong sentence: the firmware itself refused both verbs.
+        XCTAssertEqual(report.verdict,
+                       "neither GET_FF_VALUE(128) nor GET_DEVICE_CONFIG_VALUE(121) is served by this "
+                       + "firmware — rejected as UNSUPPORTED")
         XCTAssertTrue(report.render().contains("neither GET_FF_VALUE(128) nor GET_DEVICE_CONFIG_VALUE(121)"))
     }
 
+    /// Two timeouts are two timeouts. `BLEManager.send` returns without transmitting when there is no
+    /// `cmdCharacteristic` and again when the 5/MG allowlist does not carry the opcode, so a run in which
+    /// nothing reached the strap must not print a claim about what the firmware serves.
     func testSilentVerbsEndTheProbeAndAreSaidPlainly() {
         var report = DeviceConfigReadProbeReport(family: .whoop5, knownFlagKeys: flagKeys,
                                                  candidateKeys: DeviceConfigReadProbe.oxygenCandidateKeys)
@@ -260,12 +266,69 @@ final class DeviceConfigReadProbeTests: XCTestCase {
         XCTAssertNil(report.nextStep())
         XCTAssertEqual(report.featureFlagVerb, .silent)
         XCTAssertEqual(report.deviceConfigVerb, .silent)
+        XCTAssertEqual(report.verdict,
+                       "no read verb answered — GET_FF_VALUE(128) served no reply in 8s — unconfirmed; "
+                       + "GET_DEVICE_CONFIG_VALUE(121) served no reply in 8s — unconfirmed")
         let text = report.render()
-        XCTAssertTrue(text.contains("no reply to either"))
+        XCTAssertFalse(text.contains("served by this firmware"),
+                       "silence is not the firmware answering — it is not even proof a frame was sent")
+        XCTAssertFalse(text.contains("UNSUPPORTED"), "nothing was refused; nothing replied at all")
         XCTAssertTrue(text.contains("no COMMAND_RESPONSE within 8s"))
         XCTAssertTrue(text.contains("(none — the verb that would carry them did not answer)"))
     }
 
+    /// One verb refused and the other timed out: the refusal belongs to the verb that was refused. The
+    /// old `||` printed "neither … is served by this firmware — rejected as UNSUPPORTED" over a 121 that
+    /// was never refused, only never heard from.
+    func testOneRefusalIsNotGeneralisedToTheVerbThatWasNeverHeardFrom() {
+        var report = DeviceConfigReadProbeReport(family: .whoop5, knownFlagKeys: flagKeys,
+                                                 candidateKeys: DeviceConfigReadProbe.oxygenCandidateKeys)
+        guard let s128 = report.nextStep() else { return XCTFail("128") }
+        XCTAssertEqual(s128.opcode, 128)
+        report.noteReply(.init(resultCode: 3, record: [0x00]), for: s128)
+        guard let s121 = report.nextStep() else { return XCTFail("121") }
+        XCTAssertEqual(s121.opcode, 121)
+        report.noteTimeout(for: s121, seconds: 8)
+
+        XCTAssertNil(report.nextStep())
+        XCTAssertEqual(report.featureFlagVerb, .unsupported)
+        XCTAssertEqual(report.deviceConfigVerb, .silent)
+        XCTAssertEqual(report.verdict,
+                       "no read verb answered — GET_FF_VALUE(128) refused by firmware (UNSUPPORTED); "
+                       + "GET_DEVICE_CONFIG_VALUE(121) served no reply in 8s — unconfirmed")
+        XCTAssertFalse(report.render().contains("neither GET_FF_VALUE(128) nor GET_DEVICE_CONFIG_VALUE(121)"),
+                       "one refusal does not speak for the other verb")
+    }
+
+    /// An undecodable reply is affirmative evidence the strap DID transmit, so "not served by this
+    /// firmware" states the opposite of what the run observed.
+    func testAnUndecodableReplyIsNotReportedAsUnserved() {
+        var report = DeviceConfigReadProbeReport(family: .whoop5, knownFlagKeys: flagKeys, candidateKeys: [])
+        guard let s128 = report.nextStep() else { return XCTFail("128") }
+        report.noteFailure(.crc, for: s128)
+        guard let s121 = report.nextStep() else { return XCTFail("121") }
+        report.noteFailure(.envelope, for: s121)
+
+        XCTAssertEqual(report.featureFlagVerb, .undecodable)
+        XCTAssertEqual(report.deviceConfigVerb, .undecodable)
+        XCTAssertEqual(report.verdict,
+                       "no read verb answered — "
+                       + "GET_FF_VALUE(128) replied but the frame did not decode — unconfirmed; "
+                       + "GET_DEVICE_CONFIG_VALUE(121) replied but the frame did not decode — unconfirmed")
+        XCTAssertFalse(report.render().contains("served by this firmware"))
+    }
+
+    /// A probe that ended before either verb went out says so, rather than reporting an empty run as a
+    /// finding about the firmware.
+    func testAProbeThatAskedNothingClaimsNothing() {
+        let report = DeviceConfigReadProbeReport(family: .whoop5, knownFlagKeys: flagKeys, candidateKeys: [])
+        XCTAssertEqual(report.featureFlagVerb, .untried)
+        XCTAssertEqual(report.deviceConfigVerb, .untried)
+        XCTAssertEqual(report.verdict,
+                       "no read verb answered — GET_FF_VALUE(128) not asked; "
+                       + "GET_DEVICE_CONFIG_VALUE(121) not asked")
+    }
+
     func testAnAnsweringFeatureFlagVerbWalksTheFifteenRemainingKnownFlags() {
         var report = DeviceConfigReadProbeReport(family: .whoop5, knownFlagKeys: flagKeys,
                                                  candidateKeys: DeviceConfigReadProbe.oxygenCandidateKeys)

android/app/src/main/java/com/noop/protocol/DeviceConfigReadProbe.kt

@@ -29,8 +29,15 @@ package com.noop.protocol
  * (`whoop_protocol.json`, [CommandNumber]) names 121 `GET_DEVICE_CONFIG_VALUE` and 128 `GET_FF_VALUE`,
  * but a name in a table is not a served verb: opcode 96 (`ENTER_HIGH_FREQ_HISTORICAL_MODE`) is a
  * standing example of a number the table carries that nothing in the wild sends. So the probe's PRIMARY
- * deliverable is a clean verdict per verb — **answered**, **rejected as UNSUPPORTED**, or **silent** —
- * and "both verbs are unimplemented" is a useful, publishable result, not a failure.
+ * deliverable is a clean verdict per verb — **answered**, **rejected as UNSUPPORTED**, **silent**, or
+ * **undecodable** — and "both verbs are unimplemented" is a useful, publishable result, not a failure.
+ *
+ * Those four are not interchangeable, and the verdict never treats them as such. **Only UNSUPPORTED is
+ * the firmware's own answer**, so only UNSUPPORTED supports the sentence "this firmware does not serve
+ * that verb". A timeout is a fact about one run and not even proof a frame reached the strap — the send
+ * path returns without transmitting when the command characteristic is missing, and again when the 5/MG
+ * allowlist does not carry the opcode — while an undecodable reply is affirmative evidence the strap DID
+ * transmit, which is the opposite of "not served". Both are reported as **unconfirmed**.
  *
  * Only if a verb answers does the probe go on to read values: first the sixteen key names NOOP already
  * has (their VALUES on a real strap have never been read, only written), then a short list of GUESSED
@@ -317,6 +324,12 @@ class DeviceConfigReadProbeReport(
     var stopReason: String? = null
         private set
 
+    /**
+     * Per-verb reply window, in seconds, recorded by [noteTimeout]. The verdict names the window a verb
+     * went silent through instead of converting that silence into a claim about the firmware.
+     */
+    private val silenceWindow = mutableMapOf<Int, Int>()
+
     private var phase = 0            // 0 discovery, 1 known flags, 2 candidates, 3 done
     private var cursor = 0
     /** `"opcode:key"` pairs already attempted, so discovery's key is not re-read in a later phase. */
@@ -437,8 +450,13 @@ class DeviceConfigReadProbeReport(
     /**
      * Record the strap answering nothing at all within the per-step window. The verb is marked silent,
      * which retires it — one no-reply must not cost another twenty timeouts.
+     *
+     * The window is kept, not just printed, because the verdict has to be able to say how long nothing
+     * came back for. Silence is not the firmware refusing; it does not even establish that a frame was
+     * transmitted (see the header), so it can never be reported as what the firmware serves.
      */
     fun noteTimeout(step: Step, seconds: Int) {
+        silenceWindow[step.opcode] = seconds
         setStatus(VerbStatus.SILENT, step.opcode)
         _trace.add("${opcodeLabel(step.opcode)} key=\"${step.key}\" → no COMMAND_RESPONSE within ${seconds}s")
     }
@@ -467,20 +485,40 @@ class DeviceConfigReadProbeReport(
             "GET_FF_VALUE(128)"
         }
 
-    /** One-line summary of what the probe established. */
+    /**
+     * How one verb ended, worded to exactly the evidence behind it. See the file header for why the four
+     * statuses are not interchangeable; the short version is that only UNSUPPORTED is the firmware
+     * answering, so only UNSUPPORTED speaks for the firmware.
+     */
+    private fun outcome(status: VerbStatus, opcode: Int): String = when (status) {
+        VerbStatus.UNTRIED -> "not asked"
+        VerbStatus.ANSWERED -> "answered"
+        VerbStatus.UNSUPPORTED -> "refused by firmware (UNSUPPORTED)"
+        VerbStatus.SILENT -> silenceWindow[opcode]
+            ?.let { "served no reply in ${it}s — unconfirmed" }
+            ?: "served no reply — unconfirmed"
+        VerbStatus.UNDECODABLE -> "replied but the frame did not decode — unconfirmed"
+    }
+
+    /**
+     * One-line summary of what the probe established.
+     *
+     * "not served by this firmware" is a claim about the firmware, so it is made in exactly one case: the
+     * firmware refused BOTH verbs itself. Every other run in which nothing answered reports each verb
+     * against the evidence that verb produced, because two timeouts, one refusal plus one timeout, and a
+     * reply that failed to decode are three different findings that used to print as the same sentence.
+     */
     val verdict: String
         get() {
             val answered = listOf(featureFlagVerb, deviceConfigVerb).count { it == VerbStatus.ANSWERED }
             if (answered == 0) {
-                val both =
-                    "neither GET_FF_VALUE(128) nor GET_DEVICE_CONFIG_VALUE(121) is served by this firmware"
-                if (featureFlagVerb == VerbStatus.UNSUPPORTED || deviceConfigVerb == VerbStatus.UNSUPPORTED) {
-                    return "$both — rejected as UNSUPPORTED"
-                }
-                if (featureFlagVerb == VerbStatus.SILENT && deviceConfigVerb == VerbStatus.SILENT) {
-                    return "$both — no reply to either"
+                if (featureFlagVerb == VerbStatus.UNSUPPORTED && deviceConfigVerb == VerbStatus.UNSUPPORTED) {
+                    return "neither GET_FF_VALUE(128) nor GET_DEVICE_CONFIG_VALUE(121) is served by this " +
+                        "firmware — rejected as UNSUPPORTED"
                 }
-                return both
+                val ff = outcome(featureFlagVerb, DeviceConfigReadProbe.GET_FEATURE_FLAG_VALUE_CMD)
+                val dc = outcome(deviceConfigVerb, DeviceConfigReadProbe.GET_DEVICE_CONFIG_VALUE_CMD)
+                return "no read verb answered — GET_FF_VALUE(128) $ff; GET_DEVICE_CONFIG_VALUE(121) $dc"
             }
             val named = _readings.count { it.value != null }
             if (named == 0) {