From d828b66e04efd3c63de783dff34537464a33ef50 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Tue, 15 Sep 2026 11:01:49 -0700 Subject: [PATCH 1/7] fix(jring): 0x10's byte 1 is a day offset, so ask for today (#73) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An SR08 synced steps and heart rate but never a single night of sleep, however long it was worn — the ring's own display and JYouPro both showed the night. Byte 1 of the `0x10` history query is a **day offset**, not a count of days. `IRemoteService.getDataByDay(int type, int day)` reaches `BluetoothLeService.a(int, int)`, which writes the type into `bArr[0]` (`1` → 16 = `0x10` activity/sleep, `2` → 22 = `0x16` heart rate) and the `day` argument straight into `bArr[1]` — one day per call, no count anywhere. Its caller loops: `DupMainActivity.onGetMultipleSportData` decrements `P_SYNC_HISTORY_DAY` on each day's sync-end and calls `getDataByDay(1, i6)` again, counting down to 0. Read as a count, `0x10/03` asked for the day before last and `0x10/01` for yesterday. **No pass ever requested today**, and `0x10` is the only source of sleep — there is no sleep-only request — so last night could not arrive at all. Steps and HR masked it: activity comes from the ring's automatic `0x03` push, and `makeHistoryMeasurementQueryCommand` already hardcoded `16 00`, i.e. offset 0. The one request that asked for today was the one that worked. So each day gets its own request: today always, plus the rest of the backfill window once per connection (the gate matters because `runStartup` is also the ~30-minute background sync). Today leads the window rather than trailing it as the vendor's countdown does — it is the day the user opened the app to see, and it is the shape the reporter hardware-validated on an SR08, where requesting offsets 0, 1 and 2 separately produced 27 `0x11` sleep entries after `0x10/00`. We stay fire-and-forget rather than reply-driven because nothing here decodes a per-day sync-end to chain from; AGENTS.md records that divergence and what would make the vendor's chain necessary. The count reading came in with a citation to Gadgetbridge's `triggerActivityReportByDays()` — an identifier that appears nowhere in the vendor SDK, only in our own files. iOS was never affected: its `makeHistoryQueryCommand()` is the fixed hex `1000…`, i.e. offset 0, and simply never backfilled. Also corrects two comments that cited the count semantics, and drops `syncWindowDays`' "must match makeHistoryQueryCommand's default" claim — it sizes the progress bar and nothing else, as AGENTS.md already warned after that same mistake deferred this fix once. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 52 ++++++++++++++---- .../java/com/pulseloop/ring/JringDriver.kt | 51 ++++++++++------- .../java/com/pulseloop/ring/RingEncoder.kt | 25 +++++++-- .../service/EventPersistenceSubscriber.kt | 5 +- .../pulseloop/service/RingSyncCoordinator.kt | 5 +- .../ring/ExistingFamilyRefreshContractTest.kt | 55 +++++++++++++++---- 6 files changed, 144 insertions(+), 49 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 05f9e80b..74bf541b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -98,16 +98,16 @@ simply produces no reply. `CRPSyncEngine.sendSleepBackfill` therefore pulls the per connection** (not per pass — `runStartup` is also the ~30-minute background sync, and this ring funnels everything through one `fdd2` channel). -jring has the same gap, for different reasons. Its depth is `makeHistoryQueryCommand()`'s default of -1, called with no argument at `JringDriver.kt:105`, against a command that accepts up to 27. -**`RingSyncCoordinator.syncWindowDays` is not that control** — despite its "must match -makeHistoryQueryCommand's default" comment, it has exactly one use, sizing the sync-progress window -in `beginSyncProgress`, and it applies to every family. Don't cite it as a per-family request depth; -that mistake is what deferred this fix once already. Two things do make jring harder than CRP: -`JringSyncEngine.runStartup` has no once-per-connection gate, so a wider `days` re-pulls the whole -span on every ~30-minute background pass rather than once; and `0x10` returns activity *and* sleep -together — there is no sleep-only request — so each extra day costs ~96 activity packets -(15× 1-minute buckets per packet) on top of the night. +jring had the same gap and a worse one underneath it (see "#73" below: byte 1 of `0x10` is a day +*offset*, so the app never asked for today at all). One day per request, so depth is a loop, not a +number. **`RingSyncCoordinator.syncWindowDays` is not that control** — despite the "must match +makeHistoryQueryCommand's default" comment it used to carry, it has exactly one use, sizing the +sync-progress window in `beginSyncProgress`, and it applies to every family. Don't cite it as a +per-family request depth; that mistake is what deferred this fix once already. Two things still make +jring harder than CRP: `runStartup` is also the ~30-minute background sync, so a wider window must be +gated to once per connection (`historyBackfilled`) or it re-pulls the whole span every half hour; +and `0x10` returns activity *and* sleep together — there is no sleep-only request — so each extra +day costs ~96 activity packets (15× 1-minute buckets per packet) on top of the night. Consequence to keep in mind: nothing bulk-deletes real sleep any more, so a Forget followed by pairing a different ring carries the previous ring's history over. If that ever needs to change, @@ -179,6 +179,38 @@ Corollary for new protocol work: a reply that merely reports something about the serial, capabilities) is not a connection event. Give it its own `RingDecodedEvent` — as `FirmwareRevision` does — rather than hanging it off `Status`. +## `0x10`'s byte 1 is a day offset, not a day count (issue #73) + +**An SR08 synced steps and HR but never a single night of sleep**, however long it was worn. The +request depth looked fine; the day being requested was not. `getDataByDay(int type, int day)` +(`IRemoteService.aidl`) reaches `BluetoothLeService.a(int, int)`, which writes the type into +`bArr[0]` — `1` → `16` (`0x10`, activity+sleep), `2` → `22` (`0x16`, heart rate), via the +`WeatherUtil` constants jadx happens to resolve those bytes to — and then the `day` argument +**straight into `bArr[1]`**. One day per call. The caller loops: +`DupMainActivity.onGetMultipleSportData` decrements `P_SYNC_HISTORY_DAY` on each day's sync-end and +calls `getDataByDay(1, i6)` again, counting **down to 0**. + +Read as a count, our `0x10/03` asked for the day before last and `0x10/01` for yesterday, so **no +pass ever requested today** — and `0x10` is the only source of sleep, so last night could not +arrive. Steps and HR masked it: activity comes from the ring's automatic `0x03` push, and +`makeHistoryMeasurementQueryCommand` already hardcoded `16 00`, i.e. offset 0. The one request that +asked for today was the one that worked, which is the clue that was sitting in the source the whole +time. + +Two process notes worth keeping: + +- **The count reading came from a name that isn't in the vendor SDK.** The KDoc cited Gadgetbridge's + `triggerActivityReportByDays()`; that identifier appears nowhere in `decompiled-jring-offical/`, + only in our own files. A citation to a *different* project's helper is not vendor evidence — check + the decompile, which for this family means `jadx --single-class` against `classes2.dex`, since + only the `.aidl` files ship as sources. +- **We diverge from the vendor's chain deliberately.** It is reply-driven, one day at a time, + counting down; we enqueue the window's offsets in one pass, **today first**, because nothing here + decodes a per-day sync-end to drive the next request from. That is the shape the reporter + hardware-validated on an SR08 (offsets 0, 1, 2 separately → 27 `0x11` sleep entries after + `0x10/00`). If a ring is ever seen truncating a day's stream when the next request lands, the + vendor's chain is the fix and it needs a sync-end signal decoded first. + ## Colmi R11 (CRP "Da Rings") — diagnose from the capture, and decode wear state before blaming code **Read this before changing anything in `CRP*` startup, sync, all-day-monitoring, or history code — diff --git a/app/src/main/java/com/pulseloop/ring/JringDriver.kt b/app/src/main/java/com/pulseloop/ring/JringDriver.kt index 1afef832..558b4086 100644 --- a/app/src/main/java/com/pulseloop/ring/JringDriver.kt +++ b/app/src/main/java/com/pulseloop/ring/JringDriver.kt @@ -1,7 +1,8 @@ package com.pulseloop.ring -/** Days of history pulled on the first pass of a jring connection; every later pass asks for one. - * See [JringSyncEngine.historyDaysForThisPass] for why this is shorter than CRP's week. */ +/** Days of history pulled on the first pass of a jring connection; every later pass asks only for + * today. Each day is its own `0x10` request — byte 1 is a day offset, not a count (issue #73). + * See [JringSyncEngine.historyDayOffsetsForThisPass] for why this is shorter than CRP's week. */ private const val JRING_BACKFILL_DAYS = 3 @OptIn(ExperimentalStdlibApi::class) @@ -106,7 +107,9 @@ class JringSyncEngine( // had to initialise with the vendor app first. writer?.enqueue(encoder.makeAutomaticHeartRateCommand(enabled = true, cadenceMinutes = 30)) writer?.enqueue(encoder.makeBandFunctionCommand()) - writer?.enqueue(encoder.makeHistoryQueryCommand(days = historyDaysForThisPass())) + for (dayOffset in historyDayOffsetsForThisPass()) { + writer?.enqueue(encoder.makeHistoryQueryCommand(dayOffset = dayOffset)) + } writer?.enqueue(encoder.makeHistoryMeasurementQueryCommand()) } @@ -116,29 +119,39 @@ class JringSyncEngine( private var historyBackfilled = false /** - * How many days of history to ask for on this pass: the deep window once per connection, one - * day on every pass after it. + * Which days to ask for on this pass: **today always**, plus the older days of the backfill + * window once per connection. + * + * `0x10`'s byte 1 is a day offset rather than a count (issue #73 — see + * [RingEncoder.makeHistoryQueryCommand] for the vendor evidence), so each day needs its own + * request. Today (`0`) leads every pass because it is the day the user is looking at, and + * because under the old count reading it was the one day never requested at all: a single + * `0x10/01` asked for yesterday, so last night's sleep never arrived. * - * The ring holds days the app has never asked for. Before issue #43 that didn't matter, because - * connecting deleted the stored copy anyway; now that it doesn't, a single-day request means a - * user's history can only ever grow one night at a time from install, and never recovers what - * the ring already has. `0x10` takes a day count (`triggerActivityReportByDays`, capped at 27) - * and the ring replies with the days it actually has, so asking for more is safe. + * **Why the once-per-connection gate matters more here than on CRP.** [runStartup] is also the + * ~30-minute background sync (and `refresh()`/`querySleep()` route through it), so an + * unconditional wider window would re-pull the whole span every half hour forever. And `0x10` + * returns activity *and* sleep — there is no sleep-only request — so each extra day is roughly + * 96 more packets (activity arrives as 15× 1-minute buckets each), against the nights we + * actually came for. That volume, not the nights, is why this window is deliberately shorter + * than the CRP backfill's week. * - * **Why the gate matters more here than on CRP.** [runStartup] is also the ~30-minute background - * sync (and `refresh()`/`querySleep()` route through it), so an unconditional wider window would - * re-pull the whole span every half hour forever. And `0x10` returns activity *and* sleep — there - * is no sleep-only request — so each extra day is roughly 96 more packets (activity arrives as - * 15× 1-minute buckets each), against the nights we actually came for. That volume, not the - * nights, is why this window is deliberately shorter than the CRP backfill's week. + * **Known divergence from the vendor, deliberate.** JYouPro is strictly reply-driven: it asks + * for one day, waits for that day's sync-end callback, then asks for the next, counting *down* + * to today (`DupMainActivity.onGetMultipleSportData`). We enqueue the window's requests in one + * pass, newest first, because nothing here decodes a per-day sync-end to drive the next request + * from — and this is the shape the #73 reporter validated on an SR08, where requesting offsets + * 0, 1 and 2 separately produced 27 `0x11` sleep entries after `0x10/00`. If a ring is ever seen + * truncating a day's stream when the next request lands, the vendor's chain is the fix, and it + * needs a sync-end signal decoded first. * * Re-syncing the same days is harmless: activity buckets upsert by timestamp with the day total * recomputed from distinct buckets, and sleep reconciles one waking day at a time. */ - private fun historyDaysForThisPass(): Int { - if (historyBackfilled) return 1 + private fun historyDayOffsetsForThisPass(): List { + if (historyBackfilled) return listOf(0) historyBackfilled = true - return JRING_BACKFILL_DAYS + return (0 until JRING_BACKFILL_DAYS).toList() } override fun handle(event: RingDecodedEvent) { diff --git a/app/src/main/java/com/pulseloop/ring/RingEncoder.kt b/app/src/main/java/com/pulseloop/ring/RingEncoder.kt index 0134da48..16adfb73 100644 --- a/app/src/main/java/com/pulseloop/ring/RingEncoder.kt +++ b/app/src/main/java/com/pulseloop/ring/RingEncoder.kt @@ -29,15 +29,30 @@ object RingEncoder { */ fun makeDefaultUserInfoCommand(): ByteArray = hexToBytes("0299b85a00000000000000000000000000000000") /** - * Request activity + sleep history for the last N days (0x10). - * byte[1] = number of days (0-27). The ring sends back 0x10 (steps) + * Request activity + sleep history for **one** day (0x10). The ring answers with 0x10 (steps) * and 0x11 (sleep) notifications as multi-packet streams. - * Matches Gadgetbridge's triggerActivityReportByDays(). + * + * **byte[1] is a day OFFSET, not a count** (issue #73). `0` is today, `1` yesterday, and so on. + * The vendor SDK settles it: `IRemoteService.getDataByDay(int type, int day)` reaches + * `BluetoothLeService.a(int, int)`, which writes the *type* into `bArr[0]` + * (`1` -> 16 = `0x10` activity/sleep, `2` -> 22 = `0x16` heart rate) and then the `day` + * argument straight into `bArr[1]` — one day per call, no count anywhere. Its caller loops: + * `DupMainActivity.onGetMultipleSportData` decrements `P_SYNC_HISTORY_DAY` on each day's sync-end + * and calls `getDataByDay(1, i6)` again, counting down to `0`. + * + * Read as a count, `0x10/01` asks for *yesterday* and `0x10/03` for the day before last — so + * today was never requested and last night's sleep could not arrive at all, which is exactly + * what #73 reported on an SR08. Steps and HR still showed up because neither depends on this + * request for today: activity arrives on the ring's automatic 0x03 push, and + * [makeHistoryMeasurementQueryCommand] already hardcodes offset 0. + * + * The old doc credited Gadgetbridge's `triggerActivityReportByDays()`; that name appears + * nowhere in the vendor SDK and the count reading came with it. */ - fun makeHistoryQueryCommand(days: Int = 1): ByteArray { + fun makeHistoryQueryCommand(dayOffset: Int = 0): ByteArray { val cmd = ByteArray(20) cmd[0] = 0x10 - cmd[1] = days.coerceIn(0, 27).toByte() + cmd[1] = dayOffset.coerceIn(0, 27).toByte() return cmd } fun makeHistoryMeasurementQueryCommand(): ByteArray = hexToBytes("1600000000000000000000000000000000000000") diff --git a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index d69d149b..9aae681a 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -200,8 +200,9 @@ class EventPersistenceSubscriber( // History, because this keeps getting re-added: connect used to clear *all* // sleep for every family except YCBT and rebuild it from the ring. No ring // re-supplies more than its own buffer, and the two smallest re-supply a - // single day — CRP sends `queryHistorySleep(daysAgo=0)`, jring - // `makeHistoryQueryCommand()` with its 1-day default (JringDriver.kt:105, + // single day — CRP sends `queryHistorySleep(daysAgo=0)`, jring asked + // `makeHistoryQueryCommand()` for one day (and, until issue #73, for the + // wrong one: byte 1 is a day offset, so it never asked for today at all; // NOT `syncWindowDays`, which only sizes the progress bar) — so every // connect destroyed each night older than that, and a new night replaced // the last one instead of joining it (issue #43, zaggash's R11). The diff --git a/app/src/main/java/com/pulseloop/service/RingSyncCoordinator.kt b/app/src/main/java/com/pulseloop/service/RingSyncCoordinator.kt index e662e1f0..bd77828a 100644 --- a/app/src/main/java/com/pulseloop/service/RingSyncCoordinator.kt +++ b/app/src/main/java/com/pulseloop/service/RingSyncCoordinator.kt @@ -49,7 +49,10 @@ class RingSyncCoordinator( private var syncWindowEnd = 0L private var syncResetJob: Job? = null private var lastAdvanceAt = 0L - /** Days of history requested on startup — must match makeHistoryQueryCommand's default. */ + /** Width of the sync-progress window, in days. This sizes the progress bar in + * [beginSyncProgress] and nothing else — it is **not** any family's request depth, despite + * what this comment used to claim (see `AGENTS.md`; citing it as jring's depth deferred the + * #73 fix once already). */ private val syncWindowDays = 1 /** How often the stall-watcher checks, and how long without progress before it gives up. */ private val SYNC_STALL_CHECK_MS = 2_000L diff --git a/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt b/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt index 82c70320..db989eb9 100644 --- a/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt +++ b/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt @@ -24,12 +24,32 @@ class ExistingFamilyRefreshContractTest { assertEquals(startup, sleep) } - /** Byte 1 of the `0x10` history query is the day count (`triggerActivityReportByDays`). */ - private fun historyDays(sent: List): List = + /** Byte 1 of each `0x10` history query: the **day offset** it asks for — `0` today, `1` + * yesterday (issue #73). One request per day, so this is a list, not a single count. */ + private fun historyDayOffsets(sent: List): List = sent.filter { it[0].toInt() == 0x10 }.map { it[1].toInt() } @Test - fun `Jring pulls a deeper history window once per connection, then one day per pass`() { + fun `Jring always asks for today, on every pass`() { + // Issue #73, and the whole bug: byte 1 is a day offset, so the old count reading asked + // `0x10/03` (the day before last) on the first pass and `0x10/01` (yesterday) after it. + // Today was never requested by any pass, and `0x10` is the only source of sleep — so on an + // SR08 last night's sleep could not arrive at all, however many times the app synced. + val w = FakeWriter() + val engine = JringSyncEngine(w) + + engine.runStartup() + assertTrue("the first pass must ask for today", 0 in historyDayOffsets(w.sent)) + + w.sent.clear() + engine.runStartup() + engine.refresh() // routes through runStartup + engine.querySleep() // ditto + assertEquals("every later pass asks for today and nothing else", listOf(0, 0, 0), historyDayOffsets(w.sent)) + } + + @Test + fun `Jring pulls a deeper history window once per connection, then today only`() { // Issue #43. A single-day request means stored history can only grow one night at a time // from install and never recovers what the ring already holds. But runStartup is also the // ~30-minute background sync, so the deep window must NOT repeat: 0x10 returns activity as @@ -38,28 +58,39 @@ class ExistingFamilyRefreshContractTest { val engine = JringSyncEngine(w) engine.runStartup() - assertEquals(listOf(3), historyDays(w.sent)) + assertEquals(listOf(0, 1, 2), historyDayOffsets(w.sent)) w.sent.clear() engine.runStartup() - engine.refresh() // routes through runStartup - engine.querySleep() // ditto - assertEquals(listOf(1, 1, 1), historyDays(w.sent)) + assertEquals(listOf(0), historyDayOffsets(w.sent)) // A new connection builds a new engine, which backfills again. val reconnected = FakeWriter() JringSyncEngine(reconnected).runStartup() - assertEquals(listOf(3), historyDays(reconnected.sent)) + assertEquals(listOf(0, 1, 2), historyDayOffsets(reconnected.sent)) + } + + @Test + fun `the Jring backfill asks newest day first`() { + // Today leads the window: it is the day the user opened the app to see, and if a ring ever + // truncates the run when the next request lands, the day that survives is the one that + // matters. The vendor counts *down* to today instead — see historyDayOffsetsForThisPass + // for why we diverge and what would make the vendor's chain necessary. + val w = FakeWriter() + JringSyncEngine(w).runStartup() + val offsets = historyDayOffsets(w.sent) + assertEquals("today must be requested first", 0, offsets.first()) + assertEquals("offsets must be distinct and ascending", offsets.sorted().distinct(), offsets) } @Test fun `the Jring backfill window stays inside what the command encodes`() { - // makeHistoryQueryCommand coerces to 0..27; a window above that would silently truncate - // and the request would no longer mean what the constant says. + // makeHistoryQueryCommand coerces to 0..27; an offset above that would silently truncate + // and the request would ask for a different day than the caller meant. val w = FakeWriter() JringSyncEngine(w).runStartup() - val requested = historyDays(w.sent).single() - assertTrue("backfill window $requested must survive the 0..27 coerce", requested in 1..27) + val offsets = historyDayOffsets(w.sent) + assertTrue("every requested offset $offsets must survive the 0..27 coerce", offsets.all { it in 0..27 }) } @Test From 8c2e6bcdaf9b186c0a2fe796713353164cd4ad3c Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Tue, 15 Sep 2026 11:02:04 -0700 Subject: [PATCH 2/7] fix(settings): route a user's Connect through userConnect (#72) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tapping Connect on the Settings hero card did nothing after disconnecting the ring from Settings — no spinner, no error, no attempt. "Reconnect last ring" on the ring detail screen worked, which is what made it look like a UI-only fault. Both buttons reconnect the same stored ring, but through different entry points. `userDisconnect()` persists a stay-off flag so a manual Disconnect actually sticks against every auto-reconnect path — app foreground, the watchdog, `RingSyncWorker`, the coach runner — and `connectLastKnown()` honours it by returning immediately. The detail screen calls `userConnect()`, which clears the flag first; the hero card called `connectLastKnown()`, so it was suppressed by the flag in precisely the situation the button exists for. The guard is right — the caller was wrong. The hero card and the pairing screen's "Reconnect last ring" now call `userConnect()`: a user tapping Connect *is* the intent the flag was waiting for. Every genuine auto-reconnect caller is untouched and still honours it. `connectLastKnown()` gains a KDoc saying it is the auto path and that a button must not call it, since its name reads exactly like what a Connect button wants. Also fixes the same silent no-op one action along: the card rendered "Disconnect" as an enabled button whose `when` branch fell into `else -> {}`. It is wired to `userDisconnect()`, and the `when` is now exhaustive over the action enum, so a future action can't be added without handling it. Co-Authored-By: Claude Opus 5 (1M context) --- .../main/java/com/pulseloop/ring/RingBLEClient.kt | 12 +++++++++++- .../com/pulseloop/ui/components/DeviceHeroCard.kt | 8 +++++++- .../java/com/pulseloop/ui/screens/PairingScreen.kt | 5 ++++- .../java/com/pulseloop/ui/screens/SettingsScreen.kt | 7 ++++++- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt index 101b0af1..bb996668 100644 --- a/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt +++ b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt @@ -313,7 +313,17 @@ class RingBLEClient( } /** - * Reconnect to the stored ring, alternating strategies like the official QRing app + * **Auto-reconnect only. A user tapping "Connect" must call [userConnect] instead.** + * + * This path deliberately honours the stay-off flag [userDisconnect] persists, so it returns + * without doing anything at all — no scan, no state change, no error — whenever the user has + * manually disconnected. That is correct for every caller here (app foreground, the watchdog, + * `RingSyncWorker`, the coach runner) and silently wrong for a button: wiring the Settings hero + * card's Connect to this made it a no-op in precisely the situation it exists for, with nothing + * on screen to say why (issue #72). [userConnect] clears the flag first, which is what makes a + * user's Connect mean "I want the ring back" rather than "retry, if you're still allowed to". + * + * Reconnects to the stored ring, alternating strategies like the official QRing app * (BleBaseControl: `count % 3` picks direct-connect vs scan-then-connect): a direct * connect is fastest right after a drop, but parks uselessly against a ring that * isn't advertising — a scan first proves reachability. Attempts are capped diff --git a/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt index 7c4e19f2..7650edf8 100644 --- a/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt +++ b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt @@ -49,6 +49,7 @@ fun DeviceHeroCard( onOpenWearable: () -> Unit, onConnect: () -> Unit, onSetUp: () -> Unit, + onDisconnect: () -> Unit = {}, ) { val deviceType = bleState.activeDeviceType ?: storedDevice?.deviceType val wearableModel = WearableModel.model(bleState.activeWearableModelID) @@ -166,7 +167,12 @@ fun DeviceHeroCard( when (status.action) { DeviceHeroStatus.Action.CONNECT -> onConnect() DeviceHeroStatus.Action.SET_UP -> onSetUp() - else -> {} + // The card rendered "Disconnect" as an enabled button wired to + // nothing — the same silent no-op as issue #72's Connect, one + // action along. PENDING is the only state that should do nothing, + // and it is already inert via `actionEnabled`. + DeviceHeroStatus.Action.DISCONNECT -> onDisconnect() + DeviceHeroStatus.Action.PENDING -> {} } } .padding(horizontal = 12.dp) diff --git a/app/src/main/java/com/pulseloop/ui/screens/PairingScreen.kt b/app/src/main/java/com/pulseloop/ui/screens/PairingScreen.kt index 60e6635f..bfc9526e 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/PairingScreen.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/PairingScreen.kt @@ -260,7 +260,10 @@ fun PairingScreen( state.connectionState != RingConnectionState.RECONNECTING ) { SecondaryButton("Reconnect last ring", icon = Icons.Filled.Refresh) { - bleClient.connectLastKnown() + // See SettingsScreen's onConnect (issue #72): a user-initiated + // reconnect must clear the stay-off flag a manual Disconnect set, + // or it is silently suppressed. + bleClient.userConnect() } } } diff --git a/app/src/main/java/com/pulseloop/ui/screens/SettingsScreen.kt b/app/src/main/java/com/pulseloop/ui/screens/SettingsScreen.kt index ea564f7c..9c1141ac 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SettingsScreen.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SettingsScreen.kt @@ -82,7 +82,12 @@ fun SettingsScreen( storedDevice = storedDevice, lastSyncAt = coordinator?.lastSyncAt ?: storedDevice?.lastSyncAt, onOpenWearable = { navigate("settings/wearable") }, - onConnect = { bleClient?.connectLastKnown() }, + // userConnect(), not connectLastKnown(): a manual Disconnect persists a stay-off flag + // that connectLastKnown() honours by returning immediately, so this button did nothing + // at all in exactly the situation it exists for (issue #72). userConnect clears the + // flag first — the user tapping Connect *is* the intent the flag was waiting for. + onConnect = { bleClient?.userConnect() }, + onDisconnect = { bleClient?.userDisconnect() }, onSetUp = { navigate("pairing") }, ) From 32a0a1160a3c85499c71dc2bef03fcbbf8e154b7 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Tue, 15 Sep 2026 11:16:25 -0700 Subject: [PATCH 3/7] docs(agents): state the no-agent-attribution rule where agents read it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `docs/crp-r11-hardening-plan.md` and `docs/health-connect-integration.md` both said not to add a `Co-Authored-By` trailer, and 13 commits carry one anyway — because those are task-specific plan documents that nothing loads automatically. The rule was never in AGENTS.md, which is. GitHub reads the trailer as a second author and lists the address as a repository contributor, so an agent shows up in the contributors graph next to the maintainer. That is what prompted this. Existing commits are deliberately left alone: rewriting them would orphan the v2.8.0+51, v2.9.0+52-rc1, v2.9.0+53 and v2.9.1+54-rc1 tags and the published APKs hanging off them, to fix something cosmetic. --- AGENTS.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 74bf541b..e12657b3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,6 +3,22 @@ Read this before touching ring/BLE hardware code (`app/src/main/java/com/pulseloop/ring/`, `app/src/main/java/com/pulseloop/wearables/`). Full detail: `docs/qring-ble-adoption.md`. +## Commits carry no agent attribution + +**Do not add a `Co-Authored-By` trailer to commits in this repo** — not for Claude, not for any +other agent or tool. GitHub reads that trailer as a second author and lists the address as a +repository *contributor*, so an agent appears in the contributors graph beside the maintainer. + +This was already the rule, stated in `android/docs/crp-r11-hardening-plan.md` §"Ground rules" and +`android/docs/health-connect-integration.md` §4, and it was still violated on 13 commits — because +those are task-specific plan documents that nothing loads automatically, and the rule was nowhere +an agent reliably reads. It lives here now for that reason. The Claude Code harness injects a +session reminder *asking* for the trailer; that reminder defers to the project's own instructions, +and this is one. + +Applies to both repos. Leave the existing commits alone: rewriting them would orphan the release +tags and the published APKs hanging off them. + ## Ring BLE protocol work — match the vendor app, not iOS When porting or fixing a ring's BLE protocol (connect/pairing sequence, GATT characteristic From 53e1f15f278ec5e1127070813b1f51bf25f8c4ec Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Thu, 17 Sep 2026 02:09:35 -0700 Subject: [PATCH 4/7] fix(jring): page history one day at a time, holding each request until the stream settles (#73) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The day-offset fix was necessary and not sufficient. On v2.9.1+54-rc1 the same SR08 still showed 45 minutes of sleep against an expected 6h30, and the reporter's capture says why: `10 00`, `10 01`, `10 02` and `16 00` all went out inside 176 ms, with `10 02` landing between two of today's sleep packets. Four `0x11` packets arrived in all — three consecutive 15-minute blocks for today (01:15–02:00, exactly the 45 minutes displayed) and one from the previous day. The ring answers one history request at a time, so the window's days were truncating each other. `JringSyncEngine` already recorded the condition that would force this change: "if a ring is ever seen truncating a day's stream when the next request lands, the vendor's chain is the fix, and it needs a sync-end signal decoded first." This is that ring. JringHistorySync is a port of LuckRingHistorySync — the in-repo pager for streams that carry no end-of-transfer marker — with a second leg per day. Per day: `0x10/offset`, then that day's activity/sleep stream, then `0x16/offset`, then the next day, which is the vendor's own chain (`getDataByDay(1, day)` then `(2, day)`). The activity/sleep leg settles on a timer because nothing else is available: `0x10` and `0x11` are bare runs of 15 one-minute samples with nothing marking a day's end. JYouPro is in the same position and resets a 2000 ms idle timer on each reply, which is where the default comes from. Heart rate does have a real end marker — `0x16` subtype `0xFF`, already decoded as HistorySyncFinished — so that leg chains on the protocol and only falls back to the timers. Details that are load-bearing rather than incidental: - `start()` returns whether it began a pass, and `historyBackfilled` is spent only when one did. runStartup is also the ~30-minute background sync, so a pass landing mid-backfill is routine; had the declined no-op consumed the gate, a connection could lose its backfill entirely and never ask for the older days again. - A day that answers nothing still gets its HR request. `16 00` used to be unconditional and it is the request that always worked; a silent `0x10` must not take it down. - Timers are epoch-guarded. Job.cancel() does not retract a coroutine already past its delay and waiting on the monitor, so a stale settle firing into a live stream would re-create the truncation from the inside. - connectionDidEnd drops an in-flight pass. The writer outlives the driver, so a surviving timer would enqueue the next day's `0x10` into whatever connection comes next. makeHistoryMeasurementQueryCommand takes a day offset now, matching getDataByDay(2, day); it was a fixed `16 00`, which is why heart rate kept working while sleep did not. The discriminating test is the negative one — sleep frames arriving at half a settle window must not be built on top of. That is what the reporter asked for by name. Ten more cover per-day ordering, the end-marker advance proven against a one-second settle window, the stall skip, the declined re-entrant start, cancel, the timer race, out-of-leg frames, and the driver wiring on both ends (ingest feeds the pager, disconnect ends it). 1365 tests, 0 failures. Not hardware-tested here; #73's reporter has the SR08. --- AGENTS.md | 48 +++- .../java/com/pulseloop/ring/JringDriver.kt | 77 ++++-- .../com/pulseloop/ring/JringHistorySync.kt | 195 ++++++++++++++ .../java/com/pulseloop/ring/RingEncoder.kt | 18 +- .../ring/ExistingFamilyRefreshContractTest.kt | 108 ++++++-- .../pulseloop/ring/JringHistorySyncTest.kt | 247 ++++++++++++++++++ 6 files changed, 636 insertions(+), 57 deletions(-) create mode 100644 app/src/main/java/com/pulseloop/ring/JringHistorySync.kt create mode 100644 app/src/test/java/com/pulseloop/ring/JringHistorySyncTest.kt diff --git a/AGENTS.md b/AGENTS.md index e12657b3..72e5f8d0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -220,12 +220,48 @@ Two process notes worth keeping: only in our own files. A citation to a *different* project's helper is not vendor evidence — check the decompile, which for this family means `jadx --single-class` against `classes2.dex`, since only the `.aidl` files ship as sources. -- **We diverge from the vendor's chain deliberately.** It is reply-driven, one day at a time, - counting down; we enqueue the window's offsets in one pass, **today first**, because nothing here - decodes a per-day sync-end to drive the next request from. That is the shape the reporter - hardware-validated on an SR08 (offsets 0, 1, 2 separately → 27 `0x11` sleep entries after - `0x10/00`). If a ring is ever seen truncating a day's stream when the next request lands, the - vendor's chain is the fix and it needs a sync-end signal decoded first. +- **Asking for the right day was only half of it — see below.** The first fix enqueued the whole + window in one pass and recorded the condition that would force the vendor's chain. That condition + arrived within a day. + +## This ring answers one history request at a time (issue #73, second half) + +**The day-offset fix alone did not restore sleep.** On `v2.9.1+54-rc1` the same SR08 still showed +**45 minutes against an expected 6h30**, and the reporter's capture says why: `10 00`, `10 01`, +`10 02` and `16 00` all went out inside **176 ms**, with `10 02` landing *between* two of today's +sleep packets. Four `0x11` packets arrived in total — three consecutive 15-minute blocks for today +(01:15–02:00, exactly the 45 minutes displayed) and one from the previous day. A second request +mid-stream truncates the first, so the window's days were cutting each other off. + +`JringHistorySync` is the fix: one day at a time, the next request held until the current day's +stream goes quiet. Per day, `0x10/offset` → settle → `0x16/offset` → next day, which is the vendor's +own chain (`getDataByDay(1, day)` then `(2, day)`, advancing on that day's sync-end). + +- **The activity/sleep leg is time-settled, not reply-driven, and that is forced.** Nothing in the + `0x10`/`0x11` wire format marks the end of a day — both are bare runs of 15 one-minute samples. + JYouPro is in the same position and does the same thing: a **2000 ms** idle timer reset on each + reply, which is where `settleMs`' default comes from. The **heart-rate** leg does have a real end + marker (`0x16` subtype `0xFF` → `HistorySyncFinished`) and chains on it. +- **`LuckRingHistorySync` is the same machine for the same reason** — port from it rather than + inventing a third one. The jring version adds the second (HR) leg per day. +- **A re-entrant `start()` is declined, and the caller is told.** `runStartup` is also the + ~30-minute background sync, so a pass landing mid-backfill is normal; restarting would abandon + the in-flight day mid-stream, which is the truncation itself. `start` returns whether it began a + pass so `historyBackfilled` is only spent by a pass that ran — otherwise a connection can lose + its backfill to a no-op and never ask for the older days again. +- **A day that answers nothing still gets its HR request.** `16 00` used to be unconditional and it + is the request that always worked; a silent `0x10` must not take today's HR down with it. +- **The pass is dropped on disconnect** (`JringDriver.connectionDidEnd`). The writer outlives the + driver, so a timer firing after the link dropped would land a stray `0x10` in the *next* + connection — the same truncation, arriving from a connection that has already ended. +- **Timers are epoch-guarded** (`JringHistorySync.armTimer`). `Job.cancel()` does not retract a + coroutine already past its `delay` and waiting on the monitor, and a stale settle firing into a + live stream would re-create the bug from the inside. +- The one remaining divergence from the vendor is order: it counts **down** to today, we lead with + today — the day the user opened the app to see, and the one the reporter's prototype validated. +- **Don't generalise this to other families.** `YCBTHistoryTransfer` has a terminal block with a + CRC and a YCBT owner read the same history twice back to back byte-identically (11 frames, + 1916 bytes) while this SR08 was truncating. Different transports, different failure modes. ## Colmi R11 (CRP "Da Rings") — diagnose from the capture, and decode wear state before blaming code diff --git a/app/src/main/java/com/pulseloop/ring/JringDriver.kt b/app/src/main/java/com/pulseloop/ring/JringDriver.kt index 558b4086..4b7c9654 100644 --- a/app/src/main/java/com/pulseloop/ring/JringDriver.kt +++ b/app/src/main/java/com/pulseloop/ring/JringDriver.kt @@ -60,7 +60,15 @@ object JringCoordinator : WearableCoordinator { * Ported from [JringDriver] in JringDriver.swift. * Thin wrapper over RingDecoder/RingEncoder for jring devices. */ -class JringDriver(private val writer: RingCommandWriter) : WearableDriver { +class JringDriver( + private val writer: RingCommandWriter, + /** The history pager. Owned here because only the driver sees whole frames ([ingest]); handed + * to the engine so `runStartup` can seed the day window. A fresh driver is built per + * connection (`RingBLEClient.installDriver` calls `coordinator.makeDriver` on every connect), + * so nothing here needs a reconnect *reset* — but its timers do need [connectionDidEnd]. + * Injectable so a test can run a whole pass on millisecond timers. */ + private val historySync: JringHistorySync = JringHistorySync(writer), +) : WearableDriver { /** One clock per connection, shared by the decoder and the sync engine: the engine latches * the UTC offset when it sends 0x01, and the decoder subtracts that same offset off every * ring-stamped history timestamp. See [JringClock]. */ @@ -75,19 +83,42 @@ class JringDriver(private val writer: RingCommandWriter) : WearableDriver { override fun frame(command: ByteArray) = command // jring: already 20 bytes, no checksum - override fun ingest(data: ByteArray, from: String): List = - decoder.decode(data) + override fun ingest(data: ByteArray, from: String): List { + // The pager settles on whole frames, so it is fed here rather than from the engine's + // decoded-event stream: one 0x10 frame fans out into 15 ActivityBucket events, which says + // nothing about how many frames are still coming. + historySync.noteFrame(data) + return decoder.decode(data) + } + + override fun makeSyncEngine(): RingSyncEngine = JringSyncEngine(writer, clock, historySync) - override fun makeSyncEngine(): RingSyncEngine = JringSyncEngine(writer, clock) + /** + * Drop any in-flight history pass on disconnect. + * + * A fresh driver is built per connection, so no *state* needs resetting here — but the pager + * holds live timers, and [RingCommandWriter] outlives this driver. A settle firing after the + * link dropped would enqueue the next day's `0x10` into whatever connection comes next, + * landing a stray history request on top of that connection's own stream: the truncation of + * issue #73, arriving from a connection that has already ended. + */ + override fun connectionDidEnd() { + historySync.cancel() + } } /** * Ported from [JringSyncEngine] in JringSyncEngine.swift. - * Fire-and-forget sync engine for jring devices. + * Fire-and-forget sync engine for jring devices — with one exception: history goes through + * [JringHistorySync], because this ring answers one history request at a time (issue #73). */ class JringSyncEngine( private val writer: RingCommandWriter?, private val clock: JringClock = JringClock(), + /** Owned by [JringDriver] in production so it can be fed whole frames; defaulted here so a + * caller holding only a writer (tests, and any future engine-only path) still gets a pager + * rather than an unpaced burst. */ + private val historySync: JringHistorySync = JringHistorySync(writer), ) : RingSyncEngine { override val supportsCombinedMeasurement: Boolean = true private val encoder = RingEncoder @@ -107,10 +138,11 @@ class JringSyncEngine( // had to initialise with the vendor app first. writer?.enqueue(encoder.makeAutomaticHeartRateCommand(enabled = true, cadenceMinutes = 30)) writer?.enqueue(encoder.makeBandFunctionCommand()) - for (dayOffset in historyDayOffsetsForThisPass()) { - writer?.enqueue(encoder.makeHistoryQueryCommand(dayOffset = dayOffset)) - } - writer?.enqueue(encoder.makeHistoryMeasurementQueryCommand()) + // History is paged rather than enqueued: [JringHistorySync] sends one day's 0x10, holds + // the rest until that day's stream goes quiet, and asks for each day's 0x16 HR itself. + // The gate is only spent if a pass actually began — `start` declines while one is in + // flight, and a declined backfill must still be owed. + if (historySync.start(historyDayOffsetsForThisPass())) historyBackfilled = true } /** Whether this connection has already pulled the deep history window. A fresh engine is built @@ -136,23 +168,20 @@ class JringSyncEngine( * actually came for. That volume, not the nights, is why this window is deliberately shorter * than the CRP backfill's week. * - * **Known divergence from the vendor, deliberate.** JYouPro is strictly reply-driven: it asks - * for one day, waits for that day's sync-end callback, then asks for the next, counting *down* - * to today (`DupMainActivity.onGetMultipleSportData`). We enqueue the window's requests in one - * pass, newest first, because nothing here decodes a per-day sync-end to drive the next request - * from — and this is the shape the #73 reporter validated on an SR08, where requesting offsets - * 0, 1 and 2 separately produced 27 `0x11` sleep entries after `0x10/00`. If a ring is ever seen - * truncating a day's stream when the next request lands, the vendor's chain is the fix, and it - * needs a sync-end signal decoded first. + * **This is the window, not the schedule.** Which days to ask for is decided here; *when* each + * request goes out is [JringHistorySync]'s, one day at a time. The two were briefly the same + * thing and that was the second half of #73: enqueued together, the window's requests + * truncated each other on an SR08. * - * Re-syncing the same days is harmless: activity buckets upsert by timestamp with the day total - * recomputed from distinct buckets, and sleep reconciles one waking day at a time. + * **One divergence from the vendor remains, deliberate:** it counts *down* to today + * (`DupMainActivity.onGetMultipleSportData`), we lead with today. It is the day the user opened + * the app to see, so it is the day that should land first and the one that survives if a pass + * is cut short. Re-syncing the same days is harmless anyway: activity buckets upsert by + * timestamp with the day total recomputed from distinct buckets, and sleep reconciles one + * waking day at a time. */ - private fun historyDayOffsetsForThisPass(): List { - if (historyBackfilled) return listOf(0) - historyBackfilled = true - return (0 until JRING_BACKFILL_DAYS).toList() - } + private fun historyDayOffsetsForThisPass(): List = + if (historyBackfilled) listOf(0) else (0 until JRING_BACKFILL_DAYS).toList() override fun handle(event: RingDecodedEvent) { when (event) { diff --git a/app/src/main/java/com/pulseloop/ring/JringHistorySync.kt b/app/src/main/java/com/pulseloop/ring/JringHistorySync.kt new file mode 100644 index 00000000..f8809ba6 --- /dev/null +++ b/app/src/main/java/com/pulseloop/ring/JringHistorySync.kt @@ -0,0 +1,195 @@ +package com.pulseloop.ring + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch + +/** + * The jring history pager: one day at a time, and the next request is held until the current + * day's stream goes quiet. + * + * **Why this exists (issue #73, second half).** Asking for the right day was necessary but not + * sufficient. With the window enqueued in one pass, an SR08 capture showed `10 00`, `10 01`, + * `10 02` and `16 00` all going out inside **176 ms**, with `10 02` landing *between* two of + * today's sleep packets — and only four `0x11` packets arriving in total, three of them today's. + * The night displayed as 45 minutes against an expected 6h30. The ring answers one history + * request at a time; a second request mid-stream truncates the first. `JringSyncEngine` used to + * record that the vendor's reply-driven chain would be the fix "if a ring is ever seen truncating + * a day's stream when the next request lands" — this is that ring. + * + * **Why time-settled rather than reply-driven for activity/sleep.** The vendor chains on a + * per-day sync-end callback (`DupMainActivity.onGetMultipleSportData`), but nothing in the + * `0x10`/`0x11` wire format marks the end of a day: `0x10` is a bare run of 15× 1-minute buckets + * and `0x11` a bare run of 15× 1-minute sleep stages (see `RingDecoder.decodeActivityHistory` / + * `decodeSleepTimeline`). So this leg advances when the stream settles, which is what JYouPro + * itself does — it resets a 2000 ms idle timer on each `0x10`/`0x11` reply, the same constant + * [settleMs] defaults to. [LuckRingHistorySync] is the same shape for the same reason and this + * is a port of it, extended with a second leg per day. + * + * **Heart rate does have a real end marker**, so its leg doesn't guess: `0x16` subtype `0xFF` + * decodes to [RingDecodedEvent.HistorySyncFinished] and advances the day immediately. The + * settle/stall timers are only its fallback. + * + * Per day: `0x10/offset` → the day's activity+sleep stream → settles → `0x16/offset` → the day's + * HR stream → `0xFF` (or settles) → next day. A day that answers nothing at all still gets its HR + * request, because `16 00` was previously sent unconditionally and today's HR must not become + * collateral of a silent `0x10`. + * + * Replays are safe, so a timer that fires early costs a re-request and nothing else: activity + * buckets upsert by timestamp with the day total recomputed from distinct buckets, and sleep + * reconciles one waking day at a time. + */ +class JringHistorySync( + private val writer: RingCommandWriter?, + /** Quiet period that ends a leg. JYouPro's own idle timer between `0x10`/`0x11` replies. */ + private val settleMs: Long = 2_000, + /** Bound on a leg that never answers at all. Comfortably under the ring's ~20 s idle + * disconnect, so a stalled day can't cost the connection. */ + private val stallMs: Long = 6_000, +) { + private val encoder = RingEncoder + private val scope = CoroutineScope(Dispatchers.Default + SupervisorJob()) + + /** The two legs of one day, in the order the vendor asks for them. */ + private enum class Leg { ACTIVITY, HEART_RATE } + + private var queue = mutableListOf() + private var currentDay: Int? = null + private var leg: Leg = Leg.ACTIVITY + private var settleJob: Job? = null + private var stallJob: Job? = null + /** Invalidates timers that fired for a leg which has since moved on — see [armTimer]. */ + private var epoch: Long = 0 + + @get:Synchronized + val isRunning: Boolean get() = currentDay != null + + /** + * Seed the queue and request the first day. Returns whether this call began a pass. + * + * A pass already in flight wins, and the caller is told so: `runStartup` is also the + * ~30-minute background sync (and `refresh()`/`querySleep()` route through it), so a second + * pass landing mid-backfill is the normal case, not an error — and restarting would abandon + * the in-flight day mid-stream, which is the very truncation this class exists to prevent. + * The `false` is what keeps `JringSyncEngine`'s once-per-connection backfill gate from being + * consumed by a pass that never ran. + */ + @Synchronized + fun start(dayOffsets: List): Boolean { + if (isRunning || dayOffsets.isEmpty()) return false + queue = dayOffsets.toMutableList() + advance() + return true + } + + /** Abandon any in-flight pass (disconnect / teardown). A fresh driver — and so a fresh pager — + * is built per connection, so this is only reached mid-connection. */ + @Synchronized + fun cancel() { + cancelTimers() + currentDay = null + queue.clear() + } + + /** + * Called by the driver for every inbound history frame, so the pager can tell a live stream + * from a finished one. Frames for a leg that isn't in flight are ignored. + */ + @Synchronized + fun noteFrame(data: ByteArray) { + if (!isRunning || data.isEmpty()) return + when (data[0].toInt() and 0xFF) { + OPCODE_ACTIVITY, OPCODE_SLEEP -> if (leg == Leg.ACTIVITY) restartSettle() + OPCODE_HEART_RATE -> { + if (leg != Leg.HEART_RATE) return + // The one genuine end-of-stream signal in this protocol — take it rather than + // waiting out the settle window. + if (data.size >= 2 && (data[1].toInt() and 0xFF) == HR_SYNC_FINISHED) advance() + else restartSettle() + } + } + } + + // MARK: - Driving the queue + + /** Move to the next day, or finish. Always entered with the lock held. */ + private fun advance() { + cancelTimers() + if (queue.isEmpty()) { + currentDay = null + return + } + val next = queue.removeAt(0) + currentDay = next + leg = Leg.ACTIVITY + writer?.enqueue(encoder.makeHistoryQueryCommand(dayOffset = next)) + armStall() + } + + /** End the activity/sleep leg and ask for the same day's heart rate, as the vendor does + * (`getDataByDay(2, day)`). Always entered with the lock held. */ + private fun beginHeartRateLeg() { + val day = currentDay ?: return + cancelTimers() + leg = Leg.HEART_RATE + writer?.enqueue(encoder.makeHistoryMeasurementQueryCommand(dayOffset = day)) + armStall() + } + + /** A leg's stream went quiet: hand over to the next leg, or the next day. */ + private fun legSettled() { + when (leg) { + Leg.ACTIVITY -> beginHeartRateLeg() + Leg.HEART_RATE -> advance() + } + } + + // MARK: - Timers + + /** Data arrived: the stall no longer applies, and the quiet period starts over. */ + private fun restartSettle() { + cancelTimers() + settleJob = armTimer(settleMs) + } + + /** Nothing has answered this leg yet. Fires once, then moves on. */ + private fun armStall() { + stallJob = armTimer(stallMs) + } + + /** + * A timer that can't act on a leg it no longer belongs to. + * + * `Job.cancel()` doesn't retract a coroutine that has already left its `delay` and is waiting + * on this monitor, so the epoch — bumped by every [cancelTimers] — is what actually decides + * whether a fired timer is still current. Without it a settle that lost the race to an + * arriving frame would advance the day out from under a live stream: the truncation this + * class exists to prevent, reintroduced from the inside. + */ + private fun armTimer(delayMs: Long): Job { + val firedFor = epoch + return scope.launch { + delay(delayMs) + synchronized(this@JringHistorySync) { + if (epoch == firedFor) legSettled() + } + } + } + + private fun cancelTimers() { + epoch++ + settleJob?.cancel(); settleJob = null + stallJob?.cancel(); stallJob = null + } + + companion object { + private const val OPCODE_ACTIVITY = 0x10 + private const val OPCODE_SLEEP = 0x11 + private const val OPCODE_HEART_RATE = 0x16 + /** `0x16` subtype `0xFF` — "sync finished" (`RingDecoder.decodeHeartRateHistory`). */ + private const val HR_SYNC_FINISHED = 0xFF + } +} diff --git a/app/src/main/java/com/pulseloop/ring/RingEncoder.kt b/app/src/main/java/com/pulseloop/ring/RingEncoder.kt index 16adfb73..af5a879f 100644 --- a/app/src/main/java/com/pulseloop/ring/RingEncoder.kt +++ b/app/src/main/java/com/pulseloop/ring/RingEncoder.kt @@ -55,7 +55,23 @@ object RingEncoder { cmd[1] = dayOffset.coerceIn(0, 27).toByte() return cmd } - fun makeHistoryMeasurementQueryCommand(): ByteArray = hexToBytes("1600000000000000000000000000000000000000") + /** + * Request heart-rate history for **one** day (0x16), answered as the multi-packet stream + * `RingDecoder.decodeHeartRateHistory` unpacks (`0xF0` header … `0xFF` sync finished). + * + * byte[1] is a day offset on the same footing as [makeHistoryQueryCommand]'s: the vendor + * reaches both through `getDataByDay(int type, int day)`, which picks the opcode from *type* + * (`1` -> `0x10`, `2` -> `0x16`) and writes *day* into `bArr[1]` either way. This used to be a + * fixed `16 00`, which is why heart rate kept working while sleep did not (issue #73) — and + * why it now takes an offset, so [JringHistorySync] can chain each backfilled day's HR behind + * that day's activity/sleep the way the vendor does. + */ + fun makeHistoryMeasurementQueryCommand(dayOffset: Int = 0): ByteArray { + val cmd = ByteArray(20) + cmd[0] = 0x16 + cmd[1] = dayOffset.coerceIn(0, 27).toByte() + return cmd + } fun makeHeartRateStartCommand(): ByteArray = hexToBytes("14b4000000000000000000000000000000000000") fun makeHeartRateStopCommand(): ByteArray = hexToBytes("1500000000000000000000000000000000000000") /** diff --git a/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt b/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt index db989eb9..65986c69 100644 --- a/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt +++ b/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt @@ -1,6 +1,9 @@ package com.pulseloop.ring +import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test @@ -12,16 +15,27 @@ class ExistingFamilyRefreshContractTest { } } + /** A jring engine whose pager runs on millisecond timers, so a whole pass drains inside a + * test. The pager holds each day's request until the previous day's stream goes quiet + * (issue #73), so these tests must let it finish before reading the writer — the *window* + * is what they assert, and [JringHistorySyncTest] owns the *schedule*. */ + private fun fastJring(w: FakeWriter) = + JringSyncEngine(w, historySync = JringHistorySync(w, settleMs = 10, stallMs = 10)) + + /** Long enough for a three-day pass (six legs) to drain with no ring answering. */ + private suspend fun drain() = delay(400) + @Test - fun `Jring refresh and query sleep retain startup behavior`() { + fun `Jring refresh and query sleep retain startup behavior`() = runBlocking { // Each capture builds a fresh engine, i.e. a fresh connection — so all three take the // first-pass branch and must still agree. See the backfill test below for the warm case. - val startup = capture { JringSyncEngine(it).runStartup() } - val refresh = capture { JringSyncEngine(it).refresh() } - val sleep = capture { JringSyncEngine(it).querySleep() } + val startup = FakeWriter().also { fastJring(it).runStartup() } + val refresh = FakeWriter().also { fastJring(it).refresh() } + val sleep = FakeWriter().also { fastJring(it).querySleep() } + drain() - assertEquals(startup, refresh) - assertEquals(startup, sleep) + assertEquals(startup.sent.map { it.toList() }, refresh.sent.map { it.toList() }) + assertEquals(startup.sent.map { it.toList() }, sleep.sent.map { it.toList() }) } /** Byte 1 of each `0x10` history query: the **day offset** it asks for — `0` today, `1` @@ -29,67 +43,115 @@ class ExistingFamilyRefreshContractTest { private fun historyDayOffsets(sent: List): List = sent.filter { it[0].toInt() == 0x10 }.map { it[1].toInt() } + /** The same for the `0x16` heart-rate history request, which takes the same offset. */ + private fun heartRateDayOffsets(sent: List): List = + sent.filter { it[0].toInt() == 0x16 }.map { it[1].toInt() } + @Test - fun `Jring always asks for today, on every pass`() { + fun `Jring always asks for today, on every pass`() = runBlocking { // Issue #73, and the whole bug: byte 1 is a day offset, so the old count reading asked // `0x10/03` (the day before last) on the first pass and `0x10/01` (yesterday) after it. // Today was never requested by any pass, and `0x10` is the only source of sleep — so on an // SR08 last night's sleep could not arrive at all, however many times the app synced. val w = FakeWriter() - val engine = JringSyncEngine(w) + val engine = fastJring(w) engine.runStartup() + drain() assertTrue("the first pass must ask for today", 0 in historyDayOffsets(w.sent)) w.sent.clear() - engine.runStartup() - engine.refresh() // routes through runStartup - engine.querySleep() // ditto + engine.runStartup(); drain() + engine.refresh(); drain() // routes through runStartup + engine.querySleep(); drain() // ditto assertEquals("every later pass asks for today and nothing else", listOf(0, 0, 0), historyDayOffsets(w.sent)) } @Test - fun `Jring pulls a deeper history window once per connection, then today only`() { + fun `Jring pulls a deeper history window once per connection, then today only`() = runBlocking { // Issue #43. A single-day request means stored history can only grow one night at a time // from install and never recovers what the ring already holds. But runStartup is also the // ~30-minute background sync, so the deep window must NOT repeat: 0x10 returns activity as // well as sleep, roughly 96 packets per extra day. val w = FakeWriter() - val engine = JringSyncEngine(w) + val engine = fastJring(w) engine.runStartup() + drain() assertEquals(listOf(0, 1, 2), historyDayOffsets(w.sent)) w.sent.clear() engine.runStartup() + drain() assertEquals(listOf(0), historyDayOffsets(w.sent)) // A new connection builds a new engine, which backfills again. val reconnected = FakeWriter() - JringSyncEngine(reconnected).runStartup() + fastJring(reconnected).runStartup() + drain() assertEquals(listOf(0, 1, 2), historyDayOffsets(reconnected.sent)) } @Test - fun `the Jring backfill asks newest day first`() { + fun `a declined backfill is still owed`() = runBlocking { + // The once-per-connection gate is spent by a pass that *ran*. runStartup is also the + // ~30-minute background sync, so a second pass landing while the first is still draining + // is normal; if that no-op consumed the gate, a connection could lose its backfill + // entirely and never ask for the older days again. + val w = FakeWriter() + val engine = JringSyncEngine(w, historySync = JringHistorySync(w, settleMs = 10, stallMs = 10_000)) + + engine.runStartup() // day 0 in flight, and it will not answer + engine.runStartup() // declined: a pass is already running + assertEquals("the second pass adds nothing", listOf(0), historyDayOffsets(w.sent)) + + val resumed = FakeWriter() + fastJring(resumed).runStartup() + drain() + assertEquals("the window is still the full backfill", listOf(0, 1, 2), historyDayOffsets(resumed.sent)) + } + + @Test + fun `the Jring backfill asks newest day first`() = runBlocking { // Today leads the window: it is the day the user opened the app to see, and if a ring ever // truncates the run when the next request lands, the day that survives is the one that // matters. The vendor counts *down* to today instead — see historyDayOffsetsForThisPass - // for why we diverge and what would make the vendor's chain necessary. + // for why we diverge. val w = FakeWriter() - JringSyncEngine(w).runStartup() + fastJring(w).runStartup() + drain() val offsets = historyDayOffsets(w.sent) assertEquals("today must be requested first", 0, offsets.first()) assertEquals("offsets must be distinct and ascending", offsets.sorted().distinct(), offsets) } @Test - fun `the Jring backfill window stays inside what the command encodes`() { + fun `each backfilled day gets its own heart rate request, after that day's history`() = runBlocking { + // The vendor reaches both commands through getDataByDay(type, day), so 0x16 takes the same + // offset as 0x10. Before the pager this was a single hardcoded `16 00` fired alongside the + // whole window — which is also the request seen landing on top of the SR08's sleep stream. + val w = FakeWriter() + fastJring(w).runStartup() + drain() + assertEquals(listOf(0, 1, 2), heartRateDayOffsets(w.sent)) + + val history = w.sent.filter { it[0].toInt() == 0x10 || it[0].toInt() == 0x16 } + .map { (it[0].toInt() and 0xFF) to it[1].toInt() } + assertEquals( + "each day is asked for as activity-then-HR, one day at a time", + listOf(0x10 to 0, 0x16 to 0, 0x10 to 1, 0x16 to 1, 0x10 to 2, 0x16 to 2), + history, + ) + } + + @Test + fun `the Jring backfill window stays inside what the command encodes`() = runBlocking { // makeHistoryQueryCommand coerces to 0..27; an offset above that would silently truncate // and the request would ask for a different day than the caller meant. val w = FakeWriter() - JringSyncEngine(w).runStartup() - val offsets = historyDayOffsets(w.sent) + fastJring(w).runStartup() + drain() + val offsets = historyDayOffsets(w.sent) + heartRateDayOffsets(w.sent) assertTrue("every requested offset $offsets must survive the 0..27 coerce", offsets.all { it in 0..27 }) } @@ -103,12 +165,6 @@ class ExistingFamilyRefreshContractTest { assertEquals(startup, sleep) } - private fun capture(action: (FakeWriter) -> Unit): List> { - val writer = FakeWriter() - action(writer) - return writer.sent.map(ByteArray::toList) - } - private fun captureColmi(action: (ColmiSyncEngine) -> Unit): List> { val writer = FakeWriter() val engine = ColmiSyncEngine(writer, ColmiDecoder) diff --git a/app/src/test/java/com/pulseloop/ring/JringHistorySyncTest.kt b/app/src/test/java/com/pulseloop/ring/JringHistorySyncTest.kt new file mode 100644 index 00000000..88260543 --- /dev/null +++ b/app/src/test/java/com/pulseloop/ring/JringHistorySyncTest.kt @@ -0,0 +1,247 @@ +package com.pulseloop.ring + +import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking +import org.junit.Assert.* +import org.junit.Test + +/** + * The jring history pager (issue #73, second half). + * + * The bug this guards against is not "the wrong day was requested" — that was the first half, and + * [ExistingFamilyRefreshContractTest] covers it. It is that **the requests were sent together**: + * an SR08 capture showed `10 00`, `10 01`, `10 02` and `16 00` inside 176 ms, with `10 02` landing + * between two of today's sleep packets, and only four `0x11` packets arriving in all. So the + * discriminating assertion here is the negative one — that nothing further is sent *while a stream + * is still arriving*. + * + * Uses real (short) millisecond delays: [JringHistorySync] schedules on a real dispatcher, matching + * the convention in [LuckRingHistorySyncTest] and [YCBTHistoryTransfer]. + */ +class JringHistorySyncTest { + private class FakeWriter : RingCommandWriter { + val sent = mutableListOf() + override fun enqueue(command: ByteArray) { sent += command.copyOf() } + + /** Each history request as `opcode to dayOffset`, in the order it went out. */ + val requests: List> + get() = sent.map { (it[0].toInt() and 0xFF) to (it[1].toInt() and 0xFF) } + } + + private fun activityFrame() = ByteArray(20).also { it[0] = 0x10 } + private fun sleepFrame() = ByteArray(20).also { it[0] = 0x11 } + private fun hrFrame() = ByteArray(20).also { it[0] = 0x16; it[1] = 0xA0.toByte() } + private fun hrFinishedFrame() = ByteArray(20).also { it[0] = 0x16; it[1] = 0xFF.toByte() } + + private fun makeSync(writer: FakeWriter, settleMs: Long = 50, stallMs: Long = 5_000) = + JringHistorySync(writer, settleMs = settleMs, stallMs = stallMs) + + /** Only the history requests: a pass driven through [JringDriver] carries the connect + * handshake (status, time sync, locale, user info, auto-HR, band function) as well. */ + private fun historyRequests(writer: FakeWriter): List> = + writer.requests.filter { it.first == 0x10 || it.first == 0x16 } + + @Test + fun `a day's next request is held while its stream is still arriving`() = runBlocking { + // THE regression test for the RC1 capture. Sleep packets keep arriving past what would + // have been the settle deadline; nothing may be sent on top of them. + val writer = FakeWriter() + val sync = makeSync(writer, settleMs = 200) + + sync.start(listOf(0, 1, 2)) + assertEquals("only today is requested up front", listOf(0x10 to 0), writer.requests) + + repeat(5) { + sync.noteFrame(sleepFrame()) + delay(50) // well inside the settle window: the stream is live, never quiet + } + assertEquals( + "no second request while today's sleep is still streaming", + listOf(0x10 to 0), + writer.requests, + ) + + delay(500) + assertEquals( + "once the stream goes quiet, today's HR follows — and only then", + listOf(0x10 to 0, 0x16 to 0), + writer.requests, + ) + sync.cancel() + } + + @Test + fun `each day is one activity request then that day's heart rate`() = runBlocking { + val writer = FakeWriter() + val sync = makeSync(writer, settleMs = 60) + + sync.start(listOf(0, 1)) + + sync.noteFrame(activityFrame()) + sync.noteFrame(sleepFrame()) + delay(150) + assertEquals(listOf(0x10 to 0, 0x16 to 0), writer.requests) + + sync.noteFrame(hrFinishedFrame()) + delay(50) + assertEquals( + "the next day only starts after the previous day's HR finished", + listOf(0x10 to 0, 0x16 to 0, 0x10 to 1), + writer.requests, + ) + + sync.noteFrame(activityFrame()) + delay(150) + sync.noteFrame(hrFinishedFrame()) + delay(50) + assertEquals(listOf(0x10 to 0, 0x16 to 0, 0x10 to 1, 0x16 to 1), writer.requests) + assertFalse("the queue drained", sync.isRunning) + } + + @Test + fun `heart rate's own end marker advances the day without waiting out the timer`() = runBlocking { + // 0x16/0xFF is the one real end-of-stream signal in this protocol (RingDecoder decodes it + // to HistorySyncFinished). The settle window here is a full second, so an advance that + // lands 50 ms after the marker can only have come from the marker. + val writer = FakeWriter() + val sync = makeSync(writer, settleMs = 1_000, stallMs = 30_000) + + sync.start(listOf(0, 1)) + sync.noteFrame(activityFrame()) + delay(1_200) // today's activity/sleep settles into today's HR + assertEquals(listOf(0x10 to 0, 0x16 to 0), writer.requests) + + sync.noteFrame(hrFrame()) // data: re-arms the 1 s settle + sync.noteFrame(hrFinishedFrame()) // marker: must not wait for it + delay(50) + assertEquals(listOf(0x10 to 0, 0x16 to 0, 0x10 to 1), writer.requests) + sync.cancel() + } + + @Test + fun `a day that answers nothing still gets its heart rate request, then moves on`() = runBlocking { + // `16 00` used to be sent unconditionally, and it is the request that always worked. A + // silent 0x10 must not take today's HR down with it. + val writer = FakeWriter() + val sync = makeSync(writer, settleMs = 5_000, stallMs = 40) + + sync.start(listOf(0)) + delay(200) + assertEquals(listOf(0x10 to 0, 0x16 to 0), writer.requests) + assertFalse("a day nothing answers cannot hang the queue", sync.isRunning) + } + + @Test + fun `a re-entrant start is declined, not merged`() = runBlocking { + // runStartup is also the ~30-minute background sync, so this lands mid-backfill in normal + // use. Restarting would abandon the in-flight day mid-stream — the exact truncation. + val writer = FakeWriter() + val sync = makeSync(writer, settleMs = 5_000, stallMs = 5_000) + + assertTrue("the first pass begins", sync.start(listOf(0, 1, 2))) + assertFalse("a pass already in flight wins", sync.start(listOf(0))) + assertEquals(listOf(0x10 to 0), writer.requests) + + sync.cancel() + delay(50) + assertTrue("a pass may begin again once the previous one is done", sync.start(listOf(0))) + sync.cancel() + } + + @Test + fun `cancel halts the pass where it stands`() = runBlocking { + val writer = FakeWriter() + val sync = makeSync(writer, settleMs = 30, stallMs = 30) + + sync.start(listOf(0, 1, 2)) + sync.cancel() + delay(200) + assertFalse(sync.isRunning) + assertEquals("no further day is requested after cancel", listOf(0x10 to 0), writer.requests) + } + + @Test + fun `a frame arriving as the settle fires cannot advance the day twice`() = runBlocking { + // The epoch guard in armTimer: a settle coroutine already past its delay and waiting on + // the monitor must not act once an arriving frame has re-armed the leg. + val writer = FakeWriter() + val sync = makeSync(writer, settleMs = 20, stallMs = 5_000) + + sync.start(listOf(0, 1)) + repeat(20) { + sync.noteFrame(sleepFrame()) + delay(2) + } + delay(200) + assertEquals( + "exactly one HR request for today, however the timers raced", + listOf(0x10 to 0, 0x16 to 0), + writer.requests, + ) + sync.cancel() + } + + // MARK: - Wiring: the driver is what feeds and ends a pass + + @Test + fun `the driver feeds inbound frames to the pager`() = runBlocking { + // The wiring that makes all of the above reach production: JringDriver.ingest calls + // noteFrame. Fed through the real driver rather than the pager directly, because an + // engine-level test cannot see a frame at all. + val writer = FakeWriter() + val driver = JringDriver(writer, makeSync(writer, settleMs = 150, stallMs = 30_000)) + driver.makeSyncEngine().runStartup() + assertEquals(listOf(0x10 to 0), historyRequests(writer)) + + repeat(4) { + driver.ingest(sleepFrame(), RingUUIDs.NOTIFY) + delay(40) + } + assertEquals( + "a stream arriving through ingest holds the next request", + listOf(0x10 to 0), + historyRequests(writer), + ) + + delay(300) + assertEquals(listOf(0x10 to 0, 0x16 to 0), historyRequests(writer)) + driver.connectionDidEnd() + } + + @Test + fun `a disconnect drops the in-flight pass`() = runBlocking { + // The writer outlives the driver, so a timer surviving the disconnect would land a stray + // 0x10 in the *next* connection — issue #73's truncation, from a connection that ended. + val writer = FakeWriter() + val driver = JringDriver(writer, makeSync(writer, settleMs = 10, stallMs = 10)) + driver.makeSyncEngine().runStartup() + assertEquals(listOf(0x10 to 0), historyRequests(writer)) + + driver.connectionDidEnd() + delay(300) + assertEquals( + "no further history request survives the disconnect", + listOf(0x10 to 0), + historyRequests(writer), + ) + } + + @Test + fun `frames for a leg that is not in flight are ignored`() = runBlocking { + val writer = FakeWriter() + val sync = makeSync(writer, settleMs = 40, stallMs = 5_000) + + sync.noteFrame(sleepFrame()) // before any pass + assertEquals(emptyList>(), writer.requests) + + sync.start(listOf(0)) + sync.noteFrame(hrFrame()) // HR frame during the activity leg: not this leg's business + delay(150) + assertEquals( + "a stray HR frame does not settle the activity leg early", + listOf(0x10 to 0), + writer.requests, + ) + sync.cancel() + } +} From 9c017e3ac191fcf17a5dec9dc35b96112dfed85a Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Fri, 18 Sep 2026 04:16:59 -0700 Subject: [PATCH 5/7] docs(agents): the diagnostics export carries no database rows on purpose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #74's "Next" section asked the reporter for "the timestamps of the heart-rate rows stored around that minute, and whether they carry the spot flag". DiagnosticsExporter emits app info, device info, logs, raw packets, crashes and logcat — no database rows — so that question could not be answered from an export, and the ask went out twice before anyone checked the format. The reporter supplied the reason it should stay that way, and it is better than "the format doesn't happen to have it": a diagnostics file people paste into public issues is not where health readings should end up. maskPacketHex already enforces that one layer down, masking health payloads out of the raw frames, so a measurements section would undo in clear exactly what the masking exists to prevent. Recorded next to the masking rules, because the next person to want stored-row detail will be reading that section. --- AGENTS.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 72e5f8d0..4863c428 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -783,6 +783,18 @@ values intact**, because an undecoded frame fell through to `command_ack`, which `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 export carries no database rows, and that is deliberate — don't add them, and don't ask a +reporter for stored-row detail through it.** `DiagnosticsExporter` emits app info, device info, +logs, raw packets, crashes and logcat, and nothing else. Issue #74's "Next" section asked a reporter +for "the timestamps of the heart-rate rows stored around that minute, and whether they carry the +spot flag" — a question the format cannot answer, so the ask was made twice and answered neither +time. The reporter (@Albabit, #74) gave the reason the section should stay absent: **a diagnostics +file people paste into public issues is not where health readings should end up.** That is the same +principle `maskPacketHex` already enforces one layer down, so a measurements section would undo +deliberately, in clear, exactly what the masking exists to prevent. When you need to know what the +app *stored*, ask what the user sees on screen — for "is the user shown a bad reading?" that is also +the better evidence, since it answers the question the issue is actually about. + **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: From 990fff9f35d6c7b8bd8773286aee9f7f5bd8799a Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 21 Sep 2026 14:47:51 -0700 Subject: [PATCH 6/7] fix(jring): only a full 20-byte packet re-arms the settle window (#73) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RingPacket.fromData rejects anything that isn't exactly PACKET_SIZE, and the decoder with it — so noteFrame must apply the same contract, or a fragment could hold a day's stream open on data the app will never read. --- .../com/pulseloop/ring/JringHistorySync.kt | 6 +++++- .../pulseloop/ring/JringHistorySyncTest.kt | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/pulseloop/ring/JringHistorySync.kt b/app/src/main/java/com/pulseloop/ring/JringHistorySync.kt index f8809ba6..da0e3773 100644 --- a/app/src/main/java/com/pulseloop/ring/JringHistorySync.kt +++ b/app/src/main/java/com/pulseloop/ring/JringHistorySync.kt @@ -97,10 +97,14 @@ class JringHistorySync( /** * Called by the driver for every inbound history frame, so the pager can tell a live stream * from a finished one. Frames for a leg that isn't in flight are ignored. + * + * The packet-length guard mirrors [RingPacket.fromData]: the jring wire contract is one fixed + * 20-byte packet per notify, and anything else is already rejected by the decoder. A fragment + * must not re-arm the settle window, or a leg stays "live" long past its stream. */ @Synchronized fun noteFrame(data: ByteArray) { - if (!isRunning || data.isEmpty()) return + if (!isRunning || data.size != RingPacket.PACKET_SIZE) return when (data[0].toInt() and 0xFF) { OPCODE_ACTIVITY, OPCODE_SLEEP -> if (leg == Leg.ACTIVITY) restartSettle() OPCODE_HEART_RATE -> { diff --git a/app/src/test/java/com/pulseloop/ring/JringHistorySyncTest.kt b/app/src/test/java/com/pulseloop/ring/JringHistorySyncTest.kt index 88260543..c7c33dd4 100644 --- a/app/src/test/java/com/pulseloop/ring/JringHistorySyncTest.kt +++ b/app/src/test/java/com/pulseloop/ring/JringHistorySyncTest.kt @@ -226,6 +226,25 @@ class JringHistorySyncTest { ) } + @Test + fun `a frame too short to be a packet does not hold the stream open`() = runBlocking { + // RingPacket.fromData rejects anything that isn't exactly 20 bytes, and the decoder with + // it — so the pager must not treat a fragment as a live frame either, or it re-arms the + // settle window on data the app will never read and the day stalls out to the full settle. + val writer = FakeWriter() + val sync = JringHistorySync(writer, settleMs = 5_000, stallMs = 100) + + sync.start(listOf(0)) + sync.noteFrame(ByteArray(19).also { it[0] = 0x11 }) // one byte short of a packet + delay(300) + assertEquals( + "the fragment is not a frame: the leg stalls out on schedule", + listOf(0x10 to 0, 0x16 to 0), + writer.requests, + ) + sync.cancel() + } + @Test fun `frames for a leg that is not in flight are ignored`() = runBlocking { val writer = FakeWriter() From 0f68113bc418cb835d44efbcf492e11caf04182e Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 21 Sep 2026 14:47:51 -0700 Subject: [PATCH 7/7] docs(hero): the DISCONNECT branch is a tripwire, not a resurrected dead button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment claimed the card rendered Disconnect as an enabled button wired to nothing — the claim the PR description already corrected: the branch is unreachable (the row only renders when action != DISCONNECT, and DISCONNECT is only ever produced for the connected state). A wrong comment here is how the #73 fix got deferred once already; make it match the corrected account. --- .../com/pulseloop/ui/components/DeviceHeroCard.kt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt index 7650edf8..abca86a9 100644 --- a/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt +++ b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt @@ -167,11 +167,14 @@ fun DeviceHeroCard( when (status.action) { DeviceHeroStatus.Action.CONNECT -> onConnect() DeviceHeroStatus.Action.SET_UP -> onSetUp() - // The card rendered "Disconnect" as an enabled button wired to - // nothing — the same silent no-op as issue #72's Connect, one - // action along. PENDING is the only state that should do nothing, - // and it is already inert via `actionEnabled`. + // Unreachable today: the row above only renders when action != + // DISCONNECT, and DeviceHeroStatus produces DISCONNECT only for + // the connected state — so this is a tripwire, not a fix (the PR + // description once claimed a dead button here; there never was + // one). Exhaustiveness needs the branch, and if the render + // condition is ever relaxed it must call through, not do nothing. DeviceHeroStatus.Action.DISCONNECT -> onDisconnect() + // Already inert: actionEnabled is false while a connect is in flight. DeviceHeroStatus.Action.PENDING -> {} } }