From e11be6e40ee10f6322c6f3e71bb5bd73fc3cdec1 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 3 Aug 2026 09:33:25 -0700 Subject: [PATCH 1/5] fix(sleep): stop deleting stored sleep history on every connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #43 (zaggash, Colmi R11 / Da Rings): "whenever a new night is over and registered, the previous are deleted and not available anymore." EventPersistenceSubscriber ran unscoped DELETEs on sleep_sessions and sleep_stage_blocks on every connect for every family except YCBT, then rebuilt from the ring. That assumed the ring could re-supply what was deleted. None can re-supply more than its own buffer, and the two smallest re-supply a single day: CRPSyncEngine sends queryHistorySleep(daysAgo = 0), and jring's syncWindowDays is 1. So each connect discarded every night older than the last one, and the next night arrived into an emptied table instead of joining the nights before it. Colmi survived only incidentally, by requesting a multi-day big-data range. The rebuild was never load-bearing. upsertSleepSessionAtomic already reconciles one waking day at a time and is documented as idempotent, and it already folds in legacy rows keyed to the wrong day — "include legacy rows keyed to the wrong day if they overlap this packet; reconciliation re-points their surviving blocks to the correct waking day" — which was the blanket clear's stated reason to exist. So the clear was doing nothing the per-day reconcile did not already do, except destroying days the ring would never send again. A connect now retires demo rows and nothing else, for every family. That makes preservesSleepOnConnect vacuous, so it is deleted rather than left as dead code with a passing test. isConnectTransition stays: it is what keeps a device-info reply from being read as a connection, and it still gates the demo clear. Behaviour worth stating explicitly: nothing bulk-deletes real sleep any more, so Forget followed by pairing a different ring now carries the previous ring's history over instead of silently wiping it. That seems right — the data is the user's, not the ring's — but if it should change it belongs on DeviceForgotten as a deliberate choice, not as a side effect of connecting. Noted in AGENTS.md. Not included: deepening the pull so a night is not lost when a sync straddles it. The CRP vendor exposes CRPHistoryDay.YESTERDAY = 1 and jring's makeHistoryQueryCommand accepts up to 27 days, but that is per-family protocol work and wants zaggash to confirm the R11 answers daysAgo = 1 before we rely on it. The delete is the data loss; this fix stops it on its own. 791 unit tests pass. Test coverage changed shape: the three preservesSleepOnConnect cases are gone with the function, replaced by one that asserts the property that now matters — re-syncing a night leaves the nights on either side of it intact. The end-to-end "connect does not delete rows" invariant still has no unit coverage; it needs a Room harness this suite does not have. --- AGENTS.md | 34 +++++---- .../service/EventPersistenceSubscriber.kt | 72 ++++++++----------- .../java/com/pulseloop/ring/CRPDecoderTest.kt | 7 +- .../service/EventPersistenceIdentityTest.kt | 65 +++++++++-------- 4 files changed, 88 insertions(+), 90 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index dfd9c4b..737d54c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,13 +50,28 @@ overridden. Don't widen it to "any driver whose services are missing." See `docs/qring-ble-adoption.md` §5a for the full history and the decompiled source references. -## Only the client's own connect may rebuild stored data +## Connecting must never delete stored history -**Read this before touching `EventPersistenceSubscriber`'s `DeviceStateChanged` branch, or before -adding a family to `preservesSleepOnConnect`.** +**Read this before touching `EventPersistenceSubscriber`'s `DeviceStateChanged` branch.** -`RingConnectionState.CONNECTED` arrives from two unrelated places, and only one of them means a -connection was established: +**No ring re-supplies more history than its own buffer holds, so the app's copy is the only durable +one.** A connect may retire *demo* rows and nothing else. This is not a style preference — it was a +data-loss bug twice, in two different shapes (issue #43, and the sync-pass variant before it). + +The original design deleted all sleep on connect and re-pulled it, carving YCBT out via +`preservesSleepOnConnect` because YCBT re-asserts CONNECTED mid-history. That premise was false for +everyone: CRP asks `queryHistorySleep(daysAgo = 0)` and jring `syncWindowDays = 1`, so "delete +everything and ask again" capped stored sleep at a single night — a new night replaced the previous +one instead of joining it. Both the carve-out and the rebuild are gone. What protects a re-synced +day now is `upsertSleepSessionAtomic`, which reconciles one waking day at a time, idempotently, and +re-points legacy mis-keyed blocks itself — the blanket clear's own stated justification. + +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, +it belongs on `DeviceForgotten` as a deliberate choice, not as a side effect of connecting. + +`RingConnectionState.CONNECTED` also arrives from two unrelated places, and only one is a real +transition: - `RingBLEClient`'s own connect event — always carries `deviceType` (`activeCoordinator` is set by `installDriver`, which runs before the CCCD write that gates CONNECTED). @@ -65,13 +80,8 @@ connection was established: status packets. `runStartup` re-sends them, and `runStartup` is also the ~30-minute background sync — so they recur for the whole life of a connection. -The CONNECTED branch clears and rebuilds (unscoped `DELETE FROM sleep_sessions` / -`sleep_stage_blocks` for families outside `preservesSleepOnConnect`). Ungated, every background sync -pass on jring or LuckRing wiped all stored sleep and depended on that same pass re-pulling it — -losing anything past the ring's retention when the pass was interrupted or came back empty. -`isConnectTransition(event.deviceType)` is the gate. **Don't remove it, and don't try to fix this -family-by-family** — `preservesSleepOnConnect` was an attempt at that, and the set of families that -re-assert CONNECTED turned out to be most of them. +`isConnectTransition(event.deviceType)` is that gate. Keep it: it is what stops a device-info reply +from being mistaken for a connection, and it kept the demo-clear from running all session long. Corollary for new protocol work: a reply that merely reports something about the device (firmware, serial, capabilities) is not a connection event. Give it its own `RingDecodedEvent` — as diff --git a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index a9603e3..be0cacb 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -80,26 +80,26 @@ class EventPersistenceSubscriber( val device = existing ?: DeviceEntity() val state = when (event.state) { RingConnectionState.CONNECTED -> { - // Only the *client's own* connect event is a connection transition, and it - // is the only CONNECTED that may run the destructive rebuild below. Decoders - // re-assert CONNECTED mid-session from ordinary device-info replies — jring - // `0x0C`, LuckRing dev-info, YCBT status packets — and those replies are - // re-sent by `runStartup`, which is also the ~30-minute background sync. Left - // ungated, each pass would wipe every stored sleep session and then depend on - // that same pass re-pulling it. See [isConnectTransition]. + // Connecting retires demo rows so a real ring's data replaces the seeded + // preview. It destroys nothing else. Gated to the client's own connect (see + // [isConnectTransition]) because decoders re-assert CONNECTED mid-session + // from ordinary device-info replies — jring `0x0C`, LuckRing dev-info, YCBT + // status packets — which `runStartup` re-sends on every sync pass. + // + // This 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 `syncWindowDays = 1` — 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 rebuild was never load-bearing: + // [upsertSleepSessionAtomic] reconciles one waking day at a time, + // idempotently, and re-points legacy mis-keyed blocks itself — which was the + // blanket clear's stated reason for existing. if (isConnectTransition(event.deviceType)) { db.measurementDao().clearDemo() db.activityDailyDao().clearDemo() - if (preservesSleepOnConnect(event.deviceType, existing?.deviceType)) { - // YCBT status packets re-emit CONNECTED while history is still arriving. - db.sleepStageBlockDao().clearDemo() - db.sleepSessionDao().clearDemo() - } else { - // Packet-based families rebuild sleep on connect. Clear blocks with - // sessions so legacy midnight-keyed blocks cannot contaminate a rebuild. - db.sleepStageBlockDao().clear() - db.sleepSessionDao().clear() - } + db.sleepStageBlockDao().clearDemo() + db.sleepSessionDao().clearDemo() } "CONNECTED" } @@ -308,13 +308,10 @@ class EventPersistenceSubscriber( // Stamp the version onto an existing row only — never create one, so a late reply // can't resurrect a forgotten ring (same rule as DeviceStateChanged above). // - // This is why the firmware string does NOT travel as DeviceStateChanged: that - // branch treats every CONNECTED as "a connection was just established" and, for - // families outside preservesSleepOnConnect (CRP among them), answers by clearing - // sleep_sessions + sleep_stage_blocks outright. CRPSyncEngine.runStartup re-queries - // firmware on every pass — including the ~30-minute background sync — so routing it - // through there would wipe all stored sleep on each pass and rely on the same pass - // re-pulling it, losing everything past the ring's 14-day retention if it didn't. + // The firmware string also does NOT travel as DeviceStateChanged: that event means + // the *connection state* changed, which a device-info reply doesn't, and + // CRPSyncEngine.runStartup re-queries firmware on every pass — including the + // ~30-minute background sync — so it would restate CONNECTED all session long. val device = db.deviceDao().currentReal() ?: return if (event.version == device.firmwareVersion) return // blank is gated in the bridge db.deviceDao().upsert(device.copy( @@ -470,8 +467,11 @@ class EventPersistenceSubscriber( * * Ported from iOS `SleepService.reconcileWakingDay`. Unlike iOS this emits no DerivedUpdateRow * change signal: Room's reactive Flows (SleepViewModel observes `recentFlow`) already refresh - * the UI on any sleep-table write, and sleep is cleared + rebuilt from the ring on every connect - * anyway — there is no unchanged-day re-sync to optimize away. + * the UI on any sleep-table write. + * + * This function's idempotence is now what protects stored history. Connecting no longer clears + * the sleep tables (issue #43), so a re-synced day arrives on top of rows that are already + * there and has to reconcile with them rather than repopulate an emptied table. * * Wrapped in one transaction: the body below deletes every existing row's blocks up front, then * re-upserts sessions and re-inserts blocks per segment one write at a time. Without a @@ -630,7 +630,7 @@ internal fun historyMeasurementId(kind: MeasurementKind, timestamp: Long): Strin /** * True when a `DeviceStateChanged(CONNECTED, …)` is a real connection transition rather than a - * mid-session re-assertion, and may therefore run the clear-and-rebuild in [EventPersistenceSubscriber]. + * mid-session re-assertion, and may therefore retire the demo rows in [EventPersistenceSubscriber]. * * Exactly two things publish a CONNECTED event, and `deviceType` separates them cleanly: * - `RingBLEClient`'s own connect always passes `deviceType = activeCoordinator.deviceType`, which @@ -639,23 +639,13 @@ internal fun historyMeasurementId(kind: MeasurementKind, timestamp: Long): Strin * `deviceType`. Those are ordinary device-info replies — jring `0x0C`, LuckRing dev-info, YCBT * status packets — re-sent by `runStartup` on every sync pass, not connection events. * - * Before this gate, each of those replies ran the rebuild, so a background sync pass on jring or - * LuckRing dropped every `sleep_sessions` / `sleep_stage_blocks` row (unscoped `DELETE`s) and relied - * on the same pass re-pulling them — losing anything past the ring's retention when it didn't. - * A per-family allowlist can't fix this: the families that re-assert CONNECTED are most of them. + * `preservesSleepOnConnect` used to live beside this, carving YCBT out of a clear-and-rebuild that + * every other family ran on connect. Both the carve-out and the rebuild are gone (issue #43): no + * ring re-supplies more sleep than its own buffer holds, so "delete everything and ask again" + * capped stored history at whatever the ring still had — one day, on CRP and jring. */ internal fun isConnectTransition(eventDeviceType: RingDeviceType?): Boolean = eventDeviceType != null -internal fun preservesSleepOnConnect( - eventDeviceType: RingDeviceType?, - persistedDeviceType: RingDeviceType? = null, -): Boolean = when (eventDeviceType ?: persistedDeviceType) { - // All three identifiers use YCBTDriver and share its repeated status packets plus async - // history transfer. Packet-based Colmi/Jring/CRP families still clear and rebuild on connect. - RingDeviceType.YCBT, RingDeviceType.TK5, RingDeviceType.COLMI_SMART_HEALTH -> true - else -> false -} - internal fun shouldReplaceCompleteSleep( existingStart: Long, existingMinutes: Int, diff --git a/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt b/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt index e83ef4a..a4435cb 100644 --- a/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt +++ b/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt @@ -117,10 +117,9 @@ class CRPDecoderTest { @Test fun `firmware version reaches the device record as its own event, not a connection change`() { - // NOT RingDecodedEvent.Status: that bridges to DeviceStateChanged(CONNECTED), which - // persistence answers by clearing sleep_sessions for families outside - // preservesSleepOnConnect — CRP among them — and runStartup re-queries firmware on every - // ~30-minute sync pass, so that path would wipe stored sleep on each one. + // NOT RingDecodedEvent.Status: that bridges to DeviceStateChanged(CONNECTED), and a + // device-info reply says nothing about the connection. runStartup re-queries firmware on + // every ~30-minute sync pass, so that path would restate CONNECTED all session long. val decoded = RingDecodedEvent.FirmwareRevision("MOY-R1K3-2.1.6") val events = RingEventBridge.eventsFor(decoded) assertEquals("MOY-R1K3-2.1.6", (events.single() as PulseEvent.FirmwareRevision).version) diff --git a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt index a26f97f..36114da 100644 --- a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt +++ b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt @@ -10,47 +10,46 @@ import org.junit.Assert.assertTrue import org.junit.Test class EventPersistenceIdentityTest { - @Test - fun `only YCBT protocol families preserve sleep on connect`() { - assertTrue(preservesSleepOnConnect(RingDeviceType.YCBT)) - assertTrue(preservesSleepOnConnect(RingDeviceType.TK5)) - assertTrue(preservesSleepOnConnect(RingDeviceType.COLMI_SMART_HEALTH)) - assertTrue(preservesSleepOnConnect(null, RingDeviceType.YCBT)) - assertTrue(preservesSleepOnConnect(RingDeviceType.YCBT, RingDeviceType.JRING)) - assertFalse(preservesSleepOnConnect(RingDeviceType.JRING, RingDeviceType.YCBT)) - assertFalse(preservesSleepOnConnect(RingDeviceType.COLMI_R02)) - assertFalse(preservesSleepOnConnect(null)) - } - @Test fun `only the client's own connect event is a connection transition`() { // RingBLEClient always stamps the resolved family on its connect event... - assertTrue(isConnectTransition(RingDeviceType.JRING)) - assertTrue(isConnectTransition(RingDeviceType.CRP)) - assertTrue(isConnectTransition(RingDeviceType.YCBT)) - // ...and RingEventBridge never does, for any decoder's Status. + for (family in RingDeviceType.entries) assertTrue(isConnectTransition(family)) + // ...and RingEventBridge never does, for any decoder's Status — jring 0x0C, LuckRing + // dev-info, YCBT status packets, all re-sent by runStartup on every sync pass. assertFalse(isConnectTransition(null)) } + /** + * Issue #43. Connecting used to run `DELETE FROM sleep_sessions` / `sleep_stage_blocks` for + * every family except YCBT and rebuild from the ring, which capped stored history at whatever + * the ring still held — one day on CRP (`queryHistorySleep(daysAgo = 0)`) and jring + * (`syncWindowDays = 1`). The rebuild is gone, so the day-scoped reconcile below is what keeps + * a re-synced night from disturbing the nights around it. These are the cases that used to be + * covered by the deleted `preservesSleepOnConnect`. + */ @Test - fun `a decoder's Status never reaches the rebuild, whatever family it belongs to`() { - // The pairing that used to wipe sleep: no deviceType (so it's a mid-session re-assertion - // from jring 0x0C / LuckRing dev-info) AND a family outside preservesSleepOnConnect (so the - // rebuild branch runs unscoped DELETEs). The transition gate is what breaks it. - assertFalse(preservesSleepOnConnect(null, RingDeviceType.JRING)) - assertFalse(preservesSleepOnConnect(null, RingDeviceType.LUCK_RING)) - assertFalse(preservesSleepOnConnect(null, RingDeviceType.CRP)) - assertFalse(isConnectTransition(null)) - } + fun `re-syncing one night leaves the nights around it untouched`() { + val night = 1_754_000_000_000L // the night being re-synced + val dayBefore = night - 24 * 3_600_000L + val dayAfter = night + 24 * 3_600_000L + val existing = listOf( + block("prev", dayBefore, 60, "LIGHT"), + block("this", night, 60, "LIGHT"), + block("next", dayAfter, 60, "LIGHT"), + ) - @Test - fun `a real connect still rebuilds for the packet-based families`() { - // The gate must not suppress the behaviour it is protecting: a genuine connect on a - // non-YCBT family still clears and re-pulls. - assertTrue(isConnectTransition(RingDeviceType.JRING)) - assertFalse(preservesSleepOnConnect(RingDeviceType.JRING)) - assertTrue(isConnectTransition(RingDeviceType.CRP)) - assertFalse(preservesSleepOnConnect(RingDeviceType.CRP)) + val kept = replaceOverlappingSleepBlocks( + existing = existing, + replacements = listOf(block("this", night, 90, "DEEP")), // same night, revised + replacementStart = night, + replacementEnd = night + 90 * 60_000L, + ) + + // The neighbouring nights survive — that is the whole bug. + assertTrue(kept.any { it.startAt == dayBefore }) + assertTrue(kept.any { it.startAt == dayAfter }) + // ...and the re-synced night is the revised copy, not a duplicate. + assertEquals(1, kept.count { it.startAt == night }) } @Test From 132369cc890f3813e002ad0f37346b281f3d18db Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 3 Aug 2026 09:35:40 -0700 Subject: [PATCH 2/5] build: bump versionCode to 33 --- app/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 5ed9793..75dc583 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -19,7 +19,7 @@ android { // versionCode/versionName are overridable from Gradle properties so the release CI // can drive them straight from the git tag (e.g. -PappVersionCode=5 -PappVersionName=1.0.0). // Local builds fall back to the literals below. - versionCode = (project.findProperty("appVersionCode") as String?)?.toIntOrNull() ?: 32 + versionCode = (project.findProperty("appVersionCode") as String?)?.toIntOrNull() ?: 33 versionName = (project.findProperty("appVersionName") as String?) ?: "1.0.0" testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" From 7649513e94c5b40c5b0f914b58d9d044435c57f1 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 3 Aug 2026 09:51:54 -0700 Subject: [PATCH 3/5] fix(sleep): backfill the ring's older nights, and make "connect deletes nothing" structural MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to e11be6e from an adversarial review of #44. That commit stopped the bleeding but shipped three problems. Backfill the prior week's sleep, once per connection. Stopping the deletion only stops further loss — it recovers nothing, because the poll pass only ever asked `queryHistorySleep(daysAgo = 0)`. zaggash's stored history would have rebuilt one night at a time from today forward while the ring already held the back-catalogue. The earlier PR deferred this as needing hardware confirmation, which overstated the risk: CRPDecoder.decodeSleep already reads the day index out of `payload[0]` and accepts up to 14, so a reply dates itself rather than trusting what we asked for, and a day the ring has no record of produces no reply — exactly today's behaviour for that day. Sent once per connection, not per pass, and stopped at a week rather than the decoder's 14-day ceiling: runStartup is also the ~30-minute background sync and this ring funnels everything through one fdd2 channel. Replace the fake regression test. `re-syncing one night leaves the nights around it untouched` exercised replaceOverlappingSleepBlocks, which e11be6e never touched — it passed identically before the fix and would keep passing if someone re-added sleepSessionDao().clear() tomorrow. It guarded nothing, and it replaced eight assertions that at least pinned real per-family behaviour, so coverage went down in a commit whose purpose was preventing data loss. The replacement is structural rather than another assertion: ConnectPurge has members NOTHING and DEMO_ROWS and deliberately none meaning "real rows", so restoring the old behaviour now requires a new member, which fails to compile against exhaustive `when`s in both the subscriber and the test. Verified by temporarily adding an ALL_SLEEP member — the build fails with "'when' expression must be exhaustive" — rather than assuming it. Correct an AGENTS.md claim the code does not deliver. It said isConnectTransition "stops a device-info reply from being mistaken for a connection". The gate only governs what a CONNECTED event may delete; the row write below it — stateRaw, lastConnectedAt, lastSyncAt — is outside the gate and still runs for every decoder Status, so a jring 0x0C reply does restamp the device row each sync pass. Harmless, but not something the gate prevents. Also recorded two things the PR left unsaid: no bulk-delete path exists anywhere now, so there is no retention or pruning mechanism at all, and reconcileWakingDay's `if (groups.isEmpty())` is the one delete a re-sync can still perform — it should be unreachable, but it is where to look first if history goes missing again. jring has the same one-day gap (`syncWindowDays = 1` against a command that accepts 27) and is deliberately untouched: that constant also drives the activity-sync progress window, so it is not the one-line change it appears to be. 792 unit tests pass. --- AGENTS.md | 29 ++++++++++++- .../java/com/pulseloop/ring/CRPSyncEngine.kt | 33 +++++++++++++++ .../service/EventPersistenceSubscriber.kt | 28 ++++++++++--- .../com/pulseloop/ring/CRPSyncEngineTest.kt | 30 +++++++++++-- .../service/EventPersistenceIdentityTest.kt | 42 ++++++++----------- 5 files changed, 127 insertions(+), 35 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 737d54c..51e7216 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,6 +66,16 @@ one instead of joining it. Both the carve-out and the rebuild are gone. What pro day now is `upsertSleepSessionAtomic`, which reconciles one waking day at a time, idempotently, and re-points legacy mis-keyed blocks itself — the blanket clear's own stated justification. +**Stopping the deletion only stops further loss; it recovers nothing.** A ring holds days the app +has never asked for, and asking is cheap because every reply is self-describing — CRP's sleep frame +carries its own day index in `payload[0]`, which `CRPDecoder.decodeSleep` accepts up to 14, so a +night is dated from the reply rather than from the request, and a day the ring has no record of +simply produces no reply. `CRPSyncEngine.sendSleepBackfill` therefore pulls the prior week **once +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: `syncWindowDays = 1`, while +`makeHistoryQueryCommand` accepts up to 27. It is untouched for now only because that constant also +drives the activity-sync progress window, so widening it is not the one-line change it looks like. + 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, it belongs on `DeviceForgotten` as a deliberate choice, not as a side effect of connecting. @@ -80,8 +90,23 @@ transition: status packets. `runStartup` re-sends them, and `runStartup` is also the ~30-minute background sync — so they recur for the whole life of a connection. -`isConnectTransition(event.deviceType)` is that gate. Keep it: it is what stops a device-info reply -from being mistaken for a connection, and it kept the demo-clear from running all session long. +`isConnectTransition(event.deviceType)` is that gate, and `connectPurge` is what it feeds. Be precise +about its scope, because it is narrower than it looks: it decides only what a CONNECTED event may +*delete*. The row write below it — `stateRaw = "CONNECTED"`, `lastConnectedAt`, `lastSyncAt` — is +**outside** the gate and still runs for every decoder `Status`, so a jring `0x0C` reply does still +restamp the device row as freshly connected on each sync pass. That is harmless today; it is not +something the gate prevents, so don't cite it as if it were. + +Two related things worth knowing before changing this area: + +- **Nothing bulk-deletes sleep any more, anywhere in the app.** That connect path was the only + caller, so there is no retention or pruning mechanism at all now — the tables grow without bound + and a Forget doesn't reclaim them. Fine at current row sizes; a deliberate retention policy is a + separate piece of work, not something to bolt back onto connect. +- **One narrow delete path survives**, in `reconcileWakingDay`: `if (groups.isEmpty())` drops that + day's rows. It should be unreachable — `upsertSleepSession` returns early on empty stages, so the + replacements reaching it are never empty — but it is the one place a *re-sync* can still remove a + stored night, so check it first if history goes missing again. Corollary for new protocol work: a reply that merely reports something about the device (firmware, serial, capabilities) is not a connection event. Give it its own `RingDecodedEvent` — as diff --git a/app/src/main/java/com/pulseloop/ring/CRPSyncEngine.kt b/app/src/main/java/com/pulseloop/ring/CRPSyncEngine.kt index cf81997..d889edd 100644 --- a/app/src/main/java/com/pulseloop/ring/CRPSyncEngine.kt +++ b/app/src/main/java/com/pulseloop/ring/CRPSyncEngine.kt @@ -1,5 +1,8 @@ package com.pulseloop.ring +/** Nights before today to pull once per connection. See [CRPSyncEngine.sendSleepBackfill]. */ +private const val SLEEP_BACKFILL_DAYS = 6 + /** * Per-connection orchestration for a CRP ("crrepa") ring. Ported in spirit from the Moyoung * "Da Rings" connect flow (`d1/b.java` + `b1` package builders): after the link is up the app sets the @@ -116,6 +119,36 @@ class CRPSyncEngine(private val writer: RingCommandWriter?) : RingSyncEngine { send(CRPProtocol.queryTimingStressHistory()) send(CRPProtocol.queryHistoryTemp()) send(CRPProtocol.queryHistorySleep()) + sendSleepBackfill() + } + + /** Whether this connection has already backfilled older nights. Same "fresh engine per + * connection" trick as [readBacksSent]. */ + private var sleepBackfillSent = false + + /** + * Pull the nights *before* today, once per connection. + * + * The poll pass above only ever asks for `daysAgo = 0`, so the app's stored history could only + * ever grow one night at a time from whenever the user installed — and before issue #43 it + * couldn't grow at all, because each connect deleted the older nights first. Asking for the + * ring's own back-catalogue is what actually restores a user's history rather than merely + * stopping further loss. + * + * Safe to send blind. Each reply is self-describing: `payload[0]` is the ring's own day index, + * so [CRPDecoder.decodeSleep] dates a night from the reply rather than from what we asked for, + * and a day the ring has no record of simply produces no reply — the same nothing we get today. + * + * Once per connection, and deliberately short of the decoder's 14-day ceiling: [runStartup] is + * also the ~30-minute background sync, and this ring funnels the handshake, timing config, + * history pull *and* on-demand measures through one `fdd2` channel (a spot SpO2 needs ~48 s of + * it). A week is the useful-recovery/quiet-channel trade; raise it once hardware shows the ring + * answers deeper. + */ + private fun sendSleepBackfill() { + if (sleepBackfillSent) return + sleepBackfillSent = true + for (daysAgo in 1..SLEEP_BACKFILL_DAYS) send(CRPProtocol.queryHistorySleep(daysAgo)) } /** The last frame index each timing vital emits before its day is complete (vendor terminal diff --git a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index be0cacb..ef20da0 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -95,11 +95,14 @@ class EventPersistenceSubscriber( // [upsertSleepSessionAtomic] reconciles one waking day at a time, // idempotently, and re-points legacy mis-keyed blocks itself — which was the // blanket clear's stated reason for existing. - if (isConnectTransition(event.deviceType)) { - db.measurementDao().clearDemo() - db.activityDailyDao().clearDemo() - db.sleepStageBlockDao().clearDemo() - db.sleepSessionDao().clearDemo() + when (connectPurge(event.deviceType)) { + ConnectPurge.NOTHING -> {} + ConnectPurge.DEMO_ROWS -> { + db.measurementDao().clearDemo() + db.activityDailyDao().clearDemo() + db.sleepStageBlockDao().clearDemo() + db.sleepSessionDao().clearDemo() + } } "CONNECTED" } @@ -646,6 +649,21 @@ internal fun historyMeasurementId(kind: MeasurementKind, timestamp: Long): Strin */ internal fun isConnectTransition(eventDeviceType: RingDeviceType?): Boolean = eventDeviceType != null +/** + * What a CONNECTED event is allowed to remove. + * + * There is deliberately **no member meaning "real rows"**. Connecting must never delete stored + * history (issue #43), and encoding that as a type instead of a comment means re-introducing the old + * behaviour takes more than adding a `.clear()` call: it needs a new member here, which fails to + * compile against the exhaustive `when`s in [EventPersistenceSubscriber] and in + * `EventPersistenceIdentityTest`. That is the tripwire the deleted `preservesSleepOnConnect` never + * had — it made destructive clearing a per-family *option*, and every family took it but one. + */ +internal enum class ConnectPurge { NOTHING, DEMO_ROWS } + +internal fun connectPurge(eventDeviceType: RingDeviceType?): ConnectPurge = + if (isConnectTransition(eventDeviceType)) ConnectPurge.DEMO_ROWS else ConnectPurge.NOTHING + internal fun shouldReplaceCompleteSleep( existingStart: Long, existingMinutes: Int, diff --git a/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt b/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt index ddbeff6..5cab42e 100644 --- a/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt +++ b/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt @@ -21,6 +21,10 @@ class CRPSyncEngineTest { /** Temperature history is 2/22, not 2/48 — `q.b(2,48)` is the vendor's `querySleepState`. */ private val historyQueries = listOf(2 to 15, 2 to 17, 2 to 16, 2 to 47, 2 to 22, 2 to 14) + /** The older nights pulled once per connection (issue #43). Same opcode as today's sleep query, + * `daysAgo` rising in the payload — see CRPSyncEngine.sendSleepBackfill. */ + private val sleepBackfill = List(6) { 2 to 14 } + /** The read-backs that let the ring describe itself instead of us guessing: SpO2 support type, * then each all-day monitor's configured interval. See CRPSyncEngine.runStartup. */ private val readBackQueries = listOf(2 to 37, 2 to 6, 2 to 7, 2 to 8, 2 to 45, 2 to 21) @@ -42,18 +46,38 @@ class CRPSyncEngineTest { // the enables force everything on moments later. Asking afterwards would only describe the // state we just imposed. If this assertion fails, move the call site back — don't reorder the // expectation. See CRPSyncEngine.sendConnectionReadBacks. - assertEquals(listOf(1 to 1, 3 to 3) + readBackQueries + timingEnables + historyQueries, w.opcodes()) + assertEquals( + listOf(1 to 1, 3 to 3) + readBackQueries + timingEnables + historyQueries + sleepBackfill, + w.opcodes(), + ) w.sent.clear() engine.setUserProfile( UserProfileValues(metric = true, gender = 1u, age = 30u, heightCm = 180u, weightKg = 75u), ) engine.runStartup() - // A second pass on the same connection re-sends the poll work but NOT the read-backs — - // what the ring supports cannot change between syncs. + // A second pass on the same connection re-sends the poll work but NOT the read-backs, and + // NOT the sleep backfill — what the ring supports cannot change between syncs, and the older + // nights were already pulled. runStartup is the ~30-minute background sync, so anything + // repeated here lands on the single fdd2 channel every half hour forever. assertEquals(listOf(1 to 1, 3 to 3, 1 to 0) + timingEnables + historyQueries, w.opcodes()) } + @Test + fun `the sleep backfill asks for consecutive prior days, dated by the ring not by us`() { + // Issue #43. The poll pass only ever asks daysAgo=0, so without this the app could only ever + // accumulate one night per day going forward and never recover what the ring already holds. + val w = FakeWriter() + CRPSyncEngine(w).runStartup() + val sleepFrames = w.sent.filter { (it[4].toInt() to it[5].toInt()) == (2 to 14) } + // Today, then one frame per prior day, each carrying its own daysAgo in the payload. + assertEquals(1 + 6, sleepFrames.size) + assertEquals((0..6).toList(), sleepFrames.map { it[6].toInt() }) + // Every requested day stays inside the range CRPDecoder.decodeSleep will accept (<= 14), + // so a reply can't be discarded as a corrupt day index. + assertTrue(sleepFrames.all { it[6].toInt() <= 14 }) + } + /** * `runStartup` doubles as the ~30-minute background poll and is also reached from * `refresh()`/`querySleep()`. Re-asking what the ring supports on every one of those would add diff --git a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt index 36114da..41145b7 100644 --- a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt +++ b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt @@ -23,33 +23,25 @@ class EventPersistenceIdentityTest { * Issue #43. Connecting used to run `DELETE FROM sleep_sessions` / `sleep_stage_blocks` for * every family except YCBT and rebuild from the ring, which capped stored history at whatever * the ring still held — one day on CRP (`queryHistorySleep(daysAgo = 0)`) and jring - * (`syncWindowDays = 1`). The rebuild is gone, so the day-scoped reconcile below is what keeps - * a re-synced night from disturbing the nights around it. These are the cases that used to be - * covered by the deleted `preservesSleepOnConnect`. + * (`syncWindowDays = 1`). + * + * The `when` below is exhaustive on purpose: it is the actual guard. Adding a [ConnectPurge] + * member that deletes real rows stops this file compiling, which is the tripwire the old + * per-family `preservesSleepOnConnect` boolean never provided. */ @Test - fun `re-syncing one night leaves the nights around it untouched`() { - val night = 1_754_000_000_000L // the night being re-synced - val dayBefore = night - 24 * 3_600_000L - val dayAfter = night + 24 * 3_600_000L - val existing = listOf( - block("prev", dayBefore, 60, "LIGHT"), - block("this", night, 60, "LIGHT"), - block("next", dayAfter, 60, "LIGHT"), - ) - - val kept = replaceOverlappingSleepBlocks( - existing = existing, - replacements = listOf(block("this", night, 90, "DEEP")), // same night, revised - replacementStart = night, - replacementEnd = night + 90 * 60_000L, - ) - - // The neighbouring nights survive — that is the whole bug. - assertTrue(kept.any { it.startAt == dayBefore }) - assertTrue(kept.any { it.startAt == dayAfter }) - // ...and the re-synced night is the revised copy, not a duplicate. - assertEquals(1, kept.count { it.startAt == night }) + fun `no connect event may purge anything but demo rows, for any family`() { + val everyOrigin: List = RingDeviceType.entries + null + for (origin in everyOrigin) { + val purge = connectPurge(origin) + val deletesRealRows = when (purge) { + ConnectPurge.NOTHING -> false + ConnectPurge.DEMO_ROWS -> false + } + assertFalse("connect purge $purge (origin $origin) must not delete real rows", deletesRealRows) + } + // The decoder-Status case — the one that used to fire all session long — purges nothing. + assertEquals(ConnectPurge.NOTHING, connectPurge(null)) } @Test From 893aebbbe11fd0e638e9cf29d3ac092401bbde6e Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 3 Aug 2026 10:03:28 -0700 Subject: [PATCH 4/5] docs: cite jring's real sleep-request depth, not syncWindowDays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three comments and an AGENTS.md paragraph attributed jring's one-day sleep pull to RingSyncCoordinator.syncWindowDays. That constant has exactly one use — sizing the sync-progress window in beginSyncProgress — and it applies to every family, not just jring. jring's actual depth is makeHistoryQueryCommand()'s default of 1, called with no argument at JringDriver.kt:105. The misattribution came from grepping for makeHistoryQueryCommand callers: one of the hits was syncWindowDays' doc comment ("must match makeHistoryQueryCommand's default"), and the constant sitting under it was read as the control. Both values are 1 and the comment asserts they must match, so the wrong mechanism kept producing the right answer and nothing contradicted it. It was not just a mislabel. The rationale for deferring the jring fix was reasoned from it — "that constant also drives the activity-sync progress window, so widening it is not the one-line change it looks like" — which is true of syncWindowDays and irrelevant to jring's request depth. AGENTS.md now says plainly that syncWindowDays is not a per-family request depth, and records the two things that do make jring harder than CRP: JringSyncEngine.runStartup has no once-per-connection gate, so a wider `days` re-pulls the span on every ~30-minute background pass instead of once; and 0x10 returns activity and sleep together, with no sleep-only request, so each extra day costs ~96 activity packets (15x 1-minute buckets per packet) on top of the night. Comments only, no behaviour change. 792 unit tests pass. --- AGENTS.md | 18 ++++++++++++++---- .../service/EventPersistenceSubscriber.kt | 4 +++- .../service/EventPersistenceIdentityTest.kt | 2 +- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 51e7216..45641fe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,7 +60,8 @@ data-loss bug twice, in two different shapes (issue #43, and the sync-pass varia The original design deleted all sleep on connect and re-pulled it, carving YCBT out via `preservesSleepOnConnect` because YCBT re-asserts CONNECTED mid-history. That premise was false for -everyone: CRP asks `queryHistorySleep(daysAgo = 0)` and jring `syncWindowDays = 1`, so "delete +everyone: CRP asks `queryHistorySleep(daysAgo = 0)`, and jring calls `makeHistoryQueryCommand()` with +its default of 1 day (`JringDriver.kt:105`), so "delete everything and ask again" capped stored sleep at a single night — a new night replaced the previous one instead of joining it. Both the carve-out and the rebuild are gone. What protects a re-synced day now is `upsertSleepSessionAtomic`, which reconciles one waking day at a time, idempotently, and @@ -72,9 +73,18 @@ carries its own day index in `payload[0]`, which `CRPDecoder.decodeSleep` accept night is dated from the reply rather than from the request, and a day the ring has no record of simply produces no reply. `CRPSyncEngine.sendSleepBackfill` therefore pulls the prior week **once 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: `syncWindowDays = 1`, while -`makeHistoryQueryCommand` accepts up to 27. It is untouched for now only because that constant also -drives the activity-sync progress window, so widening it is not the one-line change it looks like. +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. 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, diff --git a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index ef20da0..ab97863 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -89,7 +89,9 @@ class EventPersistenceSubscriber( // This 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 `syncWindowDays = 1` — so every connect destroyed each night older + // jring `makeHistoryQueryCommand()` with its 1-day default (JringDriver.kt:105, + // 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 rebuild was never load-bearing: // [upsertSleepSessionAtomic] reconciles one waking day at a time, diff --git a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt index 41145b7..3e8f984 100644 --- a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt +++ b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt @@ -23,7 +23,7 @@ class EventPersistenceIdentityTest { * Issue #43. Connecting used to run `DELETE FROM sleep_sessions` / `sleep_stage_blocks` for * every family except YCBT and rebuild from the ring, which capped stored history at whatever * the ring still held — one day on CRP (`queryHistorySleep(daysAgo = 0)`) and jring - * (`syncWindowDays = 1`). + * (`makeHistoryQueryCommand()`'s 1-day default at `JringDriver.kt:105`). * * The `when` below is exhaustive on purpose: it is the actual guard. Adding a [ConnectPurge] * member that deletes real rows stops this file compiling, which is the tripwire the old From 9705fe4b6298b60a57681e2000fd124972ae64dd Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Mon, 3 Aug 2026 10:07:44 -0700 Subject: [PATCH 5/5] fix(sleep): backfill jring history once per connection instead of one day per pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the gap left by 7649513, which deepened CRP's sleep pull and left jring on a single day. Same defect: the ring holds days the app never asks for, so once connecting stopped deleting the stored copy (e11be6e), a user's history could still only grow one night at a time from install and never recovered what the ring already had. JringSyncEngine now asks for JRING_BACKFILL_DAYS on the first pass of a connection and one day on every pass after it. A fresh engine is built per connection (JringDriver.makeSyncEngine runs on connect), so instance state gives "once per connection" for free — the same trick CRPSyncEngine uses for its read-backs. The gate is the whole difficulty, and it is why this was previously deferred as a larger piece of work. runStartup is also the ~30-minute background sync, and refresh()/querySleep() default to it, so an unconditional wider window would re-pull the span every half hour forever rather than once. That earlier deferral also cited RingSyncCoordinator.syncWindowDays as the control, which was wrong — see 893aebb; the real constraint is volume. Window is 3 days rather than CRP's 7 because 0x10 returns activity *and* sleep and there is no sleep-only request, so each extra day costs roughly 96 more packets (activity arrives as 15x 1-minute buckets each) to reach one more night. Re-syncing the same days is harmless either way: activity buckets upsert by timestamp with the day total recomputed from distinct buckets, and sleep reconciles one waking day at a time. Tested that the gate actually holds rather than assuming it: commenting out the `historyBackfilled = true` latch — a realistic regression that still compiles — fails `Jring pulls a deeper history window once per connection` and nothing else. The existing refresh/querySleep contract test builds a fresh engine per capture, so all three take the first-pass branch and it still passes unchanged; a comment now says so, since that is no longer self-evident. Still unverified on hardware: that the ring honours days > 1. The command coerces to 0..27 and mirrors Gadgetbridge's triggerActivityReportByDays, and a ring that ignores it returns what it has, which is today's behaviour. 794 unit tests pass. --- .../java/com/pulseloop/ring/JringDriver.kt | 37 ++++++++++++++++- .../ring/ExistingFamilyRefreshContractTest.kt | 41 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/pulseloop/ring/JringDriver.kt b/app/src/main/java/com/pulseloop/ring/JringDriver.kt index 08e6ea4..1afef83 100644 --- a/app/src/main/java/com/pulseloop/ring/JringDriver.kt +++ b/app/src/main/java/com/pulseloop/ring/JringDriver.kt @@ -1,5 +1,9 @@ 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. */ +private const val JRING_BACKFILL_DAYS = 3 + @OptIn(ExperimentalStdlibApi::class) /** @@ -102,10 +106,41 @@ 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()) + writer?.enqueue(encoder.makeHistoryQueryCommand(days = historyDaysForThisPass())) writer?.enqueue(encoder.makeHistoryMeasurementQueryCommand()) } + /** Whether this connection has already pulled the deep history window. A fresh engine is built + * per connection ([JringDriver.makeSyncEngine] runs on connect), so instance state gives + * "once per connection" for free — the same trick `CRPSyncEngine` uses for its read-backs. */ + 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. + * + * 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 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. + */ + private fun historyDaysForThisPass(): Int { + if (historyBackfilled) return 1 + historyBackfilled = true + return JRING_BACKFILL_DAYS + } + override fun handle(event: RingDecodedEvent) { when (event) { // Ring-side bind handshake (0x4B), mirroring the official app's diff --git a/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt b/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt index 9bf0a71..82c7032 100644 --- a/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt +++ b/app/src/test/java/com/pulseloop/ring/ExistingFamilyRefreshContractTest.kt @@ -1,6 +1,7 @@ package com.pulseloop.ring import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Test class ExistingFamilyRefreshContractTest { @@ -13,6 +14,8 @@ class ExistingFamilyRefreshContractTest { @Test fun `Jring refresh and query sleep retain startup behavior`() { + // 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() } @@ -21,6 +24,44 @@ class ExistingFamilyRefreshContractTest { assertEquals(startup, sleep) } + /** Byte 1 of the `0x10` history query is the day count (`triggerActivityReportByDays`). */ + private fun historyDays(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`() { + // 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) + + engine.runStartup() + assertEquals(listOf(3), historyDays(w.sent)) + + w.sent.clear() + engine.runStartup() + engine.refresh() // routes through runStartup + engine.querySleep() // ditto + assertEquals(listOf(1, 1, 1), historyDays(w.sent)) + + // A new connection builds a new engine, which backfills again. + val reconnected = FakeWriter() + JringSyncEngine(reconnected).runStartup() + assertEquals(listOf(3), historyDays(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. + 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) + } + @Test fun `Colmi refresh and query sleep retain startup behavior`() { val startup = captureColmi { it.runStartup() }