diff --git a/AGENTS.md b/AGENTS.md index dfd9c4b..45641fe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,13 +50,48 @@ 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 - -**Read this before touching `EventPersistenceSubscriber`'s `DeviceStateChanged` branch, or before -adding a family to `preservesSleepOnConnect`.** - -`RingConnectionState.CONNECTED` arrives from two unrelated places, and only one of them means a -connection was established: +## Connecting must never delete stored history + +**Read this before touching `EventPersistenceSubscriber`'s `DeviceStateChanged` branch.** + +**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 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 +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, 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, +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 +100,23 @@ 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, 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/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" 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/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/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt index a9603e3..ab97863 100644 --- a/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt +++ b/app/src/main/java/com/pulseloop/service/EventPersistenceSubscriber.kt @@ -80,25 +80,30 @@ 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]. - 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. + // 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 `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, + // idempotently, and re-points legacy mis-keyed blocks itself — which was the + // blanket clear's stated reason for existing. + when (connectPurge(event.deviceType)) { + ConnectPurge.NOTHING -> {} + ConnectPurge.DEMO_ROWS -> { + db.measurementDao().clearDemo() + db.activityDailyDao().clearDemo() 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() } } "CONNECTED" @@ -308,13 +313,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 +472,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 +635,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,22 +644,27 @@ 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 -} +/** + * 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, 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/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/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() } diff --git a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt index a26f97f..3e8f984 100644 --- a/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt +++ b/app/src/test/java/com/pulseloop/service/EventPersistenceIdentityTest.kt @@ -10,47 +10,38 @@ 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. - assertFalse(isConnectTransition(null)) - } - - @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)) + 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 + * (`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 + * per-family `preservesSleepOnConnect` boolean never provided. + */ @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)) + 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