From ad33f007d5eee0606cd5e510d91338ec17c95f61 Mon Sep 17 00:00:00 2001 From: Zebsi235 Date: Wed, 2 Sep 2026 18:37:04 +0200 Subject: [PATCH 1/4] (+) ecg: the pure research modules -- allow-list, stats and log, with tests Three self-contained Kotlin modules for the MG ECG research work, with no caller yet. Database-free, no Android framework types, no BLE, and fully unit-tested without a strap. - EcgResearchAllowList: a positive allow-list of the opcodes a research probe may send, as a pure predicate over raw opcode ints. Deliberately NOT wired to the send path in this change -- that is a separate, review-sensitive one - EcgResearchStats: windowed scoring of a probe run - EcgResearchLog: the capture record and its formatting Unused on purpose. Landing them alone keeps the reviewable surface pure logic that CI can verify end to end, and leaves the send-path question -- which widens what the command sender can express -- to its own change with its own argument. One guard is deliberately deferred rather than shipped here. The allow-list test originally asserted that every dispatchable opcode is a constructible CommandNumber. That is an invariant of the SEND PATH: send() forms a frame from a CommandNumber, so an allow-listed opcode the enum cannot express could not be dispatched at all. Opcodes 124/125/139 are absent from that enum on purpose -- upstream excluded them saying Android has no ECG app layer and sends none of them -- so asserting their presence here fails for the correct reason. The guard belongs with the wiring that makes it true. Its testable half, that no FORBIDDEN opcode is reachable through the probe cases, is kept and passes. The Swift twin is deliberately absent. Apple drives its ECG probe through WhoopCommand and its own layer; there is no macOS/iOS consumer for these three, so a twin now would be untested code with no caller on either platform. The parity contract applies when a consumer exists. --- .../com/noop/protocol/EcgResearchAllowList.kt | 110 ++++++ .../java/com/noop/protocol/EcgResearchLog.kt | 371 ++++++++++++++++++ .../com/noop/protocol/EcgResearchStats.kt | 247 ++++++++++++ .../noop/protocol/EcgResearchAllowListTest.kt | 100 +++++ .../com/noop/protocol/EcgResearchLogTest.kt | 191 +++++++++ .../com/noop/protocol/EcgResearchStatsTest.kt | 125 ++++++ 6 files changed, 1144 insertions(+) create mode 100644 android/app/src/main/java/com/noop/protocol/EcgResearchAllowList.kt create mode 100644 android/app/src/main/java/com/noop/protocol/EcgResearchLog.kt create mode 100644 android/app/src/main/java/com/noop/protocol/EcgResearchStats.kt create mode 100644 android/app/src/test/java/com/noop/protocol/EcgResearchAllowListTest.kt create mode 100644 android/app/src/test/java/com/noop/protocol/EcgResearchLogTest.kt create mode 100644 android/app/src/test/java/com/noop/protocol/EcgResearchStatsTest.kt diff --git a/android/app/src/main/java/com/noop/protocol/EcgResearchAllowList.kt b/android/app/src/main/java/com/noop/protocol/EcgResearchAllowList.kt new file mode 100644 index 0000000000..9b34098810 --- /dev/null +++ b/android/app/src/main/java/com/noop/protocol/EcgResearchAllowList.kt @@ -0,0 +1,110 @@ +package com.noop.protocol + +/** + * #891 / #1100: the ALLOW-LIST that bounds what the MG ECG Research panel may put on the wire, and the + * named DENY-LIST of opcodes it must never be able to form. + * + * ## Why an allow-list, and why here + * + * The research panel exists to gather protocol evidence about the MG's ECG ("Labrador") subsystem — NOT to + * sweep the command space. The single most dangerous thing a hand-run ECG probe could do is wander into a + * neighbouring opcode: `TOGGLE_LABRADOR_FILTERED` is **139 (0x8B)**, and three codes above it sit + * **142/143/144 — `START_FIRMWARE_LOAD_NEW` / `LOAD_FIRMWARE_DATA_NEW` / `PROCESS_FIRMWARE_IMAGE_NEW`**, the + * destructive firmware-load family (`docs/PROTOCOL.md` §9.1). So this file states the *entire* set of + * opcodes the panel can dispatch as a literal, testable predicate, rather than trusting a UI never to build + * anything else. + * + * This mirrors the discipline the rest of the 5/MG send path already follows — `DeviceConfigWriteGate` + * (key-aware SET allow-list), `DeviceConfigReadProbe.isReadOnlyOpcode`, `FeatureFlagWriteGate`. Like those, + * it is a pure predicate the real BLE send path consults ([WhoopBleClient.ecgSendAdmitted] calls + * [isProbeOpcode]), so a unit test proving this rejects an opcode is proving it about the wire path and not + * a parallel copy of the rule. + * + * Pure: no Android, no I/O. Twin-free — Android is the platform growing an ECG *app* layer first (the Apple + * probe drives the same four opcodes but expresses its allow-list inline in `BLEManager.send`). + */ +object EcgResearchAllowList { + + // ---- The four ECG ("Labrador") probe opcodes — the ONLY commands the panel dispatches ---------- + + /** + * The four opcodes the panel may send, each a documented WORKING HYPOTHESIS (not a confirmed mapping — + * see `docs/PROTOCOL.md` §9.1). Every one is a plain toggle/config write with a `[revision, arg]` + * payload; none loads firmware, wipes flash, or resets a bond. Kept in [Whoop5Ecg] so the numbers live + * with the command builders and this set can never drift from what [Whoop5Ecg.commandFrame] forms. + */ + val PROBE_OPCODES: Set = setOf( + Whoop5Ecg.SELECT_WRIST_CMD, // 123 (0x7B) — PERSISTENT wrist config (own action) + Whoop5Ecg.TOGGLE_REALTIME_FILTERED_ECG_CMD, // 139 (0x8B) — live filtered stream toggle + Whoop5Ecg.TOGGLE_SAVE_RAW_ECG_CMD, // 125 (0x7D) — persist-to-flash toggle + Whoop5Ecg.MAIN_CONTROL_ECG_DATA_GENERATION_CMD, // 124 (0x7C) — start/stop generation + ) + + /** + * The two DEVICE-CONFIG opcodes the panel's persistent `enable_raw_data_w_ecg` gate uses — a WRITE + * (119) and its mandatory read-back (121). These are NOT dispatched as ECG commands; they go through + * the pre-existing, key-aware [DeviceConfigWriteGate] path ([WhoopBleClient.setEcgRawDataGate]). Named + * here only so a reader sees the panel's complete wire surface in one place. + */ + val GATE_OPCODES: Set = setOf( + DeviceConfigWriteGate.SET_DEVICE_CONFIG_VALUE_CMD, // 119 (0x77) + DeviceConfigWriteGate.GET_DEVICE_CONFIG_VALUE_CMD, // 121 (0x79) + ) + + /** True only for one of the four ECG probe opcodes. The predicate the BLE send allow-list consults. */ + fun isProbeOpcode(opcode: Int): Boolean = opcode in PROBE_OPCODES + + /** True for the two device-config gate opcodes (write + read-back). */ + fun isGateOpcode(opcode: Int): Boolean = opcode in GATE_OPCODES + + /** The complete set of opcodes reachable from the panel, ECG toggles plus the gate's two verbs. */ + val DISPATCHABLE_OPCODES: Set = PROBE_OPCODES + GATE_OPCODES + + // ---- The deny-list: opcodes this panel must NEVER be able to form (Phase 11) ------------------- + + /** + * Named dangerous / persistent-state / recovery-defeating opcodes the ECG research panel must never + * dispatch. This is defence-in-depth documentation, NOT the enforcement mechanism — enforcement is the + * positive allow-list ([isProbeOpcode]); nothing here can be sent because it is not in [PROBE_OPCODES], + * and most are not even in the curated [CommandNumber] SENDER enum so no builder can express them. + * + * Listed (rather than merely omitted) so a unit test can assert every one of them is refused by + * [isProbeOpcode] and is absent from [DISPATCHABLE_OPCODES] — the "no opcode sweep, no DFU" guarantee + * made checkable. Values are the schema numbers from [CommandNames]. + */ + val FORBIDDEN: Map = mapOf( + // Firmware load / DFU — the family that sits just above 139 in opcode space, and the reason an + // ECG sweep is dangerous. NOOP's curated CommandNumber enum omits all of these entirely. + 36 to "START_FIRMWARE_LOAD", + 37 to "LOAD_FIRMWARE_DATA", + 38 to "PROCESS_FIRMWARE_IMAGE", + 45 to "ENTER_BLE_DFU", + 83 to "VERIFY_FIRMWARE_IMAGE", + 142 to "START_FIRMWARE_LOAD_NEW", + 143 to "LOAD_FIRMWARE_DATA_NEW", + 144 to "PROCESS_FIRMWARE_IMAGE_NEW", + // Data-wipe / fuel-gauge / factory-adjacent. + 25 to "FORCE_TRIM", + 99 to "RESET_FUEL_GAUGE", + 100 to "CALIBRATE_CAPSENSE", + // Reboot / power-cycle — recovery is via the app's ordinary reconnect, never a strap reset from + // this panel. + 29 to "REBOOT_STRAP", + 32 to "POWER_CYCLE_STRAP", + // High-frequency sync entry — an uncertain, battery-heavy mode NOOP deliberately never enters + // (docs/PROTOCOL.md note on 96); the panel does not offer it. + 96 to "ENTER_HIGH_FREQ_SYNC", + 97 to "EXIT_HIGH_FREQ_SYNC", + // Optical / AFE / bias front-end writes — persistent sensor calibration, out of scope for ECG. + 39 to "SET_LED_DRIVE", + 41 to "SET_TIA_GAIN", + 43 to "SET_BIAS_OFFSET", + 61 to "SET_AFE_PARAMETERS", + ) + + /** True if [opcode] is a named-forbidden opcode. Used by the census test; never a dispatch input. */ + fun isForbidden(opcode: Int): Boolean = FORBIDDEN.containsKey(opcode) + + /** Schema label for any opcode, e.g. `"TOGGLE_LABRADOR_FILTERED(139)"`, for logs/reports. */ + fun label(opcode: Int): String = CommandNames.label(opcode) +} diff --git a/android/app/src/main/java/com/noop/protocol/EcgResearchLog.kt b/android/app/src/main/java/com/noop/protocol/EcgResearchLog.kt new file mode 100644 index 0000000000..d812c4e763 --- /dev/null +++ b/android/app/src/main/java/com/noop/protocol/EcgResearchLog.kt @@ -0,0 +1,371 @@ +package com.noop.protocol + +/** + * #891 / #1100: the PURE in-memory record of one MG ECG research capture — every TX/RX/command/marker row, + * plus the serializers that turn them into the export bundle's CSV/JSON/README files. + * + * No Android, no I/O, no clock of its own: the caller ([com.noop.ble.EcgResearchSession]) supplies both a + * monotonic and a wall-clock timestamp for every row, so the whole model is deterministic and unit-tested + * without a strap or a device. The Android wrapper owns the file writing and the clocks; this owns the + * shape of the evidence. + * + * Volume: an ECG capture must NOT sample away high-rate packets (#891), so every inbound frame is kept. + * Appends are O(1) (a row holds raw bytes, hex-encoded only at serialize time) so they never block the BLE + * callback thread. A safety cap ([maxRxRows]) bounds worst-case memory; frames past it are COUNTED + * ([rxOverflow]) and reported in `stats.json`, never silently dropped. + * + * Not medical: this records protocol bytes and user-set physical markers. It never records or derives a + * clinical interpretation — the on-strap rhythm classifier byte is decoded elsewhere and never surfaced as + * a finding. + */ +class EcgResearchLog( + val sessionId: String, + val startWallMs: Long, + val startMonotonicMs: Long, + private val maxRxRows: Int = DEFAULT_MAX_RX_ROWS, + private val maxRxBytes: Long = DEFAULT_MAX_RX_BYTES, +) { + companion object { + /** ~ enough for many minutes of a high-rate stream; past this, frames are counted not stored. */ + const val DEFAULT_MAX_RX_ROWS = 200_000 + + /** + * Bytes of inbound payload a capture will hold. A row cap alone does not bound memory: 200,000 rows + * of 240-byte records is ~48 MB of payload before object overhead, and an offload can deliver frames + * far faster than a UI can notice, so a capture left running through a long sync could push the app + * to an OOM. Whichever cap is reached first stops storage; the drops are counted either way. + */ + const val DEFAULT_MAX_RX_BYTES = 24L * 1024 * 1024 + + /** #891 flagged a repeated 1,584-byte record as a candidate raw ECG block. */ + const val CANDIDATE_RAW_RECORD_SIZE = 1584 + + /** Samples `waveform.csv` will emit before truncating — and it SAYS it truncated, in the file. */ + const val WAVEFORM_CSV_SAMPLE_CAP = 60_000 + } + + // ---- Row types --------------------------------------------------------------------------------- + + /** An experimental command written to the strap (Phase 4 TX fields). */ + data class TxRow( + val monotonicMs: Long, + val wallMs: Long, + val opcode: Int, + val payload: ByteArray, + val frame: ByteArray, + val serviceUuid: String, + val characteristicUuid: String, + val writeType: String, + ) + + /** An inbound BLE notification/read during the capture (Phase 4 RX fields). */ + data class RxRow( + val monotonicMs: Long, + val wallMs: Long, + val serviceUuid: String, + val characteristicUuid: String, + val payload: ByteArray, + val packetType: Int?, + val sequence: Int?, + val recognized: Boolean, + val rejected: Boolean, + /** The frame's CRC verdict, or null when there was nothing to check (the house `crcOk != false` + * idiom). A CRC FAILURE is recorded, not dropped: for a research capture a corrupt frame is + * evidence, and dropping it would make "the strap sent nothing" indistinguishable from "we threw it + * away". No decoder reads a row whose verdict is false. */ + val crcOk: Boolean? = null, + ) + + /** The result of one allow-listed experimental command: outcome + timing + raw ack. */ + data class CommandRow( + val monotonicMs: Long, + val wallMs: Long, + val opcode: Int, + val argHex: String, + val statusLevel: String, // Confirmed / Observed / Hypothesized + val persistence: String, // persistent / session + val ackResult: String?, // SUCCESS(1) etc. + val ackRawHex: String?, + val elapsedToAckMs: Long?, + ) + + /** A user marker or connection event (Phase 5). */ + data class EventRow( + val monotonicMs: Long, + val wallMs: Long, + val kind: String, + val detail: String, + ) + + /** A raw write-callback status (Phase 4 write callback status). */ + data class WriteStatusRow(val monotonicMs: Long, val wallMs: Long, val characteristicUuid: String, val status: Int) + + // ---- Storage ----------------------------------------------------------------------------------- + + private val lock = Any() + private val _tx = ArrayList() + private val _rx = ArrayList() + private val _commands = ArrayList() + private val _events = ArrayList() + private val _writeStatus = ArrayList() + + /** Inbound frames dropped by the [maxRxRows]/[maxRxBytes] safety caps (reported, never silent). */ + var rxOverflow: Long = 0L + private set + + /** Stored inbound payload bytes, against [maxRxBytes]. */ + var rxBytes: Long = 0L + private set + + val txRows: List get() = synchronized(lock) { _tx.toList() } + val rxRows: List get() = synchronized(lock) { _rx.toList() } + val commandRows: List get() = synchronized(lock) { _commands.toList() } + val eventRows: List get() = synchronized(lock) { _events.toList() } + + val txCount: Int get() = synchronized(lock) { _tx.size } + val rxCount: Int get() = synchronized(lock) { _rx.size } + val commandCount: Int get() = synchronized(lock) { _commands.size } + val eventCount: Int get() = synchronized(lock) { _events.size } + + // ---- Recording --------------------------------------------------------------------------------- + + fun recordTx(row: TxRow) = synchronized(lock) { _tx.add(row) } + + fun recordRx(row: RxRow) = synchronized(lock) { + if (_rx.size >= maxRxRows || rxBytes + row.payload.size > maxRxBytes) { rxOverflow++; return@synchronized } + _rx.add(row) + rxBytes += row.payload.size + } + + fun recordCommand(row: CommandRow) = synchronized(lock) { _commands.add(row) } + + fun recordEvent(row: EventRow) = synchronized(lock) { _events.add(row) } + + fun recordWriteStatus(row: WriteStatusRow) = synchronized(lock) { _writeStatus.add(row) } + + // ---- Derived stats input ----------------------------------------------------------------------- + + private fun rxRecords(): List = synchronized(lock) { + _rx.map { + EcgResearchStats.RxRecord( + monotonicMs = it.monotonicMs, + characteristic = shortUuid(it.characteristicUuid), + payload = it.payload, + packetType = it.packetType, + recognized = it.recognized, + ) + } + } + + // ---- Serializers ------------------------------------------------------------------------------- + + fun eventsCsv(): String { + val sb = StringBuilder("monotonic_ms,wall_iso,kind,detail\n") + for (e in eventRows) { + sb.append(e.monotonicMs).append(',').append(iso(e.wallMs)).append(',') + .append(csv(e.kind)).append(',').append(csv(e.detail)).append('\n') + } + return sb.toString() + } + + fun bleTxCsv(): String { + val sb = StringBuilder( + "monotonic_ms,wall_iso,opcode_dec,opcode_hex,payload_hex,frame_hex,service_uuid,char_uuid,write_type\n", + ) + for (t in txRows) { + sb.append(t.monotonicMs).append(',').append(iso(t.wallMs)).append(',') + .append(t.opcode).append(',').append(hex2(t.opcode)).append(',') + .append(hex(t.payload)).append(',').append(hex(t.frame)).append(',') + .append(csv(t.serviceUuid)).append(',').append(csv(t.characteristicUuid)).append(',') + .append(csv(t.writeType)).append('\n') + } + return sb.toString() + } + + fun bleRxCsv(): String { + val sb = StringBuilder( + "monotonic_ms,wall_iso,service_uuid,char_uuid,len,packet_type,sequence,crc_ok,recognized,rejected,payload_hex\n", + ) + for (r in rxRows) { + sb.append(r.monotonicMs).append(',').append(iso(r.wallMs)).append(',') + .append(csv(r.serviceUuid)).append(',').append(csv(r.characteristicUuid)).append(',') + .append(r.payload.size).append(',') + .append(r.packetType?.toString() ?: "").append(',') + .append(r.sequence?.toString() ?: "").append(',') + .append(r.crcOk?.toString() ?: "").append(',') + .append(r.recognized).append(',').append(r.rejected).append(',') + .append(hex(r.payload)).append('\n') + } + return sb.toString() + } + + fun commandsCsv(): String { + val sb = StringBuilder( + "monotonic_ms,wall_iso,opcode_dec,opcode_hex,command,arg_hex,status_level,persistence,ack_result,elapsed_ms,ack_raw_hex\n", + ) + for (c in commandRows) { + sb.append(c.monotonicMs).append(',').append(iso(c.wallMs)).append(',') + .append(c.opcode).append(',').append(hex2(c.opcode)).append(',') + .append(csv(CommandNames.label(c.opcode))).append(',').append(csv(c.argHex)).append(',') + .append(csv(c.statusLevel)).append(',').append(csv(c.persistence)).append(',') + .append(csv(c.ackResult ?: "")).append(',') + .append(c.elapsedToAckMs?.toString() ?: "").append(',') + .append(csv(c.ackRawHex ?: "")).append('\n') + } + return sb.toString() + } + + /** Only the RX rows the framing parser did NOT recognise — the unknown-packet corpus (Phase 6/7). */ + fun unknownPacketsCsv(): String { + val sb = StringBuilder("monotonic_ms,wall_iso,char_uuid,len,packet_type,crc_ok,payload_hex\n") + for (r in rxRows) { + if (r.recognized) continue + sb.append(r.monotonicMs).append(',').append(iso(r.wallMs)).append(',') + .append(csv(r.characteristicUuid)).append(',').append(r.payload.size).append(',') + .append(r.packetType?.toString() ?: "").append(',') + .append(r.crcOk?.toString() ?: "").append(',').append(hex(r.payload)).append('\n') + } + return sb.toString() + } + + /** + * `waveform.csv` — the i16 samples carried by each filled type-43 record, long format + * (record, monotonic_ms, sample_index, value). Non-diagnostic raw samples for plotting/analysis. + * + * The type-43 body opens with a constant 5×i16 sub-header at byte offsets 24..33; the waveform is the + * i16 LE series from offset 34 to 236. Empty (baseline) records are skipped. Capped so one long capture + * can't produce an unbounded file. + */ + fun waveformCsv(): String { + val sb = StringBuilder("record,monotonic_ms,sample_index,value\n") + var rec = 0 + var total = 0 + val cap = WAVEFORM_CSV_SAMPLE_CAP + for (r in rxRows) { + if (r.crcOk == false) continue // never plot bytes that failed CRC + val vals = Whoop5Ecg.realtimeRawSamples(r.payload) ?: continue + // A record of all zeros is baseline, not waveform, and would swamp the file. A record that has + // ANY content is written in full, zeros included: trimming a trailing zero run (as this once did) + // silently edits the evidence, and a run of zeros at the end of a real record is itself a fact. + if (vals.all { it == 0 }) continue + for ((idx, v) in vals.withIndex()) { + sb.append(rec).append(',').append(r.monotonicMs).append(',').append(idx).append(',').append(v).append('\n') + if (++total >= cap) { + sb.append("# truncated at ").append(cap).append(" samples\n") + return sb.toString() + } + } + rec++ + } + return sb.toString() + } + + /** `stats.json` — non-diagnostic protocol statistics for a maintainer. */ + fun statsJson(): String { + val records = rxRecords() + val pps = EcgResearchStats.packetsPerSecondByCharacteristic(records) + val hist = EcgResearchStats.payloadSizeHistogram(records) + val largest = EcgResearchStats.largestPayloadSizes(records, 8) + val typeCounts = EcgResearchStats.packetTypeCounts(records) + val repeatedSizes = hist.filterValues { it >= 3 } + val candidateRawCount = EcgResearchStats.countOfSize(records, CANDIDATE_RAW_RECORD_SIZE) + val maxZero = EcgResearchStats.maxZeroFractionOfLargeRecords(records, 64) + // "appeared after Start" — measured around the first command-sequence marker if the run set one. + val startMarker = eventRows.firstOrNull { it.kind == "command_sequence_started" }?.monotonicMs + val delta = startMarker?.let { EcgResearchStats.typeDeltaAround(records, it) } + // A changing-position map for the most common large record size (candidate counter/timestamp fields). + val dominantLarge = hist.entries.filter { it.key >= 64 }.maxByOrNull { it.value }?.key + val changing = dominantLarge?.let { size -> + EcgResearchStats.changingPositionsAcross( + synchronized(lock) { _rx.filter { it.payload.size == size }.map { it.payload } }, + ) + } + + val sb = StringBuilder() + sb.append("{\n") + sb.append(" \"sessionId\": ").append(jstr(sessionId)).append(",\n") + sb.append(" \"startWallIso\": ").append(jstr(iso(startWallMs))).append(",\n") + sb.append(" \"counts\": {") + sb.append("\"tx\": ").append(txCount).append(", \"rx\": ").append(rxCount) + sb.append(", \"commands\": ").append(commandCount).append(", \"events\": ").append(eventCount) + sb.append(", \"unknownRx\": ").append(EcgResearchStats.unrecognizedCount(records)) + sb.append(", \"rxOverflowDropped\": ").append(rxOverflow).append("},\n") + sb.append(" \"packetsPerSecondByCharacteristic\": ").append(jnumMap(pps)).append(",\n") + sb.append(" \"payloadSizeHistogram\": ").append(jintMap(hist.mapKeys { it.key.toString() })).append(",\n") + sb.append(" \"largestPayloadSizes\": ").append(jintList(largest)).append(",\n") + sb.append(" \"packetTypeCounts\": ") + .append(jintMap(typeCounts.mapKeys { it.key?.toString() ?: "unrecognized" })).append(",\n") + sb.append(" \"repeatedSizes\": ").append(jintMap(repeatedSizes.mapKeys { it.key.toString() })).append(",\n") + sb.append(" \"candidateRawRecordSize\": ").append(CANDIDATE_RAW_RECORD_SIZE).append(",\n") + sb.append(" \"candidateRawRecordCount\": ").append(candidateRawCount).append(",\n") + sb.append(" \"maxZeroFractionOfLargeRecords\": ").append(maxZero?.let { round3(it).toString() } ?: "null").append(",\n") + sb.append(" \"changingBytePositionsForSize\": ") + .append(dominantLarge?.toString() ?: "null").append(",\n") + sb.append(" \"changingBytePositions\": ").append(jintList(changing ?: emptyList())).append(",\n") + sb.append(" \"packetTypesAppearedAfterStart\": ") + .append(jNullableIntList(delta?.appearedAfter)).append(",\n") + sb.append(" \"packetTypesDisappearedAfterStop\": ") + .append(jNullableIntList(delta?.disappearedAfter)).append("\n") + sb.append("}\n") + return sb.toString() + } + + // ---- formatting helpers (pure) ----------------------------------------------------------------- + + /** ISO-8601 UTC to the second, computed WITHOUT java.time.Instant.now so it is deterministic. */ + private fun iso(wallMs: Long): String { + var days = Math.floorDiv(wallMs, 86_400_000L) + val msOfDay = Math.floorMod(wallMs, 86_400_000L) + val hh = (msOfDay / 3_600_000L).toInt() + val mm = ((msOfDay % 3_600_000L) / 60_000L).toInt() + val ss = ((msOfDay % 60_000L) / 1000L).toInt() + // civil-from-days (Howard Hinnant's algorithm), epoch 1970-01-01. + var z = days + 719_468L + val era = (if (z >= 0) z else z - 146_096) / 146_097 + val doe = z - era * 146_097 + val yoe = (doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365 + val y = yoe + era * 400 + val doy = doe - (365 * yoe + yoe / 4 - yoe / 100) + val mp = (5 * doy + 2) / 153 + val d = (doy - (153 * mp + 2) / 5 + 1).toInt() + val m = (if (mp < 10) mp + 3 else mp - 9).toInt() + val year = (if (m <= 2) y + 1 else y).toInt() + return "%04d-%02d-%02dT%02d:%02d:%02dZ".format(year, m, d, hh, mm, ss) + } + + private fun csv(s: String): String = + if (s.any { it == ',' || it == '"' || it == '\n' || it == '\r' }) { + "\"" + s.replace("\"", "\"\"") + "\"" + } else s + + private fun jstr(s: String): String { + val sb = StringBuilder("\"") + for (c in s) when (c) { + '"' -> sb.append("\\\"") + '\\' -> sb.append("\\\\") + '\n' -> sb.append("\\n") + '\r' -> sb.append("\\r") + '\t' -> sb.append("\\t") + else -> if (c < ' ') sb.append("\\u%04x".format(c.code)) else sb.append(c) + } + return sb.append('"').toString() + } + + private fun jintList(xs: List): String = xs.joinToString(", ", "[", "]") + private fun jNullableIntList(xs: List?): String = + (xs ?: emptyList()).joinToString(", ", "[", "]") { it?.toString() ?: "null" } + private fun jintMap(m: Map): String = + m.entries.joinToString(", ", "{", "}") { "${jstr(it.key)}: ${it.value}" } + private fun jnumMap(m: Map): String = + m.entries.joinToString(", ", "{", "}") { "${jstr(it.key)}: ${round3(it.value)}" } + private fun round3(d: Double): Double = Math.round(d * 1000.0) / 1000.0 + + private fun hex2(v: Int): String = "0x%02X".format(v and 0xFF) + private fun hex(b: ByteArray): String = b.joinToString("") { "%02x".format(it.toInt() and 0xFF) } + + /** Last 4 hex of a 128-bit UUID, or the string itself if short — the compact char label stats use. */ + private fun shortUuid(u: String): String { + val first = u.substringBefore('-') + return if (first.length >= 4) first.takeLast(4) else u + } +} diff --git a/android/app/src/main/java/com/noop/protocol/EcgResearchStats.kt b/android/app/src/main/java/com/noop/protocol/EcgResearchStats.kt new file mode 100644 index 0000000000..0fea7921a9 --- /dev/null +++ b/android/app/src/main/java/com/noop/protocol/EcgResearchStats.kt @@ -0,0 +1,247 @@ +package com.noop.protocol + +/** + * #891 / #1100: PURE, NON-DIAGNOSTIC protocol statistics over a captured MG ECG research session. + * + * Everything here is a shape/timing observation about bytes on a wire — packet rates, payload sizes, + * zero-fill, which byte positions move between records. NOTHING here interprets a waveform, and the + * vocabulary is deliberately neutral: a fast integer stream is a "candidate high-rate signal", never "the + * ECG". Provenance of any waveform is a question for a human reading the raw bytes, not a statistic. + * + * Pure (no Android, no I/O, no clock): every input is passed in, so the whole file is unit-tested without a + * strap. Consumed by [EcgResearchLog] to build `stats.json` for the export bundle. + */ +object EcgResearchStats { + + /** One inbound frame as the stats see it. [payload] is the on-wire bytes; [packetType] is the decoded + * type byte when the framing parser named one, else null; [recognized] is whether the parser mapped it. */ + data class RxRecord( + val monotonicMs: Long, + val characteristic: String, + val payload: ByteArray, + val packetType: Int?, + val recognized: Boolean, + ) { + val size: Int get() = payload.size + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is RxRecord) return false + return monotonicMs == other.monotonicMs && characteristic == other.characteristic && + packetType == other.packetType && recognized == other.recognized && + payload.contentEquals(other.payload) + } + + override fun hashCode(): Int { + var h = monotonicMs.hashCode() + h = 31 * h + characteristic.hashCode(); h = 31 * h + (packetType ?: -1) + h = 31 * h + recognized.hashCode(); h = 31 * h + payload.contentHashCode() + return h + } + } + + /** Fraction (0..1) of bytes in [payload] between [from], inclusive, and [to], exclusive, that are 0x00. + * An out-of-range or empty window returns 0.0. */ + fun zeroByteFraction(payload: ByteArray, from: Int = 0, to: Int = payload.size): Double { + val lo = from.coerceIn(0, payload.size) + val hi = to.coerceIn(lo, payload.size) + if (hi <= lo) return 0.0 + var zeros = 0 + for (i in lo until hi) if (payload[i].toInt() == 0) zeros++ + return zeros.toDouble() / (hi - lo) + } + + /** True when every byte in [payload] over [from, to) is 0x00 (a genuinely empty region — the "is the + * big raw area all zeros?" question #891 asks of large records). Empty window is NOT all-zero. */ + fun isAllZero(payload: ByteArray, from: Int = 0, to: Int = payload.size): Boolean { + val lo = from.coerceIn(0, payload.size) + val hi = to.coerceIn(lo, payload.size) + if (hi <= lo) return false + for (i in lo until hi) if (payload[i].toInt() != 0) return false + return true + } + + /** Byte positions where [a] and [b] differ (shared prefix only). A SEQUENCE-COUNTER hunt: a field that + * changes between consecutive same-shape records is a counter/timestamp candidate. */ + fun changingBytePositions(a: ByteArray, b: ByteArray): List { + val n = minOf(a.size, b.size) + val out = ArrayList() + for (i in 0 until n) if (a[i] != b[i]) out.add(i) + return out + } + + /** Union of positions that EVER change across a run of same-length records (the counter/timestamp map). */ + fun changingPositionsAcross(records: List): List { + if (records.size < 2) return emptyList() + val changing = sortedSetOf() + for (i in 1 until records.size) changing.addAll(changingBytePositions(records[i - 1], records[i])) + return changing.toList() + } + + /** packets/sec per characteristic across the whole window (count ÷ span seconds; a single packet or a + * zero span reports the raw count as a per-second rate so the number is never divided by zero). */ + fun packetsPerSecondByCharacteristic(records: List): Map { + val byChar = records.groupBy { it.characteristic } + return byChar.mapValues { (_, rs) -> + if (rs.size < 2) rs.size.toDouble() + else { + val span = (rs.maxOf { it.monotonicMs } - rs.minOf { it.monotonicMs }).coerceAtLeast(1) + rs.size.toDouble() * 1000.0 / span + } + } + } + + /** Payload-size histogram: exact size in bytes → how many records had it. */ + fun payloadSizeHistogram(records: List): Map { + val h = HashMap() + for (r in records) h[r.size] = (h[r.size] ?: 0) + 1 + return h.toSortedMap() + } + + /** The [n] largest distinct payload sizes seen, descending. */ + fun largestPayloadSizes(records: List, n: Int = 5): List = + records.map { it.size }.distinct().sortedDescending().take(n) + + /** Count of records whose payload is exactly [size] bytes — e.g. the repeated 1,584-byte record #891 + * flags as a candidate raw block. */ + fun countOfSize(records: List, size: Int): Int = records.count { it.size == size } + + /** Packet-type census: decoded type byte (or null for unrecognised) → count. */ + fun packetTypeCounts(records: List): Map { + val h = LinkedHashMap() + for (r in records) h[r.packetType] = (h[r.packetType] ?: 0) + 1 + return h + } + + /** How many records the framing parser did NOT recognise — the unknown/rejected tally (Phase 6/7). */ + fun unrecognizedCount(records: List): Int = records.count { !it.recognized } + + /** + * Packet types present only AFTER a boundary time [afterMs] (candidate "appeared after Start") and those + * present only BEFORE it (candidate "disappeared after Stop"). Type identity is the decoded type byte; + * null (unrecognised) is folded in as its own bucket. Purely descriptive — never asserts causation. + */ + data class TypeDelta(val appearedAfter: List, val disappearedAfter: List) + + fun typeDeltaAround(records: List, afterMs: Long): TypeDelta { + val before = records.filter { it.monotonicMs < afterMs }.map { it.packetType }.toSet() + val after = records.filter { it.monotonicMs >= afterMs }.map { it.packetType }.toSet() + return TypeDelta( + appearedAfter = (after - before).toList(), + disappearedAfter = (before - after).toList(), + ) + } + + /** Largest zero-byte fraction over any record at least [minSize] bytes — surfaces the "large records are + * mostly padding" observation. Returns null when no record is that big. */ + fun maxZeroFractionOfLargeRecords(records: List, minSize: Int = 64): Double? { + val big = records.filter { it.size >= minSize } + if (big.isEmpty()) return null + return big.maxOf { zeroByteFraction(it.payload) } + } + + /** + * Estimate beats-per-minute from a waveform by normalised autocorrelation — NON-DIAGNOSTIC, and + * deliberately willing to return null. + * + * ## The trap this is written around (#194) + * + * The strap's raw records carry a FIXED sample count ([Whoop5Ecg.SAMPLES_PER_RAW_RECORD] = 101). Anything + * that repeats per record — a constant sub-header, a re-sent block, a seam where two records join — is + * periodic at the record period, and at 100 Hz that period is 1.01 s == 59.4 bpm: inside this search band + * and inside resting heart-rate range. An autocorrelation fed a concatenated record stream will therefore + * report a confident ~59 bpm out of pure framing, with no cardiac content at all. That is the exact + * failure mode behind NOOP's withdrawn PPG->HR estimate, so when [excludeLag] is supplied: + * + * 1. lags within [excludeLagTolerance] of it cannot win, and + * 2. the winner must also beat the strength AT the artefact lag ([artefactDominance]). Step 1 alone is + * not enough — a broad peak centred on the artefact lag still has strong shoulders just outside the + * notch, so a notch by itself just reports the artefact as a slightly different number. + * + * The cost is stated rather than hidden: a genuine heart rate that lands in that narrow band is + * indistinguishable from the artefact and is therefore DECLINED. Null means "this method cannot tell", + * never "no rhythm". + * + * ## Two things that made an earlier version wrong + * + * - **Comparability across lags.** A raw lag product sums over `n - lag` terms, so longer lags accumulate + * fewer terms and score lower for arithmetic rather than physiological reasons. Each lag is scored as a + * true normalised cross-correlation over its own overlap, so lags are comparable and [minStrength] + * means the same thing everywhere. + * - **Octave errors.** A periodic signal correlates just as well at twice its period, and with a + * non-integer number of cycles the double-period lag can score marginally higher — reporting 55 bpm for + * a 110 bpm input. So the winner is the SHORTEST lag within [octaveTolerance] of the best score, not + * the argmax. + */ + fun estimateBpm( + samples: IntArray, + fs: Int = 100, + minBpm: Int = 40, + maxBpm: Int = 180, + minStrength: Double = 0.3, + excludeLag: Int? = null, + excludeLagTolerance: Int = 2, + octaveTolerance: Double = 0.85, + artefactDominance: Double = 1.0, + ): Int? { + if (fs <= 0 || samples.size < fs) return null // need at least ~1 second + val n = samples.size + val mean = samples.average() + val c = DoubleArray(n) { samples[it] - mean } + var e0 = 0.0 + for (v in c) e0 += v * v + if (e0 <= 0.0) return null // a flat trace has no period to find + val loLag = (60.0 * fs / maxBpm).toInt().coerceAtLeast(1) + val hiLag = (60.0 * fs / minBpm).toInt().coerceAtMost(n - 1) + if (hiLag <= loLag) return null + + /** True normalised cross-correlation of the series against itself at [lag], over their overlap. */ + fun ncc(lag: Int): Double { + val overlap = n - lag + if (overlap <= 1) return 0.0 + var num = 0.0 + var eA = 0.0 + var eB = 0.0 + for (i in 0 until overlap) { + val a = c[i] + val b = c[i + lag] + num += a * b + eA += a * a + eB += b * b + } + if (eA <= 0.0 || eB <= 0.0) return 0.0 + return num / kotlin.math.sqrt(eA * eB) + } + + fun notched(lag: Int) = excludeLag != null && kotlin.math.abs(lag - excludeLag) <= excludeLagTolerance + + val strength = DoubleArray(hiLag + 1) + var best = Double.NEGATIVE_INFINITY + for (lag in loLag..hiLag) { + strength[lag] = ncc(lag) + if (!notched(lag) && strength[lag] > best) best = strength[lag] + } + if (best < minStrength) return null + + // Step 2 of the artefact guard: if the record period itself correlates at least as well as the best + // candidate outside the notch, the candidate is that peak's shoulder and not an independent rhythm. + if (excludeLag != null) { + var artefact = Double.NEGATIVE_INFINITY + for (lag in loLag..hiLag) if (notched(lag) && strength[lag] > artefact) artefact = strength[lag] + if (artefact != Double.NEGATIVE_INFINITY && best < artefact * artefactDominance) return null + } + + // Octave correction, over PEAKS only. Taking the shortest lag that merely clears the threshold picks + // a point on the rising shoulder of the real peak instead of the peak: a 45 bpm input (lag 133) has + // correlation 0.86 at lag 122, which cleared the floor and reported 49 bpm. A candidate must be a + // local maximum, and then the shortest such peak is the fundamental rather than a sub-harmonic. + val floor = best * octaveTolerance + for (lag in loLag..hiLag) { + if (notched(lag) || strength[lag] < floor) continue + val risingInto = lag == loLag || strength[lag] >= strength[lag - 1] + val fallingAfter = lag == hiLag || strength[lag] >= strength[lag + 1] + if (risingInto && fallingAfter) return (60.0 * fs / lag).toInt() + } + return null + } +} diff --git a/android/app/src/test/java/com/noop/protocol/EcgResearchAllowListTest.kt b/android/app/src/test/java/com/noop/protocol/EcgResearchAllowListTest.kt new file mode 100644 index 0000000000..9be5d00778 --- /dev/null +++ b/android/app/src/test/java/com/noop/protocol/EcgResearchAllowListTest.kt @@ -0,0 +1,100 @@ +package com.noop.protocol + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The panel's dispatch surface is a positive allow-list of four ECG opcodes. These tests pin that it can + * NEVER form anything else — most importantly the firmware-load family that sits three codes above 139. + */ +class EcgResearchAllowListTest { + + @Test + fun theFourProbeOpcodesAreExactlyTheLabradorCommands() { + assertEquals( + setOf(123, 124, 125, 139), + EcgResearchAllowList.PROBE_OPCODES, + ) + assertTrue(EcgResearchAllowList.isProbeOpcode(Whoop5Ecg.SELECT_WRIST_CMD)) + assertTrue(EcgResearchAllowList.isProbeOpcode(Whoop5Ecg.TOGGLE_REALTIME_FILTERED_ECG_CMD)) + assertTrue(EcgResearchAllowList.isProbeOpcode(Whoop5Ecg.TOGGLE_SAVE_RAW_ECG_CMD)) + assertTrue(EcgResearchAllowList.isProbeOpcode(Whoop5Ecg.MAIN_CONTROL_ECG_DATA_GENERATION_CMD)) + } + + @Test + fun noOpcodeOutsideTheFourIsDispatchableAsAProbeCommand() { + // The whole "no opcode sweep" guarantee, as a census: every byte value 0..255 except the four is + // refused by the predicate the real BLE send path consults. + for (op in 0..255) { + val expected = op in setOf(123, 124, 125, 139) + assertEquals("opcode $op", expected, EcgResearchAllowList.isProbeOpcode(op)) + } + } + + @Test + fun theFirmwareLoadFamilyIsRefusedAndNamed() { + // 139 is TOGGLE_LABRADOR_FILTERED; 142/143/144 are the destructive firmware-load family right above + // it. They must be refused AND named in the deny-list so the refusal is testable. + for (op in listOf(142, 143, 144, 36, 37, 38, 45, 83)) { + assertFalse("must not dispatch $op", EcgResearchAllowList.isProbeOpcode(op)) + assertFalse(EcgResearchAllowList.DISPATCHABLE_OPCODES.contains(op)) + assertTrue("must name $op as forbidden", EcgResearchAllowList.isForbidden(op)) + } + } + + @Test + fun rebootWipeDfuAndHighFreqAreAllForbidden() { + for (op in listOf(29, 32, 25, 99, 96, 97, 100)) { + assertTrue("forbidden: $op", EcgResearchAllowList.isForbidden(op)) + assertFalse(EcgResearchAllowList.isProbeOpcode(op)) + } + } + + @Test + fun noForbiddenOpcodeIsEverDispatchable() { + for (op in EcgResearchAllowList.FORBIDDEN.keys) { + assertFalse("forbidden $op leaked into dispatch set", EcgResearchAllowList.DISPATCHABLE_OPCODES.contains(op)) + } + } + + @Test + fun theGateOpcodesAreThe119And121DeviceConfigVerbsOnly() { + assertEquals(setOf(119, 121), EcgResearchAllowList.GATE_OPCODES) + // The FEATURE-FLAG SET verb (120) is NOT a gate opcode — the gate is device-config only. + assertFalse(EcgResearchAllowList.isGateOpcode(120)) + } + + @Test + fun dispatchableIsExactlyTheFourProbesPlusTwoGateVerbs() { + assertEquals(setOf(123, 124, 125, 139, 119, 121), EcgResearchAllowList.DISPATCHABLE_OPCODES) + } + + /** + * The companion guard — that every DISPATCHABLE opcode is a constructible [CommandNumber] — is + * deliberately NOT here. It is an invariant of the SEND PATH: `send()` forms a frame from a + * `CommandNumber`, so an allow-listed opcode the enum cannot express could not be dispatched at all. + * + * Nothing in this change dispatches anything, and opcodes 124/125/139 are absent from that enum on + * purpose — upstream excluded them with a comment saying Android has no ECG app layer and sends + * none of them. Asserting their presence here would fail for the correct reason, so the guard lands + * with the send-path wiring that makes it true and gives it something worth protecting. + */ + @Test + fun noForbiddenOpcodeIsReachableThroughTheProbeCases() { + for (op in EcgResearchAllowList.FORBIDDEN.keys) { + assertFalse(EcgResearchAllowList.PROBE_OPCODES.contains(op)) + } + } + + @Test + fun theFirmwareLoadNewFamilyIsNotEvenInTheDispatchSet() { + // 142/143/144 sit three codes above 139 in opcode space; the census must show they cannot be formed. + assertNull(EcgResearchAllowList.FORBIDDEN[139]) // 139 is a PROBE opcode, not forbidden + assertEquals("START_FIRMWARE_LOAD_NEW", EcgResearchAllowList.FORBIDDEN[142]) + assertEquals("LOAD_FIRMWARE_DATA_NEW", EcgResearchAllowList.FORBIDDEN[143]) + assertEquals("PROCESS_FIRMWARE_IMAGE_NEW", EcgResearchAllowList.FORBIDDEN[144]) + } +} diff --git a/android/app/src/test/java/com/noop/protocol/EcgResearchLogTest.kt b/android/app/src/test/java/com/noop/protocol/EcgResearchLogTest.kt new file mode 100644 index 0000000000..bfa728da5b --- /dev/null +++ b/android/app/src/test/java/com/noop/protocol/EcgResearchLogTest.kt @@ -0,0 +1,191 @@ +package com.noop.protocol + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class EcgResearchLogTest { + + private fun newLog(maxRx: Int = EcgResearchLog.DEFAULT_MAX_RX_ROWS) = + EcgResearchLog("mg-ecg-20260819-101112", startWallMs = 0L, startMonotonicMs = 0L, maxRxRows = maxRx) + + private fun tx(op: Int, mono: Long) = EcgResearchLog.TxRow( + monotonicMs = mono, wallMs = mono, opcode = op, + payload = byteArrayOf(0x01, 0x01), frame = byteArrayOf(0xAA.toByte(), 0x01), + serviceUuid = "fd4b0001", characteristicUuid = "fd4b0002", writeType = "NO_RESPONSE", + ) + + private fun rx(mono: Long, size: Int, type: Int?, recognized: Boolean) = EcgResearchLog.RxRow( + monotonicMs = mono, wallMs = mono, serviceUuid = "fd4b0001", characteristicUuid = "fd4b0003", + payload = ByteArray(size), packetType = type, sequence = null, recognized = recognized, + rejected = !recognized, + ) + + @Test + fun recordsCountAcrossAllStreams() { + val log = newLog() + log.recordTx(tx(139, 10)) + log.recordRx(rx(20, 1584, 0x28, false)) + log.recordCommand( + EcgResearchLog.CommandRow(30, 30, 139, "01", "Observed", "session", "SUCCESS(1)", "aabb", 42), + ) + log.recordEvent(EcgResearchLog.EventRow(5, 5, "electrode_contact_started", "")) + assertEquals(1, log.txCount) + assertEquals(1, log.rxCount) + assertEquals(1, log.commandCount) + assertEquals(1, log.eventCount) + } + + @Test + fun eventCsvCarriesTimestampsAndMarkers() { + val log = newLog() + log.recordEvent(EcgResearchLog.EventRow(1234, 0L, "electrode_contact_started", "left index")) + val csv = log.eventsCsv() + assertTrue(csv.startsWith("monotonic_ms,wall_iso,kind,detail\n")) + assertTrue(csv.contains("1234,1970-01-01T00:00:00Z,electrode_contact_started,left index")) + } + + @Test + fun txCsvCarriesDecimalAndHexOpcodeAndFrame() { + val log = newLog() + log.recordTx(tx(139, 10)) + val csv = log.bleTxCsv() + assertTrue(csv.contains("opcode_dec,opcode_hex")) + assertTrue(csv.contains("10,1970-01-01T00:00:00Z,139,0x8B,0101,aa01,fd4b0001,fd4b0002,NO_RESPONSE")) + } + + @Test + fun unknownPacketsCsvContainsOnlyUnrecognisedRows() { + val log = newLog() + log.recordRx(rx(1, 20, 40, recognized = true)) // recognised — excluded + log.recordRx(rx(2, 1584, null, recognized = false)) // unknown — included + val csv = log.unknownPacketsCsv() + val lines = csv.trim().split("\n") + assertEquals(2, lines.size) // header + one unknown row + assertTrue(lines[1].contains(",1584,")) + } + + @Test + fun rxCapCountsOverflowInsteadOfDroppingSilently() { + val log = newLog(maxRx = 2) + log.recordRx(rx(1, 10, 40, true)) + log.recordRx(rx(2, 10, 40, true)) + log.recordRx(rx(3, 10, 40, true)) // over the cap + assertEquals(2, log.rxCount) + assertEquals(1L, log.rxOverflow) + assertTrue(log.statsJson().contains("\"rxOverflowDropped\": 1")) + } + + @Test + fun statsJsonReportsCandidateRawRecordsAndZeroFill() { + val log = newLog() + repeat(3) { log.recordRx(rx(it.toLong(), 1584, 0x28, recognized = false)) } + val json = log.statsJson() + assertTrue(json.contains("\"sessionId\": \"mg-ecg-20260819-101112\"")) + assertTrue(json.contains("\"candidateRawRecordSize\": 1584")) + assertTrue(json.contains("\"candidateRawRecordCount\": 3")) + // three all-zero 1584-byte records -> max zero fraction 1.0 + assertTrue(json.contains("\"maxZeroFractionOfLargeRecords\": 1.0")) + assertTrue(json.contains("\"unknownRx\": 3")) + } + + @Test + fun statsJsonSurfacesTypesAppearingAfterAStartMarker() { + val log = newLog() + log.recordRx(rx(0, 4, 40, true)) // type 40 present only BEFORE the marker + log.recordEvent(EcgResearchLog.EventRow(100, 100, "command_sequence_started", "")) + log.recordRx(rx(200, 1584, 99, false)) // type 99 present only AFTER the marker + val json = log.statsJson() + assertTrue(json.contains("\"packetTypesAppearedAfterStart\": [99]")) + assertTrue(json.contains("\"packetTypesDisappearedAfterStop\": [40]")) + } + + // ---- CRC verdict + volume bounds (added with the CRC-gating of the decode path) ---------------- + + /** A 240-byte type-43 record whose waveform region carries [value] in every sample. */ + private fun rawRecordRow(monotonicMs: Long, value: Int, crcOk: Boolean?): EcgResearchLog.RxRow { + val f = ByteArray(Whoop5Ecg.RAW_RECORD_LENGTH) + f[Whoop5Ecg.RAW_TYPE_OFFSET] = PacketType.REALTIME_RAW_DATA.rawValue.toByte() + var i = Whoop5Ecg.RAW_WAVEFORM_START + while (i + 1 < Whoop5Ecg.RAW_BODY_END) { + f[i] = (value and 0xFF).toByte() + f[i + 1] = ((value shr 8) and 0xFF).toByte() + i += 2 + } + return EcgResearchLog.RxRow( + monotonicMs = monotonicMs, wallMs = 1_700_000_000_000L, serviceUuid = "fd4b", + characteristicUuid = "fd4b0003", payload = f, + packetType = PacketType.REALTIME_RAW_DATA.rawValue, sequence = null, + recognized = crcOk != false, rejected = crcOk == false, crcOk = crcOk, + ) + } + + @Test + fun aCrcFailureIsRecordedAndFlaggedRatherThanDropped() { + val log = EcgResearchLog("s", 1_700_000_000_000L, 0L) + log.recordRx(rawRecordRow(10, 100, crcOk = true)) + log.recordRx(rawRecordRow(20, 200, crcOk = false)) + log.recordRx(rawRecordRow(30, 300, crcOk = null)) + + // Dropping a corrupt frame would make "the strap sent nothing" and "we threw it away" identical. + assertEquals(3, log.rxCount) + val csv = log.bleRxCsv() + val header = csv.lineSequence().first().split(",") + val crcCol = header.indexOf("crc_ok") + assertTrue("the rx CSV must carry the verdict", crcCol >= 0) + val rows = csv.trim().lines().drop(1).map { it.split(",") } + assertEquals("true", rows[0][crcCol]) + assertEquals("false", rows[1][crcCol]) + // "nothing to check" must be distinguishable from "failed": an empty field, not the word false. + assertEquals("", rows[2][crcCol]) + } + + @Test + fun theWaveformExportNeverPlotsBytesThatFailedCrc() { + val log = EcgResearchLog("s", 1_700_000_000_000L, 0L) + log.recordRx(rawRecordRow(10, 111, crcOk = true)) + log.recordRx(rawRecordRow(20, 222, crcOk = false)) // corruption would render as a spike + val csv = log.waveformCsv() + assertTrue("the CRC-good record should be plotted", csv.contains(",111")) + assertFalse("a CRC-failed record must never reach the waveform", csv.contains(",222")) + } + + @Test + fun anAllZeroRecordIsBaselineAndSkippedButRealZerosAreKept() { + val log = EcgResearchLog("s", 1_700_000_000_000L, 0L) + log.recordRx(rawRecordRow(10, 0, crcOk = true)) // pure baseline: skipped entirely + assertEquals("record,monotonic_ms,sample_index,value\n", log.waveformCsv()) + + // A record with content keeps its full 101 samples — no trailing-zero trimming, which would edit + // the evidence. Build one with a single non-zero lead sample. + val f = ByteArray(Whoop5Ecg.RAW_RECORD_LENGTH) + f[Whoop5Ecg.RAW_TYPE_OFFSET] = PacketType.REALTIME_RAW_DATA.rawValue.toByte() + f[Whoop5Ecg.RAW_WAVEFORM_START] = 9 + val log2 = EcgResearchLog("s", 1_700_000_000_000L, 0L) + log2.recordRx( + EcgResearchLog.RxRow( + 1, 1_700_000_000_000L, "fd4b", "fd4b0003", f, + PacketType.REALTIME_RAW_DATA.rawValue, null, true, false, true, + ), + ) + assertEquals( + "every sample of a non-empty record is written", + Whoop5Ecg.SAMPLES_PER_RAW_RECORD, + log2.waveformCsv().trim().lines().size - 1, + ) + } + + @Test + fun theCaptureIsBoundedByBytesNotJustRows() { + // A row cap alone does not bound memory: 200k x 240 B is ~48 MB before object overhead, and an + // offload can fill that faster than a UI notices. Whichever cap is hit first stops storage, and the + // drops are counted either way. + val log = EcgResearchLog("s", 1_700_000_000_000L, 0L, maxRxRows = 1_000, maxRxBytes = 1_000L) + repeat(20) { log.recordRx(rawRecordRow(it.toLong(), 5, crcOk = true)) } + assertTrue("the byte cap should bite well before the row cap", log.rxCount in 1..4) + assertTrue("bytes stored must stay inside the cap", log.rxBytes <= 1_000L) + assertEquals("every dropped frame is counted", (20 - log.rxCount).toLong(), log.rxOverflow) + assertTrue("the bundle must report the drops", log.statsJson().contains("\"rxOverflowDropped\": ${log.rxOverflow}")) + } +} diff --git a/android/app/src/test/java/com/noop/protocol/EcgResearchStatsTest.kt b/android/app/src/test/java/com/noop/protocol/EcgResearchStatsTest.kt new file mode 100644 index 0000000000..bed58d2717 --- /dev/null +++ b/android/app/src/test/java/com/noop/protocol/EcgResearchStatsTest.kt @@ -0,0 +1,125 @@ +package com.noop.protocol + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class EcgResearchStatsTest { + + private fun rec(ms: Long, ch: String, payload: ByteArray, type: Int? = null, recognized: Boolean = true) = + EcgResearchStats.RxRecord(ms, ch, payload, type, recognized) + + @Test + fun zeroByteFractionCountsZerosInWindow() { + val p = byteArrayOf(0, 0, 1, 2, 0, 0) + assertEquals(4.0 / 6.0, EcgResearchStats.zeroByteFraction(p), 1e-9) + assertEquals(1.0, EcgResearchStats.zeroByteFraction(byteArrayOf(0, 0, 0)), 1e-9) + assertEquals(0.0, EcgResearchStats.zeroByteFraction(byteArrayOf(1, 2, 3)), 1e-9) + // windowed + assertEquals(1.0, EcgResearchStats.zeroByteFraction(p, 0, 2), 1e-9) + assertEquals(0.0, EcgResearchStats.zeroByteFraction(p, 2, 4), 1e-9) + // empty / out of range + assertEquals(0.0, EcgResearchStats.zeroByteFraction(ByteArray(0)), 1e-9) + } + + @Test + fun isAllZeroDetectsEmptyRegions() { + assertTrue(EcgResearchStats.isAllZero(ByteArray(1584))) + assertTrue(EcgResearchStats.isAllZero(byteArrayOf(1, 0, 0, 0, 2), 1, 4)) + assertFalse(EcgResearchStats.isAllZero(byteArrayOf(0, 0, 1))) + // an empty window is not "all zero" + assertFalse(EcgResearchStats.isAllZero(ByteArray(0))) + } + + @Test + fun changingBytePositionsFindsCounterLikeFields() { + val a = byteArrayOf(0x10, 0x00, 0x05, 0x00) + val b = byteArrayOf(0x11, 0x00, 0x05, 0x00) // only byte 0 moved + assertEquals(listOf(0), EcgResearchStats.changingBytePositions(a, b)) + + val c = byteArrayOf(0x12, 0x00, 0x06, 0x00) // bytes 0 and 2 moved vs b + assertEquals(listOf(0, 2), EcgResearchStats.changingPositionsAcross(listOf(a, b, c))) + } + + @Test + fun packetsPerSecondByCharacteristic() { + // char "0003": 3 packets across 2000ms span => 1.5/s. char "0005": single packet => raw count 1. + val records = listOf( + rec(0, "0003", ByteArray(20)), + rec(1000, "0003", ByteArray(20)), + rec(2000, "0003", ByteArray(20)), + rec(500, "0005", ByteArray(8)), + ) + val pps = EcgResearchStats.packetsPerSecondByCharacteristic(records) + assertEquals(1.5, pps["0003"]!!, 1e-9) + assertEquals(1.0, pps["0005"]!!, 1e-9) + } + + @Test + fun payloadSizeHistogramAndLargestAndRepeatedRecordCount() { + val records = listOf( + rec(0, "a", ByteArray(1584)), + rec(1, "a", ByteArray(1584)), + rec(2, "a", ByteArray(1584)), + rec(3, "a", ByteArray(20)), + ) + assertEquals(mapOf(20 to 1, 1584 to 3), EcgResearchStats.payloadSizeHistogram(records)) + assertEquals(listOf(1584, 20), EcgResearchStats.largestPayloadSizes(records)) + assertEquals(3, EcgResearchStats.countOfSize(records, 1584)) + } + + @Test + fun packetTypeCountsAndUnrecognized() { + val records = listOf( + rec(0, "a", ByteArray(4), type = 40, recognized = true), + rec(1, "a", ByteArray(4), type = 40, recognized = true), + rec(2, "a", ByteArray(4), type = null, recognized = false), + ) + assertEquals(2, EcgResearchStats.packetTypeCounts(records)[40]) + assertEquals(1, EcgResearchStats.packetTypeCounts(records)[null]) + assertEquals(1, EcgResearchStats.unrecognizedCount(records)) + } + + @Test + fun typeDeltaAroundAStartBoundary() { + val records = listOf( + rec(0, "a", ByteArray(4), type = 40), // before + rec(100, "a", ByteArray(4), type = 40), // before + rec(200, "a", ByteArray(4), type = 40), // after (still 40) + rec(250, "a", ByteArray(1584), type = 41), // after only — candidate "appeared after Start" + ) + val delta = EcgResearchStats.typeDeltaAround(records, afterMs = 150) + assertEquals(listOf(41), delta.appearedAfter) + assertTrue(delta.disappearedAfter.isEmpty()) + } + + @Test + fun estimateBpmFindsAKnownRateAndRejectsNoise() { + // Synthetic 72 bpm at 100 Hz over 6 s: period = 100*60/72 ≈ 83.3 samples. + val fs = 100 + val n = fs * 6 + val periodSamples = 60.0 * fs / 72.0 + val beat = IntArray(n) { (1000 * Math.sin(2 * Math.PI * it / periodSamples)).toInt() } + val bpm = EcgResearchStats.estimateBpm(beat, fs) + assertNotNull(bpm) + assertTrue("expected ~72, got $bpm", bpm!! in 68..76) + + // Flat/near-zero -> no confident rhythm -> null (never a fabricated number). + assertNull(EcgResearchStats.estimateBpm(IntArray(fs * 3) { 0 }, fs)) + // Too short -> null. + assertNull(EcgResearchStats.estimateBpm(IntArray(fs / 2) { it }, fs)) + } + + @Test + fun maxZeroFractionOfLargeRecords() { + val records = listOf( + rec(0, "a", ByteArray(1584)), // all zero, big + rec(1, "a", byteArrayOf(1, 2, 3)), // small — ignored by minSize + ) + assertEquals(1.0, EcgResearchStats.maxZeroFractionOfLargeRecords(records, 64)!!, 1e-9) + assertNull(EcgResearchStats.maxZeroFractionOfLargeRecords(listOf(rec(0, "a", ByteArray(4))), 64)) + } +} From 2f173395be8c76bea10553f71c89bb813126244c Mon Sep 17 00:00:00 2001 From: Zebsi235 Date: Sun, 6 Sep 2026 16:55:28 +0200 Subject: [PATCH 2/4] (+) ecg: the BPM estimator's own suite, recovered onto the pure-modules split EcgResearchStats.estimateBpm ships in this split, but the suite that defends it did not. EcgResearchStatsTest covers the estimator with one injected rate plus noise and a short buffer -- it does not cover the artefact that got NOOP's earlier PPG->HR estimate withdrawn (#194). EcgBpmEstimatorTest is that cover: eight injected rates rather than one, because a single match proves nothing and the method has to move with the input; the tiled-record artefact that unguarded reports as a confident 59 bpm; both sides of the notch; the declined-band cost stated rather than hidden; and the no-high-rate-bias case the unnormalised version failed. Recovered from the unsplit tree, where it was written. No source change -- it targets EcgResearchStats.estimateBpm, which this branch already carries. --- .../com/noop/protocol/EcgBpmEstimatorTest.kt | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 android/app/src/test/java/com/noop/protocol/EcgBpmEstimatorTest.kt diff --git a/android/app/src/test/java/com/noop/protocol/EcgBpmEstimatorTest.kt b/android/app/src/test/java/com/noop/protocol/EcgBpmEstimatorTest.kt new file mode 100644 index 0000000000..4b3eb8b345 --- /dev/null +++ b/android/app/src/test/java/com/noop/protocol/EcgBpmEstimatorTest.kt @@ -0,0 +1,133 @@ +package com.noop.protocol + +import kotlin.math.PI +import kotlin.math.abs +import kotlin.math.sin +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * #891/#1100 + the #194 lesson: [EcgResearchStats.estimateBpm]. + * + * The repo's rule for deriving a physiological signal from raw sensor data is that a single match proves + * nothing — the method has to track a VARYING input, and for a synthetic test that means recovering SEVERAL + * injected values rather than one. The previous test injected one rate. This injects a spread, and then + * attacks the estimator with the specific artefact that got the earlier PPG->HR estimate withdrawn: the + * strap's records carry a fixed 101 samples, so a seam between records is periodic at ~59 bpm all by itself. + */ +class EcgBpmEstimatorTest { + + private val fs = 100 + + /** A clean sinusoid at [bpm], [seconds] long, at [fs] Hz. */ + private fun beats(bpm: Int, seconds: Double = 8.0, amplitude: Int = 1000): IntArray { + val n = (fs * seconds).toInt() + val f = bpm / 60.0 + return IntArray(n) { (amplitude * sin(2 * PI * f * it / fs)).toInt() } + } + + @Test + fun recoversSeveralDifferentInjectedRates() { + // One match is a coincidence; the method has to move with the input. + for (bpm in intArrayOf(45, 52, 70, 88, 110, 132, 150, 170)) { + val got = EcgResearchStats.estimateBpm(beats(bpm), fs) + assertNotNull("no estimate at $bpm bpm", got) + // Lag quantisation at 100 Hz coarsens with rate, so allow a proportional tolerance. + val tolerance = maxOf(3.0, bpm * 0.06) + assertTrue( + "expected ~$bpm bpm, got $got", + abs(got!! - bpm) <= tolerance, + ) + } + } + + @Test + fun theEstimateRisesAndFallsWithTheInput() { + // Monotonicity across the band: the ordering of the estimates must match the ordering of the truth. + val truth = intArrayOf(50, 75, 100, 125, 150) + val got = truth.map { EcgResearchStats.estimateBpm(beats(it), fs)!! } + assertTrue("estimates not monotonic with input: $got", got.zipWithNext().all { (a, b) -> a < b }) + } + + /** + * THE regression that matters. A buffer assembled by tiling the SAME record content over and over has no + * cardiac information in it at all — its only periodicity is the record length. At 101 samples and 100 Hz + * that is lag 101 == ~59 bpm: inside the search band, and a thoroughly plausible resting heart rate. + * + * This is the shape that got the earlier PPG->HR estimate withdrawn, so it is pinned twice: that the + * artefact really is detectable (or the test proves nothing), and that the guarded call refuses it. + */ + @Test + fun aPureRecordSeamArtefactIsRefusedRatherThanReportedAsFiftyNineBpm() { + val period = Whoop5Ecg.SAMPLES_PER_RAW_RECORD + // One record's worth of arbitrary but fixed content, tiled — exactly what a repeated block looks like. + var seed = 987654321L + val block = IntArray(period) { + seed = (seed * 6364136223846793005L + 1442695040888963407L) + ((seed shr 33).toInt() % 500) + } + val tiled = IntArray(period * 8) { block[it % period] } + + // Unguarded, the artefact is reported with confidence, and it lands on a resting heart rate. + val unguarded = EcgResearchStats.estimateBpm(tiled, fs, excludeLag = null) + assertNotNull("the artefact should be detectable at all, or this test proves nothing", unguarded) + assertTrue( + "the artefact was expected near 60*100/101 = 59 bpm, got $unguarded", + abs(unguarded!! - 59) <= 4, + ) + + // Guarded — the way the live view calls it — it declines rather than manufacturing physiology. + assertNull(EcgResearchStats.estimateBpm(tiled, fs, excludeLag = period)) + } + + @Test + fun excludingTheRecordPeriodStillFindsRatesEitherSideOfIt() { + // The guard must be a narrow notch, not a hole that swallows the resting band. + val period = Whoop5Ecg.SAMPLES_PER_RAW_RECORD + for (bpm in intArrayOf(45, 52, 70, 80)) { + val got = EcgResearchStats.estimateBpm(beats(bpm), fs, excludeLag = period) + assertNotNull("the notch swallowed $bpm bpm", got) + assertTrue("expected ~$bpm bpm, got $got", abs(got!! - bpm) <= maxOf(3.0, bpm * 0.06)) + } + } + + @Test + fun theCostOfTheGuardIsStatedNotHidden() { + // A genuine rate that lands ON the artefact lag is indistinguishable from the artefact, so the method + // declines. Note the notch ALONE would not do this — it would just report the peak's shoulder as a + // slightly different number; it is the dominance check that makes the refusal real. Pinned so the + // documented cost cannot quietly regress into a confident false reading. + val onTheNotch = 60 * fs / Whoop5Ecg.SAMPLES_PER_RAW_RECORD // 59 bpm + assertNotNull(EcgResearchStats.estimateBpm(beats(onTheNotch), fs, excludeLag = null)) + assertNull( + "a rate on the artefact lag must be declined, not reported", + EcgResearchStats.estimateBpm(beats(onTheNotch), fs, excludeLag = Whoop5Ecg.SAMPLES_PER_RAW_RECORD), + ) + } + + @Test + fun noiseAndFlatlineAndTooShortAllReturnNull() { + assertNull(EcgResearchStats.estimateBpm(IntArray(0), fs)) + assertNull("a buffer under one second cannot be judged", EcgResearchStats.estimateBpm(IntArray(fs - 1) { it }, fs)) + assertNull("a flat trace has no variance", EcgResearchStats.estimateBpm(IntArray(400) { 0 }, fs)) + // Deterministic pseudo-noise: no dominant period should clear the strength floor. + var seed = 12345L + val noise = IntArray(800) { + seed = (seed * 6364136223846793005L + 1442695040888963407L) + ((seed shr 33).toInt() % 200) + } + assertNull("pseudo-noise should not yield a confident rate", EcgResearchStats.estimateBpm(noise, fs)) + } + + @Test + fun theSearchIsNotBiasedTowardHighRatesByOverlapLength() { + // The lag score is a mean over the overlap, not a raw sum: an unnormalised sum shrinks with lag and + // pulls the winner toward short lags (high BPM). A slow rate in a short-ish buffer is where that + // bias showed, so pin it. + val got = EcgResearchStats.estimateBpm(beats(46, seconds = 4.0), fs) + assertNotNull(got) + assertTrue("a slow rate was pulled high (got $got)", abs(got!! - 46) <= 5) + } +} From f9ff5a210e5b37ec19cdc2cdb110575bbb6d6145 Mon Sep 17 00:00:00 2001 From: Zebsi235 Date: Sun, 6 Sep 2026 22:59:49 +0200 Subject: [PATCH 3/4] (c) ecg: project voice in two comments -- the app, not we --- android/app/src/main/java/com/noop/protocol/EcgResearchLog.kt | 2 +- .../app/src/test/java/com/noop/protocol/EcgResearchLogTest.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/android/app/src/main/java/com/noop/protocol/EcgResearchLog.kt b/android/app/src/main/java/com/noop/protocol/EcgResearchLog.kt index d812c4e763..b687088b6c 100644 --- a/android/app/src/main/java/com/noop/protocol/EcgResearchLog.kt +++ b/android/app/src/main/java/com/noop/protocol/EcgResearchLog.kt @@ -71,7 +71,7 @@ class EcgResearchLog( val rejected: Boolean, /** The frame's CRC verdict, or null when there was nothing to check (the house `crcOk != false` * idiom). A CRC FAILURE is recorded, not dropped: for a research capture a corrupt frame is - * evidence, and dropping it would make "the strap sent nothing" indistinguishable from "we threw it + * evidence, and dropping it would make "the strap sent nothing" indistinguishable from "the app threw it * away". No decoder reads a row whose verdict is false. */ val crcOk: Boolean? = null, ) diff --git a/android/app/src/test/java/com/noop/protocol/EcgResearchLogTest.kt b/android/app/src/test/java/com/noop/protocol/EcgResearchLogTest.kt index bfa728da5b..a369da7d07 100644 --- a/android/app/src/test/java/com/noop/protocol/EcgResearchLogTest.kt +++ b/android/app/src/test/java/com/noop/protocol/EcgResearchLogTest.kt @@ -128,7 +128,7 @@ class EcgResearchLogTest { log.recordRx(rawRecordRow(20, 200, crcOk = false)) log.recordRx(rawRecordRow(30, 300, crcOk = null)) - // Dropping a corrupt frame would make "the strap sent nothing" and "we threw it away" identical. + // Dropping a corrupt frame would make "the strap sent nothing" and "the app threw it away" identical. assertEquals(3, log.rxCount) val csv = log.bleRxCsv() val header = csv.lineSequence().first().split(",") From 83709d74fafccfdaaf70e2cec1fe4f4748e719f3 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Mon, 7 Sep 2026 10:18:44 +1200 Subject: [PATCH 4/4] ecg: make the estimator's artefact guard a decision, not a default estimateBpm took excludeLag as an optional parameter defaulting to null, so the shortest and most natural call, estimateBpm(samples, fs), silently selected the unguarded form. Measured on a tiled 101-sample record at 100 Hz carrying no rhythm whatsoever, that form returns 59, a believable resting heart rate manufactured from nothing. It is the withdrawn PPG->HR failure exactly, which is the failure this method's whole shape exists to avoid. The guard itself is right and costs nothing: on the same buffer, supplying the record period returns null, and a genuine 72 bpm sine reads 72 either way. So the parameter simply loses its default. Passing the period switches the guard on; passing null says this source is not record-tiled and accepts an unguarded estimate. Both remain available, but only as something somebody chose. The compiler then finds every caller that had not chosen: nine call sites across the two test files, six of which had taken the unguarded path without saying so. All now state excludeLag = null explicitly, which is the correct reading for a synthetic non-tiled fixture and makes the choice visible where it is made. No behavioural change. The existing artefact test already pins both sides, that an unguarded call reports the artefact near 59 and a guarded one declines, so the contract stays covered. Android suite 5466 green. --- .../java/com/noop/protocol/EcgResearchStats.kt | 11 ++++++++++- .../java/com/noop/protocol/EcgBpmEstimatorTest.kt | 14 +++++++------- .../java/com/noop/protocol/EcgResearchStatsTest.kt | 6 +++--- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/android/app/src/main/java/com/noop/protocol/EcgResearchStats.kt b/android/app/src/main/java/com/noop/protocol/EcgResearchStats.kt index 0fea7921a9..c1be24885d 100644 --- a/android/app/src/main/java/com/noop/protocol/EcgResearchStats.kt +++ b/android/app/src/main/java/com/noop/protocol/EcgResearchStats.kt @@ -151,6 +151,15 @@ object EcgResearchStats { * periodic at the record period, and at 100 Hz that period is 1.01 s == 59.4 bpm: inside this search band * and inside resting heart-rate range. An autocorrelation fed a concatenated record stream will therefore * report a confident ~59 bpm out of pure framing, with no cardiac content at all. That is the exact + * [excludeLag] is REQUIRED and has no default, which is the whole point of it. Passing the record + * period switches the guard on; passing null says "this source is not record-tiled" and accepts an + * unguarded estimate. Both are defensible, but only as a decision somebody made: with a default the + * shortest call, `estimateBpm(samples, fs)`, silently selected the unguarded form, and on a tiled + * 101-sample record at 100 Hz that form returns 59 — a believable resting rate manufactured from a + * buffer holding no rhythm at all. That is #194 exactly, and a wrong number nobody chose is the + * failure this whole method is shaped around. Requiring the argument costs one word at each call site + * and makes the unguarded path impossible to take by accident. + * * failure mode behind NOOP's withdrawn PPG->HR estimate, so when [excludeLag] is supplied: * * 1. lags within [excludeLagTolerance] of it cannot win, and @@ -179,7 +188,7 @@ object EcgResearchStats { minBpm: Int = 40, maxBpm: Int = 180, minStrength: Double = 0.3, - excludeLag: Int? = null, + excludeLag: Int?, excludeLagTolerance: Int = 2, octaveTolerance: Double = 0.85, artefactDominance: Double = 1.0, diff --git a/android/app/src/test/java/com/noop/protocol/EcgBpmEstimatorTest.kt b/android/app/src/test/java/com/noop/protocol/EcgBpmEstimatorTest.kt index 4b3eb8b345..196961d522 100644 --- a/android/app/src/test/java/com/noop/protocol/EcgBpmEstimatorTest.kt +++ b/android/app/src/test/java/com/noop/protocol/EcgBpmEstimatorTest.kt @@ -32,7 +32,7 @@ class EcgBpmEstimatorTest { fun recoversSeveralDifferentInjectedRates() { // One match is a coincidence; the method has to move with the input. for (bpm in intArrayOf(45, 52, 70, 88, 110, 132, 150, 170)) { - val got = EcgResearchStats.estimateBpm(beats(bpm), fs) + val got = EcgResearchStats.estimateBpm(beats(bpm), fs, excludeLag = null) assertNotNull("no estimate at $bpm bpm", got) // Lag quantisation at 100 Hz coarsens with rate, so allow a proportional tolerance. val tolerance = maxOf(3.0, bpm * 0.06) @@ -47,7 +47,7 @@ class EcgBpmEstimatorTest { fun theEstimateRisesAndFallsWithTheInput() { // Monotonicity across the band: the ordering of the estimates must match the ordering of the truth. val truth = intArrayOf(50, 75, 100, 125, 150) - val got = truth.map { EcgResearchStats.estimateBpm(beats(it), fs)!! } + val got = truth.map { EcgResearchStats.estimateBpm(beats(it), fs, excludeLag = null)!! } assertTrue("estimates not monotonic with input: $got", got.zipWithNext().all { (a, b) -> a < b }) } @@ -109,16 +109,16 @@ class EcgBpmEstimatorTest { @Test fun noiseAndFlatlineAndTooShortAllReturnNull() { - assertNull(EcgResearchStats.estimateBpm(IntArray(0), fs)) - assertNull("a buffer under one second cannot be judged", EcgResearchStats.estimateBpm(IntArray(fs - 1) { it }, fs)) - assertNull("a flat trace has no variance", EcgResearchStats.estimateBpm(IntArray(400) { 0 }, fs)) + assertNull(EcgResearchStats.estimateBpm(IntArray(0), fs, excludeLag = null)) + assertNull("a buffer under one second cannot be judged", EcgResearchStats.estimateBpm(IntArray(fs - 1) { it }, fs, excludeLag = null)) + assertNull("a flat trace has no variance", EcgResearchStats.estimateBpm(IntArray(400) { 0 }, fs, excludeLag = null)) // Deterministic pseudo-noise: no dominant period should clear the strength floor. var seed = 12345L val noise = IntArray(800) { seed = (seed * 6364136223846793005L + 1442695040888963407L) ((seed shr 33).toInt() % 200) } - assertNull("pseudo-noise should not yield a confident rate", EcgResearchStats.estimateBpm(noise, fs)) + assertNull("pseudo-noise should not yield a confident rate", EcgResearchStats.estimateBpm(noise, fs, excludeLag = null)) } @Test @@ -126,7 +126,7 @@ class EcgBpmEstimatorTest { // The lag score is a mean over the overlap, not a raw sum: an unnormalised sum shrinks with lag and // pulls the winner toward short lags (high BPM). A slow rate in a short-ish buffer is where that // bias showed, so pin it. - val got = EcgResearchStats.estimateBpm(beats(46, seconds = 4.0), fs) + val got = EcgResearchStats.estimateBpm(beats(46, seconds = 4.0), fs, excludeLag = null) assertNotNull(got) assertTrue("a slow rate was pulled high (got $got)", abs(got!! - 46) <= 5) } diff --git a/android/app/src/test/java/com/noop/protocol/EcgResearchStatsTest.kt b/android/app/src/test/java/com/noop/protocol/EcgResearchStatsTest.kt index bed58d2717..c69fa1483f 100644 --- a/android/app/src/test/java/com/noop/protocol/EcgResearchStatsTest.kt +++ b/android/app/src/test/java/com/noop/protocol/EcgResearchStatsTest.kt @@ -103,14 +103,14 @@ class EcgResearchStatsTest { val n = fs * 6 val periodSamples = 60.0 * fs / 72.0 val beat = IntArray(n) { (1000 * Math.sin(2 * Math.PI * it / periodSamples)).toInt() } - val bpm = EcgResearchStats.estimateBpm(beat, fs) + val bpm = EcgResearchStats.estimateBpm(beat, fs, excludeLag = null) assertNotNull(bpm) assertTrue("expected ~72, got $bpm", bpm!! in 68..76) // Flat/near-zero -> no confident rhythm -> null (never a fabricated number). - assertNull(EcgResearchStats.estimateBpm(IntArray(fs * 3) { 0 }, fs)) + assertNull(EcgResearchStats.estimateBpm(IntArray(fs * 3) { 0 }, fs, excludeLag = null)) // Too short -> null. - assertNull(EcgResearchStats.estimateBpm(IntArray(fs / 2) { it }, fs)) + assertNull(EcgResearchStats.estimateBpm(IntArray(fs / 2) { it }, fs, excludeLag = null)) } @Test