diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 104f1c5..02697bd 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -21,7 +21,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() ?: 38 + versionCode = (project.findProperty("appVersionCode") as String?)?.toIntOrNull() ?: 39 versionName = (project.findProperty("appVersionName") as String?) ?: "2.7.0" testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" @@ -172,6 +172,23 @@ dependencies { // series, so it is a faithful API reference for this pin. implementation("androidx.health.connect:connect-client:1.1.0") + // Phase 9 (iOS #96 stage A): barcode scanner. ML Kit's bundled-model artifact runs + // regardless of Play Services state (the -play-services variant would dead-end on + // devices without the updated services). CameraX 1.4.x for preview + analysis. + implementation("com.google.mlkit:barcode-scanning:17.3.0") + val cameraXVersion = "1.4.2" + implementation("androidx.camera:camera-core:$cameraXVersion") + // The CameraX camera2 implementation artifact is "camera-camera2" (the partial's + // "camera2" coordinate does not exist on Google Maven). + implementation("androidx.camera:camera-camera2:$cameraXVersion") + implementation("androidx.camera:camera-lifecycle:$cameraXVersion") + implementation("androidx.camera:camera-view:$cameraXVersion") + + // CameraX's ProcessCameraProvider.getInstance() exposes Guava ListenableFuture in its + // signature, but the graph also carries Google's "9999.0-empty-to-avoid-conflict-with-guava" + // stub, which strips the class at compile time. Full guava restores it. + implementation("com.google.guava:guava:33.3.1-android") + debugImplementation("androidx.compose.ui:ui-tooling") debugImplementation("androidx.compose.ui:ui-test-manifest") diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index cd67289..b2fd07b 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -21,6 +21,12 @@ + + + + + diff --git a/app/src/test/java/com/pulseloop/coach/LocalOpenAICompatClientTest.kt b/app/src/test/java/com/pulseloop/coach/LocalOpenAICompatClientTest.kt index 09375ac..3720901 100644 --- a/app/src/test/java/com/pulseloop/coach/LocalOpenAICompatClientTest.kt +++ b/app/src/test/java/com/pulseloop/coach/LocalOpenAICompatClientTest.kt @@ -38,13 +38,31 @@ class LocalOpenAICompatClientTest { input: List, tools: List = emptyList(), previousResponseId: String? = null, + textFormat: JsonObject? = null, ) = JsonObject(buildMap { put("model", JsonPrimitive("qwen3:8b")) put("input", JsonArray(input)) put("tools", JsonArray(tools)) previousResponseId?.let { put("previous_response_id", JsonPrimitive(it)) } + textFormat?.let { put("text", JsonObject(mapOf("format" to it))) } }) + /** A caller-supplied strict schema, shaped like MealEstimator's `meal_estimate`. */ + private val mealFormat = JsonObject(mapOf( + "type" to JsonPrimitive("json_schema"), + "name" to JsonPrimitive("meal_estimate"), + "strict" to JsonPrimitive(true), + "schema" to JsonObject(mapOf( + "type" to JsonPrimitive("object"), + "properties" to JsonObject(mapOf( + "name" to JsonObject(mapOf("type" to JsonPrimitive("string"))), + "calories" to JsonObject(mapOf("type" to JsonPrimitive("number"))), + )), + "required" to JsonArray(listOf(JsonPrimitive("name"), JsonPrimitive("calories"))), + "additionalProperties" to JsonPrimitive(false), + )), + )) + private val functionTool = JsonObject(mapOf( "type" to JsonPrimitive("function"), "name" to JsonPrimitive("get_hr"), @@ -315,4 +333,66 @@ class LocalOpenAICompatClientTest { assertTrue(assistantIdx in 0 until toolIdx) assertEquals("call_1", m[toolIdx]["tool_call_id"]!!.jsonPrimitive.content) } + + // ── Caller-supplied schema (iOS #96 meal estimator / summary generator) ── + + @Test + fun `a caller schema replaces the coach_response schema in response_format`() { + val body = client(structured = LocalStructuredOutput.JSON_SCHEMA) + .buildRequestBody(request(listOf(msg("user", "hi")), textFormat = mealFormat)) + val schema = body["response_format"]!!.jsonObject["json_schema"]!!.jsonObject + assertEquals("meal_estimate", schema["name"]!!.jsonPrimitive.content) + // The point of the fix: a guided-decoding backend must not be handed the chat schema for + // a meal call — it would force a shape MealAnalysisLogic.decode can never parse. + assertEquals( + mealFormat["schema"]!!.jsonObject, + schema["schema"]!!.jsonObject, + ) + } + + @Test + fun `a caller schema suppresses the coach_response prompt instruction`() { + val body = client(structured = LocalStructuredOutput.JSON_SCHEMA) + .buildRequestBody(request(listOf(msg("system", "SYS"), msg("user", "hi")), textFormat = mealFormat)) + val system = content(messages(body).first { role(it) == "system" }) + assertTrue(system.startsWith("SYS")) + assertFalse(system.contains("coach_response")) + assertFalse(system.contains("response_type")) + // …and states the caller's schema instead, which is what carries the shape when the + // user's Response format is OFF. + assertTrue(system.contains("meal_estimate")) + assertTrue(system.contains("calories")) + } + + @Test + fun `no caller schema keeps the coach_response instruction and schema`() { + val body = client(structured = LocalStructuredOutput.JSON_SCHEMA) + .buildRequestBody(request(listOf(msg("user", "hi")))) + assertEquals( + "coach_response", + body["response_format"]!!.jsonObject["json_schema"]!!.jsonObject["name"]!!.jsonPrimitive.content, + ) + assertTrue(content(messages(body).first { role(it) == "system" }).contains("coach_response")) + } + + @Test + fun `response format off still sends no response_format even with a caller schema`() { + // OFF means "my backend rejects response_format"; a caller's schema does not override + // that — it travels in the prompt, and structured callers decode fence-tolerantly. + val body = client(structured = LocalStructuredOutput.OFF) + .buildRequestBody(request(listOf(msg("user", "hi")), textFormat = mealFormat)) + assertNull(body["response_format"]) + assertTrue(content(messages(body).first { role(it) == "system" }).contains("meal_estimate")) + } + + @Test + fun `a malformed text format is ignored rather than replacing the coach schema`() { + val notJsonSchema = JsonObject(mapOf("type" to JsonPrimitive("text"))) + val body = client(structured = LocalStructuredOutput.JSON_SCHEMA) + .buildRequestBody(request(listOf(msg("user", "hi")), textFormat = notJsonSchema)) + assertEquals( + "coach_response", + body["response_format"]!!.jsonObject["json_schema"]!!.jsonObject["name"]!!.jsonPrimitive.content, + ) + } } diff --git a/app/src/test/java/com/pulseloop/coach/tools/NutritionToolsTest.kt b/app/src/test/java/com/pulseloop/coach/tools/NutritionToolsTest.kt new file mode 100644 index 0000000..da6b515 --- /dev/null +++ b/app/src/test/java/com/pulseloop/coach/tools/NutritionToolsTest.kt @@ -0,0 +1,221 @@ +package com.pulseloop.coach.tools + +import com.pulseloop.coach.orchestration.MealUpdates +import com.pulseloop.data.entity.MealEntryEntity +import com.pulseloop.nutrition.FoodProduct +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import java.time.Instant +import java.time.LocalDate +import java.time.ZoneId + +/** + * Unit tests for the pure logic factored out of [NutritionTools] (iOS PR #96 port). + * The tool bodies take a concrete Room db (the repo has no in-memory harness) and can't be + * exercised end-to-end here, so the behavior that can drift — timestamp resolution, the + * source/confidence raw-string mapping, the search clamp and query validation, the per-product + * payload shape, and the meal-update application — lives in pure [NutritionTools] members and + * is tested directly, with an injected clock/zone wherever time matters. + */ +class NutritionToolsTest { + /** A fixed "now": 2026-08-22 15:04:05 UTC — afternoon, so local "today" is 2026-08-22/23. */ + private val now = Instant.parse("2026-08-22T15:04:05Z").toEpochMilli() + + // resolveTimestamp goes through CoachDataAccess.parseLocalDate, which anchors to the system + // default zone — so these tests run in that same zone (like CoachActionTest does) and + // compute their expectations in it, staying valid on any CI machine's timezone. + private val zone = ZoneId.systemDefault() + + private fun dayOf(instantMs: Long): String = + Instant.ofEpochMilli(instantMs).atZone(zone).toLocalDate().toString() + + // ── resolveTimestamp ─────────────────────────────────────────────── + + @Test + fun todayWithoutTimeLandsAtNow() { + assertEquals(now, NutritionTools.resolveTimestamp(dayOf(now), null, now, zone)) + } + + @Test + fun pastDayWithoutTimeLandsAtNoon() { + // "2026-08-20" can never be the local day of [now] (max zone offset ±14 keeps [now] + // on 2026-08-22/23 local), so this deterministically takes the noon branch. + val expected = LocalDate.parse("2026-08-20").atTime(12, 0).atZone(zone).toInstant().toEpochMilli() + assertEquals(expected, NutritionTools.resolveTimestamp("2026-08-20", null, now, zone)) + } + + @Test + fun explicitTimeIsHonoredOnAPastDay() { + val expected = LocalDate.parse("2026-08-20").atTime(14, 30).atZone(zone).toInstant().toEpochMilli() + assertEquals(expected, NutritionTools.resolveTimestamp("2026-08-20", "14:30", now, zone)) + } + + @Test + fun explicitTimeIsHonoredOnTodayToo() { + // An explicit time wins even on today (iOS: the stamped time short-circuits the + // isDateInToday fallback). + val startOfDay = Instant.ofEpochMilli(now).atZone(zone).toLocalDate() + .atStartOfDay(zone).toInstant().toEpochMilli() + assertEquals(startOfDay + 8 * 3600_000L, NutritionTools.resolveTimestamp(dayOf(now), "08:00", now, zone)) + } + + @Test + fun unstampableTimeFallsBackToNoonOrNow() { + // 25:99 can't be stamped (iOS's bySettingHour returns null) — a past day falls to + // noon, today falls to the current clock time. + val expectedNoon = LocalDate.parse("2026-08-20").atTime(12, 0).atZone(zone).toInstant().toEpochMilli() + assertEquals(expectedNoon, NutritionTools.resolveTimestamp("2026-08-20", "25:99", now, zone)) + assertEquals(now, NutritionTools.resolveTimestamp(dayOf(now), "25:99", now, zone)) + } + + @Test + fun invalidDateFallsBackToToday() { + // iOS: parseLocalDate(date) ?? startOfDay(now) — and today with no time is now. + assertEquals(now, NutritionTools.resolveTimestamp("not-a-date", null, now, zone)) + } + + // ── source / confidence / meal_type raw mapping ──────────────────── + + @Test + fun sourceIsOffSearchOnlyForDatabaseWithAProductCode() { + // The honesty core: a row is database-verified only when grounded in a real OFF code. + assertEquals("off_search", NutritionTools.resolveSourceRaw("database", "3017620422003")) + assertEquals("llm_estimate", NutritionTools.resolveSourceRaw("database", null)) + assertEquals("llm_estimate", NutritionTools.resolveSourceRaw("estimate", "3017620422003")) + assertEquals("llm_estimate", NutritionTools.resolveSourceRaw("estimate", null)) + } + + @Test + fun confidenceMapsToTheIosRawValues() { + assertEquals("known", NutritionTools.decodeConfidenceRaw("high")) + assertEquals("partial", NutritionTools.decodeConfidenceRaw("medium")) + assertEquals("unknown", NutritionTools.decodeConfidenceRaw("low")) + assertEquals("unknown", NutritionTools.decodeConfidenceRaw("bogus")) + assertEquals("unknown", NutritionTools.decodeConfidenceRaw(null)) + } + + @Test + fun mealTypeSetMatchesTheAppPickers() { + // The same four raw strings MealLogDialog's chips offer (iOS MealType rawValues). + for (t in listOf("breakfast", "lunch", "dinner", "snack")) { + assertTrue(t in NutritionTools.mealTypeRawValues) + } + assertFalse("brunch" in NutritionTools.mealTypeRawValues) + } + + // ── search limit clamp + query validation ────────────────────────── + + @Test + fun searchLimitClampsTo1Through5() { + // iOS: min(5, max(1, Int(maxResults ?? 5))) + assertEquals(1, NutritionTools.clampSearchLimit(0.0)) + assertEquals(1, NutritionTools.clampSearchLimit(-3.0)) + assertEquals(5, NutritionTools.clampSearchLimit(99.0)) + assertEquals(3, NutritionTools.clampSearchLimit(3.0)) + assertEquals(2, NutritionTools.clampSearchLimit(2.9)) // Int() truncates + assertEquals(5, NutritionTools.clampSearchLimit(null)) + } + + @Test + fun searchQueryRequiresTwoCharsAfterTrim() { + assertEquals("query too short", NutritionTools.searchQueryError("a")) + assertEquals("query too short", NutritionTools.searchQueryError(" ")) + assertNull(NutritionTools.searchQueryError("ab")) + assertNull(NutritionTools.searchQueryError(" apples ")) + } + + // ── per-product payload ──────────────────────────────────────────── + + @Test + fun payloadCarriesOptionalsOnlyWhenPresent() { + val full = FoodProduct( + code = "301", name = "Yogurt", brand = "Acme", energyKcal100g = 97.4, + protein100g = 10.0, carbs100g = 12.0, fat100g = 3.0, + servingSizeText = "1 cup (240 ml)", servingQuantityG = 240.0, + ) + val obj = NutritionTools.foodProductPayload(full).jsonObject + assertEquals("301", obj["code"]!!.jsonPrimitive.content) + assertEquals("Yogurt", obj["name"]!!.jsonPrimitive.content) + assertEquals("Acme", obj["brand"]!!.jsonPrimitive.content) + assertEquals("1 cup (240 ml)", obj["serving"]!!.jsonPrimitive.content) + assertEquals(240.0, obj["serving_g"]!!.jsonPrimitive.content.toDoubleOrNull()!!, 1e-9) + val per100 = obj["per_100g"]!!.jsonObject + // kcal is rounded to an integer, like iOS's energyKcal100g.rounded(). + assertEquals(97.0, per100["kcal"]!!.jsonPrimitive.content.toDoubleOrNull()!!, 1e-9) + assertEquals(10.0, per100["protein_g"]!!.jsonPrimitive.content.toDoubleOrNull()!!, 1e-9) + assertEquals(12.0, per100["carbs_g"]!!.jsonPrimitive.content.toDoubleOrNull()!!, 1e-9) + assertEquals(3.0, per100["fat_g"]!!.jsonPrimitive.content.toDoubleOrNull()!!, 1e-9) + + val bare = FoodProduct(code = "302", name = "Plain", energyKcal100g = 100.6) + val bareObj = NutritionTools.foodProductPayload(bare).jsonObject + assertFalse(bareObj.containsKey("brand")) + assertFalse(bareObj.containsKey("serving")) + assertFalse(bareObj.containsKey("serving_g")) + assertEquals(101.0, bareObj["per_100g"]!!.jsonObject["kcal"]!!.jsonPrimitive.content.toDoubleOrNull()!!, 1e-9) + } + + // ── applyMealUpdates ─────────────────────────────────────────────── + + private fun entry(sourceRaw: String) = MealEntryEntity( + date = 0L, timestamp = 0L, name = "Oatmeal", mealTypeRaw = "breakfast", + calories = 300.0, sourceRaw = sourceRaw, + ) + + @Test + fun numericChangeMarksADatabaseRowEdited() { + // iOS: a user-requested correction to a database/estimate row marks it edited. + val updated = NutritionTools.applyMealUpdates(MealUpdates(calories = 320.0), entry("off_search")) + assertEquals(320.0, updated.calories, 1e-9) + assertTrue(updated.userEdited) + } + + @Test + fun numericChangeDoesNotMarkAManualRowEdited() { + val updated = NutritionTools.applyMealUpdates(MealUpdates(calories = 320.0), entry("manual")) + assertEquals(320.0, updated.calories, 1e-9) + assertFalse(updated.userEdited) + } + + @Test + fun nonNumericChangeDoesNotMarkEdited() { + val updated = NutritionTools.applyMealUpdates( + MealUpdates(name = "Oats", notes = "had berries"), entry("off_search")) + assertEquals("Oats", updated.name) + assertEquals("had berries", updated.notes) + assertFalse(updated.userEdited) + } + + @Test + fun unknownMealTypeIsIgnoredNotAnError() { + // iOS guards with MealType(rawValue:) — an invalid type is simply not applied. + val updated = NutritionTools.applyMealUpdates(MealUpdates(mealType = "brunch"), entry("off_search")) + assertEquals("breakfast", updated.mealTypeRaw) + assertFalse(updated.userEdited) + } + + @Test + fun allNullUpdatesOnlyBumpUpdatedAt() { + val e = entry("llm_estimate") + val updated = NutritionTools.applyMealUpdates(MealUpdates(), e) + assertEquals(e.copy(updatedAt = updated.updatedAt), updated) + } + + @Test + fun localTimeStringFormatsHourAndMinute() { + // Pure conversion — no parseLocalDate involved — so a fixed zone is safe. + val utc = ZoneId.of("UTC") + assertEquals( + "08:05", + NutritionTools.localTimeString(Instant.parse("2026-08-22T08:05:00Z").toEpochMilli(), utc), + ) + assertEquals( + "23:59", + NutritionTools.localTimeString(Instant.parse("2026-08-22T23:59:00Z").toEpochMilli(), utc), + ) + } +} diff --git a/app/src/test/java/com/pulseloop/notifications/CoachNotificationDataTriggerTest.kt b/app/src/test/java/com/pulseloop/notifications/CoachNotificationDataTriggerTest.kt new file mode 100644 index 0000000..e53b65f --- /dev/null +++ b/app/src/test/java/com/pulseloop/notifications/CoachNotificationDataTriggerTest.kt @@ -0,0 +1,94 @@ +package com.pulseloop.notifications + +import com.pulseloop.ring.PulseEvent +import com.pulseloop.ring.RingConnectionState +import java.time.Instant +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * Tests for [CoachNotificationDataTrigger]'s handle() contract — the iOS #94 + * subscriber behavior: it fires only on SyncProgress("done"), coalesces + * back-to-back completions into a single attempt after the settle window, and + * stays silent when the feature is off. + * + * These drive the internal handle() directly on a virtual Main dispatcher rather + * than publishing through the shared [PulseEventBus]: the bus fans out on real + * Dispatchers.Default threads, and bridging that into a virtual test clock is + * racy (it was a flaky failure). The bus itself is covered by PulseEventBusTest, + * and the one-line events.collect { handle(it) } wiring in start() is exercised + * in production, so the deterministic handle() tests here are the meaningful + * slice. + */ +class CoachNotificationDataTriggerTest { + + @After + fun tearDown() { + Dispatchers.resetMain() + } + + private fun settings( + coachEnabled: Boolean = true, + notificationsEnabled: Boolean = true, + ): () -> CoachCheckinSettings = { + CoachCheckinSettings(coachEnabled, notificationsEnabled, "sk-test", "gpt-5.4") + } + + @Test + fun `fires only on a completed sync and coalesces back-to-back completions`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + var calls = 0 + val trigger = CoachNotificationDataTrigger( + checkinSettings = settings(), + runDueSlot = { + calls++ + CoachNotificationOutcome.Sent(CoachNotificationSlot.MORNING) + }, + ) + try { + // Other sync stages and unrelated events are ignored — no settle window is armed: + trigger.handle(PulseEvent.SyncProgress("Syncing sleep…")) + trigger.handle(PulseEvent.HeartRateSample(72, Instant.now())) + trigger.handle(PulseEvent.BatteryLevel(88)) + trigger.handle(PulseEvent.DeviceStateChanged(RingConnectionState.CONNECTED, "AA:BB:CC")) + advanceUntilIdle() + assertEquals(0, calls) + + // A full sync completion arms the settle window; a back-to-back completion + // coalesces into the same single attempt (the earlier debounce is cancelled). + trigger.handle(PulseEvent.SyncProgress("done")) + trigger.handle(PulseEvent.SyncProgress("done")) + advanceUntilIdle() + assertEquals(1, calls) + } finally { + trigger.destroy() + } + } + + @Test + fun `a disabled feature never wakes the runner`() = runTest { + Dispatchers.setMain(UnconfinedTestDispatcher(testScheduler)) + var calls = 0 + val trigger = CoachNotificationDataTrigger( + checkinSettings = settings(coachEnabled = false), + runDueSlot = { + calls++ + CoachNotificationOutcome.Sent(CoachNotificationSlot.MORNING) + }, + ) + try { + trigger.handle(PulseEvent.SyncProgress("done")) + advanceUntilIdle() + assertEquals(0, calls) + } finally { + trigger.destroy() + } + } +} diff --git a/app/src/test/java/com/pulseloop/notifications/CoachNotificationSlotRunnerTest.kt b/app/src/test/java/com/pulseloop/notifications/CoachNotificationSlotRunnerTest.kt new file mode 100644 index 0000000..86ace03 --- /dev/null +++ b/app/src/test/java/com/pulseloop/notifications/CoachNotificationSlotRunnerTest.kt @@ -0,0 +1,225 @@ +package com.pulseloop.notifications + +import com.pulseloop.data.dao.CoachNotificationRecordDao +import com.pulseloop.data.entity.CoachNotificationRecordEntity +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.async +import kotlinx.coroutines.test.runTest +import java.time.LocalDateTime +import java.time.ZoneId +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Gate tests for [CoachNotificationSlotRunner] — the iOS #94 contract: the + * in-flight guard, per-(day, slot) dedupe, the disabled gate, and the two + * "not now, not yet" skips (stale data, unsynced sleep) that are deliberately + * NOT recorded so the data trigger can fire the slot later. + * + * The app has no in-memory-Room/Robolectric harness (see ActivityAggregatesTest's + * note), so the runner is exercised against an in-memory fake of the small + * [CoachNotificationRecordDao] interface — the dedupe query runs against the + * same rows the fake inserts, so the key logic is the real one. + */ +class CoachNotificationSlotRunnerTest { + + /** In-memory [CoachNotificationRecordDao]: inserts and the dedupe EXISTS query + * share one row list, so tests exercise the real interface + real key logic. */ + private class InMemoryRecordDao : CoachNotificationRecordDao { + val records = mutableListOf() + + override suspend fun insert(record: CoachNotificationRecordEntity) { + records += record + } + + override suspend fun recent(limit: Int): List = + records.sortedByDescending { it.createdAt }.take(limit) + + override suspend fun existsForDateKeyAndSlot(dateKey: Long, slotRaw: String): Boolean = + records.any { it.dateKey == dateKey && it.slotRaw == slotRaw } + + override suspend fun clear() { + records.clear() + } + } + + /** Mutable harness: one runner wired to in-memory fakes, all knobs default to + * "healthy, fresh, inside the morning window, feature enabled." */ + private class Harness( + var coachEnabled: Boolean = true, + var notificationsEnabled: Boolean = true, + var apiKey: String = "sk-test", + var fresh: Boolean = true, + var latestMeasurementAt: Long? = null, + var sleepSessionEndAt: Long? = null, + var deviceFullSyncAt: Long? = null, + var policy: CoachStaleDataPolicy = CoachStaleDataPolicy.SKIP, + ) { + val recordDao = InMemoryRecordDao() + val delivered = mutableListOf>() + var sleepRetriesScheduled = 0 + var generateGate: CompletableDeferred? = null + + // Lambdas read the mutable knobs via this. so a test mutating a knob + // (h.fresh = false, ...) is seen by the runner on its next run. + val runner = CoachNotificationSlotRunner( + settings = { + CoachCheckinSettings( + this.coachEnabled, this.notificationsEnabled, this.apiKey, "gpt-5.4", + ) + }, + recordDao = recordDao, + latestSleepSessionEndAt = { this.sleepSessionEndAt }, + currentDeviceFullSyncAt = { this.deviceFullSyncAt }, + latestMeasurementTimestamp = { this.latestMeasurementAt }, + staleDataPolicy = policy, + ensureFreshData = { this.fresh }, + generate = { slot, _ -> + this.generateGate?.await() + CoachNotificationContent("AI-${slot.name}", "ai body") + }, + deliver = { t, b -> this.delivered += t to b }, + onSleepRetryNeeded = { this.sleepRetriesScheduled++ }, + clock = { NOW_MORNING }, + ) + } + + private companion object { + private fun at(local: LocalDateTime): Long = + local.atZone(ZoneId.systemDefault()).toInstant().toEpochMilli() + + /** 09:00 — inside the default morning window (08:00–12:00). */ + val NOW_MORNING = at(LocalDateTime.of(2026, 8, 22, 9, 0)) + val NOW_MORNING_TOMORROW = at(LocalDateTime.of(2026, 8, 23, 9, 0)) + /** 15:00 — inside no slot window. */ + val NOW_AFTERNOON = at(LocalDateTime.of(2026, 8, 22, 15, 0)) + } + + @Test + fun `a due slot sends once and records the dedupe key`() = runTest { + val h = Harness() + assertEquals( + CoachNotificationOutcome.Sent(CoachNotificationSlot.MORNING), + h.runner.runDueSlot(), + ) + assertEquals(1, h.recordDao.records.size) + val rec = h.recordDao.records.single() + assertEquals(CoachNotificationSlotRunner.dateKeyFor(NOW_MORNING), rec.dateKey) + assertEquals("morning", rec.slotRaw) + assertEquals(listOf("AI-MORNING" to "ai body"), h.delivered) + } + + @Test + fun `a second run for the same day and slot is skippedDuplicate`() = runTest { + val h = Harness() + assertEquals(CoachNotificationOutcome.Sent(CoachNotificationSlot.MORNING), h.runner.runDueSlot()) + assertEquals(CoachNotificationOutcome.SkippedDuplicate, h.runner.runDueSlot()) + // No double record, no double delivery — the worker/data-trigger safety net. + assertEquals(1, h.recordDao.records.size) + assertEquals(1, h.delivered.size) + } + + @Test + fun `the same slot the next day is not a duplicate`() = runTest { + val h = Harness() + assertEquals(CoachNotificationOutcome.Sent(CoachNotificationSlot.MORNING), h.runner.runDueSlot()) + assertEquals( + CoachNotificationOutcome.Sent(CoachNotificationSlot.MORNING), + h.runner.runDueSlot(now = NOW_MORNING_TOMORROW), + ) + assertEquals(2, h.recordDao.records.size) + } + + @Test + fun `outside every slot window is skippedNoSlot`() = runTest { + val h = Harness() + assertEquals(CoachNotificationOutcome.SkippedNoSlot, h.runner.runDueSlot(now = NOW_AFTERNOON)) + assertTrue(h.recordDao.records.isEmpty()) + assertTrue(h.delivered.isEmpty()) + } + + @Test + fun `a concurrent entry while a run is in flight is skippedDuplicate`() = runTest { + val h = Harness() + val gate = CompletableDeferred() + h.generateGate = gate + + // The first run holds the static in-flight guard while generation awaits. + val first = async(start = CoroutineStart.UNDISPATCHED) { h.runner.runDueSlot() } + assertEquals(CoachNotificationOutcome.SkippedDuplicate, h.runner.runDueSlot()) + + gate.complete(CoachNotificationContent("done", "body")) + assertEquals(CoachNotificationOutcome.Sent(CoachNotificationSlot.MORNING), first.await()) + assertEquals(1, h.recordDao.records.size) + assertEquals(1, h.delivered.size) + } + + @Test + fun `a disabled coach or opt-in is skippedDisabled with no delivery`() = runTest { + val coachOff = Harness(coachEnabled = false) + assertEquals(CoachNotificationOutcome.SkippedDisabled, coachOff.runner.runDueSlot()) + assertTrue(coachOff.recordDao.records.isEmpty()) + assertTrue(coachOff.delivered.isEmpty()) + + val optInOff = Harness(notificationsEnabled = false) + assertEquals(CoachNotificationOutcome.SkippedDisabled, optInOff.runner.runDueSlot()) + assertTrue(optInOff.recordDao.records.isEmpty()) + assertTrue(optInOff.delivered.isEmpty()) + } + + @Test + fun `a stale-data skip is not recorded so a later fresh run still sends`() = runTest { + val h = Harness() + h.fresh = false + assertEquals(CoachNotificationOutcome.SkippedStaleData, h.runner.runDueSlot()) + // The whole point of iOS #94: nothing was recorded, so the slot is still + // deliverable. + assertTrue(h.recordDao.records.isEmpty()) + assertTrue(h.delivered.isEmpty()) + + // A full sync lands (what the data trigger is waiting for) — the slot fires. + h.fresh = true + assertEquals(CoachNotificationOutcome.Sent(CoachNotificationSlot.MORNING), h.runner.runDueSlot()) + assertEquals(1, h.recordDao.records.size) + assertEquals(1, h.delivered.size) + } + + @Test + fun `sendWithLastKnown sends with data but skips an empty store`() = runTest { + val h = Harness(policy = CoachStaleDataPolicy.SEND_WITH_LAST_KNOWN) + h.fresh = false + assertEquals(CoachNotificationOutcome.SkippedNoData, h.runner.runDueSlot()) + h.latestMeasurementAt = NOW_MORNING - 60_000L + assertEquals(CoachNotificationOutcome.Sent(CoachNotificationSlot.MORNING), h.runner.runDueSlot()) + assertEquals(1, h.recordDao.records.size) + } + + @Test + fun `a morning slot blocked on sleep data is not recorded and schedules the retry`() = runTest { + val h = Harness() + // Last night ended 5h ago (recent) but the last full sync is OLDER than that — + // the one shape CoachSleepSyncGate.sleepDataSynced blocks on. + h.sleepSessionEndAt = NOW_MORNING - 5 * 3600_000L + h.deviceFullSyncAt = NOW_MORNING - 10 * 3600_000L + + assertEquals(CoachNotificationOutcome.SkippedNoSleepData, h.runner.runDueSlot()) + assertTrue(h.recordDao.records.isEmpty()) + assertTrue(h.delivered.isEmpty()) + assertEquals(1, h.sleepRetriesScheduled) + } + + @Test + fun `a missing api key delivers the generic check-in and records the slot`() = runTest { + val h = Harness(apiKey = "") + assertEquals(CoachNotificationOutcome.Sent(CoachNotificationSlot.MORNING), h.runner.runDueSlot()) + assertEquals( + listOf(CoachNotificationSlotRunner.GENERIC_TITLE to CoachNotificationSlotRunner.GENERIC_BODY), + h.delivered, + ) + // ...and a later trigger run the same day (fresh data landing) can't double-send. + assertEquals(CoachNotificationOutcome.SkippedDuplicate, h.runner.runDueSlot()) + assertEquals(1, h.recordDao.records.size) + } +} diff --git a/app/src/test/java/com/pulseloop/nutrition/NutritionMathTest.kt b/app/src/test/java/com/pulseloop/nutrition/NutritionMathTest.kt new file mode 100644 index 0000000..1cff9a3 --- /dev/null +++ b/app/src/test/java/com/pulseloop/nutrition/NutritionMathTest.kt @@ -0,0 +1,54 @@ +package com.pulseloop.nutrition + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** Unit tests for the pure conversions in [NutritionMath] — OFF's unit quirks live here. */ +class NutritionMathTest { + @Test + fun kcalPerKJIsTheReciprocalOf4184() { + assertEquals(1.0 / 4.184, NutritionMath.kcalPerKJ, 0.0) + } + + @Test + fun scaledScalesPer100gToGrams() { + assertEquals(100.0, NutritionMath.scaled(per100g = 250.0, grams = 40.0), 1e-9) + assertEquals(539.0, NutritionMath.scaled(per100g = 539.0, grams = 100.0), 1e-9) + assertEquals(0.0, NutritionMath.scaled(per100g = 539.0, grams = 0.0), 1e-9) + } + + @Test + fun energyKcalPrefersKcalWhenBothArePresent() { + // kcal wins even when kJ is also present — 2250 kJ would be ~537.8 kcal, not 539. + assertEquals(539.0, NutritionMath.energyKcal(kcal = 539.0, kJ = 2250.0)!!, 1e-9) + } + + @Test + fun energyKcalFallsBackToKJWhenKcalIsMissing() { + assertEquals(500.0, NutritionMath.energyKcal(kcal = null, kJ = 2092.0)!!, 1e-9) + } + + @Test + fun energyKcalOfZeroKcalStillWins() { + // A present 0.0 kcal is data (a product can legitimately be recorded with 0 kcal), + // not absence — iOS's `if let kcal` guards on presence, not on non-zero. + assertEquals(0.0, NutritionMath.energyKcal(kcal = 0.0, kJ = 4184.0)!!, 1e-9) + } + + @Test + fun energyKcalIsNullWhenNeitherIsPresent() { + assertNull(NutritionMath.energyKcal(kcal = null, kJ = null)) + } + + @Test + fun sodiumMgConvertsGramsToMilligrams() { + assertEquals(42.8, NutritionMath.sodiumMg(fromGrams = 0.0428)!!, 1e-9) + assertEquals(0.0, NutritionMath.sodiumMg(fromGrams = 0.0)!!, 1e-9) + } + + @Test + fun sodiumMgIsNullForMissingGrams() { + assertNull(NutritionMath.sodiumMg(fromGrams = null)) + } +} diff --git a/app/src/test/java/com/pulseloop/nutrition/OFFProductDecodeTest.kt b/app/src/test/java/com/pulseloop/nutrition/OFFProductDecodeTest.kt new file mode 100644 index 0000000..45b426d --- /dev/null +++ b/app/src/test/java/com/pulseloop/nutrition/OFFProductDecodeTest.kt @@ -0,0 +1,187 @@ +package com.pulseloop.nutrition + +import kotlinx.serialization.json.Json +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.fail +import org.junit.Test + +/** + * Decode + normalization tests for the OFF wire DTOs (the port of OpenFoodFactsTypes.swift). + * The fixtures are shaped like real OFF payloads: hyphenated nutriments keys, string + * numbers, kJ-only energy, and both brands shapes. + */ +class OFFProductDecodeTest { + private val json = Json { ignoreUnknownKeys = true } + + /** (a) A valid v2 product decodes; sodium arrives in grams and comes back in mg. */ + @Test + fun validV2ProductDecodesWithSodiumInMilligrams() { + val raw = """ + { + "status": 1, + "product": { + "code": "3017620422003", + "product_name": "Nutella", + "brands": "Ferrero", + "nutriments": { + "energy-kcal_100g": 539, + "energy_100g": 2250, + "proteins_100g": 6.3, + "carbohydrates_100g": "57.5", + "fat_100g": 30.9, + "fiber_100g": 3.4, + "sugars_100g": 56.3, + "saturated-fat_100g": 10.6, + "sodium_100g": 0.0428 + }, + "serving_size": "15 g", + "serving_quantity": 15 + } + } + """.trimIndent() + val response = json.decodeFromString(OFFProductResponse.serializer(), raw) + assertEquals(1, response.status!!) + val product = requireNotNull(response.product?.asFoodProduct()) + assertEquals("3017620422003", product.code) + assertEquals("Nutella", product.name) + assertEquals("Ferrero", product.brand) + // kcal wins over the co-present kJ. + assertEquals(539.0, product.energyKcal100g, 1e-9) + assertEquals(6.3, product.protein100g, 1e-9) + // "57.5" arrived as a string — OFFNumber must decode it. + assertEquals(57.5, product.carbs100g, 1e-9) + assertEquals(30.9, product.fat100g, 1e-9) + assertEquals(3.4, product.fiber100g!!, 1e-9) + assertEquals(56.3, product.sugars100g!!, 1e-9) + assertEquals(10.6, product.saturatedFat100g!!, 1e-9) + // 0.0428 g sodium per 100g -> 42.8 mg. + assertEquals(42.8, product.sodiumMg100g!!, 1e-9) + assertEquals("15 g", product.servingSizeText) + assertEquals(15.0, product.servingQuantityG!!, 1e-9) + } + + /** (b) A row with no name is unusable — dropped, not surfaced. */ + @Test + fun rowWithoutNameIsDropped() { + val dto = json.decodeFromString( + OFFProductDTO.serializer(), + """{"code":"123","nutriments":{"energy-kcal_100g":100}}""", + ) + assertNull(dto.asFoodProduct()) + } + + /** A row with no energy in any form is dropped too. */ + @Test + fun rowWithoutEnergyIsDropped() { + val dto = json.decodeFromString( + OFFProductDTO.serializer(), + """{"code":"123","product_name":"Mystery","nutriments":{"proteins_100g":1}}""", + ) + assertNull(dto.asFoodProduct()) + } + + /** (c) Energy only in kJ converts to kcal — under BOTH kJ key spellings. */ + @Test + fun kJOnlyEnergyConvertsToKcal() { + // v2 product API spelling. + val v2 = json.decodeFromString( + OFFProductDTO.serializer(), + """{"code":"1","product_name":"X","nutriments":{"energy_100g":2092}}""", + ) + assertEquals(500.0, v2.asFoodProduct()!!.energyKcal100g, 1e-9) + + // Search-a-licious spelling. + val searchALicious = json.decodeFromString( + OFFProductDTO.serializer(), + """{"code":"2","product_name":"Y","nutriments":{"energy-kj_100g":2092}}""", + ) + assertEquals(500.0, searchALicious.asFoodProduct()!!.energyKcal100g, 1e-9) + } + + /** (d) v2's comma-separated brands string keeps only the first token. */ + @Test + fun commaSeparatedBrandsYieldFirstToken() { + val dto = json.decodeFromString( + OFFProductDTO.serializer(), + """{"code":"3","product_name":"Z","brands":"Ferrero, Nutella, Unbranded","nutriments":{"energy-kcal_100g":10}}""", + ) + assertEquals("Ferrero", dto.asFoodProduct()!!.brand) + } + + /** (e) Search-a-licious' brands array is accepted and its first token used. */ + @Test + fun brandsArrayIsAccepted() { + val dto = json.decodeFromString( + OFFProductDTO.serializer(), + """{"code":"4","product_name":"W","brands":["Ferrero","Nutella"],"nutriments":{"energy-kcal_100g":10}}""", + ) + assertEquals("Ferrero", dto.asFoodProduct()!!.brand) + } + + /** A malformed brands value must not fail the product decode (iOS's try? cascade). */ + @Test + fun malformedBrandsYieldNoBrandButKeepTheProduct() { + val dto = json.decodeFromString( + OFFProductDTO.serializer(), + """{"code":"5","product_name":"V","brands":42,"nutriments":{"energy-kcal_100g":10}}""", + ) + val product = dto.asFoodProduct() + assertNotNull(product) + assertNull(product!!.brand) + } + + /** (f) The lossy search array drops malformed elements and keeps the good ones. */ + @Test + fun lossySearchArrayDropsMalformedElements() { + val raw = """ + { + "hits": [ + {"code":"a","product_name":"Good One","brands":"B","nutriments":{"energy-kcal_100g":100}}, + "this whole element is not even an object", + {"code":"b","product_name":"Good Two","nutriments":{"energy-kj_100g":4184}}, + {"code":7,"product_name":"Bad code type","nutriments":{"energy-kcal_100g":5}} + ] + } + """.trimIndent() + val response = OFFSearchResponse.decode(json, raw) + val products = response.results.mapNotNull { it.asFoodProduct() } + assertEquals(2, products.size) + assertEquals(listOf("a", "b"), products.map { it.code }) + // The second good row carried kJ-only energy and was converted on the way in. + assertEquals(1000.0, products[1].energyKcal100g, 1e-9) + } + + /** The legacy products key works when hits is absent. */ + @Test + fun legacyProductsKeyIsUsedWhenHitsIsMissing() { + val raw = """{"products":[{"code":"a","product_name":"Good","nutriments":{"energy-kcal_100g":100}}]}""" + val response = OFFSearchResponse.decode(json, raw) + assertEquals(1, response.results.size) + assertEquals("a", response.results[0].code) + } + + /** hits wins when both keys are present (iOS `hits ?? products`). */ + @Test + fun hitsWinsOverProductsWhenBothArePresent() { + val raw = """ + { + "hits": [{"code":"hits","product_name":"H","nutriments":{"energy-kcal_100g":1}}], + "products": [{"code":"products","product_name":"P","nutriments":{"energy-kcal_100g":1}}] + } + """.trimIndent() + assertEquals("hits", OFFSearchResponse.decode(json, raw).results.single().code) + } + + /** A search body whose root is not an object is a decode failure, not an empty list. */ + @Test + fun nonObjectSearchBodyFails() { + try { + OFFSearchResponse.decode(json, "[1,2,3]") + fail("expected a decode failure") + } catch (expected: Exception) { + // parseToJsonElement succeeds (it is valid JSON) but jsonObject throws. + } + } +} diff --git a/app/src/test/java/com/pulseloop/nutrition/OpenFoodFactsClientTest.kt b/app/src/test/java/com/pulseloop/nutrition/OpenFoodFactsClientTest.kt new file mode 100644 index 0000000..90c1723 --- /dev/null +++ b/app/src/test/java/com/pulseloop/nutrition/OpenFoodFactsClientTest.kt @@ -0,0 +1,180 @@ +package com.pulseloop.nutrition + +import kotlinx.coroutines.runBlocking +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Before +import org.junit.Test + +/** + * Drives [OpenFoodFactsClient] against a real MockWebServer so the HTTP status mapping, + * the User-Agent requirement, and the fields= trim are tested through a real socket — + * the same approach [com.pulseloop.coach.openai.ResponsesHttpTest] uses for the coach + * client. The injectable host bases keep the production endpoints as the defaults. + */ +class OpenFoodFactsClientTest { + private lateinit var server: MockWebServer + private lateinit var client: OpenFoodFactsClient + + @Before + fun setUp() { + server = MockWebServer() + server.start() + val base = server.url("/").toString() + client = OpenFoodFactsClient(productBase = base, searchBase = base) + } + + @After + fun tearDown() { + server.shutdown() + } + + private fun v2ProductBody(status: Int = 1): String = """ + { + "status": $status, + "product": { + "code": "3017620422003", + "product_name": "Nutella", + "brands": "Ferrero", + "nutriments": {"energy-kcal_100g": 539, "sodium_100g": 0.0428}, + "serving_size": "15 g", + "serving_quantity": 15 + } + } + """.trimIndent() + + @Test + fun productFoundReturnsTheNormalizedProduct() = runBlocking { + server.enqueue(MockResponse().setBody(v2ProductBody())) + val product = client.product("3017620422003") + assertNotNull(product) + assertEquals("3017620422003", product!!.code) + assertEquals(539.0, product.energyKcal100g, 1e-9) + // Sodium arrives in grams on the wire and is stored in mg. + assertEquals(42.8, product.sodiumMg100g!!, 1e-9) + + val request = server.takeRequest() + assertEquals("/api/v2/product/3017620422003", request.requestUrl?.encodedPath) + // fields= trims the payload to what the app actually stores. + assertEquals(OpenFoodFactsClient.PRODUCT_FIELDS, request.requestUrl?.queryParameter("fields")) + // OFF requires a custom User-Agent identifying the app + contact. + assertTrue(request.getHeader("User-Agent")!!.startsWith("PulseLoop/")) + } + + /** OFF answers 404 for unknown barcodes — a valid "not found", not an error. */ + @Test + fun product404ReturnsNull() = runBlocking { + server.enqueue(MockResponse().setResponseCode(404).setBody("""{"status":0}""")) + assertNull(client.product("0000000000000")) + } + + /** status 0 over a 200 also means "not found". */ + @Test + fun productStatusZeroReturnsNull() = runBlocking { + server.enqueue(MockResponse().setBody(v2ProductBody(status = 0))) + assertNull(client.product("3017620422003")) + } + + /** 429 is RateLimited, never retried — the caller must back off and offer manual entry. */ + @Test + fun product429ThrowsRateLimited() = runBlocking { + server.enqueue(MockResponse().setResponseCode(429)) + try { + client.product("3017620422003") + fail("expected OpenFoodFactsError.RateLimited") + } catch (expected: OpenFoodFactsError.RateLimited) { + } + assertEquals("one attempt only — a 429 is an answer, not a retry trigger", 1, server.requestCount) + } + + /** Any other non-2xx is a distinct HttpStatus error. */ + @Test + fun product500ThrowsHttpStatus() = runBlocking { + server.enqueue(MockResponse().setResponseCode(500)) + val thrown = try { + client.product("3017620422003"); null + } catch (e: OpenFoodFactsError) { + e + } + val http = thrown as? OpenFoodFactsError.HttpStatus + assertNotNull("expected OpenFoodFactsError.HttpStatus, got $thrown", http) + assertEquals(500, http!!.code) + } + + /** A transport failure (connection refused) is a Network error, not a Decoding one. */ + @Test + fun unreachableServerSurfacesAsNetworkError() = runBlocking { + val offline = OpenFoodFactsClient(productBase = "http://127.0.0.1:1") + try { + offline.product("123") + fail("expected OpenFoodFactsError.Network") + } catch (expected: OpenFoodFactsError.Network) { + } + } + + /** A 200 whose body is not JSON is a Decoding error. */ + @Test + fun nonJsonBodySurfacesAsDecodingError() = runBlocking { + server.enqueue(MockResponse().setBody("this is not json")) + try { + client.product("123") + fail("expected OpenFoodFactsError.Decoding") + } catch (expected: OpenFoodFactsError.Decoding) { + } + } + + @Test + fun searchReturnsNormalizedResultsAndSendsTheSearchContract() = runBlocking { + server.enqueue( + MockResponse().setBody( + """ + { + "hits": [ + {"code":"a","product_name":"Choc One","brands":["Bar","Brand"],"nutriments":{"energy-kcal_100g":100}}, + "malformed element", + {"code":"b","product_name":"Choc Two","nutriments":{"energy-kj_100g":4184}} + ] + } + """.trimIndent(), + ), + ) + val results = client.search("dark chocolate") + assertEquals(2, results.size) + assertEquals(listOf("a", "b"), results.map { it.code }) + assertEquals("Bar", results[0].brand) + assertEquals(1000.0, results[1].energyKcal100g, 1e-9) + + val request = server.takeRequest() + assertEquals("/search", request.requestUrl?.encodedPath) + // Query parameters are URL-encoded on the wire and come back decoded here. + assertEquals("dark chocolate", request.requestUrl?.queryParameter("q")) + // Default page size is 10 (iOS signature default). + assertEquals("10", request.requestUrl?.queryParameter("page_size")) + assertEquals(OpenFoodFactsClient.PRODUCT_FIELDS, request.requestUrl?.queryParameter("fields")) + assertTrue(request.getHeader("User-Agent")!!.startsWith("PulseLoop/")) + } + + @Test + fun searchRespectsAnExplicitPageSize() = runBlocking { + server.enqueue(MockResponse().setBody("""{"hits":[]}""")) + client.search("oat", pageSize = 3) + assertEquals("3", server.takeRequest().requestUrl?.queryParameter("page_size")) + } + + @Test + fun search429ThrowsRateLimited() = runBlocking { + server.enqueue(MockResponse().setResponseCode(429)) + try { + client.search("chocolate") + fail("expected OpenFoodFactsError.RateLimited") + } catch (expected: OpenFoodFactsError.RateLimited) { + } + assertEquals(1, server.requestCount) + } +} diff --git a/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt b/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt index a4435cb..acdd946 100644 --- a/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt +++ b/app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt @@ -117,9 +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), 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. + // NOT RingDecodedEvent.Status: that bridges to DeviceStateChanged(CONNECTED), and a firmware + // reply says nothing about the connection — bridging it would restamp the device row as + // freshly connected every time a firmware string arrived. 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) @@ -152,6 +152,28 @@ class CRPDecoderTest { assertEquals(3, sent[5].toInt() and 0xFF) } + @Test + fun `a non-text firmware payload is acked rather than coerced into a version`() { + // Whatever decodeFirmwareVersion returns is shown verbatim in the Settings device card, so a + // payload that isn't a version string must ack. `String(bytes, UTF_8)` would have coerced + // the first into a U+FFFD run and the second into control junk, and published both as a + // firmware version. The second is the narrow-trim case: the vendor's `trim { it <= ' ' }` + // would have yielded "A" from it. + val payloads = listOf( + byteArrayOf(0xC3.toByte(), 0x28, 0xA0.toByte(), 0xFF.toByte()), // invalid UTF-8 + byteArrayOf(0x01, 0x02, 0x03, 0x41), // valid UTF-8, binary + ) + for (payload in payloads) { + val frame = CRPProtocol.frame(3, CRPCommands.CMD_QUERY_FIRMWARE_VERSION, payload) + val events = CRPDecoder.decode(frame, fdd3) + assertTrue( + "payload must not publish a version", + events.none { it is RingDecodedEvent.FirmwareRevision }, + ) + assertTrue(events.single() is RingDecodedEvent.CommandAck) + } + } + @Test fun `unrecognised group3 cmd is acked, not dropped`() { val ev = CRPDecoder.decode(CRPProtocol.frame(3, 2, byteArrayOf(0)), fdd3)[0] @@ -201,6 +223,26 @@ class CRPDecoderTest { assertEquals(1, driver.ingest(full.copyOfRange(4, full.size), fdd3).size) } + @Test + fun `connect is held until the command-reply channel is live`() { + // Without this override the connect counts as up on whichever notify char finishes its CCCD + // write first — for CRP that is fdd1 (steps), never fdd3 (every command reply), so the + // runStartup handshake would write into a channel we aren't listening to yet. + val driver = CRPDriver(null) + assertEquals( + listOf(RequiredSubscription(CRPUUIDs.CHAR_CMD_NOTIFY, SubscriptionMode.NOTIFICATION)), + driver.requiredSubscriptionsBeforeConnected, + ) + // A required subscription that isn't a declared notify char could never be satisfied, and + // the connect would fail its topology check — guard against it. + for (required in driver.requiredSubscriptionsBeforeConnected) { + assertTrue( + "${required.uuid} is not a declared notify characteristic", + driver.notifyUUIDs.any { it.equals(required.uuid, ignoreCase = true) }, + ) + } + } + // ── Sleep history (group 2 / cmd 14, vendor e1/j.b) ────────────────────────────────────── @Test diff --git a/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt b/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt index 5cab42e..0c1c95c 100644 --- a/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt +++ b/app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt @@ -25,29 +25,29 @@ class CRPSyncEngineTest { * `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) + /** The connection-scoped self-description queries, sent once per connection: the firmware + * version, SpO2 support type, then each all-day monitor's configured interval. + * See CRPSyncEngine.runStartup / sendConnectionQueries. */ + private val connectionQueries = listOf(3 to 3, 2 to 37, 2 to 6, 2 to 7, 2 to 8, 2 to 45, 2 to 21) /** The all-day monitor enables sent on connect (default ALL_ON): HR, HRV, stress, SpO2, temp — * see CRPSyncEngine.applyTimingSettings. Without these a fresh R11 records no history. */ private val timingEnables = listOf(1 to 6, 1 to 7, 1 to 39, 1 to 8, 1 to 13) @Test - fun `runStartup sends set-time, firmware query, user info, default monitor enables, then the history pull`() { - // The firmware query is 3/3 (`b1/l.k` -> d1/b.queryFirmwareVersion), NOT the 7/1 it used to - // send -- that opcode is the vendor's `querySavedGomoreKey` and the R11 never answers it. + fun `runStartup sends set-time, the connection queries, user info, default monitor enables, then the history pull`() { val w = FakeWriter() val engine = CRPSyncEngine(w) engine.runStartup() - // set-time, firmware query, read-backs, default-on monitor enables, then the history pull. + // set-time, then the once-per-connection self-description queries (firmware + read-backs), + // the default-on monitor enables, then the history pull. // - // The read-backs MUST precede the enables: they report each monitor's current interval, and - // 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. + // The connection queries MUST precede the enables: the state queries report each monitor's + // current interval, and 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.sendConnectionQueries. assertEquals( - listOf(1 to 1, 3 to 3) + readBackQueries + timingEnables + historyQueries + sleepBackfill, + listOf(1 to 1) + connectionQueries + timingEnables + historyQueries + sleepBackfill, w.opcodes(), ) @@ -56,11 +56,12 @@ class CRPSyncEngineTest { 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, and + // A second pass on the same connection re-sends the poll work but NOT the connection + // queries (firmware included — a firmware string is as immutable as the sensor roster) 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()) + assertEquals(listOf(1 to 1, 1 to 0) + timingEnables + historyQueries, w.opcodes()) } @Test @@ -80,28 +81,48 @@ class CRPSyncEngineTest { /** * `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 - * six writes per pass to the single `fdd2` channel a spot SpO2 needs for ~48 s. A fresh engine is - * built per connection, so the next connection asks again. + * `refresh()`/`querySleep()`. Re-asking what the ring supports (and its firmware) on every one + * of those would add seven writes per pass to the single `fdd2` channel a spot SpO2 needs for + * ~48 s. A fresh engine is built per connection, so the next connection asks again. */ @Test - fun `read-backs are sent once per connection, not once per poll pass`() { + fun `connection queries are sent once per connection, not once per poll pass`() { val w = FakeWriter() val engine = CRPSyncEngine(w) engine.runStartup() - assertTrue(w.opcodes().containsAll(readBackQueries)) + assertTrue(w.opcodes().containsAll(connectionQueries)) w.sent.clear() engine.runStartup() engine.runStartup() - for (q in readBackQueries) { - assertTrue("read-back $q must not repeat within a connection", q !in w.opcodes()) + for (q in connectionQueries) { + assertTrue("connection query $q must not repeat within a connection", q !in w.opcodes()) } // A new connection builds a new engine, which asks again. val reconnected = FakeWriter() CRPSyncEngine(reconnected).runStartup() - assertTrue(reconnected.opcodes().containsAll(readBackQueries)) + assertTrue(reconnected.opcodes().containsAll(connectionQueries)) + } + + @Test + fun `firmware is asked once per connection, not on every poll pass`() { + // runStartup IS the ~30-minute background sync. A firmware string is exactly as immutable + // as the sensor roster gated beside it, and fdd2 is the scarce channel (a spot SpO2 needs + // ~48 s of it). + val w = FakeWriter() + val engine = CRPSyncEngine(w) + engine.runStartup() + assertTrue("firmware asked on the first pass", (3 to 3) in w.opcodes()) + + w.sent.clear() + engine.runStartup() + assertTrue("firmware must not repeat every pass", (3 to 3) !in w.opcodes()) + + // A new connection builds a new engine, which asks again. + val reconnected = FakeWriter() + CRPSyncEngine(reconnected).runStartup() + assertTrue((3 to 3) in reconnected.opcodes()) } @Test @@ -191,6 +212,22 @@ class CRPSyncEngineTest { assertEquals(1, w.sent.size) } + @Test + fun `the follow-up guard distinguishes days`() { + // The engine already issues multi-day sleep requests (sendSleepBackfill); the moment the + // timing vitals get the same backfill, a key without `day` would silently swallow day 1's + // frame-1 follow-up. A different day must be a different follow-up. + val w = FakeWriter() + val engine = CRPSyncEngine(w) + engine.runStartup(); w.sent.clear() + engine.handle(RingDecodedEvent.TimingHistoryFrame(cmd = CRPCommands.CMD_QUERY_TIMING_HR, day = 0, frameIndex = 0)) + engine.handle(RingDecodedEvent.TimingHistoryFrame(cmd = CRPCommands.CMD_QUERY_TIMING_HR, day = 1, frameIndex = 0)) + assertEquals("a different day is a different follow-up", 2, w.sent.size) + // queryTimingHeartRateHistory frames the [day][frameIndex] payload at frame bytes 6/7. + assertEquals(0, w.sent[0][6].toInt()) // day 0 in the payload + assertEquals(1, w.sent[1][6].toInt()) // day 1 + } + @Test fun `applyUserProfile pushes user info immediately`() { val w = FakeWriter() diff --git a/app/src/test/java/com/pulseloop/ring/RWfitDriverTest.kt b/app/src/test/java/com/pulseloop/ring/RWfitDriverTest.kt index ef5e874..8d46ecf 100644 --- a/app/src/test/java/com/pulseloop/ring/RWfitDriverTest.kt +++ b/app/src/test/java/com/pulseloop/ring/RWfitDriverTest.kt @@ -193,7 +193,11 @@ class RWfitDriverTest { } @Test - fun `a JieLi link does not request legacy history`() { + fun `a JieLi link requests its history streams as bare triples after the handshake`() { + // JieLi has no manifest: the vendor requests each 05-group stream directly with the bare + // {5, type, 0x10} triple, no payload (blesdk/service/y.java:345-537, e.g. + // TRingHeartRateStatisticsActivity.java:545). The burst covers every stream RWfitJLHistory + // decodes, in HistoryType order (breathe has no JieLi type and drops out). val writer = RecordingWriter() val driver = RWfitDriver(writer) driver.connectionDidStart() @@ -201,9 +205,110 @@ class RWfitDriverTest { driver.makeSyncEngine().runStartup() - // Device info, time and battery only — the 05-group history bodies aren't decodable yet, so - // requesting them would spend the link on frames we could only log. - assertEquals(3, writer.frames.size) + assertTrue("expected 0xAB frames", writer.frames.all { it[0] == 0xAB.toByte() }) + assertEquals(12, writer.frames.size) // device info + time + battery + 9 history streams + val triples = writer.frames.map { Triple(it[6].toInt() and 0xFF, it[7].toInt() and 0xFF, it[8].toInt() and 0xFF) } + assertEquals( + listOf( + Triple(2, 4, 0x10), // device info + Triple(2, 1, 0), // time sync + Triple(2, 3, 0x10), // battery + Triple(5, 2, 0x10), // steps + Triple(5, 5, 0x10), // sleep + Triple(5, 3, 0x10), // heart rate + Triple(5, 4, 0x10), // blood pressure + Triple(5, 9, 0x10), // SpO2 + Triple(5, 8, 0x10), // temperature + Triple(5, 10, 0x10), // HRV + Triple(5, 13, 0x10), // stress + Triple(5, 16, 0x10), // blood sugar + ), + triples, + ) + } + + @Test + fun `JieLi history is requested once per connection, not per poll pass`() { + // runStartup doubles as the ~30-minute background sync: the handshake frames go out again, + // but the 05-group burst must not — the ring would just re-send buffers we already hold. + val writer = RecordingWriter() + val driver = RWfitDriver(writer) + driver.connectionDidStart() + driver.servicesDiscovered(listOf(RWfitProtocol.SERVICE_UUID, RWfitProtocol.JIELI_SERVICE_UUID)) + fun historyFrames() = writer.frames.count { it[0] == 0xAB.toByte() && (it[6].toInt() and 0xFF) == 5 } + + driver.makeSyncEngine().runStartup() + assertEquals(9, historyFrames()) + driver.makeSyncEngine().runStartup() + assertEquals(9, historyFrames()) + } + + @Test + fun `a reconnected JieLi link re-requests its history`() { + // reset() runs on connectionDidEnd/Start, re-arming the once-per-connection gate. A real + // reconnect re-runs GATT discovery (servicesDiscovered) before runStartup, as in the + // production connect sequence. + val writer = RecordingWriter() + val driver = RWfitDriver(writer) + driver.connectionDidStart() + driver.servicesDiscovered(listOf(RWfitProtocol.SERVICE_UUID, RWfitProtocol.JIELI_SERVICE_UUID)) + driver.makeSyncEngine().runStartup() + + driver.connectionDidEnd() + driver.connectionDidStart() + driver.servicesDiscovered(listOf(RWfitProtocol.SERVICE_UUID, RWfitProtocol.JIELI_SERVICE_UUID)) + writer.clear() + driver.makeSyncEngine().runStartup() + + assertEquals(12, writer.frames.size) + assertEquals(9, writer.frames.count { (it[6].toInt() and 0xFF) == 5 }) + } + + @Test + fun `a JieLi history reply decodes through the driver and is acked`() { + // {5,3,16} heart-rate reply: two 6-byte records, the second a zero-bpm "no reading" slot + // the vendor drops (x5/b.java V @1318-1320). The frame is app-ACKed (flag 0x11) before the + // decode result is produced, as on every other JieLi inbound. + val writer = RecordingWriter() + val driver = RWfitDriver(writer) + driver.connectionDidStart() + driver.servicesDiscovered(listOf(RWfitProtocol.SERVICE_UUID, RWfitProtocol.JIELI_SERVICE_UUID)) + + fun be32(v: Long) = byteArrayOf( + ((v shr 24) and 0xFF).toByte(), ((v shr 16) and 0xFF).toByte(), + ((v shr 8) and 0xFF).toByte(), (v and 0xFF).toByte(), + ) + val records = be32(0x00_0B_C0_00) + byteArrayOf(72, 0x00) + + be32(0x00_0B_C0_3C) + byteArrayOf(0x00, 0x00) + val frame = RWfitJLCodec().encode(RWfitProtocol.JLTriple(0x05, 0x03, 0x10), records) + + val events = driver.ingest(frame, "n") + + val measurements = events.filterIsInstance() + assertEquals(1, measurements.size) + assertEquals(MeasurementKind.HEART_RATE, measurements[0].kind_field) + assertEquals(72.0, measurements[0].value, 0.0) + val ack = writer.frames.single() + assertEquals(0xAB.toByte(), ack[0]) + assertEquals(0x11, ack[1].toInt() and 0xFF) // FLAG_ACK + assertArrayEquals(byteArrayOf(0x05, 0x03, 0x10), ack.copyOfRange(6, 9)) + } + + @Test + fun `an unported JieLi history key still decodes to nothing and the frame is still acked`() { + // e.g. sport {5,14,16}: no PulseLoop metric, so the driver logs and drops the records + // (the frame itself is still ACKed — ACK-before-decode is a link discipline, not a + // parse verdict). + val writer = RecordingWriter() + val driver = RWfitDriver(writer) + driver.connectionDidStart() + driver.servicesDiscovered(listOf(RWfitProtocol.SERVICE_UUID, RWfitProtocol.JIELI_SERVICE_UUID)) + + val frame = RWfitJLCodec().encode(RWfitProtocol.JLTriple(0x05, 0x14, 0x10), ByteArray(16)) + + assertTrue(driver.ingest(frame, "n").isEmpty()) + val ack = writer.frames.single() + assertEquals(0x11, ack[1].toInt() and 0xFF) } @Test diff --git a/app/src/test/java/com/pulseloop/ring/RWfitJLHistoryTest.kt b/app/src/test/java/com/pulseloop/ring/RWfitJLHistoryTest.kt new file mode 100644 index 0000000..64bd7b4 --- /dev/null +++ b/app/src/test/java/com/pulseloop/ring/RWfitJLHistoryTest.kt @@ -0,0 +1,323 @@ +package com.pulseloop.ring + +import java.time.Instant +import java.util.TimeZone +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Vendor-layout oracles for the JieLi (`0xAB`) `05`-group history decoders, hand-assembled from + * the parser offsets in `x5/b.java` (`decompiled-rwfit-official/sources/`) rather than from the + * implementation. Every fixture comment cites the vendor line it was built from. + * + * Timestamps are asserted the same way `RWfitDecoderTest` asserts legacy ones — through the same + * tz correction the decoder applies, not against a hard-coded epoch, because the correction + * (`utils/b.java:250-252`, `getOffset(now)`) is timezone-dependent. [raw2000] inverts the + * decoder's conversion so a fixture stamped with a plain Unix instant comes back as that instant. + */ +class RWfitJLHistoryTest { + + /** The vendor's JieLi correction: `utils.b.i() / 1000` = `getOffset(now)` (`utils/b.java:250-252`). */ + private fun jlTzCorrection(): Long = + TimeZone.getDefault().getOffset(System.currentTimeMillis()) / 1000L + + /** Unix seconds → the ring's epoch-2000 stamp the decoder must turn back into those seconds. */ + private fun raw2000(unix: Long): Long = unix - RWfitJLHistory.JIELI_EPOCH_SECONDS + jlTzCorrection() + + private fun be32(v: Long) = byteArrayOf( + ((v shr 24) and 0xFF).toByte(), ((v shr 16) and 0xFF).toByte(), + ((v shr 8) and 0xFF).toByte(), (v and 0xFF).toByte(), + ) + + private fun be16(v: Int) = byteArrayOf(((v shr 8) and 0xFF).toByte(), (v and 0xFF).toByte()) + private fun be24(v: Int) = byteArrayOf( + ((v shr 16) and 0xFF).toByte(), ((v shr 8) and 0xFF).toByte(), (v and 0xFF).toByte(), + ) + + private val t0 = 1_723_000_000L + + /** One 6-byte series record: `[ts2000 u32][value byte @+4][pad]` (V/S/W/Y at `x5/b.java:1314,1371,1486,1150`). */ + private fun rec6(unix: Long, value: Int): ByteArray = be32(raw2000(unix)) + byteArrayOf(value.toByte(), 0x00) + + /** One 6-byte series record: `[ts2000 u32][value u16 @+4..5]` (U/R at `x5/b.java:1261,1097`) — the stride is exactly 6. */ + private fun rec6u16(unix: Long, value: Int): ByteArray = be32(raw2000(unix)) + be16(value) + + /** One 7-byte sleep record: `[ts2000 u32][model @+4][2 unused]` (Z at `x5/b.java:1537,1538`). */ + private fun recSleep(unix: Long, model: Int): ByteArray = + be32(raw2000(unix)) + byteArrayOf(model.toByte(), 0x00, 0x00) + + // ── Steps (a0, id -60) ─────────────────────────────────────────────────────── + + @Test + fun `steps decodes 16-byte records with 3-byte count and decimetre distance`() { + // a0() @1558-1574: [ts u32][pad @+4][steps d(i+5,i+7) @1570][calorie d(i+8,i+11)/10 @1571] + // [distance d(i+12,i+15)/10000 @1572], stride 16 @1573. Distance raw is decimetres, so + // metres = raw/10 (the vendor renders raw/10000 as km). The middle record has 0 steps and + // is dropped, as in the iOS port's bucket filter. + val rec1 = be32(raw2000(t0)) + byteArrayOf(0x00) + be24(8421) + be32(3100) + be32(124_000) + val rec2 = be32(raw2000(t0 + 3600)) + byteArrayOf(0x00) + be24(0) + be32(0) + be32(0) + val rec3 = be32(raw2000(t0 + 7200)) + byteArrayOf(0x00) + be24(1234) + be32(450) + be32(2_000) + + val events = RWfitJLHistory.decodeSteps(rec1 + rec2 + rec3) + + assertEquals(2, events.size) + val b1 = events[0] as RingDecodedEvent.ActivityBucket + assertEquals(8421, b1.steps) + assertEquals(12_400, b1.distanceMeters) // 124000 raw decimetres / 10 + assertEquals(t0, b1._timestamp.epochSecond) + val b2 = events[1] as RingDecodedEvent.ActivityBucket + assertEquals(1234, b2.steps) + assertEquals(200, b2.distanceMeters) + assertEquals(t0 + 7200, b2._timestamp.epochSecond) + } + + @Test + fun `steps timestamp is epoch-2000 base minus the getOffset correction`() { + // A raw stamp of 0 must land on 2000-01-01T00:00:00Z minus the zone correction — the + // `+ 946684800` at a0() @1562, not the 2001 base the old triage notes carried. + val rec = be32(0) + byteArrayOf(0x00) + be24(1) + be32(0) + be32(0) + val event = (RWfitJLHistory.decodeSteps(rec).single() as RingDecodedEvent.ActivityBucket) + assertEquals( + Instant.ofEpochSecond(RWfitJLHistory.JIELI_EPOCH_SECONDS - jlTzCorrection()), + event._timestamp, + ) + } + + @Test + fun `steps ignores a partial 16-byte tail`() { + // The vendor loop's guard only tests the record start (a0 @1555) and would run off the end + // of a torn body; the port stops at the boundary instead — identical on well-formed bodies. + val full = be32(raw2000(t0)) + byteArrayOf(0x00) + be24(500) + be32(10) + be32(100) + val torn = full + full.copyOfRange(0, 10) + assertEquals(1, RWfitJLHistory.decodeSteps(torn).size) + assertTrue(RWfitJLHistory.decodeSteps(ByteArray(0)).isEmpty()) + } + + // ── Heart rate (V, id -62) ────────────────────────────────────────────────── + + @Test + fun `heart rate decodes 6-byte records and drops zero bpm`() { + // V() @1296-1321: [ts u32][hr @+4 @1314][pad], stride 6 @1317; the vendor drops + // `hr == 0` itself (@1318-1320) — a "no reading" slot, not a 0 bpm sample. + val p = rec6(t0, 72) + rec6(t0 + 60, 0) + rec6(t0 + 120, 88) + byteArrayOf(0xDE.toByte(), 0xAD.toByte(), 0xBE.toByte()) // torn tail + val events = RWfitJLHistory.decodeHeartRate(p).filterIsInstance() + + assertEquals(2, events.size) + assertEquals(MeasurementKind.HEART_RATE, events[0].kind_field) + assertEquals(72.0, events[0].value, 0.0) + assertEquals(t0, events[0]._timestamp.epochSecond) + assertEquals(88.0, events[1].value, 0.0) + assertEquals(t0 + 120, events[1]._timestamp.epochSecond) + } + + // ── Blood pressure (T, id -64) ────────────────────────────────────────────── + + @Test + fun `blood pressure emits systolic and diastolic from a 6-byte record`() { + // T() @1187-1210: [ts u32][sp @+4 @1205-1207][dp @+5 @1208], stride 6 @1209. A zero in + // either field is a "no sample" slot and yields nothing. + val body = + be32(raw2000(t0)) + byteArrayOf(120.toByte(), 78) + + be32(raw2000(t0 + 60)) + byteArrayOf(0, 78) + + be32(raw2000(t0 + 120)) + byteArrayOf(120.toByte(), 0) + + val events = RWfitJLHistory.decodeBloodPressure(body).filterIsInstance() + + assertEquals(2, events.size) + assertEquals(MeasurementKind.BLOOD_PRESSURE_SYSTOLIC, events[0].kind_field) + assertEquals(120.0, events[0].value, 0.0) + assertEquals(MeasurementKind.BLOOD_PRESSURE_DIASTOLIC, events[1].kind_field) + assertEquals(78.0, events[1].value, 0.0) + assertEquals(events[0]._timestamp, events[1]._timestamp) + assertEquals(t0, events[0]._timestamp.epochSecond) + } + + // ── Sleep (Z, id -66) ─────────────────────────────────────────────────────── + + @Test + fun `sleep reconstructs a session from stage-transition records`() { + // Z() @1523-1539 decodes the flat {time, model} stream; the session build is the vendor's + // consumer (s1.java): 0x11 opens (1004-1006), 0x22 closes (1008-1018), segment N spans the + // gap to record N+1 (1134-1135). The 0x11 marker's own segment counts as light (1149-1151). + val p = recSleep(t0, 0x11) + + recSleep(t0 + 600, 1) + // deep segment starts 10 min in + recSleep(t0 + 1200, 2) + // light segment starts 20 min in + recSleep(t0 + 1800, 0x22) // wakeup + + val timeline = (RWfitJLHistory.decodeSleep(p).single() as RingDecodedEvent.SleepTimeline) + + assertEquals(30, timeline.stages.size) + assertEquals(SleepStage.LIGHT, timeline.stages[0]) // the 0x11 marker's segment + assertEquals(SleepStage.DEEP, timeline.stages[10]) + assertEquals(SleepStage.LIGHT, timeline.stages[20]) + assertEquals(t0, timeline._timestamp.epochSecond) // anchored at the 0x11 marker + assertTrue(timeline.completeSession) + } + + @Test + fun `sleep maps the vendor stage bytes 1 deep 2 light 0 and 3 awake 4 rem`() { + // s1.java:1139-1157 — NOT the legacy 0/1/2/3 map (s1.java:1636-1645 is the 0x7E consumer). + val p = recSleep(t0, 0x11) + + recSleep(t0 + 60, 2) + + recSleep(t0 + 120, 1) + + recSleep(t0 + 180, 4) + + recSleep(t0 + 240, 3) + + recSleep(t0 + 300, 0) + + recSleep(t0 + 360, 0x22) + + val stages = (RWfitJLHistory.decodeSleep(p).single() as RingDecodedEvent.SleepTimeline).stages + + assertEquals(listOf(SleepStage.LIGHT, SleepStage.LIGHT, SleepStage.DEEP, SleepStage.REM, + SleepStage.AWAKE, SleepStage.AWAKE), stages) + } + + @Test + fun `sleep counts the session-start marker segment as light like the vendor`() { + // s1.java:1149-1151: the 0x11 marker's own segment (gap to the next record) is tallied + // into lightTime, so a night that is otherwise all awake still carries one light minute. + val p = recSleep(t0, 0x11) + + recSleep(t0 + 60, 0) + + recSleep(t0 + 120, 0x22) + + val stages = (RWfitJLHistory.decodeSleep(p).single() as RingDecodedEvent.SleepTimeline).stages + assertEquals(listOf(SleepStage.LIGHT, SleepStage.AWAKE), stages) + } + + @Test + fun `sleep emits nothing for a marker pair with no minutes and an unclosed tail`() { + // A back-to-back 0x11/0x22 pair has a zero-minute marker segment, so no stages accumulate + // (minutes > 0 guard, s1.java's delta division); a session without its 0x22 marker + // produces no DataSleep in the vendor either (s1.java:1113). + val p = recSleep(t0, 0x11) + + recSleep(t0, 0x22) + + recSleep(t0 + 7200, 0x11) + + recSleep(t0 + 7320, 1) // no wakeup marker follows + + assertTrue(RWfitJLHistory.decodeSleep(p).isEmpty()) + } + + @Test + fun `sleep decodes two closed sessions and drops a torn 7-byte tail`() { + val p = recSleep(t0, 0x11) + + recSleep(t0 + 120, 1) + + recSleep(t0 + 240, 0x22) + + recSleep(t0 + 3600, 0x11) + + recSleep(t0 + 3720, 2) + + recSleep(t0 + 3840, 0x22) + + recSleep(t0 + 7200, 0x11).copyOfRange(0, 4) // torn tail: 4 of 7 bytes + + val timelines = RWfitJLHistory.decodeSleep(p).filterIsInstance() + + assertEquals(2, timelines.size) + assertEquals(t0, timelines[0]._timestamp.epochSecond) + assertEquals(t0 + 3600, timelines[1]._timestamp.epochSecond) + } + + // ── Temperature (U, id -68) ───────────────────────────────────────────────── + + @Test + fun `temperature is raw u16 over 10 with no legacy plus-200 offset`() { + // U() @1243-1263: setTemp(d(i+4, i+5) / 10.0f) @1261 — the value is already °C×10 on the + // JieLi wire (the +200 encoding belongs to the 0x7E stream's u0()). Raw 0 = no sample. + val p = rec6u16(t0, 365) + rec6u16(t0 + 60, 0) + val events = RWfitJLHistory.decodeTemperature(p).filterIsInstance() + + assertEquals(1, events.size) + assertEquals(MeasurementKind.TEMPERATURE, events[0].kind_field) + assertEquals(36.5, events[0].value, 1e-9) + assertEquals(t0, events[0]._timestamp.epochSecond) + } + + // ── SpO2 (S, id -70) ──────────────────────────────────────────────────────── + + @Test + fun `spo2 decodes byte 4 and drops zero`() { + // S() @1132-1154: setBloodOxy(bArr[i+4] & 255) @1150-1152, stride 6 @1153. + val p = rec6(t0, 97) + rec6(t0 + 60, 0) + val events = RWfitJLHistory.decodeSpo2(p).filterIsInstance() + + assertEquals(1, events.size) + assertEquals(MeasurementKind.SPO2, events[0].kind_field) + assertEquals(97.0, events[0].value, 0.0) + assertEquals(t0, events[0]._timestamp.epochSecond) + } + + // ── HRV (W, id -72) ───────────────────────────────────────────────────────── + + @Test + fun `hrv decodes byte 4 and drops zero`() { + // W() @1353-1375: setHrv(bArr[i+4] & 255) @1371-1373, stride 6 @1374. + val p = rec6(t0, 42) + rec6(t0 + 60, 0) + val events = RWfitJLHistory.decodeHrv(p).filterIsInstance() + + assertEquals(1, events.size) + assertEquals(MeasurementKind.HRV, events[0].kind_field) + assertEquals(42.0, events[0].value, 0.0) + } + + // ── Stress (Y, id -74) ────────────────────────────────────────────────────── + + @Test + fun `stress decodes byte 4 and drops zero like the vendor`() { + // Y() @1467-1492: setPressure(bArr[i+4] & 255) @1486-1488, stride 6 @1489, and the vendor + // drops value == 0 itself (@1490-1492). + val p = rec6(t0, 33) + rec6(t0 + 60, 0) + val events = RWfitJLHistory.decodeStress(p).filterIsInstance() + + assertEquals(1, events.size) + assertEquals(MeasurementKind.STRESS, events[0].kind_field) + assertEquals(33.0, events[0].value, 0.0) + } + + // ── Blood sugar (R, id -112) ──────────────────────────────────────────────── + + @Test + fun `blood sugar is u16 over 10 mmol converted to the app unit mg per dL`() { + // R() @1085-1098: setSugar(d(i+4, i+5) / 10.0f) @1097 — the vendor displays mmol/L + // (SugarStatisticsFragment.java:439-442), and this app's BLOOD_SUGAR kind speaks mg/dL + // everywhere, so the port converts with the standard glucose factor (18.016), same + // convention as YCBTHealthRecords.bloodSugarMgdl. Raw 0 = no sample. + val p = rec6u16(t0, 56) + rec6u16(t0 + 60, 0) + val events = RWfitJLHistory.decodeBloodSugar(p).filterIsInstance() + + assertEquals(1, events.size) + assertEquals(MeasurementKind.BLOOD_SUGAR, events[0].kind_field) + assertEquals(5.6 * 18.016, events[0].value, 1e-9) + assertEquals(t0, events[0]._timestamp.epochSecond) + } + + // ── Dispatch ──────────────────────────────────────────────────────────────── + + @Test + fun `unknown 05 keys decode to null so the driver keeps logging them`() { + // Sport {5,14,16} (Q @1011), Muslim count {5,23,16} (X @1405) and the rest of the + // 05-group table (y5/c.java:105-166) have no PulseLoop metric — decode() must say "not + // ported", not fabricate a layout. The {.,.,0x30} delete variants have no vendor parser at + // all, so they land in the same branch. + assertNull(RWfitJLHistory.decode(0x0E, ByteArray(12))) + assertNull(RWfitJLHistory.decode(0x14, ByteArray(12))) + assertNull(RWfitJLHistory.decode(0x17, ByteArray(12))) + } + + @Test + fun `every ported key answers through the shared dispatch`() { + val keys = listOf( + RWfitProtocol.JLDataType.STEPS, + RWfitProtocol.JLDataType.HEART_RATE, + RWfitProtocol.JLDataType.BLOOD_PRESSURE, + RWfitProtocol.JLDataType.SLEEP, + RWfitProtocol.JLDataType.TEMPERATURE, + RWfitProtocol.JLDataType.SPO2, + RWfitProtocol.JLDataType.HRV, + RWfitProtocol.JLDataType.STRESS, + RWfitProtocol.JLDataType.BLOOD_SUGAR, + ) + for (key in keys) { + // An empty body (bare-triple reply from a ring that holds no records) decodes to + // nothing, never an error — the vendor's loops simply don't run (e.g. a0 @1555). + assertTrue("$key", RWfitJLHistory.decode(key, ByteArray(0)).orEmpty().isEmpty()) + } + } +} diff --git a/app/src/test/java/com/pulseloop/strava/StravaTCXBuilderTest.kt b/app/src/test/java/com/pulseloop/strava/StravaTCXBuilderTest.kt index 317200f..4d17cc7 100644 --- a/app/src/test/java/com/pulseloop/strava/StravaTCXBuilderTest.kt +++ b/app/src/test/java/com/pulseloop/strava/StravaTCXBuilderTest.kt @@ -119,6 +119,26 @@ class StravaTCXBuilderTest { assertEquals(start + 100_000, intervals[1].end) } + @Test + fun `pause intervals pair through the gps_stopped and gps_started markers Android writes`() { + // LiveWorkoutManager.pause/resume write `paused`+`gps_stopped` sharing one timestamp, + // then `resumed`+`gps_started` sharing another (mirrors iOS PulseServices). The pairing + // must react only to `paused`/`resumed` and treat the gps_* markers as transparent. + val pausedAt = start + 15_000L + val resumedAt = start + 45_000L + val events = listOf( + ActivityEventEntity(id = "1", sessionId = "s1", kind = "paused", timestamp = pausedAt), + ActivityEventEntity(id = "2", sessionId = "s1", kind = "gps_stopped", timestamp = pausedAt), + ActivityEventEntity(id = "3", sessionId = "s1", kind = "resumed", timestamp = resumedAt), + ActivityEventEntity(id = "4", sessionId = "s1", kind = "gps_started", timestamp = resumedAt), + ) + val intervals = StravaTCXBuilder.pauseIntervals(events, endedAt = start + 60_000L) + + assertEquals(1, intervals.size) + assertEquals(pausedAt, intervals[0].start) + assertEquals(resumedAt, intervals[0].end) + } + @Test fun `sport attribute uses the three TCX-legal values`() { fun sportOf(type: String) = StravaTCXBuilder diff --git a/app/src/test/java/com/pulseloop/ui/screens/BarcodeSymbologiesTest.kt b/app/src/test/java/com/pulseloop/ui/screens/BarcodeSymbologiesTest.kt new file mode 100644 index 0000000..1961edb --- /dev/null +++ b/app/src/test/java/com/pulseloop/ui/screens/BarcodeSymbologiesTest.kt @@ -0,0 +1,41 @@ +package com.pulseloop.ui.screens + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The scanner's accepted symbology set — iOS restricts the VisionKit data scanner to + * [.ean13, .ean8, .upce, .code128] (BarcodeScannerSheet.swift:56), the four Open Food Facts + * is keyed on. Anything else (QR, PDF417, Data Matrix, …) must stay out of the set so a + * poster QR code in frame can never be delivered as a "barcode". + */ +class BarcodeSymbologiesTest { + + @Test + fun acceptsExactlyTheFourOpenFoodFactsSymbologies() { + assertEquals( + setOf("EAN-13", "EAN-8", "UPC-E", "Code-128"), + BarcodeSymbologies.accepted, + ) + } + + @Test + fun acceptsEachOfTheFour() { + assertTrue(BarcodeSymbologies.isAccepted(BarcodeSymbologies.EAN13)) + assertTrue(BarcodeSymbologies.isAccepted(BarcodeSymbologies.EAN8)) + assertTrue(BarcodeSymbologies.isAccepted(BarcodeSymbologies.UPCE)) + assertTrue(BarcodeSymbologies.isAccepted(BarcodeSymbologies.CODE128)) + } + + @Test + fun rejectsSymbologiesIosDoesNotScan() { + assertFalse(BarcodeSymbologies.isAccepted("QR")) + assertFalse(BarcodeSymbologies.isAccepted("PDF417")) + assertFalse(BarcodeSymbologies.isAccepted("Data Matrix")) + // UPC-A rides in as an EAN-13 with a leading zero; it is not a separate accepted name. + assertFalse(BarcodeSymbologies.isAccepted("UPC-A")) + assertFalse(BarcodeSymbologies.isAccepted("")) + } +} diff --git a/app/src/test/java/com/pulseloop/ui/screens/MealAnalysisLogicTest.kt b/app/src/test/java/com/pulseloop/ui/screens/MealAnalysisLogicTest.kt new file mode 100644 index 0000000..2f78618 --- /dev/null +++ b/app/src/test/java/com/pulseloop/ui/screens/MealAnalysisLogicTest.kt @@ -0,0 +1,130 @@ +package com.pulseloop.ui.screens + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Unit tests for the pure decision logic ported from iOS's MealEstimator + * (MealAnalysisSheet.swift) — the enable predicates, the fence-tolerant decode, the + * confidence-to-provenance mapping, and the meal-type inference. + */ +class MealAnalysisLogicTest { + + // ── canAnalyze — iOS MealAnalysisSheet.swift:39-41 ───────────────── + + @Test + fun canAnalyzeWithAnImageAlone() { + assertTrue(MealAnalysisLogic.canAnalyze(hasImage = true, description = "")) + } + + @Test + fun canAnalyzeWithThreeTrimmedCharacters() { + // iOS trims whitespace BEFORE counting, so padding never qualifies a too-short + // description. + assertTrue(MealAnalysisLogic.canAnalyze(hasImage = false, description = "two eggs")) + assertTrue(MealAnalysisLogic.canAnalyze(hasImage = false, description = " abc ")) + } + + @Test + fun cannotAnalyzeShortOrBlankDescriptionsWithoutImage() { + assertFalse(MealAnalysisLogic.canAnalyze(hasImage = false, description = "")) + // "ab" padded to 4 raw characters still trims to 2. + assertFalse(MealAnalysisLogic.canAnalyze(hasImage = false, description = " ab ")) + } + + // ── canSave — iOS MealAnalysisSheet.swift:43-45 ──────────────────── + + @Test + fun canSavesWithNumericCalories() { + assertTrue(MealAnalysisLogic.canSave(name = "Omelette", calories = "520")) + assertTrue(MealAnalysisLogic.canSave(name = " Omelette ", calories = "520.5")) + } + + @Test + fun cannotSaveWithoutANameOrANumber() { + assertFalse(MealAnalysisLogic.canSave(name = "", calories = "520")) + assertFalse(MealAnalysisLogic.canSave(name = " ", calories = "520")) + // iOS Double(calories) == nil — any non-numeric string fails. + assertFalse(MealAnalysisLogic.canSave(name = "Omelette", calories = "")) + assertFalse(MealAnalysisLogic.canSave(name = "Omelette", calories = "about 500")) + } + + // ── confidence mapping — iOS save(), MealAnalysisSheet.swift:306 ─── + + @Test + fun confidenceMapsToProvenanceKnownPartialUnknown() { + assertEquals("known", MealAnalysisLogic.confidenceRaw("high")) + assertEquals("partial", MealAnalysisLogic.confidenceRaw("medium")) + assertEquals("unknown", MealAnalysisLogic.confidenceRaw("low")) + // Anything else (missing, garbage) lands on unknown, like iOS's else branch. + assertEquals("unknown", MealAnalysisLogic.confidenceRaw(null)) + assertEquals("unknown", MealAnalysisLogic.confidenceRaw("HIGH")) + } + + // ── inferred meal type — iOS NutritionModels.swift:20-27 ────────── + + @Test + fun inferredMealTypeFollowsTheClockBuckets() { + assertEquals("breakfast", MealAnalysisLogic.inferredMealType(4)) + assertEquals("breakfast", MealAnalysisLogic.inferredMealType(10)) + assertEquals("lunch", MealAnalysisLogic.inferredMealType(11)) + assertEquals("lunch", MealAnalysisLogic.inferredMealType(14)) + assertEquals("snack", MealAnalysisLogic.inferredMealType(15)) + assertEquals("dinner", MealAnalysisLogic.inferredMealType(17)) + assertEquals("dinner", MealAnalysisLogic.inferredMealType(21)) + assertEquals("snack", MealAnalysisLogic.inferredMealType(22)) + assertEquals("snack", MealAnalysisLogic.inferredMealType(3)) + } + + // ── fence-tolerant decode — iOS MealEstimator.decode :413-422 ───── + + private val fullJson = + """{"name":"Rice and beans","calories":450.0,"protein_g":15.0,"carbs_g":80.0,"fat_g":6.0,"assumptions":"1 cup cooked","confidence":"high"}""" + + @Test + fun decodesPlainJsonObject() { + val e = MealAnalysisLogic.decode(fullJson) + assertNotNull(e) + assertEquals("Rice and beans", e!!.name) + assertEquals(450.0, e.calories, 1e-9) + assertEquals(15.0, e.proteinG, 1e-9) + assertEquals(80.0, e.carbsG, 1e-9) + assertEquals(6.0, e.fatG, 1e-9) + assertEquals("high", e.confidence) + } + + @Test + fun decodesInsideMarkdownFences() { + val fenced = "```json\n" + fullJson + "\n```" + assertEquals("Rice and beans", MealAnalysisLogic.decode(fenced)!!.name) + } + + @Test + fun decodesInsideSurroundingProse() { + val prose = "Here is your estimate:\n" + fullJson + "\nHope this helps!" + assertEquals("Rice and beans", MealAnalysisLogic.decode(prose)!!.name) + } + + @Test + fun toleratesUnknownKeysAndMissingAssumptions() { + val text = "{\"name\":\"Soup\",\"calories\":120,\"protein_g\":4,\"carbs_g\":10,\"fat_g\":2,\"confidence\":\"medium\",\"extra\":true}" + val e = MealAnalysisLogic.decode(text)!! + assertEquals("Soup", e.name) + // iOS decodes assumptions as a present String; the Android port defaults it so a + // provider omitting the key still yields a usable estimate. + assertEquals("", e.assumptions) + } + + @Test + fun returnsNullForGarbageAndMissingRequiredFields() { + assertNull(MealAnalysisLogic.decode("")) + assertNull(MealAnalysisLogic.decode("no json here at all")) + // A JSON object missing required fields is unusable — same as iOS's decode failure. + assertNull(MealAnalysisLogic.decode("{\"name\":\"Soup\"}")) + assertNull(MealAnalysisLogic.decode("{}")) + } +} diff --git a/docs/crp-r11-hardening-plan.md b/docs/crp-r11-hardening-plan.md new file mode 100644 index 0000000..013a360 --- /dev/null +++ b/docs/crp-r11-hardening-plan.md @@ -0,0 +1,504 @@ +# CRP (Colmi R11) driver hardening — implementation plan + +**Ledger item:** iOS PR [#93](https://github.com/saksham2001/PulseLoopiOS/pull/93), triaged +2026-08-22 in [`ios-sync.md`](ios-sync.md) (§ "2026-08-22 triage"). +**Branch:** `ios-sync-triage-2026-08-22`. +**Scope:** five small, independent fixes to the CRP driver that already exists on Android. +**Effort:** S–M total. No schema change, no UI, no new files. + +--- + +## 0. Read this first — what this task is and isn't + +iOS PR #93 is **not** a normal upstream feature to port. It is the *iOS port of this repo's own +Android CRP work*, so the driver, the decoder, the sync engine, the "Colmi R11 (Da Rings app)" +pairing card and the not-worn measurement hint are all **already present here**. Do not port them +again. If you find yourself writing a `CRPDecoder`, you are in the wrong task. + +What Android is missing is the hardening iOS added **afterwards**, in commit `4d65b60` +("fix(crp): reset the frame assembler across reconnects, gate connect on fdd3"), which was an +adversarial review of that branch. Five of its eight findings apply to Android. Three do not, and +§7 explains why so nobody re-ports them. + +**The iOS commit is your reference implementation.** Read it before you start: + +```sh +git -C show 4d65b60 +``` + +The iOS repo is the parent directory of this one (`../` from `android/`), on branch `main`. +Every item below cites the exact Swift hunk it corresponds to. **Judge behaviour, not syntax** — +a Swift fix ports as a Kotlin rule, and item 4 in particular needs materially different Kotlin, +because Kotlin's UTF-8 decode is not Swift's. + +### Ground rules + +- Read `AGENTS.md` at this repo root and at the parent repo root first. +- **Do not add a `Co-Authored-By` trailer** to commits in this repo. +- These five items are **independent**. Land them in one commit or five; item 1 is the most + valuable and stands alone if the rest slip. +- Every item has a test. The suite is `./gradlew testDebugUnitTest` and was **1099 tests, 0 + failures** at the tip of `ios-sync-triage-2026-08-22`. Do not finish below that count. +- **No hardware is available in this environment.** Nothing here needs a ring: all five are unit + testable. Say so plainly in the commit rather than implying hardware verification. + +### Files you will touch + +| File | Items | +|---|---| +| `app/src/main/java/com/pulseloop/ring/CRPDriver.kt` | 1 | +| `app/src/main/java/com/pulseloop/ring/CRPSyncEngine.kt` | 2, 3 | +| `app/src/main/java/com/pulseloop/ring/CRPDecoder.kt` | 4, 5 | +| `app/src/test/java/com/pulseloop/ring/CRPSyncEngineTest.kt` | 2, 3 | +| `app/src/test/java/com/pulseloop/ring/CRPDecoderTest.kt` | 1, 4, 5 | + +--- + +## 1. Gate `CONNECTED` on the `fdd3` reply channel + +**Severity: highest of the five. Do this one even if you do nothing else.** + +### The bug + +`CRPDriver` does not override `requiredSubscriptionsBeforeConnected`, so it inherits the default +`emptyList()` from `WearableDriver` (`WearableDriver.kt:32`). With an empty list, +`SubscriptionSetupGate.isReady` falls back to *first-notify* readiness — `completed.isNotEmpty()` +(`SubscriptionSetupGate.kt:31-35`). The connection therefore counts as up on whichever notify +characteristic finishes its CCCD write first. + +For CRP that is `fdd1` (the current-steps push), **never** `fdd3`, which carries *every* command +reply. `CONNECTED` is what runs `CRPSyncEngine.runStartup`, so the handshake can write its whole +sequence — set-time, firmware query, six read-backs, five timing enables, six history queries and +the six-day sleep backfill, ~26 frames — into a channel the app is not yet listening to. + +A reply lost that way is **indistinguishable from a slow one**. That is the exact signature of the +wrong-opcode bug this driver already fixed once (the group-7 firmware query that produced +23 sends / 0 replies in the 2026-07-25 capture), so a regression here would be diagnosed as a +protocol problem, not a connect-ordering problem. + +### The fix + +In `CRPDriver.kt`, alongside the other topology declarations: + +```kotlin + /** + * Hold CONNECTED until `fdd3` is live. Without this the connection counts as up on whichever + * notify characteristic completes its CCCD write first — for CRP that is `fdd1` (the steps + * push), never `fdd3`, which carries *every* command reply. CONNECTED is what runs + * [CRPSyncEngine.runStartup], so a handshake begun too early would write the clock, firmware + * query, read-backs, timing config and the whole history pull into a channel we aren't + * listening to yet, and each lost reply is indistinguishable from a slow one. + * + * Only `fdd3` is required: `fdd1`/`fdd6`/`2a37` carry no reply the handshake waits on, so + * gating on them would only delay the connect. NOTIFICATION, not INDICATION — CRP's + * characteristics are notify (unlike YCBT's indicate pair). + */ + override val requiredSubscriptionsBeforeConnected = listOf( + RequiredSubscription(CRPUUIDs.CHAR_CMD_NOTIFY, SubscriptionMode.NOTIFICATION), + ) +``` + +`RingBLEClient` already threads this through — `installDriver` builds the gate from it +(`RingBLEClient.kt:900-904`). There is no wiring to add. + +**Reference:** iOS `CRPDriver.swift`, the `requiredSubscriptionsBeforeConnected` property added in +`4d65b60`. Android's element type is `RequiredSubscription` (uuid + mode), not iOS's bare `CBUUID`. + +**Model this on:** `YCBTDriver.kt:43-46`, the only existing Android driver that overrides this. + +### The test + +Add to `CRPDecoderTest.kt` (where the other driver-topology tests live): + +```kotlin + @Test + fun `connect is held until the command-reply channel is live`() { + val driver = CRPDriver(null) + assertEquals( + listOf(RequiredSubscription(CRPUUIDs.CHAR_CMD_NOTIFY, SubscriptionMode.NOTIFICATION)), + driver.requiredSubscriptionsBeforeConnected, + ) + // A required subscription that isn't a declared notify char could never be satisfied, and + // the connect would hang until the watchdog killed it. + for (required in driver.requiredSubscriptionsBeforeConnected) { + assertTrue( + "${required.uuid} is not a declared notify characteristic", + driver.notifyUUIDs.any { it.equals(required.uuid, ignoreCase = true) }, + ) + } + } +``` + +That second assertion is not padding — it is the failure mode that would turn this fix into a +connect hang. + +--- + +## 2. Stop re-querying firmware on every poll pass + +### The bug + +`CRPSyncEngine.runStartup` sends `CRPProtocol.queryFirmwareVersion()` unconditionally +(`CRPSyncEngine.kt:42`), while the six read-backs immediately below it are gated behind +`readBacksSent`. The KDoc on `sendConnectionReadBacks` argues the gate exists because the single +`fdd2` channel is scarce and a spot SpO2 needs ~48 s of it — an argument the line above it +contradicts. + +`runStartup` **is** the poll pass: `RingSyncWorker`'s ~30-minute background sync and the foreground +`syncNow()` both re-invoke it. So this is one extra write on the scarce channel every half hour, +forever, for a string that cannot change between syncs. + +### The fix + +In `CRPSyncEngine.kt`: + +1. Delete the `send(CRPProtocol.queryFirmwareVersion())` call and its comment block from + `runStartup` (currently lines 37-42). +2. Move that send to the **top** of `sendConnectionReadBacks()`, before `querySupportSpO2Type()`. +3. Rename `sendConnectionReadBacks` → `sendConnectionQueries` and `readBacksSent` → + `connectionQueriesSent`. iOS did this because "read-backs" no longer describes the set once + firmware joins it. Rename the `CRPSyncEngineTest` helper `readBackQueries` to match. +4. Fold the firmware rationale (the 7/1-vs-3/3 opcode history — keep it, it is hard-won) into the + `sendConnectionQueries` KDoc. +5. Update that KDoc's "six writes" to "seven writes". + +**Ordering constraint — do not disturb it.** `sendConnectionQueries()` must still run **before** +`applyTimingSettings(...)`. The state queries report each monitor's *current* interval, and +`applyTimingSettings` force-enables everything moments later; asking afterwards would only describe +the state we just imposed, which answers nothing. `CRPSyncEngineTest` pins this. If that assertion +fails, fix the call site, not the expectation. + +**Reference:** iOS `CRPSyncEngine.swift` — the `sendConnectionReadBacks` → `sendConnectionQueries` +rename hunk in `4d65b60`. + +### Also update the now-false comments + +Two comments elsewhere assert the old behaviour and become wrong: + +- `CRPDecoder.kt:195-197` — "…and [CRPSyncEngine.runStartup] re-queries firmware on every sync + pass". After this change it does not. +- `CRPDecoderTest.kt:119-122`, the test named *`firmware version reaches the device record as its + own event, not a connection change`* — its comment says "runStartup re-queries firmware on every + ~30-minute sync pass, so that path would restate CONNECTED all session long." + +The **test itself stays and must keep passing** — firmware must still not bridge to +`DeviceStateChanged`. Only the justification changes: a firmware reply says nothing about the +connection, and bridging it to CONNECTED would restamp the device row as freshly connected. Rewrite +the comment; do not delete the test. + +### The tests + +Two existing tests in `CRPSyncEngineTest.kt` assert the current behaviour and **must** be updated — +they will fail, and that failure is correct: + +- **line 37**, `runStartup sends set-time, firmware query, user info, default monitor enables, then + the history pull`. Both `assertEquals` calls embed `3 to 3` in the expected opcode list. First + pass: `3 to 3` moves from position 2 into the read-back group. Second pass: `3 to 3` must + **disappear** — expected becomes `listOf(1 to 1, 1 to 0) + timingEnables + historyQueries`. + Rename the test to match its new meaning. +- **line 88**, `read-backs are sent once per connection, not once per poll pass`. Add firmware to + the set it guards, and rename to `connection queries are sent once per connection…`. + +Then add the explicit regression: + +```kotlin + @Test + fun `firmware is asked once per connection, not on every poll pass`() { + // runStartup IS the ~30-minute background sync. A firmware string is exactly as immutable + // as the sensor roster gated beside it, and fdd2 is the scarce channel (a spot SpO2 needs + // ~48 s of it). + val w = FakeWriter() + val engine = CRPSyncEngine(w) + engine.runStartup() + assertTrue("firmware asked on the first pass", (3 to 3) in w.opcodes()) + + w.sent.clear() + engine.runStartup() + assertTrue("firmware must not repeat every pass", (3 to 3) !in w.opcodes()) + + // A new connection builds a new engine, which asks again. + val reconnected = FakeWriter() + CRPSyncEngine(reconnected).runStartup() + assertTrue((3 to 3) in reconnected.opcodes()) + } +``` + +**Reference:** iOS `CRPSyncEngineTests.swift`, +`testConnectionQueriesAreSentOncePerConnectionNotPerPass`. + +--- + +## 3. Key the timing follow-up guard on `day` as well as `cmd` + +### The bug + +`CRPSyncEngine.kt:104` declares `requestedTimingFrames` as `mutableSetOf()`, and line 179 keys +it `event.cmd * 100 + nextIndex`. The `day` is not in the key. + +Today every timing query is `day = 0`, so nothing is broken *right now*. But this engine **already +issues multi-day requests** — `sendSleepBackfill()` walks `1..SLEEP_BACKFILL_DAYS` (6 days). The +moment the timing vitals get the same backfill treatment, day 1's frame-1 follow-up is silently +swallowed because day 0 already inserted the same key. Silently: no error, just a day that never +completes its multi-frame pull. + +This is pre-emptive, and worth doing because the failure is invisible when it lands. + +### The fix + +In `CRPSyncEngine.kt`, replace the `Int` key with a data class: + +```kotlin + /** One timing-history follow-up we've already asked for. Keyed on `day` as well as `cmd` — + * today's queries are all day 0, but this engine already issues multi-day requests for sleep + * ([sendSleepBackfill]), and a key without `day` would silently swallow day 1's frame-1 + * follow-up the moment the timing vitals get the same backfill treatment. */ + private data class TimingFrameRequest(val cmd: Int, val day: Int, val frameIndex: Int) + + /** Frame follow-ups already requested this poll pass, so a ring that re-sends the same frame + * can't trigger a request storm. Cleared at the start of every [queryAllHistory] pass so each + * sync re-pulls the full timeline. */ + private val requestedTimingFrames = mutableSetOf() +``` + +and at the guard (line 179): + +```kotlin + val request = TimingFrameRequest(event.cmd, event.day, nextIndex) + if (!requestedTimingFrames.add(request)) return +``` + +`requestedTimingFrames.clear()` in `queryAllHistory()` is unchanged. + +**Reference:** iOS `CRPSyncEngine.swift`, the `TimingFrameRequest` struct in `4d65b60`. + +### The test + +`a repeated frame does not spam duplicate follow-up requests` (line 181) must still pass unchanged — +that is the property this must not break. Add: + +```kotlin + @Test + fun `the follow-up guard distinguishes days`() { + val w = FakeWriter() + val engine = CRPSyncEngine(w) + engine.runStartup() + w.sent.clear() + engine.handle(RingDecodedEvent.TimingHistoryFrame(cmd = 15, day = 0, frameIndex = 0)) + engine.handle(RingDecodedEvent.TimingHistoryFrame(cmd = 15, day = 1, frameIndex = 0)) + assertEquals("a different day is a different follow-up", 2, w.sent.size) + assertEquals(0, w.sent[0][6].toInt()) // day 0 in the payload + assertEquals(1, w.sent[1][6].toInt()) // day 1 + } +``` + +Check `RingDecodedEvent.TimingHistoryFrame`'s actual constructor signature and the payload byte +offset against `CRPProtocol.queryTimingHeartRateHistory(day, frameIndex)` before trusting the +indices above — index 6 is what iOS asserts and Android's frame layout matches, but verify rather +than assume. + +**Reference:** iOS `CRPSyncEngineTests.swift`, `testFollowUpGuardDistinguishesDays`. + +--- + +## 4. Validate the firmware string instead of coercing it + +**This is the item whose Kotlin differs most from the Swift. Read carefully.** + +### The bug + +`CRPDecoder.decodeFirmwareVersion` (`CRPDecoder.kt:199-204`) does: + +```kotlin +val version = String(payload, Charsets.UTF_8).trim { it <= ' ' } +``` + +`String(bytes, UTF_8)` in Kotlin/JVM is **lenient**: invalid byte sequences are silently replaced +with U+FFFD. It cannot fail. So a binary payload becomes a row of replacement characters and is +published as `RingDecodedEvent.FirmwareRevision` — and whatever this returns is **shown verbatim in +the Settings device card**. The user sees replacement characters presented as their ring's firmware +version. + +### The fix + +Strict-decode, then trim padding, then reject anything still holding a control byte: + +```kotlin + private fun decodeFirmwareVersion(payload: ByteArray): List? { + // Validated, not coerced: whatever this returns is shown verbatim in Settings. + // `String(bytes, UTF_8)` is LENIENT on the JVM — it substitutes U+FFFD for invalid bytes + // and cannot fail — so a binary payload would render as a row of replacement characters + // presented as a firmware version. A REPORTing decoder throws instead. + val decoder = Charsets.UTF_8.newDecoder() + .onMalformedInput(CodingErrorAction.REPORT) + .onUnmappableCharacter(CodingErrorAction.REPORT) + val raw = try { + decoder.decode(ByteBuffer.wrap(payload)).toString() + } catch (_: CharacterCodingException) { + return null // not text at all — the caller acks it + } + // Trim only what firmwares actually pad with: whitespace and NUL. Deliberately narrower + // than the vendor's `trim { it <= ' ' }`, which strips ALL control bytes and would let a + // binary payload's leading junk come off so whatever printable byte followed passed as a + // "version" (01 02 03 41 -> "A"). Padding comes off, then anything still holding a control + // byte is rejected outright rather than salvaged. + val trimmed = raw.trim { it.isWhitespace() || it == NUL } + if (trimmed.isEmpty()) return null + if (trimmed.any { it.isISOControl() }) return null + return listOf(RingDecodedEvent.FirmwareRevision(trimmed)) + } +``` + +where `NUL` is the NUL character — declare it as a private constant, `private const val NUL = '\u0000'`, so the predicate stays readable. + +Imports needed: `java.nio.ByteBuffer`, `java.nio.charset.CharacterCodingException`, +`java.nio.charset.CodingErrorAction`. + +Returning `null` is already the "nothing readable" contract — the caller at `CRPDecoder.kt:135` +falls through to `CommandAck`. Do not change that path. + +**Item 5 is this same edit** — the narrower trim is the second half of the same function, kept as a +separate ledger row only because iOS listed it separately. There is nothing extra to do for it. + +**Reference:** iOS `CRPDecoder.swift`, `decodeFirmwareVersion` in `4d65b60`. Swift's +`String(bytes:encoding:)` returns `nil` on invalid UTF-8, which is why the Swift version needs no +explicit decoder — **Kotlin has no equivalent one-liner**, hence the `CharsetDecoder`. + +### The tests + +Three existing tests in `CRPDecoderTest.kt` must keep passing unchanged — they are the regression +net against over-tightening: + +- `firmware version decodes as the UTF-8 string the vendor reads` (line 109) — `MOY-R1K3-2.1.6`. +- `firmware version tolerates NUL padding` (line 130) — trailing NULs still trimmed. +- `empty firmware payload is acked, not reported as a blank version` (line 138) — a lone `0x00` + still acks. + +Add: + +```kotlin + @Test + fun `a non-text firmware payload is acked rather than coerced into a version`() { + // Whatever decodeFirmwareVersion returns is shown verbatim in Settings, so a payload that + // isn't a version string must ack. `String(bytes, UTF_8)` would have coerced the first into + // a U+FFFD run and the second into control junk, and published both as a firmware version. + val payloads = listOf( + byteArrayOf(0xC3.toByte(), 0x28, 0xA0.toByte(), 0xFF.toByte()), // invalid UTF-8 + byteArrayOf(0x01, 0x02, 0x03, 0x41), // valid UTF-8, binary + ) + for (payload in payloads) { + val frame = CRPProtocol.frame(3, CRPCommands.CMD_QUERY_FIRMWARE_VERSION, payload) + val events = CRPDecoder.decode(frame, fdd3) + assertTrue( + "payload must not publish a version", + events.none { it is RingDecodedEvent.FirmwareRevision }, + ) + assertTrue(events.single() is RingDecodedEvent.CommandAck) + } + } +``` + +The second payload is the one that proves the *narrow* trim (item 5): under the vendor's +`trim { it <= ' ' }` it would have yielded the version string `"A"`. + +**Reference:** iOS `CRPDecoderTests.swift`, +`testNonTextFirmwarePayloadsAreRejectedRatherThanCoerced` — same two payloads. + +--- + +## 5. (Same edit as item 4) + +Kept as its own row because the ledger and the iOS commit list it separately. The narrower trim is +implemented by the `trim { it.isWhitespace() || it == NUL }` + `isISOControl()` rejection in §4. +Nothing further to do. + +--- + +## 6. Verification + +```sh +./gradlew compileDebugKotlin # KSP/Room validate on the way through +./gradlew testDebugUnitTest # expect >= 1099 + your new tests, 0 failures +``` + +Count the suite the way the ledger does: + +```sh +python3 - <<'PY' +import glob, re +t = f = e = 0 +for p in glob.glob('app/build/test-results/testDebugUnitTest/*.xml'): + m = re.search(r'tests="(\d+)".*?failures="(\d+)".*?errors="(\d+)"', open(p).read(4000)) + if m: t += int(m[1]); f += int(m[2]); e += int(m[3]) +print(f"tests={t} failures={f} errors={e}") +PY +``` + +**What cannot be verified here:** every one of these is about BLE timing or wire-format edge cases +that need zaggash's R11 to observe for real. The unit tests pin the *rules*; they do not prove the +ring behaves as assumed. Item 1 in particular changes when `CONNECTED` fires, which is exactly the +kind of change that looks fine in tests and reveals itself on hardware. State this honestly in the +commit message — do not write "verified" for anything that wasn't. + +If hardware does become available, the honest test for item 1 is: pair the R11, confirm the +handshake completes rather than partially answering, and confirm the connect doesn't hang (a +required subscription that never completes would stall until the 30 s watchdog). + +--- + +## 7. Do NOT port these three + +They are in the iOS commit and they do not apply here. Recorded so nobody re-ports them. + +### 7a. Frame-assembler reset across reconnects + +This was iOS's headline bug: there, auto-reconnect re-dials with a bare `central.connect` and keeps +the `CRPDriver` instance, so a frame left half-assembled when the old link dropped is completed with +bytes from the new one and decoded as genuine. Because the group-2 history frames are long and +multi-notification, the spliced result is a *fabricated vital sample*, not a parse failure. + +**Android is safe by construction:** + +- It connects with `autoConnect = false` (`RingBLEClient.kt:804`), matching the official QRing app. +- Every reconnect path funnels through `beginConnect`, which calls `installDriver` + (`RingBLEClient.kt:787`). +- `installDriver` builds a fresh driver via `coordinator.makeDriver` and immediately calls + `driver.connectionDidStart()` (`RingBLEClient.kt:897-900`). + +So each link gets a brand-new `CRPDriver` with a brand-new `CRPFrameAssembler`. Adding a `reset()` +hook would be dead code. + +`CRPDriver`'s KDoc already states this invariant correctly. **Keep it accurate.** The single change +that would reintroduce the iOS bug is a reconnect path that reuses a driver instead of reinstalling +it — if you ever make that change, this item comes back, and `connectionDidStart`/`connectionDidEnd` +already exist on `WearableDriver` (`WearableDriver.kt:50-51`) to hang the reset on. `RWfitDriver` +and `YCBTDriver` show the pattern. + +### 7b. Half-open sleep-backfill loop + +iOS used `for daysAgo in 1...crpSleepBackfillDays`, which **traps at runtime** if the documented +tuning knob is turned down to today-only (`1...0` is an invalid `ClosedRange`). iOS changed it to +`1..<(n + 1)`. + +Kotlin's `1..SLEEP_BACKFILL_DAYS` is an `IntRange`, and `1..0` is simply **empty** — no exception, +the loop body doesn't run. `CRPSyncEngine.kt:151` is already safe at `SLEEP_BACKFILL_DAYS = 0`. +Changing it to `until` would be churn. + +### 7c. "Once per connection" comment corrections + +iOS's comments claimed connection scope for state that is really per-driver-install, and were +rewritten. Android's already say the right thing: `readBacksSent` and `sleepBackfillSent` are both +documented as per-engine-instance, with a fresh engine built per connect +(`CRPSyncEngine.kt:63-65`, `124-126`). + +Note the nuance if you touch them: on Android the "fresh engine per connect" claim is *true* +(§7a), which is why the comments are accurate here and were not on iOS. + +--- + +## 8. When you're done + +1. Update the port-queue row for #93 in [`ios-sync.md`](ios-sync.md) — flip `☐` to `☑` and put your + commit SHA in the **Android commit** column. +2. Remove #93 from the "▶ RESUME HERE" list and drop the count back to two threads. +3. If you land only some items, say which in the row rather than flipping it — a half-done row that + reads as done is worse than an open one. diff --git a/docs/ios-sync.md b/docs/ios-sync.md index c1479ef..90e2eac 100644 --- a/docs/ios-sync.md +++ b/docs/ios-sync.md @@ -11,9 +11,14 @@ intentional platform differences listed at the bottom. (PR merges — not individual commits — are the unit of triage.) 2. For each PR: read the diff (`git diff ^1 `), decide a verdict, and add a row. Judge **behavior**, not code — a Swift fix ports as a Kotlin rule. -3. When a PORT/ADAPT item ships, fill in its **Android commit** column. +3. When a PORT/ADAPT item ships, fill in its **Android commit** column **and** remove it from + [Outstanding — the single list](#outstanding--the-single-list). 4. Update **Last triaged iOS commit** below. +**Just want to know what to work on?** Read [Outstanding — the single list](#outstanding--the-single-list) +and stop there. The port queue is the per-PR audit trail; the session notes are history. Neither is +the work list, and assembling one from all three is how items get missed. + **Verdicts:** `PORT` (Android needs it) · `ADAPT` (concept ports, implementation differs) · `PARTIAL` (some of it applies) · `ALREADY-HAVE` (Android already does this) · `SKIP` (iOS-only / docs / CI) · `BLOCKED` (depends on something Android lacks) @@ -24,10 +29,57 @@ intentional platform differences listed at the bottom. |---|---| | **Canonical iOS repo** | `github.com/saksham2001/PulseLoopiOS` (always `main`) | | **Fork baseline (iOS)** | `600c7a8` — Merge PR #6, 2026-06-20 | -| **Last triaged iOS commit** | `88c0f6b` — Merge PR #131 (sleep hypnogram alignment + scrubber), 2026-08-08 | -| **Last triage date** | 2026-08-08 | -| **Last port date** | 2026-08-08 — PR #45 (ios_sync_2026-08-08, 5 plan commits + 2 CR remediation commits = 7 total) | -| **Range covered** | 11 first-parent items since `0d1b965` (2026-07-18): PRs #73, #94–#100, #130, #131 + 1 direct commit (`160c775`) → **10 ported, #130 backed out** | +| **Last triaged iOS commit** | `439ca81` — Merge PR #93 (Colmi R11 CRP driver), 2026-08-09 | +| **Last triage date** | 2026-08-22 | +| **Last port date** | 2026-08-23 — PR #96 nutrition **complete**: barcode scanner + AI meal analysis (`e80c76c`) on top of the OFF client (`a13238d`) and five coach tools (`05d8833`); plus the self-hosted-provider schema fix they exposed (`d3d1371`). 2026-08-22 — #130 RWfit JieLi history (`c9be848`), Workout pause intervals (`71f251e`), PR #94 `CoachNotificationDataTrigger` (`9d43227`), PR #93 hardening (`c95b6e8`) | +| **Range covered** | 12 first-parent items since `0d1b965` (2026-07-18): PRs #73, #94–#100, #130, #131, #93 + 1 direct commit (`160c775`) → **all 12 ported** (#130 was backed out as fabricated, then rebuilt from the vendor decompile — see its row below). Verified against a live `git fetch` on 2026-08-23: `origin/main` is `439ca81` and local `main` is 0 commits behind, so upstream is fully triaged and ported | + +--- + +## Outstanding — the single list + +Everything upstream that is **not yet on Android `main`**, in one place. This replaces reading the +port queue, the resume block and the session notes to assemble the picture yourself. The port queue +below is the per-PR audit trail; **this table is the work list.** + +Ordered by readiness, not size. **As of 2026-08-23 no row can be started** — the port queue is +empty. Two rows need ring hardware to verify code that is already written; the third needs an +Android screen that does not exist yet. Nothing here is waiting on someone to finish a port. + +| # | Item | What is actually left | Size | Ready? | +|---|------|----------------------|------|--------| +| 1 | **#82 YCBT (TK5 + SmartHealth-Colmi)** | Protocol layer is on `main` (`7a941a5`, `849131d`). **No code known to be missing** — what is missing is a live connect against real hardware. If one fails, re-read `BleHelper.java`'s connect sequence: the vendor's MTU/bonding/pacing timing was deliberately *not* copied (see the 2026-07-19 note). | — | ⛔ needs hardware | +| 2 | **#90 LuckRing / TK18** | Protocol layer is on `main` (`57e1e23`). Same position as #82: no known code gap, never validated against a real TK18. | — | ⛔ needs hardware | +| 3 | **#79 Activity Year trends** | Divide the in-progress current month by elapsed days, not a full 30/31. The `S` is the *iOS* fix; Android has no Activity-trends screen at all, so the real scope is building the screen first. This is the one remaining **feature** gap — it is not blocked on hardware. | S (iOS) / L (Android) | ⛔ blocked — Android has no Activity-trends screen to fix | + +### Not on this list, and why + +- **#80 Health Connect** — done. Phases 0–6 (all of them) are complete and **merged to `main`** at + `11abb92`. The `feat/health-connect-foundation` branch the port-queue row names still exists on + `origin`, but it is fully contained in `main` — read `main`, not the branch. +- **PR #45 review remediation** — done. The 2026-08-09 review's parity bugs in #95/#98/#99/#100 and + the #94 regression were all fixed in `8df67b1` + `8f81c40`. Only rows 2–4 above survive from it. +- **#130 RWfit rebuild** — done, including the JieLi `0xAB` history bodies (`c9be848`, + 2026-08-22). The rebuild is on `main` (the ledger's `feat/rwfit-vendor-rebuild` is stale). + **Never hardware-validated** — see the rebuild section's own caveat before shipping it. +- **#96 nutrition subset** — done, all four parts. The OFF client + cache (`a13238d`) and five + coach tools (`05d8833`) landed 2026-08-22; the barcode scanner + AI meal analysis landed + 2026-08-23 (`e80c76c`). Device-verified end to end — see the session note below. +- Everything else in the port queue is `☑` or `⊘`. + +### Branch note + +Two different kinds of stale branch reference appear below, and both resolve the same way — the +work is on `main`: + +- **Merged and deleted** — `feat/rwfit-vendor-rebuild` (`8d16513`), `iOS_sync_2026-07-16` + (`0b971ac`), `ios_sync_2026-08-08` (`4434841`). All three are ancestors of `main`; neither the + local nor the `origin` ref still exists. +- **Merged but still present** — `feat/health-connect-foundation` (`11abb92`), + `feat/rwfit-ring-family` (`b073dad`). The refs exist on `origin` and are fully contained in + `main`, so checking one out gains nothing. + +**Check `main` before believing a branch reference in a session note below** (verified 2026-08-22). --- @@ -103,10 +155,10 @@ seeded-data mode). **SKIP** — no portable behavior, Android has its own indepe | # | iOS PR | Merged | Title | Verdict | Effort | Android commit | |---|--------|--------|-------|---------|--------|----------------| | ☑ | [#73](https://github.com/saksham2001/PulseLoopiOS/pull/73) `7a30014` | ~07-20 | Privacy & Data Reset (Unpair Ring / Reset App Data / Unpair+Reset) | **PORT** | S–M | `802789d` | -| ☑ | [#94](https://github.com/saksham2001/PulseLoopiOS/pull/94) `459f7f1` | ~07-21 | Background syncs + `StaleDataPolicy` + data-gated coach notifications | **ADAPT** | M | `0ca53a1` + `c4aab74` (CR fix: wire STALE_DATA_WINDOW_MS) | +| ☑ | [#94](https://github.com/saksham2001/PulseLoopiOS/pull/94) `459f7f1` | ~07-21 | Background syncs + `StaleDataPolicy` + data-gated coach notifications | **ADAPT** | M | `0ca53a1` + `c4aab74` (CR fix: wire STALE_DATA_WINDOW_MS) + **`9d43227`** (the data-trigger feature itself — the bus subscriber + (dateKey,slotRaw) dedupe + stale-skip — was the one part of #94 never ported) | | ☑ | [#95](https://github.com/saksham2001/PulseLoopiOS/pull/95) `dae95ab` | ~07-22 | HR zone colors/thresholds (evidence-based defaults + Standard/Auto/Custom modes + resting-HR baseline learning) | **PORT** | M–L | `0ca53a1` | | ☑ | [#97](https://github.com/saksham2001/PulseLoopiOS/pull/97) `cb8e1cd` | ~07-23 | LittleMeatball R10M YCBT support + 9 shared YCBT bugfixes | **ALREADY-HAVE** | — | iOS PR is itself a port of PulseLoopAndroid#31 | -| ☑ | [#96](https://github.com/saksham2001/PulseLoopiOS/pull/96) `c0def0f` | ~07-24 | Calorie + macro nutrition tracking (meal logging, barcode scan, OFF search, AI photo analysis, coach `log_meal` tool, intake goals, provenance tags) | **ADAPT (subset — manual meal logging + goals only; no OFF search, barcode, AI photo or coach `log_meal`)** | XL | `4084671` + `c4aab74` (CR fix: null-goal guard, dead button wired) | +| ☑ | [#96](https://github.com/saksham2001/PulseLoopiOS/pull/96) `c0def0f` | ~07-24 | Calorie + macro nutrition tracking (meal logging, barcode scan, OFF search, AI photo analysis, coach `log_meal` tool, intake goals, provenance tags) | **ADAPT — complete 2026-08-23.** Manual meal logging + goals (`4084671`+`c4aab74`), OFF client + 500-row cache (`a13238d`), five coach tools (`05d8833`), and finally both camera features (`e80c76c`): ML Kit + CameraX barcode scanner restricted to OFF's four symbologies, and the four-phase AI meal-analysis sheet running one structured single-shot call through the existing coach provider stack. Two divergences, both deliberate: the photo is **not persisted** (`MealEntryEntity` has no photo-ref column and this port ships no migration), and the entry buttons gate on the coach master toggle alone (Android has neither iOS's photo-analysis sub-toggle nor an on-device provider mode). Landing it also exposed and fixed a self-hosted-provider bug (`d3d1371`) — see the session note. | XL | `4084671` + `c4aab74` + `a13238d` + `05d8833` + `e80c76c` | | ☑ | [#99](https://github.com/saksham2001/PulseLoopiOS/pull/99) `f06be51` | ~07-25 | Full-data JSON export/import (all models → single JSON file, atomic wipe-and-restore on import) | **PORT** | M | `802789d` + `c4aab74` (CR fix: atomic transaction, wearableLogs roundtrip, BuildConfig appVersion) | | ☑ | [#100](https://github.com/saksham2001/PulseLoopiOS/pull/100) `4947628` | ~07-26 | Strava OAuth connect + TCX upload (GPS-HR merge, auto-dedup, token refresh) + shareable PNG stat cards | **ADAPT** | L | `4ce34dc` + `c4aab74` (CR fix: mobile endpoint, intent-filter, redirect handler, pollUntilDone, BuildConfig secrets, shared OkHttpClient) | | ☑ | — `160c775` | ~07-26 | Set version to 2.5.0 + read About version from bundle | **ALREADY-HAVE** | — | `68c9788` (versionName → 2.5.0 to match iOS MARKETING_VERSION) | @@ -114,6 +166,7 @@ seeded-data mode). **SKIP** — no portable behavior, Android has its own indepe | ☑ | [#130](https://github.com/saksham2001/PulseLoopiOS/pull/130) `cf5c0f4` | ~08-04 | RWfit ring family (dual 0x7E/0xAB protocol, full metric set, service-UUID recognition) | **ADAPT** | L–XL | Backed out of PR #45, then **rebuilt from `decompiled-rwfit-official/`** on `feat/rwfit-vendor-rebuild`. Legacy `0x7E` path complete; JieLi `0xAB` framing complete but its history bodies are not decoded yet. **No hardware validation.** See below. | | ☑ | [#131](https://github.com/saksham2001/PulseLoopiOS/pull/131) `88c0f6b` | ~08-08 | Sleep hypnogram label alignment + press-and-hold stage scrubber (+ sync spinner rewrite, iOS-only) | **ADAPT** | S–M | `802789d` | | ☑ | [#80](https://github.com/saksham2001/PulseLoopiOS/pull/80) `c1275ad` | 07-11 | **Apple Health sync → Health Connect** (per-type toggles, vitals/sleep/activity/workout export, backfill choice, remove-all). Re-triaged 2026-08-09 from SKIP: the *behaviour* ports even though HealthKit doesn't. Write-only; profile import can't port (Health Connect has no DOB/sex type). Design + 7-phase plan in [`health-connect-integration.md`](health-connect-integration.md); reference implementation is `Gadgetbridge/` at the parent repo root, not iOS. Not blocked by the Play Store — the declaration form is a publishing gate, and Gadgetbridge ships this sideload-only. | **ADAPT** | XL | **Phases 0–6 complete** on `feat/health-connect-foundation` (write-only, 16 `WRITE_*` / 0 `READ_*`; lifecycle, removal, grant/revocation resets, archive-restore stamp, docs). Runtime-verified API 35. See `health-connect-integration.md` §8 | +| ☑ | [#93](https://github.com/saksham2001/PulseLoopiOS/pull/93) `439ca81` | 08-09 | **Colmi R11 CRP driver** — the iOS port *of Android's own* CRP work, so the driver itself is ALREADY-HAVE. The **adversarial-review hardening** iOS added on top in `4d65b60` (5 gaps, 2026-08-22 triage note below) is now ported. | **PARTIAL** (hardening only) | S–M | `c95b6e8` (2026-08-22) — fdd3 connect gate, firmware once-per-connection, day-keyed follow-up guard, validated + narrow-trim firmware string (items 4+5). See [`crp-r11-hardening-plan.md`](crp-r11-hardening-plan.md) | ## Port priority — open items (as of 2026-08-08) @@ -123,15 +176,15 @@ seeded-data mode). **SKIP** — no portable behavior, Android has its own indepe > out so the other 10 items can land. Version bumped to 2.5.0 (`68c9788`) to match iOS > MARKETING_VERSION. -> **▶ RESUME HERE (next session):** Two open threads, in order: -> 1. **PR #45 review remediation** — the 2026-08-09 review found parity bugs in #95, #98, -> #99, #100 and a regression in #94. See "Session notes — 2026-08-09 cross-platform -> review" below. -> 2. **#130 RWfit redo** — on `feat/rwfit-ring-family`, rebuilt from -> `decompiled-rwfit-official/` (see the backed-out section below for what was wrong), -> then recombined. +> **▶ RESUME HERE:** see [**Outstanding — the single list**](#outstanding--the-single-list) above. +> It consolidates every open thread that used to be split across this block, the port queue and the +> session notes. **As of 2026-08-23 the port queue is empty** — a live `git fetch` puts +> `origin/main` at `439ca81` with local `main` 0 commits behind, and every first-parent item since +> `0d1b965` is ported. The three remaining rows are all blocked: #82 and #90 need ring hardware to +> validate code that is already written, #79 needs an Android Activity-trends screen that does not +> exist. **Do not start a port from this block — there is none to start.** > -> Next triage after those: `git -C log --first-parent --oneline 88c0f6b..main`. +> Next triage after those: `git -C log --first-parent --oneline 439ca81..main`. > > **Newly queued, independent of the two above:** **#80 → Health Connect** (re-triaged > 2026-08-09 from SKIP to ADAPT/XL). Design and a 7-phase implementation plan are written up in @@ -439,6 +492,159 @@ their own M-sized item and drop to Tier 2/3; only #61d/#61e are Tier-1-sized. - **#79 Activity Year-trends** (S) — blocked: no Activity-trends screen on Android yet (not created by #57's redesign either). - ~~**#74 Measurement-Frequency relocation**~~ ✅ **DONE** `368a3f2` (2026-07-19) — see the session note below. +### 2026-08-23 session — #96 camera features, on a real device + +Closes #96. Both camera features landed in `e80c76c`, plus a coach-provider fix in `d3d1371` +that finishing them exposed. + +**Runtime-verified on a Pixel 10 Pro (API 37, arm64), debug build, against the user's own +vLLM server** — not emulated, not inferred from tests: + +- **Barcode → OFF → prefill → save.** A real packaged-food scan resolved through Open Food + Facts and persisted: `sourceRaw=off_barcode`, `offProductCode=0010878850577`. Read back out + of `pulseloop.db`, not just off the screen. +- **Describe → LLM → review → save.** Text-only path: 317 kcal / P18 C29 F15, persisted with + `sourceRaw=llm_estimate`, `confidenceRaw=partial` (medium→partial), `notes` = the assumptions + string, meal type inferred `lunch` at 13:00. +- **Photo → vision → review → save.** *Needed human hands — the phone was handed back for + this one.* A photographed plate came back as "Spaghetti with Meatballs, Basil & Parmesan", + 880 kcal, with assumptions describing detail only visible in the image ("5 medium pan-fried + meatballs… ~20g shaved parmesan"). That text is the proof the `CoachAttachmentStore` + downscale → base64 `input_image` pipeline actually reached the model. Row persisted correctly. +- **Failed phase + retry** rendered correctly — observed for real, before the fix below. +- ML Kit initialized on device (its prefs file exists); empty crash buffer, no app-level + error or warning lines throughout. + +**The bug this exposed — worth reading before touching the local provider.** +`LocalOpenAICompatClient` ignored the caller's `text.format` entirely and substituted the coach +chat's own `coach_response` schema, in `response_format` *and* in the system prompt via +`CoachResponseSchema.promptInstruction`. On a guided-decoding backend that is not degradation, +it is impossibility: the model was constrained to one shape and instructed to produce that same +wrong shape, so `MealAnalysisLogic.decode` could never parse it. The meal estimator failed +**every** call with "The AI didn't return a usable estimate" until fixed. Every other adapter +already translated that field (`OpenRouterClient.chatResponseFormat`), so the local client was +the outlier — and `CoachSummaryGenerator` sends `text.format` the same way and had the same +latent bug. `Response format = OFF` still sends no `response_format`: that setting means the +backend rejects the field, and a caller does not get to override the user's compatibility choice. + +**Two deliberate divergences from iOS**, both as instructed: + +- The photo is **not persisted**. `MealEntryEntity` genuinely has no photo-ref column + (`NutritionEntities.kt:6-36`), and this port ships no schema migration, so the image feeds + the analysis call and is then discarded. iOS stores it via `CoachAttachmentStore` into + `photoRefJSON` (`MealAnalysisSheet.swift:296-307`). +- The entry buttons gate on **coach-enabled alone**. iOS gates on coach + cloud provider + a + nutrition photo-analysis sub-toggle (`NutritionView.swift:36-48`); Android has neither that + pref nor an on-device provider mode, so the three-part gate collapses to one. + +**Meal confidence now matches iOS** (`0765b37`, DB v22 → v23). `MealEntryEntity.confidenceRaw` +defaulted to `"medium"`, which is not a value in the known/partial/unknown vocabulary anything +else uses — every deliberate writer maps onto those three +(`NutritionTools.decodeConfidenceRaw`, `MealAnalysisLogic.confidenceRaw`, `MeasurementModal`, +`MetricsService`), and every other entity in the schema already defaults to `"known"`. A stored +`"medium"` was therefore never anyone's intent, only the default leaking through. iOS has no +such value at all: `MealEntry.init` defaults to `.known` (`NutritionModels.swift:99`) and its +reader falls back to `.known` for an unrecognized raw (:147). Entity + `DataArchive` DTO now +default to `"known"`, and `MIGRATION_22_23` rewrites the rows that already carry `"medium"` +(unconditional — no legitimate row can hold it). This also let the `MealLogSave.confidenceRaw` +plumbing go: with the default correct there is nothing to override, which is exactly iOS's +arrangement. + +Verified in place on the Pixel: `user_version` 23, no crash on upgrade, the existing +`off_barcode` row moved `medium` → `known` while both `llm_estimate` rows kept `partial`. +**No migration unit test** — this module sets `exportSchema = false`, so there is no +`MigrationTestHelper` harness to hang one on. Worth knowing before you try to add one. + +Suite 1191 → 1211, 0 failures. + +**Carry-forward rules from this session** (the durable bits, so they survive without a memory +store): + +1. **A new structured, non-chat caller on the coach provider stack must be tried against the + *local* provider, not just OpenAI/Gemini.** That is where `text.format` was being silently + discarded, and the failure mode is total, not partial. `d3d1371` fixed the client; it did not + make the class of bug impossible. +2. **`Response format = OFF` is a user compatibility choice, not a capability hint.** It means + the backend rejects `response_format` outright. A caller's schema never overrides it — the + schema goes in the prompt, and every structured caller decodes fence-tolerantly. +3. **Three ring families are shipped-but-unvalidated**: YCBT (#82), LuckRing/TK18 (#90) and + RWfit (#130). #130 in particular was rebuilt entirely from the vendor decompile with no + hardware. Blind-porting *from the decompiled vendor app* is the normal practice here; + porting from iOS parity or guesswork is what got PR #45 backed out. Say "no hardware + validation" on the PR. +4. **#79 is the one remaining feature gap and its `S` is misleading** — that sizes the iOS fix. + Android has no Activity-trends screen at all, so the real work is building the screen. +5. **Upstream is fully triaged as of 2026-08-23** (live `git fetch`: `origin/main` = `439ca81`, + local `main` 0 behind). Local `main` carries one extra commit, `f6eb177`, a demo-seed that is + not upstream — do not try to "port" it. + +### 2026-08-22 triage (since `88c0f6b` → `439ca81`, 12 commits / 1 first-parent) + +Exactly **one** untriaged first-parent item: **PR #93, the Colmi R11 CRP driver** (25 files, +2876 ins). It is not a normal upstream item — it is the *iOS port of this repo's own Android CRP +work* (`5427dc0 fix(crp): port the R11 opcode corrections and read-backs from Android`), so the +driver, the pairing card and the wear-state UX are all ALREADY-HAVE here. Verified present on +Android before writing this: `CRPDriver/CRPDecoder/CRPProtocol/CRPSyncEngine/CRPCoordinator`, +`WearableModel.colmiR11CRP` ("Colmi R11 (Da Rings app)", `forcedFamilyScanMatches`), and the +not-worn measurement hint (`RingSyncCoordinator.measureNotWorn` → `Screens.kt:320`). + +**What Android does NOT have** is the hardening iOS added afterwards in `4d65b60` ("reset the +frame assembler across reconnects, gate connect on fdd3"), an adversarial review of that branch. +Five of its eight findings apply here; three do not, for reasons worth recording so nobody +re-ports them. + +**Port these five (☑ done 2026-08-22, `c95b6e8`).** Step-by-step instructions, with the Kotlin and +the tests, are in [`crp-r11-hardening-plan.md`](crp-r11-hardening-plan.md); this list is the summary. + +1. **`CONNECTED` fires before the reply channel is live.** `CRPDriver` doesn't override + `requiredSubscriptionsBeforeConnected` (only `YCBTDriver` does), so the connection counts as up + on the first successful CCCD write — which is `fdd1` (steps), never `fdd3`, which carries every + command reply. `RingBLEClient.kt:903` already threads the driver's list through, so this is a + one-property override. Consequence today: `runStartup` writes its whole handshake (~26 frames) + into a channel we may not be listening to yet, and a lost reply is indistinguishable from a slow + one — the exact signature of the opcode bug this driver just fixed. **Highest value of the five.** +2. **Firmware is re-queried on every poll pass.** `CRPSyncEngine.runStartup` sends + `queryFirmwareVersion()` outside the `readBacksSent` gate, while the six read-backs beside it are + gated. A firmware string is exactly as immutable as a sensor roster, and this ring funnels the + handshake, timing config, history pull *and* on-demand measures through the single `fdd2` + channel (a spot SpO2 alone needs ~48 s of it). Fold it into `sendConnectionReadBacks()`. +3. **The timing-history follow-up guard ignores the day.** `requestedTimingFrames` is keyed + `cmd * 100 + frameIndex`. Every timing query is day 0 today, but this engine already issues + multi-day sleep requests (`SLEEP_BACKFILL_DAYS = 6`), so the moment vitals get the same backfill + the key silently swallows day 1's frame-1 follow-up. Key on `day` as well as `cmd`. +4. **The firmware string is coerced, not validated.** `CRPDecoder.decodeFirmwareVersion` does + `String(payload, Charsets.UTF_8)`, which substitutes U+FFFD rather than failing — a binary + payload renders as replacement characters and is presented as a firmware version. Do strict + UTF-8, then trim padding, then reject any remaining control byte to an ack. +5. **The trim is the vendor's, and it is too wide.** The same function uses `trim { it <= ' ' }`, + which strips a binary payload's leading junk and passes whatever follows — `01 02 03 41` → `"A"`. + iOS deliberately narrowed this. Ports together with (4). + +**Do NOT port these three — they don't apply to Android:** + +- **Frame-assembler reset across reconnects.** This was iOS's headline bug: auto-reconnect there + re-dialled with a bare `central.connect` and kept the driver instance, so a half-assembled frame + from the dropped link was completed with bytes from the new one and decoded as a genuine (but + fabricated) vital sample. **Android is already safe by construction** — it connects with + `autoConnect = false` (`RingBLEClient.kt:804`, matching the official QRing app) and *every* + reconnect path funnels through `beginConnect`, which calls `installDriver` (`:787`) and builds a + fresh `CRPDriver` with a fresh `CRPFrameAssembler`. Adding a `reset()` hook here would be dead + code. `CRPDriver`'s KDoc already states this invariant correctly; keep it accurate if the + reconnect path is ever changed to reuse a driver, because that is what would reintroduce the bug. +- **Half-open sleep-backfill loop.** iOS used `1...0`, which traps if the tuning knob is turned + down to today-only. Kotlin's `1..0` is simply an empty range — `for (daysAgo in + 1..SLEEP_BACKFILL_DAYS)` is already safe at `SLEEP_BACKFILL_DAYS = 0`. +- **"Once per connection" comment corrections.** Android's comments already say the right thing + (`readBacksSent` is documented as per-engine-instance, and a fresh engine is built per connect). + +**Effort:** S–M in total; (1) is a one-line override, (2)–(3) are small engine edits, (4)–(5) are +one decoder function plus tests. No schema change, no UI. Existing `CRPDecoderTest` / +`CRPSyncEngineTest` / `CRPProtocolTest` are the natural homes for the oracles — iOS added 63 lines +to `CRPDecoderTests.swift` and 33 to `CRPSyncEngineTests.swift` in the same commit, so port those. + +**Also noted:** the iOS-side R11 branch (`feat/colmi-r11-crp-driver`, `4d65b60`) is fully merged +into iOS `main`; nothing is outstanding on that branch. + ### 2026-07-20 PR #28 review + fix pass (branch `iOS_sync_2026-07-16`) Full code review of the sync batch (two passes, 10 parallel agents total, cross-referenced @@ -1399,12 +1605,14 @@ the pre-fix builder. Suite: 794 → 812. ### Still open -- **#94's actual feature** is `CoachNotificationDataTrigger` (run the due slot when a sync - completes, recovering a slot skipped for stale data). Not ported — it's an event-bus subscriber, - not the window constant that was mistaken for it. +- ~~**#94's actual feature**~~ **now ported in `9d43227`** (`CoachNotificationDataTrigger` + bus subscriber + (dateKey,slotRaw) dedupe + stale-skip). It was an event-bus subscriber, not the + window constant that had been mistaken for it. - **#96 subset**: no OFF search, no barcode scan, no AI photo analysis, no coach `log_meal` tool. -- **Pause intervals**: `activity_events` is never written on Android, so TCX can't drop paused - trackpoints yet. `totalPauseSeconds` is honoured. +- ~~**Pause intervals**~~ **now ported in `71f251e`**: `LiveWorkoutManager.pause/resume` + write the `paused`/`resumed` (+ `gps_stopped`/`gps_started`) `activity_events` and + `StravaUploader` reads them into `StravaTCXBuilder.pauseIntervals()`, so paused trackpoints drop. + `totalPauseSeconds` was already honoured. --- @@ -1496,10 +1704,15 @@ on, whereas "fixing" it would put us an hour off theirs. - **Legacy `0x7E`: complete.** Framing, serials, XOR, the `0xFE`/`0xFF` handshake, multi-packet reassembly, all six history streams, battery, manifest-gated cascade. -- **JieLi `0xAB`: framing complete, payloads not.** Handshake, battery, time sync and the ACK - discipline work; the `05`-group history bodies have their own per-type layouts that have **not** - been extracted. `RWfitSyncEngine` therefore does not request history on a JieLi link, and the - driver logs those frames rather than guessing at them. +- **JieLi `0xAB`: framing and history payloads complete (updated 2026-08-22).** Handshake, + battery, time sync and the ACK discipline work; the `05`-group history bodies are now decoded + per-type from the vendor parsers (`RWfitJLHistory.kt`; layouts in `x5/b.java` + `a0`/`V`/`T`/`Z`/`U`/`S`/`W`/`Y`/`R`, each cited), and `RWfitSyncEngine` fires the whole ported + catalog once per connection as bare `{5, type, 0x10}` triples — no payload, the vendor's own + request shape (`y.java:345-537`, `TRingHeartRateStatisticsActivity.java:545`). Remaining gaps: + sport `{5,14,16}` (`Q`), Muslim count `{5,23,16}` (`X`) and the other non-metric `05` keys (the + driver still logs those), and the `{5,x,0x30}` delete variants — which have **no vendor parser** + in `x5/b.java` at all and are never sent here. - **Feature bitmap not decoded** (`x5/b.java i()` → `SupportMenuBean`), so `bitmapGatedCapabilities` is declared but nothing grants from it yet. Manual/realtime measurement and the per-SKU sensors stay ungranted rather than being handed out unconditionally — the vendor has no legacy on-demand @@ -1509,8 +1722,11 @@ on, whereas "fixing" it would put us an hour off theirs. ### Testing -49 unit tests across `RWfitCodecTest` (20), `RWfitDecoderTest` (16) and `RWfitDriverTest` (13), -asserting vendor byte layouts rather than the implementation. Suite: 812 → 866. +Unit tests across `RWfitCodecTest` (20), `RWfitDecoderTest` (17), `RWfitJLHistoryTest` (17) and +`RWfitDriverTest` (21), asserting vendor byte layouts rather than the implementation. The 2026-08-22 +JieLi payload port added `RWfitJLHistoryTest` and replaced the old "JieLi does not request history" +driver test with the burst/once-per-connection/reply-decode set; parent re-ran the full suite after +landing: **1191 tests, 0 failures** (was 1170). **No hardware validation.** Nothing here has talked to a real RWfit ring. Say so on the PR.