diff --git a/AGENTS.md b/AGENTS.md index 37d0db5..f46ddef 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -228,9 +228,15 @@ supporting evidence as the cause. - **Read-backs exist — ask the ring instead of guessing.** `querySupportSpO2Type` (`2/37`) answers NOT_SUPPORT / SLEEP_OXYGEN / TIMING_OXYGEN, and the monitor-state queries `2/6` HR, `2/7` HRV, `2/8` SpO2, `2/45` stress, `2/21` temp each report the configured interval (`0` = off). These are - how you tell "the monitor is switched off" apart from "this ring lacks the sensor" — the open - question for stress (`2/47`) and temperature, both 23-sent/0-answered. Send them + how you tell "the monitor is switched off" apart from "this ring lacks the sensor". Send them **once per connection**, not per poll pass: `runStartup` is also the ~30-minute background sync. + The R100 capture in issue #58 shows what a *useful* answer looks like and what silence means: + `2/6` HR, `2/7` HRV, `2/8` SpO2 all replied `05` (5-minute interval, enabled) and `2/21` temp + replied `06`, while `2/45` stress and `2/37` SpO2-type answered **nothing across 22 sync passes** + — the same ring that also never answers the stress history query `2/47`. On that ring stress is + absent, not switched off. Note the app still advertises `STRESS` for it, because + `CRPCoordinator.capabilities` is a static family set, not something the ring confirmed — a + capability list is not evidence about an individual ring. - **Group 7 is Gomore, not device info — an opcode read off a decompiled builder is a guess until you check its caller.** Firmware was queried on `7/1` and never answered (23 sends, 0 replies), which read like ring firmware ignoring a valid vendor command. It wasn't: every builder in `b1/r` @@ -248,10 +254,17 @@ supporting evidence as the cause. firmware string is not. Sibling group-3 queries confirmed from their callers: `3/0` reset, `3/1` shutDown, `3/4` firmware hash, `3/6` real-time battery, `3/7` wear state, `3/14` restart, `3/22` binding reminder. -- **Temperature history is `2/22`, not `2/48`.** `q.b(2,48)` is the vendor's `querySleepState` - (`d1/b.java` line 650); real temp history is `i0.b(day, frameIndex)` = `q.c(2,22,[day,idx])`, the - same shape as the other timing histories. Its sample layout is still unconfirmed — no non-empty - capture yet — so the reply stays an ack. +- **Temperature history is `2/22`, not `2/48`, and its layout is now CONFIRMED (issue #58).** + `q.b(2,48)` is the vendor's `querySleepState` (`d1/b.java` line 650); real temp history is + `i0.b(day, frameIndex)` = `q.c(2,22,[day,idx])`, the same shape as the other timing histories. + The R100 capture attached to issue #58 is the first non-empty temperature reply anyone has sent + us, and it matches the vendor parser `e1/m` byte for byte: `[day][frameIndex]` then + **little-endian 2-byte tenths of a degree Celsius** per 5-minute slot, 72 slots/frame, terminal + index **3** (four frames/day, like HRV), clamp **28.0–50.0 °C** with anything outside meaning "no + reading" (`e1/m.a`). Decoded in `CRPDecoder.decodeTimingHistory`. Note what the missing decode + cost beyond the samples: with no `TimingHistoryFrame` marker emitted, `CRPSyncEngine` never + advanced the cursor, so the ring was asked for frame 0 on every pass and **never** for frames + 1-3 — 18:00 onward of every day was unreachable. - **The multi-frame follow-up is hardware-validated** (was open on rc3): HR asked frames (0,0)+(0,1) and got both; HRV asked (0,0)…(0,3) and got all four. HR history decoded 27 readings at 00:10–11:35 local (46–104 bpm), HRV 11 readings (30–56 ms), sleep 12 records across light/deep/REM — so the @@ -280,6 +293,227 @@ supporting evidence as the cause. connect" premise is false — match the vendor (query state / apply saved config). And the vendor sends spot measures on a priority path (`insertNotificationMessage`) distinct from config/history (`insertBleMessage`). +- **The R100 (issue #58) is a second CRP ring, and a more talkative one than the R11.** White-label, + ships with the same "Da Rings" app, firmware `MOY-R2E3-*`, catalogued as `WearableModel.R100`. It + advertises a usable name (`R100` / `R100_`) so it matches at scan instead of relying on the + post-connect `fdda` re-route. What its capture settles: temperature history decodes (above), + all-day HR/HRV return real frames, all-day SpO2 returns almost entirely empty ones (2 frames with + data against 44 empty — the ring records little, the decode is fine), and stress is unanswered on + both its history and state opcodes. When a CRP question needs a non-empty capture, this ring is + now the better source. - Whenever you touch CRP measure/sync/all-day behavior, hardware-validate with the ring owner (zaggash) — and for a "measure broken" report, first get a capture of **several** Measure presses with the ring snug and still, to separate a contact failure from a real code bug. + +## Spot measurements: the ring's own verdict beats our window (issue #59) + +**Read this before touching `HRSampleWindow`, `SpotMeasurementGate`, or `RingSyncCoordinator`'s +measure legs.** + +A YCBT ring ends a spot measurement itself with **`04 0e [mode, status]`** — `bArr[0]` is the same +mode byte `03 2f` started with, `bArr[1]` is `1` success / `2` failed / anything else cancelled +(vendor `BaseMeasureActivity.onDataResponse`, `decompiled-smarthealth/.../BaseMeasureActivity.java:259`). +The vendor reads **no value** out of that frame; on success it calls `syncData()` and pulls the +reading from history. We decode it the same way: `RingDecodedEvent.MeasurementComplete` carries the +mode and the verdict and nothing else, and `SpotMeasurementGate` honours it by token so a +completion can only end the measurement it names. + +What the `Ale-Hop2211` captures in #59 established about how these rings actually behave. None of +it is safe to assume away: + +- **Warm-up is not the same as the cached echo.** `HRSampleWindow` drops the first 5 s because the + ring answers instantly with its last stored bpm. That ring sent nothing for 14 s, then spent ~12 s + on a *pre-converged plateau* (47 47 47, 46 46 46) before stepping to the real rate (84 … 81) at + ~26 s. A whole-window median picks the plateau every time: it is both the majority of the window + and the most self-consistent thing in it. **Never settle a median over everything collected.** +- **Which settle rule a run gets is decided by whether the ring ended it** + (`HRSampleWindow.settled(ringChoosesLastSample)`, wired to the ring's `04 0e` success verdict on + *this run*, not to the family's ability to send one). A run that hits our ceiling without a + `04 0e` is one the ring never finished, so it falls back to the consistency gate whatever the + family. A ring that ends its own measurement **logs the last plausible + sample of the run** (vendor band `HEART_RATE_VISIBLE_MIN..MAX` = 40..220), so the app reports + that. RC-3 feedback on #59 established it directly: three spot measurements captured with no stop + command, each read back out of the ring's memory before any app touched it, stored value == + last streamed sample **3/3** (65, 58, 72). It is a discriminating test here, unlike SpO2 where + tail and last coincide — the rate is still **climbing** when the ring stops, so every + tail-weighted rule lands below the ring's answer (94 against the ring's 93 on rc5, and up to + 18 bpm out on those captures). Disagreeing with the ring is not a better number, it is a second + number: the ring's copy arrives on the next sync and ours yields to it (issue #60). A ring with + **no** completion signal keeps the tail rule, because nothing chose its last sample — the leg + just ran out of window. Don't widen the last-sample rule to every family; that is the same + over-generalisation rc5 had to correct for the ring-copy rule. +- **The ring's stored sample is not a converged reading, and that is not ours to fix.** It stops + while the value is still rising (34.2 s, 49.2 s, 34.2 s across those three runs), so "which + sample did the firmware choose" and "is that sample any good" have different answers and only the + first is the app's. The reporter raised this himself and argued against correcting for it: + inventing a better number app-side would disagree with the row the ring re-supplies. +- **These rings stream in bursts.** Three samples about a second apart, then **4-6 s of silence**. + The contact-lost gap was 3 s, so it fired mid-measurement on a ring that was working perfectly and + aborted the leg before the sensor had converged at all. It is 8 s now. Size this against the + burstiest cadence in a capture, never against the average one. +- **SpO2 is not heart rate, and the HR rules must not be copied onto it.** RC-1 feedback on #59 + included an instrumented SpO2 capture from the same ring, and every property differs: samples + start at t+13 s, there is a **24 s** silence in the middle (against HR's 4-6 s), the run lasts + **50 s** (against 35), and the values *rise to a peak of 99 then decline to 94* rather than + converging. So: there is no cached echo to discard here, the HR contact-gap would abort it + outright if it were ever applied to this leg, and HR's tail rule would report the decline. + `Spo2SampleWindow` settles on the **last plausible sample** (vendor band 70–100), because that is + what the ring itself logs. Three sources agree (RC-2 feedback on #59): five captures read back + against the ring's own history matched the last sample 5/5 (a median matched 4/5); the one run + that collapsed 98 → 87 is stored by the ring as 87, so it was a bad measurement, not a rule + failure; and the vendor app (`BloodOxygenMeasureActivity.onEvent`) never settles at all — it + shows each frame and on `04 0e` re-reads the ring's history. Don't put a median or a tail rule + back: anything cleverer than the ring disagrees with the row the ring will later re-supply. + There is still no value-based early exit for this leg — the collapsing run's late burst changed + the answer, so "a value in hand" is not "done"; only `04 0e` (or the ceiling) is. +- **A window ceiling is per family** (`RingSyncEngine.spotHeartRateSeconds`, + `spotSpo2Seconds`). YCBT HR is 45 s because that ring self-terminates at ~35 s; YCBT SpO2 is + 75 s because five captures put its `04 0e` at t+63.1 s (the default 60 s timed out just before + it); everyone else keeps 30 s / 60 s. This is only safe *because* the leg ends on `04 0e` — + raising the default for families that never send one would make every measurement visibly slower + for nothing. +- **`04 0e` success is not an abort for BP and HRV.** Those legs read no value out of the push + (the vendor re-syncs history), and HRV has no live-value frame at all, so treating success as + "stop polling" turned a measurement the ring called successful into a failure. Only the ring's + *failure* verdict ends them early (`pollForValue`'s abort predicate tests `== false`). + +**Collecting a whole run is gated on the ring saying when it is done** +(`RingSyncEngine.signalsMeasurementCompletion`, true only for YCBT). Don't widen it: a leg that +waits for a completion signal no family sends just idles out its window, and the CRP R11 answers a +spot SpO2 with one value after ~48 s of silence and nothing further — waiting past it would turn a +working measurement into a minute-long stare at a progress bar. Families without the signal keep +"first plausible value wins" for SpO2. + +**A spot measurement's output is one reading, not a stream.** While one is settling, the +coordinator closes a gate on that kind's live samples and reopens it before publishing the settled +value once, `spot = true`. The gate is a **bus event** (`PulseEvent.LiveSampleGate`), not a shared +flag: `EventPersistenceSubscriber` collects behind the ring on its own dispatcher, so a flag read at +write time let every sample already queued in the bus through the moment it flipped — the event is +ordered against the samples it governs. Before any of this, every converging PPG estimate was stored +as its own heart-rate row stamped with the moment it arrived — a failed measurement left a whole +train of readings that were never the user's heart rate (this is what prompted issue #60). A live +*workout* is the opposite case: there the stream **is** the data, so a measurement that runs during +one neither closes the gate nor publishes a second row for a reading the stream already stored. + +## A complete sleep record retires only its own run (issue #63) + +A YCBT ring closes a sleep session when the wearer gets up and opens a new one when they settle, +so one night can be two `af fa` records minutes apart (00:12–03:18 and 03:21–07:24 in the report). +`SleepSegmentation` merges those into one stored row, which is right. What was wrong: a complete +record used to be treated as authoritative for **every block of every row it overlapped**, so on the +next sync pass the second record landed on the merged row and wiped the first record's three hours +with its own stale copy — the night read 4 h 06. `completeSessionSurvivors` now retires only the +contiguous run of blocks the packet's interval sits in (overlapping blocks, plus anything abutting +end-to-start with no gap, in both directions). A shortened re-send still retires its own stale head +or tail; a neighbouring session across even a one-minute gap is untouched. The vendor +(`DataUnpack` case 4 → one `Sleep` row per `af fa` block, `queryByYearToDay` returns a list) never +lets one record displace another. `YCBTHealthRecords.sleep` also resynchronises on the `af fa` magic +now, so a record with a wrong declared length can't swallow the sessions after it. + +**A session's `totalMinutes` is time asleep, not the span from its start to its end.** Merging is +what made the two diverge: one row covering both records also covers the minutes between them, so +the reporter's night read 8 h 10 (23:51–08:02) against the 268 + 140 minutes its two records +declared, 6 h 48. The vendor draws the same distinction and the reporter found where — +`SleepActivity:695` builds each history entry as `deepSleepTotal + lightSleepTotal + remTotal`, +carries `wakeDuration` separately, and takes `startTime` from the first record of the day and +`endTime` from the last, never conflating the two. `asleepMinutes(blocks)` (SleepInsights.kt) is +the single definition; `spanMinutes` is the other number. Three things to keep straight: + +- It is **"every stage except AWAKE"**, not "DEEP + LIGHT + REM". Same sum on a ring that labels + its stages (YCBT's sleep tag 4 *is* AWAKE), but `SleepStage.UNKNOWN` is the `else` branch of + every decoder here — an unrecognised stage byte *inside* a sleep record. Naming three stages + would drop minutes that were slept, and could zero a night on a ring we decode only partly. +- **Anything positioning against wall-clock time scales by `spanMinutes`.** The hypnogram's x axis + did use `totalMinutes` and would silently compress and mislabel every tick otherwise. +- **Stored rows were repaired once** (`DataRepairs.repairSleepDurationsIfNeeded`, prefs key + `sleepAsleepMinutesRepair.v1`), because ring history only reaches back about a week and a + re-sync would leave older nights reading the old way forever. It recomputes from each session's + own blocks and skips a session with none rather than zeroing it — `byDay` and `earliestDay` both + filter `totalMinutes > 0`, so a zero hides the night. + +Still open, and a fair ask: showing a split night's two records **separately** as well as merged. + +## Live workout HR on Colmi is a sport session, not an HR stream (issue #64) + +The QRing app never touches the realtime-HR commands during an activity. `SportRunningActivity` +sends `PhoneSportReq.getSportStatus(1, sportType)` = `0x77 01 ` on entry and consumes the +ring's own unsolicited `0x78` telemetry (`DeviceNotifyRsp`: `[dataType][status][durMin×2][bpm] +[steps×3][metres×3][cal×3]` after the opcode) until `0x77 04`. No timer, no keepalive — the ring +drives the cadence, which is the near-constant LED and ~10 s readings the reporter sees there. +`ColmiSyncEngine.startWorkoutHeartRate` does the same. Rules that matter: + +- **Don't re-send the start mid-workout.** The coordinator restarts the stream after every spot + measure; on this path that must be a no-op or the ring's own sport record resets. +- **Silence gets one resume (`0x77 03`), then the session is given up** for the plain HR stream + (`sportWatchdogTick`), as it is when the ring rejects `0x77` outright (the error-flag reply). + Those two are sticky for the engine's life (`sportRejected`), like the `0x1E` refusal — the ring + has shown it will not run a sport session at all. **A `0x78` status 3 is not one of them**: it is + the vendor's own "this session finished" push, which its running screen answers by closing the + screen. It ends the session for the rest of that workout (`sportEndedByRing`) and the next + workout starts a fresh one; making it sticky would let one ring-side timeout cost every later + workout the protocol this issue exists to add. The old `0x1E` → `0x69` path is unchanged + underneath and is what a fallback lands on. +- `0x77` on the command channel is `PhoneSportReq`; the same number as a big-data *action* is + interval temperature on the other characteristic. They are unrelated. +- **Every `0x78` frame decodes to a `SportTelemetry` event first** (kind `sport_telemetry`, in the + redactor's masked set), with the bpm as a separate `HeartRateSample` only when plausible. A + warm-up frame has bpm 0 but still carries live steps, distance and calories; returning nothing + for it made it `unknown` and exported it in clear — the decode-gap-becomes-privacy-gap failure + the diagnostics section below warns about. +- Untested on hardware as of this note: the reporter (issue #64, Colmi R09) has the ring. + +## Deleting a reading needs a tombstone, not just a DELETE (issue #60) + +History measurements are keyed `history::` and written with `upsert`, on purpose: +re-syncing a day the ring still holds must update the same row rather than duplicate it (see the +measurement-duplicate bug). That idempotence is also what would undo a deletion — delete the row and +the next sync writes it straight back, with nothing failing anywhere. + +So `measurement_deletions` (v24) remembers the deletion, `EventPersistenceSubscriber.upsertUnlessDeleted` +is the single gate every deterministic-id write goes through, and `MeasurementDeletion` owns the two +rules callers must not have to remember: tombstone anything regenerable, and delete a blood-pressure +reading as both of its rows. A live reading's id is a fresh UUID nothing regenerates, so it is +deleted without a tombstone — `MeasurementDeletionDao.record` applies that split, and +`MeasurementDeletionTest` guards the id-prefix agreement that makes it work. + +Tombstones ride in the archive (`PulseArchive.measurementDeletions`) because a restore wipes every +table first; without them a backup round trip would forget the deletions while the ring still holds +the days behind them. + +**The ring owns a spot reading it logged itself.** RC-1's doubled rows (two HR rows per +measurement, same minute, 79/82) are the ring **logging the spot reading into its own history** — +confirmed on RC-2 by reading five SpO2 runs back out of the ring's memory at the exact times they +were taken — which a later history sync imports as a `history::` row next to the UUID row +we stored for our settled value. Our row is stored with `sourceRaw = "spot"`, and +`EventPersistenceSubscriber.adoptRingsCopy` deletes it when a history sample of the same kind lands +within 90 s: the ring's row wins because it is the one that regenerates on every sync (so it is the +one a tombstone can hold down). + +**Only a reading the ring itself completed may take part, and the gate is at write time.** A row is +marked `"spot"` only when the ring reported *that run's* success (`04 0e`, carried on the event as +`ringWillLogIt`, which `RingSyncCoordinator` sets from the run's own verdict — a ring that ends a +measurement with its own verdict is one whose vendor app reads the value back out of history). +Everything else stores a plain `"live"` row exactly as before, with nothing that could delete it. +This is per run, not per family: a YCBT run that times out at our ceiling with no `04 0e` was +never logged by the ring, and marking it `"spot"` would let the next sync delete it in favour of an +unrelated all-day grid sample. A history sample the user has tombstoned adopts nothing either — it +is re-sent on every sync, and letting it retire spot rows would delete a retaken reading each time. Do not widen this to all families: CRP and Colmi record all-day +HR/SpO2 on a **five-minute grid**, so with a ±90 s match window most spot measurements would have an +unrelated grid sample within reach and the user's own reading would be deleted in favour of it. + +**Known limit, worth stating when a user asks:** a reading already exported to Health Connect stays +there. The export doesn't retain HC record ids, so there is nothing to delete against. + +## Diagnostics masking keeps the routing header (issue #58) + +`DiagnosticsRedactor.maskPacketHex` masks a health frame's payload but keeps the leading bytes that +say *which* record it is — 6 for CRP (`FD DA 10 len group cmd`), 4 for YCBT, 1 elsewhere. Masking +from byte 1 made every health frame in a report indistinguishable, which is why issue #58's capture +could not answer whether an all-day SpO₂ reply carried samples. Those header bytes are the same ones +the app writes when it *asks* for the record, and outbound queries are exported unmasked, so keeping +them costs no privacy. + +The inverse failure is worth remembering too: CRP temperature frames were exported **with their +values intact**, because an undecoded frame fell through to `command_ack`, which isn't in +`HEALTH_KINDS`. A decode gap silently became a privacy gap. When you add a decoder for a frame that +carries physiological values, check that its `decodedKind` is one the redactor masks. diff --git a/app/src/main/java/com/pulseloop/PulseLoopApplication.kt b/app/src/main/java/com/pulseloop/PulseLoopApplication.kt index 65d91ae..15e966d 100644 --- a/app/src/main/java/com/pulseloop/PulseLoopApplication.kt +++ b/app/src/main/java/com/pulseloop/PulseLoopApplication.kt @@ -27,6 +27,7 @@ class PulseLoopApplication : Application() { // main thread and is prefs-gated so it executes once. Safe to race the first sync — the // ring can't connect before this completes a couple of DELETE statements. appScope.launch { DataRepairs.runIfNeeded(this@PulseLoopApplication) } + appScope.launch { DataRepairs.repairSleepDurationsIfNeeded(this@PulseLoopApplication) } // Home-screen widgets (iOS #44): publish the snapshot on every foreground/background // edge (the iOS scene-phase triggers — catches goal/unit/profile edits that don't run diff --git a/app/src/main/java/com/pulseloop/coach/summaries/CoachSummaryContextBuilder.kt b/app/src/main/java/com/pulseloop/coach/summaries/CoachSummaryContextBuilder.kt index dd9ad8e..bb8974c 100644 --- a/app/src/main/java/com/pulseloop/coach/summaries/CoachSummaryContextBuilder.kt +++ b/app/src/main/java/com/pulseloop/coach/summaries/CoachSummaryContextBuilder.kt @@ -4,6 +4,7 @@ import com.pulseloop.coach.context.CoachContextBuilder import com.pulseloop.coach.context.CoachContextPacket import com.pulseloop.data.PulseLoopDatabase import com.pulseloop.data.entity.* +import com.pulseloop.ring.SleepStage import com.pulseloop.service.* import kotlinx.serialization.Serializable import kotlinx.serialization.encodeToString @@ -107,9 +108,12 @@ object CoachSummaryContextBuilder { val deepPct: Int, val activitySteps: Int?, ) - val deepMin = blocks.filter { it.stageRaw == "deep" }.sumOf { it.durationMinutes } - val lightMin = blocks.filter { it.stageRaw == "light" }.sumOf { it.durationMinutes } - val awakeMin = blocks.filter { it.stageRaw == "awake" }.sumOf { it.durationMinutes } + // stageRaw is persisted as the SleepStage enum NAME, which is uppercase. These three + // matched lowercase literals and were therefore always zero, so every sleep summary the + // coach has ever been given said the user had no deep, light or awake sleep at all. + val deepMin = blocks.filter { it.stageRaw == SleepStage.DEEP.name }.sumOf { it.durationMinutes } + val lightMin = blocks.filter { it.stageRaw == SleepStage.LIGHT.name }.sumOf { it.durationMinutes } + val awakeMin = blocks.filter { it.stageRaw == SleepStage.AWAKE.name }.sumOf { it.durationMinutes } val p = SleepDayPacket( date = session.date.toString(), diff --git a/app/src/main/java/com/pulseloop/coach/tools/ToolImplementations.kt b/app/src/main/java/com/pulseloop/coach/tools/ToolImplementations.kt index f8fac52..0f19d07 100644 --- a/app/src/main/java/com/pulseloop/coach/tools/ToolImplementations.kt +++ b/app/src/main/java/com/pulseloop/coach/tools/ToolImplementations.kt @@ -635,17 +635,18 @@ object ActionTools { if (coordinator == null || !coordinator.isConnected) { ToolResult("""{"status":"unavailable","note":"Ring is not connected — cannot take a live reading."}""") } else { - kotlinx.coroutines.runBlocking { + // The leg's return value is the measurement's only output (issue #59/#60): the + // settled reading, or null when it failed. The live mirrors are deliberately NOT it — + // they hold the last raw sample the ring streamed, which on a converging sensor is the + // pre-converged plateau, and they are not cleared when a measurement fails. Reading + // them here reported a stale 46 bpm as a completed measurement. + val value = kotlinx.coroutines.runBlocking { when (kind) { "hr" -> coordinator.measureHR() "spo2" -> coordinator.measureSpO2() + else -> null } } - val value = when (kind) { - "hr" -> coordinator.latestHRValue - "spo2" -> coordinator.latestSpO2Value - else -> null - } if (value != null) { ToolResult("""{"status":"completed","kind":"$kind","value":$value,"unit":"${if (kind == "hr") "bpm" else "%"}"}""") } else { diff --git a/app/src/main/java/com/pulseloop/data/DataArchive.kt b/app/src/main/java/com/pulseloop/data/DataArchive.kt index 0987227..c975add 100644 --- a/app/src/main/java/com/pulseloop/data/DataArchive.kt +++ b/app/src/main/java/com/pulseloop/data/DataArchive.kt @@ -35,6 +35,12 @@ data class PulseArchive( /** iOS #96 nutrition. Not in iOS's own `DataArchive.swift` — see the note on [MealEntryDTO]. */ val mealEntries: List = emptyList(), val foodProducts: List = emptyList(), + /** + * Issue #60: readings the user deleted. Carried in the archive because a restore wipes every + * table first — without these, a restore would forget the deletions while the ring still holds + * the days behind them, and the next sync would put every deleted reading back. + */ + val measurementDeletions: List = emptyList(), ) @Serializable data class DeviceDTO( @@ -54,6 +60,10 @@ data class PulseArchive( val createdAt: Long, ) +@Serializable data class MeasurementDeletionDTO( + val measurementId: String, val kindRaw: String, val timestamp: Long, val deletedAt: Long, +) + @Serializable data class ActivityDailyDTO( val id: String, val date: Long, val steps: Int = 0, val calories: Double = 0.0, val distanceMeters: Double = 0.0, val activeMinutes: Int = 0, diff --git a/app/src/main/java/com/pulseloop/data/DataArchiveService.kt b/app/src/main/java/com/pulseloop/data/DataArchiveService.kt index 156b816..c5dde91 100644 --- a/app/src/main/java/com/pulseloop/data/DataArchiveService.kt +++ b/app/src/main/java/com/pulseloop/data/DataArchiveService.kt @@ -310,6 +310,12 @@ object DataArchiveService { lastUsedAt = c.long("lastUsedAt"), useCount = c.int_("useCount"), ) }, + measurementDeletions = collect("measurement_deletions") { c -> + MeasurementDeletionDTO( + measurementId = c.str("measurementId"), kindRaw = c.str("kindRaw"), + timestamp = c.long("timestamp"), deletedAt = c.long("deletedAt"), + ) + }, ) } @@ -369,6 +375,18 @@ object DataArchiveService { rawPacketId = m.rawPacketId, createdAt = m.createdAt, )) } + // Restore the tombstones BEFORE anything can sync against them (issue #60): a restored + // history reading the user had deleted must not survive the round trip. + if (archive.measurementDeletions.isNotEmpty()) { + db.measurementDeletionDao().insertAll( + archive.measurementDeletions.map { + MeasurementDeletionEntity( + measurementId = it.measurementId, kindRaw = it.kindRaw, + timestamp = it.timestamp, deletedAt = it.deletedAt, + ) + } + ) + } for (a in archive.activityDaily) { db.activityDailyDao().upsert(ActivityDailyEntity( id = a.id, date = a.date, steps = a.steps, calories = a.calories, @@ -448,20 +466,32 @@ object DataArchiveService { sp.errorMessage?.let { put("errorMessage", it) } }) } + // A backup written before issue #63 stores each night's span as its duration, and the + // one-time repair has already run (and will not again) on the install restoring it — + // so restate every restored night from its own blocks here, exactly as the repair + // does, rather than carry the archived number through verbatim. A session with no + // blocks in the archive keeps what it had; there is nothing to recompute from. + val restoredBlocks = archive.sleepStageBlocks.map { block -> + SleepStageBlockEntity( + id = block.id, sessionId = block.sessionId, startAt = block.startAt, + startMinute = block.startMinute, durationMinutes = block.durationMinutes, + stageRaw = block.stageRaw, + ) + }.groupBy { it.sessionId } for (ss in archive.sleepSessions) { - db.sleepSessionDao().upsert(SleepSessionEntity( + val archived = SleepSessionEntity( id = ss.id, date = ss.date, startAt = ss.startAt, endAt = ss.endAt, totalMinutes = ss.totalMinutes, score = ss.score, syncedAt = ss.syncedAt, sourceRaw = ss.sourceRaw, createdAt = ss.createdAt, updatedAt = ss.updatedAt, - )) - } - for (block in archive.sleepStageBlocks) { - db.sleepStageBlockDao().insert(SleepStageBlockEntity( - id = block.id, sessionId = block.sessionId, startAt = block.startAt, - startMinute = block.startMinute, durationMinutes = block.durationMinutes, - stageRaw = block.stageRaw, - )) + ) + val blocks = restoredBlocks[ss.id].orEmpty() + val restated = if (blocks.isEmpty()) archived else { + val asleep = archived.copy(totalMinutes = com.pulseloop.service.asleepMinutes(blocks)) + asleep.copy(score = com.pulseloop.service.SleepScore.calculate(asleep, blocks).score) + } + db.sleepSessionDao().upsert(restated) } + restoredBlocks.values.flatten().forEach { db.sleepStageBlockDao().insert(it) } for (conv in archive.coachConversations) { db.coachConversationDao().upsert(CoachConversationEntity( id = conv.id, title = conv.title, createdAt = conv.createdAt, diff --git a/app/src/main/java/com/pulseloop/data/DataRepairs.kt b/app/src/main/java/com/pulseloop/data/DataRepairs.kt index 5601ea4..6fd7f1a 100644 --- a/app/src/main/java/com/pulseloop/data/DataRepairs.kt +++ b/app/src/main/java/com/pulseloop/data/DataRepairs.kt @@ -1,6 +1,9 @@ package com.pulseloop.data import android.content.Context +import androidx.room.withTransaction +import com.pulseloop.service.SleepScore +import com.pulseloop.service.asleepMinutes import com.pulseloop.util.TimeUtil /** @@ -45,4 +48,45 @@ object DataRepairs { } prefs.edit().putBoolean(key, true).apply() } + + /** + * Restate every stored night's `totalMinutes` as time asleep rather than the span from its + * start to its end (issue #63). Ring history only reaches back about a week, so a re-sync + * would leave every older night reading the old way indefinitely — and the two numbers differ + * by the awake stretches plus, on a night the ring split into two records, the gap between + * them: 8 h 10 against 6 h 48 on the reporter's night. + * + * Recomputed from each session's own stage blocks, which are exact — they are a run-length + * encoding of a per-minute stage list and are de-overlapped on merge. A session with no blocks + * left is skipped rather than zeroed: there is nothing to recompute from, and a zero would + * hide the night entirely (`byDay` and `earliestDay` both filter on `totalMinutes > 0`). + * Demo rows are repaired too, so a seeded night and a real one report the same kind of number. + * + * The stored `score` is recomputed with it, since the score's denominators moved with the + * definition. And the whole pass is one transaction: it runs at app start alongside the first + * sync, and a row-by-row read-modify-write outside one could overwrite a night the reconcile + * had just rewritten with a stale snapshot of it. + */ + suspend fun repairSleepDurationsIfNeeded( + context: Context, + db: PulseLoopDatabase = PulseLoopDatabase.getInstance(context), + ) { + val prefs = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) + val key = "sleepAsleepMinutesRepair.v1" + if (prefs.getBoolean(key, false)) return + val now = System.currentTimeMillis() + db.withTransaction { + for (session in db.sleepSessionDao().all()) { + val blocks = db.sleepStageBlockDao().forSession(session.id) + if (blocks.isEmpty()) continue + val asleep = asleepMinutes(blocks) + if (asleep == session.totalMinutes) continue + val restated = session.copy(totalMinutes = asleep, updatedAt = now) + db.sleepSessionDao().upsert( + restated.copy(score = SleepScore.calculate(restated, blocks).score) + ) + } + } + prefs.edit().putBoolean(key, true).apply() + } } diff --git a/app/src/main/java/com/pulseloop/data/DemoDataSeeder.kt b/app/src/main/java/com/pulseloop/data/DemoDataSeeder.kt index 5a30109..0aa1855 100644 --- a/app/src/main/java/com/pulseloop/data/DemoDataSeeder.kt +++ b/app/src/main/java/com/pulseloop/data/DemoDataSeeder.kt @@ -281,10 +281,12 @@ object DemoDataSeeder { block } + // `totalMinutes` is time asleep, not the span (issue #63) — the pattern above tiles the + // whole night including its two awake stretches, so the headline is the blocks less those. val session = SleepSessionEntity( id = sessionId, date = wakeDayStart, startAt = sleepStart, endAt = wake, - totalMinutes = totalMinutes, + totalMinutes = com.pulseloop.service.asleepMinutes(blocks), sourceRaw = "demo", ) db.sleepStageBlockDao().deleteBySession(sessionId) @@ -301,7 +303,7 @@ object DemoDataSeeder { val napSession = SleepSessionEntity( id = napId, date = wakeDayStart, startAt = napStart, endAt = napEnd, - totalMinutes = nap.minutes, + totalMinutes = com.pulseloop.service.asleepMinutes(napBlocks), sourceRaw = "demo", ) db.sleepStageBlockDao().deleteBySession(napId) diff --git a/app/src/main/java/com/pulseloop/data/MeasurementDeletion.kt b/app/src/main/java/com/pulseloop/data/MeasurementDeletion.kt new file mode 100644 index 0000000..71453b5 --- /dev/null +++ b/app/src/main/java/com/pulseloop/data/MeasurementDeletion.kt @@ -0,0 +1,64 @@ +package com.pulseloop.data + +import androidx.room.withTransaction +import com.pulseloop.data.entity.MeasurementEntity +import com.pulseloop.ring.MeasurementKind + +/** + * Deleting individual readings (issue #60). + * + * A measurement can be wrong in ways nothing downstream can detect — a ring worn loosely, a + * measurement started by mistake, a reading taken mid-movement — and until now there was no way to + * remove one, so a bad value stayed in the record forever and dragged every average computed over + * it. Deletion only: a recorded health value may be removed, never edited into a different number. + * + * Two rules make a delete actually stick, and both live here so no caller has to remember them: + * + * * **Tombstone anything the ring can re-send.** History rows are keyed `history::` and + * written with `upsert` so a re-synced day is idempotent; without a tombstone the next sync + * would restore exactly the reading the user removed. + * * **Delete a blood-pressure reading as a pair.** One reading is stored as two rows (systolic and + * diastolic) sharing a timestamp. Removing one would leave a half reading that charts as a + * systolic with no diastolic. + */ +object MeasurementDeletion { + + /** The two rows one blood-pressure reading is stored as. */ + private val BLOOD_PRESSURE_KINDS = listOf( + MeasurementKind.BLOOD_PRESSURE_SYSTOLIC, + MeasurementKind.BLOOD_PRESSURE_DIASTOLIC, + ) + + /** + * Delete [measurements] and remember the ones a later sync could rewrite. Atomic: a delete that + * lost its tombstone half would come back on the next sync, which is the bug this exists to + * prevent. + * + * Returns the number of rows removed. + */ + suspend fun delete(db: PulseLoopDatabase, measurements: List): Int { + if (measurements.isEmpty()) return 0 + return db.withTransaction { + db.measurementDeletionDao().record(measurements) + db.measurementDao().deleteByIds(measurements.map { it.id }) + measurements.size + } + } + + /** [delete], resolving ids back to rows first — what the readings list has to hand. */ + suspend fun deleteByIds(db: PulseLoopDatabase, ids: List): Int { + if (ids.isEmpty()) return 0 + return delete(db, db.measurementDao().byIds(ids)) + } + + /** + * Delete the blood-pressure reading taken at [timestamp] — both of its rows, whichever of them + * the caller happened to be looking at. + */ + suspend fun deleteBloodPressureAt(db: PulseLoopDatabase, timestamp: Long): Int { + val rows = BLOOD_PRESSURE_KINDS.flatMap { kind -> + db.measurementDao().range(kind.name, timestamp, timestamp) + } + return delete(db, rows) + } +} diff --git a/app/src/main/java/com/pulseloop/data/PulseLoopDatabase.kt b/app/src/main/java/com/pulseloop/data/PulseLoopDatabase.kt index 3184487..d31ca3a 100644 --- a/app/src/main/java/com/pulseloop/data/PulseLoopDatabase.kt +++ b/app/src/main/java/com/pulseloop/data/PulseLoopDatabase.kt @@ -42,13 +42,16 @@ import com.pulseloop.data.entity.* CoachNotificationRecordEntity::class, MealEntryEntity::class, CachedFoodProductEntity::class, + MeasurementDeletionEntity::class, ], - version = 23, + version = 24, exportSchema = false, ) abstract class PulseLoopDatabase : RoomDatabase() { abstract fun deviceDao(): DeviceDao abstract fun measurementDao(): MeasurementDao + /** Issue #60: the tombstones that keep a deleted reading deleted across re-syncs. */ + abstract fun measurementDeletionDao(): MeasurementDeletionDao abstract fun activityDailyDao(): ActivityDailyDao abstract fun activityBucketDao(): ActivityBucketDao abstract fun deviceMeasurementConfigDao(): DeviceMeasurementConfigDao @@ -107,7 +110,7 @@ abstract class PulseLoopDatabase : RoomDatabase() { "coach_tool_calls", "user_profiles", "user_goals", "raw_packets", "derived_updates", "coach_summaries", "wearable_logs", "coach_notification_records", - "meal_entries", "food_products", + "meal_entries", "food_products", "measurement_deletions", ) @Volatile private var INSTANCE: PulseLoopDatabase? = null @@ -442,6 +445,27 @@ abstract class PulseLoopDatabase : RoomDatabase() { } } + /** + * v23 → v24: the deleted-reading tombstones (issue #60). Deleting a history-sourced + * measurement only sticks if the deletion outlives the row — see + * [com.pulseloop.data.entity.MeasurementDeletionEntity]. + */ + private val MIGRATION_23_24 = object : Migration(23, 24) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL( + """ + CREATE TABLE IF NOT EXISTS `measurement_deletions` ( + `measurementId` TEXT NOT NULL, + `kindRaw` TEXT NOT NULL, + `timestamp` INTEGER NOT NULL, + `deletedAt` INTEGER NOT NULL, + PRIMARY KEY(`measurementId`) + ) + """.trimIndent() + ) + } + } + private fun adoptStableMeasurementIdentities(db: SupportSQLiteDatabase) { db.execSQL("DROP INDEX IF EXISTS `index_measurements_kindRaw_timestamp_sourceRaw`") db.execSQL( @@ -531,6 +555,7 @@ abstract class PulseLoopDatabase : RoomDatabase() { MIGRATION_20_21, MIGRATION_21_22, MIGRATION_22_23, + MIGRATION_23_24, ) // Downgrades only (sideloading an older APK). A blanket destructive // fallback would silently wipe every measurement, sleep session, and diff --git a/app/src/main/java/com/pulseloop/data/dao/Daos.kt b/app/src/main/java/com/pulseloop/data/dao/Daos.kt index 2e05804..4434db9 100644 --- a/app/src/main/java/com/pulseloop/data/dao/Daos.kt +++ b/app/src/main/java/com/pulseloop/data/dao/Daos.kt @@ -73,9 +73,29 @@ interface MeasurementDao { @Insert suspend fun insert(measurement: MeasurementEntity) + /** Timestamps of one kind's rows from one source since [since] — primes the persistence + * subscriber's memory of the spot readings it stored (issue #60). */ + @Query("SELECT timestamp FROM measurements WHERE kindRaw = :kind AND sourceRaw = :source AND timestamp >= :since") + suspend fun timestampsBySource(kind: String, source: String, since: Long): List + + /** Remove one kind's rows from one source inside a window — how a spot reading yields to the + * ring's own copy of it once history supplies that (issue #60). */ + @Query("DELETE FROM measurements WHERE kindRaw = :kind AND sourceRaw = :source AND timestamp BETWEEN :start AND :end") + suspend fun deleteBySourceBetween(kind: String, source: String, start: Long, end: Long): Int + @Upsert suspend fun upsert(measurement: MeasurementEntity) + /** The rows behind a set of ids — how the readings list resolves a user's pick back into the + * entities [com.pulseloop.data.MeasurementDeletion] needs to tombstone (issue #60). */ + @Query("SELECT * FROM measurements WHERE id IN (:ids)") + suspend fun byIds(ids: List): List + + /** Delete one reading (issue #60). Pair with [MeasurementDeletionDao.record] for anything the + * ring could re-sync, or the next history pass writes it back. */ + @Query("DELETE FROM measurements WHERE id IN (:ids)") + suspend fun deleteByIds(ids: List) + @Query("DELETE FROM measurements WHERE sourceRaw = 'demo'") suspend fun clearDemo() @@ -348,6 +368,10 @@ interface SleepSessionDao { @Query("SELECT MIN(date) FROM sleep_sessions WHERE totalMinutes > 0") suspend fun earliestDay(): Long? + /** Every stored session, for the one-time repairs in `DataRepairs`. */ + @Query("SELECT * FROM sleep_sessions") + suspend fun all(): List + @Upsert suspend fun upsert(session: SleepSessionEntity) @@ -598,3 +622,35 @@ interface FoodProductDao { @Query("DELETE FROM food_products") suspend fun clear() } + +/** + * The tombstones behind "delete this reading" (issue #60) — see [MeasurementDeletionEntity] for + * why a delete needs a memory at all. + */ +@Dao +interface MeasurementDeletionDao { + /** Asked on every history write, so a re-sync can't restore a reading the user removed. */ + @Query("SELECT EXISTS(SELECT 1 FROM measurement_deletions WHERE measurementId = :id)") + suspend fun isDeleted(id: String): Boolean + + @Insert(onConflict = OnConflictStrategy.REPLACE) + suspend fun insertAll(rows: List) + + /** + * Remember [measurements] as deleted — but only the ones a later sync could actually rewrite. + * A live reading is stored under a fresh UUID that nothing regenerates, so tombstoning it would + * grow this table for no benefit. + */ + suspend fun record(measurements: List) { + val regenerable = measurements + .filter { it.id.startsWith(HISTORY_ID_PREFIX) } + .map { MeasurementDeletionEntity(measurementId = it.id, kindRaw = it.kindRaw, timestamp = it.timestamp) } + if (regenerable.isNotEmpty()) insertAll(regenerable) + } + + companion object { + /** The prefix `EventPersistenceSubscriber.historyMeasurementId` builds its stable ids from. + * A measurement whose id starts with this is one the ring can hand us again. */ + const val HISTORY_ID_PREFIX = "history:" + } +} diff --git a/app/src/main/java/com/pulseloop/data/entity/CoreEntities.kt b/app/src/main/java/com/pulseloop/data/entity/CoreEntities.kt index 2290f53..6fc8b54 100644 --- a/app/src/main/java/com/pulseloop/data/entity/CoreEntities.kt +++ b/app/src/main/java/com/pulseloop/data/entity/CoreEntities.kt @@ -260,3 +260,25 @@ data class UserGoalEntity( const val DEFAULT_CALORIES = 500 } } + +/** + * A reading the user deleted (issue #60). + * + * Removing a row is not enough on its own. History-sourced measurements are keyed by + * `history::` and written with `upsert`, precisely so a re-sync of a day the ring + * still holds is idempotent — which also means the next sync would put a deleted reading straight + * back. This table is the memory that says not to: the deletion outlives the row, so the same + * reading stays gone across every later sync of the same day. + * + * Only rows the ring can regenerate need an entry; a live reading's id is a fresh UUID that will + * never be written again. [MeasurementDeletionDao.record] applies that rule. + */ +@Entity(tableName = "measurement_deletions") +data class MeasurementDeletionEntity( + /** The deleted measurement's primary key — deterministic, or this row would be pointless. */ + @PrimaryKey val measurementId: String, + val kindRaw: String, + /** The reading's own timestamp, kept so a future retention sweep can age these out. */ + val timestamp: Long, + val deletedAt: Long = System.currentTimeMillis(), +) diff --git a/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsExporter.kt b/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsExporter.kt index 1e1ee5b..fb7f9ee 100644 --- a/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsExporter.kt +++ b/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsExporter.kt @@ -88,7 +88,9 @@ object DiagnosticsExporter { addJsonObject { put("at", Instant.ofEpochMilli(pkt.timestamp).toString()) put("direction", pkt.directionRaw) - put("hex", if (mask) DiagnosticsRedactor.maskPacketHex(pkt.hexPayload, kind) else pkt.hexPayload) + put("hex", if (mask) { + DiagnosticsRedactor.maskPacketHex(pkt.hexPayload, kind, device?.deviceTypeRaw ?: "") + } else pkt.hexPayload) put("decoded", kind) } } diff --git a/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsRedactor.kt b/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsRedactor.kt index 5fa48f5..72ca9aa 100644 --- a/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsRedactor.kt +++ b/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsRedactor.kt @@ -18,18 +18,42 @@ object DiagnosticsRedactor { private val HEALTH_KINDS = setOf( "activity", "activity_bucket", "hr_sample", "spo2_progress", "spo2_result", "sleep_timeline", "history_measurement", "stress_sample", "hrv_sample", "temperature_sample", + "sport_telemetry", ) private val MAC = Regex("\\b([0-9A-Fa-f]{2}:){5}[0-9A-Fa-f]{2}\\b") /** - * For a health-measurement frame, keep the opcode byte and mask the rest (the values live - * in the payload). Non-health frames are returned unchanged. [hex] is contiguous lowercase. + * How many leading bytes of a frame are routing, not measurement, per protocol family. + * + * Masking from byte 1 keeps a health frame's values out of a report but also throws away the + * bytes that say *which* value it was: on CRP the group and command live at offsets 4 and 5, so + * a masked frame could not be told apart from any other health frame, and issue #58 could not + * establish from the attached report whether an all-day SpO₂ reply carried samples or not. + * These headers carry no physiological data — they are the same bytes the app writes when it + * *asks* for the record, and outbound queries are already exported unmasked — so keeping them + * costs no privacy and is most of what a protocol report is for. */ - fun maskPacketHex(hex: String, kind: String): String { + private fun headerBytes(deviceType: String): Int = when (deviceType) { + // `FD DA 10 ` — CRPProtocol.HEADER_SIZE. + "CRP" -> 6 + // ` ` — YCBTFrame.frame(). Every family that drives a + // YCBTDriver, including the hardware-validated R10M path (`RingDeviceType.YCBT`). + "YCBT", "TK5", "COLMI_SMART_HEALTH" -> 4 + // Everything else (Colmi/QRing, jring, LuckRing, RWfit) puts its opcode in byte 0. + else -> 1 + } + + /** + * For a health-measurement frame, keep the routing header and mask the rest (the values live + * in the payload). Non-health frames are returned unchanged. [hex] is contiguous lowercase; + * [deviceType] is the report's `RingDeviceType` name, which decides how long that header is. + */ + fun maskPacketHex(hex: String, kind: String, deviceType: String = ""): String { if (kind !in HEALTH_KINDS || hex.length <= 2) return hex val byteCount = hex.length / 2 - return hex.substring(0, 2) + "··".repeat(byteCount - 1) + val keep = headerBytes(deviceType).coerceAtMost(byteCount - 1) + return hex.substring(0, keep * 2) + "··".repeat(byteCount - keep) } /** Mask BLE MAC addresses anywhere in free text (logcat, log messages, metadata). */ diff --git a/app/src/main/java/com/pulseloop/ring/CRPDecoder.kt b/app/src/main/java/com/pulseloop/ring/CRPDecoder.kt index 76c0fb1..95c2e41 100644 --- a/app/src/main/java/com/pulseloop/ring/CRPDecoder.kt +++ b/app/src/main/java/com/pulseloop/ring/CRPDecoder.kt @@ -270,10 +270,11 @@ object CRPDecoder { /** * Decode a CRP all-day "timing" vital-history reply (group 2). Returns null for a non-timing - * group-2 cmd (e.g. temp cmd 48) so the caller falls back to an ack. Layout, confirmed against - * zaggash's R11 rc2 capture and the vendor parsers `e1/{f,g,d,l}.java`: + * group-2 cmd so the caller falls back to an ack. Layout, confirmed against zaggash's R11 rc2 + * capture and the vendor parsers `e1/{f,g,d,l,m}.java`: * `[day][frameIndex][slot samples…]` — one 5-minute slot per sample, `0` = no reading. - * HR/SpO2/stress use one byte per slot; HRV uses a little-endian 2-byte value per slot. Each + * HR/SpO2/stress use one byte per slot; HRV and temperature use a little-endian 2-byte value + * per slot (temperature in tenths of a degree Celsius — `e1/m.a`). Each * slot's absolute time is `localMidnight(today − day) + (frameIndex*slotsPerFrame + slot)*5min`, * matching the vendor's `w0.b.a()/5` slot indexing. Emits a [RingDecodedEvent.HistoryMeasurement] * per valid slot (invalid/zero slots dropped, per the vendor's per-vital clamp) plus a trailing @@ -286,11 +287,25 @@ object CRPDecoder { val kind: MeasurementKind val twoByte: Boolean val valid: (Int) -> Boolean + // Raw slot value → the unit the app stores. Only temperature is scaled; the rest are + // already in their own units. + var scale = 1.0 when (cmd) { CRPCommands.CMD_QUERY_TIMING_HR -> { kind = MeasurementKind.HEART_RATE; twoByte = false; valid = { it in 40..200 } } CRPCommands.CMD_QUERY_TIMING_SPO2 -> { kind = MeasurementKind.SPO2; twoByte = false; valid = { it in 1..100 } } CRPCommands.CMD_QUERY_TIMING_HRV -> { kind = MeasurementKind.HRV; twoByte = true; valid = { it in 1..300 } } CRPCommands.CMD_QUERY_TIMING_STRESS -> { kind = MeasurementKind.STRESS; twoByte = false; valid = { it in 1..100 } } + // Temperature history (issue #58). Same `[day][frameIndex][slots…]` shape as the + // others, little-endian 2-byte tenths of a degree, clamped by the vendor to 28.0–50.0 °C + // with anything outside meaning "no reading" (`e1/m.a`). The layout was unconfirmed for + // months because every R11 capture came back empty; the R100 capture attached to #58 is + // the first non-empty one (`fdda 10 98 02 16 …` with slots reading 36.3/35.8/36.2 °C), + // and it matches the vendor parser byte for byte. Until it landed these frames fell + // through to a bare ack, so the samples were dropped *and* no next-frame pull was ever + // triggered — the ring was asked for frame 0 forever and never for the rest of the day. + CRPCommands.CMD_QUERY_HISTORY_TEMP -> { + kind = MeasurementKind.TEMPERATURE; twoByte = true; valid = { it in 280..500 }; scale = 0.1 + } else -> return null } // [day][frameIndex] header; anything shorter is malformed. @@ -317,7 +332,7 @@ object CRPDecoder { if (valid(value)) { val globalSlot = frameIndex * slotsPerFrame + slot val ts = midnight.plusSeconds(globalSlot.toLong() * TIMING_SLOT_MINUTES * 60) - events.add(RingDecodedEvent.HistoryMeasurement(kind, value.toDouble(), ts)) + events.add(RingDecodedEvent.HistoryMeasurement(kind, value * scale, ts)) } i += step slot++ diff --git a/app/src/main/java/com/pulseloop/ring/CRPProtocol.kt b/app/src/main/java/com/pulseloop/ring/CRPProtocol.kt index 5b1fb9a..d14def6 100644 --- a/app/src/main/java/com/pulseloop/ring/CRPProtocol.kt +++ b/app/src/main/java/com/pulseloop/ring/CRPProtocol.kt @@ -107,8 +107,13 @@ object CRPCommands { /** Temperature history. **Not 48** — `q.b(2,48)` is the vendor's `querySleepState` (`d1/b.java` * line 650); the real temperature history is `i0.b(day, frameIndex)` = `q.c(2,22, [day, idx])`, * the same `[day, frameIndex]` shape as the other timing histories. We queried 48 for months and - * the ring never answered — see zaggash's 2026-07-25 capture, 23 sends and 0 replies. Its sample - * layout is still unconfirmed by a non-empty capture, so the reply stays an ack for now. */ + * the ring never answered — see zaggash's 2026-07-25 capture, 23 sends and 0 replies. + * + * The sample layout is confirmed as of issue #58, whose R100 capture is the first non-empty + * temperature reply anyone has sent us: little-endian 2-byte tenths of a degree Celsius, one + * per 5-minute slot, 72 slots per frame, four frames per day (terminal index 3). The vendor + * parser is `e1/m` — `d()` splits `[day][frameIndex]` off the front, `e()` walks the rest two + * bytes at a time, and `a()` divides by 10 and rejects anything outside 28.0–50.0 °C. */ const val CMD_QUERY_HISTORY_TEMP = 22 // b1/i0.b: q.c(2,22, [day, frameIndex]) const val HISTORY_DAY_TODAY = 0 // CRPHistoryDay.TODAY; YESTERDAY = 1 diff --git a/app/src/main/java/com/pulseloop/ring/CRPSyncEngine.kt b/app/src/main/java/com/pulseloop/ring/CRPSyncEngine.kt index 056d6a5..5ae4575 100644 --- a/app/src/main/java/com/pulseloop/ring/CRPSyncEngine.kt +++ b/app/src/main/java/com/pulseloop/ring/CRPSyncEngine.kt @@ -160,10 +160,13 @@ class CRPSyncEngine(private val writer: RingCommandWriter?) : RingSyncEngine { } /** The last frame index each timing vital emits before its day is complete (vendor terminal - * index: HR/SpO2/stress finalize at frame 1 — two 144-slot frames; HRV at frame 3 — four - * 72-slot frames). A reply below this index triggers a pull of the next frame. */ - private fun terminalFrameIndex(cmd: Int): Int = - if (cmd == CRPCommands.CMD_QUERY_TIMING_HRV) 3 else 1 + * index: HR/SpO2/stress finalize at frame 1 — two 144-slot frames; HRV and temperature at + * frame 3 — four 72-slot frames, since both carry 2 bytes per slot; `e1/m.d` requests the + * next frame until `3 == index`). A reply below this index triggers a pull of the next frame. */ + private fun terminalFrameIndex(cmd: Int): Int = when (cmd) { + CRPCommands.CMD_QUERY_TIMING_HRV, CRPCommands.CMD_QUERY_HISTORY_TEMP -> 3 + else -> 1 + } /** Build the next-frame query for a timing vital, or null for a non-timing cmd. */ private fun timingQuery(cmd: Int, day: Int, frameIndex: Int): ByteArray? = when (cmd) { @@ -171,6 +174,7 @@ class CRPSyncEngine(private val writer: RingCommandWriter?) : RingSyncEngine { CRPCommands.CMD_QUERY_TIMING_HRV -> CRPProtocol.queryTimingHrvHistory(day, frameIndex) CRPCommands.CMD_QUERY_TIMING_SPO2 -> CRPProtocol.queryTimingSpO2History(day, frameIndex) CRPCommands.CMD_QUERY_TIMING_STRESS -> CRPProtocol.queryTimingStressHistory(day, frameIndex) + CRPCommands.CMD_QUERY_HISTORY_TEMP -> CRPProtocol.queryHistoryTemp(day, frameIndex) else -> null } diff --git a/app/src/main/java/com/pulseloop/ring/ColmiDecoder.kt b/app/src/main/java/com/pulseloop/ring/ColmiDecoder.kt index e23a4ba..4782d38 100644 --- a/app/src/main/java/com/pulseloop/ring/ColmiDecoder.kt +++ b/app/src/main/java/com/pulseloop/ring/ColmiDecoder.kt @@ -66,6 +66,7 @@ object ColmiDecoder { } ColmiCommandID.REALTIME_HEART_RATE_ERROR -> listOf(RingDecodedEvent.HeartRateComplete(_timestamp = now)) + ColmiCommandID.SPORT_NOTIFY -> decodeSportNotify(v, now) ColmiCommandID.NOTIFICATION -> decodeNotification(v, now) ColmiCommandID.BP_READ -> decodeBpResponse(v, now) else -> listOf(RingDecodedEvent.CommandAck(commandId = v[0])) @@ -97,6 +98,23 @@ object ColmiDecoder { ) } + /** + * Sport telemetry the ring pushes during a phone-driven sport session (issue #64). Byte layout + * after the opcode is `DeviceNotifyRsp.loadData` as `SportRunningActivity` reads it: + * `[dataType][status][durMin hi][durMin lo][bpm][steps×3][metres×3][cal×1000 ×3]`. Only the + * bpm is consumed here — the workout's steps and distance come from the phone and the ring's + * own activity history — and QRing shows it only when positive, so zero is a warm-up frame. + */ + private fun decodeSportNotify(v: List, now: Instant): List { + if (v.size < 6) return emptyList() + val bpm = v[5].toInt() + // The telemetry event comes first so it is the frame's diagnostic kind (and masked) even + // on a warm-up frame whose bpm is zero — those still carry steps, distance and calories. + val telemetry = RingDecodedEvent.SportTelemetry(bpm = bpm, _timestamp = now) + return if (bpm in 30..220) listOf(telemetry, RingDecodedEvent.HeartRateSample(bpm = bpm, _timestamp = now)) + else listOf(telemetry) + } + private fun decodeNotification(v: List, now: Instant): List = when (v[1]) { ColmiCommandID.NOTIF_BATTERY -> listOf(RingDecodedEvent.Battery(percent = v[2].toInt())) ColmiCommandID.NOTIF_LIVE_ACTIVITY -> { diff --git a/app/src/main/java/com/pulseloop/ring/ColmiEncoder.kt b/app/src/main/java/com/pulseloop/ring/ColmiEncoder.kt index 276c951..d5e3090 100644 --- a/app/src/main/java/com/pulseloop/ring/ColmiEncoder.kt +++ b/app/src/main/java/com/pulseloop/ring/ColmiEncoder.kt @@ -128,6 +128,26 @@ object ColmiEncoder { ) } + /** + * `PhoneSportReq.getSportStatus(status, sportType)` = `[0x77, status, sportType]` — the QRing + * app's whole live-activity protocol (issue #64). Start on entering the running screen, pause / + * resume from its buttons, stop on finish. The ring answers each with an `AppSportRsp` echo and, + * between start and stop, pushes `0x78` telemetry on its own. + */ + fun phoneSport(status: UByte, sportType: UByte): ByteArray = + byteArrayOf(ColmiCommandID.PHONE_SPORT.toByte(), status.toByte(), sportType.toByte()) + + /** The QRing sport type for one of this app's activity types (`ActivityMeta.ORDER`). Anything + * without a vendor counterpart is "Other sports", which is a real entry in QRing's list. */ + fun sportType(activityType: String): UByte = when (activityType) { + "walk" -> ColmiCommandID.SPORT_TYPE_WALK + "run" -> ColmiCommandID.SPORT_TYPE_RUN + "cycle" -> ColmiCommandID.SPORT_TYPE_CYCLE + "hike" -> ColmiCommandID.SPORT_TYPE_HIKE + "yoga" -> ColmiCommandID.SPORT_TYPE_YOGA + else -> ColmiCommandID.SPORT_TYPE_OTHER + } + fun realtimeHeartRate(enable: Boolean): ByteArray = byteArrayOf(ColmiCommandID.REALTIME_HEART_RATE.toByte(), if (enable) 0x01 else 0x02) diff --git a/app/src/main/java/com/pulseloop/ring/ColmiProtocol.kt b/app/src/main/java/com/pulseloop/ring/ColmiProtocol.kt index 7fcd976..dc8b7ec 100644 --- a/app/src/main/java/com/pulseloop/ring/ColmiProtocol.kt +++ b/app/src/main/java/com/pulseloop/ring/ColmiProtocol.kt @@ -42,6 +42,32 @@ object ColmiCommandID { const val FIND_DEVICE: UByte = 0x50u const val MANUAL_HEART_RATE: UByte = 0x69u // CMD_START_REAL_TIME (105) const val REALTIME_STOP: UByte = 0x6Au // CMD_STOP_REAL_TIME (106) + /** + * Phone-driven sport session (`PhoneSportReq`, opcode 119): `[0x77, status, sportType]`. This + * is how the QRing app runs a live activity — it never touches the realtime-HR commands there. + * While the session is on, the ring pushes [SPORT_NOTIFY] on its own cadence (issue #64). + * Command channel only; [BIG_DATA_INTERVAL_TEMPERATURE] is a big-data *action* that happens + * to share the number on the other characteristic. + */ + const val PHONE_SPORT: UByte = 0x77u + /** Sport telemetry pushed by the ring during a [PHONE_SPORT] session (`DeviceNotifyRsp`, + * opcode 120): `[0x78, dataType, status, durMinHi, durMinLo, bpm, steps×3, metres×3, cal×3]` + * (`SportRunningActivity.MyDeviceNotifyListener`). */ + const val SPORT_NOTIFY: UByte = 0x78u + // PhoneSportReq statuses (SportRunningActivity / SportPrepareActivity). + const val SPORT_START: UByte = 0x01u + const val SPORT_PAUSE: UByte = 0x02u + const val SPORT_RESUME: UByte = 0x03u + const val SPORT_STOP: UByte = 0x04u + /** Status byte the ring reports in [SPORT_NOTIFY] when it ended the session itself. */ + const val SPORT_ENDED_BY_RING: UByte = 0x03u + // PhoneSportReq sport types (SportRunningActivity.sportMap → strings.xml). + const val SPORT_TYPE_WALK: UByte = 0x04u + const val SPORT_TYPE_RUN: UByte = 0x07u + const val SPORT_TYPE_HIKE: UByte = 0x08u + const val SPORT_TYPE_CYCLE: UByte = 0x09u + const val SPORT_TYPE_OTHER: UByte = 0x0Au + const val SPORT_TYPE_YOGA: UByte = 0x16u const val NOTIFICATION: UByte = 0x73u const val BIG_DATA_V2: UByte = 0xBCu const val FACTORY_RESET: UByte = 0xFFu diff --git a/app/src/main/java/com/pulseloop/ring/ColmiSyncEngine.kt b/app/src/main/java/com/pulseloop/ring/ColmiSyncEngine.kt index af55537..4b7866d 100644 --- a/app/src/main/java/com/pulseloop/ring/ColmiSyncEngine.kt +++ b/app/src/main/java/com/pulseloop/ring/ColmiSyncEngine.kt @@ -107,6 +107,30 @@ class ColmiSyncEngine( * frame so the ring's own measurement log records the reading (0 = cancelled). */ private var lastManualBpm = 0 + // Phone-driven sport session (issue #64). See [startWorkoutHeartRate]. + @Volatile private var sportActive = false + private var sportType: UByte = ColmiCommandID.SPORT_TYPE_OTHER + private var sportWatchdogJob: Job? = null + /** Wall-clock of the last `0x78` telemetry frame — the sport watchdog's only evidence that the + * ring is actually in the session it was asked to start. */ + @Volatile private var lastSportFrameAt = 0L + /** One resume is tried per silence before giving the session up; see [sportWatchdogTick]. */ + @Volatile private var sportResumeSent = false + /** + * The ring ended *this workout's* sport session itself (`0x78` status 3 — the vendor's own + * "session finished" push, which its running screen answers by closing the screen). The rest + * of the workout runs on the plain HR stream, but the next workout starts a fresh session: + * a ring timeout, or the user stopping on the ring, says nothing about whether the ring + * supports sport sessions. Cleared by [stopWorkoutHeartRate]. + */ + @Volatile private var sportEndedByRing = false + /** + * This ring refused, or never fed, a phone sport session, so workouts run on the plain HR + * stream ([startHeartRate]) instead. Sticky for the engine's life like [realtimeRejected] and + * for the same reason: a family-wide protocol is either there or it isn't. + */ + @Volatile private var sportRejected = false + companion object { fun isHistoryOpcode(op: UByte): Boolean = op == ColmiCommandID.SYNC_ACTIVITY || @@ -121,6 +145,11 @@ class ColmiSyncEngine( * Generous on purpose: the R09's SpO2 twin streamed warm-up frames for ~25 s before its * first reading, so a shorter window would restart a measurement that was working. */ private const val FALLBACK_STREAM_IDLE_MS = 30_000L + + /** How long a sport session may go without a `0x78` push before the watchdog acts. Wider + * than the ring's ~10 s cadence by a margin for the optical warm-up (~25 s on the R09's + * SpO2 twin) and one missed push. */ + internal const val SPORT_TELEMETRY_IDLE_MS = 45_000L } override fun runStartup() { @@ -206,6 +235,27 @@ class ColmiSyncEngine( onRealtimeHeartRateRejected() return } + // Sport telemetry (0x78) is proof the ring is in the session we asked for. Two things say + // it isn't, and they are not the same thing: the ring *refusing* the start (0x77 with the + // error flag) means this firmware has no sport sessions at all, while status 3 means this + // one ended. Both put the rest of the workout on the plain HR stream; only the first is + // remembered past it. + when (frame?.get(0)?.toUByte()) { + ColmiCommandID.SPORT_NOTIFY -> { + if (frame.size > 2 && frame[2].toUByte() == ColmiCommandID.SPORT_ENDED_BY_RING) { + if (sportActive) { + sportEndedByRing = true + endSportSession(sendStop = false, sticky = false) + } + } else { + lastSportFrameAt = System.currentTimeMillis() + sportResumeSent = false + } + } + (ColmiCommandID.PHONE_SPORT or 0x80u) -> + if (sportActive) endSportSession(sendStop = false, sticky = true) + else -> Unit + } // Any 0x69 *heart-rate* frame is proof the fallback stream is alive. Reading type matters: // a concurrent 0x69/3 SpO2 measure shares the opcode and must not stand in for HR traffic. if (frame?.get(0)?.toUByte() == ColmiCommandID.MANUAL_HEART_RATE && @@ -656,6 +706,81 @@ class ColmiSyncEngine( } } + /** + * A live workout is a **ring-side sport session**, not a bare HR stream (issue #64). + * + * The QRing app never uses the realtime-HR commands during an activity. Its running screen + * sends `PhoneSportReq.getSportStatus(1, sportType)` = `0x77 01 ` on entry, and from + * then on the ring pushes `0x78` telemetry — duration, **bpm**, steps, distance, calories — + * on its own cadence until `0x77 04`. There is no app-side timer and no keepalive. That is + * the near-constant green LED and ~10 s readings the reporter sees in QRing; PulseLoop's + * chain of one-shot `0x69` measurements re-armed after 30 s of silence is the once-a-minute + * flash he sees here. + * + * Idempotent in the way the coordinator relies on: a restart after a spot measure must not + * re-send the start, which would reset the ring's own sport record mid-workout. If the ring + * has stopped pushing, the watchdog re-issues a *resume* first and only then gives up on the + * session ([sportWatchdogTick]). A ring that rejects `0x77`, or never pushes `0x78` even after + * a resume, is moved onto [startHeartRate] and stays there ([sportRejected]); a ring that + * merely ended *this* session keeps the protocol for the next workout ([sportEndedByRing]). + */ + override fun startWorkoutHeartRate(activityType: String) { + if (sportRejected || sportEndedByRing) { startHeartRate(); return } + if (sportActive) return + sportActive = true + sportType = encoder.sportType(activityType) + lastSportFrameAt = System.currentTimeMillis() + sportResumeSent = false + writer?.enqueue(encoder.phoneSport(ColmiCommandID.SPORT_START, sportType)) + sportWatchdogJob?.cancel() + sportWatchdogJob = scope.launch { + while (isActive) { + delay(REALTIME_KEEPALIVE_MS) + sportWatchdogTick(System.currentTimeMillis()) + } + } + } + + /** + * One pass of the sport watchdog. Silence past [SPORT_TELEMETRY_IDLE_MS] gets a single + * `0x77 03` (resume — QRing's own answer to a paused session, and harmless to a running one); + * silence that outlasts that as well means this ring is not going to feed the session, and + * the workout falls back to the plain HR stream rather than spending the rest of it blind. + */ + internal fun sportWatchdogTick(now: Long) { + if (!sportActive) return + if (now - lastSportFrameAt < SPORT_TELEMETRY_IDLE_MS) return + if (!sportResumeSent) { + sportResumeSent = true + lastSportFrameAt = now + writer?.enqueue(encoder.phoneSport(ColmiCommandID.SPORT_RESUME, sportType)) + return + } + endSportSession(sendStop = true, sticky = true) + } + + /** End the running sport session and put the rest of the workout on the plain HR stream. + * [sticky] remembers the failure for every later workout on this connection — true only when + * the ring has shown it will not run a sport session at all. */ + private fun endSportSession(sendStop: Boolean, sticky: Boolean) { + sportWatchdogJob?.cancel(); sportWatchdogJob = null + sportActive = false + if (sticky) sportRejected = true + if (sendStop) writer?.enqueue(encoder.phoneSport(ColmiCommandID.SPORT_STOP, sportType)) + startHeartRate() + } + + override fun stopWorkoutHeartRate() { + sportWatchdogJob?.cancel(); sportWatchdogJob = null + sportEndedByRing = false + if (sportActive) { + sportActive = false + writer?.enqueue(encoder.phoneSport(ColmiCommandID.SPORT_STOP, sportType)) + } + // Tear down the plain stream too, for a workout that fell back onto it. + stopHeartRate() + } + override fun stopHeartRate() { realtimeKeepaliveJob?.cancel(); realtimeKeepaliveJob = null if (manualHRActive) { diff --git a/app/src/main/java/com/pulseloop/ring/PulseEventBus.kt b/app/src/main/java/com/pulseloop/ring/PulseEventBus.kt index 254dd9a..446c3cd 100644 --- a/app/src/main/java/com/pulseloop/ring/PulseEventBus.kt +++ b/app/src/main/java/com/pulseloop/ring/PulseEventBus.kt @@ -44,13 +44,42 @@ sealed class PulseEvent { data class ActivityUpdate(val timestamp: java.time.Instant, val steps: Int, val distanceMeters: Double, val calories: Double) : PulseEvent() data class ActivityBucket(val timestamp: java.time.Instant, val steps: Int, val distanceMeters: Double) : PulseEvent() data object ActivitySyncReset : PulseEvent() - data class HeartRateSample(val bpm: Int, val timestamp: java.time.Instant) : PulseEvent() + /** + * [spot] marks the one settled reading a spot measurement publishes for itself (issue #60); + * false for the ring's live stream. [ringWillLogIt] additionally says this ring writes that + * measurement into its **own** history, so a later sync will hand the same reading back and + * the two copies have to be reconciled — true only for a family whose ring reports its own + * completion (`RingSyncEngine.signalsMeasurementCompletion`), which is the same property: + * the ring decides the number and the vendor app re-reads it. Meaningless unless [spot]. + */ + data class HeartRateSample( + val bpm: Int, + val timestamp: java.time.Instant, + val spot: Boolean = false, + val ringWillLogIt: Boolean = false, + ) : PulseEvent() data class HeartRateComplete(val timestamp: java.time.Instant) : PulseEvent() - data class Spo2Result(val value: Int, val timestamp: java.time.Instant) : PulseEvent() + /** [spot] and [ringWillLogIt] as on [HeartRateSample]. */ + data class Spo2Result( + val value: Int, + val timestamp: java.time.Instant, + val spot: Boolean = false, + val ringWillLogIt: Boolean = false, + ) : PulseEvent() /** The ring ended a live-SpO₂ run (error or natural finish) — no more results coming. */ data class Spo2Complete(val timestamp: java.time.Instant) : PulseEvent() /** A live measurement command was refused (not worn, sensor busy, unsupported). */ data class MeasurementRejected(val mode: Int) : PulseEvent() + /** The ring ended a spot measurement on its own and reported the verdict (issue #59). + * [mode] names which measurement finished; [success] is the ring's own success byte. */ + data class MeasurementComplete(val mode: Int, val success: Boolean, val timestamp: java.time.Instant) : PulseEvent() + /** + * The coordinator opening or closing the gate on storing [kind]'s live samples (issue #60). + * Travels through the bus rather than a shared flag so it is ordered against the samples it + * governs: everything the ring streamed before the gate reopened is dropped, whatever the + * persistence collector's lag, and the settled reading published after it is stored. + */ + data class LiveSampleGate(val kind: MeasurementKind, val closed: Boolean) : PulseEvent() data class BloodPressureSample( val systolic: Int, val diastolic: Int, diff --git a/app/src/main/java/com/pulseloop/ring/RingDecodedEvent.kt b/app/src/main/java/com/pulseloop/ring/RingDecodedEvent.kt index 497c202..6f8da6d 100644 --- a/app/src/main/java/com/pulseloop/ring/RingDecodedEvent.kt +++ b/app/src/main/java/com/pulseloop/ring/RingDecodedEvent.kt @@ -69,9 +69,11 @@ sealed class RingDecodedEvent { is ActivityBucket -> this._timestamp is HeartRateSample -> this._timestamp is HeartRateComplete -> this._timestamp + is SportTelemetry -> this._timestamp is Spo2Progress -> this._timestamp is Spo2Result -> this._timestamp is Spo2Complete -> this._timestamp + is MeasurementComplete -> this._timestamp is SleepTimeline -> this._timestamp is HistoryMeasurement -> this._timestamp is StressSample -> this._timestamp @@ -129,6 +131,21 @@ sealed class RingDecodedEvent { override val debugJSON = """{"bpm":$bpm,"error":$isError}""" } + /** + * A Colmi `0x78` sport-session telemetry push (issue #64), emitted for *every* such frame so + * the diagnostics redactor has a health kind to mask. The bpm rides separately as a + * [HeartRateSample] when it is plausible; a warm-up frame carries bpm 0 but still carries the + * workout's live step count, distance and calories, which must not reach a report in clear. + */ + data class SportTelemetry( + val bpm: Int, + val _timestamp: Instant + ) : RingDecodedEvent() { + override val kind = "sport_telemetry" + override val confidence = DecodeConfidence.PARTIAL + override val debugJSON = """{"bpm":$bpm}""" + } + data class HeartRateComplete( val _timestamp: Instant ) : RingDecodedEvent() { @@ -163,6 +180,28 @@ sealed class RingDecodedEvent { override val debugJSON = "{}" } + /** + * The ring itself ended a spot measurement and said how it went (YCBT `04 0e`, issue #59). + * + * [mode] is the same measurement-mode byte the start command carried ([YCBTMeasurementMode]), + * so a completion can only ever end the measurement it names. [success] is the vendor's + * `bArr[1] == 1`; 2 is "failed" and anything else "cancelled", which are both failures here. + * + * Carries no value on purpose: the vendor app reacts to a success by re-syncing history + * (`BaseMeasureActivity.onDataResponse` → `syncData()`), never by reading a reading out of + * this frame. Its job is to say *when* the measurement is over, which is exactly what the app + * could not tell before — the ring goes quiet and the leg idled out its whole window. + */ + data class MeasurementComplete( + val mode: Int, + val success: Boolean, + val _timestamp: Instant + ) : RingDecodedEvent() { + override val kind = "measurement_complete" + override val confidence = DecodeConfidence.KNOWN + override val debugJSON = """{"mode":$mode,"success":$success}""" + } + data class SleepTimeline( val _timestamp: Instant, val stages: List, diff --git a/app/src/main/java/com/pulseloop/ring/RingEventBridge.kt b/app/src/main/java/com/pulseloop/ring/RingEventBridge.kt index 0ecf489..acfac28 100644 --- a/app/src/main/java/com/pulseloop/ring/RingEventBridge.kt +++ b/app/src/main/java/com/pulseloop/ring/RingEventBridge.kt @@ -83,6 +83,9 @@ object RingEventBridge { is RingDecodedEvent.Status -> listOf(PulseEvent.DeviceStateChanged(RingConnectionState.CONNECTED, decoded.address, decoded.firmware)) + // Sport telemetry is a diagnostics marker; its bpm arrives as its own HeartRateSample. + is RingDecodedEvent.SportTelemetry -> emptyList() + is RingDecodedEvent.TimeSyncAck, is RingDecodedEvent.CommandAck, is RingDecodedEvent.Unknown -> emptyList() @@ -97,6 +100,9 @@ object RingEventBridge { is RingDecodedEvent.Spo2Complete -> listOf(PulseEvent.Spo2Complete(decoded._timestamp)) + is RingDecodedEvent.MeasurementComplete -> + listOf(PulseEvent.MeasurementComplete(decoded.mode, decoded.success, decoded._timestamp)) + is RingDecodedEvent.Spo2Progress -> emptyList() // Phase 1 does not fan these out diff --git a/app/src/main/java/com/pulseloop/ring/WearableDriver.kt b/app/src/main/java/com/pulseloop/ring/WearableDriver.kt index 6bd9f86..daf81ae 100644 --- a/app/src/main/java/com/pulseloop/ring/WearableDriver.kt +++ b/app/src/main/java/com/pulseloop/ring/WearableDriver.kt @@ -120,10 +120,53 @@ data class UserProfileValues( * Per-device orchestration of command flows. */ interface RingSyncEngine { + companion object { + /** + * Fallback bound on the live-HR leg of a spot measurement, for a family that gives no + * other signal that it has finished. Long enough for a settled optical reading, short + * enough that a ring which will never produce one doesn't hold the sensor on. + */ + const val DEFAULT_SPOT_HEART_RATE_SECONDS = 30 + /** Default ceiling on the SpO₂ leg. iOS raised this 40 → 60 (`c8969a4`): the R99's + * successful sweep took 38 s while another ran past 41 s with no result. */ + const val DEFAULT_SPOT_SPO2_SECONDS = 60 + } + /** True only for protocols with one native command that returns a combined vitals packet. * Capability bits such as manual BP/glucose do not imply this transport feature. */ val supportsCombinedMeasurement: Boolean get() = false + /** + * How long the live-HR leg of a spot measurement may run on this family (issue #59). + * + * A ceiling, not a duration: the leg ends the moment the ring says it is done. Only a family + * that never says so actually spends this long, which is why it can be raised for a family + * that *does* — the ring in #59 needs ~26 s of warm-up before its PPG converges and ends the + * measurement itself at ~35 s, so at the default it was cut off just as its readings became + * real, while raising the default for everyone would make every other family's measurement + * visibly slower for nothing. + */ + val spotHeartRateSeconds: Int get() = DEFAULT_SPOT_HEART_RATE_SECONDS + + /** + * Ceiling on the SpO₂ leg of a spot measurement, in seconds. The same reasoning as + * [spotHeartRateSeconds]: a family whose ring ends the measurement itself may need a longer + * fallback bound than the default, because the ring's own `04 0e` is what actually ends the + * leg and the window must be long enough for it to arrive. + */ + val spotSpo2Seconds: Int get() = DEFAULT_SPOT_SPO2_SECONDS + + /** + * True when this family's ring tells us a spot measurement has ended (issue #59's `04 0e`). + * + * It gates whether a leg may keep collecting: a leg that waits for a completion signal no + * family sends would simply idle out its whole window, which is why the SpO2 leg keeps its + * "first plausible value wins" behaviour everywhere else. The CRP R11 in particular answers a + * spot SpO2 with one value after ~48 s of silence and nothing further — waiting past it would + * turn a working measurement into a minute-long stare at a progress bar for no gain. + */ + val signalsMeasurementCompletion: Boolean get() = false + fun runStartup() fun handle(event: RingDecodedEvent) @@ -144,6 +187,16 @@ interface RingSyncEngine { fun syncSleepNow() {} fun startHeartRate() fun stopHeartRate() + /** + * Begin the live-HR stream for a workout of [activityType] (`ActivityMeta.ORDER`). Defaults to + * the plain stream; a family whose vendor app runs a live activity as a ring-side *sport + * session* rather than a bare HR stream overrides this (Colmi, issue #64). Idempotent: the + * coordinator calls it again after every interruption to bring the stream back. + */ + fun startWorkoutHeartRate(activityType: String) { startHeartRate() } + /** End what [startWorkoutHeartRate] began. Distinct from [stopHeartRate] so a spot measure's + * stop mid-workout cannot end the ring's sport session. */ + fun stopWorkoutHeartRate() { stopHeartRate() } fun measureHeartRateSpot() { startHeartRate() } fun startSpO2() fun stopSpO2() diff --git a/app/src/main/java/com/pulseloop/ring/YCBTDecoder.kt b/app/src/main/java/com/pulseloop/ring/YCBTDecoder.kt index e373ace..8c9276d 100644 --- a/app/src/main/java/com/pulseloop/ring/YCBTDecoder.kt +++ b/app/src/main/java/com/pulseloop/ring/YCBTDecoder.kt @@ -135,14 +135,37 @@ class YCBTDecoder { if (events.isEmpty()) listOf(RingDecodedEvent.CommandAck(commandId = cmd.toUByte())) else events } YCBTDevControl.MEASUREMENT_RESULT -> { - // SmartHealth acknowledges this push but its proprietary unpackParseData layout - // is not available in the decompile. Do not infer mode/result fields from bytes. - listOf(RingDecodedEvent.CommandAck(commandId = cmd.toUByte())) + measurementResultEvents(payload, now) + ?: listOf(RingDecodedEvent.CommandAck(commandId = cmd.toUByte())) } else -> listOf(RingDecodedEvent.CommandAck(commandId = cmd.toUByte())) } } + /** + * `04 0e` — the ring finished a spot measurement and is reporting the verdict (issue #59). + * + * Layout, read off the vendor app rather than guessed: `BaseMeasureActivity.onDataResponse` + * (`decompiled-smarthealth/.../home/activity/BaseMeasureActivity.java:259`) requires + * `length > 1`, matches `bArr[0]` against the screen's own `getType()` — the same mode byte + * `appStartMeasurement(onOff, type)` sent on `03 2f` — and then switches on `bArr[1]`: + * `1` success, `2` failed, anything else cancelled. + * + * The vendor reads no value out of this frame (on success it calls `syncData()` and pulls the + * reading from history), so neither do we. Returns null for a payload the vendor would have + * ignored, so the caller falls back to a bare ack. + */ + private fun measurementResultEvents(p: ByteArray, now: Instant): List? { + if (p.size < 2) return null + return listOf( + RingDecodedEvent.MeasurementComplete( + mode = p[0].toInt() and 0xFF, + success = (p[1].toInt() and 0xFF) == YCBTDevControl.RESULT_SUCCESS, + _timestamp = now, + ) + ) + } + private fun measurementStatusEvents(p: ByteArray, now: Instant): List { if (p.size < 3) return emptyList() val value = p[2].toInt() and 0xFF diff --git a/app/src/main/java/com/pulseloop/ring/YCBTHealthRecords.kt b/app/src/main/java/com/pulseloop/ring/YCBTHealthRecords.kt index 28943cb..62628d3 100644 --- a/app/src/main/java/com/pulseloop/ring/YCBTHealthRecords.kt +++ b/app/src/main/java/com/pulseloop/ring/YCBTHealthRecords.kt @@ -164,37 +164,51 @@ object YCBTHealthRecords { // MARK: Sleep (variable-length sessions) + /** One `af fa` record's stage segment, kept with its own start (issue #63). */ + private data class SleepSegment(val stage: SleepStage, val startSeconds: Int, val seconds: Int) + fun sleep(buffer: ByteArray): List { val headerLength = 20 val segmentLength = 8 val events = mutableListOf() var cursor = 0 while (cursor + headerLength <= buffer.size) { + // Every session opens with `af fa` (DataUnpack case 4 reads and discards both bytes). + // A record whose declared length disagrees with its real one would otherwise leave + // the cursor mid-record and silently mis-parse every later session of the night + // (issue #63 is exactly a multi-session night), so resynchronise on the magic. + if (!isSleepSessionHeader(buffer, cursor)) { + val next = nextSleepSessionHeader(buffer, cursor + 1) ?: break + cursor = next + continue + } val recordLength = YCBTBytes.u16(buffer, cursor + 2) + // The vendor reads the session's own bounds out of the header (DataUnpack's + // `startTime` at +4, `endTime` at +8) rather than inferring them from the segments. + val headerStart = YCBTBytes.u32(buffer, cursor + 4) + val headerEnd = YCBTBytes.u32(buffer, cursor + 8) val segmentsStart = cursor + headerLength val declared = maxOf(0, recordLength - headerLength) / segmentLength val available = (buffer.size - segmentsStart) / segmentLength val segmentCount = minOf(declared, available) - val stages = mutableListOf() - var sessionStart: Instant? = null + val segments = mutableListOf() val seenStarts = mutableSetOf() for (index in 0 until segmentCount) { val offset = segmentsStart + index * segmentLength val stage = sleepStage(buffer[offset].toInt() and 0xFF) ?: continue val segmentStart = YCBTBytes.u32(buffer, offset + 1) + // The vendor de-duplicates on the segment's start time (`sleepStartTime`). if (!seenStarts.add(segmentStart)) continue - val segmentSeconds = YCBTBytes.u24(buffer, offset + 5) - if (sessionStart == null) sessionStart = YCBTBytes.date(segmentStart) - val remaining = MAX_SLEEP_SESSION_MINUTES - stages.size - if (remaining <= 0) break - val minutes = kotlin.math.round(segmentSeconds / 60.0).toInt().coerceIn(1, remaining) - repeat(minutes) { stages.add(stage) } + segments.add(SleepSegment(stage, segmentStart, YCBTBytes.u24(buffer, offset + 5))) } - if (sessionStart != null && stages.isNotEmpty()) { + val stages = placeStages(segments, headerStart, headerEnd) + if (segments.isNotEmpty() && stages.isNotEmpty()) { + val start = if (usableHeaderBounds(headerStart, headerEnd)) headerStart + else segments.first().startSeconds events.add( RingDecodedEvent.SleepTimeline( - _timestamp = sessionStart, + _timestamp = YCBTBytes.date(start), stages = stages, completeSession = true, ) @@ -205,6 +219,73 @@ object YCBTHealthRecords { return events } + private fun usableHeaderBounds(startSeconds: Int, endSeconds: Int): Boolean = + startSeconds > 0 && endSeconds > startSeconds && + (endSeconds - startSeconds) / 60 <= MAX_SLEEP_SESSION_MINUTES + + /** + * A record's minute-by-minute stage timeline (issue #63). + * + * The stored run has to end where the ring says the session ended, because + * `completeSessionSurvivors` grows its retirement run across blocks that abut end-to-start. + * Concatenating `round(seconds / 60)` per segment from the first segment's start does not: + * segments carry a one-second gap between each pair and each rounds independently, so a + * night's derived end drifts from its real one — 470 minutes against a declared 474 on the + * captured night in `YCBTHealthRecordsTest`. A single minute of drift in the other direction + * is enough to make one record of a split night abut the next, and the whole of the earlier + * session is then retired in favour of the later one: 5 h 22 of a two-record night vanished + * that way. + * + * So each segment is placed at its own `sleepStartTime` for its own `sleepLen`, and the run + * spans exactly the header's `startTime`..`endTime`. The one-second gaps round away against + * the minute grid; a gap the ring really left reads as wake, which is what it is. + * + * A record with unusable header bounds (a synthetic or truncated one) keeps the old + * concatenation, since there is nothing better to place against. + */ + private fun placeStages( + segments: List, + headerStart: Int, + headerEnd: Int, + ): List { + if (segments.isEmpty()) return emptyList() + if (!usableHeaderBounds(headerStart, headerEnd)) { + val stages = mutableListOf() + for (segment in segments) { + val remaining = MAX_SLEEP_SESSION_MINUTES - stages.size + if (remaining <= 0) break + val minutes = kotlin.math.round(segment.seconds / 60.0).toInt().coerceIn(1, remaining) + repeat(minutes) { stages.add(segment.stage) } + } + return stages + } + val total = kotlin.math.round((headerEnd - headerStart) / 60.0).toInt() + .coerceIn(1, MAX_SLEEP_SESSION_MINUTES) + // AWAKE is the honest filler: the ring reports wake as its own segment type (0xf4), so a + // minute no segment claims is one the ring did not call sleep. + val timeline = MutableList(total) { SleepStage.AWAKE } + for (segment in segments) { + val from = kotlin.math.round((segment.startSeconds - headerStart) / 60.0).toInt() + val until = kotlin.math.round( + (segment.startSeconds.toLong() + segment.seconds - headerStart) / 60.0 + ).toInt() + for (minute in maxOf(0, from) until minOf(total, until)) timeline[minute] = segment.stage + } + return timeline + } + + private fun isSleepSessionHeader(buffer: ByteArray, at: Int): Boolean = + at + 1 < buffer.size && buffer[at] == 0xAF.toByte() && buffer[at + 1] == 0xFA.toByte() + + private fun nextSleepSessionHeader(buffer: ByteArray, from: Int): Int? { + var i = from + while (i + 1 < buffer.size) { + if (isSleepSessionHeader(buffer, i)) return i + i++ + } + return null + } + private fun sleepStage(tag: Int): SleepStage? { return when (tag and 0x0f) { 1 -> SleepStage.DEEP diff --git a/app/src/main/java/com/pulseloop/ring/YCBTSyncEngine.kt b/app/src/main/java/com/pulseloop/ring/YCBTSyncEngine.kt index 1c7debf..bdaeb1c 100644 --- a/app/src/main/java/com/pulseloop/ring/YCBTSyncEngine.kt +++ b/app/src/main/java/com/pulseloop/ring/YCBTSyncEngine.kt @@ -20,6 +20,27 @@ class YCBTSyncEngine( private var requestActivityAfterStartupHistory = false private var historyCapabilities = profile.baselineCapabilities + /** + * 45 s, not the family default (issue #59). A captured YCBT run shows the PPG spending its + * first ~26 s on a flat pre-converged plateau and the ring ending the measurement itself at + * ~35 s with `04 0e`; at 30 s the leg was cut off within a few samples of the real reading and + * reported that it never steadied. The measurement still ends on `04 0e` — this only raises + * the fallback ceiling far enough that the ring gets to send it. + */ + override val spotHeartRateSeconds: Int = 45 + + /** + * Five instrumented SpO₂ captures on the #59 ring all ended with `04 0e` at **t+63.1 s**, to + * the tenth of a second, regardless of when samples arrived or stopped (the RC-1 capture ended + * at t+50 s, so it is per-session, not a constant). At the 60 s default the leg timed out three + * seconds before the ring's own verdict and settled on the fallback; this leaves room for it. + */ + override val spotSpo2Seconds: Int = 75 + + /** These rings end a spot measurement with `04 0e` (issue #59), confirmed on hardware for both + * heart rate (`{00 01}`) and SpO2 (`{02 01}`). */ + override val signalsMeasurementCompletion: Boolean = true + companion object { private val HISTORY_TYPES: List = listOf( YCBTHistoryType.SPORT, YCBTHistoryType.SLEEP, YCBTHistoryType.HEART, diff --git a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index 1ba5e92..7dba90a 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -22,6 +22,24 @@ class EventPersistenceSubscriber( */ private val onDataPersisted: (() -> Unit)? = null, ) { + /** + * The kinds whose live samples are currently *not* stored, because a spot measurement is + * settling them (issue #60). Driven by [PulseEvent.LiveSampleGate], which the coordinator + * sends through the same bus as the samples, so this collector sees the gate close before the + * samples it must drop and reopen before the settled reading it must keep — whatever its lag + * behind the ring. Only touched from the collector, so it needs no lock. + */ + private val closedGates = mutableSetOf() + + /** + * Timestamps of the spot readings this app stored itself, per kind — the rows a ring that + * logs its own spot measurements will later re-supply from history (issue #60, RC-1: two HR + * rows per measurement, same minute, values a few bpm apart). Loaded lazily from the table + * and kept current here so the history path can answer "is this the ring's copy of one of + * ours?" without a query per history sample. See [adoptRingsCopy]. + */ + private val spotReadings = mutableMapOf>() + private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob()) private var job: Job? = null @@ -68,8 +86,80 @@ class EventPersistenceSubscriber( else -> false } + /** + * Write a measurement the ring can re-supply — unless the user deleted it (issue #60). + * + * Every caller here uses a deterministic `history::` id so that re-syncing a + * day the ring still holds updates the same row instead of duplicating it. That idempotence is + * exactly what would undo a deletion, so the tombstone is checked on the way in. A reading with + * no tombstone is written as before. + */ + private suspend fun upsertUnlessDeleted(measurement: MeasurementEntity): Boolean { + if (db.measurementDeletionDao().isDeleted(measurement.id)) return false + db.measurementDao().upsert(measurement) + return true + } + + /** + * A live reading of [kind]: the ring's stream, or a spot measurement's settled value. + * + * [awaitingRingsCopy] marks the second case *on a ring that logs its own spot measurements*, + * so a later history sync can recognise the ring's copy and replace ours ([adoptRingsCopy]). + * A spot reading from a ring that does not log one is stored exactly as before — a plain live + * row, with no pending reconciliation and nothing that could delete it. + */ + private suspend fun storeLiveReading( + kind: MeasurementKind, + value: Double, + unit: String, + at: Long, + awaitingRingsCopy: Boolean, + ) { + db.measurementDao().insert(MeasurementEntity( + kindRaw = kind.name, value = value, unit = unit, timestamp = at, + sourceRaw = if (awaitingRingsCopy) SOURCE_SPOT else "live", + )) + if (awaitingRingsCopy) spotReadingsOf(kind).add(at) + } + + private suspend fun spotReadingsOf(kind: MeasurementKind): MutableList = + spotReadings.getOrPut(kind) { + db.measurementDao().timestampsBySource(kind.name, SOURCE_SPOT, System.currentTimeMillis() - SPOT_LOOKBACK_MS) + .toMutableList() + } + + /** + * The ring's copy of a spot reading replaces ours (issue #60, confirmed on RC-2). + * + * The YCBT ring logs every spot measurement into its own history — verified by reading five + * SpO₂ runs back out of the ring's memory at the exact times they were taken — and the vendor + * app's whole reaction to a `04 0e` success is to re-sync that history: the ring decides the + * number, the app re-reads it. So when a history sample of the same kind lands within + * [SPOT_MATCH_MS] of a reading we stored for our own settled value, it is that measurement + * seen from the ring's side, and keeping both is the doubled row the tester saw. The ring's + * row wins because it is the one that regenerates on every sync (and so the one a tombstone + * can hold down); ours is deleted outright. + * + * **Only rings that actually log spot readings take part.** A row is marked [SOURCE_SPOT] at + * write time only when the ring reported its own completion, so this can never fire on a CRP + * or Colmi ring — where the nearest history sample is an unrelated point on the five-minute + * all-day grid and would land inside [SPOT_MATCH_MS] of most measurements. + */ + private suspend fun adoptRingsCopy(kind: MeasurementKind, historyAt: Long) { + if (kind != MeasurementKind.HEART_RATE && kind != MeasurementKind.SPO2) return + val ours = spotReadingsOf(kind) + if (ours.isEmpty()) return + val matched = spotReadingsMatching(ours, historyAt) + if (matched.isEmpty()) return + db.measurementDao().deleteBySourceBetween(kind.name, SOURCE_SPOT, historyAt - SPOT_MATCH_MS, historyAt + SPOT_MATCH_MS) + ours.removeAll(matched) + } + private suspend fun persistUnsafe(event: PulseEvent) { when (event) { + // A start/end verdict for one spot measurement (issue #59) — a control signal for the + // coordinator's poll loop, carrying no value of its own. Nothing to store. + is PulseEvent.MeasurementComplete -> Unit is PulseEvent.DeviceStateChanged -> { // Never resurrect a forgotten ring: after Forget / Factory Reset clears the // device row, the ring's own teardown still emits a late DISCONNECTED — only @@ -160,30 +250,36 @@ class EventPersistenceSubscriber( )) recordBatterySample(event.percent, now) } + is PulseEvent.LiveSampleGate -> { + if (event.closed) closedGates.add(event.kind) else closedGates.remove(event.kind) + } is PulseEvent.HeartRateSample -> { - db.measurementDao().insert(MeasurementEntity( - kindRaw = MeasurementKind.HEART_RATE.name, - value = event.bpm.toDouble(), unit = "bpm", - timestamp = event.timestamp.toEpochMilli(), - sourceRaw = "live", - )) + if (!event.spot && MeasurementKind.HEART_RATE in closedGates) return + storeLiveReading( + MeasurementKind.HEART_RATE, event.bpm.toDouble(), "bpm", + event.timestamp.toEpochMilli(), awaitsRingsCopy(event.spot, event.ringWillLogIt), + ) } is PulseEvent.Spo2Result -> { - db.measurementDao().insert(MeasurementEntity( - kindRaw = MeasurementKind.SPO2.name, - value = event.value.toDouble(), unit = "%", - timestamp = event.timestamp.toEpochMilli(), - sourceRaw = "live", - )) + if (!event.spot && MeasurementKind.SPO2 in closedGates) return + storeLiveReading( + MeasurementKind.SPO2, event.value.toDouble(), "%", + event.timestamp.toEpochMilli(), awaitsRingsCopy(event.spot, event.ringWillLogIt), + ) } is PulseEvent.HistoryMeasurement -> { - db.measurementDao().upsert(MeasurementEntity( - id = historyMeasurementId(event.kind, event.timestamp.toEpochMilli()), + val at = event.timestamp.toEpochMilli() + val written = upsertUnlessDeleted(MeasurementEntity( + id = historyMeasurementId(event.kind, at), kindRaw = event.kind.name, value = event.value, unit = event.kind.unit, - timestamp = event.timestamp.toEpochMilli(), + timestamp = at, sourceRaw = "history", )) + // A sample the user deleted adopts nothing: it is re-sent on every sync, and + // letting it retire our spot rows within ±90 s would delete a retaken reading + // each time the ring re-supplied the one that was already deleted. + if (written) adoptRingsCopy(event.kind, at) } is PulseEvent.StressSample -> { val measurement = MeasurementEntity( @@ -197,7 +293,7 @@ class EventPersistenceSubscriber( timestamp = event.timestamp.toEpochMilli(), sourceRaw = "colmi", ) - if (event.isHistory) db.measurementDao().upsert(measurement) + if (event.isHistory) upsertUnlessDeleted(measurement) else db.measurementDao().insert(measurement) } is PulseEvent.HrvSample -> { @@ -233,8 +329,8 @@ class EventPersistenceSubscriber( sourceRaw = if (event.isHistory) "history" else "live", ) if (event.isHistory) { - db.measurementDao().upsert(systolic) - db.measurementDao().upsert(diastolic) + upsertUnlessDeleted(systolic) + upsertUnlessDeleted(diastolic) } else { db.measurementDao().insert(systolic) db.measurementDao().insert(diastolic) @@ -262,7 +358,7 @@ class EventPersistenceSubscriber( timestamp = event.timestamp.toEpochMilli(), sourceRaw = "live", ) - if (event.isHistory) db.measurementDao().upsert(measurement) + if (event.isHistory) upsertUnlessDeleted(measurement) else db.measurementDao().insert(measurement) } is PulseEvent.ActivityUpdate -> { @@ -454,14 +550,16 @@ class EventPersistenceSubscriber( if (existing.isEmpty()) emptyList() else db.sleepStageBlockDao().forSessions(existing.map { it.id }) - // YCBT complete records are authoritative for their interval, including shortened - // revisions. Packet-based families replace only the packet interval. In both cases the - // unaffected blocks remain available for SleepSegmentation to preserve separate naps. + // YCBT complete records are authoritative for the ring session they describe, including + // shortened revisions. Packet-based families replace only the packet interval. In both + // cases the unaffected blocks remain available for SleepSegmentation to preserve + // separate naps — and, for a complete record, the *other* ring sessions of the same night + // (issue #63): see [completeSessionSurvivors] for why "the session it describes" is a + // contiguous run of blocks and not every block of every row the packet touches. val replacements = buildStageBlocks("", ts, stages) val dayBlocks = replaceOverlappingSleepBlocks( existing = if (completeSession) { - val replacedSessionIds = overlapping.mapTo(mutableSetOf()) { it.id } - existingBlocks.filterNot { it.sessionId in replacedSessionIds } + completeSessionSurvivors(existingBlocks, ts, packetEnd) } else { existingBlocks }, @@ -542,7 +640,7 @@ class EventPersistenceSubscriber( val now = System.currentTimeMillis() for ((seg, row) in matched) { val id = row?.id ?: "sleep-$dayStart-${seg.start}" - val totalMin = ((seg.end - seg.start) / 60_000L).toInt().coerceAtLeast(0) + val totalMin = asleepMinutes(seg.blocks) val deepMin = seg.blocks .filter { it.stageRaw == SleepStage.DEEP.name } .sumOf { it.durationMinutes } @@ -651,11 +749,44 @@ class EventPersistenceSubscriber( } } - private companion object { - const val MAX_SLEEP_TIMELINE_MINUTES = 24 * 60 + companion object { + /** `sourceRaw` of a spot measurement's settled reading **on a ring that will also log it + * itself** (issue #60) — i.e. a row still awaiting reconciliation with the ring's copy. + * Reads alongside `"live"` everywhere a source is filtered; nothing treats the two apart + * except [adoptRingsCopy]. */ + const val SOURCE_SPOT = "spot" + /** How far a ring-history sample may sit from one of our spot readings and still be the + * ring's copy of it. The measurement itself runs 35–63 s and the ring stamps its log to + * the minute, so 90 s covers a stamp at either end of the run without reaching the ring's + * own all-day samples five minutes apart. */ + const val SPOT_MATCH_MS = 90_000L + /** How far back [spotReadings] is primed from the table on first use. */ + const val SPOT_LOOKBACK_MS = 7L * 24 * 60 * 60_000 + private const val MAX_SLEEP_TIMELINE_MINUTES = 24 * 60 } } +/** + * Is this reading one the ring will hand back from its own history, so that the two copies have to + * be reconciled later ([EventPersistenceSubscriber.adoptRingsCopy])? Both halves are required: + * a streamed sample is not a spot reading, and a spot reading from a ring that keeps no log of it + * has no second copy coming. Getting the second half wrong is not cosmetic — on a CRP or Colmi + * ring the nearest history sample is an unrelated point on the five-minute all-day grid, so the + * reconciliation would delete readings the user deliberately took (issue #60). + */ +internal fun awaitsRingsCopy(spot: Boolean, ringWillLogIt: Boolean): Boolean = spot && ringWillLogIt + +/** + * Which of our spot readings ([ours], epoch millis) a ring-history sample stamped [historyAt] is + * the ring's own copy of — the pure half of [EventPersistenceSubscriber.adoptRingsCopy] + * (issue #60): within [window] either side, and nothing else. + */ +internal fun spotReadingsMatching( + ours: List, + historyAt: Long, + window: Long = EventPersistenceSubscriber.SPOT_MATCH_MS, +): List = ours.filter { kotlin.math.abs(it - historyAt) <= window } + internal fun historyMeasurementId(kind: MeasurementKind, timestamp: Long): String = "history:${kind.key}:$timestamp" @@ -725,6 +856,52 @@ internal fun shouldReplaceCompleteSleep( incomingMinutes: Int, ): Boolean = existingStart == incomingStart || incomingMinutes > existingMinutes +/** + * The stored stage blocks a *complete* ring session leaves standing (issue #63). + * + * A YCBT ring closes a sleep session when it sees the wearer get up and opens a new one when they + * settle again, so one night can be two `af fa` records three minutes apart. SleepSegmentation + * (60-minute gap) rightly merges those into one stored row. The old rule then treated a complete + * record as authoritative for **every block of every row it overlapped** — so on the next sync + * pass, when the second record arrived on top of the merged row, it wiped the first record's + * three hours along with its own stale copy, and the night read 4 h 06 instead of 6 h 08. The + * vendor app keeps every record as its own session and never lets one displace another. + * + * What a complete record *is* authoritative for is the ring session it describes, which in the + * stored blocks is the contiguous run the packet's interval sits in: blocks that overlap the + * interval, and any block that abuts that run end-to-start without a gap, in either direction. + * That still lets a shortened revision retire its own stale head or tail (those abut the new + * interval), while a neighbouring session on the far side of even a one-minute gap is untouched. + * Blocks overlapping the interval itself are dropped here as well; the caller's interval replace + * would only trim them, and a complete record has no use for what it trimmed off. + */ +internal fun completeSessionSurvivors( + existing: List, + replacementStart: Long, + replacementEnd: Long, +): List { + fun end(block: SleepStageBlockEntity) = block.startAt + block.durationMinutes * 60_000L + var runStart = replacementStart + var runEnd = replacementEnd + val retired = mutableSetOf() + var grew = true + while (grew) { + grew = false + for (block in existing) { + if (block.id in retired) continue + val overlaps = block.startAt < runEnd && end(block) > runStart + val abuts = block.startAt == runEnd || end(block) == runStart + if (overlaps || abuts) { + retired.add(block.id) + runStart = minOf(runStart, block.startAt) + runEnd = maxOf(runEnd, end(block)) + grew = true + } + } + } + return existing.filterNot { it.id in retired } +} + internal fun replaceOverlappingSleepBlocks( existing: List, replacements: List, diff --git a/app/src/main/java/com/pulseloop/service/HRSampleWindow.kt b/app/src/main/java/com/pulseloop/service/HRSampleWindow.kt index 3225ad6..957b14d 100644 --- a/app/src/main/java/com/pulseloop/service/HRSampleWindow.kt +++ b/app/src/main/java/com/pulseloop/service/HRSampleWindow.kt @@ -12,40 +12,101 @@ import kotlin.math.abs * is sent — before the sensor has read anything. Everything inside [warmupMs] is therefore dropped; * without that, a measurement "succeeds" in two seconds on a number from hours ago. * * **Scatter.** Finger motion and poor contact make the PPG estimate jump around instead of holding - * within a few beats. A majority of the window must agree ([band], [majority]) or we report nothing: - * a heart rate the user has no reason to doubt, but shouldn't trust, is worse than an honest retry. + * within a few beats. A majority of the considered samples must agree ([band], [majority]) or we + * report nothing: a heart rate the user has no reason to doubt, but shouldn't trust, is worse than + * an honest retry. + * + * ## Two settle rules, and which ring gets which (issue #59) + * + * **A ring that says when it has finished decides the reading itself.** On the YCBT family the + * ring ends the measurement with `04 0e`, and the vendor app's reaction to that is `syncData()` — + * it re-reads the value out of the ring's history rather than computing one. Its measure screen + * (`HeartRateMeasureActivity.onEvent`, `com.yucheng.smarthealthpro`) never settles either: it + * overwrites the displayed bpm with every realtime frame, dropping only values outside + * `HEART_RATE_VISIBLE_MIN..MAX` (40..220). So what the user is shown, and what the ring logs, is + * the **last plausible sample of the run**. + * + * The reporter on #59 established that directly rather than by inference: three spot measurements + * captured with no stop command, each read back out of the ring's own memory before any app + * touched it, and the stored value equalled the last streamed sample three times out of three + * (65, 58, 72). It is a discriminating test on this ring, unlike SpO2 where tail and last coincide + * — the rate is still climbing when the ring stops, so every tail-weighted rule lands *below* the + * ring's answer, by as much as 18 bpm on those runs. Disagreeing with the ring is not a better + * number, it is a second number: the ring's copy arrives on the next sync and ours yields to it + * (issue #60), so a settle that disagrees only shows the user one value and then stores another. + * + * Note what this rule does *not* claim. The ring stops while the value is still rising, so its + * stored sample is the honest answer to "which sample did the firmware choose" and not to "has + * this converged". The second question is the firmware's to answer, and inventing a better number + * app-side would be worse than reporting the ring's. + * + * **A ring that never says it is done gets the tail rule instead**, because nothing else can end + * its window: the leg simply runs out, and "whichever sample happened to arrive as the timer + * expired" is a coincidence rather than a choice. There, [stableValue] still judges the tail of + * the window with a consistency gate. Judging the *whole* window is what issue #59 opened on: that + * ring's PPG spends its first ~26 s on a flat pre-converged plateau — 47 47 47, then 46 46 46, + * against a real rate of 81 — which is both the majority of the window and the most self-consistent + * thing in it, so a whole-window median returned it and the user was shown a confident number that + * was never their heart rate. The tail costs nothing on a ring that streams a steady rate + * throughout: its tail agrees with its head. + * + * Widening the last-sample rule to every family would repeat the mistake rc5 had to correct for + * the ring-copy rule — evidence gathered on one ring, generalised to rings it was never taken from. + * + * Samples are appended by the Main collector and judged from whichever thread runs the measuring + * coroutine (the coach's tools poll from IO), so every member that touches [samples] is + * synchronised. */ class HRSampleWindow(private val clock: () -> Long = System::currentTimeMillis) { /** Discard window for the cached echo described above. */ private val warmupMs = 5_000L - /** A gap this long between collected samples means we've stopped getting real data (ring slipped). */ - private val contactGapMs = 3_000L + /** + * A gap this long between collected samples means we've stopped getting real data (ring slipped). + * + * Sized for the burstiest cadence we've measured, not the average one: the #59 ring emits + * samples in bursts of three about a second apart and then goes quiet for **4–6 s** before the + * next burst. At the old 3 s this fired mid-measurement on a ring that was working perfectly, + * aborting the leg before its sensor had even converged — which is most of why that ring could + * never produce a reading. Raising it costs only how quickly a genuinely slipped ring is + * noticed, and the measurement window still bounds that. + */ + private val contactGapMs = 8_000L + /** How far back from the newest sample the settle looks. See the class note. */ + private val settleTailMs = 12_000L private val minSamples = 6 private val band = 8 // bpm neighbourhood around the median - private val majority = 0.6 // this much of the window must sit inside that band + private val majority = 0.6 // this much of the considered samples must sit inside that band + + private data class Sample(val bpm: Int, val at: Long) private var startedAt: Long? = null - private val samples = mutableListOf() - private var lastSampleAt: Long? = null + private val samples = mutableListOf() /** * True once a *real* (post-warm-up) reading has landed — which is what distinguishes a fresh * measurement from the stale live value still on screen from the last one. */ - val receivedReading: Boolean get() = samples.isNotEmpty() + val receivedReading: Boolean get() = synchronized(samples) { samples.isNotEmpty() } fun begin(now: Long = clock()) { - startedAt = now - samples.clear() - lastSampleAt = null + synchronized(samples) { + startedAt = now + samples.clear() + } } - /** Collect a sample, unless it's still inside the warm-up echo. */ - fun collect(bpm: Int, now: Long = clock()) { - val started = startedAt ?: return - if (now - started < warmupMs) return - samples.add(bpm) - lastSampleAt = now + /** + * Collect a sample. Returns false — and keeps nothing — when the sample is still inside the + * warm-up echo, or when no measurement is running. Callers use that answer to keep the echo + * out of the live value on screen as well as out of the settle. + */ + fun collect(bpm: Int, now: Long = clock()): Boolean { + synchronized(samples) { + val started = startedAt ?: return false + if (now - started < warmupMs) return false + samples.add(Sample(bpm, now)) + return true + } } /** @@ -53,21 +114,59 @@ class HRSampleWindow(private val clock: () -> Long = System::currentTimeMillis) * since nothing has been collected yet. */ fun contactLost(now: Long = clock()): Boolean { - val last = lastSampleAt ?: return false - return now - last > contactGapMs + val last = synchronized(samples) { samples.lastOrNull() } ?: return false + return now - last.at > contactGapMs } /** - * The settled reading: the median of the samples that agree with each other — or null if they - * never did. + * The reading this measurement settled on. [ringChoosesLastSample] is + * `RingSyncEngine.signalsMeasurementCompletion` — a ring that ends its own measurement is one + * whose vendor app reads the value back out of history rather than deciding it. See the class + * note for why those are different questions. + */ + fun settled(ringChoosesLastSample: Boolean): Int? = + if (ringChoosesLastSample) lastPlausible else stableValue + + /** + * The last sample inside the vendor's visible band — what its measure screen leaves on the + * display, and what the ring logs for itself. Null when the run produced no plausible sample + * at all, which is a failed measurement rather than a reading of zero. + * + * The band is the only filter, deliberately: it keeps a trailing dropout frame from becoming + * the reading without second-guessing a ring that is reporting a real, if unconverged, rate. + */ + val lastPlausible: Int? + get() = synchronized(samples) { samples.lastOrNull { it.bpm in PLAUSIBLE }?.bpm } + + /** + * The settled reading for a ring with no completion signal: the median of the tail samples + * that agree with each other — or null if they never did. */ val stableValue: Int? get() { - if (samples.size < minSamples) return null - val sorted = samples.sorted() + val considered = synchronized(samples) { + if (samples.size < minSamples) return null + tail() + } + val sorted = considered.sorted() val median = sorted[sorted.size / 2] val cluster = sorted.filter { abs(it - median) <= band } // stays sorted - if (cluster.size < samples.size * majority) return null + if (cluster.size < considered.size * majority) return null return cluster[cluster.size / 2] } + + /** The samples the settle judges: the last [settleTailMs] of them, floored at [minSamples]. + * Callers hold the [samples] lock. */ + private fun tail(): List { + val newest = samples.last().at + val byTime = samples.count { newest - it.at <= settleTailMs } + val take = maxOf(byTime, minSamples).coerceAtMost(samples.size) + return samples.takeLast(take).map { it.bpm } + } + + companion object { + /** The vendor's `TransUtils.HEART_RATE_VISIBLE_MIN..MAX` — the band its measure screen + * applies to every realtime frame before displaying it. */ + val PLAUSIBLE: IntRange = 40..220 + } } diff --git a/app/src/main/java/com/pulseloop/service/LiveWorkoutManager.kt b/app/src/main/java/com/pulseloop/service/LiveWorkoutManager.kt index 3ecf80b..9b5cdb7 100644 --- a/app/src/main/java/com/pulseloop/service/LiveWorkoutManager.kt +++ b/app/src/main/java/com/pulseloop/service/LiveWorkoutManager.kt @@ -77,7 +77,7 @@ class LiveWorkoutManager( ) db.activitySessionDao().upsert(session) - coordinator.startWorkoutHeartRate() + coordinator.startWorkoutHeartRate(type) if (useGps) gps.start(session.id, type) polling.start(session.id) startForegroundService(session.type) diff --git a/app/src/main/java/com/pulseloop/service/RingSyncCoordinator.kt b/app/src/main/java/com/pulseloop/service/RingSyncCoordinator.kt index 3156fb7..5bf0048 100644 --- a/app/src/main/java/com/pulseloop/service/RingSyncCoordinator.kt +++ b/app/src/main/java/com/pulseloop/service/RingSyncCoordinator.kt @@ -81,6 +81,9 @@ class RingSyncCoordinator( /** The samples of the HR measurement in flight, and the rule for whether they settled — see * [HRSampleWindow], which owns the warm-up echo and the consistency gate (iOS #66). */ private val hrWindow = HRSampleWindow() + /** The SpO2 samples of the measurement in flight, and the rule for settling them — see + * [Spo2SampleWindow]. Only consulted for a family that says when it has finished. */ + private val spo2Window = Spo2SampleWindow() /** The refusal fast-fail gate for spot measurements (iOS `c8969a4`) — the ring's `03 2f` * verdict can only ever abort the measurement it names, while it is actually running. */ private val spot = SpotMeasurementGate() @@ -88,31 +91,78 @@ class RingSyncCoordinator( * [latestHRValue] from passing for a fresh reading. */ val measurementReceivedReading: Boolean get() = hrWindow.receivedReading + /** + * While a spot measurement is settling, the live sample stream is working, not reporting, and + * must not be written to history (issue #60). + * + * A spot measurement's output is **one** reading — the settled value the leg returns. The + * samples it settles *from* are a sensor converging: on the #59 ring the PPG spends its first + * ~26 s on a plateau tens of bpm below the real rate, and every one of those estimates used to + * be stored as its own heart-rate row stamped with the moment it arrived. One failed + * measurement therefore left a whole train of readings that were never the user's heart rate, + * with no way to remove them, and they drag every average built over that window. SpO₂ is the + * same story since its leg started collecting the whole run (RC-2): twelve values over ~50 s. + * + * So the leg closes the gate on that kind's live samples for its duration, reopens it, and + * then publishes the settled value once. The gate is a **bus event**, not a flag: the + * persistence collector runs behind the ring's stream on its own dispatcher, so a flag read at + * write time still let every sample already queued in the bus through the moment it flipped — + * the exact rows this rule exists to stop. An event travels in order with the samples it + * governs. A live *workout* is the opposite case for heart rate — there the stream is the + * data — so a measurement that runs during one neither closes the gate nor publishes a second + * row for the reading the stream already stored. + */ + private fun gateLiveSamples(kind: MeasurementKind, closed: Boolean) { + PulseEventBus.publishBlocking(PulseEvent.LiveSampleGate(kind, closed)) + } + val connectionState: RingConnectionState get() = client.state.value.connectionState val isConnected: Boolean get() = connectionState == RingConnectionState.CONNECTED /** Selects the single-packet Jring measurement flow. YCBT advertises manual BP/glucose * capabilities but measures each vital with separate AppStartMeasurement modes. */ val supportsCombinedMeasurement: Boolean get() = engine?.supportsCombinedMeasurement == true - private val hrMeasureSeconds = HR_MEASURE_SECONDS.toLong() - private val spo2MeasureSeconds = SPO2_MEASURE_SECONDS.toLong() + /** The HR leg's ceiling for the ring that is actually connected (issue #59). */ + private val hrMeasureSeconds: Long get() = (engine?.spotHeartRateSeconds ?: HR_MEASURE_SECONDS).toLong() + /** + * Upper bound on the whole sequential sweep for the connected ring — what the Vitals countdown + * runs against, so it can't finish while a leg is still measuring. + * + * Summed over the legs [measureSpot] will *actually* run, gated on the same capabilities, and + * using this ring's own HR ceiling (issue #59). The flat sum of all four legs told an RC-1 + * tester his measurement would take 188 s when his ring runs two of them; a countdown that + * overstates by 80 s is worse than no countdown, because the user reads it as a promise. + */ + val spotMeasureSeconds: Int + get() { + val caps = client.state.value.activeCapabilities + var total = 3 + if (caps.contains(WearableCapability.MANUAL_HEART_RATE)) total += hrMeasureSeconds.toInt() + if (caps.contains(WearableCapability.MANUAL_SPO2)) total += spo2MeasureSeconds.toInt() + if (caps.contains(WearableCapability.MANUAL_BLOOD_PRESSURE)) total += BP_MEASURE_SECONDS + if (caps.contains(WearableCapability.MANUAL_HRV)) total += HRV_MEASURE_SECONDS + return total + } + /** The SpO₂ leg's ceiling for the ring that is actually connected (issue #59 RC-2). */ + private val spo2MeasureSeconds: Long get() = (engine?.spotSpo2Seconds ?: SPO2_MEASURE_SECONDS).toLong() private val combinedMeasureSeconds = COMBINED_MEASURE_SECONDS.toLong() companion object { /** Duration of a combined spot measurement (0x23→0x24); also drives the UI countdown. */ const val COMBINED_MEASURE_SECONDS = 45 - /** Window for the live-HR leg of a spot measurement. */ - const val HR_MEASURE_SECONDS = 30 - /** Window for the live-SpO₂ leg of a spot measurement. iOS raised this 40 → 60 - * (`c8969a4`): the R99's successful sweep took 38s while another attempt ran past 41s - * with no result — at 40s the outcome is a coin toss where the user watches the ring's - * red LED work and gets an error anyway. */ - const val SPO2_MEASURE_SECONDS = 60 - /** Intentional UX upper bound for sequential HR + SpO₂ + BP + HRV; drives the countdown. - * Derived from the legs so the countdown can't desync when one is tuned. Post-#66 the - * HR leg samples its full window by design, so this is a real bound, not slack. */ + /** Default window for the live-HR leg of a spot measurement. A family that ends its own + * measurement may raise its own ceiling — see [RingSyncEngine.spotHeartRateSeconds]. */ + const val HR_MEASURE_SECONDS = RingSyncEngine.DEFAULT_SPOT_HEART_RATE_SECONDS + /** Default window for the live-SpO₂ leg of a spot measurement. A family that ends its + * own measurement may raise its own ceiling — see [RingSyncEngine.spotSpo2Seconds]. */ + const val SPO2_MEASURE_SECONDS = RingSyncEngine.DEFAULT_SPOT_SPO2_SECONDS const val BP_MEASURE_SECONDS = 40 const val HRV_MEASURE_SECONDS = 40 + /** Intentional UX upper bound for sequential HR + SpO₂ + BP + HRV; drives the countdown. + * Derived from the legs so the countdown can't desync when one is tuned. Post-#66 the + * HR leg samples its full window by design, so this is a real bound, not slack. This is + * the bound for a ring with the default HR window; with one connected, prefer the + * instance's [spotMeasureSeconds], which uses that ring's own HR ceiling (issue #59). */ const val SPOT_MEASURE_SECONDS = HR_MEASURE_SECONDS + SPO2_MEASURE_SECONDS + BP_MEASURE_SECONDS + HRV_MEASURE_SECONDS + 3 /** Max time to wait for the pre-factory-reset history sync before resetting anyway. */ @@ -290,15 +340,19 @@ class RingSyncCoordinator( // MARK: - Workout HR streaming - fun startWorkoutHeartRate() { + /** The activity type of the workout whose stream is running — what a restart re-sends. */ + private var workoutActivityType: String = "other" + + fun startWorkoutHeartRate(activityType: String = "other") { if (!isConnected) return - engine?.startHeartRate() + workoutActivityType = activityType + engine?.startWorkoutHeartRate(activityType) workoutHRActive = true } fun stopWorkoutHeartRate() { if (!workoutHRActive) return - engine?.stopHeartRate() + engine?.stopWorkoutHeartRate() workoutHRActive = false engine?.syncVitalsHistory() } @@ -315,7 +369,7 @@ class RingSyncCoordinator( */ fun restartWorkoutHeartRateIfActive() { if (!workoutHRActive || !isConnected) return - engine?.startHeartRate() + engine?.startWorkoutHeartRate(workoutActivityType) } fun querySleep() { @@ -430,10 +484,19 @@ class RingSyncCoordinator( hrNoReadingReported = false measureNotWorn = false hrWindow.begin() + // A streaming workout owns the bpm stream; otherwise this leg does (see gateLiveSamples). + val ownsStream = !workoutHRActive + if (ownsStream) gateLiveSamples(MeasurementKind.HEART_RATE, closed = true) val spotToken = spot.begin(YCBTMeasurementMode.HEART_RATE) engine?.measureHeartRateSpot() var result: Int? = null + // True only when the ring itself called this run a success. That — not the family's + // ability to do so — is what makes its last sample the reading and its history the owner + // of the row: a run that hits our ceiling without a `04 0e` is one the ring never finished + // and never logged, so storing it as "spot" would let the next sync delete it in favour of + // an unrelated all-day grid sample. + var completedByRing = false try { // Sample the full window in 0.5s steps: handle() drops everything inside the 5s warm-up // (the ring's cached-echo bpm) and collects the rest. We break out early only where @@ -446,11 +509,20 @@ class RingSyncCoordinator( if (hrNoReadingReported || spot.isRejected(spotToken)) { aborted = true; break } // Ring removed / BLE dropped mid-measure → fail rather than settle a truncated window. if (!isConnected) { aborted = true; break } + // The ring ended the measurement itself (YCBT `04 0e`, issue #59). Its own verdict + // beats our window: on success settle what we have instead of idling out the rest + // of a window the ring has already stopped streaming into; on failure, abort. + val completed = spot.completedSuccessfully(spotToken) + if (completed != null) { aborted = !completed; completedByRing = completed; break } // Contact lost after readings began (ring slipped / hand moved). if (hrWindow.contactLost()) { aborted = true; break } delay(500) } - result = if (aborted) null else hrWindow.stableValue + // Which sample is the reading depends on whether the ring chose one: a ring that + // ended this run logs the value it displayed last, and ours has to be that same value + // or the ring's copy will simply replace it on the next sync (issue #59). A run the + // ring did not end falls back to the consistency gate, whatever the family. + result = if (aborted) null else hrWindow.settled(ringChoosesLastSample = completedByRing) } finally { spot.end(spotToken) // Always switch the optical sensor off — even if the caller's coroutine is @@ -459,6 +531,23 @@ class RingSyncCoordinator( // The stop also tears down the workout's realtime stream; bring it straight back. restartWorkoutHeartRateIfActive() hrState = if (result != null) MeasureState.DONE else MeasureState.FAILED + if (ownsStream) { + // Reopen the gate BEFORE publishing, or the one reading worth keeping is the one + // reading dropped; both travel the bus in this order. + gateLiveSamples(MeasurementKind.HEART_RATE, closed = false) + // The measurement's actual output, stored once. A failed measurement stores + // nothing — "we couldn't read it" is not a heart rate. During a streaming workout + // the stream already stored every sample, so publishing the settled value there + // would only add a duplicate row stamped with a made-up time. + result?.let { settled -> + PulseEventBus.publishBlocking( + PulseEvent.HeartRateSample( + bpm = settled, timestamp = java.time.Instant.now(), + spot = true, ringWillLogIt = completedByRing, + ) + ) + } + } } return result } @@ -470,18 +559,41 @@ class RingSyncCoordinator( latestSpO2Value = null spo2NoReadingReported = false measureNotWorn = false + spo2Window.begin() + gateLiveSamples(MeasurementKind.SPO2, closed = true) val spotToken = spot.begin(YCBTMeasurementMode.SPO2) engine?.startSpO2() var result: Int? = null + var completedByRing = false // see measureHR — the ring's verdict on THIS run owns the row try { - // Abort early when the ring reports the run ended with an error (finger off, - // ring not worn) or refused the start, instead of idling out the full window. - result = pollForValue(spo2MeasureSeconds, { latestSpO2Value }, { spo2NoReadingReported || spot.isRejected(spotToken) }) + result = if (engine?.signalsMeasurementCompletion == true) { + // The ring will say when it is done, so collect the whole run and settle it + // (issue #59 RC-1). Returning the first plausible sample handed back a reading + // taken 37 s before the ring finished, with nine better ones still to come. + settleSpO2(spotToken).also { completedByRing = it.completedByRing }.value + } else { + // No completion signal: the first plausible value is all we will ever be sure of, + // and waiting out the window past it buys nothing. Abort early when the ring + // reports the run ended with an error (finger off, ring not worn) or refused it. + pollForValue(spo2MeasureSeconds, { latestSpO2Value }, { spo2NoReadingReported || spot.isRejected(spotToken) }) + } } finally { spot.end(spotToken) engine?.stopSpO2() // stop the sensor even on cancellation (see measureHR) restartWorkoutHeartRateIfActive() // the stop preempts the workout's HR stream spo2State = if (result != null) MeasureState.DONE else MeasureState.FAILED + // Reopen the gate before publishing, or the one reading worth keeping is dropped. + gateLiveSamples(MeasurementKind.SPO2, closed = false) + // The measurement's actual output, stored once — and what the card then shows, so the + // settled value is on screen rather than whichever sample happened to arrive last. + result?.let { settled -> + PulseEventBus.publishBlocking( + PulseEvent.Spo2Result( + value = settled, timestamp = java.time.Instant.now(), + spot = true, ringWillLogIt = completedByRing, + ) + ) + } } return result } @@ -499,7 +611,7 @@ class RingSyncCoordinator( result = pollForValue( BP_MEASURE_SECONDS.toLong(), { latestBloodPressure }, - { bloodPressureNoReadingReported || spot.isRejected(spotToken) }, + { bloodPressureNoReadingReported || spot.isRejected(spotToken) || spot.completedSuccessfully(spotToken) == false }, ) } finally { spot.end(spotToken) @@ -523,7 +635,7 @@ class RingSyncCoordinator( result = pollForValue( HRV_MEASURE_SECONDS.toLong(), { latestHrvValue }, - { hrvNoReadingReported || spot.isRejected(spotToken) }, + { hrvNoReadingReported || spot.isRejected(spotToken) || spot.completedSuccessfully(spotToken) == false }, ) } finally { spot.end(spotToken) @@ -554,6 +666,40 @@ class RingSyncCoordinator( } } + /** + * Run the SpO2 leg to its natural end and settle what it collected — the path for a family + * whose ring reports completion ([RingSyncEngine.signalsMeasurementCompletion]). + * + * Mirrors the HR leg's structure: sample the window in 0.5 s steps, break out only where + * continuing is pointless, and report a value only when the leg was not aborted. + */ + private suspend fun settleSpO2(spotToken: SpotMeasurementGate.Token): SettledRun { + var aborted = false + var completedByRing = false + val steps = (spo2MeasureSeconds * 2).toInt() // 0.5s granularity + for (i in 0 until steps) { + if (spo2NoReadingReported || spot.isRejected(spotToken)) { aborted = true; break } + if (!isConnected) { aborted = true; break } + val completed = spot.completedSuccessfully(spotToken) + if (completed != null) { aborted = !completed; completedByRing = completed; break } + delay(500) + } + return SettledRun(if (aborted) null else spo2Window.settled, completedByRing) + } + + /** A leg's outcome: the reading, and whether the ring itself ended the run successfully — + * which is what decides whether the ring's history will carry its own copy of it. */ + private data class SettledRun(val value: Int?, val completedByRing: Boolean) + + /** + * Poll for the first value, giving up early when [abort] says continuing is pointless. + * + * For the BP and HRV legs a ring's `04 0e` **success** is deliberately not an abort: the leg + * reads no value out of that push (the vendor re-syncs history instead), and the HRV leg has + * no live-value frame at all, so treating success as "stop" turned a measurement the ring + * called successful into a failure. Only the ring's failure verdict ends those legs early; + * on success they keep their window, as they did before #59. + */ private suspend fun pollForValue( windowSec: Long, value: () -> T?, @@ -572,9 +718,19 @@ class RingSyncCoordinator( private fun handle(event: PulseEvent) { when (event) { + is PulseEvent.LiveSampleGate -> Unit // our own; consumed by EventPersistenceSubscriber is PulseEvent.HeartRateSample -> { - latestHRValue = event.bpm - if (hrState == MeasureState.MEASURING) hrWindow.collect(event.bpm) + if (event.spot) return // our own settled reading, not a ring sample + // During a spot measure the window decides what counts: a sample it rejects is the + // ring's cached echo (a bpm from hours ago, stamped now), so it must not become the + // live value either — that echo is exactly the number issue #59 saw on the card + // next to a "couldn't get a steady reading" error. Outside a measurement there is + // no window to consult and the workout stream's value passes straight through. + if (hrState == MeasureState.MEASURING) { + if (hrWindow.collect(event.bpm)) latestHRValue = event.bpm + } else { + latestHRValue = event.bpm + } } is PulseEvent.HeartRateComplete -> { if (hrState == MeasureState.MEASURING && !measurementReceivedReading) { @@ -582,7 +738,10 @@ class RingSyncCoordinator( } } is PulseEvent.Spo2Result -> { + // Our own settled reading is already on the card; the window only sees the ring. + if (event.spot) return latestSpO2Value = event.value + if (spo2State == MeasureState.MEASURING) spo2Window.collect(event.value) } is PulseEvent.HrvSample -> { if (hrvState == MeasureState.MEASURING) latestHrvValue = event.value @@ -597,6 +756,12 @@ class RingSyncCoordinator( spo2NoReadingReported = true } } + // The ring ended a spot measurement itself and said how it went (YCBT `04 0e`, + // issue #59). Ownership is by token inside the gate, so this can only ever end the + // measurement it names, and only while that measurement is actually running. + is PulseEvent.MeasurementComplete -> { + spot.noteCompleted(event.mode, event.success) + } is PulseEvent.MeasurementRejected -> { spot.noteRejected(event.mode) when (event.mode) { diff --git a/app/src/main/java/com/pulseloop/service/SleepInsights.kt b/app/src/main/java/com/pulseloop/service/SleepInsights.kt index caa956b..2a02349 100644 --- a/app/src/main/java/com/pulseloop/service/SleepInsights.kt +++ b/app/src/main/java/com/pulseloop/service/SleepInsights.kt @@ -68,7 +68,15 @@ object SleepScore { session: SleepSessionEntity, blocks: List, ): SleepScoreResult { + // Two denominators, because since issue #63 a session carries two numbers. `total` is + // time asleep: stage shares and the duration band are judged against it, as sleep-stage + // percentages conventionally are (of total sleep time, not time in bed) and as the headline + // the user sees. `span` is first block to last: the awake share and the "does this ring + // label awake at all" heuristic are judged against it, since both are about time in bed — + // and judged against asleep time the coverage test below would be true for every ring, + // handing a ring that never labels awake the full awake sub-score for nothing. val total = if (session.totalMinutes > 0) session.totalMinutes.toDouble() else 0.0 + val span = session.spanMinutes.toDouble().takeIf { it > 0 } ?: total // stageRaw is persisted as the SleepStage enum name (uppercase) — match it exactly. // Some rings report REM in big-data sleep; the score model has no REM band, so fold // REM into deep (both are restorative sleep the deep band rewards). @@ -79,12 +87,12 @@ object SleepScore { val awake = minutesFor(SleepStage.AWAKE) val coveredStageMin = blocks.sumOf { it.durationMinutes.toDouble() } val hasAwakeSignal = blocks.any { it.stageRaw == SleepStage.AWAKE.name } || - awake > 0 || (total > 0 && coveredStageMin >= total * 0.95) + awake > 0 || (span > 0 && coveredStageMin >= span * 0.95) val totalHours = total / 60 val deepPct = if (total > 0) (deep / total) * 100 else 0.0 val lightPct = if (total > 0) (light / total) * 100 else 0.0 - val awakePct: Double? = if (total > 0 && hasAwakeSignal) (awake / total) * 100 else null + val awakePct: Double? = if (span > 0 && hasAwakeSignal) (awake / span) * 100 else null val duration = bandScore(totalHours, 7.5, 8.5, 6.0, 9.5, 3.0, 12.0, 35.0) val deepScore = bandScore(deepPct, 13.0, 23.0, 5.0, 35.0, 0.0, 45.0, 30.0) @@ -104,6 +112,46 @@ object SleepScore { // ── Formatting ──────────────────────────────────────────────────────────── +/** + * How long the wearer was **asleep** across these stage blocks — every stage the ring recorded, + * less the ones it marked awake (issue #63). + * + * This is a session's `totalMinutes`, and it is deliberately not the span from its start to its + * end. A night the ring splits into two records is stored as one row covering both, so the span + * also counts the minutes between them: on the reporter's night the app said 8 h 10 (23:51 to + * 08:02) for a night whose two records declared 268 and 140 minutes, 6 h 48. Neither figure is + * wrong, but only one of them is a duration — the other is a range, and the sleep card already + * shows the range on its own line underneath. See [spanMinutes] for that one. + * + * The vendor app draws the same distinction, and the reporter found where (`SleepActivity:695`, + * `com.yucheng.smarthealthpro`): each history entry is built as + * `deepSleepTotal + lightSleepTotal + remTotal`, with `wakeDuration` carried separately and + * excluded, while `startTime` comes from the first record of the day and `endTime` from the last. + * It never conflates the two either. + * + * Implemented as "every stage except AWAKE" rather than "DEEP + LIGHT + REM" on purpose. The two + * are the same sum on any ring that labels its stages — YCBT's sleep tag 4 *is* AWAKE — but + * `SleepStage.UNKNOWN` is the `else` branch of every decoder in this app, an unrecognised stage + * byte *inside* a sleep record. Those minutes were slept; naming three stages explicitly would + * silently drop them, and on a ring whose stage codes we don't fully decode it could zero out a + * whole night. + */ +fun asleepMinutes(blocks: List): Int = + blocks.filter { it.stageRaw != SleepStage.AWAKE.name } + .sumOf { it.durationMinutes } + .coerceAtLeast(0) + +/** + * How long the session covers on the clock, first stage block to last — which since issue #63 is a + * different number from [SleepSessionEntity.totalMinutes], the time actually asleep. + * + * Use this wherever wall-clock position matters (a hypnogram's x axis, a timeline); use + * `totalMinutes` wherever a duration is being reported. The sleep card shows both, as the vendor + * app does: the duration as the headline, the span as the range underneath it. + */ +val SleepSessionEntity.spanMinutes: Int + get() = ((endAt - startAt) / 60_000L).toInt().coerceAtLeast(0) + object SleepFormat { fun duration(minutes: Int?): String { if (minutes == null || minutes < 0) return "—" diff --git a/app/src/main/java/com/pulseloop/service/Spo2SampleWindow.kt b/app/src/main/java/com/pulseloop/service/Spo2SampleWindow.kt new file mode 100644 index 0000000..2ca35c5 --- /dev/null +++ b/app/src/main/java/com/pulseloop/service/Spo2SampleWindow.kt @@ -0,0 +1,73 @@ +package com.pulseloop.service + +/** + * The SpO₂ samples of one spot measurement, and the rule for turning them into a reading + * (issue #59, RC-1 and RC-2 feedback). + * + * ## Why this exists at all + * + * The SpO₂ leg used to return the **first** plausible sample and stop. On the instrumented + * `Ale-Hop2211` capture that is 96 % at t+13 s — thirty-seven seconds before the ring finished, + * and before nine further samples arrived (99, 98, 98, 98, 96, 96, 95, 94, 94). Whatever the right + * answer is, "whichever sample happened to arrive first" is not it, and returning early also made + * the ring's own `04 0e` completion unreachable for this leg. + * + * Note what is *not* the problem here, because it differs from heart rate: there is no cached echo + * to discard. That ring sends nothing at all for the first 13 s, so a time-based warm-up window + * would drop nothing and inventing one would be guessing. + * + * ## Why the last plausible sample + * + * RC-2 shipped a median because one capture could not say whether the peak or the tail of a run + * was the honest number. Three independent sources then agreed on the answer (issue #59, RC-2 + * feedback): + * + * * **The ring's own log.** These rings write each spot reading into their history. Read back + * against the raw streams of five captures, the stored value equalled the **last** streamed + * sample five times out of five; the median matched four (it parted company on a run ending + * 97×3, 99, 99, 98×3, 99 — ring 99, median 98). + * * **A collapsing run is a bad measurement, not a rule failure.** The one run whose late burst + * fell from 98 to 86–87 is stored by the ring as 87. There was no eleven-point error for a + * settle rule to avoid; the ring itself calls that run 87. + * * **The vendor app does not settle at all.** `BloodOxygenMeasureActivity.onEvent` + * (`com.zhuoting.healthyucheng` 1.27.96) overwrites the on-screen value with every realtime + * frame, dropping only zero and anything outside `BLOOD_OXYGEN_VISIBLE_MIN..MAX` (70..100), and + * on `04 0e` success re-reads the ring's history rather than deciding a number itself. + * + * So the ring decides, and the app's job is to agree with it: the reading is the last sample the + * ring streamed, filtered by the vendor's plausibility band and nothing else. Anything cleverer + * disagrees with what the ring will log — which is exactly the doubled, slightly-different + * readings issue #60's tester saw. + */ +class Spo2SampleWindow { + private val samples = mutableListOf() + + /** True once any plausible reading has landed — distinguishes a fresh measurement from a stale + * value. */ + val receivedReading: Boolean get() = synchronized(samples) { samples.isNotEmpty() } + + fun begin() { + synchronized(samples) { samples.clear() } + } + + /** + * Collect a sample. Returns false — and keeps nothing — when it is outside the vendor's + * plausibility band, so a zero or a dropout can neither become the reading nor mark the + * measurement as having read something. + */ + fun collect(percent: Int): Boolean { + if (percent !in PLAUSIBLE) return false + synchronized(samples) { samples.add(percent) } + return true + } + + /** The settled reading: the last plausible sample the ring streamed, or null if there was none. */ + val settled: Int? + get() = synchronized(samples) { samples.lastOrNull() } + + companion object { + /** The vendor's `BLOOD_OXYGEN_VISIBLE_MIN..MAX`; also the band every other decoder in this + * app already applies to a live SpO₂ value. */ + val PLAUSIBLE: IntRange = 70..100 + } +} diff --git a/app/src/main/java/com/pulseloop/service/SpotMeasurementGate.kt b/app/src/main/java/com/pulseloop/service/SpotMeasurementGate.kt index 56edaba..b60c0df 100644 --- a/app/src/main/java/com/pulseloop/service/SpotMeasurementGate.kt +++ b/app/src/main/java/com/pulseloop/service/SpotMeasurementGate.kt @@ -4,53 +4,101 @@ import java.util.concurrent.atomic.AtomicInteger /** * Ported from SpotMeasurementGate in RingSyncCoordinator.swift (iOS `c8969a4`, riding the #82 - * sync): the fast-fail rule for a **refused** spot measurement. + * sync): the fast-fail rule for a **refused** spot measurement, extended for issue #59 to carry + * the ring's own end-of-measurement verdict as well. * * A YCBT ring answers `03 2f` with a verdict byte, and it refuses modes it has no sensor for * (the R99 refuses HRV `0x0a`). Without this gate the coordinator would poll a ring that already - * said no for the full measurement window before reporting a generic failure. + * said no for the full measurement window before reporting a generic failure. The same ring also + * *ends* a measurement itself with `04 0e` once its PPG has converged (issue #59), which is the + * other half of the same problem: without it the leg idles out its whole window after the ring + * has already gone quiet, then reports the reading was never steady. * * The danger in aborting on a device-pushed signal is aborting the *wrong* thing, so ownership - * is by token, not by mode: a refusal may only ever cancel the measurement it names, while that - * measurement is actually running. Tokens matter because spot measurements really do overlap — - * the workout poll service fires on its own timer while the user (or the coach's action tools) - * can start another reading, and nothing serializes those flows against each other. + * is by token, not by mode: a refusal or completion may only ever end the measurement it names, + * while that measurement is actually running. Tokens matter because spot measurements really do + * overlap — the workout poll service fires on its own timer while the user (or the coach's action + * tools) can start another reading, and nothing serializes those flows against each other. + * + * Those flows also run on different threads: the ring's verdicts land on the Main collector while + * a coach `trigger_measurement` polls from `Dispatchers.IO` under `runBlocking`. Every access to + * [inFlight] is therefore synchronised — a `04 0e` iterating the map while another leg's `end()` + * removes its token would otherwise throw `ConcurrentModificationException` on the Main collector, + * which has no handler and takes the process down with it. */ class SpotMeasurementGate { /** A handle to one in-flight spot measurement. Identity is [id], **not** the mode, so two * flows that somehow ran the same mode at once still could not end or abort each other. */ data class Token internal constructor(internal val id: Int, val mode: Int) - /** The measurements currently mid-poll, and whether the ring has refused each. */ - private val inFlight = LinkedHashMap() + /** What the ring has said about one in-flight measurement, if anything. */ + private enum class Outcome { RUNNING, REJECTED, SUCCEEDED, FAILED } + + /** The measurements currently mid-poll, and what the ring has said about each. */ + private val inFlight = LinkedHashMap() private val nextId = AtomicInteger(0) /** Arm the gate for one measurement and hand back its handle. */ fun begin(mode: Int): Token { val token = Token(nextId.getAndIncrement(), mode) - inFlight[token] = false + synchronized(inFlight) { inFlight[token] = Outcome.RUNNING } return token } /** Disarm [token] — and only [token]. Called on every exit path (success, timeout, * rejection); the measurement that finishes first must not disarm one still running. */ fun end(token: Token) { - inFlight.remove(token) + synchronized(inFlight) { inFlight.remove(token) } } /** Has the ring refused **this** measurement? What each poll loop's abort check asks, so a * refusal can only ever end the measurement it actually named. */ - fun isRejected(token: Token): Boolean = inFlight[token] ?: false + fun isRejected(token: Token): Boolean = synchronized(inFlight) { inFlight[token] == Outcome.REJECTED } + + /** + * Has the ring *ended* **this** measurement, and did it call it a success? `null` while the + * ring is still measuring — which is the normal answer for every family that never sends a + * completion, so a poll loop that consults this keeps its window as the fallback bound. + * + * A rejection is deliberately not reported here: refusal is a start-time verdict with its own + * abort path ([isRejected]), and folding the two together would let a refusal look like a + * finished measurement whose samples are worth settling. + */ + fun completedSuccessfully(token: Token): Boolean? = synchronized(inFlight) { + when (inFlight[token]) { + Outcome.SUCCEEDED -> true + Outcome.FAILED -> false + else -> null + } + } /** The ring refused [mode]. Honoured only by the in-flight measurement(s) actually running * it — a late reply for a mode nothing is polling is ignored. */ fun noteRejected(mode: Int) { - for (token in inFlight.keys) { - if (token.mode == mode) inFlight[token] = true + synchronized(inFlight) { + for (token in inFlight.keys) { + if (token.mode == mode) inFlight[token] = Outcome.REJECTED + } + } + } + + /** + * The ring ended [mode] itself and reported [success]. Same ownership rule as [noteRejected]: + * only the measurement(s) actually running that mode see it, and a refusal already recorded + * for this token wins — a ring that refuses a start and then pushes a stray completion must + * not turn its own refusal into a settled reading. + */ + fun noteCompleted(mode: Int, success: Boolean) { + synchronized(inFlight) { + for (token in inFlight.keys) { + if (token.mode == mode && inFlight[token] == Outcome.RUNNING) { + inFlight[token] = if (success) Outcome.SUCCEEDED else Outcome.FAILED + } + } } } /** The modes currently mid-poll. Read by tests; the coordinator drives everything through * tokens. */ - val modesInFlight: Set get() = inFlight.keys.map { it.mode }.toSet() + val modesInFlight: Set get() = synchronized(inFlight) { inFlight.keys.map { it.mode }.toSet() } } diff --git a/app/src/main/java/com/pulseloop/ui/PulseLoopApp.kt b/app/src/main/java/com/pulseloop/ui/PulseLoopApp.kt index 7b4cb38..224731a 100644 --- a/app/src/main/java/com/pulseloop/ui/PulseLoopApp.kt +++ b/app/src/main/java/com/pulseloop/ui/PulseLoopApp.kt @@ -70,9 +70,12 @@ fun PulseLoopApp() { val persistence = remember { // Every persisted ring-sync batch republishes the widget snapshot (debounced 2 s), // mirroring the iOS PulseDataChange → WidgetSnapshotPublisher pipeline. - EventPersistenceSubscriber(context, db) { - com.pulseloop.widgets.WidgetSnapshotPublisher.publishDebounced(context) - } + EventPersistenceSubscriber( + context, db, + onDataPersisted = { + com.pulseloop.widgets.WidgetSnapshotPublisher.publishDebounced(context) + }, + ) } val batteryAlerts = remember { com.pulseloop.service.BatteryAlertMonitor(context) } val providerStore = remember { com.pulseloop.coach.config.CoachProviderSettingsStore(context) } diff --git a/app/src/main/java/com/pulseloop/ui/screens/DebugScreen.kt b/app/src/main/java/com/pulseloop/ui/screens/DebugScreen.kt index 2319243..bfea407 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/DebugScreen.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/DebugScreen.kt @@ -253,6 +253,8 @@ private fun labelFor(event: PulseEvent): String = when (event) { is PulseEvent.Spo2Result -> "SpO₂" is PulseEvent.Spo2Complete -> "SpO₂ Done" is PulseEvent.MeasurementRejected -> "Measure Rejected" + is PulseEvent.MeasurementComplete -> if (event.success) "Measure Done" else "Measure Failed" + is PulseEvent.LiveSampleGate -> if (event.closed) "Live Samples Held" else "Live Samples Stored" is PulseEvent.BloodPressureSample -> "Blood Pressure" is PulseEvent.BloodSugarSample -> "Glucose" is PulseEvent.WearState -> if (event.worn) "Worn" else "Not Worn" diff --git a/app/src/main/java/com/pulseloop/ui/screens/Screens.kt b/app/src/main/java/com/pulseloop/ui/screens/Screens.kt index c138a86..2f60aaf 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/Screens.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/Screens.kt @@ -80,10 +80,13 @@ fun VitalsScreen( val spotMode = !combinedMode && ( state.supportsManualHr || state.supportsManualSpo2 || state.supportsBP ) + // The spot bound is per-ring: a family that ends its own HR measurement gets a longer + // ceiling (issue #59), and the countdown must not run out while the leg is still measuring. val measureSeconds = if (combinedMode) com.pulseloop.service.RingSyncCoordinator.COMBINED_MEASURE_SECONDS else - com.pulseloop.service.RingSyncCoordinator.SPOT_MEASURE_SECONDS + coordinator?.spotMeasureSeconds + ?: com.pulseloop.service.RingSyncCoordinator.SPOT_MEASURE_SECONDS // Card chrome state (value / status / trend / footer) is factory-built once per state // emission, off the composition path — the cards below run no threshold math. // remember{}: the ApiKeyStore constructor does Keystore + encrypted-prefs I/O — too @@ -601,7 +604,23 @@ fun VitalDetailScreen( } } - // 4. Reference zones — colored dot + label + range per zone. + // 4. Readings — every individual measurement in the window, newest first, each + // deletable (issue #60). Collapsed by default: a Month of all-day history runs to + // hundreds of rows, and the chart above is what the screen is normally for. + if (state.readings.isNotEmpty()) { + item { + ReadingsCard( + readings = state.readings, + metric = metric, + period = state.period, + unitLabel = state.thresholds?.unitLabel ?: "", + gUnit = gUnit, + onDelete = { vm.deleteReading(it) }, + ) + } + } + + // 5. Reference zones — colored dot + label + range per zone. if (state.engineZones.isNotEmpty()) { item { val cardShape = RoundedCornerShape(20.dp) @@ -649,7 +668,7 @@ fun VitalDetailScreen( } } - // 5. What this means + // 6. What this means item { val cardShape = RoundedCornerShape(20.dp) Column( @@ -677,7 +696,7 @@ fun VitalDetailScreen( } } - // 6. Estimated-metric disclaimer (BP + glucose only, iOS warning card). + // 7. Estimated-metric disclaimer (BP + glucose only, iOS warning card). metricDisclaimer(metric)?.let { disclaimer -> item { val cardShape = RoundedCornerShape(20.dp) @@ -709,6 +728,160 @@ fun VitalDetailScreen( } } +/** + * The window's individual readings, with a delete affordance per row (issue #60). + * + * Deletion only — a recorded health value can be removed but never edited into a different + * number — and always behind a confirmation, because it cannot be undone: the row is gone, and a + * reading the ring supplied is tombstoned so the next sync of that day won't restore it. + */ +@Composable +private fun ReadingsCard( + readings: List, + metric: String, + period: Period, + unitLabel: String, + gUnit: com.pulseloop.service.GlucoseUnit, + onDelete: (VitalDetailViewModel.Reading) -> Unit, +) { + var expanded by remember { mutableStateOf(false) } + var pendingDelete by remember { mutableStateOf(null) } + // Rows are composed eagerly inside one LazyColumn item — the parent's 18dp item spacing means + // the card can't be split across items without losing its chrome — so the list is paged rather + // than rendered whole. A Month of 5-minute all-day history is thousands of readings; a page of + // PAGE_SIZE is what someone scanning for a bad measurement actually reads. + var visibleCount by remember(readings.size) { mutableStateOf(READINGS_PAGE_SIZE) } + val cardShape = RoundedCornerShape(20.dp) + + fun readingText(r: VitalDetailViewModel.Reading): String { + val primary = formatStat(r.value, metric, gUnit) + return r.secondary?.let { "$primary/${formatStat(it, metric, gUnit)}" } ?: primary + } + + Column( + Modifier + .fillMaxWidth() + .clip(cardShape) + .background(PulseColors.card) + .border(1.dp, PulseColors.borderSubtle, cardShape), + ) { + Row( + Modifier + .fillMaxWidth() + .clickable { expanded = !expanded } + .padding(16.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + "READINGS", + fontSize = 11.sp, + fontWeight = FontWeight.SemiBold, + letterSpacing = 1.sp, + color = PulseColors.textMuted, + ) + Spacer(Modifier.weight(1f)) + Text( + readings.size.toString(), + fontSize = 12.sp, + color = PulseColors.textMuted, + modifier = Modifier.padding(end = 6.dp), + ) + Icon( + if (expanded) Icons.Filled.ExpandLess else Icons.Filled.ExpandMore, + contentDescription = if (expanded) "Hide readings" else "Show readings", + tint = PulseColors.textMuted, + modifier = Modifier.size(20.dp), + ) + } + + if (expanded) { + // Not a nested LazyColumn: this card already sits inside one, and nesting two + // scrollers in the same axis crashes Compose. Bounded by [visibleCount] instead. + readings.take(visibleCount).forEach { reading -> + Row( + Modifier.fillMaxWidth().padding(start = 16.dp, end = 6.dp, bottom = 4.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + tooltipTime(reading.timestamp, period), + fontSize = 13.sp, + color = PulseColors.textSecondary, + modifier = Modifier.weight(1f), + ) + Text( + readingText(reading), + fontSize = 13.sp, + fontWeight = FontWeight.Medium, + color = PulseColors.textPrimary, + ) + if (unitLabel.isNotEmpty()) { + Text( + " $unitLabel", + fontSize = 11.sp, + color = PulseColors.textMuted, + ) + } + IconButton( + onClick = { pendingDelete = reading }, + modifier = Modifier.size(36.dp), + ) { + Icon( + Icons.Filled.DeleteOutline, + contentDescription = "Delete this reading", + tint = PulseColors.textMuted, + modifier = Modifier.size(18.dp), + ) + } + } + } + if (readings.size > visibleCount) { + TextButton( + onClick = { visibleCount += READINGS_PAGE_SIZE }, + modifier = Modifier.padding(start = 8.dp), + ) { + Text("Show ${minOf(READINGS_PAGE_SIZE, readings.size - visibleCount)} more", fontSize = 13.sp) + } + } + Spacer(Modifier.height(8.dp)) + } + } + + pendingDelete?.let { reading -> + AlertDialog( + onDismissRequest = { pendingDelete = null }, + title = { Text("Delete this reading?") }, + text = { + Text( + buildString { + append(readingText(reading)) + if (unitLabel.isNotEmpty()) append(" $unitLabel") + append(" at ") + append(tooltipTime(reading.timestamp, period)) + append(".\n\nThis can't be undone.") + // Say so plainly rather than letting a user wonder why the reading + // didn't come back after a re-sync — that is deliberate. + if (reading.fromHistory) { + append(" The reading stays deleted the next time this day syncs.") + } + }, + ) + }, + confirmButton = { + TextButton(onClick = { + onDelete(reading) + pendingDelete = null + }) { Text("Delete", color = MaterialTheme.colorScheme.error) } + }, + dismissButton = { + TextButton(onClick = { pendingDelete = null }) { Text("Cancel") } + }, + ) + } +} + +/** How many readings the list shows before asking (issue #60) — see [ReadingsCard]. */ +private const val READINGS_PAGE_SIZE = 50 + /** iOS-style segmented control for Today/Week/Month. */ @Composable private fun PeriodSegmentedControl(selected: Period, onSelect: (Period) -> Unit) { diff --git a/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt b/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt index 0b92f6a..78132eb 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SleepScreen.kt @@ -47,6 +47,7 @@ import com.pulseloop.service.SleepFormat import com.pulseloop.service.SleepInsights import com.pulseloop.service.SleepQualityLabel import com.pulseloop.service.SleepRangeKey +import com.pulseloop.service.spanMinutes import com.pulseloop.ui.components.CoachMessageCard import com.pulseloop.ui.theme.PulseColors import com.pulseloop.ui.viewmodels.SleepViewModel @@ -149,7 +150,7 @@ private fun androidx.compose.foundation.lazy.LazyListScope.sessionPageItems( item { SessionHero(session, blocks) } item { VisualizationCard(eyebrow = "Stages", title = "Sleep architecture", legend = true) { - SleepHypnogram(blocks = blocks, totalMin = session.totalMinutes, startTs = session.startAt) + SleepHypnogram(blocks = blocks, spanMin = session.spanMinutes, startTs = session.startAt) } } item { @@ -203,7 +204,7 @@ private fun SleepCarousel( ) SessionHero(s, blocks) VisualizationCard(eyebrow = "Stages", title = "Sleep architecture", legend = true) { - SleepHypnogram(blocks = blocks, totalMin = s.totalMinutes, startTs = s.startAt) + SleepHypnogram(blocks = blocks, spanMin = s.spanMinutes, startTs = s.startAt) } val byStage = blocks.groupBy { it.stageRaw }.mapValues { (_, b) -> b.sumOf { it.durationMinutes } } SleepStageSummaryCards( @@ -441,7 +442,7 @@ private fun LegendItem(label: String, color: Color) { @Composable private fun SleepHypnogram( blocks: List, - totalMin: Int, + spanMin: Int, startTs: Long, height: androidx.compose.ui.unit.Dp = 210.dp, ) { @@ -457,7 +458,11 @@ private fun SleepHypnogram( val sorted = remember(blocks) { blocks.filter { it.durationMinutes > 0 && it.stageRaw != "UNKNOWN" }.sortedBy { it.startMinute } } - val safeTotal = if (totalMin > 0) totalMin else 1 + // The x axis is wall-clock time, so it is scaled by the session's SPAN, never by its duration. + // Since issue #63 those are different numbers — `totalMinutes` is time asleep and excludes the + // awake stretches and the gap between a split night's two records, so scaling by it would + // compress the plot and mislabel every tick. + val safeTotal = if (spanMin > 0) spanMin else 1 val ticks = listOf(0, safeTotal / 3, safeTotal * 2 / 3, safeTotal).map { offset -> clockTime(startTs + offset * 60_000L) } diff --git a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt index 68e0d25..d850585 100644 --- a/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt +++ b/app/src/main/java/com/pulseloop/ui/viewmodels/ViewModels.kt @@ -5,6 +5,7 @@ import androidx.lifecycle.viewModelScope import com.pulseloop.data.DemoDataPolicy import com.pulseloop.data.PulseLoopDatabase import com.pulseloop.data.dao.Bucket +import com.pulseloop.data.dao.MeasurementDeletionDao import com.pulseloop.data.entity.* import com.pulseloop.ring.* import com.pulseloop.coach.summaries.CoachSummaryKind @@ -956,6 +957,27 @@ class VitalDetailViewModel( val engineZones: List = emptyList(), val loading: Boolean = true, val isBP: Boolean = false, + /** The window's individual readings, newest first — the list the user deletes from + * (issue #60). Demo/seeded rows are included: a user clearing seeded noise out of a + * chart is the same gesture as removing a bad measurement. */ + val readings: List = emptyList(), + ) + + /** + * One reading as the detail list shows it (issue #60). + * + * [ids] is a list because a blood-pressure reading is stored as two rows sharing a timestamp, + * and deleting one without the other would leave a systolic charted against no diastolic. + * [value]/[secondary] are already in display units, so the list agrees with the chart above it. + */ + data class Reading( + val ids: List, + val timestamp: Long, + val value: Double, + val secondary: Double? = null, + /** True when the ring supplied this from its own log, so a re-sync could restore it — + * which is why deleting it also writes a tombstone. Shown as provenance in the list. */ + val fromHistory: Boolean = false, ) private val _state = MutableStateFlow(DetailState()) @@ -1152,10 +1174,25 @@ class VitalDetailViewModel( val thisAvg = if (points.isNotEmpty()) points.average() else null val trend = computeTrend(thisAvg, prevAvg, if (allValues.isNotEmpty()) allValues.max() - allValues.min() else 1.0) + // One list row per reading, pairing each systolic with the diastolic at the same + // instant so a delete takes the whole reading. + val diaByTime = diaSamples.associateBy { it.timestamp } + val bpReadings = sysSamples.map { sys -> + val dia = diaByTime[sys.timestamp] + Reading( + ids = listOfNotNull(sys.id, dia?.id), + timestamp = sys.timestamp, + value = sys.value, + secondary = dia?.value, + fromHistory = sys.id.startsWith(MeasurementDeletionDao.HISTORY_ID_PREFIX), + ) + }.asReversed() + _state.update { it.copy( anchor = anchor, points = points, secondary = secondary, labels = labels, timestamps = times, + readings = bpReadings, // iOS uses the window's last reading, not the global latest. latest = points.lastOrNull(), min = allValues.minOrNull(), avg = thisAvg, max = allValues.maxOrNull(), @@ -1206,10 +1243,20 @@ class VitalDetailViewModel( com.pulseloop.service.VitalsThresholdEngine.zones(k, physiology, baseline = baseline) } ?: emptyList() + val readings = samples.map { + Reading( + ids = listOf(it.id), + timestamp = it.timestamp, + value = convert(it.value), + fromHistory = it.id.startsWith(MeasurementDeletionDao.HISTORY_ID_PREFIX), + ) + }.asReversed() + _state.update { it.copy( anchor = anchor, points = points, secondary = emptyList(), labels = labels, timestamps = times, + readings = readings, // iOS uses the window's last reading, not the global latest. latest = points.lastOrNull(), min = points.minOrNull(), avg = thisAvg, max = points.maxOrNull(), @@ -1221,6 +1268,25 @@ class VitalDetailViewModel( } } + /** + * Remove one reading from the record (issue #60). + * + * Deletion, never editing: a stored health value may be taken out, but never changed into a + * different number. [com.pulseloop.data.MeasurementDeletion] owns the two rules that make it + * stick — tombstone anything the ring could re-sync, and take a blood-pressure reading's two + * rows together — so this only has to refresh the window afterwards. + */ + fun deleteReading(reading: Reading) { + viewModelScope.launch { + try { + com.pulseloop.data.MeasurementDeletion.deleteByIds(db, reading.ids) + } catch (_: Exception) { + // A failed delete must not take the screen down; the row simply stays. + } + try { refresh() } catch (_: Exception) {} + } + } + private fun buildLabels(buckets: List, period: Period): List { if (buckets.isEmpty()) return emptyList() val zone = ZoneId.systemDefault() diff --git a/app/src/main/java/com/pulseloop/wearables/WearableModel.kt b/app/src/main/java/com/pulseloop/wearables/WearableModel.kt index dc3a1f9..9480a84 100644 --- a/app/src/main/java/com/pulseloop/wearables/WearableModel.kt +++ b/app/src/main/java/com/pulseloop/wearables/WearableModel.kt @@ -118,6 +118,30 @@ data class WearableModel( imageRes = R.drawable.ring_yawell_r11, ) + /** + * The **R100** (issue #58) — a white-label CRP ring that ships with the same Moyoung + * "Da Rings" app as [COLMI_R11_CRP], firmware `MOY-R2E3-*`. Its reporter had to know to + * pick the R11 card for it; this card is so they don't. + * + * Unlike the R11 it advertises a usable name, so it can be matched at scan instead of + * relying on the post-connect `fdda` re-route. The pattern is deliberately loose about the + * suffix because the only capture we have is a redacted diagnostics report, which strips a + * `_` serial — the raw name is `R100` or `R100_`. The underscore is what keeps + * this off [SMARTHEALTH_NAME_PATTERN], whose serial is **space**-separated: an earlier + * `^R100([ _-].*)?$` also matched `R100 1A2B`, and because this entry precedes + * [COLMI_SMARTHEALTH] in [CATALOG] it would have taken a SmartHealth-firmware ring onto the + * CRP driver, which connects and then never syncs. It cannot collide with the Colmi + * `^R10_[0-9A-F]{4}$` either. + * + * Blurb omits stress: the R100 answers neither the stress-history query (`2/47`) nor the + * stress monitor-state read-back (`2/45`) — 22 sends, 0 replies in the #58 capture. + */ + val R100 = WearableModel( + id = "r100", displayName = "R100", brand = "Da Rings", family = RingDeviceType.CRP, + tint = PulseColors.hrv, blurb = "HR · SpO₂ · HRV · Temp · Sleep", + advertisedNamePatterns = listOf("^R100(_[0-9A-Fa-f]+)?$"), + ) + // Yawell-branded variants val YAWELL_R05 = colmi("yawell-r05", "Yawell R05", "Yawell", "^R05_[0-9A-F]{4}$", R.drawable.ring_yawell_r05) val YAWELL_R10 = colmi("yawell-r10", "Yawell R10", "Yawell", "^R10_[0-9A-F]{4}$", R.drawable.ring_yawell_r10) @@ -205,7 +229,7 @@ data class WearableModel( val CATALOG: List = listOf( COLMI_R02, COLMI_R06, COLMI_R10, YAWELL_R11, JRING, COLMI_R03, COLMI_R07, COLMI_R08, COLMI_R09, COLMI_R11, COLMI_R12, - YAWELL_R05, YAWELL_R10, H59, R10M, TK5, LUCK_RING_TK18, COLMI_R11_CRP, RWFIT, + YAWELL_R05, YAWELL_R10, H59, R10M, TK5, LUCK_RING_TK18, COLMI_R11_CRP, R100, RWFIT, // Broadest pattern last: every narrower QRing-Colmi/TK5 entry above gets first shot // in modelForAdvertisedName's scan, so this can only match a name nothing else claims. COLMI_SMARTHEALTH, diff --git a/app/src/test/java/com/pulseloop/data/MeasurementDeletionTest.kt b/app/src/test/java/com/pulseloop/data/MeasurementDeletionTest.kt new file mode 100644 index 0000000..1a15702 --- /dev/null +++ b/app/src/test/java/com/pulseloop/data/MeasurementDeletionTest.kt @@ -0,0 +1,95 @@ +package com.pulseloop.data + +import com.pulseloop.data.dao.MeasurementDeletionDao +import com.pulseloop.data.entity.MeasurementDeletionEntity +import com.pulseloop.data.entity.MeasurementEntity +import com.pulseloop.ring.MeasurementKind +import com.pulseloop.service.historyMeasurementId +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The tombstone rule behind deleting a reading (issue #60), tested against the DAO's own default + * implementation — Room only supplies the queries, so `record`'s decision about *what* is worth + * remembering is plain logic and is where the bug would be. + */ +class MeasurementDeletionTest { + + private class FakeDeletionDao : MeasurementDeletionDao { + val rows = mutableMapOf() + override suspend fun isDeleted(id: String) = id in rows + override suspend fun insertAll(rows: List) { + rows.forEach { this.rows[it.measurementId] = it } + } + } + + private fun measurement(id: String, kind: MeasurementKind, timestamp: Long, source: String) = + MeasurementEntity( + id = id, kindRaw = kind.name, value = 46.0, unit = "bpm", + timestamp = timestamp, sourceRaw = source, + ) + + /** + * A history reading is written with `upsert` under a deterministic id, so the next sync of that + * day would restore it. That is exactly the row the tombstone exists for. + */ + @Test + fun `a history reading is remembered as deleted`() = runTest { + val dao = FakeDeletionDao() + val id = historyMeasurementId(MeasurementKind.HEART_RATE, 1_700_000_000_000L) + + dao.record(listOf(measurement(id, MeasurementKind.HEART_RATE, 1_700_000_000_000L, "history"))) + + assertTrue(dao.isDeleted(id)) + assertEquals(MeasurementKind.HEART_RATE.name, dao.rows.getValue(id).kindRaw) + assertEquals(1_700_000_000_000L, dao.rows.getValue(id).timestamp) + } + + /** + * A live reading's id is a fresh UUID that nothing regenerates. Tombstoning it would grow the + * table forever for a row that can never come back on its own. + */ + @Test + fun `a live reading is deleted without a tombstone`() = runTest { + val dao = FakeDeletionDao() + val id = java.util.UUID.randomUUID().toString() + + dao.record(listOf(measurement(id, MeasurementKind.HEART_RATE, 1_700_000_000_000L, "live"))) + + assertEquals("nothing to remember for a one-off id", 0, dao.rows.size) + } + + @Test + fun `a mixed batch remembers only the regenerable rows`() = runTest { + val dao = FakeDeletionDao() + val historyId = historyMeasurementId(MeasurementKind.SPO2, 42L) + + dao.record( + listOf( + measurement(historyId, MeasurementKind.SPO2, 42L, "history"), + measurement(java.util.UUID.randomUUID().toString(), MeasurementKind.SPO2, 42L, "live"), + ) + ) + + assertEquals(1, dao.rows.size) + assertTrue(dao.isDeleted(historyId)) + } + + /** + * The id scheme lives in `EventPersistenceSubscriber` and the prefix that recognises it lives + * on the DAO. If those two ever drift, deletes of history rows silently stop sticking and the + * readings come back on the next sync — with nothing failing. This is the tripwire. + */ + @Test + fun `every history id carries the prefix the tombstone rule matches on`() { + for (kind in MeasurementKind.entries) { + val id = historyMeasurementId(kind, 1_700_000_000_000L) + assertTrue( + "$kind history id must start with ${MeasurementDeletionDao.HISTORY_ID_PREFIX}: $id", + id.startsWith(MeasurementDeletionDao.HISTORY_ID_PREFIX), + ) + } + } +} diff --git a/app/src/test/java/com/pulseloop/diagnostics/DiagnosticsRedactorTest.kt b/app/src/test/java/com/pulseloop/diagnostics/DiagnosticsRedactorTest.kt new file mode 100644 index 0000000..c55b2f8 --- /dev/null +++ b/app/src/test/java/com/pulseloop/diagnostics/DiagnosticsRedactorTest.kt @@ -0,0 +1,93 @@ +package com.pulseloop.diagnostics + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The privacy scrub applied to an exported diagnostics report: physiological payloads are masked, + * the routing bytes that say which record a frame is are not (issue #58). + */ +class DiagnosticsRedactorTest { + + /** A real CRP temperature-history reply: `FD DA 10 98 02 16` then the day, frame index and + * slot values. The header must survive so a reader can tell it from any other health frame. */ + private val crpTempFrame = "fdda1098021600000000000000000000" + "6b01" + "00".repeat(20) + + @Test + fun `a CRP health frame keeps its group and command but loses every value`() { + val masked = DiagnosticsRedactor.maskPacketHex(crpTempFrame, "history_measurement", "CRP") + + assertEquals("fdda109802 16 identifies the record", "fdda10980216", masked.take(12)) + assertTrue("no sample bytes survive", masked.drop(12).all { it == '·' }) + assertEquals("length is preserved", crpTempFrame.length, masked.length) + } + + @Test + fun `a YCBT health frame keeps its four-byte header`() { + val frame = "041300" + "48".repeat(9) + val masked = DiagnosticsRedactor.maskPacketHex(frame, "hr_sample", "COLMI_SMART_HEALTH") + + assertEquals("04130048", masked.take(8)) + assertTrue(masked.drop(8).all { it == '·' }) + } + + /** Every family behind a YCBTDriver shares the frame — the R10M path (`YCBT`) and TK5 must not + * fall to the one-byte rule, or their health frames export as an anonymous `04··…`. */ + @Test + fun `every YCBT-driven family keeps the four-byte header`() { + val frame = "041300" + "48".repeat(9) + for (family in listOf("YCBT", "TK5", "COLMI_SMART_HEALTH")) { + val masked = DiagnosticsRedactor.maskPacketHex(frame, "hr_sample", family) + assertEquals(family, "04130048", masked.take(8)) + assertTrue(family, masked.drop(8).all { it == '·' }) + } + } + + /** Families whose opcode is byte 0 keep exactly that, as before — and so does an unknown one. */ + @Test + fun `other families keep only the opcode byte`() { + val frame = "69" + "5a".repeat(15) + for (type in listOf("COLMI_R02", "JRING", "LUCK_RING", "")) { + val masked = DiagnosticsRedactor.maskPacketHex(frame, "hr_sample", type) + assertEquals("opcode kept for $type", "69", masked.take(2)) + assertTrue("payload masked for $type", masked.drop(2).all { it == '·' }) + } + } + + /** + * A Colmi `0x78` sport push with bpm 0 (warm-up, contact lost) still carries the workout's + * live steps, distance and calories. It used to decode to nothing, fall through to `unknown`, + * and export in clear while the neighbouring frames with a bpm were masked. + */ + @Test + fun `a sport telemetry frame is masked even when it carried no heart rate`() { + val frame = "7801" + "0000" + "00" + "0007d0" + "0005dc" + "00c350" + val masked = DiagnosticsRedactor.maskPacketHex(frame, "sport_telemetry", "COLMI") + assertEquals("78", masked.take(2)) + assertTrue("steps, distance and calories do not survive", masked.drop(2).all { it == '·' }) + } + + @Test + fun `control frames are never masked`() { + val frame = "fdda100603030102" + assertEquals(frame, DiagnosticsRedactor.maskPacketHex(frame, "firmware_revision", "CRP")) + assertEquals(frame, DiagnosticsRedactor.maskPacketHex(frame, "command_ack", "CRP")) + } + + /** A health frame shorter than its family's header must still lose a byte to masking rather + * than being exported whole. */ + @Test + fun `a frame shorter than the header still masks its tail`() { + val masked = DiagnosticsRedactor.maskPacketHex("fdda1098", "history_measurement", "CRP") + assertEquals("fdda10··", masked) + } + + @Test + fun `MAC addresses are scrubbed from free text`() { + assertEquals( + "connected to ··:··:··:··:··:·· ok", + DiagnosticsRedactor.scrubText("connected to A4:C1:38:9F:2B:07 ok"), + ) + } +} diff --git a/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt b/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt index acdd946..f13133d 100644 --- a/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt +++ b/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt @@ -379,6 +379,59 @@ class CRPDecoderTest { assertTrue(samples.any { it.value == 32.0 && it._timestamp == Instant.parse("2026-07-24T00:50:00Z") }) } + /** + * Real group-2/cmd-22 temperature frame (day 0, frame 0) from the R100 capture attached to + * issue #58 — the first non-empty temperature reply we have seen from any CRP ring, and the + * evidence that settled a layout `CRPProtocol` had carried as unconfirmed for months. + */ + private val tempHistoryFrame = + "fdda10980216000000000000000000006b01000000000000000000000000000000000000660100000000000000006a010000" + + "000000000000690100000000000000006b010000000000000000680100000000000000000000000000000000000069010000" + + "0000000000006901000000000000000064010000000000000000640100000000000000006601000000000000000000000000" + + "0000" + + @Test + fun `temperature history frame decodes little-endian tenths of a degree per slot`() { + val now = Instant.parse("2026-08-31T12:00:00Z") + val samples = CRPDecoder.decode(hexToBytes(tempHistoryFrame), fdd3, now, ZoneId.of("UTC")) + .filterIsInstance() + assertEquals(11, samples.size) + assertTrue(samples.all { it.kind_field == MeasurementKind.TEMPERATURE }) + // Slot 4 (0x016b = 363 tenths) → 36.3 °C at 00:20. 2 bytes/slot, so 72 slots per frame. + assertEquals(36.3, samples.first().value, 0.001) + assertEquals(Instant.parse("2026-08-31T00:20:00Z"), samples.first()._timestamp) + // Slot 64 (0x0166) → 35.8 °C at 05:20 — the last reading in the frame. + assertEquals(35.8, samples.last().value, 0.001) + assertEquals(Instant.parse("2026-08-31T05:20:00Z"), samples.last()._timestamp) + // Every sample is a plausible skin temperature, i.e. nothing decoded as raw tenths. + assertTrue(samples.all { it.value in 28.0..50.0 }) + } + + /** The vendor rejects anything outside 28.0–50.0 °C as "no reading" (`e1/m.a`), which is how a + * slot the ring never filled stays out of the record instead of charting as 0 °C. */ + @Test + fun `temperature slots outside the vendor's plausible range are dropped`() { + // day 0, frame 0, then: 0 (empty), 271 (27.1 °C, too low), 501 (50.1 °C, too high), 365. + val payload = byteArrayOf(0, 0) + + byteArrayOf(0, 0, 0x0F, 0x01, 0xF5.toByte(), 0x01, 0x6D, 0x01) + val frame = CRPProtocol.frame(CRPCommands.GROUP_HISTORY, CRPCommands.CMD_QUERY_HISTORY_TEMP, payload) + val samples = CRPDecoder.decode(frame, fdd3).filterIsInstance() + assertEquals(1, samples.size) + assertEquals(36.5, samples.single().value, 0.001) + } + + /** Temperature frames must drive the next-frame pull like every other timing vital — before + * issue #58 they fell through to a bare ack, so the ring was asked for frame 0 forever. */ + @Test + fun `temperature history frame emits the follow-up marker`() { + val frame = CRPProtocol.frame( + CRPCommands.GROUP_HISTORY, CRPCommands.CMD_QUERY_HISTORY_TEMP, byteArrayOf(0, 2) + ByteArray(144) + ) + val marker = CRPDecoder.decode(frame, fdd3).filterIsInstance().single() + assertEquals(CRPCommands.CMD_QUERY_HISTORY_TEMP, marker.cmd) + assertEquals(2, marker.frameIndex) + } + @Test fun `an all-zero timing frame yields no samples, only the follow-up marker`() { // zaggash's SpO2 timeline came back all-zero (no all-day SpO2 recorded) — decode must not diff --git a/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt b/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt index 0c1c95c..5dff048 100644 --- a/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt +++ b/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt @@ -198,6 +198,24 @@ class CRPSyncEngineTest { assertTrue(w.sent.isEmpty()) } + /** Temperature is 2 bytes/slot like HRV, so it also spans four 72-slot frames (`e1/m.d` asks + * for the next index until `3 == index`). Before issue #58 it emitted no marker at all, so + * frames 1-3 — 18:00 onward of every day — were never requested. */ + @Test + fun `temperature walks four frames like HRV`() { + val w = FakeWriter() + val engine = CRPSyncEngine(w) + engine.runStartup(); w.sent.clear() + for (idx in 0..2) { + engine.handle(RingDecodedEvent.TimingHistoryFrame(CRPCommands.CMD_QUERY_HISTORY_TEMP, 0, idx)) + assertEquals(listOf(2 to 22), w.opcodes()) + assertEquals(idx + 1, w.sent.last()[7].toInt()) + w.sent.clear() + } + engine.handle(RingDecodedEvent.TimingHistoryFrame(CRPCommands.CMD_QUERY_HISTORY_TEMP, 0, 3)) + assertTrue("terminal temperature frame must not request another", w.sent.isEmpty()) + } + @Test fun `a repeated frame does not spam duplicate follow-up requests`() { val w = FakeWriter() diff --git a/app/src/test/java/com/pulseloop/ring/ColmiDecoderTest.kt b/app/src/test/java/com/pulseloop/ring/ColmiDecoderTest.kt index fe500a0..a6abad6 100644 --- a/app/src/test/java/com/pulseloop/ring/ColmiDecoderTest.kt +++ b/app/src/test/java/com/pulseloop/ring/ColmiDecoderTest.kt @@ -34,6 +34,26 @@ class ColmiDecoderTest { // MARK: Framing / checksum + /** Issue #64 + the redactor rule in AGENTS.md: a `0x78` frame's diagnostic kind must be one + * the redactor masks, whether or not the frame carried a plausible bpm. */ + @Test + fun `every sport telemetry frame is tagged for masking, and only a plausible bpm becomes a sample`() { + fun frame(bpm: Int) = ColmiPacket.frame(byteArrayOf( + 0x78, 0x01, 0x00, 0x00, 0x00, bpm.toByte(), + 0x00, 0x07, 0xd0.toByte(), // 2000 steps + )) + val warmUp = ColmiDecoder.decodeNormal(frame(0)) + assertEquals(1, warmUp.size) + assertTrue("a 0-bpm frame is still tagged", warmUp[0] is RingDecodedEvent.SportTelemetry) + assertEquals("sport_telemetry", warmUp[0].kind) + + val live = ColmiDecoder.decodeNormal(frame(132)) + assertEquals(2, live.size) + assertTrue("the telemetry tag comes first so it is the frame's diagnostic kind", + live[0] is RingDecodedEvent.SportTelemetry) + assertEquals(132, (live[1] as RingDecodedEvent.HeartRateSample).bpm) + } + @Test fun `frame appends checksum and is 16 bytes`() { val framed = ColmiPacket.frame(byteArrayOf(0x03)) diff --git a/app/src/test/java/com/pulseloop/ring/ColmiSportModeTest.kt b/app/src/test/java/com/pulseloop/ring/ColmiSportModeTest.kt new file mode 100644 index 0000000..5d370c3 --- /dev/null +++ b/app/src/test/java/com/pulseloop/ring/ColmiSportModeTest.kt @@ -0,0 +1,188 @@ +package com.pulseloop.ring + +import org.junit.Assert.* +import org.junit.Test + +/** + * Issue #64 — a Colmi R09 in a PulseLoop workout flashed its LED now and then and produced a bpm + * about once a minute, where the QRing app flashes almost constantly and reads every ~10 s. + * + * QRing does not use the realtime-HR commands in a live activity at all. `SportRunningActivity` + * sends `PhoneSportReq.getSportStatus(1, sportType)` = `0x77 01 ` on entry and consumes the + * ring's own unsolicited `0x78` telemetry (`DeviceNotifyRsp`; bpm at payload byte 4) until it + * sends `0x77 04`. There is no timer and no keepalive on that path — the ring drives the cadence. + */ +class ColmiSportModeTest { + + private class RecordingWriter : RingCommandWriter { + val commands = mutableListOf() + override fun enqueue(command: ByteArray) { commands.add(command) } + fun opcodes(): List = commands.map { it[0].toInt() and 0xFF } + fun clear() = commands.clear() + } + + /** `[0x78][dataType][status][durMin×2][bpm][steps×3][metres×3][cal×3]` as the ring pushes it. */ + private fun sportFrame(bpm: Int, status: Int = 1): ByteArray = ColmiPacket.frame(byteArrayOf( + 0x78, 0x00, status.toByte(), 0x00, 0x05, bpm.toByte(), + 0x00, 0x01, 0x2C, 0x00, 0x00, 0x64, 0x00, 0x00, 0x00, + )) + + private fun engineWith(writer: RecordingWriter) = ColmiSyncEngine(writer, ColmiDecoder) + + @Test + fun `a workout starts a ring-side sport session, not an HR stream`() { + val writer = RecordingWriter() + val engine = engineWith(writer) + + engine.startWorkoutHeartRate("cycle") + + assertArrayEquals(byteArrayOf(0x77, 0x01, 0x09), writer.commands.single()) + engine.destroy() + } + + @Test + fun `activity types map onto QRing's sport list, unknown ones onto Other sports`() { + assertEquals(0x04.toUByte(), ColmiEncoder.sportType("walk")) + assertEquals(0x07.toUByte(), ColmiEncoder.sportType("run")) + assertEquals(0x09.toUByte(), ColmiEncoder.sportType("cycle")) + assertEquals(0x08.toUByte(), ColmiEncoder.sportType("hike")) + assertEquals(0x16.toUByte(), ColmiEncoder.sportType("yoga")) + assertEquals(0x0A.toUByte(), ColmiEncoder.sportType("gym")) + assertEquals(0x0A.toUByte(), ColmiEncoder.sportType("other")) + } + + @Test + fun `sport telemetry decodes to a heart-rate sample from payload byte 4`() { + val events = ColmiDecoder.decodeNormal(sportFrame(bpm = 132)) + val sample = events.filterIsInstance().single() + assertEquals(132, sample.bpm) + } + + @Test + fun `a warm-up telemetry frame with no bpm is not a reading`() { + assertTrue(ColmiDecoder.decodeNormal(sportFrame(bpm = 0)).none { it is RingDecodedEvent.HeartRateSample }) + } + + @Test + fun `a restart after a spot measure does not reset the ring's sport record`() { + val writer = RecordingWriter() + val engine = engineWith(writer) + engine.startWorkoutHeartRate("run") + writer.clear() + + // The coordinator's spot-measure cleanup: stop the spot stream, then bring the workout back. + engine.stopHeartRate() + engine.startWorkoutHeartRate("run") + + assertTrue("no 0x77 may be re-sent mid-session: got ${writer.opcodes()}", 0x77 !in writer.opcodes()) + engine.destroy() + } + + @Test + fun `ending the workout stops the sport session`() { + val writer = RecordingWriter() + val engine = engineWith(writer) + engine.startWorkoutHeartRate("walk") + writer.clear() + + engine.stopWorkoutHeartRate() + + assertArrayEquals(byteArrayOf(0x77, 0x04, 0x04), writer.commands.first()) + engine.destroy() + } + + @Test + fun `a ring that rejects the sport start falls back to the plain stream, and stays there`() { + val writer = RecordingWriter() + val engine = engineWith(writer) + engine.startWorkoutHeartRate("run") + writer.clear() + + engine.handleRawNotify(ColmiPacket.frame(byteArrayOf(0xF7.toByte(), 0x01))) + assertEquals("fallback probes the realtime stream as before", listOf(0x1E), writer.opcodes()) + + engine.stopWorkoutHeartRate() + writer.clear() + engine.startWorkoutHeartRate("run") + assertEquals("the refusal is remembered for the next workout", listOf(0x1E), writer.opcodes()) + engine.destroy() + } + + @Test + fun `the watchdog resumes a silent session once, then gives it up`() { + val writer = RecordingWriter() + val engine = engineWith(writer) + val t0 = 1_000_000L + engine.startWorkoutHeartRate("cycle") + writer.clear() + + // Fresh telemetry: nothing to do. + engine.handleRawNotify(sportFrame(bpm = 120)) + engine.sportWatchdogTick(System.currentTimeMillis() + 10_000) + assertTrue(writer.commands.isEmpty()) + + // Past the idle bound: one resume. + engine.sportWatchdogTick(System.currentTimeMillis() + ColmiSyncEngine.SPORT_TELEMETRY_IDLE_MS + 1) + assertArrayEquals(byteArrayOf(0x77, 0x03, 0x09), writer.commands.single()) + writer.clear() + + // Still silent after the resume: stop the session and fall back. + engine.sportWatchdogTick(System.currentTimeMillis() + 2 * ColmiSyncEngine.SPORT_TELEMETRY_IDLE_MS + 2) + assertEquals(listOf(0x77, 0x1E), writer.opcodes()) + assertArrayEquals(byteArrayOf(0x77, 0x04, 0x09), writer.commands.first()) + engine.destroy() + } + + @Test + fun `the ring ending the session itself moves the workout onto the plain stream`() { + val writer = RecordingWriter() + val engine = engineWith(writer) + engine.startWorkoutHeartRate("run") + writer.clear() + + engine.handleRawNotify(sportFrame(bpm = 0, status = 3)) + + assertEquals(listOf(0x1E), writer.opcodes()) + engine.destroy() + } + + /** + * Status 3 is the vendor's "this session finished" push — its running screen answers by + * closing the screen, not by concluding the ring cannot do sport sessions. A ring timeout or + * the user stopping on the ring must not cost every later workout on the connection the + * protocol this issue is about. + */ + @Test + fun `a session the ring ended does not disable sport mode for the next workout`() { + val writer = RecordingWriter() + val engine = engineWith(writer) + engine.startWorkoutHeartRate("cycle") + engine.handleRawNotify(sportFrame(bpm = 0, status = 3)) + engine.stopWorkoutHeartRate() + writer.clear() + + engine.startWorkoutHeartRate("cycle") + + assertArrayEquals( + "the next workout starts a fresh sport session", + byteArrayOf(0x77, 0x01, 0x09), writer.commands.first(), + ) + engine.destroy() + } + + /** …but the rest of *that* workout stays on the plain stream: re-sending the start after a + * spot measure would ask a ring that just ended its session to open another one. */ + @Test + fun `after the ring ends a session the same workout does not reopen it`() { + val writer = RecordingWriter() + val engine = engineWith(writer) + engine.startWorkoutHeartRate("cycle") + engine.handleRawNotify(sportFrame(bpm = 0, status = 3)) + writer.clear() + + engine.startWorkoutHeartRate("cycle") // the coordinator's post-spot-measure restart + + assertTrue("no 0x77 mid-workout: got ${writer.opcodes()}", 0x77 !in writer.opcodes()) + engine.destroy() + } +} diff --git a/app/src/test/java/com/pulseloop/ring/PairingMatchingTest.kt b/app/src/test/java/com/pulseloop/ring/PairingMatchingTest.kt index e8d64b2..e2cfac7 100644 --- a/app/src/test/java/com/pulseloop/ring/PairingMatchingTest.kt +++ b/app/src/test/java/com/pulseloop/ring/PairingMatchingTest.kt @@ -169,6 +169,10 @@ class PairingMatchingTest { "H59_anything" to "h59", "R10M FCF4" to "r10m", "R10M_FCF4" to "r10m", + // Issue #58's CRP ring. Both suffix forms, because the only capture we have is a + // redacted report and it strips a `_` serial. + "R100" to "r100", + "R100_1A2B" to "r100", ) for ((name, modelID) in expected) { assertEquals(name, modelID, WearableModel.modelForAdvertisedName(name)?.id) @@ -363,4 +367,24 @@ class PairingMatchingTest { val defaultCaps = WearableCapability.fromCsv("") assertTrue(defaultCaps.isEmpty()) } + + /** The R100 card must not shadow, or be shadowed by, the neighbours its name sits between: + * the Colmi R10 (`R10_xxxx`) and the space-suffixed SmartHealth convention (issue #58). */ + @Test + fun `the R100 pattern cannot collide with the R10 or the SmartHealth convention`() { + assertEquals("yawell-r10", WearableModel.modelForAdvertisedName("R10_DEAD")?.id) + assertEquals("colmi-r10", WearableModel.modelForAdvertisedName("COLMI R10_xyz")?.id) + assertEquals("r100", WearableModel.modelForAdvertisedName("R100_DEAD")?.id) + // A space-separated 4-hex suffix is the SmartHealth convention, not an R100 serial — and + // the R100 card precedes the SmartHealth one, so a loose pattern here would take a + // SmartHealth ring onto the CRP driver, which connects and then never syncs. + assertEquals( + "colmi-smarthealth", + WearableModel.modelForAdvertisedName("R100 1A2B")?.id, + ) + // A SmartHealth-convention name (space + four hex) still lands on the broad card. + assertEquals("colmi-smarthealth", WearableModel.modelForAdvertisedName("Ale-Hop2211 E1C7")?.id) + // And the R100 routes to the CRP driver family, not to Colmi's. + assertEquals(RingDeviceType.CRP, WearableModel.modelForAdvertisedName("R100")?.family) + } } diff --git a/app/src/test/java/com/pulseloop/ring/PulseEventBusTest.kt b/app/src/test/java/com/pulseloop/ring/PulseEventBusTest.kt index 63e11e9..97a5028 100644 --- a/app/src/test/java/com/pulseloop/ring/PulseEventBusTest.kt +++ b/app/src/test/java/com/pulseloop/ring/PulseEventBusTest.kt @@ -2,6 +2,7 @@ package com.pulseloop.ring import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.async +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.take import kotlinx.coroutines.flow.toList @@ -25,4 +26,33 @@ class PulseEventBusTest { assertEquals(eventCount, withTimeout(2_000) { collected.await() }.size) } + + /** + * Issue #60: the spot-measurement gate rides the bus *because* delivery keeps publish order. + * The coordinator sends close → (ring samples) → open → settled reading, and the persistence + * collector must see them in exactly that order however far behind the ring it runs. If this + * ever fails, the gate silently lets queued samples through again. + */ + @Test + fun `non-suspending publishes are delivered in publish order`() = runBlocking { + val now = java.time.Instant.now() + val sent = listOf( + PulseEvent.LiveSampleGate(MeasurementKind.HEART_RATE, closed = true), + PulseEvent.HeartRateSample(47, now), + PulseEvent.HeartRateSample(46, now), + PulseEvent.HeartRateSample(81, now), + PulseEvent.LiveSampleGate(MeasurementKind.HEART_RATE, closed = false), + PulseEvent.HeartRateSample(81, now, spot = true), + ) + val collected = async(start = CoroutineStart.UNDISPATCHED) { + PulseEventBus.events + .filter { it is PulseEvent.LiveSampleGate || it is PulseEvent.HeartRateSample } + .take(sent.size) + .toList() + } + + sent.forEach { PulseEventBus.publishBlocking(it) } + + assertEquals(sent, withTimeout(2_000) { collected.await() }) + } } diff --git a/app/src/test/java/com/pulseloop/ring/YCBTDecoderTest.kt b/app/src/test/java/com/pulseloop/ring/YCBTDecoderTest.kt index 02b5c47..f5e0492 100644 --- a/app/src/test/java/com/pulseloop/ring/YCBTDecoderTest.kt +++ b/app/src/test/java/com/pulseloop/ring/YCBTDecoderTest.kt @@ -117,11 +117,36 @@ class YCBTDecoderTest { assertTrue(decodeStatus(byteArrayOf(0x03, 0x01, 16)) is RingDecodedEvent.CommandAck) } + /** + * `04 0e [mode, status]` — the ring ending a spot measurement itself (issue #59). Layout from + * the vendor app: `BaseMeasureActivity.onDataResponse` matches `bArr[0]` against the screen's + * own measurement type and switches on `bArr[1]` (1 success, 2 failed, else cancelled). + */ + @Test + fun `measurement result push reports which measurement ended and how`() { + fun result(payload: ByteArray): RingDecodedEvent { + val frame = YCBTFrame.validating(YCBTFrame.frame(byteArrayOf(0x04, 0x0e) + payload))!! + return decoder.decode(frame).single() + } + + // The reporter's capture: heart rate (mode 0x00), success. + val ok = result(byteArrayOf(0x00, 0x01)) as RingDecodedEvent.MeasurementComplete + assertEquals(YCBTMeasurementMode.HEART_RATE, ok.mode) + assertTrue(ok.success) + + val failed = result(byteArrayOf(0x02, 0x02)) as RingDecodedEvent.MeasurementComplete + assertEquals(YCBTMeasurementMode.SPO2, failed.mode) + assertFalse("2 is the vendor's `measure failed`", failed.success) + + val cancelled = result(byteArrayOf(0x00, 0x03)) as RingDecodedEvent.MeasurementComplete + assertFalse("anything but 1 is a failure", cancelled.success) + } + + /** The vendor ignores a payload it cannot match against a type + status pair; so do we. */ @Test - fun `measurement result push remains an ack without an evidenced payload layout`() { - val frame = YCBTFrame.validating(YCBTFrame.frame(byteArrayOf(0x04, 0x0e, 0x01, 0x02)))!! - val events = decoder.decode(frame) - assertTrue(events.single() is RingDecodedEvent.CommandAck) + fun `a measurement result too short to name a mode stays an ack`() { + val frame = YCBTFrame.validating(YCBTFrame.frame(byteArrayOf(0x04, 0x0e, 0x01)))!! + assertTrue(decoder.decode(frame).single() is RingDecodedEvent.CommandAck) } @Test diff --git a/app/src/test/java/com/pulseloop/ring/YCBTHealthRecordsTest.kt b/app/src/test/java/com/pulseloop/ring/YCBTHealthRecordsTest.kt index 00417ef..659cbbe 100644 --- a/app/src/test/java/com/pulseloop/ring/YCBTHealthRecordsTest.kt +++ b/app/src/test/java/com/pulseloop/ring/YCBTHealthRecordsTest.kt @@ -122,6 +122,37 @@ class YCBTHealthRecordsTest { assertEquals(2, timelines.size) } + /** + * Issue #63: the night the ring split in two — the second record must decode as its own + * session with its own start, not be folded into or lost behind the first. + */ + @Test + fun `two adjacent sessions decode as two timelines with their own starts`() { + val firstStart = 0x31def01c + val secondStart = firstStart + (3 * 60 + 9) * 60 // 03:21, three minutes after 03:18 + val first = sleepSession(listOf(0xf2 to 124 * 60, 0xf1 to 62 * 60), baseStart = firstStart) + val second = sleepSession(listOf(0xf2 to 119 * 60, 0xf1 to 124 * 60), baseStart = secondStart) + val timelines = YCBTHealthRecords.sleep(first + second).filterIsInstance() + + assertEquals(2, timelines.size) + assertEquals(186, timelines[0].stages.size) + assertEquals(243, timelines[1].stages.size) + assertEquals(YCBTBytes.date(firstStart), timelines[0]._timestamp) + assertEquals(YCBTBytes.date(secondStart), timelines[1]._timestamp) + } + + /** A record whose declared length lies must not take the following session down with it: + * the parser resynchronises on the next `af fa`. */ + @Test + fun `a record with a wrong declared length does not swallow the next session`() { + val bad = sleepSession(listOf(0xf2 to 60 * 60)).also { it[2] = 12 } // claims 12 bytes: no segments + val good = sleepSession(listOf(0xf1 to 30 * 60)) + val timelines = YCBTHealthRecords.sleep(bad + good).filterIsInstance() + + assertEquals(1, timelines.size) + assertEquals(30, timelines.single().stages.size) + } + @Test fun `nap segment does not truncate the night`() { val session = sleepSession(listOf( @@ -269,7 +300,104 @@ class YCBTHealthRecordsTest { assertFalse(YCBTHealthRecords.decode(capturedHeartRecords, YCBTHistoryType.HEART).isEmpty()) } - private fun sleepSession(segments: List>): ByteArray { + /** + * Issue #63: the stored run has to end where the ring says the session ended. Concatenating + * each segment's rounded minutes drifts — 470 against this record's declared 474 — and + * `completeSessionSurvivors` grows its retirement run across blocks that abut end-to-start, + * so a minute of drift in the other direction retires a whole neighbouring session. + */ + @Test + fun `a session spans exactly the header's declared start and end`() { + val event = YCBTHealthRecords.sleep(capturedNight).first() as RingDecodedEvent.SleepTimeline + val headerStart = 0x31dee99f + val headerEnd = 0x31df58bd + assertEquals(YCBTBytes.date(headerStart), event._timestamp) + assertEquals((headerEnd - headerStart) / 60, event.stages.size) + } + + /** + * Issue #63: the night that broke on rc6 — two records a minute apart. The first record's + * segments round one minute long, which under concatenation put its end exactly on the + * second's start; the merge rule then read the two as one contiguous run and retired the + * first. Placed against the header, the minute of gap survives. + */ + @Test + fun `a record whose segments round long still ends at its declared end`() { + val firstStart = 0x31def01c + val firstEnd = firstStart + 322 * 60 + val secondStart = firstEnd + 60 + // Sub-minute segments: concatenation floors each at a minute and overshoots the record. + val first = sleepRecord( + firstStart, + firstEnd, + listOf(0xf2 to 320 * 60) + List(4) { 0xf1 to 20 }, + ) + val second = sleepRecord(secondStart, secondStart + 151 * 60, listOf(0xf2 to 151 * 60)) + val timelines = YCBTHealthRecords.sleep(first + second) + .filterIsInstance() + + assertEquals(2, timelines.size) + assertEquals(322, timelines[0].stages.size) + val firstEndInstant = timelines[0]._timestamp.plusSeconds(322 * 60L) + assertTrue(firstEndInstant.isBefore(timelines[1]._timestamp)) + assertEquals(60L, timelines[1]._timestamp.epochSecond - firstEndInstant.epochSecond) + } + + /** A record the ring bounds itself reports wake for the minutes no segment claims. */ + @Test + fun `minutes no segment claims read as awake`() { + val start = 0x31def01c + val record = sleepRecord(start, start + 60 * 60, listOf(0xf2 to 30 * 60)) + val event = YCBTHealthRecords.sleep(record).first() as RingDecodedEvent.SleepTimeline + assertEquals(60, event.stages.size) + assertEquals(30, event.stages.count { it == SleepStage.LIGHT }) + assertEquals(30, event.stages.count { it == SleepStage.AWAKE }) + assertTrue(event.stages.take(30).all { it == SleepStage.LIGHT }) + } + + /** A record whose header carries no bounds keeps the segment-concatenation reading. */ + @Test + fun `a record with no header bounds falls back to concatenated segments`() { + val event = YCBTHealthRecords.sleep(sleepSession(listOf(0xf2 to 30 * 60, 0xf1 to 30 * 60))) + .first() as RingDecodedEvent.SleepTimeline + assertEquals(60, event.stages.size) + } + + /** `af fa` record with the vendor header's `startTime` (+4) and `endTime` (+8) filled in. */ + private fun sleepRecord( + startSeconds: Int, + endSeconds: Int, + segments: List>, + ): ByteArray { + val recordLength = 20 + segments.size * 8 + val out = mutableListOf() + fun u16(value: Int) { + out.add((value and 0xFF).toByte()) + out.add(((value shr 8) and 0xFF).toByte()) + } + fun u32(value: Int) { + u16(value and 0xFFFF) + u16((value shr 16) and 0xFFFF) + } + out.add(0xaf.toByte()) + out.add(0xfa.toByte()) + u16(recordLength) + u32(startSeconds) + u32(endSeconds) + repeat(8) { out.add(0) } + var at = startSeconds + for ((tag, seconds) in segments) { + out.add(tag.toByte()) + u32(at) + out.add((seconds and 0xFF).toByte()) + out.add(((seconds shr 8) and 0xFF).toByte()) + out.add(((seconds shr 16) and 0xFF).toByte()) + at += seconds + } + return out.toByteArray() + } + + private fun sleepSession(segments: List>, baseStart: Int = 0x31def01c): ByteArray { val recordLength = 20 + segments.size * 8 val out = mutableListOf() out.add(0xaf.toByte()) @@ -278,7 +406,7 @@ class YCBTHealthRecordsTest { out.add((recordLength shr 8).toByte()) repeat(16) { out.add(0) } for ((index, segment) in segments.withIndex()) { - val start = 0x31def01c + index * 3600 + val start = baseStart + index * 3600 out.add(segment.first.toByte()) out.add((start and 0xFF).toByte()) out.add(((start shr 8) and 0xFF).toByte()) diff --git a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt index e9f3c57..65384f8 100644 --- a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt +++ b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt @@ -114,6 +114,45 @@ class EventPersistenceIdentityTest { assertEquals(listOf("LIGHT", "DEEP", "LIGHT"), merged.map { it.stageRaw }) } + /** + * Issue #63: the ring split one night into two sessions three minutes apart (00:12–03:18 and + * 03:21–07:24). Once stored, SleepSegmentation has merged them into one row, so on the next + * sync pass the second record overlaps a row that also holds the first record's blocks. It + * must retire only its own stale copy — never the first session across the gap. + */ + @Test + fun `a complete record does not retire the other session of the same night`() { + val a = 1_725_408_720_000L // 00:12 + val b = a + (3 * 60 + 9) * 60_000L // 03:21 — three minutes after 03:18 + val existing = listOf( + block("a-1", a, 124, "LIGHT"), + block("a-2", a + 124 * 60_000L, 62, "DEEP"), // ends 03:18 + block("b-1", b, 119, "LIGHT"), + block("b-2", b + 119 * 60_000L, 124, "DEEP"), // ends 07:24 + ) + + val survivors = completeSessionSurvivors(existing, b, b + 243 * 60_000L) + + assertEquals(listOf("a-1", "a-2"), survivors.map { it.id }) + } + + /** The rule the old block wipe was there for: a shortened re-send of the *same* session must + * still retire its stale tail, which abuts the revised interval without a gap. */ + @Test + fun `a shortened complete record still retires its own stale tail and head`() { + val start = 1_725_408_720_000L + val existing = listOf( + block("head", start, 10, "AWAKE"), // revision now starts 10 min later + block("mid", start + 10 * 60_000L, 100, "LIGHT"), + block("tail", start + 110 * 60_000L, 20, "LIGHT"), // revision now ends 20 min earlier + block("nap", start + 131 * 60_000L, 30, "LIGHT"), // one-minute gap: a different session + ) + + val survivors = completeSessionSurvivors(existing, start + 10 * 60_000L, start + 110 * 60_000L) + + assertEquals(listOf("nap"), survivors.map { it.id }) + } + @Test fun `short nap cannot replace a longer night on the same waking day`() { val nightStart = 1_721_234_000_000L @@ -156,4 +195,38 @@ class EventPersistenceIdentityTest { durationMinutes = duration, stageRaw = stage, ) + + /** + * Issue #60: only a ring that logs its own spot measurements leaves a second copy to reconcile. + * A CRP or Colmi ring does not, and its all-day history sits on a five-minute grid — so a spot + * reading there must never be marked as awaiting a ring copy, or the next unrelated grid + * sample within the match window would delete a reading the user asked for. + */ + @Test + fun `only a spot reading from a ring that logs it awaits the ring's copy`() { + assertTrue(awaitsRingsCopy(spot = true, ringWillLogIt = true)) + assertFalse("a CRP/Colmi spot reading has no second copy coming", + awaitsRingsCopy(spot = true, ringWillLogIt = false)) + assertFalse("a streamed sample is not a spot reading", + awaitsRingsCopy(spot = false, ringWillLogIt = true)) + assertFalse(awaitsRingsCopy(spot = false, ringWillLogIt = false)) + } + + /** + * Issue #60, RC-2: the ring logs each spot reading into its own history, and a later sync + * imports it next to the row we stored for our settled value. The match rule that lets the + * ring's copy replace ours must reach a stamp at either end of a 35–63 s measurement and must + * not reach the ring's own all-day samples five minutes apart. + */ + @Test + fun `a history sample adopts only the spot reading it is the ring's copy of`() { + val ours = listOf(1_000_000L, 1_300_000L) // two spot readings, five minutes apart + // The ring stamps to the minute, so its copy may sit up to a measurement's length away. + assertEquals(listOf(1_000_000L), spotReadingsMatching(ours, 1_000_000L + 60_000)) + assertEquals(listOf(1_000_000L), spotReadingsMatching(ours, 1_000_000L - 60_000)) + // An all-day sample two and a half minutes from either is nobody's copy. + assertTrue(spotReadingsMatching(ours, 1_150_000L).isEmpty()) + // Nothing of ours: nothing to adopt, whatever the ring sends. + assertTrue(spotReadingsMatching(emptyList(), 1_000_000L).isEmpty()) + } } diff --git a/app/src/test/java/com/pulseloop/service/HRSampleWindowTest.kt b/app/src/test/java/com/pulseloop/service/HRSampleWindowTest.kt index 0b9b76b..d352112 100644 --- a/app/src/test/java/com/pulseloop/service/HRSampleWindowTest.kt +++ b/app/src/test/java/com/pulseloop/service/HRSampleWindowTest.kt @@ -2,6 +2,7 @@ package com.pulseloop.service 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 @@ -75,9 +76,168 @@ class HRSampleWindowTest { assertFalse("no samples collected yet during warm-up", f.window.contactLost()) f.advance(4_000); f.window.collect(70) // first real sample at t=6s f.advance(1_000) - assertFalse("within the 3s contact gap", f.window.contactLost()) - f.advance(3_500) - assertTrue("more than 3s since the last sample", f.window.contactLost()) + assertFalse("within the contact gap", f.window.contactLost()) + f.advance(8_500) + assertTrue("more than 8s since the last sample", f.window.contactLost()) + } + + @Test + fun `collect reports whether the sample was kept`() { + val f = Fixture() + assertFalse("no measurement running", f.window.collect(70)) + f.window.begin() + f.advance(1_000) + assertFalse("inside the warm-up echo", f.window.collect(70)) + f.advance(5_000) + assertTrue("past the warm-up", f.window.collect(70)) + } + + /** + * Issue #59, replayed from the reporter's instrumented capture of an `Ale-Hop2211` YCBT ring. + * The PPG spends ~26 s on a pre-converged plateau (47 47 47, 46 46 46) before stepping to the + * real rate (84 … 81), and the ring ends the measurement itself at ~35 s. The plateau is both + * the majority of the window and the most consistent thing in it, so the old whole-window + * median reported 46 — a number that was never this user's heart rate. + */ + @Test + fun `the pre-converged plateau does not outvote the converged tail`() { + val f = Fixture() + f.window.begin() + val capture = listOf( + 14_100L to 47, 15_100L to 47, 16_100L to 47, + 22_100L to 46, 23_100L to 46, 24_100L to 46, + 26_100L to 84, 27_100L to 84, 28_100L to 84, + 32_100L to 82, 33_100L to 81, 34_100L to 81, 35_100L to 81, + ) + for ((at, bpm) in capture) { + f.now = at + assertTrue("t+${at}ms is past the warm-up", f.window.collect(bpm)) + } + val settled = f.window.stableValue + assertNotNull("the capture is a successful measurement", settled) + assertTrue("settles on the converged rate, not the 46 bpm plateau: got $settled", settled!! >= 80) + } + + /** + * The counterpart to the test above: the bursty cadence that produced that capture — three + * samples about a second apart, then 4-6 s of silence — must not read as a slipped ring. + * At the old 3 s gap this aborted the leg at t+19 s, before the sensor had converged at all. + */ + @Test + fun `a bursty ring is not mistaken for lost contact`() { + val f = Fixture() + f.window.begin() + f.now = 14_100; f.window.collect(47) + f.now = 15_100; f.window.collect(47) + f.now = 16_100; f.window.collect(47) + f.now = 21_000 + assertFalse("still inside the ring's 4-6s burst gap", f.window.contactLost()) + f.now = 22_100; f.window.collect(46) + assertFalse(f.window.contactLost()) + } + + // MARK: - The ring's own choice (issue #59, RC-3 read-back) + + /** + * The three spot measurements the reporter captured with no stop command and then read back + * out of the ring's own memory before letting any app near it. The ring stored the **last** + * streamed sample all three times, and the run is still climbing when the ring stops — so this + * is a discriminating test, unlike SpO2 where the tail and the last sample coincide. + * + * The elided middle of capture 2 is written out as the groups the report named; what matters + * to the rule is the shape of the tail, which is quoted verbatim there. + */ + private val readBackCaptures = listOf( + Triple( + "capture 1 (08:14:28)", + listOf(47, 47, 47, 46, 46, 45, 45, 45, 46, 46, 47, 55, 65, 65), + 65, + ), + Triple( + "capture 2 (08:18:18)", + listOf(47, 47, 47, 44, 44, 44, 49, 49, 49, 51, 51, 53, 53, 54, 55, 57, 58, 58), + 58, + ), + Triple( + "capture 3 (08:21:44)", + listOf(47, 47, 47, 46, 46, 46, 47, 47, 55, 66, 72, 72), + 72, + ), + ) + + /** Replays a whole run past the warm-up at the ring's ~1 s burst cadence. */ + private fun Fixture.replay(stream: List) { + window.begin() + now = 14_000 + for (bpm in stream) { + now += 1_000 + window.collect(bpm) + } + } + + @Test + fun `a ring that ends its own measurement settles on the value it will log`() { + for ((name, stream, ringStored) in readBackCaptures) { + val f = Fixture() + f.replay(stream) + assertEquals( + "$name: the app must report what the ring stored", + ringStored, + f.window.settled(ringChoosesLastSample = true), + ) + } + } + + /** + * The point of the previous test: a tail-weighted rule lands *below* the ring's answer on a + * climbing run, which is how the app came to show 94 against the ring's 93. Kept as a test so + * that "just use the tail everywhere" reads as a deliberate regression rather than a tidy-up. + */ + @Test + fun `the tail rule disagrees with the ring on a climbing run`() { + for ((name, stream, ringStored) in readBackCaptures) { + val f = Fixture() + f.replay(stream) + val tail = f.window.settled(ringChoosesLastSample = false) + assertNotNull("$name: the tail rule still produces a reading", tail) + assertTrue( + "$name: tail $tail should sit below the ring's $ringStored", + tail!! < ringStored, + ) + } + } + + /** + * A ring with no completion signal keeps the tail rule, because nothing chose its last sample + * — the leg simply ran out of window. Its steady stream settles where it always did. + */ + @Test + fun `a ring with no completion signal still gets the consistency gate`() { + val f = Fixture() + f.replay(listOf(30, 200, 60, 150, 40, 190, 55, 170)) // scattered: no majority agrees + assertNull(f.window.settled(ringChoosesLastSample = false)) + } + + @Test + fun `a trailing dropout frame cannot become the reading`() { + val f = Fixture() + f.replay(listOf(47, 55, 65, 72, 0)) // ring drops out on the last frame + assertEquals(72, f.window.settled(ringChoosesLastSample = true)) + } + + @Test + fun `a run with no plausible sample is a failed measurement`() { + val f = Fixture() + f.replay(listOf(0, 0, 255, 0)) + assertNull(f.window.settled(ringChoosesLastSample = true)) + } + + @Test + fun `one plausible sample is enough when the ring chose it`() { + val f = Fixture() + f.replay(listOf(0, 88)) // below stableValue's 6-sample floor + assertEquals(88, f.window.settled(ringChoosesLastSample = true)) + assertNull("the tail rule still needs a run to judge", f.window.settled(ringChoosesLastSample = false)) } @Test diff --git a/app/src/test/java/com/pulseloop/service/SleepInsightsTest.kt b/app/src/test/java/com/pulseloop/service/SleepInsightsTest.kt index d314e9c..b93f0d8 100644 --- a/app/src/test/java/com/pulseloop/service/SleepInsightsTest.kt +++ b/app/src/test/java/com/pulseloop/service/SleepInsightsTest.kt @@ -101,6 +101,40 @@ class SleepInsightsTest { assertTrue("Score ${result.score} should be <= 100", result.score <= 100) } + /** + * Since issue #63 `totalMinutes` is time asleep, so a night's blocks always cover at least + * that many minutes. Judged against it, the "does this ring label awake at all" coverage + * heuristic would be true for every ring, and a ring that never labels awake would be handed + * the full awake sub-score for nothing. It is judged against the span instead. + */ + @Test + fun `a ring that never labels awake and covers only part of the night reports no awake share`() { + // 8 h in bed, 6 h of labelled sleep, no AWAKE blocks anywhere: 75% coverage of the span. + val s = session(360).copy(endAt = session(360).startAt + 480 * 60_000L) + val blocks = buildList { + repeat(300) { add(SleepStageBlockEntity(sessionId = s.id, startAt = 0, startMinute = 0, durationMinutes = 1, stageRaw = "LIGHT")) } + repeat(60) { add(SleepStageBlockEntity(sessionId = s.id, startAt = 0, startMinute = 0, durationMinutes = 1, stageRaw = "DEEP")) } + } + val result = SleepScore.calculate(s, blocks) + assertNull("no awake signal: coverage is 75% of the span, not 100% of time asleep", result.awakePct) + assertEquals("stage shares are of time asleep", 17, result.deepPct) // 60/360 + } + + @Test + fun `the awake share is of time in bed, and the stage shares of time asleep`() { + // 7 h in bed = 420 min span; 30 min awake; 390 min asleep. + val s = session(390).copy(endAt = session(390).startAt + 420 * 60_000L) + val blocks = buildList { + repeat(300) { add(SleepStageBlockEntity(sessionId = s.id, startAt = 0, startMinute = 0, durationMinutes = 1, stageRaw = "LIGHT")) } + repeat(90) { add(SleepStageBlockEntity(sessionId = s.id, startAt = 0, startMinute = 0, durationMinutes = 1, stageRaw = "DEEP")) } + repeat(30) { add(SleepStageBlockEntity(sessionId = s.id, startAt = 0, startMinute = 0, durationMinutes = 1, stageRaw = "AWAKE")) } + } + val result = SleepScore.calculate(s, blocks) + assertEquals(7, result.awakePct) // 30/420 + assertEquals(23, result.deepPct) // 90/390 + assertEquals(77, result.lightPct) // 300/390 + } + @Test fun testQualityLabelThresholds() { assertEquals(SleepQualityLabel.EXCELLENT, SleepScore.qualityLabel(85)) diff --git a/app/src/test/java/com/pulseloop/service/SleepSegmentationTest.kt b/app/src/test/java/com/pulseloop/service/SleepSegmentationTest.kt index 99e4738..0c8a1ec 100644 --- a/app/src/test/java/com/pulseloop/service/SleepSegmentationTest.kt +++ b/app/src/test/java/com/pulseloop/service/SleepSegmentationTest.kt @@ -23,6 +23,63 @@ class SleepSegmentationTest { startMinute = startMin, durationMinutes = durMin, stageRaw = stage.name, ) + // ── asleepMinutes / spanMinutes (issue #63) ────────────────────────── + + /** + * The reporter's night: the ring split it into two records nine minutes apart, 23:51–05:02 and + * 05:11–08:02, and the app reported 8 h 10 — the span, including the gap and the awake time + * inside each record. The vendor app reports `deepSleepTotal + lightSleepTotal + remTotal` + * instead (`SleepActivity:695`) and shows the span only as a range. + */ + @Test + fun `a duration is time asleep, and the span is a separate number`() { + val base = 0L + val blocks = listOf( + block(base, 0, 240, SleepStage.LIGHT), // 23:51 record: 4h asleep + block(base, 240, 71, SleepStage.DEEP), // … 1h11 more, ends at min 311 + block(base, 320, 100, SleepStage.LIGHT), // 05:11 record after a 9-minute gap + block(base, 420, 40, SleepStage.REM), + ) + assertEquals("deep + light + rem, gap excluded", 451, asleepMinutes(blocks)) + } + + @Test + fun `awake blocks are excluded from the duration but not from the span`() { + val base = 0L + val blocks = listOf( + block(base, 0, 120, SleepStage.LIGHT), + block(base, 120, 30, SleepStage.AWAKE), + block(base, 150, 90, SleepStage.DEEP), + ) + assertEquals(210, asleepMinutes(blocks)) + val row = SleepSessionEntity( + id = "s", date = base, startAt = base, endAt = base + 240 * minute, + totalMinutes = asleepMinutes(blocks), + ) + assertEquals("the span still covers the awake stretch", 240, row.spanMinutes) + } + + /** + * UNKNOWN is the `else` branch of every sleep decoder in this app — a stage byte we did not + * recognise inside a record the ring called sleep. Those minutes were slept, so summing three + * named stages instead of excluding AWAKE would silently drop them. + */ + @Test + fun `an unrecognised stage still counts as sleep`() { + val base = 0L + val blocks = listOf( + block(base, 0, 60, SleepStage.UNKNOWN), + block(base, 60, 60, SleepStage.LIGHT), + block(base, 120, 20, SleepStage.AWAKE), + ) + assertEquals(120, asleepMinutes(blocks)) + } + + @Test + fun `a night with no blocks has no duration`() { + assertEquals(0, asleepMinutes(emptyList())) + } + // ── segment ────────────────────────────────────────────────────────── @Test diff --git a/app/src/test/java/com/pulseloop/service/Spo2SampleWindowTest.kt b/app/src/test/java/com/pulseloop/service/Spo2SampleWindowTest.kt new file mode 100644 index 0000000..c9a290f --- /dev/null +++ b/app/src/test/java/com/pulseloop/service/Spo2SampleWindowTest.kt @@ -0,0 +1,99 @@ +package com.pulseloop.service + +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 SpO₂ settle (issue #59, RC-2 feedback): the reading is the last plausible sample, because + * that is what the ring itself logs — see [Spo2SampleWindow] for the three sources behind that. + * The five captures below are @Albabit's, each lined up against the value read back out of the + * ring's own history for that run. + */ +class Spo2SampleWindowTest { + + @Test + fun `nothing collected settles to nothing`() { + val w = Spo2SampleWindow() + w.begin() + assertNull(w.settled) + assertFalse(w.receivedReading) + } + + @Test + fun `a single sample is its own reading`() { + val w = Spo2SampleWindow() + w.begin() + assertTrue(w.collect(97)) + assertTrue(w.receivedReading) + assertEquals(97, w.settled) + } + + /** Captures 1–3: one tight burst. Ring stored 98. */ + @Test + fun `a tight burst settles on its last sample`() { + val w = Spo2SampleWindow() + w.begin() + listOf(99, 98, 98, 98).forEach { w.collect(it) } + assertEquals(98, w.settled) + } + + /** + * Capture 4: the late burst collapses from 98 to 86–87. The ring stored **87** — that run was + * simply bad, and the reading must say so rather than rescue it from the earlier burst. + */ + @Test + fun `a collapsing run settles on the collapse, as the ring does`() { + val w = Spo2SampleWindow() + w.begin() + listOf(98, 98, 98, 86, 86, 86, 87, 87, 87).forEach { w.collect(it) } + assertEquals(87, w.settled) + } + + /** Capture 5: the one run where the median (98) disagreed with what the ring stored (99). */ + @Test + fun `the run that split median from ring settles with the ring`() { + val w = Spo2SampleWindow() + w.begin() + listOf(97, 97, 97, 99, 99, 98, 98, 98, 99).forEach { w.collect(it) } + assertEquals(99, w.settled) + } + + /** The original RC-1 capture: rises to 99 then declines to 94 before `04 0e`. Ring behaviour + * says the last sample is the reading; the old first-sample leg would have said 96. */ + @Test + fun `the RC-1 capture settles on its tail, not its first sample`() { + val w = Spo2SampleWindow() + w.begin() + listOf(96, 96, 96, 99, 98, 98, 98, 96, 96, 95, 94, 94).forEach { w.collect(it) } + assertEquals(94, w.settled) + } + + /** The vendor's plausibility band (70..100): a zero or a dropout is neither the reading nor + * evidence that the ring has read anything yet. */ + @Test + fun `implausible samples are dropped and do not count as a reading`() { + val w = Spo2SampleWindow() + w.begin() + assertFalse(w.collect(0)) + assertFalse(w.collect(69)) + assertFalse(w.collect(101)) + assertFalse(w.receivedReading) + assertNull(w.settled) + assertTrue(w.collect(97)) + assertFalse(w.collect(0)) + assertEquals(97, w.settled) + } + + @Test + fun `begin clears a prior run`() { + val w = Spo2SampleWindow() + w.begin() + listOf(90, 91, 92).forEach { w.collect(it) } + w.begin() + assertFalse(w.receivedReading) + assertNull(w.settled) + } +} diff --git a/app/src/test/java/com/pulseloop/service/SpotMeasurementGateTest.kt b/app/src/test/java/com/pulseloop/service/SpotMeasurementGateTest.kt index f324a71..e92c0ff 100644 --- a/app/src/test/java/com/pulseloop/service/SpotMeasurementGateTest.kt +++ b/app/src/test/java/com/pulseloop/service/SpotMeasurementGateTest.kt @@ -3,6 +3,7 @@ package com.pulseloop.service import com.pulseloop.ring.YCBTMeasurementMode import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test @@ -119,4 +120,50 @@ class SpotMeasurementGateTest { gate.noteRejected(YCBTMeasurementMode.HEART_RATE) assertTrue("HR was still mid-poll when the ring refused it", gate.isRejected(hr)) } + + /** Issue #59: the ring ends the measurement itself and says whether it worked. */ + @Test + fun `a completion of the measurement in flight reports the ring's verdict`() { + val gate = SpotMeasurementGate() + val hr = gate.begin(YCBTMeasurementMode.HEART_RATE) + assertNull("nothing said yet", gate.completedSuccessfully(hr)) + + gate.noteCompleted(YCBTMeasurementMode.HEART_RATE, success = true) + + assertEquals(true, gate.completedSuccessfully(hr)) + assertFalse("a completion is not a refusal", gate.isRejected(hr)) + } + + @Test + fun `a completion of a different mode cannot end the one in flight`() { + val gate = SpotMeasurementGate() + val hr = gate.begin(YCBTMeasurementMode.HEART_RATE) + + gate.noteCompleted(YCBTMeasurementMode.SPO2, success = true) + + assertNull("only the measurement the ring named may be ended", gate.completedSuccessfully(hr)) + } + + /** A refusal is a start-time verdict and must survive a stray completion pushed after it — + * otherwise a refused measurement would look like one worth settling samples from. */ + @Test + fun `a refusal is not overwritten by a later completion`() { + val gate = SpotMeasurementGate() + val hrv = gate.begin(YCBTMeasurementMode.HRV) + + gate.noteRejected(YCBTMeasurementMode.HRV) + gate.noteCompleted(YCBTMeasurementMode.HRV, success = true) + + assertTrue(gate.isRejected(hrv)) + assertNull(gate.completedSuccessfully(hrv)) + } + + @Test + fun `a completion for a mode nothing is running is ignored`() { + val gate = SpotMeasurementGate() + gate.noteCompleted(YCBTMeasurementMode.HEART_RATE, success = true) + + val hr = gate.begin(YCBTMeasurementMode.HEART_RATE) + assertNull("a late completion must not end the next measurement", gate.completedSuccessfully(hr)) + } }