diff --git a/PulseLoop/Events/PulseEventBus.swift b/PulseLoop/Events/PulseEventBus.swift index 84de7d80..0c656f93 100644 --- a/PulseLoop/Events/PulseEventBus.swift +++ b/PulseLoop/Events/PulseEventBus.swift @@ -76,9 +76,6 @@ actor PulseEventBus { final class EventPersistenceSubscriber { private let context: ModelContext private var task: Task? - /// Days (midnight) whose ring-history activity total has been reset during the current sync run, so - /// each day is zeroed once on its first bucket rather than all days up front. - private var activityDaysResetThisRun: Set = [] init(context: ModelContext) { self.context = context @@ -158,17 +155,13 @@ final class EventPersistenceSubscriber { payloadJSON: #"{"steps":\#(row.steps),"calories":\#(Int(row.calories)),"distance_m":\#(Int(row.distanceMeters))}"# )) case let .activityBucket(timestamp, steps, distanceMeters): - // Per-quarter-hour ring history: sum into the day (calories omitted — unverified field). - // Reset a day's total only on the *first* bucket seen for it this sync run, so a stalled or - // aborted sync never leaves a day zeroed-but-empty (idempotent re-sync without up-front wipe). - let dayKey = Calendar.current.startOfDay(for: timestamp) - let resetThisDay = !activityDaysResetThisRun.contains(dayKey) - if resetThisDay { activityDaysResetThisRun.insert(dayKey) } - ActivityService.applyActivityBucket(date: timestamp, steps: steps, distanceMeters: distanceMeters, resetDay: resetThisDay, context: context) + // Per-quarter-hour ring history: upserted by timestamp + the day total recomputed as the + // sum of distinct buckets, so re-syncs are idempotent (no drift). Calories omitted. + ActivityService.applyActivityBucket(date: timestamp, steps: steps, distanceMeters: distanceMeters, context: context) case .activitySyncReset: - // A fresh ring history sync is starting: clear the per-run reset tracking so each day gets - // zeroed once on its first incoming bucket (not all days up front). - activityDaysResetThisRun.removeAll() + // No longer needed — bucket upsert-by-timestamp makes re-syncs idempotent on its own. + // Kept as a no-op so the (still-published) event doesn't fall through to `unknown`. + break case let .heartRateSample(bpm, timestamp): persistMeasurement(kind: .heartRate, value: Double(bpm), timestamp: timestamp, source: .live, kindLabel: "hr_sample") case let .spo2Result(value, timestamp): diff --git a/PulseLoop/Models/PulseModels.swift b/PulseLoop/Models/PulseModels.swift index 8d3a3094..76c29b67 100644 --- a/PulseLoop/Models/PulseModels.swift +++ b/PulseLoop/Models/PulseModels.swift @@ -499,6 +499,32 @@ final class ActivitySample { } } +/// One intraday activity bucket from a ring's history sync (e.g. a Colmi quarter-hour `0x43` sample). +/// Keyed by `startEpoch` (the bucket's unix start time) so re-syncing the same bucket **replaces** it +/// rather than accumulating — the daily total is then the sum of distinct buckets at read time. This +/// is the GadgetBridge model and the fix for daily totals drifting upward across repeated syncs. +@Model +final class ActivityBucketSample { + /// Bucket start time in unix seconds — unique, so the same bucket upserts instead of duplicating. + @Attribute(.unique) var startEpoch: Int + var date: Date // startOfDay for the bucket, for fast per-day queries + var timestamp: Date // bucket start instant + var steps: Int + var distanceMeters: Double + var source: String + var updatedAt: Date + + init(timestamp: Date, steps: Int, distanceMeters: Double, source: String = "ring_history") { + self.startEpoch = Int(timestamp.timeIntervalSince1970) + self.date = Calendar.current.startOfDay(for: timestamp) + self.timestamp = timestamp + self.steps = steps + self.distanceMeters = distanceMeters + self.source = source + self.updatedAt = Date() + } +} + @Model final class ActivityGpsPoint { @Attribute(.unique) var id: UUID diff --git a/PulseLoop/Persistence/ModelContainerFactory.swift b/PulseLoop/Persistence/ModelContainerFactory.swift index 2afaf919..6e7d774b 100644 --- a/PulseLoop/Persistence/ModelContainerFactory.swift +++ b/PulseLoop/Persistence/ModelContainerFactory.swift @@ -14,6 +14,7 @@ enum ModelContainerFactory { UserGoal.self, ActivitySession.self, ActivitySample.self, + ActivityBucketSample.self, ActivityGpsPoint.self, ActivityEvent.self, ActivitySensorPollEvent.self, diff --git a/PulseLoop/Persistence/SeedData.swift b/PulseLoop/Persistence/SeedData.swift index b627b472..b4351661 100644 --- a/PulseLoop/Persistence/SeedData.swift +++ b/PulseLoop/Persistence/SeedData.swift @@ -163,6 +163,7 @@ enum SeedData { deleteAll(UserGoal.self, context) deleteAll(ActivitySession.self, context) deleteAll(ActivitySample.self, context) + deleteAll(ActivityBucketSample.self, context) deleteAll(ActivityGpsPoint.self, context) deleteAll(ActivityEvent.self, context) deleteAll(CoachConversation.self, context) diff --git a/PulseLoop/PulseLoopApp.swift b/PulseLoop/PulseLoopApp.swift index 22c19891..dda04fe4 100644 --- a/PulseLoop/PulseLoopApp.swift +++ b/PulseLoop/PulseLoopApp.swift @@ -47,6 +47,9 @@ struct PulseLoopApp: App { } self.container = container + // One-time cleanup of activity totals inflated by the old accumulator bug. + ActivityService.migrateInflatedActivityIfNeeded(context: container.mainContext) + // Don't bring up CoreBluetooth under tests (see `isRunningUnitTests`). let client = RingBLEClient(startManager: !runningTests) let coordinator = RingSyncCoordinator(client: client, context: container.mainContext) diff --git a/PulseLoop/RingProtocol/ColmiEncoder.swift b/PulseLoop/RingProtocol/ColmiEncoder.swift index ae7b78e3..fb41c1bb 100644 --- a/PulseLoop/RingProtocol/ColmiEncoder.swift +++ b/PulseLoop/RingProtocol/ColmiEncoder.swift @@ -53,6 +53,16 @@ struct ColmiEncoder { func writePref(_ command: UInt8, enabled: Bool) -> [UInt8] { [command, ColmiCommandID.prefWrite, enabled ? 0x01 : 0x00] } + + /// Enable/disable all-day **heart-rate** monitoring. Auto-HR (`0x16`) has a different shape from the + /// other prefs (per GadgetBridge `onSetHeartRateMeasurementInterval`): the on/off flag is + /// `0x01`(on)/`0x02`(off) — *not* `0x01`/`0x00` — and it carries the sampling interval in minutes. + /// The interval is rounded to a 5-minute multiple, 5…60. Without this the ring records no background + /// HR, so the HR-history sync (`0x15`) comes back empty. + func autoHeartRate(enabled: Bool, intervalMinutes: Int = 5) -> [UInt8] { + let interval = UInt8(min(60, max(5, (intervalMinutes / 5) * 5))) + return [ColmiCommandID.autoHRPref, ColmiCommandID.prefWrite, enabled ? 0x01 : 0x02, interval] + } /// Temperature pref has an extra `0x03` byte before the read/write flag. func readTempPref() -> [UInt8] { [ColmiCommandID.autoTempPref, 0x03, ColmiCommandID.prefRead] } func readGoals() -> [UInt8] { [ColmiCommandID.goals, ColmiCommandID.prefRead] } diff --git a/PulseLoop/RingProtocol/ColmiSyncEngine.swift b/PulseLoop/RingProtocol/ColmiSyncEngine.swift index 1b111da8..6c8fd436 100644 --- a/PulseLoop/RingProtocol/ColmiSyncEngine.swift +++ b/PulseLoop/RingProtocol/ColmiSyncEngine.swift @@ -62,8 +62,10 @@ final class ColmiSyncEngine: RingSyncEngine { writer?.enqueue(Data(encoder.readPref(ColmiCommandID.autoHRVPref))) writer?.enqueue(Data(encoder.readTempPref())) writer?.enqueue(Data(encoder.readGoals())) - // Enable all-day measurement so the ring actually accumulates data the big-data history can - // return (without these, SpO2/stress/HRV/temp history come back empty — e.g. spot SpO2 fails). + // Enable all-day measurement so the ring actually accumulates data the history sync can + // return (without these, the metric history comes back empty). + // Heart rate uses a dedicated 0x16 command (different shape from the other prefs). + writer?.enqueue(Data(encoder.autoHeartRate(enabled: true, intervalMinutes: 5))) writer?.enqueue(Data(encoder.writePref(ColmiCommandID.autoSpo2Pref, enabled: true))) writer?.enqueue(Data(encoder.writePref(ColmiCommandID.autoStressPref, enabled: true))) writer?.enqueue(Data(encoder.writePref(ColmiCommandID.autoHRVPref, enabled: true))) diff --git a/PulseLoop/Services/PulseServices.swift b/PulseLoop/Services/PulseServices.swift index 3a2b7c97..744f5097 100644 --- a/PulseLoop/Services/PulseServices.swift +++ b/PulseLoop/Services/PulseServices.swift @@ -588,30 +588,60 @@ enum ActivityService { /// Tag for days whose totals are summed from ring history buckets (vs. live cumulative updates). static let ringHistorySource = "ring_history" - /// Add one intraday activity **bucket** into its day. Unlike `applyActivityUpdate` (which ratchets - /// cumulative live totals with `max()`), buckets are *summed* — the ring sends ~96 quarter-hour - /// buckets per day and the daily total is their sum. Idempotency across re-syncs comes from - /// `resetDay: true` on the first bucket of each day in a sync run (replace, then sum). Calories are - /// intentionally not summed (the ring's calorie field is unverified). + /// One-time cleanup of `ActivityDaily` rows inflated by the old `+=` accumulator bug (steps that + /// compounded into the millions across repeated syncs). Deletes ring-history daily rows so they get + /// recomputed cleanly from buckets on the next sync. Idempotent + UserDefaults-gated so it runs once. + static func migrateInflatedActivityIfNeeded(context: ModelContext) { + let key = "activityBucketMigration.v1" + guard !UserDefaults.standard.bool(forKey: key) else { return } + for row in MetricsRepository.activityRows(context: context) where row.source == ringHistorySource { + context.delete(row) + } + try? context.save() + UserDefaults.standard.set(true, forKey: key) + } + + /// Persist one intraday activity **bucket** from ring history (e.g. a Colmi quarter-hour `0x43` + /// sample) and recompute its day's total. The bucket is **upserted by its start time** into + /// `ActivityBucketSample`, so re-syncing the same bucket *replaces* it (never accumulates), and the + /// day's `ActivityDaily.steps/distance` is recomputed as the **sum of distinct buckets** for that + /// day. This is the GadgetBridge model and fixes daily totals drifting upward across repeated syncs. + /// Calories are intentionally not summed (the ring's calorie field is unverified). @discardableResult - static func applyActivityBucket(date: Date, steps: Int, distanceMeters: Double, resetDay: Bool = false, syncedAt: Date = Date(), context: ModelContext) -> ActivityDaily { + static func applyActivityBucket(date timestamp: Date, steps: Int, distanceMeters: Double, syncedAt: Date = Date(), context: ModelContext) -> ActivityDaily { + let dayStart = Calendar.current.startOfDay(for: timestamp) + let epoch = Int(timestamp.timeIntervalSince1970) + + // Upsert the bucket sample by its unique start epoch (replace on re-sync). + if let existing = (try? context.fetch(FetchDescriptor( + predicate: #Predicate { $0.startEpoch == epoch } + )))?.first { + existing.steps = steps + existing.distanceMeters = distanceMeters + existing.updatedAt = Date() + } else { + context.insert(ActivityBucketSample(timestamp: timestamp, steps: steps, distanceMeters: distanceMeters, source: ringHistorySource)) + } + // Persist the upsert so the recompute fetch below reliably sees it (SwiftData fetches don't + // always include pending inserts). + try? context.save() + + // Recompute the day's total from all its buckets (sum of distinct samples). + let buckets = (try? context.fetch(FetchDescriptor( + predicate: #Predicate { $0.date == dayStart } + ))) ?? [] + let totalSteps = buckets.reduce(0) { $0 + $1.steps } + let totalDistance = buckets.reduce(0.0) { $0 + $1.distanceMeters } + let row: ActivityDaily - if let existing = MetricsRepository.activity(on: date, context: context) { + if let existing = MetricsRepository.activity(on: dayStart, context: context) { row = existing } else { - row = ActivityDaily(date: date, source: ringHistorySource) + row = ActivityDaily(date: dayStart, source: ringHistorySource) context.insert(row) } - // On the first bucket of a fresh sync run, replace the day's totals (don't accumulate across - // re-syncs). Subsequent buckets for the same day sum in. This keeps re-syncs idempotent without - // zeroing days up front (so a stalled sync can't blank a day that gets no data). - if resetDay { - row.steps = steps - row.distanceMeters = distanceMeters - } else { - row.steps += steps - row.distanceMeters += distanceMeters - } + row.steps = totalSteps + row.distanceMeters = totalDistance row.source = ringHistorySource row.syncedAt = syncedAt row.updatedAt = Date() diff --git a/PulseLoopTests/ActivityServiceTests.swift b/PulseLoopTests/ActivityServiceTests.swift index 8086cca0..aac98ff7 100644 --- a/PulseLoopTests/ActivityServiceTests.swift +++ b/PulseLoopTests/ActivityServiceTests.swift @@ -14,6 +14,36 @@ final class ActivityServiceTests: XCTestCase { XCTAssertEqual(row?.steps, 6000, "counters only ratchet upward") } + func testActivityBucketsSumPerDayAndResyncIsIdempotent() throws { + let context = try TestSupport.makeContext() + let day = Calendar.current.startOfDay(for: Date()) + // Three 15-min buckets on the same day. + let buckets: [(min: Int, steps: Int, dist: Double)] = [ + (0, 100, 70), (15, 250, 180), (30, 50, 35), + ] + func runSync() { + for b in buckets { + let ts = Calendar.current.date(byAdding: .minute, value: b.min, to: day)! + ActivityService.applyActivityBucket(date: ts, steps: b.steps, distanceMeters: b.dist, context: context) + } + } + runSync() + let afterFirst = MetricsRepository.activity(on: day, context: context) + XCTAssertEqual(afterFirst?.steps, 400, "day total = sum of distinct buckets") + XCTAssertEqual(afterFirst?.distanceMeters ?? 0, 285, accuracy: 0.001) + + // Re-sync the exact same buckets — must NOT accumulate (upsert by timestamp). + runSync() + let afterResync = MetricsRepository.activity(on: day, context: context) + XCTAssertEqual(afterResync?.steps, 400, "re-sync is idempotent (no drift)") + + // A bucket re-sent with the same timestamp but updated value replaces, not adds. + let firstTs = day + ActivityService.applyActivityBucket(date: firstTs, steps: 999, distanceMeters: 700, context: context) + let afterUpdate = MetricsRepository.activity(on: day, context: context) + XCTAssertEqual(afterUpdate?.steps, 400 - 100 + 999, "updated bucket replaces its old value") + } + func testHRActiveMinutesLiveDense() throws { let context = try TestSupport.makeContext() // 6 dense live samples in one minute, all above the active threshold (>=100 bpm floor). diff --git a/PulseLoopTests/ColmiDecoderTests.swift b/PulseLoopTests/ColmiDecoderTests.swift index 711d50a1..cc9b2c7e 100644 --- a/PulseLoopTests/ColmiDecoderTests.swift +++ b/PulseLoopTests/ColmiDecoderTests.swift @@ -289,17 +289,13 @@ final class ColmiDecoderTests: XCTestCase { func testActivityBucketSummingIsIdempotentAcrossResync() throws { let context = try TestSupport.makeContext() - // Mirror EventPersistenceSubscriber: reset each day once on its first bucket of the run. + // Buckets are upserted by timestamp, so a re-sync of the same packets must not inflate. func runSync() { - var resetDays: Set = [] for hex in Self.realActivityBuckets { guard let data = try? Data(hexString: hex) else { continue } let events = decoder.decodeHistory(data, day: activityNow, calendar: calendar, now: activityNow) if case let .activityBucket(ts, steps, dist) = events.first { - let dayKey = calendar.startOfDay(for: ts) - let reset = !resetDays.contains(dayKey) - resetDays.insert(dayKey) - ActivityService.applyActivityBucket(date: ts, steps: steps, distanceMeters: dist, resetDay: reset, context: context) + ActivityService.applyActivityBucket(date: ts, steps: steps, distanceMeters: dist, context: context) } } try? context.save() @@ -371,6 +367,20 @@ final class ColmiDecoderTests: XCTestCase { XCTAssertNil(driver.commandUUID) XCTAssertFalse(driver.usesCommandChannel(for: Data([0x14]))) } + + // MARK: Auto-HR enable (0x16) — distinct shape from the other prefs + + func testAutoHeartRateEnableFormat() { + let enc = ColmiEncoder() + // Enabled at 5-min interval: 16 02 01 05 (on/off flag is 0x01/0x02, plus interval minutes). + XCTAssertEqual(enc.autoHeartRate(enabled: true, intervalMinutes: 5), [0x16, 0x02, 0x01, 0x05]) + // Disabled uses 0x02 (not 0x00). + XCTAssertEqual(enc.autoHeartRate(enabled: false, intervalMinutes: 5)[2], 0x02) + // Interval is rounded to a 5-min multiple and clamped to 5...60. + XCTAssertEqual(enc.autoHeartRate(enabled: true, intervalMinutes: 0)[3], 5) + XCTAssertEqual(enc.autoHeartRate(enabled: true, intervalMinutes: 999)[3], 60) + XCTAssertEqual(enc.autoHeartRate(enabled: true, intervalMinutes: 12)[3], 10) + } } /// A no-op command writer for driver tests.