diff --git a/AGENTS.md b/AGENTS.md index 05f9e80..e12657b 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 @@ -98,16 +114,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 +195,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 1afef83..558b408 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/RingBLEClient.kt b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt index 101b0af..bb99666 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/ring/RingEncoder.kt b/app/src/main/java/com/pulseloop/ring/RingEncoder.kt index 0134da4..16adfb7 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 d69d149..9aae681 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 e662e1f..bd77828 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/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt index 7c4e19f..7650edf 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 60e6635..bfc9526 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 ea564f7..9c1141a 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") }, ) diff --git a/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt b/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt index 82c7032..db989eb 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