diff --git a/AGENTS.md b/AGENTS.md index 37d0db50..5bb0a635 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,327 @@ 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 spot HR measurement is refused while a workout is running.** The live-sample gate is one switch +per kind, so whichever of the two closed it decides whether the other's samples are stored, and the +leg sampled that decision once at the start. Running both meant either the workout's samples were +dropped for the length of the leg, or — if the workout started inside it — the leg's converging +samples were stored as workout rows and its settled value never published. A workout starting mid-leg +now aborts the leg, and the gate is reopened unconditionally in the `finally` because the leg is what +closed it; leaving it closed would silently drop that workout's samples for the rest of the session. +The workout screen is already showing live bpm, so refusing costs nothing. No unit test: +`RingSyncCoordinator` needs a BLE client and has no harness. + +## The terminal block is the authority on a history transfer, not the header (issue #69) + +`YCBTHistoryTransfer.handleTerminal` used to require the terminal block's packet count to equal the +one the header declared. **The ring contradicts its own estimate one frame later, so that check threw +away whole record types.** On the reporter's `Ale-Hop2211` (COLMI_SMART_HEALTH, firmware 2.04) the +`05 09` header declared **5** packets for 840 bytes and the ring then sent **6** — it packs whole +20-byte records into each frame (7 × 20 = 140) instead of filling it, so the header's +`ceil(bytes / frame)` estimate is one short. Bytes and CRC were correct on every transfer; we +nacked, retried once, and skipped the type. + +What that cost: the composite `05 18` record is the **only** source of SpO₂ history on this family — +the dedicated `05 1a` query was sent ten times in one session and never answered once — so hourly +SpO₂ vanished while HR survived from `05 15`. Respiratory rate, HRV, BP, temperature and blood sugar +ride in the same record. It only bites above one packet, which is why the record imported while the +day was young and stopped once it grew, and why it looked like a regression introduced by a release +that had touched nothing nearby. + +The vendor doesn't check the count either: at `Sync_Block_Verify` (128) `DataUnpack` reads the two +count bytes into locals it never compares, sizes its buffer from the **terminal's** length, and +accepts on `crc16_compute(...) == crc` alone. So: size the buffer from the header if you like, but +validate against the terminal's byte count and the CRC. The CRC is the integrity check; a packet +count is an estimate, and this firmware proves it can be wrong. + +**The general lesson, because this shape recurs here.** A silent "reject and skip" on a +self-consistency check is much harder to see than a decode bug: the frames arrive, the log shows +them as `unknown` (which is what *every* accumulating history frame looks like — 427 sleep frames in +the same export say `unknown` and sleep imports fine), and nothing fails anywhere. When a whole +metric is missing but its neighbours are not, check what the transfer did with the block before +suspecting the parser. + +## 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. + +**A zero-length segment must not shadow a real one.** The vendor de-duplicates segments on +`sleepStartTime` (`DataUnpack` case 4 keeps the first it sees) and the ring emits zero-length +segments, so a zero-length segment sharing a start with a real one used to take its place and drop +it. All four duplicate starts in the reporter's thirteen-record night dump are exactly that — a +zero-length LIGHT ahead of a real 76–105 s segment — and since `placeStages` reads an unclaimed +minute as wake, the dropped minutes surfaced as wake instead. The vendor has the same de-duplication +and does not care, because its headline comes from the header's own totals rather than from the +segment array; ours is counted off the timeline, so it does. Zero-length segments are skipped before +the de-duplication now. + +**The record's header declares its own time asleep, and it agrees with the timeline.** When +`deepSleepCount` (+12) reads `0xffff`, the three following `u16`s are **seconds**, in the order +`rapidEyeMovementTotal` (+14), `deepSleepTotal` (+16), `lightSleepTotal` (+18) — REM first, not +deep — and `wakeDuration` is the summed length of the `0xf4` segments (`DataUnpack.java:1736-1805`). +Otherwise +14 is `lightSleepCount`, deep/light are minutes×60, and REM is absent. On the reporter's +dump `deepSleepCount` is `0xffff` on 13/13 records and deep+light+rem agrees with the timeline we +count to within a minute on 12 of 13. So the counted timeline is sound; if a headline ever needs to +be independent of placement and rounding, the declared totals are there. Note the record length at ++2 is **bytes, not minutes** — it reads plausibly as a minute count (244, 164, 268, 140) and was +misread that way for several rounds of this issue. + +**A complete record retires only what its interval overlaps — abutting blocks are left alone.** The +rule used to grow its run across any block meeting it end-to-start, so that a shortened re-send could +retire its own stale tail. But a block carries no record identity — `sessionId` is the *merged* row, +shared by every record of the night — so "my stale tail" and "the neighbouring record" are the same +shape from the same side, and two records meeting with no gap read as one run: the second wiped the +first. The reporter's ring closes one record and opens the next 33 seconds later, so whether the two +round to the same minute is a coin toss, and losing it costs a whole session. The trade is a stale +tail surviving a genuine shortening, which is minutes rather than hours and rarer than it was now +that a re-send reproduces the record's declared bounds instead of a drifted end. Carrying the +originating record's start on each block would allow both, and is the fix if the tail ever bites. + +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. + +**A deleted `spot` row needs a range tombstone, because its id is a UUID.** The split above — +tombstone the regenerable ids, delete the UUIDs outright — is right for a `"live"` row and wrong for +a `"spot"` one: a `"spot"` row exists *precisely because* the ring logged that measurement itself and +will hand it back under a `history:` id on the next sync. Tombstoning by id alone let the ring's copy +walk straight in, so the reading came back and had to be deleted twice. The ring stamps its log to +the minute rather than to our settled instant, so the tombstone cannot name the id it must suppress: +`MeasurementDeletionDao.record` writes a `spot::` row instead and `isSpotDeleted` matches +it the way `adoptRingsCopy` matches the pair it reconciles — same kind, within ±90 s. A measurement +retaken inside that window inherits the suppression: it is stored and displayed (it is our own row, +not a history write) but never adopts the ring's copy. That is the same ±90 s ambiguity +`adoptRingsCopy` already carries, and it fails towards keeping a reading the user asked for. + +**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. + +**The header length must come from the packet's own family, and a half-assembled frame has no header +at all.** Two further shapes of the same failure, fixed together: + +- Masking used to use the **connected** ring's family for every stored packet, so a report exported + after switching rings masked the old family's frames with the wrong header length — and CRP's is the + longest, so a Colmi frame masked as CRP kept five bytes of samples. `raw_packets.deviceTypeRaw` + (v25) records the family at capture time (`PulseEvent.RawPacket.deviceType`), and a row from before + it existed masks from byte 1: less useful, never wrong in the direction that leaks. +- A CRP reply spanning several notifications only decodes when its last chunk lands, so every chunk + before that reached the log as `unknown` — which is deliberately exported **whole**, because control + and pairing frames decode to nothing and are what most connection reports are taken for. Half an + all-day health reply left in clear that way. `RingDecodedEvent.FramePending` names a chunk instead, + and the two cases mask differently: `frame_start` keeps its family's header (it is there, and it is + what identifies the reply), `frame_chunk` keeps byte 0 and nothing else, because the middle of a + frame is payload from byte 0 on. diff --git a/app/src/main/java/com/pulseloop/PulseLoopApplication.kt b/app/src/main/java/com/pulseloop/PulseLoopApplication.kt index 65d91ae5..15e966d8 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 dd9ad8eb..bb8974c7 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 f8fac52c..0f19d07b 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 0987227e..8872c099 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, @@ -188,6 +198,7 @@ data class PulseArchive( val id: String, val timestamp: Long, val directionRaw: String, val commandId: Int, val hexPayload: String, val decodedKind: String? = null, val decodedJSON: String? = null, val confidenceRaw: String = "unknown", + val deviceTypeRaw: String? = null, val createdAt: Long, ) diff --git a/app/src/main/java/com/pulseloop/data/DataArchiveService.kt b/app/src/main/java/com/pulseloop/data/DataArchiveService.kt index 156b8167..38f7b1a9 100644 --- a/app/src/main/java/com/pulseloop/data/DataArchiveService.kt +++ b/app/src/main/java/com/pulseloop/data/DataArchiveService.kt @@ -242,6 +242,7 @@ object DataArchiveService { directionRaw = c.str("directionRaw"), commandId = c.int_("commandId"), hexPayload = c.str("hexPayload"), decodedKind = c.strOrNull("decodedKind"), decodedJSON = c.strOrNull("decodedJSON"), confidenceRaw = c.str("confidenceRaw"), + deviceTypeRaw = c.strOrNull("deviceTypeRaw"), createdAt = c.long("createdAt"), ) }, @@ -310,6 +311,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 +376,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 +467,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, @@ -530,7 +561,8 @@ object DataArchiveService { id = rp.id, timestamp = rp.timestamp, directionRaw = rp.directionRaw, commandId = rp.commandId, hexPayload = rp.hexPayload, decodedKind = rp.decodedKind, decodedJSON = rp.decodedJSON, - confidenceRaw = rp.confidenceRaw, createdAt = rp.createdAt, + confidenceRaw = rp.confidenceRaw, deviceTypeRaw = rp.deviceTypeRaw, + createdAt = rp.createdAt, )) } for (du in archive.derivedUpdates) { diff --git a/app/src/main/java/com/pulseloop/data/DataRepairs.kt b/app/src/main/java/com/pulseloop/data/DataRepairs.kt index 5601ea47..6fd7f1a8 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 5a301095..0aa18556 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 00000000..71453b53 --- /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 3184487a..98c0c060 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 = 25, 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,42 @@ 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() + ) + } + } + + /** + * v24 -> v25: `raw_packets.deviceTypeRaw`, so the diagnostics report masks each captured + * frame against the family that sent it rather than against whichever ring is connected + * when the report is exported (a Colmi frame masked with CRP's six-byte header exports five + * bytes of samples). Nullable with no backfill on purpose — the family of an already + * captured packet is not recoverable, and the redactor masks a row with no family from + * byte 1, which is the conservative reading. `raw_packets` is a 1000-row debug ring buffer, + * so existing rows age out within a session or two of use. + */ + private val MIGRATION_24_25 = object : Migration(24, 25) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("ALTER TABLE `raw_packets` ADD COLUMN `deviceTypeRaw` TEXT") + } + } + private fun adoptStableMeasurementIdentities(db: SupportSQLiteDatabase) { db.execSQL("DROP INDEX IF EXISTS `index_measurements_kindRaw_timestamp_sourceRaw`") db.execSQL( @@ -531,6 +570,8 @@ abstract class PulseLoopDatabase : RoomDatabase() { MIGRATION_20_21, MIGRATION_21_22, MIGRATION_22_23, + MIGRATION_23_24, + MIGRATION_24_25, ) // 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 2e058048..beb33e09 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,78 @@ 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) + + /** + * Is the ring's own copy of a deleted spot reading due back? A `spot` row's id is a fresh UUID, + * so it cannot be tombstoned by id — but the reading it holds *is* regenerable, under the + * `history:` id the ring's copy arrives with up to + * [EventPersistenceSubscriber.SPOT_MATCH_MS][com.pulseloop.service.EventPersistenceSubscriber.Companion.SPOT_MATCH_MS] + * away. Matched the way `adoptRingsCopy` matches the pair it reconciles: same kind, within the + * window either side. + */ + @Query(""" + SELECT EXISTS(SELECT 1 FROM measurement_deletions + WHERE kindRaw = :kind AND measurementId LIKE 'spot:%' AND timestamp BETWEEN :from AND :to) + """) + suspend fun isSpotDeleted(kind: String, from: Long, to: Long): Boolean + + /** + * 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. + * + * **A `spot` row is the exception, and it needs a range.** It carries a UUID like any live row, + * but it exists precisely because the ring logs that measurement itself and will hand it back + * under a `history:` id on the next sync ([isSpotDeleted]) — so tombstoning by id alone let the + * ring's copy walk straight back in, and the reading had to be deleted twice. The ring stamps + * its log to the minute rather than to our settled instant, so the tombstone cannot name the id + * it has to suppress; it records the kind and our timestamp instead, under a deterministic + * `spot:` key so re-deleting the same reading replaces the row rather than growing the table. + * + * A measurement retaken inside that window inherits the suppression: the retake itself is + * stored and displayed (it is our own row, not a history write), it simply never adopts the + * ring's copy and stays marked `spot`. That is the same ±90 s ambiguity `adoptRingsCopy` + * already carries, and it fails in the direction that keeps a reading the user asked for. + */ + suspend fun record(measurements: List) { + val regenerable = measurements.mapNotNull { row -> + when { + row.id.startsWith(HISTORY_ID_PREFIX) -> + MeasurementDeletionEntity(measurementId = row.id, kindRaw = row.kindRaw, timestamp = row.timestamp) + row.sourceRaw == SPOT_SOURCE -> + MeasurementDeletionEntity( + measurementId = "$SPOT_ID_PREFIX${row.kindRaw}:${row.timestamp}", + kindRaw = row.kindRaw, + timestamp = row.timestamp, + ) + else -> null + } + } + 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:" + /** Key prefix for a range tombstone standing in for a deleted `spot` row — see [record]. + * Distinct from [HISTORY_ID_PREFIX] so `isDeleted`'s id lookup can never match one. */ + const val SPOT_ID_PREFIX = "spot:" + /** `EventPersistenceSubscriber.SOURCE_SPOT`, duplicated to keep this DAO off the service + * layer. Asserted equal in `MeasurementDeletionTest`. */ + const val SPOT_SOURCE = "spot" + } +} 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 2290f532..6fc8b543 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/data/entity/SleepCoachEntities.kt b/app/src/main/java/com/pulseloop/data/entity/SleepCoachEntities.kt index 278aacc6..60ba4270 100644 --- a/app/src/main/java/com/pulseloop/data/entity/SleepCoachEntities.kt +++ b/app/src/main/java/com/pulseloop/data/entity/SleepCoachEntities.kt @@ -136,6 +136,10 @@ data class RawPacketEntity( val decodedKind: String? = null, val decodedJSON: String? = null, val confidenceRaw: String = "unknown", + /** The protocol family that produced this frame, for the diagnostics report's masking — see + * [com.pulseloop.ring.PulseEvent.RawPacket.deviceType]. Null on rows captured before it was + * recorded, which the redactor masks conservatively. */ + val deviceTypeRaw: String? = null, val createdAt: 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 1e1ee5be..4fb26c4e 100644 --- a/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsExporter.kt +++ b/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsExporter.kt @@ -88,7 +88,14 @@ 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) + // The family that captured the packet, not the one connected now: a report + // exported after switching rings would otherwise mask each frame with the + // wrong header length, and the family with the longest header (CRP, 6 + // bytes) would keep five bytes of another family's samples. A row from + // before the family was recorded has none, and masks from byte 1. + put("hex", if (mask) { + DiagnosticsRedactor.maskPacketHex(pkt.hexPayload, kind, pkt.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 5fa48f51..f6693aed 100644 --- a/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsRedactor.kt +++ b/app/src/main/java/com/pulseloop/diagnostics/DiagnosticsRedactor.kt @@ -14,22 +14,77 @@ package com.pulseloop.diagnostics * carry no vitals and are kept whole — that's the data most connection/pairing bugs need. */ object DiagnosticsRedactor { + /** `RingDecodedEvent.FramePending.kind` for the chunk that opens a frame — it carries the + * routing header, so it masks like any other health frame of its family. */ + const val FRAME_START = "frame_start" + /** `RingDecodedEvent.FramePending.kind` for a chunk in the middle of a frame: no header, so + * nothing beyond byte 0 may survive. Both are asserted against the decoder in + * `DiagnosticsRedactorTest`. */ + const val FRAME_CONTINUATION = "frame_chunk" + /** Decoded kinds whose BLE payload carries a physiological/health value → mask payload. */ 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", + // Half of a frame still being reassembled (RingDecodedEvent.FramePending). Masked because + // what it will decode to is not yet known, and a chunk of a multi-frame health reply holds + // samples in the same shape as the assembled one. `unknown` stays unmasked deliberately — + // control and pairing frames are what most connection reports are taken for — so a chunk + // has to be named something other than that, or it leaves by the wrong door. + FRAME_START, FRAME_CONTINUATION, ) 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 survive masking for [kind] on [deviceType]. + * + * A continuation chunk is the exception to the header rule: it is the *middle* of a frame, so it + * has no routing header at all and byte 0 onwards is payload. Keeping its family's header length + * would keep up to six bytes of samples, which is the opposite of the intent. + */ + private fun keepBytes(kind: String, deviceType: String): Int = + if (kind == FRAME_CONTINUATION) 1 else headerBytes(deviceType) + + /** + * How many leading bytes of a frame are routing, not measurement, per protocol family. + * + * **An unrecognised family keeps one byte, and that matters for more than tidiness.** The + * header length has to come from the family that *sent the packet*, not from whichever ring + * happens to be connected when the report is exported: masking a Colmi frame with CRP's + * six-byte header would export five bytes of someone's samples. Packets captured before the + * family was recorded alongside them have no answer here, so they fall to this branch and are + * masked from byte 1 — less useful, never wrong in the direction that leaks. + * + * 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. + */ + 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): String { + 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 = keepBytes(kind, 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 76c0fb1f..95c2e417 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/CRPDriver.kt b/app/src/main/java/com/pulseloop/ring/CRPDriver.kt index 03ab8946..662f2b82 100644 --- a/app/src/main/java/com/pulseloop/ring/CRPDriver.kt +++ b/app/src/main/java/com/pulseloop/ring/CRPDriver.kt @@ -56,7 +56,12 @@ class CRPDriver(private val writer: RingCommandWriter?) : WearableDriver { // Framed command replies (fdd3) reassemble across notifications; everything else is a // self-contained push routed by source characteristic inside CRPDecoder. if (from.lowercase().contains("fdd3")) { - val frame = assembler.append(data) ?: return emptyList() + // A mid-frame chunk is reported rather than dropped, so the diagnostics log can mask it: + // it may hold half an all-day health reply, and an unnamed chunk exports unmasked. + val frame = assembler.append(data) + ?: return listOf( + RingDecodedEvent.FramePending(data, startsFrame = CRPProtocol.isFrameStart(data)) + ) return CRPDecoder.decode(frame, from) } return CRPDecoder.decode(data, from) diff --git a/app/src/main/java/com/pulseloop/ring/CRPProtocol.kt b/app/src/main/java/com/pulseloop/ring/CRPProtocol.kt index 5b1fb9aa..d14def68 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 056d6a5f..5ae45759 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 e23a4ba6..4782d38a 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 276c951b..d5e30901 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 7fcd9768..dc8b7ec8 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 af55537e..4b7866dd 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 254dd9ad..608b01a5 100644 --- a/app/src/main/java/com/pulseloop/ring/PulseEventBus.kt +++ b/app/src/main/java/com/pulseloop/ring/PulseEventBus.kt @@ -33,24 +33,69 @@ sealed class PulseEvent { /** Emitted when the user forgets the ring, so persistence clears the stored model identity. */ data object DeviceForgotten : PulseEvent() data class BatteryLevel(val percent: Int) : PulseEvent() - data class RawPacket(val direction: PacketDirection, val data: ByteArray, val decoded: RingDecodedEvent) : PulseEvent() { + /** + * One BLE frame as it went over the wire, for the debug log and the diagnostics report. + * + * [deviceType] is the family that sent or received it, captured here rather than looked up at + * export time: the report masks a health frame by keeping its routing header, how long that + * header is depends on the family, and the ring connected when a report is exported is not + * necessarily the one that produced the packets in it. + */ + data class RawPacket( + val direction: PacketDirection, + val data: ByteArray, + val decoded: RingDecodedEvent, + val deviceType: RingDeviceType? = null, + ) : PulseEvent() { override fun equals(other: Any?): Boolean { if (this === other) return true if (other !is RawPacket) return false - return direction == other.direction && data.contentEquals(other.data) && decoded == other.decoded + return direction == other.direction && data.contentEquals(other.data) && + decoded == other.decoded && deviceType == other.deviceType } - override fun hashCode(): Int = 31 * (31 * direction.hashCode() + data.contentHashCode()) + decoded.hashCode() + override fun hashCode(): Int = + 31 * (31 * (31 * direction.hashCode() + data.contentHashCode()) + decoded.hashCode()) + + (deviceType?.hashCode() ?: 0) } 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/RingBLEClient.kt b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt index bae70186..101b0af1 100644 --- a/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt +++ b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt @@ -1057,7 +1057,8 @@ class RingBLEClient( if (op.attempts == 0) { PulseEventBus.publishBlocking( PulseEvent.RawPacket(PacketDirection.OUTGOING, op.data, - RingDecodedEvent.CommandAck(commandId = if (op.data.isNotEmpty()) op.data[0].toUByte() else 0u)) + RingDecodedEvent.CommandAck(commandId = if (op.data.isNotEmpty()) op.data[0].toUByte() else 0u), + deviceType = activeCoordinator?.deviceType) ) } if (op.attempts == 0) { @@ -1586,7 +1587,8 @@ class RingBLEClient( raw = value, ) PulseEventBus.publishBlocking( - PulseEvent.RawPacket(PacketDirection.INCOMING, value, diagnostic) + PulseEvent.RawPacket(PacketDirection.INCOMING, value, diagnostic, + deviceType = activeCoordinator?.deviceType) ) for (decoded in decodedEvents) { if (!acceptsCallback(callbackForgetGeneration)) break diff --git a/app/src/main/java/com/pulseloop/ring/RingDecodedEvent.kt b/app/src/main/java/com/pulseloop/ring/RingDecodedEvent.kt index 497c2029..41840b72 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 @@ -95,6 +97,7 @@ sealed class RingDecodedEvent { is BloodPressureSample -> this._timestamp is BloodSugarSample -> this._timestamp is Unknown -> Instant.EPOCH + is FramePending -> Instant.EPOCH } data class ActivityUpdate( @@ -129,6 +132,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 +181,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, @@ -401,6 +441,37 @@ sealed class RingDecodedEvent { override val debugJSON = """{"mgdl":$mgdl}""" } + /** + * One notification of a logical frame that is still being reassembled — no decode yet, and + * possibly never one of its own (the frame decodes as a whole). + * + * It exists for the diagnostics report. A mid-frame chunk used to reach the raw-packet log as + * [Unknown], and `unknown` is deliberately *not* masked on export — control and pairing frames + * are the data most connection bugs need, and they decode to nothing. But an intermediate chunk + * of a multi-frame **health** reply carries samples in exactly the same shape, so those were + * exported whole: the same class of defect as issue #58's undecoded temperature frames, where a + * decode gap had quietly become a privacy gap. A chunk is now named for what it is, and the + * redactor masks it because it cannot know what the assembled frame will turn out to be. + */ + data class FramePending(val raw: ByteArray, val startsFrame: Boolean) : RingDecodedEvent() { + /** + * The two cases are named apart because they mask differently: only the opening chunk holds + * the frame's routing header, so a continuation chunk keeping six bytes "of header" would be + * keeping six bytes of samples. + */ + override val kind = if (startsFrame) "frame_start" else "frame_chunk" + override val confidence = DecodeConfidence.UNKNOWN + override val debugJSON = "{}" + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is FramePending) return false + return raw.contentEquals(other.raw) && startsFrame == other.startsFrame + } + + override fun hashCode(): Int = 31 * raw.contentHashCode() + startsFrame.hashCode() + } + data class Unknown( val commandId: UByte, val raw: ByteArray diff --git a/app/src/main/java/com/pulseloop/ring/RingEventBridge.kt b/app/src/main/java/com/pulseloop/ring/RingEventBridge.kt index 0ecf4895..42b8c5a3 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 @@ -139,6 +145,9 @@ object RingEventBridge { if (decoded.mgdl in bloodSugarRange) listOf(PulseEvent.BloodSugarSample(decoded.mgdl, decoded._timestamp)) else emptyList() } + + // Half a frame carries nothing to act on; it exists only so the raw-packet log can mask it. + is RingDecodedEvent.FramePending -> emptyList() } private fun isPlausibleHistoryMeasurement(kind: MeasurementKind, value: Double): Boolean { diff --git a/app/src/main/java/com/pulseloop/ring/WearableDriver.kt b/app/src/main/java/com/pulseloop/ring/WearableDriver.kt index 6bd9f86c..daf81ae3 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 e373ace4..8c9276da 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 28943cbf..7e3789cf 100644 --- a/app/src/main/java/com/pulseloop/ring/YCBTHealthRecords.kt +++ b/app/src/main/java/com/pulseloop/ring/YCBTHealthRecords.kt @@ -164,37 +164,62 @@ 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) - 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) } + // A zero-length segment claims no minute of the timeline, so it must not take the + // start time's place in the de-duplication and shadow a real segment sharing it. + // The ring emits both: in the night dump on issue #63, all four duplicate starts + // across thirteen records are a zero-length LIGHT ahead of a real 76–105 s + // segment, and dropping the real one leaves those minutes unclaimed — which + // `placeStages` then reads as wake. The vendor drops them too + // (`DataUnpack` case 4 keeps the first `sleepStartTime` it sees) and gets away + // with it because its headline comes from the header's own totals rather than + // from the segment array; ours is counted off the timeline, so it cannot. + if (segmentSeconds <= 0) continue + // The vendor de-duplicates on the segment's start time (`sleepStartTime`). + if (!seenStarts.add(segmentStart)) continue + segments.add(SleepSegment(stage, segmentStart, segmentSeconds)) } - 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 +230,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/YCBTHistoryTransfer.kt b/app/src/main/java/com/pulseloop/ring/YCBTHistoryTransfer.kt index 2af2ea99..ead919f4 100644 --- a/app/src/main/java/com/pulseloop/ring/YCBTHistoryTransfer.kt +++ b/app/src/main/java/com/pulseloop/ring/YCBTHistoryTransfer.kt @@ -33,8 +33,6 @@ class YCBTHistoryTransfer( private var retriedCurrentType = false private var unsupported: MutableSet = mutableSetOf() private var bufferCap = DEFAULT_BUFFER_CAP - private var expectedPackets: Int? = null - private var expectedBytes: Int? = null private var watchdogJob: Job? = null private var typeDeadline: Long? = null @@ -80,8 +78,6 @@ class YCBTHistoryTransfer( private fun advance(): List { cancelWatchdog() buffer = ByteArray(0) - expectedPackets = null - expectedBytes = null bufferCap = DEFAULT_BUFFER_CAP retriedCurrentType = false if (queue.isEmpty()) { @@ -127,9 +123,9 @@ class YCBTHistoryTransfer( private fun handleHeader(type: YCBTHistoryType, payload: ByteArray): List { if (payload.size < YCBTHealth.HEADER_PAYLOAD_LENGTH) return advance() - expectedPackets = YCBTBytes.u16(payload, 2) + // The header's totals are an estimate used only to size the buffer; the terminal block is + // the authority on what actually arrived (issue #69). val totalBytes = YCBTBytes.u32(payload, 6) - expectedBytes = totalBytes buffer = ByteArray(0) bufferCap = totalBytes.coerceIn(0, MAX_BUFFER_CAP) state = State.RECEIVING @@ -146,11 +142,21 @@ class YCBTHistoryTransfer( private fun handleTerminal(type: YCBTHistoryType, payload: ByteArray): List { if (state == State.REQUEST_SENT && buffer.isEmpty()) return emptyList() if (payload.size < YCBTHealth.TERMINAL_PAYLOAD_LENGTH) return advance() - val packets = YCBTBytes.u16(payload, 0) val bytes = YCBTBytes.u16(payload, 2) - val terminalMatchesHeader = packets == expectedPackets && bytes == expectedBytes + // The terminal block's own byte count against what we assembled, and then the CRC. The + // packet count is deliberately NOT checked against the header's (issue #69). + // + // The vendor doesn't check it either: at `Sync_Block_Verify` (128) `DataUnpack` reads the + // two count bytes into locals it never compares, sizes its buffer from the *terminal's* + // length, and accepts on `crc16_compute(...) == crc` alone. Requiring the header's estimate + // to match cost a reporter every composite `05 18` record on their ring — the header + // declared 5 packets for 840 bytes while the ring then sent 6, because it packs whole + // 20-byte records per frame (7 × 20 = 140) rather than filling each one. Bytes and CRC were + // both correct every time; we rejected 42 records over an estimate the ring itself + // contradicts one frame later. It only bit above one packet, which is why the same record + // imported while the day was young and stopped once it grew. val terminalMatchesBuffer = bytes == buffer.size - if (!terminalMatchesHeader || !terminalMatchesBuffer) { + if (!terminalMatchesBuffer) { writer?.enqueue(YCBTHealthCommand.historyBlockAck(status = YCBTHealth.ACK_CRC_FAILURE)) return retryOrSkip(type) } diff --git a/app/src/main/java/com/pulseloop/ring/YCBTSyncEngine.kt b/app/src/main/java/com/pulseloop/ring/YCBTSyncEngine.kt index 1c7debf9..bdaeb1c7 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 1ba5e922..e9ab166b 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,92 @@ 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 + // The same reading deleted from the other side. A spot reading the user removed before the + // ring's copy of it arrived has no `history:` tombstone to find — its row was a UUID — so + // without this the ring's copy is written under an id nothing suppresses and the reading + // comes back, needing a second delete. Only kinds `adoptRingsCopy` reconciles can produce + // such a tombstone, so no other write pays for the lookup. + if (measurement.kindRaw in SPOT_RECONCILED_KINDS && + db.measurementDeletionDao().isSpotDeleted( + measurement.kindRaw, + measurement.timestamp - SPOT_MATCH_MS, + measurement.timestamp + SPOT_MATCH_MS, + ) + ) 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 +262,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 +305,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 +341,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 +370,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 -> { @@ -306,6 +414,7 @@ class EventPersistenceSubscriber( commandId = event.data.getOrNull(0)?.toInt()?.and(0xFF) ?: 0, hexPayload = event.data.joinToString("") { "%02x".format(it) }, decodedKind = event.decoded.kind, + deviceTypeRaw = event.deviceType?.name, )) } is PulseEvent.ActivitySyncReset -> {} @@ -454,14 +563,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 +653,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 +762,47 @@ 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 + /** The kinds [adoptRingsCopy] reconciles, and so the only ones a `spot:` range tombstone + * can exist for. Kept as the raw names the measurement rows carry. */ + val SPOT_RECONCILED_KINDS = setOf(MeasurementKind.HEART_RATE.name, MeasurementKind.SPO2.name) + /** 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 +872,47 @@ 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: the blocks its + * own interval overlaps, and nothing else. Blocks overlapping the interval are dropped here rather + * than trimmed; the caller's interval replace would only trim them, and a complete record has no + * use for what it trimmed off. + * + * **Abutting blocks are left standing, and that is a correction.** This rule used to grow the run + * across any block meeting it end-to-start, so a shortened re-send could retire its own stale tail. + * But a block has no record identity — `sessionId` is the *merged* row, shared by every record of + * the night — so "my stale tail" and "the neighbouring record" are the same shape seen from the + * same side, and the rule could not tell them apart. Two records meeting with no gap therefore read + * as one run and the second wiped the first: on the reporter's ring the records sit 33 seconds + * apart, which is a coin toss on whether they round to the same minute, and losing that toss costs + * a whole session. The shortened-re-send case is also much weaker than it was — since a record's + * timeline is placed against its own declared header bounds, a re-send of the same record produces + * the same end rather than one that drifted — so the trade is a stale tail surviving until the + * record is re-sent shorter *again* against a session that could vanish outright. Carrying the + * originating record's start on each block would allow both, and is the fix if the tail ever + * actually bites. + */ +internal fun completeSessionSurvivors( + existing: List, + replacementStart: Long, + replacementEnd: Long, +): List { + fun end(block: SleepStageBlockEntity) = block.startAt + block.durationMinutes * 60_000L + return existing.filterNot { block -> + block.startAt < replacementEnd && end(block) > replacementStart + } +} + 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 3225ad6f..957b14d5 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 3ecf80b7..9b5cdb7a 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 3156fb7e..3687d93d 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() { @@ -424,16 +478,32 @@ class RingSyncCoordinator( suspend fun measureHR(): Int? { if (hrState == MeasureState.MEASURING) return null if (!isConnected) { hrState = MeasureState.FAILED; return null } + // A workout owns the bpm stream for its whole duration, and this leg cannot share it: the + // live-sample gate is one switch per kind, so whichever of the two closed it decides + // whether the other's samples are stored. Running anyway used to mean the workout's + // samples were dropped for the length of the leg, or — if the workout started inside it — + // the leg's converging samples were stored as workout rows and its settled value never + // published. The workout screen is already showing live bpm, so refusing costs nothing. + if (workoutHRActive) return null hrState = MeasureState.MEASURING // Do NOT clear latestHRValue — it's the live value the workout UI shows, so a new // measurement keeps the last reading on screen until a fresh one replaces it. hrNoReadingReported = false measureNotWorn = false hrWindow.begin() + // This leg owns the bpm stream: a workout cannot have been running (refused above), and one + // that starts mid-leg aborts it rather than taking the gate out from under us. + 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 +516,24 @@ 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 } + // A workout started inside the leg and now owns the stream. Abort: from here the + // samples arriving are the workout's, and settling them would both publish a + // reading built from someone else's stream and leave the gate closed against it. + if (workoutHRActive) { 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 +542,22 @@ 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 + // Reopen the gate BEFORE publishing, or the one reading worth keeping is the one + // reading dropped; both travel the bus in this order. Unconditional: this leg closed + // the gate, so it owes the reopen even where a workout started underneath it — leaving + // it closed would silently drop that workout's samples for the rest of the session. + 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. An aborted leg has no result, + // so a workout that interrupted this one publishes nothing here either. + result?.let { settled -> + PulseEventBus.publishBlocking( + PulseEvent.HeartRateSample( + bpm = settled, timestamp = java.time.Instant.now(), + spot = true, ringWillLogIt = completedByRing, + ) + ) + } } return result } @@ -470,18 +569,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 +621,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 +645,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 +676,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 +728,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 +748,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 +766,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 caa956b7..2a023499 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 00000000..2ca35c5c --- /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 56edaba7..b60c0dfe 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 7b4cb38f..224731af 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 23192434..bfea4071 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 c138a866..2f60aafc 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 0b92f6a7..78132eba 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 68e0d254..d8505857 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 dc3a1f9d..9480a84e 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 00000000..e96bac00 --- /dev/null +++ b/app/src/test/java/com/pulseloop/data/MeasurementDeletionTest.kt @@ -0,0 +1,152 @@ +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 isSpotDeleted(kind: String, from: Long, to: Long) = rows.values.any { + it.kindRaw == kind && + it.measurementId.startsWith(MeasurementDeletionDao.SPOT_ID_PREFIX) && + it.timestamp in from..to + } + 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)) + } + + /** + * A `spot` row is a UUID like any live row, but the reading in it *is* regenerable: the ring + * logged that measurement itself and hands it back under a `history:` id on the next sync. With + * only an id tombstone the ring's copy walked straight back in and the reading had to be deleted + * twice. The tombstone records a range instead, because the ring stamps its log to the minute + * rather than to our settled instant. + */ + @Test + fun `a deleted spot reading suppresses the ring's copy of it`() = runTest { + val dao = FakeDeletionDao() + val at = 1_700_000_000_000L + + dao.record(listOf(measurement(java.util.UUID.randomUUID().toString(), MeasurementKind.HEART_RATE, at, "spot"))) + + assertEquals(1, dao.rows.size) + assertTrue("the ring's copy 40 s later", dao.isSpotDeleted(MeasurementKind.HEART_RATE.name, at - 50_000, at + 50_000)) + assertEquals( + "a different kind at the same moment is a different reading", + false, dao.isSpotDeleted(MeasurementKind.SPO2.name, at - 50_000, at + 50_000), + ) + assertEquals( + "an all-day sample an hour away is not that measurement", + false, dao.isSpotDeleted(MeasurementKind.HEART_RATE.name, at + 3_600_000, at + 3_700_000), + ) + } + + /** Re-deleting the same spot reading must replace its tombstone, not add one. */ + @Test + fun `a spot tombstone is keyed deterministically`() = runTest { + val dao = FakeDeletionDao() + val at = 1_700_000_000_000L + + repeat(3) { + dao.record(listOf(measurement(java.util.UUID.randomUUID().toString(), MeasurementKind.SPO2, at, "spot"))) + } + + assertEquals(1, dao.rows.size) + } + + /** + * `record` matches the source string the service layer writes. The two constants are declared + * apart so the DAO does not depend on the service package, which makes this the tripwire: if + * they drift, a deleted spot reading silently comes back on the next sync. + */ + @Test + fun `the spot source the DAO matches is the one the service writes`() { + assertEquals( + com.pulseloop.service.EventPersistenceSubscriber.SOURCE_SPOT, + MeasurementDeletionDao.SPOT_SOURCE, + ) + } + + /** + * 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 00000000..fa7e7387 --- /dev/null +++ b/app/src/test/java/com/pulseloop/diagnostics/DiagnosticsRedactorTest.kt @@ -0,0 +1,156 @@ +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 == '·' }) + } + + /** + * A chunk of a frame still being reassembled. It used to reach the log as `unknown`, which is + * deliberately exported whole (control and pairing frames decode to nothing and are what most + * connection reports are for) — so half an all-day health reply left in clear. + */ + @Test + fun `an assembler chunk is masked rather than exported whole`() { + val chunk = "6b01" + "5a".repeat(18) + val masked = DiagnosticsRedactor.maskPacketHex(chunk, DiagnosticsRedactor.FRAME_CONTINUATION, "CRP") + + assertEquals("length is preserved", chunk.length, masked.length) + assertTrue("no sample bytes survive", masked.drop(2).all { it == '·' }) + } + + /** + * A continuation chunk is the middle of a frame, so byte 0 onwards is payload — it must not be + * masked as though its family's header were sitting in front of it. The opening chunk is the + * one that has the header, and keeping it is what makes a multi-frame reply identifiable. + */ + @Test + fun `only the opening chunk of a frame keeps its header`() { + val hex = "fdda1098021600000000" + "5a".repeat(10) + + val start = DiagnosticsRedactor.maskPacketHex(hex, DiagnosticsRedactor.FRAME_START, "CRP") + val continuation = DiagnosticsRedactor.maskPacketHex(hex, DiagnosticsRedactor.FRAME_CONTINUATION, "CRP") + + assertEquals("fdda10980216", start.take(12)) + assertTrue(start.drop(12).all { it == '·' }) + assertEquals("fd", continuation.take(2)) + assertTrue("a mid-frame chunk keeps nothing but byte 0", continuation.drop(2).all { it == '·' }) + } + + /** The decoder names these kinds and the redactor matches on them; drift means a chunk exports + * in clear with nothing failing. */ + @Test + fun `the chunk kinds the redactor masks are the ones the decoder emits`() { + val opening = com.pulseloop.ring.RingDecodedEvent.FramePending(byteArrayOf(1), startsFrame = true) + val middle = com.pulseloop.ring.RingDecodedEvent.FramePending(byteArrayOf(1), startsFrame = false) + + assertEquals(DiagnosticsRedactor.FRAME_START, opening.kind) + assertEquals(DiagnosticsRedactor.FRAME_CONTINUATION, middle.kind) + } + + /** + * The header length has to come from the family that captured the packet. Exporting a report + * after switching rings used to mask every stored frame with the connected ring's header, and + * CRP's is the longest — six bytes of a one-byte-header family's samples kept in clear. + */ + @Test + fun `a packet with no recorded family is masked from byte one`() { + val colmiFrame = "69" + "5a".repeat(15) + + val unknownFamily = DiagnosticsRedactor.maskPacketHex(colmiFrame, "hr_sample", "") + val asCrp = DiagnosticsRedactor.maskPacketHex(colmiFrame, "hr_sample", "CRP") + + assertEquals("69", unknownFamily.take(2)) + assertTrue("only the opcode survives", unknownFamily.drop(2).all { it == '·' }) + assertEquals( + "CRP's six-byte header is what a wrongly-attributed packet would have leaked", + "695a5a5a5a5a", asCrp.take(12), + ) + } + + @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 acdd946b..ad9beea2 100644 --- a/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt +++ b/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt @@ -217,9 +217,13 @@ class CRPDecoderTest { val steps = driver.ingest(byteArrayOf(0x05, 0x00, 0x00), fdd1) assertTrue(steps.single() is RingDecodedEvent.ActivityUpdate) - // A framed reply split across two fdd3 notifications yields exactly one decoded event. + // A framed reply split across two fdd3 notifications decodes once, when it completes. The + // incomplete half reports itself as a pending chunk rather than as nothing: the diagnostics + // log masks by decoded kind, and an unnamed chunk of a health reply exports in clear. val full = CRPProtocol.frame(1, 9, byteArrayOf(0x50)) - assertTrue(driver.ingest(full.copyOfRange(0, 4), fdd3).isEmpty()) + val firstHalf = driver.ingest(full.copyOfRange(0, 4), fdd3).single() + assertTrue(firstHalf is RingDecodedEvent.FramePending) + assertTrue("the opening chunk carries the frame header", (firstHalf as RingDecodedEvent.FramePending).startsFrame) assertEquals(1, driver.ingest(full.copyOfRange(4, full.size), fdd3).size) } @@ -379,6 +383,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 0c1c95ce..5dff0489 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 fe500a06..a6abad65 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 00000000..5d370c34 --- /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 e8d64b28..e2cfac7b 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 63e11e9a..97a5028f 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 02b5c47b..f5e0492f 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 00417ef9..79a7cf3a 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,126 @@ 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 }) + } + + /** + * Issue #63: the ring emits zero-length segments, and one that shares a start time with a + * real segment used to take its place in the de-duplication — the real segment was dropped + * and the minutes it claimed read as wake instead. All four duplicate starts in the + * reporter's thirteen-record night dump are this shape: a zero-length LIGHT immediately + * ahead of a real 76–105 s segment. + */ + @Test + fun `a zero-length segment does not shadow a real segment sharing its start`() { + val start = 0x31def01c + val record = sleepRecord(start, start + 10 * 60, listOf( + 0xf2 to 5 * 60, // LIGHT, 00:00–05:00 + 0xf1 to 0, // zero-length DEEP at 05:00 — claims no minute + 0xf3 to 5 * 60, // REM, 05:00–10:00, sharing the zero-length segment's start + )) + val event = YCBTHealthRecords.sleep(record).first() as RingDecodedEvent.SleepTimeline + + assertEquals(5, event.stages.count { it == SleepStage.LIGHT }) + assertEquals(5, event.stages.count { it == SleepStage.REM }) + assertFalse(event.stages.contains(SleepStage.AWAKE)) + } + + /** 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 +428,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/ring/YCBTHistoryTransferTest.kt b/app/src/test/java/com/pulseloop/ring/YCBTHistoryTransferTest.kt index bd457a1b..184b80ce 100644 --- a/app/src/test/java/com/pulseloop/ring/YCBTHistoryTransferTest.kt +++ b/app/src/test/java/com/pulseloop/ring/YCBTHistoryTransferTest.kt @@ -102,6 +102,50 @@ class YCBTHistoryTransferTest { assertEquals(listOf(71.0, 66.0), heartRates(done)) } + /** + * Issue #69: the header's packet count is an estimate the ring contradicts one frame later, and + * requiring the two to agree threw away every composite record on a reporter's ring. + * + * Their `05 09` header declared **5** packets for 840 bytes; the ring then sent **6**, because it + * packs whole 20-byte records into each frame (7 × 20 = 140) instead of filling it. Bytes and CRC + * were correct on every transfer. The vendor ignores the count too — at `Sync_Block_Verify` + * `DataUnpack` reads the two bytes into locals it never compares, sizes its buffer from the + * terminal's own length, and accepts on the CRC alone. + */ + @Test + fun `a header packet count the ring contradicts does not fail the transfer`() { + val writer = FakeWriter() + val transfer = YCBTHistoryTransfer(writer = writer) + transfer.start(types = listOf(YCBTHistoryType.HEART)) + + transfer.handle(cmd = 0x06, payload = header(records = 2, packets = 5, bytes = heartBuffer.size)) + transfer.handle(cmd = 0x15, payload = heartBuffer) + + writer.sent.clear() + val done = transfer.handle(cmd = 0x80, payload = terminal(packets = 6, buffer = heartBuffer)) + + assertEquals("the records are kept", listOf(71.0, 66.0), heartRates(done)) + assertArrayEquals("and the block is accepted", ackAccepted, writer.sent[0]) + } + + /** The byte count the terminal itself reports must still match what arrived — that is the check + * that catches a genuinely short transfer, and it is independent of the packet estimate. */ + @Test + fun `a terminal reporting more bytes than arrived still nacks`() { + val writer = FakeWriter() + val transfer = YCBTHistoryTransfer(writer = writer) + transfer.start(types = listOf(YCBTHistoryType.HEART)) + + transfer.handle(cmd = 0x06, payload = header(records = 2, packets = 1, bytes = heartBuffer.size)) + transfer.handle(cmd = 0x15, payload = heartBuffer.copyOfRange(0, 6)) + + writer.sent.clear() + val out = transfer.handle(cmd = 0x80, payload = terminal(packets = 1, buffer = heartBuffer)) + + assertTrue("nothing is decoded from a short buffer", heartRates(out).isEmpty()) + assertArrayEquals(ackCrcFailure, writer.sent[0]) + } + @Test fun `CRC mismatch nacks and retries the type once`() { val writer = FakeWriter() diff --git a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt index e9f3c572..f7e016b5 100644 --- a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt +++ b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt @@ -114,6 +114,74 @@ 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 }) + } + + /** + * A complete record retires what its interval overlaps and leaves what merely touches it. + * + * The rule used to grow across abutting blocks so a shortened re-send could retire its own + * stale head and tail — but a block carries no record identity, so that is indistinguishable + * from the neighbouring record of a split night, and it cost a whole session (see the + * zero-gap case below). The stale tail survives instead: a re-send now reproduces the record's + * declared bounds rather than a drifted end, so a record shortening itself is the rarer event + * of the two, and it costs minutes rather than hours. + */ + @Test + fun `a shortened complete record leaves the blocks that only abut it`() { + 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("head", "tail", "nap"), survivors.map { it.id }) + } + + /** + * Issue #63, the case the three-minute gap in the original report hid: two records of one night + * that meet with **no** gap. The reporter's ring closes one record and opens the next 33 seconds + * later, so whether the two round to the same minute is a coin toss — and when they did, the + * abut rule read them as one run and the second record's re-send wiped the first's five hours. + */ + @Test + fun `a complete record does not retire the session it meets with no gap`() { + val a = 1_725_408_720_000L + val b = a + 322 * 60_000L // the first record's blocks end exactly here + val existing = listOf( + block("a-1", a, 200, "LIGHT"), + block("a-2", a + 200 * 60_000L, 122, "DEEP"), // ends exactly at b + block("b-1", b, 151, "LIGHT"), + ) + + val survivors = completeSessionSurvivors(existing, b, b + 151 * 60_000L) + + assertEquals(listOf("a-1", "a-2"), 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 +224,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 0b9b76bb..d3521127 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 d314e9c4..b93f0d81 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 99e47382..0c8a1ec9 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 00000000..c9a290f7 --- /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 f324a71c..e92c0ff0 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)) + } }