diff --git a/AGENTS.md b/AGENTS.md index 05f9e80..4863c42 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,74 @@ 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. +- **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 **Read this before changing anything in `CRP*` startup, sync, all-day-monitoring, or history code — @@ -699,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: diff --git a/app/src/main/java/com/pulseloop/ring/JringDriver.kt b/app/src/main/java/com/pulseloop/ring/JringDriver.kt index 1afef83..4b7c965 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) @@ -59,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]. */ @@ -74,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 @@ -106,8 +138,11 @@ 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())) - 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 @@ -116,30 +151,37 @@ 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. * - * 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. + * `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. * - * **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. + * **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. * - * 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. + * **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. + * + * **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 historyDaysForThisPass(): Int { - if (historyBackfilled) return 1 - historyBackfilled = true - return JRING_BACKFILL_DAYS - } + 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 0000000..da0e377 --- /dev/null +++ b/app/src/main/java/com/pulseloop/ring/JringHistorySync.kt @@ -0,0 +1,199 @@ +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. + * + * 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.size != RingPacket.PACKET_SIZE) 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/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..af5a879 100644 --- a/app/src/main/java/com/pulseloop/ring/RingEncoder.kt +++ b/app/src/main/java/com/pulseloop/ring/RingEncoder.kt @@ -29,18 +29,49 @@ 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 + } + /** + * 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 makeHistoryMeasurementQueryCommand(): ByteArray = hexToBytes("1600000000000000000000000000000000000000") fun makeHeartRateStartCommand(): ByteArray = hexToBytes("14b4000000000000000000000000000000000000") fun makeHeartRateStopCommand(): ByteArray = hexToBytes("1500000000000000000000000000000000000000") /** 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..abca86a 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,15 @@ fun DeviceHeroCard( when (status.action) { DeviceHeroStatus.Action.CONNECT -> onConnect() DeviceHeroStatus.Action.SET_UP -> onSetUp() - else -> {} + // 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 -> {} } } .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..65986c6 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,54 +15,144 @@ 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 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() } + /** 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`() = 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 = fastJring(w) + + engine.runStartup() + drain() + assertTrue("the first pass must ask for today", 0 in historyDayOffsets(w.sent)) + + w.sent.clear() + 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 one day per pass`() { + 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() - assertEquals(listOf(3), historyDays(w.sent)) + drain() + 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)) + drain() + 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)) + fastJring(reconnected).runStartup() + drain() + assertEquals(listOf(0, 1, 2), historyDayOffsets(reconnected.sent)) } @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. + 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() - JringSyncEngine(w).runStartup() - val requested = historyDays(w.sent).single() - assertTrue("backfill window $requested must survive the 0..27 coerce", requested in 1..27) + 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. + val w = FakeWriter() + 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 `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() + 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 }) } @Test @@ -72,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 0000000..c7c33dd --- /dev/null +++ b/app/src/test/java/com/pulseloop/ring/JringHistorySyncTest.kt @@ -0,0 +1,266 @@ +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 `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() + 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() + } +}