diff --git a/lib/ble/ble_engine.dart b/lib/ble/ble_engine.dart index ccaa775..63a0560 100644 --- a/lib/ble/ble_engine.dart +++ b/lib/ble/ble_engine.dart @@ -603,6 +603,125 @@ class BleEngine { /// True while live is in the background HR-only downgrade. bool get liveHrOnly => _liveEnabled && _liveHrOnly; + + // ── link power (issue #200) ───────────────────────────────────────────────── + // Android's connection priority was requested ONCE at connect setup and never + // stepped back down, so an ~11.25 ms interval was held for the entire life of + // a deliberately-permanent connection. [desiredLinkPriority] decides what the + // link should be running at; [_applyLinkPriority] is the one place that talks + // to the radio, and it is a no-op when nothing changed. + bool _backgrounded = false; + LinkPriority? _appliedPriority; + bool _priorityInFlight = false; + bool _priorityRestale = false; + + /// Bumped by every teardown. Captured before a priority request and re-checked + /// after it: `_teardownSession` clears `_appliedPriority` at its top but nulls + /// `_session` only after awaiting subscription cancels, so an identity check + /// on the session alone still passes inside that window — and the reply then + /// restores the value teardown had just cleared, leaving the NEXT connection + /// convinced it had already asked. + int _linkGeneration = 0; + + /// True from the start of connect setup until INIT has been sent. Setup is + /// discovery + subscribes + SET_CLOCK + INIT and is immediately followed by + /// the first flash drain, so it wants the fast interval for the same reason + /// an offload does — but it must ask for it through [_applyLinkPriority] like + /// everything else, or the direct request races the serialized ones and + /// leaves `_appliedPriority` describing a target the radio never got. + bool _connectSetup = false; + + /// What the link should be running at given the engine's CURRENT state. The + /// wiring under test: that `_connectSetup` counts as offload-grade traffic, + /// and that `sendInit` clears it again. + @visibleForTesting + LinkPriority linkPriorityForCurrentState() => desiredLinkPriority( + offloadActive: _offloadActive || _connectSetup, + background: _backgrounded, + hasLiveConsumer: _liveEnabled && !_liveHrOnly, + ); + + @visibleForTesting + void debugBeginConnectSetup() => _connectSetup = true; + + /// Told by AppState on every foreground/background transition. Drives the + /// connection interval — see [desiredLinkPriority]. + void setBackground(bool value) { + if (_backgrounded == value) return; + _backgrounded = value; + unawaited(_applyLinkPriority()); + } + + /// Bring the link to the priority the current state calls for. + /// + /// SERIALIZED, and the target is recomputed inside the loop rather than at + /// call time. Every caller fires this unawaited from a state transition + /// (background, live mode, offload), so two can overlap; if they did, the + /// slower one's completion would write ITS target into `_appliedPriority` + /// last. The radio would then sit at one interval while the field claimed + /// another, and the `want == _appliedPriority` check below — the thing that + /// keeps this from spamming the radio — would skip the next legitimate + /// step-down, leaving the link fast exactly when it should go quiet. + Future _applyLinkPriority() async { + if (!Platform.isAndroid) return; // iOS picks its own interval + if (_priorityInFlight) { + // Someone is mid-request; make them re-evaluate when they land rather + // than issuing a competing one. + _priorityRestale = true; + return; + } + _priorityInFlight = true; + try { + do { + _priorityRestale = false; + final session = _session; + if (session == null || !session.connected) return; + final want = desiredLinkPriority( + offloadActive: _offloadActive || _connectSetup, + background: _backgrounded, + hasLiveConsumer: _liveEnabled && !_liveHrOnly, + ); + if (want == _appliedPriority) continue; + final generation = _linkGeneration; + try { + await session.device.requestConnectionPriority( + connectionPriorityRequest: switch (want) { + LinkPriority.high => ConnectionPriority.high, + LinkPriority.balanced => ConnectionPriority.balanced, + LinkPriority.lowPower => ConnectionPriority.lowPower, + }, + ); + // Only remember it if the link we asked is still the live one. A + // teardown during the await clears `_appliedPriority` precisely so + // the next session re-requests from scratch (Android resets the + // interval per GATT connection); writing this session's target in + // afterwards would make the new link skip its own request. + if (generation != _linkGeneration || + !identical(_session, session) || + !session.connected) { + // Do not record it against the dead link, and do not swallow a + // transition that arrived while we were waiting: loop once more so + // the replacement session (if there is one) gets its own target. + _log('Link priority reply arrived after teardown — discarded.'); + _priorityRestale = true; + continue; + } + _appliedPriority = want; + _log('Link priority → ${want.name}.'); + } catch (e) { + // Leave `_appliedPriority` alone so this is retried. The retry is the + // keep-alive tick calling back in, NOT this loop — spinning here + // against a radio that just refused would hammer it. Without a + // retry at all, a failed step-DOWN would hold the fast interval + // until the next state change, which overnight means until morning. + _log('requestConnectionPriority(${want.name}) failed: $e'); + } + } while (_priorityRestale); + } finally { + _priorityInFlight = false; + } + } + bool _offloadActive = false; final List _offloadFrames = []; bool _drainingOffloadFrames = false; @@ -632,6 +751,30 @@ class BleEngine { // caller pauses the auto-reconnect loop instead of pinning the radio forever. // A single successful bond clears it (see the createBond block below). final BondRefusalGiveUp _bondGiveUp = BondRefusalGiveUp(); + + /// Clear a bond-refusal auto-reconnect pause whose cooldown has expired, and + /// report whether the pause is still in force (issue #208). + /// + /// The pause was previously cleared in exactly ONE place: the `createBond()` + /// success branch. That branch is inside the connect path, which the pause + /// itself stops from ever running — so the flag latched for the life of the + /// process, and the Android foreground service made sure the process outlived + /// any reason for it. [BondRefusalGiveUp.stillPaused] expires it after a + /// cooldown; a band that genuinely will not bond simply re-trips. + bool refreshAutoReconnectPause() { + if (!state.autoReconnectPaused) return false; + if (_bondGiveUp.stillPaused(DateTime.now())) return true; + state.autoReconnectPaused = false; + // Clear what the pause put on screen, too. Leaving `needsRepairGuide` set + // tells the user to re-pair while auto-reconnect has quietly re-armed + // behind the message, and a `bondRefusals` count that keeps climbing while + // the give-up streak restarts at 1 no longer means anything. + state.needsRepairGuide = false; + state.bondRefusals = 0; + _log('[RECONNECT] bond-refusal pause expired — auto-reconnect re-armed.'); + onState(state); + return false; + } // Real per-chunk failure tracking (see ChunkFailureLedger doc) — persists // across reconnects like marginal-radio/post-bond-loop/bond-give-up, since // the whole point is catching the SAME token failing across sessions. @@ -1096,13 +1239,14 @@ class BleEngine { } catch (e) { _log('requestMtu failed: $e — MTU stays at the connection default.'); } - if (Platform.isAndroid) { - try { - await device.requestConnectionPriority( - connectionPriorityRequest: ConnectionPriority.high, - ); - } catch (_) {} - } + // Setup is immediately followed by INIT + the first flash drain, which is + // exactly when throughput matters, so `_connectSetup` asks for the fast + // interval — through the SAME serialized helper as every other + // transition. `_applyLinkPriority` steps it back down once the offload + // ends (issue #200); before that, `high` was requested here and then held + // for the entire life of a deliberately-permanent connection. + _connectSetup = true; + await _applyLinkPriority(); if (!session.connected) { _log('connect: link dropped during setup.'); @@ -1290,7 +1434,50 @@ class BleEngine { } _send(Cmd.toggleRealtimeHr, const [0x01]); } - _send(Cmd.getBatteryLevel, const []); + // Battery is a DISPLAY value that moves over hours. Polling it on every + // 30 s keep-alive tick was 2,880 radio round-trips a day for a handful of + // real changes (issue #200). + // + // BUT it is also load-bearing for liveness: `_lastRx` only advances on an + // inbound notification, and with no live stream armed the battery REPLY is + // the only inbound traffic this link generates (LINK_VALID is a write; the + // band is not known to answer it). Left purely on a 5-minute cadence, a + // quiet link would sail past the 120 s fuse and get bounced — trading a + // power win for a reconnect storm. So: poll on the slow cadence normally, + // and force one as soon as silence approaches the fuse. + unawaited( + _pollBatteryIfDue( + force: sinceLastRx.inSeconds > kLivenessFuseSeconds ~/ 2, + ), + ); + // Cheap retry hook for a priority request that failed earlier: a no-op + // whenever the link already sits at the wanted interval. + unawaited(_applyLinkPriority()); + } + + DateTime? _lastBatteryPollAt; + + /// Ask the band for its battery level, at most once per + /// [kBatteryPollIntervalSeconds]. + /// + /// The stamp moves only after the write actually goes out, so a failed write + /// does not buy five minutes of silence — and [getBattery] shares this path + /// so the read AppState does right after connecting isn't immediately + /// followed by a duplicate from the first keep-alive tick. + Future _pollBatteryIfDue({bool force = false}) async { + final last = _lastBatteryPollAt; + if (!force && + last != null && + DateTime.now().difference(last).inSeconds < + kBatteryPollIntervalSeconds) { + return; + } + // `_send` swallows write failures and reports them as false. Stamping + // regardless would buy five minutes of silence off a write that never left + // the phone. + if (await _send(Cmd.getBatteryLevel, const [])) { + _lastBatteryPollAt = DateTime.now(); + } } /// Trigger a historical offload, floored by [BackfillPolicy] (manual / @@ -2731,9 +2918,19 @@ class BleEngine { // ── high-level flows ───────────────────────────────────────────────────────────── Future sendInit() async { _log('Sending 5-packet INIT…'); - for (final pkt in initPackets) { - await _write(pkt); - await Future.delayed(const Duration(milliseconds: 120)); + try { + for (final pkt in initPackets) { + await _write(pkt); + await Future.delayed(const Duration(milliseconds: 120)); + } + } finally { + // Setup is over. The flood INIT triggers raises the link on its own via + // `_setOffloadActive`, so from here the ordinary rules apply — and an + // idle link stops paying for the fast interval. + if (_connectSetup) { + _connectSetup = false; + unawaited(_applyLinkPriority()); + } } } @@ -2925,7 +3122,7 @@ class BleEngine { _log('SET_ADVERTISING_NAME → "$name"'); } - Future getBattery() => _send(Cmd.getBatteryLevel, const []); + Future getBattery() => _pollBatteryIfDue(force: true); Future getHello() => _send(Cmd.getHelloHarvard, const [0x00]); Future buzz() => buzzPattern(hapticShortPulse); @@ -2963,6 +3160,7 @@ class BleEngine { Future enableLiveStreams() async { _liveEnabled = true; _liveHrOnly = false; + unawaited(_applyLinkPriority()); // a live consumer earns the fast interval _armTime = DateTime.now(); // marginal-radio detector measures arm→drop latency await _send(Cmd.toggleRealtimeHr, const [0x01]); @@ -3010,6 +3208,7 @@ class BleEngine { if (_session?.connected != true) return; _liveEnabled = true; _liveHrOnly = true; + unawaited(_applyLinkPriority()); // downgraded to HR-only ⇒ step the link down await _send(Cmd.toggleRealtimeHr, const [0x01]); final offOps = >[ [ @@ -3066,6 +3265,7 @@ class BleEngine { } _liveEnabled = false; _liveHrOnly = false; + unawaited(_applyLinkPriority()); // no live consumer left _armTime = null; state.liveHr = null; // No phase change — we stay `listening`; only the live R10/R11/optical streams @@ -3102,6 +3302,17 @@ class BleEngine { final session = _session; if (session == null) return; session.intentionalClose = intentional; + // Per-link state: Android resets the connection interval on every new GATT + // connection, so a remembered priority would make the next link skip its + // request. The battery stamp resets too — a fresh session should read the + // level once rather than inheriting the last link's 5-minute cooldown. + _appliedPriority = null; + _lastBatteryPollAt = null; + // Every failure exit in `_doConnect` between setting this and `sendInit` + // skips the clear in sendInit's finally, which would leave the target + // pinned at `high` for the life of the process. + _connectSetup = false; + _linkGeneration++; _drain?.onLinkDown(); _drain = null; // Fire a final derive for anything stored-but-not-yet-derived, then disarm the @@ -3133,6 +3344,9 @@ class BleEngine { void _setOffloadActive(bool active) { if (_offloadActive == active) return; _offloadActive = active; + // An offload is the one thing that genuinely needs the fast interval; as + // soon as it ends the link steps back down (issue #200). + unawaited(_applyLinkPriority()); onOffloadState?.call(active); } diff --git a/lib/compute/manual_session.dart b/lib/compute/manual_session.dart index 096e25e..a1f7182 100644 --- a/lib/compute/manual_session.dart +++ b/lib/compute/manual_session.dart @@ -415,3 +415,135 @@ List supersededSuggestionIds( } return out; } + +/// The best available scoring of a live-captured session's window: the tallies +/// the live gauge accumulated in RAM, reconciled against a re-score of the SAME +/// window from the 1 Hz substrate. +class ReconciledSessionScore { + const ReconciledSessionScore({ + this.strain, + this.calories, + this.maxHr, + this.zoneMinutes = const [], + this.changed = false, + }); + + final double? strain; + final double? calories; + final int? maxHr; + final List zoneMinutes; + + /// True when the substrate improved on at least one stored field — the only + /// case worth a write. + final bool changed; +} + +/// Reconcile a stored live session against a substrate re-score of its window. +/// +/// WHY THIS EXISTS (issue #206): a live session's strain/calories/zone minutes +/// are accumulated in RAM, one tick per second, by the foreground app. That +/// accumulator sees nothing while the app is suspended — iOS suspends the 1 Hz +/// `Timer.periodic` the moment the app backgrounds, and an app the OS kills +/// mid-workout resumes with an EMPTY accumulator (`_reconcileOrphanedLiveWorkout` +/// rehydrates the row, not the tallies). Stop the workout after that and the +/// stored strain describes only the handful of minutes the app happened to be +/// awake for — commonly a few sub-resting minutes, whose Banister TRIMP is +/// exactly 0, which `strainScore` reports as a confident `0.0`. The user sees a +/// real duration, real avg/max HR and real zone bands next to "0.0 Strain". +/// +/// The band, meanwhile, banked the whole window at 1 Hz. Once that window has +/// drained into `decoded_onehz`, re-scoring it through the SAME method +/// ([computeManualSessionStats]) recovers the real number. +/// +/// THE MERGE RULE IS `max`, and that is deliberate — not a heuristic: +/// both numbers are the same monotone function (TRIMP is a sum of +/// per-minute non-negative terms) evaluated over SUBSETS of one window's +/// minutes. The live tally saw the minutes the app was awake for; the substrate +/// sees the minutes the band has drained so far. Each is therefore a LOWER +/// BOUND on the true score, and the larger one is strictly the better estimate. +/// Taking the max can never double-count (it is a max over two views of one +/// window, not a sum) and it is monotone under repeated application, so calling +/// this again after more of the window drains only ever improves the value and +/// converges. Averaging or preferring one source outright would both be wrong: +/// the substrate is empty right after a workout (the band has not offloaded +/// yet) and the live tally is empty after an app kill. +/// +/// Absent stays absent: a null on both sides stays null rather than becoming +/// `0.0`. [substrate] must be the re-score of exactly `[start_ts, end_ts)`. +ReconciledSessionScore reconcileSessionScore({ + required double? liveStrain, + required double? liveCalories, + required int? liveMaxHr, + required List liveZoneMinutes, + required ManualSessionStats substrate, + + /// True when [substrate] covers essentially the whole window, i.e. the band + /// has finished handing this workout over. + /// + /// It then REPLACES the live tally rather than being maxed against it, and + /// that distinction matters more than it looks. The `max` rule is only + /// monotone while the scoring function is fixed, and it is not: the score + /// depends on the trailing nightly resting HR and on Tanaka HRmax, both of + /// which move. Maxing every re-score against the stored value would make a + /// session converge to the highest strain ANY resting-HR the profile has + /// ever reported would have produced — one artefactually low nightly RHR + /// would inflate a workout permanently, with no way back down. Once the + /// window is fully covered there is nothing left to recover, so the honest + /// value is simply the current score. + bool substrateIsComplete = false, +}) { + // No substrate for this window (not drained yet, or pruned) — the live tally + // is all the evidence there is. + if (substrate.isUnscored) { + return ReconciledSessionScore( + strain: liveStrain, + calories: liveCalories, + maxHr: liveMaxHr, + zoneMinutes: liveZoneMinutes, + ); + } + + // ONE definition of the rule, for every scalar. Complete coverage: the + // substrate IS the answer, falling back to the live value only where it has + // nothing to say. Partial: both sides are lower bounds over subsets of the + // same minutes, so the larger is the better estimate and the smaller is just + // a less complete view. + T? better(T? live, T? sub) { + if (substrateIsComplete) return sub ?? live; + if (live == null) return sub; + if (sub == null) return live; + return live >= sub ? live : sub; + } + + final strain = better(liveStrain, substrate.strain); + final calories = better(liveCalories, substrate.calories); + final maxHr = better(liveMaxHr, substrate.maxHr); + + // Zone minutes are a vector of the same lower-bound quantity, so take the + // side with more total measured minutes rather than mixing two partial + // splits (a per-element max would invent a total neither source observed). + // Same shape as `better`: an empty substrate vector says nothing, so it must + // not wipe a stored split just because coverage is complete (zone minutes + // need a HRmax the profile may not carry, so an empty vector is a real case). + double total(List z) => z.fold(0.0, (a, b) => a + b); + final zone = substrate.zoneMinutes.isEmpty + ? liveZoneMinutes + : (substrateIsComplete || + total(substrate.zoneMinutes) > total(liveZoneMinutes) + ? substrate.zoneMinutes + : liveZoneMinutes); + + final changed = + strain != liveStrain || + calories != liveCalories || + maxHr != liveMaxHr || + !identical(zone, liveZoneMinutes); + + return ReconciledSessionScore( + strain: strain, + calories: calories, + maxHr: maxHr, + zoneMinutes: zone, + changed: changed, + ); +} diff --git a/lib/data/db.dart b/lib/data/db.dart index a410d4c..2f880d2 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -875,6 +875,12 @@ class LocalDb { /// Used by the pedometer sync to tell "this day really had no steps" from /// "this read came back empty" before it replaces a day wholesale — see /// [replacePhoneCoverageForDay], which is delete-then-insert. + /// + /// Also the UI's source discriminator: it is the same quantity + /// [liveStepsForDay] tests to decide which source owns the day, so a screen + /// can ask "did the phone actually cover today?" instead of approximating it + /// with "is the toggle on". Those differ exactly when the toggle is on and + /// the phone has no data, where the band still owns the day. static Future phoneStepsForDay(String day) async { final db = await instance; final r = await db.rawQuery( @@ -4539,6 +4545,35 @@ class LocalDb { ); } + /// Update ONLY a session's derived score columns. + /// + /// Deliberately not `putSession`: that is INSERT-OR-REPLACE over the whole + /// row, so a re-score computed from a snapshot would also rewrite columns it + /// never read — `hrr_bpm` (backfilled by the derivation engine) and `type` + /// (the athlete correcting a mislabelled workout) are both written by their + /// own narrow UPDATEs and would be reverted. Returns the number of rows + /// changed (0 when the session has since been deleted). + static Future setSessionScores( + String id, { + required double? strain, + required double? calories, + required int? maxHr, + required String zoneMinJson, + }) async { + final db = await instance; + return db.update( + 'sessions', + { + 'strain': strain, + 'calories': calories, + 'max_hr': maxHr, + 'zone_min_json': zoneMinJson, + }, + where: 'id = ?', + whereArgs: [id], + ); + } + static Future?> session(String id) async { final db = await instance; final rows = await db.query( diff --git a/lib/data/local_repository.dart b/lib/data/local_repository.dart index 5a966dd..ef7bb5c 100644 --- a/lib/data/local_repository.dart +++ b/lib/data/local_repository.dart @@ -108,6 +108,19 @@ abstract class LocalRepository { throw UnimplementedError('re-layer: getWorkout'); Future deleteWorkout(String id) => throw UnimplementedError('re-layer: deleteWorkout'); + + /// Re-score recent finished sessions against the 1 Hz substrate now in the + /// DB, correcting a live session whose in-RAM tallies missed the part of the + /// workout the app slept through (issue #206). Returns the number of rows + /// whose strain changed. Best-effort — never throws. + /// + /// The default MUST match the implementation's: Dart resolves an omitted + /// optional from the STATIC receiver type, and every caller holds this + /// interface — so a different default here is the one that actually runs. + /// Three days is the raw-retention horizon; nothing older has substrate left + /// to re-score from. + Future rescoreRecentSessions({int sinceDays = 3}) => + throw UnimplementedError('re-layer: rescoreRecentSessions'); Future> startWorkout(String type, {String? title}) => throw UnimplementedError('re-layer: startWorkout'); Future> endWorkout(String workoutId) => diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index d3c5004..923eaf1 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -2016,9 +2016,14 @@ class LocalRepositoryImpl extends LocalRepository { @override Future> getWorkout(String id) async { - final r = await LocalDb.session(id); - if (r == null) return const {}; - final w = _workoutOf(r); + final stored = await LocalDb.session(id); + if (stored == null) return const {}; + // Reconcile the live tallies against the substrate BEFORE projecting the + // row (issue #206) — a session the app slept through stores a strain built + // from the few minutes it was awake for. Persists on improvement, so the + // list and the share card see the corrected value too. + final rescored = await _rescoreSessionFromSubstrate(stored); + final w = _workoutOf(rescored.row); final startTs = w['start_ts'] as int?; if (startTs == null) return w; final endTs = @@ -2030,7 +2035,10 @@ class LocalRepositoryImpl extends LocalRepository { // hr / avg_hr / min_hr / zone_bands / recovery_curve / hr_drift_pct / // time_to_peak_min; without a producer they were blank everywhere. try { - final hrRows = await LocalDb.hrSamplesInRange(startTs, endTs); + // Reuse the rows the rescore above already read for this exact window + // rather than scanning it a second time on every detail open. + final hrRows = rescored.hrRows ?? + await LocalDb.hrSamplesInRange(startTs, endTs); if (hrRows.isNotEmpty) { final ts = [for (final e in hrRows) (e['rec_ts'] as num).toInt()]; final hr = [for (final e in hrRows) (e['hr'] as num).toInt()]; @@ -2382,6 +2390,224 @@ class LocalRepositoryImpl extends LocalRepository { }; } + /// Re-score a finished session's strain/calories/max-HR/zone-minutes from the + /// 1 Hz substrate and persist the result when the substrate improves on what + /// the live accumulator managed to see (issue #206). + /// + /// The live tallies only cover the minutes the foreground app was awake for; + /// an app suspended or killed mid-workout stores a strain covering a fraction + /// of the window — often a few sub-resting minutes, which score a confident + /// `0.0`. Once the band offloads that window, the substrate holds the whole + /// thing. [reconcileSessionScore] documents why merging the two by `max` is + /// the correct rule; the short version is that both are lower bounds over + /// subsets of the same window's minutes. + /// + /// Self-healing by construction: it re-runs whenever the row is read or a + /// drain lands, and the merge is monotone, so a partially-drained window + /// improves on each pass and converges. Returns the row with the reconciled + /// values applied (never null-out a stored value), writing back only on a + /// real change. Best-effort — never throws into a read path. + Future<({Map row, List>? hrRows})> + _rescoreSessionFromSubstrate(Map row) async { + final id = row['id']; + final startTs = (row['start_ts'] as num?)?.toInt(); + final endTs = (row['end_ts'] as num?)?.toInt(); + // A live row is still accumulating; scoring it here would race the tally. + if (id is! String || + startTs == null || + endTs == null || + endTs <= startTs || + (row['status']?.toString() ?? '') != 'done') { + return (row: row, hrRows: null); + } + try { + // Returned to the caller: `getWorkout` enriches from the SAME 1 Hz window + // straight after this, and a two-hour session is ~7200 rows to scan twice. + final hrRows = await LocalDb.hrSamplesInRange(startTs, endTs); + if (hrRows.isEmpty) return (row: row, hrRows: hrRows); + + final profile = Profile.fromMap(getProfileMap()); + final hrBpm = [for (final e in hrRows) (e['hr'] as num).toInt()]; + final raw = computeManualSessionStats( + hrTs: [for (final e in hrRows) (e['rec_ts'] as num).toInt()], + hrBpm: hrBpm, + profile: profile, + zoneMaxHr: _profileMaxHr().toDouble(), + restingHr: + await _recentRestingHr() ?? profile.restingHrManual?.toDouble(), + ); + // `computeManualSessionStats` reports the raw 1 Hz peak. Persisting that + // writes a PPG spike into the column `getWorkout` deliberately refuses to + // floor against (issue #127) — and once raw ages out past retention the + // list has no smoothed value left to prefer, so the artefact would become + // permanent. Store the spike-suppressed peak instead. + final stats = ManualSessionStats( + avgHr: raw.avgHr, + maxHr: smoothedMaxHr(hrBpm, age: _profileAge()) ?? raw.maxHr, + strain: raw.strain, + calories: raw.calories, + zoneMinutes: raw.zoneMinutes, + hrSampleCount: raw.hrSampleCount, + ); + + // "Complete" = the band has handed over essentially the whole window. + // 1 Hz means one sample per second, so sample count vs window seconds is + // the coverage ratio; 90% absorbs the usual handful of dropped seconds. + final windowSec = endTs - startTs; + final complete = + windowSec > 0 && stats.hrSampleCount >= (windowSec * 0.9).floor(); + + final merged = reconcileSessionScore( + substrateIsComplete: complete, + liveStrain: (row['strain'] as num?)?.toDouble(), + liveCalories: (row['calories'] as num?)?.toDouble(), + liveMaxHr: (row['max_hr'] as num?)?.toInt(), + liveZoneMinutes: [ + for (final v in _decodeList(row['zone_min_json'])) + if (v is num) v.toDouble(), + ], + substrate: stats, + ); + if (!merged.changed) return (row: row, hrRows: hrRows); + + // `putSession` is INSERT-OR-REPLACE on the whole row, and everything + // above this point awaited (two substrate reads). A retime or a + // `stopWorkout` finalize landing in that window would be silently + // reverted — old start/end/status written back over the new ones. Re-read + // and bail if the row moved under us; the next sweep (or the next open) + // scores the new window. + final current = await LocalDb.session(id); + if (current == null || + (current['start_ts'] as num?)?.toInt() != startTs || + (current['end_ts'] as num?)?.toInt() != endTs || + current['status']?.toString() != row['status']?.toString()) { + // The row moved under us. Return the fresh row but NOT the rows we + // read — they describe the old window, and `getWorkout` would enrich + // the new one with them (a negative time-to-peak, zones over the wrong + // span). The next pass scores the new window. + return (row: current ?? row, hrRows: null); + } + + final zoneJson = jsonEncode( + merged.zoneMinutes.any((v) => v > 0) ? merged.zoneMinutes : const [], + ); + // Score columns ONLY, via a targeted UPDATE. `putSession` is + // INSERT-OR-REPLACE over the whole row, so it also rewrites columns this + // code never looked at — `hrr_bpm` (backfilled by the derive) and `type` + // (the user's own correction) are both written by narrow UPDATEs that + // the re-read above cannot detect. + await LocalDb.setSessionScores( + id, + strain: merged.strain, + calories: merged.calories, + maxHr: merged.maxHr, + zoneMinJson: zoneJson, + ); + final updated = { + ...current, + 'strain': merged.strain, + 'calories': merged.calories, + 'max_hr': merged.maxHr, + 'zone_min_json': zoneJson, + }; + return (row: updated, hrRows: hrRows); + } catch (_) { + return (row: row, hrRows: null); // best-effort: the stored row renders + } + } + + /// Sessions this process has already scored as FINISHED, keyed by id and the + /// window they had at the time (`id@endTs`). + /// + /// The skip cannot key on the window alone: a session that was still `live` + /// during an earlier sweep is skipped by [_rescoreSessionFromSubstrate] (its + /// tally is still accumulating), and once the frontier moved past its end a + /// window-only rule would skip it forever after it finished — leaving the + /// list showing the stale live-tally strain until someone opened it. Keying + /// on the window too means a retimed session is rescored rather than assumed + /// settled. + final Set _rescoredSessions = {}; + + /// Re-score recent finished sessions against the substrate now in the DB. + /// Called after a drain lands, so a workout whose window arrived late is + /// corrected on the LIST too, not only when its detail screen is opened. + /// Returns how many rows changed. + /// + /// Runs on the DB-owning (main) isolate by necessity — the sqflite handle is + /// not portable to another isolate — so it is bounded rather than offloaded: + /// the window is the raw-retention horizon (older windows are pruned and can + /// never improve) and anything already covered by a previous pass is skipped + /// outright, which leaves an ordinary drain doing no substrate reads at all. + + @override + Future rescoreRecentSessions({int sinceDays = 3}) async { + final nowSec = DateTime.now().millisecondsSinceEpoch ~/ 1000; + var changed = 0; + try { + // Local-midnight bound, not `now - n * 86400`: a DST day is 23 or 25 + // hours, so a flat day-length silently moves the window by an hour. + final fromTs = + localDayStartSec(dayLabelOf(DateTime.now().subtract( + Duration(days: sinceDays), + ))) ?? + (nowSec - sinceDays * 86400); + final rows = await LocalDb.sessionsInRange(fromTs, nowSec); + // Where the durable record frontier stands NOW. A finished session whose + // window sits behind it has all the substrate it is ever going to get. + // Falls back to the newest decoded row so an import-only install (no + // band, so no `rec_ts_hw` cursor) still gets the skip rather than + // re-scanning every session's window on every pass. + final frontier = await LocalDb.getCursorInt('rec_ts_hw') ?? + await LocalDb.lastDecodedRecTs() ?? + 0; + final seen = {}; + for (final r in rows) { + final id = r['id']?.toString(); + final endTs = (r['end_ts'] as num?)?.toInt(); + final key = (id == null || endTs == null) ? null : '$id@$endTs'; + if (key != null) seen.add(key); + // Settled: finished, fully covered, and already scored in that state. + if (key != null && + endTs! <= frontier && + _rescoredSessions.contains(key)) { + continue; + } + final after = await _rescoreSessionFromSubstrate(r); + // Count only what THIS pass wrote (the bail path returns a re-read row + // whose values may differ for reasons we had nothing to do with), and + // count ALL the scored columns — a pass that fixes calories or the zone + // split without moving strain still changed what the list shows. + const scored = ['strain', 'calories', 'max_hr', 'zone_min_json']; + if (after.hrRows != null && + scored.any((k) => '${after.row[k]}' != '${r[k]}')) { + changed++; + } + // Record it only once it is genuinely finished AND actually scored: a + // live row is skipped by the helper and must be revisited after it + // ends, and a row whose read threw (null rows) would otherwise be + // written off for the rest of the process on a transient DB error. + // The insertion condition MUST match the skip condition, `endTs <= + // frontier` included. Without it, a workout scored while the band had + // only handed over part of its window got stamped as settled, and the + // next drain — the one carrying the REST of that window — skipped it. + // The partial score then stood on the list until someone opened the + // detail screen, which is exactly the case the sweep exists for. + if (key != null && + endTs! <= frontier && + after.hrRows != null && + (r['status']?.toString() ?? '') == 'done') { + _rescoredSessions.add(key); + } + } + // Drop anything that aged out of the window so the set can't grow + // without bound across a long-lived process. + _rescoredSessions.retainWhere(seen.contains); + } catch (_) { + /* best-effort */ + } + return changed; + } + /// Most recent nightly resting HR from `metric_series`, or null. Bounded to /// the last week so a stale figure from a long gap can't anchor TRIMP. Future _recentRestingHr() async { diff --git a/lib/health/health_export.dart b/lib/health/health_export.dart index b14546d..4b22945 100644 --- a/lib/health/health_export.dart +++ b/lib/health/health_export.dart @@ -1024,34 +1024,11 @@ class HealthExporter { } } - HealthWorkoutActivityType _activity(String? type) { - switch ((type ?? '').toLowerCase()) { - case 'run': - case 'running': - return HealthWorkoutActivityType.RUNNING; - case 'cycle': - case 'cycling': - case 'bike': - case 'biking': - return HealthWorkoutActivityType.BIKING; - case 'walk': - case 'walking': - return HealthWorkoutActivityType.WALKING; - case 'swim': - case 'swimming': - return HealthWorkoutActivityType.SWIMMING; - case 'strength': - case 'weights': - case 'lifting': - return HealthWorkoutActivityType.STRENGTH_TRAINING; - case 'yoga': - return HealthWorkoutActivityType.YOGA; - case 'hiit': - return HealthWorkoutActivityType.HIGH_INTENSITY_INTERVAL_TRAINING; - default: - return HealthWorkoutActivityType.OTHER; - } - } + // `isApple`, not `Platform.isIOS`: every other platform decision in this file + // (the HRV type, the delete list, the store name) keys off the same getter, + // and a divergence here would hand macOS the Health Connect spellings. + HealthWorkoutActivityType _activity(String? type) => + healthActivityForType(type, ios: isApple); static Map? _decode(Object? json) { if (json is! String) return null; @@ -1083,3 +1060,65 @@ class HealthExporter { return DateTime(y, m, d); } } + +/// The app's workout-type key -> platform health activity type. +/// +/// Parameterised by [ios] rather than reading `Platform` directly so a unit +/// test can exercise BOTH platform branches on a host VM (where `Platform.isIOS` +/// and `Platform.isAndroid` are both false) — see +/// `test/workout_health_mapping_test.dart`. +/// +/// Why the platform branches exist at all: `health`'s `writeWorkoutData` rejects +/// (throws `HealthException`, before the platform channel) any activity type +/// absent from that platform's own supported set, and the two platforms spell +/// the strength and swim families differently: +/// +/// | app key | iOS | Android | +/// |------------|--------------------------------|------------------| +/// | `strength` | TRADITIONAL_STRENGTH_TRAINING | STRENGTH_TRAINING| +/// | `swim` | SWIMMING | SWIMMING_POOL | +/// +/// iOS has no bare `STRENGTH_TRAINING`; Android has neither `TRADITIONAL_`/ +/// `FUNCTIONAL_STRENGTH_TRAINING` nor bare `SWIMMING`. Using one spelling for +/// both platforms silently drops every workout of that type on the other one — +/// that is issue #184 (no strength workout ever reached Apple Health) and the +/// same latent bug existed for swims on Android. +/// +/// Anything unmapped falls back to `OTHER`, which both platforms accept, so an +/// unrecognised or autodetected type still lands in the health store. +@visibleForTesting +HealthWorkoutActivityType healthActivityForType( + String? type, { + required bool ios, +}) { + switch ((type ?? '').toLowerCase()) { + case 'run': + case 'running': + return HealthWorkoutActivityType.RUNNING; + case 'cycle': + case 'cycling': + case 'bike': + case 'biking': + return HealthWorkoutActivityType.BIKING; + case 'walk': + case 'walking': + return HealthWorkoutActivityType.WALKING; + case 'swim': + case 'swimming': + return ios + ? HealthWorkoutActivityType.SWIMMING + : HealthWorkoutActivityType.SWIMMING_POOL; + case 'strength': + case 'weights': + case 'lifting': + return ios + ? HealthWorkoutActivityType.TRADITIONAL_STRENGTH_TRAINING + : HealthWorkoutActivityType.STRENGTH_TRAINING; + case 'yoga': + return HealthWorkoutActivityType.YOGA; + case 'hiit': + return HealthWorkoutActivityType.HIGH_INTENSITY_INTERVAL_TRAINING; + default: + return HealthWorkoutActivityType.OTHER; + } +} diff --git a/lib/import/import_container.dart b/lib/import/import_container.dart new file mode 100644 index 0000000..c23f197 --- /dev/null +++ b/lib/import/import_container.dart @@ -0,0 +1,315 @@ +// import_container.dart — what did the user actually hand us? +// +// WHY THIS EXISTS (issues #199, #160) +// +// Every importer took the path from a `FileType.any` picker and piped it +// straight into `utf8.decoder`. Users picked the file their OTHER app told them +// to export — a WHOOP "My Data" export (a ZIP of CSVs) or a NOOP full backup +// (`.noopbak`, a ZIP holding `noop-backup.sqlite`) — and got: +// +// FormatException: Unexpected extension byte (at offset 10) +// FormatException: Invalid UTF-8 byte (at offset 10) +// +// Offset 10 is not a coincidence and it is not an encoding problem. A ZIP's +// first ten bytes (`PK\x03\x04`, version, flags, method) are all < 0x80, so the +// UTF-8 decoder always survives exactly that far and then hits byte 10 — the low +// byte of the DOS modification time, the first byte in the file that can have +// its high bit set. Byte 18 (the compressed size) is the next such field, which +// is where the other reported offset comes from. Both reports are ZIPs. +// +// (A genuinely mis-encoded CSV — latin1/cp1252, a Spanish or French export — +// fails differently: "Missing extension byte". None of the reports show that, +// so the "it's a localized CSV" theory does not explain them. Lenient decoding +// is still applied below, but as a separate, smaller fix.) +// +// So: sniff the container before decoding. A ZIP of CSVs is unwrapped and +// imported for real; anything we cannot use gets a message naming the file we +// DO want, instead of a byte offset. + +import 'dart:io'; + +import 'package:archive/archive.dart'; +import 'package:path/path.dart' as p; + +/// What the first bytes of the picked file say it is. +enum ImportContainer { + /// Plain text — decode and parse it. + text, + + /// PKZIP. A WHOOP export, or a `.noopbak`. + zip, + + /// A raw SQLite database (a `noop-backup.sqlite` extracted by hand, say). + sqlite, + + /// gzip — not a container we unwrap, but worth naming precisely. + gzip, + + /// Text, but UTF-16 rather than UTF-8 — re-savable by the user. + utf16, + + /// Binary of some other kind. + binary, +} + +/// An import that failed for a reason the user can act on. Distinct from a +/// `FormatException` so the UI can show guidance rather than a byte offset. +class ImportFormatException implements Exception { + const ImportFormatException(this.message); + final String message; + @override + String toString() => message; +} + +/// Classify a file from its leading bytes. Pure — [head] is the first ~16 bytes. +/// +/// Magic numbers: `PK\x03\x04` (and the empty/spanned variants `PK\x05\x06`, +/// `PK\x07\x08`) for ZIP; `SQLite format 3\x00` for SQLite; `\x1f\x8b` for gzip. +/// Everything else is called text unless it holds a NUL or a run of control +/// bytes, which no CSV export contains. +ImportContainer sniffImportContainer(List head) { + // UTF-16 (what Excel writes for "Unicode text") is full of NUL bytes and + // would otherwise be called binary — technically true, useless to the user. + if (head.length >= 2 && + ((head[0] == 0xFF && head[1] == 0xFE) || + (head[0] == 0xFE && head[1] == 0xFF))) { + return ImportContainer.utf16; + } + if (head.length >= 4 && + head[0] == 0x50 && + head[1] == 0x4B && + (head[2] == 0x03 || head[2] == 0x05 || head[2] == 0x07)) { + return ImportContainer.zip; + } + const sqliteMagic = 'SQLite format 3'; + if (head.length >= sqliteMagic.length && + String.fromCharCodes(head.take(sqliteMagic.length)) == sqliteMagic) { + return ImportContainer.sqlite; + } + if (head.length >= 2 && head[0] == 0x1F && head[1] == 0x8B) { + return ImportContainer.gzip; + } + for (final b in head) { + // NUL, or a control byte that is not tab/LF/CR — not a CSV. + if (b == 0x00 || (b < 0x09) || (b > 0x0D && b < 0x20)) { + return ImportContainer.binary; + } + } + return ImportContainer.text; +} + +/// Read enough of [path] to classify it. +Future sniffFile(String path) async { + final f = File(path); + final raf = await f.open(); + try { + return sniffImportContainer(await raf.read(16)); + } finally { + await raf.close(); + } +} + +/// True for a ZIP member we can actually parse as an export. +bool _isCsvMember(String name) { + final base = p.basename(name).toLowerCase(); + // `__MACOSX/._foo.csv` resource forks are AppleDouble binaries, not CSVs. + return base.endsWith('.csv') && + !base.startsWith('._') && + !name.startsWith('__MACOSX/'); +} + +/// Ceilings for archive extraction. A picked file is local and user-chosen, so +/// this is not a hostile-input boundary — but a malformed or pathological zip +/// should fail with a message rather than take the process out trying to +/// materialise it. The uncompressed ceiling is generous: a 90-day NOOP raw +/// export really is hundreds of megabytes. +const int _kMaxArchiveMembers = 5000; +const int _kMaxUncompressedBytes = 4 * 1024 * 1024 * 1024; // 4 GiB + +/// CSV files on disk for an import, plus the temp directory (if any) that has +/// to be cleaned up once they have been read. +class ResolvedImportFiles { + ResolvedImportFiles(this.paths, this._tempDir); + + final List paths; + final Directory? _tempDir; + + /// Delete anything extracted for this import. Safe to call more than once, + /// and never throws — a leftover temp file is not worth failing an import + /// that otherwise succeeded. + Future dispose() async { + final dir = _tempDir; + if (dir == null) return; + try { + if (dir.existsSync()) await dir.delete(recursive: true); + } catch (_) { + /* the OS reclaims the temp dir eventually */ + } + } +} + +/// Resolve the picked paths into CSV files on disk, unwrapping ZIP archives. +/// +/// [flavor] names the importer in error messages ('NOOP', 'WHOOP'). Extracted +/// members are written to a temp directory; the caller MUST `dispose()` the +/// result once it has finished reading them, or a large export leaves a full +/// second copy behind. Nothing is ever copied into app storage. +/// +/// Throws [ImportFormatException] with actionable guidance for anything we +/// cannot parse: a database, an archive of databases, a gzip, binary junk. +Future resolveImportCsvPaths( + List paths, { + required String flavor, +}) async { + final out = []; + Directory? tempDir; + var archiveIndex = 0; + try { + for (final path in paths) { + final kind = await sniffFile(path); + switch (kind) { + case ImportContainer.text: + out.add(path); + case ImportContainer.zip: + tempDir ??= + await Directory.systemTemp.createTemp('openstrap_import_'); + // One subdirectory per archive: the multi-select WHOOP path can hand + // us two exports that each contain `data.csv`, and a shared + // destination made the second overwrite the first (and returned the + // survivor's path twice). + final into = Directory(p.join(tempDir.path, 'a${archiveIndex++}')); + await into.create(recursive: true); + out.addAll( + await _extractCsvMembers(path, flavor: flavor, dir: into), + ); + case ImportContainer.sqlite: + throw ImportFormatException( + '“${p.basename(path)}” is a database file, not a $flavor CSV ' + 'export. In NOOP, use Export → raw sensor CSV and pick the ' + '“noop-raw-sensors-….csv” file it writes.', + ); + case ImportContainer.gzip: + throw ImportFormatException( + '“${p.basename(path)}” is a gzip archive. Unzip it first and pick ' + 'the CSV inside.', + ); + case ImportContainer.utf16: + throw ImportFormatException( + '“${p.basename(path)}” is saved as UTF-16 text. Re-save it as ' + 'UTF-8 CSV and import it again.', + ); + case ImportContainer.binary: + throw ImportFormatException( + '“${p.basename(path)}” is not a text file, so there is nothing to ' + 'read as a $flavor CSV export.', + ); + } + } + } catch (_) { + // A later file failing must not strand what an earlier ZIP already wrote. + await ResolvedImportFiles(const [], tempDir).dispose(); + rethrow; + } + return ResolvedImportFiles(out, tempDir); +} + +Future> _extractCsvMembers( + String path, { + required String flavor, + required Directory dir, +}) async { + final name = p.basename(path); + final Archive archive; + // STREAMED, not `decodeBytes(readAsBytes())`. A 90-day NOOP raw export is + // hundreds of megabytes; buffering the whole archive AND then each member in + // memory would OOM the phone on exactly the export this path exists to + // import — and the rest of the import pipeline is carefully streamed for the + // same reason. `InputFileStream` reads the archive off disk as it decodes. + final input = InputFileStream(path); + try { + archive = ZipDecoder().decodeStream(input); + } catch (e) { + await input.close(); + throw ImportFormatException( + 'Could not read “$name” as an archive: $e', + ); + } + + if (archive.files.length > _kMaxArchiveMembers) { + throw ImportFormatException( + '“$name” holds ${archive.files.length} entries, which is far more than ' + 'any $flavor export — refusing to unpack it.', + ); + } + + final csvFiles = [ + for (final f in archive.files) + if (f.isFile && _isCsvMember(f.name)) f, + ]; + + final declaredBytes = + csvFiles.fold(0, (sum, f) => sum + (f.size > 0 ? f.size : 0)); + if (declaredBytes > _kMaxUncompressedBytes) { + throw ImportFormatException( + '“$name” unpacks to more than ' + '${(declaredBytes / (1024 * 1024 * 1024)).toStringAsFixed(1)} GB of ' + 'CSV, which is not something we can import.', + ); + } + + if (csvFiles.isEmpty) { + // The `.noopbak` case, and the single most-reported one: an archive whose + // payload is a SQLite database. Name the file we actually want rather than + // failing on its bytes. + final hasDb = archive.files.any( + (f) => + f.isFile && + (f.name.toLowerCase().endsWith('.sqlite') || + f.name.toLowerCase().endsWith('.db')), + ); + if (hasDb) { + throw ImportFormatException( + '“$name” is a full NOOP backup — it holds NOOP\'s own database, which ' + 'we can\'t read. In NOOP, open Export and choose the raw 1 Hz sensor ' + 'CSV (“noop-raw-sensors-….csv”), then import that file here.', + ); + } + throw ImportFormatException( + '“$name” is an archive with no CSV files inside ' + '(${archive.files.length} entr${archive.files.length == 1 ? 'y' : 'ies'}). ' + 'Pick the $flavor CSV export instead.', + ); + } + + final out = []; + final used = {}; + try { + for (final f in csvFiles) { + // Members can share a basename (`daily/data.csv`, `workouts/data.csv`). + // Flattening them onto one destination silently dropped one file and + // parsed the survivor twice. + var base = p.basename(f.name); + if (!used.add(base)) { + final stem = p.basenameWithoutExtension(base); + final ext = p.extension(base); + var n = 2; + while (!used.add(base = '$stem-$n$ext')) { + n++; + } + } + final destPath = p.join(dir.path, base); + // `writeContent` decompresses straight to disk — never materialising the + // member, which for a raw sensor export is the big one. + final sink = OutputFileStream(destPath); + try { + f.writeContent(sink); + } finally { + await sink.close(); + } + out.add(destPath); + } + } finally { + await input.close(); + } + return out; +} diff --git a/lib/import/noop_import.dart b/lib/import/noop_import.dart index 627508e..1db618d 100644 --- a/lib/import/noop_import.dart +++ b/lib/import/noop_import.dart @@ -44,6 +44,12 @@ import '../compute/derivation_engine.dart'; import '../compute/profile.dart'; import '../compute/substrate.dart'; import '../data/db.dart'; +import 'import_container.dart'; + +/// Last path segment, for error messages (avoids a `package:path` import just +/// for this one use). +String _basenameOf(String path) => + path.contains('/') ? path.substring(path.lastIndexOf('/') + 1) : path; class NoopImportResult { final int days; @@ -114,11 +120,59 @@ class NoopImporter { DerivationEngine engine, { void Function(int days)? onProgress, }) async { - final file = File(path); + var file = File(path); if (!await file.exists()) { throw const FileSystemException('CSV not found'); } + // What did the user actually pick? A `.noopbak` is a ZIP around NOOP's + // SQLite database, and feeding its bytes to `utf8.decoder` is what produced + // the "Invalid UTF-8 byte (at offset 10)" in issues #160/#199. Resolve the + // container first: a ZIP of CSVs is unwrapped, and anything unusable throws + // an [ImportFormatException] naming the file we DO want. + final resolved = await resolveImportCsvPaths([path], flavor: 'NOOP'); + if (resolved.paths.isEmpty) { + await resolved.dispose(); + throw const ImportFormatException( + 'That archive holds no NOOP CSV export.', + ); + } + // A NOOP raw-sensor export is ONE CSV. An archive holding several is not + // something to guess at: this importer streams a single file and derives in + // a rolling two-day window, so silently taking the first match would import + // a fraction of the archive and still report success. + final candidates = resolved.paths + .where((p) => p.toLowerCase().contains('raw-sensor')) + .toList(); + final usable = candidates.isEmpty ? resolved.paths : candidates; + if (usable.length > 1) { + final names = usable.map(_basenameOf).take(4).join(', '); + await resolved.dispose(); + throw ImportFormatException( + 'That archive holds ${usable.length} CSV files ($names…). Import the ' + 'raw sensor CSV on its own so nothing is silently skipped.', + ); + } + file = File(usable.first); + + try { + return await _importResolvedFile( + file, + profile, + engine, + onProgress: onProgress, + ); + } finally { + // Anything unpacked from an archive is ours to clean up, success or not. + await resolved.dispose(); + } + } + static Future _importResolvedFile( + File file, + Profile profile, + DerivationEngine engine, { + void Function(int days)? onProgress, + }) async { // Rolling buffer: keeps at most the CURRENT + PREVIOUS local date of samples. final secs = {}; // ts(sec) → channels final rrTs = []; // beat end time (epoch ms) @@ -168,11 +222,21 @@ class NoopImporter { return (i != null && i < f.length) ? f[i] : ''; } - final lines = - file.openRead().transform(utf8.decoder).transform(const LineSplitter()); + // `allowMalformed` — a CSV exported under a non-UTF-8 locale should import + // with a mangled character in a column we don't read, not abort the whole + // file. (This is NOT what issues #160/#199 hit; those were ZIPs, handled + // above. It is the smaller, real second-order problem underneath them.) + final lines = file + .openRead() + .transform(const Utf8Decoder(allowMalformed: true)) + .transform(const LineSplitter()); + var sawHeader = false; + String? firstLine; await for (final line in lines) { if (line.isEmpty || line.startsWith('#')) continue; + firstLine ??= line; if (line.startsWith('unix_s,')) { + sawHeader = true; // Header → (re)build the name→index map and skip. final h = line.split(','); col = {for (var i = 0; i < h.length; i++) h[i].trim(): i}; @@ -269,6 +333,26 @@ class NoopImporter { stepsBanked += await _flushStepCoverage(e.value, e.key); } + // A file we could read but could not USE is a failure, not a "0 days" + // success. Without a recognised header the positional fallback silently + // misparses (it is the pre-drift layout), and a localized or unrelated CSV + // simply drops every row — both used to end at "NOOP: imported 0 days", + // which reads as "the app is broken" with nothing to act on. + if (totalRows == 0) { + final head = firstLine ?? ''; + final preview = head.isEmpty + ? '' + : ' (first line: "${head.length > 80 ? '${head.substring(0, 80)}…' : head}")'; + throw ImportFormatException( + sawHeader + ? 'That NOOP export has a header we recognise but no rows we could ' + 'read — every row was empty or out of range.' + : 'That file does not look like a NOOP raw-sensor export: no ' + '"unix_s,…" header row was found$preview. In NOOP, use ' + 'Export → raw sensor CSV.', + ); + } + await engine.finalizeImport(profile); return NoopImportResult(daysDone, totalRows, lateRows, stepsBanked); } diff --git a/lib/import/whoop_import.dart b/lib/import/whoop_import.dart index eb8288f..4e22236 100644 --- a/lib/import/whoop_import.dart +++ b/lib/import/whoop_import.dart @@ -18,6 +18,7 @@ import '../compute/derivation_engine.dart' show kAlgoVersion, DerivationEngine; import '../compute/profile.dart'; import '../compute/substrate.dart' show localDateLabel; import '../data/db.dart'; +import 'import_container.dart'; class WhoopImportResult { final int days; @@ -84,14 +85,60 @@ class WhoopImporter { } catch (_) { rawDays = const {}; } - for (final path in paths) { + // WHOOP's own "My Data" export arrives as a ZIP of CSVs, and users pick the + // ZIP — its bytes hit `utf8.decoder` and threw "Unexpected extension byte + // (at offset 10)" (issue #199). Unwrap it first; anything we can't parse + // throws an actionable [ImportFormatException] instead. + final resolved = await resolveImportCsvPaths(paths, flavor: 'WHOOP'); + try { + return await _importResolvedCsvs( + resolved.paths, + rawDays: rawDays, + engine: engine, + profile: profile, + onProgress: onProgress, + days: days, + workouts: workouts, + skipped: skipped, + ); + } finally { + // Anything unpacked from an archive is ours to clean up, success or not. + await resolved.dispose(); + } + } + + static Future _importResolvedCsvs( + List csvPaths, { + required Set rawDays, + DerivationEngine? engine, + Profile? profile, + void Function(int done)? onProgress, + required int days, + required int workouts, + required int skipped, + }) async { + var recognisedFiles = 0; + final headersSeen = []; + for (final path in csvPaths) { final rows = await _readCsv(path); - if (rows.length < 2) continue; + if (rows.isEmpty) continue; final header = rows.first; final col = { for (var i = 0; i < header.length; i++) header[i].trim().toLowerCase(): i }; final kind = _classify(col); + if (kind == _Kind.unknown) { + headersSeen.add(header.take(6).join(', ')); + continue; + } + // Count the file as recognised on its HEADER, before the empty check + // below: an export whose files carry the right columns but no rows (a + // week with no workouts, say) is a valid export we simply have nothing + // to import from. Skipping it first made it indistinguishable from a + // file we don't understand, and the caller then told the user to + // re-download in English. + recognisedFiles++; + if (rows.length < 2) continue; for (var r = 1; r < rows.length; r++) { final f = rows[r]; if (f.isEmpty) continue; @@ -112,6 +159,23 @@ class WhoopImporter { } } } + // Nothing recognised is a failure, not a "0 days" success. The columns are + // matched against exact ENGLISH header names, so a WHOOP export downloaded + // in another language classifies as unknown for every file and used to end + // silently at "WHOOP: imported 0 days" — reported as the app being broken. + if (recognisedFiles == 0) { + throw ImportFormatException( + csvPaths.isEmpty + ? 'No CSV files were found to import.' + : 'None of those files look like a WHOOP export. We match the ' + 'English column names WHOOP writes (e.g. "Recovery score %", ' + '"Activity name", "Sleep onset"), so an export downloaded in ' + 'another language will not be recognised — re-download it ' + 'with WHOOP set to English.' + '${headersSeen.isEmpty ? '' : ' Columns found: ${headersSeen.first}.'}', + ); + } + if (engine != null && profile != null) { await engine.finalizeImport(profile); } @@ -364,7 +428,9 @@ class WhoopImporter { static Future>> _readCsv(String path) async { final lines = File(path) .openRead() - .transform(utf8.decoder) + // Lenient: a WHOOP export saved under a non-UTF-8 locale should lose a + // character, not the whole import. + .transform(const Utf8Decoder(allowMalformed: true)) .transform(const LineSplitter()); final out = >[]; await for (final line in lines) { diff --git a/lib/state/app_state.dart b/lib/state/app_state.dart index f6d4103..d40609d 100644 --- a/lib/state/app_state.dart +++ b/lib/state/app_state.dart @@ -73,7 +73,12 @@ import '../sync/high_freq_wake_window.dart'; import '../sync/ios_bg_task.dart'; import '../sync/paired_device.dart'; import '../sync/sync_policy.dart' - show isLinkStale, StalenessTier, stalenessTierFor; + show + isLinkStale, + ReconnectSupervisorAction, + StalenessTier, + stalenessTierFor, + superviseReconnect; import '../sync/update_service.dart'; import '../telemetry/telemetry_service.dart'; import '../telemetry/health_uploader.dart'; @@ -173,6 +178,24 @@ class AppState extends ChangeNotifier { bool _keepAlive = false; bool _reconnecting = false; + + /// When the current reconnect ATTEMPT started, for the supervisor's + /// staleness check (issue #208). Per-attempt, not per-loop: a loop against a + /// band left at home legitimately runs for hours, so loop age says nothing + /// about whether anything is stuck — only an attempt that never returns does. + DateTime? _attemptStartedAt; + + /// Which reconnect loop is the live one. Bumped whenever a loop starts, so a + /// loop that was declared wedged and replaced can recognise itself as + /// superseded if it ever unblocks: without this its `finally` would clear the + /// REPLACEMENT's `_reconnecting`/`_attemptStartedAt`, and the supervisor + /// would then start a third loop while two are already connecting. + int _reconnectGeneration = 0; + + /// Level-triggered reconnect supervision. The loop's only trigger used to be + /// the `connected → disconnected` edge, so any abandoned loop was permanent. + /// This ticks regardless of edges and re-arms — see [superviseReconnect]. + Timer? _reconnectSupervisor; Timer? _backfillTimer; String _prevConn = 'disconnected'; // Last battery snapshot pushed to the Band Battery widget — so we only reload @@ -242,7 +265,10 @@ class AppState extends ChangeNotifier { // bucket is one platform call, so the 7-day backfill window is up to 168 of // them; only today can still change, and only yesterday if the app did not // run then. The full window runs on the explicit gestures instead. - if (phoneStepsEnabled) unawaited(syncPhoneSteps()); + if (phoneStepsEnabled) { + unawaited(syncPhoneSteps()); + unawaited(_refreshPhoneStepsToday()); + } // Best-effort, no prompt: learn the current health-permission state so the // Profile toggle reflects reality on open. if (healthSyncEnabled) unawaited(checkHealth()); @@ -449,8 +475,28 @@ class AppState extends ChangeNotifier { phoneStepsEnabled = ok; notifyListeners(); // The user just asked for this, so pull the full backfill window rather - // than the cheap routine one. - if (ok) unawaited(syncPhoneSteps(days: PhonePedometer.fullSyncDays)); + // than the cheap routine one — and then RE-DERIVE, symmetric with + // [disablePhoneSteps]. Banking rows into `live_coverage` changes nothing a + // screen can see: they all read scalars persisted in `day_result`/ + // `metric_series`, and the only automatic derive is drain-triggered. Grant + // the permission with the band not connected and, without this, the step + // tile keeps showing a dash indefinitely — indistinguishable from the + // feature not working. + if (ok) { + unawaited(() async { + await syncPhoneSteps(days: PhonePedometer.fullSyncDays); + // `_reanalyzeForOverride` no-ops while another derive is running, and + // the full sync above takes long enough (7 days of hourly platform + // reads) that a drain-triggered pass can easily have started. Dropping + // it silently leaves the freshly-banked rows out of `day_result` and + // the tile on a dash — the exact "looks broken" symptom this call was + // added to prevent. Wait for the other pass, bounded, then run. + for (var i = 0; i < 60 && reanalyzing; i++) { + await Future.delayed(const Duration(seconds: 1)); + } + await _reanalyzeForOverride(); + }()); + } return ok; } @@ -479,6 +525,8 @@ class AppState extends ChangeNotifier { phoneStepsEnabled = false; phoneStepsLastSyncedDays = null; phoneStepsLastTotal = null; + phoneStepsToday = 0; + _phoneStepsDay = null; try { await LocalDb.clearPhoneCoverage(); } catch (e) { @@ -497,6 +545,46 @@ class AppState extends ChangeNotifier { int? phoneStepsLastSyncedDays; int? phoneStepsLastTotal; + /// Steps the PHONE has banked for today, mirroring `liveStepsForDay`'s own + /// source rule (phone wins only when it actually has data). Screens add the + /// band's live count on top of the day total, and must not do that once the + /// phone owns the day — both count the same walk. Gating on + /// [phoneStepsEnabled] alone was wrong: with the toggle on and no phone data + /// (iOS read denied, nothing writing to Health Connect) the band still owns + /// the day and its live steps were being thrown away. + int phoneStepsToday = 0; + + /// Which local day [phoneStepsToday] was read for. The cache is worthless + /// past midnight, and a process here routinely lives for days (Android + /// foreground service, iOS suspend/resume), so a day-less cache would hold + /// yesterday's answer through the whole of today — suppressing the band's + /// live count on a day the phone has not contributed a single step to. + String? _phoneStepsDay; + + /// True when today's step total comes from the phone, so band live steps are + /// already accounted for and must not be added again. Unknown-or-stale reads + /// as false: showing the band's live count is the safe direction (a lost + /// count is invisible, a doubled one is a wrong number). + bool get todayStepsFromPhone => + phoneStepsEnabled && + _phoneStepsDay == todayLabel() && + phoneStepsToday > 0; + + Future _refreshPhoneStepsToday() async { + if (!phoneStepsEnabled) return; + try { + final day = todayLabel(); + final n = await LocalDb.phoneStepsForDay(day); + if (n != phoneStepsToday || day != _phoneStepsDay) { + phoneStepsToday = n; + _phoneStepsDay = day; + notifyListeners(); + } + } catch (_) { + /* best-effort — the gate just falls back to showing band live steps */ + } + } + /// Pull the last [days] days of phone step counts into `live_coverage`. /// /// Idempotent (delete-then-insert per day, scoped to the phone source), so @@ -509,6 +597,7 @@ class AppState extends ChangeNotifier { final r = await _phonePedometer.syncRecent(days: days); phoneStepsLastSyncedDays = r.daysRead; phoneStepsLastTotal = r.totalSteps; + await _refreshPhoneStepsToday(); notifyListeners(); return r.daysRead; } catch (e) { @@ -832,6 +921,12 @@ class AppState extends ChangeNotifier { // nothing new and keeps both signals consistent with each other. isForegroundActive: () => !_background, ); + // Seed the engine's link-power state (issue #200). `setBackground` is + // otherwise only called on TRANSITIONS, and a headless start begins + // backgrounded — without this the very case that most needs the cheap + // connection interval would run at the fast one until the user next + // foregrounded the app. + engine.setBackground(_background); repo = LocalRepositoryImpl(getProfileMap: () => user); // iOS BGProcessing/BGAppRefresh wakes while the FOREGROUND app owns the band // skip the headless BLE path (it would fight FBP for the peripheral) — route @@ -907,6 +1002,7 @@ class AppState extends ChangeNotifier { // ChangeNotifier (which throws in release). _tapSub?.cancel(); _stopBackfillTimer(); + _stopReconnectSupervisor(); _alarmGraceTimer?.cancel(); _alarmGraceTimer = null; _spotTimer?.cancel(); @@ -1018,6 +1114,23 @@ class AppState extends ChangeNotifier { }, )); TelemetryService.instance.breadcrumb('derive: $mode done'); + // A drain can bank band coverage and a day can have rolled over since the + // last read — both change which source owns today's steps. + unawaited(_refreshPhoneStepsToday()); + // The drain that triggered this pass may have landed the 1 Hz window of a + // workout the app slept through, whose strain/calories were scored from + // whatever few minutes the foreground tally saw (issue #206). Re-score + // recent sessions against the substrate now that it is here, so the + // workout LIST is corrected too and not just a detail screen someone + // happens to open. Monotone and idempotent — see reconcileSessionScore. + try { + final fixed = await repo?.rescoreRecentSessions() ?? 0; + if (fixed > 0) { + _log('[derive] rescored $fixed session(s) from substrate'); + } + } catch (e) { + _log('[derive] session rescore failed: $e'); + } await LocalDb.refreshComputeFreshness(); _bumpInsightsRevision(); notifyListeners(); // screens re-fetch from the derived store @@ -1592,6 +1705,7 @@ class AppState extends ChangeNotifier { if (isPaired) { if (_background) { _keepAlive = true; + _startReconnectSupervisor(); if (Platform.isAndroid) EdgeTracking.start(); if (Platform.isIOS) { IosBleRestore.foregroundActive = true; @@ -1784,6 +1898,9 @@ class AppState extends ChangeNotifier { /// On Android the Edge Tracking foreground service keeps the process + connection alive. Future pauseForBackground() async { _background = true; + // Step the Android link down to a power-saving connection interval — see + // `desiredLinkPriority` (issue #200). + engine.setBackground(true); // Defer derivation while backgrounded — running the heavy derive pass on a // short background BLE wake gets the app killed (iOS CPU watchdog / jetsam). // Capture keeps running; queued derive jobs drain on foreground return. @@ -1982,10 +2099,43 @@ class AppState extends ChangeNotifier { // the live-session screen shows steps FOR THIS WORKOUT (not since connection). int? _workoutRawBase; + /// Whether ANY gait-capable accel sample has reached us since the active + /// workout began, so [workoutStepsMeasured] can tell "did not move" apart + /// from "the band never sent anything to count". + /// + /// Deliberately a latch and NOT a comparison against `_liveSamples`: + /// `_resetLivePedometer()` zeroes that counter on every (re)connect, and it + /// runs mid-workout. A counter comparison therefore went permanently + /// "unmeasured" after the first reconnect — steps stuck on a dash for the + /// rest of the workout and `stopWorkout` banking none — which is the same + /// trap `_resetLivePedometer` already sidesteps for `_workoutRawBase` by + /// rebasing it negative rather than dropping it. + bool _workoutSawSamples = false; + /// Steps taken since the active workout started (real, live, gain-applied). - /// 0 when no workout is running. This is what the workout screen shows. - int get workoutSteps { - if (activeWorkout == null || _workoutRawBase == null) return 0; + /// 0 when no workout is running. + /// + /// Prefer [workoutStepsMeasured] in anything user-facing: this coerces an + /// unmeasured workout to 0, which is only safe because the two remaining + /// callers treat 0 as "omit" (the finish card hides the stat, `stopWorkout` + /// leaves the column unset). + int get workoutSteps => workoutStepsMeasured ?? 0; + + /// Steps for the active workout, or NULL when nothing gait-capable was ever + /// measured for it (issue #183). + /// + /// The live count needs the band's 100 Hz accel stream. That stream is + /// routinely absent even during a perfectly good workout: the sticky + /// standard-HR fallback suppresses it, the background downgrade turns it off, + /// and a pocketed phone can drop it entirely — while GPS distance and the + /// 1 Hz HR keep flowing. Reporting `0` in that state is a fabricated + /// measurement, and it is what the issue screenshotted: a mile walked, HR and + /// distance both right, "0 STEPS" beside them. + int? get workoutStepsMeasured { + if (activeWorkout == null || _workoutRawBase == null) return null; + // Nothing gait-capable has arrived for this workout — unmeasured, as + // opposed to zero steps having been measured. + if (!_workoutSawSamples) return null; final raw = _liveRaw - _workoutRawBase!; return raw > 0 ? (raw * ana.StepParams.gain).round() : 0; } @@ -1998,6 +2148,8 @@ class AppState extends ChangeNotifier { void _ingestLiveMagsAt(proto.ImuFrame f, int nowMs) { final mags = f.mags; if (mags.isEmpty) return; + // Survives `_resetLivePedometer()` — see [_workoutSawSamples]. + if (activeWorkout != null) _workoutSawSamples = true; // Append this frame's |a|(g) samples (gravity INCLUDED — AN-2554's dynamic // threshold rides the ~1 g baseline). `e` is this frame's 1 Hz-equivalent // ENMO (mean |a| − 1 g), read below by the stillness nudge and the posture @@ -2383,6 +2535,79 @@ class AppState extends ChangeNotifier { notifyListeners(); } + /// Cadence of the reconnect supervisor. Cheap — the tick reads local flags + /// and does nothing at all unless the app is paired, wants a link, and does + /// not have one. + static const Duration _reconnectSupervisorInterval = Duration(minutes: 1); + + /// Start the level-triggered reconnect supervision (issue #208). + /// + /// Deliberately NOT tied to connection state: it must keep ticking precisely + /// when everything else has given up. It is the backstop for the failure the + /// issue describes — a reconnect loop abandoned by a throw (or wedged on an + /// await that never returns), after which the app sits at 'disconnected' with + /// no edge left to re-trigger it and, on Android, a foreground service making + /// sure the process never restarts to clear the state. + void _startReconnectSupervisor() { + _reconnectSupervisor ??= Timer.periodic( + _reconnectSupervisorInterval, + (_) => _superviseReconnect(), + ); + } + + /// Stop supervising. Called from `dispose` and from every path that stops + /// wanting a link at all (unpair / endSession) — otherwise the tick outlives + /// its purpose and keeps poking the engine once a minute forever. + void _stopReconnectSupervisor() { + _reconnectSupervisor?.cancel(); + _reconnectSupervisor = null; + // Cancelling the timer is not enough: a `_reconnect()` can still be parked + // inside `waitForOsAutoConnect` for up to 15 minutes. Bumping the + // generation retires it — it exits at its next loop check and its `finally` + // leaves the flags alone. Without this, `endSession()` followed by a fresh + // `openSession()` lets that zombie wake up and become the live loop, + // reconnecting and re-running the whole post-connect block underneath the + // new session. + _reconnectGeneration++; + _reconnecting = false; + _attemptStartedAt = null; + engine.clearReconnecting(); + } + + void _superviseReconnect() { + if (_disposed) return; + // Expire a bond-refusal pause whose cooldown has run out before deciding — + // otherwise the supervisor faithfully observes a flag that nothing can ever + // clear (issue #208). + engine.refreshAutoReconnectPause(); + final action = superviseReconnect( + paired: paired != null, + keepAlive: _keepAlive, + connected: engine.isConnected, + loopRunning: _reconnecting, + autoReconnectPaused: device.autoReconnectPaused, + connectInFlight: busy, + attemptRunningFor: _attemptStartedAt == null + ? null + : DateTime.now().difference(_attemptStartedAt!), + ); + switch (action) { + case ReconnectSupervisorAction.none: + return; + case ReconnectSupervisorAction.start: + _log('[RECONNECT] supervisor: disconnected with no loop running — ' + 'starting one.'); + unawaited(_reconnect()); + case ReconnectSupervisorAction.restartStale: + _log('[RECONNECT] supervisor: the current attempt has been running ' + 'since $_attemptStartedAt with no link — treating it as wedged ' + 'and starting a fresh loop.'); + _reconnecting = false; + _attemptStartedAt = null; + unawaited(_reconnect()); + } + } + void _startBackfillTimer() { if (!_keepAlive || paired == null || !engine.isConnected) return; _backfillTimer ??= Timer.periodic(_backfillInterval, (_) { @@ -2398,6 +2623,29 @@ class AppState extends ChangeNotifier { Future _runPeriodicBackfill() async { if (!_keepAlive || paired == null || busy || _reconnecting) return; if (!engine.isConnected) return; + // BACKGROUND: leave periodic offloads to the engine's own timer, which is + // floored by `BackfillPolicy` (900 s + an empty-streak backoff). This timer + // runs every 10 minutes and drives `requestHistorySync()`, whose `manual` + // trigger is deliberately NEVER floored — so backgrounded, the two together + // meant a radio-waking offload round roughly every ten minutes all day and + // all night, bypassing the very rate limit written to prevent that (issue + // #200). Foreground keeps the faster cadence: the user can see the data. + if (_background) { + // The OFFLOAD is what we're skipping — the engine's own floored timer + // owns that. The wake-window re-plan is NOT the engine's: nothing else + // re-evaluates it on a stable connection, and it only flips on as the + // 90-minute pre-wake window opens. Skipping it outright meant a band + // that connected at 22:00 and stayed connected never armed high-frequency + // sync for that night at all. + try { + await _refreshHighFreqWakeWindow(); + } catch (e) { + _log('Wake-window refresh failed: $e'); + } + _log('Periodic history refresh skipped — backgrounded; the engine\'s ' + 'floored 15-min backfill owns the offload.'); + return; + } if (_syncBurst != null) { _log('Periodic history refresh skipped — a sync burst is already running.'); return; @@ -2604,6 +2852,7 @@ class AppState extends ChangeNotifier { _keepAlive = false; BandOwnership.markForegroundIntent(false); _stopBackfillTimer(); + _stopReconnectSupervisor(); IosBleRestore.foregroundActive = false; await EdgeTracking.stop(); await IosBleRestore.disarm(); @@ -2867,6 +3116,12 @@ class AppState extends ChangeNotifier { // background): don't tear it down and reconnect — just reclaim ownership. final wasBackground = _background; _background = false; + engine.setBackground(false); + // Coming back after hours (or days) suspended: re-read the phone's steps + // for whatever day it is NOW. + if (phoneStepsEnabled) { + unawaited(syncPhoneSteps()); + } // Back in the foreground with an OS CPU/memory budget again — let the // scheduler drain any derive jobs that queued (durably) while backgrounded. _deriveScheduler.setBackground(false); @@ -2913,6 +3168,9 @@ class AppState extends ChangeNotifier { _setBusy(true); lastError = null; _keepAlive = true; + // From here on we WANT a link for the life of the process, so the level- + // triggered supervisor runs from here on too (issue #208). + _startReconnectSupervisor(); try { // INSIDE the guard, and no `paired!`. This block used to sit BETWEEN // _setBusy(true) and the try, force-unwrapping `paired`. The resume path @@ -3017,6 +3275,8 @@ class AppState extends ChangeNotifier { return; } _reconnecting = true; + _attemptStartedAt = DateTime.now(); + final generation = ++_reconnectGeneration; BandOwnership.markForegroundIntent(true); _log('[OWNERSHIP] reconnect intent on (${BandOwnership.debugState})'); try { @@ -3026,12 +3286,26 @@ class AppState extends ChangeNotifier { // ReconnectPolicy. The engine's single in-flight guard guarantees this loop // can never overlap a foreground connect on the same band. int attempt = 0; - while (_keepAlive && !engine.isConnected && !device.autoReconnectPaused) { + while (_keepAlive && + !engine.isConnected && + !device.autoReconnectPaused && + generation == _reconnectGeneration) { attempt++; + _attemptStartedAt = DateTime.now(); // Surface `reconnecting` while the loop backs off, so the UI shows a // connecting-style state instead of flat 'disconnected'. engine.markReconnecting(); var connected = false; + // PER-ATTEMPT containment (issue #208). Everything below can throw — + // `_ensureForegroundLease`, `_claimBand`/teardown inside connect, the + // post-connect stream setup. This whole loop used to sit inside ONE + // try/catch, so a single throw abandoned it permanently: the engine + // settles on 'disconnected', and the `connected → disconnected` edge + // that is the loop's only trigger can never fire again. On Android the + // foreground service then keeps the process alive forever, so nothing + // ever cleared it — the band never reconnected until the user forgot + // and re-paired it. A failed attempt is now just a failed attempt. + try { // ANDROID OS-MANAGED FALLBACK: once direct attempts keep failing — or // while backgrounded, where the process can be frozen between our Dart // backoff timers — arm a flutter_blue_plus autoConnect pending connect @@ -3103,11 +3377,14 @@ class AppState extends ChangeNotifier { }), ); _startBackfillTimer(); - break; + break; + } + } catch (e) { + _log('Reconnect attempt $attempt failed: $e — retrying.'); } } } catch (e) { - _log('Reconnect failed: $e'); + _log('Reconnect loop aborted: $e'); } finally { // this used to only check !_keepAlive, but the while loop above can // ALSO exit because device.autoReconnectPaused flipped true mid-loop @@ -3115,15 +3392,25 @@ class AppState extends ChangeNotifier { // left foreground intent stuck on forever, which blocks every // headless background-sync entry point (BandOwnership.tryAcquireHeadless // gates on this being off). same bug shape as the foregroundActive fix. - if (!_keepAlive || device.autoReconnectPaused) { - BandOwnership.markForegroundIntent(false); - _log('[OWNERSHIP] reconnect intent off (${BandOwnership.debugState})'); + if (generation != _reconnectGeneration) { + // Superseded: the supervisor declared this loop wedged and started a + // replacement, which now owns the flags and the band claim. Clearing + // them here would clobber the live loop's state and let the supervisor + // start a third one. + _log('[RECONNECT] loop #$generation was superseded — leaving the ' + 'replacement\'s state alone.'); + } else { + if (!_keepAlive || device.autoReconnectPaused) { + BandOwnership.markForegroundIntent(false); + _log('[OWNERSHIP] reconnect intent off (${BandOwnership.debugState})'); + } + _reconnecting = false; + _attemptStartedAt = null; + // If we gave up (keepAlive dropped / never connected), stop advertising + // `reconnecting` — fall back to a truthful 'disconnected'. No-op when + // the loop exited via a successful connect (phase is `listening`). + engine.clearReconnecting(); } - _reconnecting = false; - // If we gave up (keepAlive dropped / never connected), stop advertising - // `reconnecting` — fall back to a truthful 'disconnected'. No-op when - // the loop exited via a successful connect (phase is `listening`). - engine.clearReconnecting(); } } @@ -3224,6 +3511,7 @@ class AppState extends ChangeNotifier { BandOwnership.markForegroundIntent(false); _log('[OWNERSHIP] endSession intent off (${BandOwnership.debugState})'); _stopBackfillTimer(); + _stopReconnectSupervisor(); await engine.disconnect(); _releaseForegroundLease(); } @@ -3597,6 +3885,7 @@ class AppState extends ChangeNotifier { unawaited(engine.retryFullLiveStreams()); } _workoutRawBase = _liveRaw; + _workoutSawSamples = false; // A first night may have been derived since init. This read finishes // after the session below is constructed, so it back-fills the anchor on // `activeWorkout` when it lands rather than blocking the start. @@ -3803,6 +4092,7 @@ class AppState extends ChangeNotifier { // snapshot: steps count from zero going forward, same as // calories/strain/zone-minutes already (honestly) do here. _workoutRawBase = _liveRaw; + _workoutSawSamples = false; // A first night may have been derived since init. This read finishes // after the session below is constructed, so it back-fills the anchor on // `activeWorkout` when it lands rather than blocking the start. @@ -3860,7 +4150,9 @@ class AppState extends ChangeNotifier { _deriveScheduler.setWorkoutActive(false); final w = activeWorkout!; final finalKcal = w.calories.round(); - final wSteps = workoutSteps; // real steps taken during this workout + // Nullable: an unmeasured workout must leave the column unset rather than + // bank a zero that reads as "you took no steps". + final wSteps = workoutStepsMeasured; // Persist the finalized session before clearing the live state. zone_min = // the per-zone seconds the 1 Hz tick accumulated (Z1..Z5, minutes). final id = w.workoutId ?? 'w${w.startTime.millisecondsSinceEpoch}'; @@ -3879,7 +4171,7 @@ class AppState extends ChangeNotifier { 'zone_min_json': jsonEncode( zoneMin.any((v) => v > 0) ? zoneMin : const [], ), - if (wSteps > 0) 'steps': wSteps, + if (wSteps != null && wSteps > 0) 'steps': wSteps, 'source': 'manual', 'created_at': w.startTime.millisecondsSinceEpoch, }; @@ -3893,6 +4185,7 @@ class AppState extends ChangeNotifier { } activeWorkout = null; _workoutRawBase = null; + _workoutSawSamples = false; notifyListeners(); _log('Live session ended. Burned $finalKcal kcal.'); LiveActivity.end(); @@ -3925,6 +4218,7 @@ class AppState extends ChangeNotifier { _deriveScheduler.setWorkoutActive(false); activeWorkout = null; _workoutRawBase = null; + _workoutSawSamples = false; LiveActivity.end(); } diff --git a/lib/sync/sync_policy.dart b/lib/sync/sync_policy.dart index abda54a..b4eb7d0 100644 --- a/lib/sync/sync_policy.dart +++ b/lib/sync/sync_policy.dart @@ -464,20 +464,56 @@ class PostBondTimeoutLoopDetector { /// A single successful bond resets the streak. class BondRefusalGiveUp { final int giveUpThreshold; - BondRefusalGiveUp({this.giveUpThreshold = 5}); + + /// How long the auto-reconnect pause lasts before the loop is allowed to try + /// again (issue #208). + /// + /// The pause used to be permanent in practice: it is cleared ONLY inside the + /// `createBond()` success branch, which lives inside the connect path, which + /// the pause itself prevents from ever running again. A user who hit five + /// refusals — a transient stack/OEM condition, not necessarily a broken bond — + /// had auto-reconnect off for the life of the process, with the Android + /// foreground service making sure the process never restarted to clear it. + /// A cooldown keeps the intended behaviour (stop hammering a band that will + /// not bond) without the dead end. + final Duration cooldown; + + BondRefusalGiveUp({ + this.giveUpThreshold = 5, + this.cooldown = const Duration(minutes: 30), + }); int _consecutive = 0; bool gaveUp = false; + DateTime? _gaveUpAt; int get consecutive => _consecutive; + /// When the pause was entered, or null if not paused. + DateTime? get gaveUpAt => _gaveUpAt; + + /// Whether the auto-reconnect pause is still in force at [now]. Once the + /// cooldown expires the latch clears itself and the streak restarts, so a + /// band that still refuses simply re-trips after another [giveUpThreshold] + /// refusals rather than being retried forever. + bool stillPaused(DateTime now) { + if (!gaveUp) return false; + final since = _gaveUpAt; + if (since != null && now.difference(since) >= cooldown) { + reset(); + return false; + } + return true; + } + /// Feed a bond refusal/timeout. Returns true EXACTLY ONCE, on the call that /// crosses the threshold (the caller then pauses reconnect + surfaces the guide). - bool bondRefused() { + bool bondRefused({DateTime? now}) { if (gaveUp) return false; _consecutive++; if (_consecutive >= giveUpThreshold) { gaveUp = true; + _gaveUpAt = now ?? DateTime.now(); return true; } return false; @@ -485,14 +521,12 @@ class BondRefusalGiveUp { /// A bond that succeeded (or a session that got past bonding). Clears the /// streak AND the give-up latch so a later refusal run can trip again. - void bondSucceeded() { - _consecutive = 0; - gaveUp = false; - } + void bondSucceeded() => reset(); void reset() { _consecutive = 0; gaveUp = false; + _gaveUpAt = null; } } @@ -679,3 +713,129 @@ class NoDurableProgressEscalation { _gaveUp = false; } } + +// ── link power policy (issue #200) ─────────────────────────────────────────── + +/// How aggressively to run the Android GATT link right now. +/// +/// Android exposes three connection-interval presets. `high` is ~11.25 ms with +/// zero slave latency — the phone's controller services ~89 connection events a +/// second, and the host is woken for every one that carries data. +enum LinkPriority { high, balanced, lowPower } + +/// Battery poll cadence. The band's battery is a DISPLAY value that moves on the +/// order of hours; nothing in the sync path depends on it. It was being read on +/// every 30 s keep-alive tick — 2,880 radio round-trips a day for a number that +/// changes a few times. +const int kBatteryPollIntervalSeconds = 300; + +/// The connection priority the link should be running at. +/// +/// WHY THIS EXISTS (issue #200): the engine requested `high` once, at connect +/// setup, "for the drain" — and never stepped back down. Since the connection is +/// deliberately permanent (foreground service + START_STICKY + keep-alive +/// watchdog + CDM presence relaunch), that meant an ~11.25 ms interval held 24/7, +/// including all night with nothing to say. The app also steers users into a +/// battery-optimization exemption, so Doze never damps it either. That +/// configuration — not the timers the reporter suspected — is the dominant +/// drain. +/// +/// The rule: pay for a fast interval only while something is actually consuming +/// the link. +/// • an offload in flight → `high` (throughput is the whole point), +/// • a live consumer in the foreground (workout, spot check, breathing) → +/// `high`; the 100 Hz streams need the bandwidth, +/// • foreground, idle → `balanced`, +/// • background with no live consumer → `lowPower`. +/// +/// SAFETY: this changes throughput, never correctness. The drain is +/// commit-before-ACK and resumes from a durable cursor, so a slower interval can +/// only make an offload take longer — and an offload always raises the priority +/// back to `high` first. The one real constraint is the liveness fuse +/// ([kLivenessFuseSeconds]): whatever interval we sit at must still let the 1 Hz +/// notify or the keep-alive response arrive inside 120 s, which `lowPower` +/// (~500 ms interval) does with three orders of magnitude to spare. +LinkPriority desiredLinkPriority({ + required bool offloadActive, + required bool background, + required bool hasLiveConsumer, +}) { + if (offloadActive) return LinkPriority.high; + if (hasLiveConsumer && !background) return LinkPriority.high; + return background ? LinkPriority.lowPower : LinkPriority.balanced; +} + +// ── reconnect supervision (issue #208) ─────────────────────────────────────── + +/// Why the supervisor decided to act (for logging — a silent self-heal that +/// nobody can see in a log is how this class of bug hides). +enum ReconnectSupervisorAction { + /// Nothing to do: connected, unpaired, paused, or a loop is already running. + none, + + /// No loop is running and we are not connected — start one. + start, + + /// A loop has been "running" far too long with nothing to show for it; the + /// in-flight flag is stale (an await that never returned). Clear it and start + /// a fresh loop. + restartStale, +} + +/// Level-triggered reconnect supervision. +/// +/// WHY THIS EXISTS: `_reconnect()` was EDGE-triggered — the only thing that +/// called it was the `connected → disconnected` transition. The loop itself is +/// unbounded and correct, but it sits inside one try/catch, so ANY throw inside +/// it (a foreground-lease acquisition, a claim/teardown error, a stream setup +/// failure) abandoned the loop for good. After that the engine sits at +/// 'disconnected', so the edge can never fire again, and on Android the +/// foreground service guarantees the process never restarts to clear it. That +/// is the reported "shows reconnecting, then disconnected, forever, until you +/// forget the band and re-pair". +/// +/// An await that never returns produces the same dead end with the in-flight +/// flag stuck true, which no amount of re-triggering fixes — hence +/// [ReconnectSupervisorAction.restartStale]. +/// +/// This is the pure decision. The caller runs a timer, feeds it observations, +/// and acts on the verdict. +ReconnectSupervisorAction superviseReconnect({ + required bool paired, + required bool keepAlive, + required bool connected, + required bool loopRunning, + required bool autoReconnectPaused, + + /// Time since the CURRENT attempt started — not since the loop did. A loop + /// legitimately runs for hours against a band left at home; an individual + /// attempt does not. + required Duration? attemptRunningFor, + + /// Something else is already driving a connect (a user-initiated + /// `openSession`). Starting a second loop underneath it makes two callers + /// race the same peripheral and re-run the whole post-connect block. + bool connectInFlight = false, + Duration staleAfter = const Duration(minutes: 25), +}) { + if (!paired || + !keepAlive || + connected || + autoReconnectPaused || + connectInFlight) { + return ReconnectSupervisorAction.none; + } + if (!loopRunning) return ReconnectSupervisorAction.start; + // MUST stay comfortably above the longest legitimate single attempt. The + // Android OS-autoConnect branch waits up to 15 minutes per pass, so a + // threshold measured from the LOOP's start (rather than the attempt's) and + // set at 20 minutes fired mid-way through a perfectly healthy second pass — + // tearing down a live loop and, worse, letting the abandoned attempt's + // eventual `disconnect()` cancel the OS pending connect the replacement was + // waiting on. That turned the supervisor into a cause of the very + // never-reconnects symptom it exists to cure. + if (attemptRunningFor != null && attemptRunningFor >= staleAfter) { + return ReconnectSupervisorAction.restartStale; + } + return ReconnectSupervisorAction.none; +} diff --git a/lib/ui/activity/live_session_screen.dart b/lib/ui/activity/live_session_screen.dart index d160cfc..6765c3d 100644 --- a/lib/ui/activity/live_session_screen.dart +++ b/lib/ui/activity/live_session_screen.dart @@ -310,7 +310,7 @@ class _LiveSessionScreenState extends State peakHr: w?.maxHrSeen ?? 0, calories: w?.calories ?? 0, strain: w?.strain, - steps: app.workoutSteps, + steps: app.workoutStepsMeasured, ); // AWAIT: stopWorkout flushes the GPS route tail; navigating before it // completes raced the finish screen's route load (missing tail / no map). @@ -707,7 +707,9 @@ class WorkoutFinishSnapshot { /// nullable all the way to the finish card: a `?? 0` here would print a /// confident "0.0" for a session that was simply never scored. final double? strain; - final int steps; + /// Null when nothing gait-capable was measured for the workout — the finish + /// card omits the stat rather than showing a zero. + final int? steps; const WorkoutFinishSnapshot({ required this.type, required this.duration, @@ -841,9 +843,14 @@ class _WorkoutFinishScreenState extends State strain > 0 && (strain - tw.value).abs() < 0.15; final ms = recs.record('most_steps'); + // Prefer the PERSISTED count, like the build path does — the snapshot + // can be empty for a workout whose row already carries real steps. + final steps = (d['steps'] as num?)?.toInt() ?? s.steps; + // An unmeasured workout can't set a step record. _prSteps = ms != null && - s.steps > 0 && - (s.steps - ms.value).abs() < 1.5; + steps != null && + steps > 0 && + (steps - ms.value).abs() < 1.5; } }); } catch (_) {} @@ -1066,7 +1073,7 @@ class _WorkoutFinishScreenState extends State /// These figures COUNT UP with the reveal, so unlike the other sections they /// legitimately rebuild per frame — but it is a handful of Text widgets, not /// a map or a route re-derivation. - Widget _heroStats(int peak, int? avg, int kcal, int steps) { + Widget _heroStats(int peak, int? avg, int kcal, int? steps) { Widget stat(String v, String label) => Expanded(child: _FinishStat(v, label)); return AnimatedBuilder( @@ -1082,7 +1089,8 @@ class _WorkoutFinishScreenState extends State stat(peak > 0 ? '${(peak * p).round()}' : '—', 'PEAK BPM'), stat(avg != null ? '${(avg * p).round()}' : '—', 'AVG BPM'), stat('${(kcal * p).round()}', 'KCAL'), - if (steps > 0) stat('${(steps * p).round()}', 'STEPS'), + if (steps != null && steps > 0) + stat('${(steps * p).round()}', 'STEPS'), ], ), ), @@ -1388,7 +1396,7 @@ class _WorkoutFinishScreenState extends State duration: s.duration, when: DateTime.now(), maxHr: _maxHr, - strain: (d?['strain'] as num?)?.toDouble() ?? s.strain ?? 0, + strain: (d?['strain'] as num?)?.toDouble() ?? s.strain, calories: (d?['calories'] as num?)?.toInt() ?? s.calories.round(), route: _route, avgHr: (d?['avg_hr'] as num?)?.toInt(), @@ -2449,7 +2457,11 @@ class _SessionSheet extends StatelessWidget { final zone = zoneIndex.clamp(0, 5); final zoneColor = AppColors.zoneOnDark(zone); final isRoute = distance != null; - final steps = context.select((a) => a.workoutSteps); + // Nullable: the band's 100 Hz accel stream is routinely absent during a + // perfectly good workout (standard-HR fallback, background downgrade), and + // printing a confident "0 STEPS" next to a real distance and a real HR is a + // fabricated measurement — issue #183 screenshotted exactly that. + final steps = context.select((a) => a.workoutStepsMeasured); return Container( decoration: BoxDecoration( @@ -2511,7 +2523,9 @@ class _SessionSheet extends StatelessWidget { ), Expanded( child: _SheetStat( - isRoute ? '${workout.calories.round()}' : '$steps', + isRoute + ? '${workout.calories.round()}' + : (steps?.toString() ?? '—'), isRoute ? 'KCAL' : 'STEPS', ), ), diff --git a/lib/ui/activity/workout_share_card.dart b/lib/ui/activity/workout_share_card.dart index 9980748..7bcfbe0 100644 --- a/lib/ui/activity/workout_share_card.dart +++ b/lib/ui/activity/workout_share_card.dart @@ -547,12 +547,19 @@ WorkoutShareData buildWorkoutShareData({ required Duration duration, required DateTime when, required int maxHr, - required double strain, + + /// Null when the session was never scored (a profile anchor the Banister + /// formula needs is missing, or no HR was ever captured for the window). + /// Nullable all the way to the card: a `?? 0` at the call site prints a + /// confident "0.0 Strain" for a workout we simply could not score, which is + /// the same fabrication issue #206 reported on the detail gauge. + required double? strain, required int calories, WorkoutRoute? route, int? avgHr, }) { final hasRoute = route != null && route.hasPath; + final strainText = strain?.toStringAsFixed(1) ?? '—'; final title = type.isEmpty ? 'Workout' : type[0].toUpperCase() + type.substring(1); @@ -568,13 +575,13 @@ WorkoutShareData buildWorkoutShareData({ (_shareDuration(duration), 'Time'), // Moving pace, like everywhere else — see the note in _GpsControlPanel. (units.pace(route.distanceMeters, route.movingSec), 'Pace'), - (strain.toStringAsFixed(1), 'Strain'), + (strainText, 'Strain'), ]; } else { heroValue = _shareDuration(duration); heroUnit = ''; stats = [ - (strain.toStringAsFixed(1), 'Strain'), + (strainText, 'Strain'), ('$calories', 'Kcal'), (avgHr != null && avgHr > 0 ? '$avgHr' : '—', 'Avg bpm'), ]; diff --git a/lib/ui/screens/screens.dart b/lib/ui/screens/screens.dart index fd2481f..61c208f 100644 --- a/lib/ui/screens/screens.dart +++ b/lib/ui/screens/screens.dart @@ -277,9 +277,15 @@ class _ActivityDetailState extends State<_ActivityDetail> { @override Widget build(BuildContext context) { - // Live steps from the in-flight session count toward TODAY only. + // Live steps from the in-flight session count toward TODAY only — and only + // when the BAND is the day's step source. With phone steps on, the day + // total is already the phone's count of the same walk (`liveStepsForDay` + // prefers phone rows outright rather than summing), so adding the wrist's + // live count would double-count it. final live = _isToday - ? context.select((a) => a.liveSteps) + ? context.select( + (a) => a.todayStepsFromPhone ? 0 : a.liveSteps, + ) : 0; // Was context.watch() — rebuilt this whole board on every one of // AppState's 67 notifyListeners() sources. Only `user` (for step_goal) is diff --git a/lib/ui/today/today_screen.dart b/lib/ui/today/today_screen.dart index 3ddd18b..255f86a 100644 --- a/lib/ui/today/today_screen.dart +++ b/lib/ui/today/today_screen.dart @@ -186,8 +186,16 @@ class _TodayScreenState extends State // the REBUILD TRIGGER is scoped. liveSteps is included (already rate- // limited to ~1/s at the source) so the steps tile doesn't freeze mid-walk // while waiting on an unrelated dbCounts change. - context.select, bool, String, int)>( - (a) => (a.dbCounts, a.reanalyzing, a.reanalyzeProgress, a.liveSteps), + context.select, bool, String, int, bool)>( + (a) => ( + a.dbCounts, + a.reanalyzing, + a.reanalyzeProgress, + a.liveSteps, + // The steps tile stops adding the band's live count the moment the + // phone owns the day, so a flip has to rebuild it. + a.todayStepsFromPhone, + ), ); final app = context.read(); final t = TodayData.fromJson(data); @@ -349,7 +357,18 @@ class _TodayScreenState extends State t: t, sparks: _sparks, stepsWeek: _stepsWeek, - liveSteps: context.read().liveSteps, + // Band-derived live steps are an addend to the day metric ONLY + // while the band is the day's step source. `liveStepsForDay` + // deliberately lets phone rows WIN OUTRIGHT over band rows rather + // than summing them (both count the same walk — one from the + // pocket, one from the wrist), so adding the wrist's live count on + // top of a phone-sourced day total re-introduces exactly the + // double count that rule exists to prevent. `todayStepsFromPhone` + // mirrors the DB's own rule, so a day the phone did not actually + // cover still shows the band's live count. + liveSteps: context.read().todayStepsFromPhone + ? 0 + : context.read().liveSteps, onOpen: _open, hasAiBriefing: hasAiBriefing, aiBriefing: hasAiBriefing ? BriefingStore.read(period) : null, @@ -1137,7 +1156,16 @@ class TodayVitals extends StatelessWidget { crossAxisAlignment: CrossAxisAlignment.start, mainAxisSize: MainAxisSize.min, children: [ - const TileHeader('Steps', trailing: Tag('est')), + // 'measured', not 'est' — nothing estimates steps any more. The 1 Hz + // estimator was removed (a per-day gravity reference dominated by the + // sleep block put its SNR at ~1); what is left is a real pedometer, + // the phone's or the band's 100 Hz stream, and a day with neither + // shows no number rather than a guess. The detail screen was updated + // to say so and this tile was missed. + // Same tag, same colour as the steps detail screen — `Tag`'s default + // is the warning amber, which reads as a caution rather than a + // statement of confidence. + TileHeader('Steps', trailing: Tag('measured', color: DomainAccent.steps)), const SizedBox(height: Sp.x2), BigStat( value: steps > 0 ? '$steps' : null, diff --git a/lib/ui/workouts/workouts_screen.dart b/lib/ui/workouts/workouts_screen.dart index 304a807..b5ca14b 100644 --- a/lib/ui/workouts/workouts_screen.dart +++ b/lib/ui/workouts/workouts_screen.dart @@ -418,8 +418,9 @@ class _WorkoutsScreenState extends State { duration: Duration(minutes: (w['duration_min'] as num?)?.toInt() ?? 0), peakHr: (w['max_hr'] as num?)?.toInt() ?? 0, calories: ((w['calories'] as num?) ?? 0).toDouble(), - strain: ((w['strain'] as num?) ?? 0).toDouble(), - steps: (w['steps'] as num?)?.toInt() ?? 0, + strain: (w['strain'] as num?)?.toDouble(), + // Nullable: an unmeasured workout is not a zero-step one. + steps: (w['steps'] as num?)?.toInt(), ); Navigator.of(context).push( themedRoute( @@ -1213,7 +1214,7 @@ class _WorkoutDetailBodyState extends State<_WorkoutDetailBody> { ? DateTime.fromMillisecondsSinceEpoch(startTs * 1000).toLocal() : DateTime.now(), maxHr: context.read().maxHr, - strain: (d['strain'] as num?)?.toDouble() ?? 0, + strain: (d['strain'] as num?)?.toDouble(), calories: (d['calories'] as num?)?.toInt() ?? 0, route: _route, avgHr: (d['avg_hr'] as num?)?.toInt(), diff --git a/pubspec.lock b/pubspec.lock index 78feaf7..a8d19ef 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -34,7 +34,7 @@ packages: source: hosted version: "5.3.1" archive: - dependency: transitive + dependency: "direct main" description: name: archive sha256: a96e8b390886ee8abb49b7bd3ac8df6f451c621619f52a26e815fdcf568959ff diff --git a/pubspec.yaml b/pubspec.yaml index 2eccdf5..3df2b9a 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -89,6 +89,12 @@ dependencies: path: ^1.9.0 path_provider: ^2.1.5 + # Unwrapping the archives users actually pick on the import screen: a WHOOP + # "My Data" export is a ZIP of CSVs, and a NOOP `.noopbak` is a ZIP holding a + # SQLite database (issues #199, #160). Already present transitively; declared + # directly because `lib/import/import_container.dart` imports it. + archive: ^4.0.9 + # Background scheduled derivation (the heavy nightly pass: sleep staging + # 24-h spectra). Android: a real OS-scheduled WorkManager job. iOS: see the # honest caveat in lib/compute/background_derivation.dart — app-killed heavy diff --git a/test/import_container_test.dart b/test/import_container_test.dart new file mode 100644 index 0000000..f730fd9 Binary files /dev/null and b/test/import_container_test.dart differ diff --git a/test/link_priority_policy_test.dart b/test/link_priority_policy_test.dart new file mode 100644 index 0000000..d3275b8 --- /dev/null +++ b/test/link_priority_policy_test.dart @@ -0,0 +1,144 @@ +// Issue #200: ~22% of an S22's battery in a day with 15 minutes of screen-on. +// +// The reporter blamed the 10 s heartbeat and the 15-minute backfill. The actual +// dominant cost was that Android's connection priority was requested once, at +// connect setup, and never stepped back down — an ~11.25 ms interval with zero +// slave latency, held 24/7 on a connection that is permanent by design, with a +// battery-optimization exemption ensuring Doze never damps it. +// +// These pin the stepping rule. The load-bearing property is the LAST test: an +// offload always runs at the fast interval, whatever else is going on, because +// throughput during a drain is what the fast interval was for. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/ble/ble_engine.dart'; +import 'package:openstrap_edge/sync/sync_policy.dart'; + +void main() { + test('an idle backgrounded link runs at the cheap interval', () { + expect( + desiredLinkPriority( + offloadActive: false, + background: true, + hasLiveConsumer: false, + ), + LinkPriority.lowPower, + reason: 'the overnight state, and where the drain was being spent', + ); + }); + + test('idle in the foreground is balanced, not high', () { + expect( + desiredLinkPriority( + offloadActive: false, + background: false, + hasLiveConsumer: false, + ), + LinkPriority.balanced, + ); + }); + + test('a foreground live consumer keeps the fast interval', () { + // A workout / spot check / breathing session streams 100 Hz; that genuinely + // needs the bandwidth. + expect( + desiredLinkPriority( + offloadActive: false, + background: false, + hasLiveConsumer: true, + ), + LinkPriority.high, + ); + }); + + test('a live consumer in the BACKGROUND does not hold the link high', () { + // Backgrounded, the engine downgrades to the compact 1 Hz stream, so the + // bandwidth argument no longer applies. + expect( + desiredLinkPriority( + offloadActive: false, + background: true, + hasLiveConsumer: true, + ), + LinkPriority.lowPower, + ); + }); + + test('an offload ALWAYS gets the fast interval', () { + // The invariant that keeps this change throughput-only: whatever else is + // true, a drain in flight raises the link first. Sync correctness never + // depends on the interval (commit-before-ACK, durable cursor), but making a + // drain crawl would be a real regression, so this is exhaustive. + for (final background in [false, true]) { + for (final live in [false, true]) { + expect( + desiredLinkPriority( + offloadActive: true, + background: background, + hasLiveConsumer: live, + ), + LinkPriority.high, + reason: 'background=$background live=$live', + ); + } + } + }); + + test('the battery poll is minutes apart, not seconds', () { + // It rode the 30 s keep-alive tick: 2,880 radio round-trips a day for a + // display value that changes a handful of times. + expect(kBatteryPollIntervalSeconds, greaterThanOrEqualTo(300)); + // Still far inside the liveness fuse, so it can never be the thing that + // starves `sinceLastRx` and bounces a healthy link. + expect(kBatteryPollIntervalSeconds, greaterThan(kLivenessFuseSeconds)); + }); + + group('the engine feeds its own state into that rule', () { + // The policy tests above prove the RULE. These prove the WIRING, which is + // where the bug actually was: the connect-setup boost used to be a direct + // radio call that bypassed the serialized path entirely. + // + // Honest limit: `_doConnect` cannot run on the test host (flutter_blue_plus + // is unsupported there), so what is covered is the flag's effect on the + // target and `sendInit`'s clearing of it — not the assignment inside + // `_doConnect` itself. + late BleEngine engine; + + setUp(() { + TestWidgetsFlutterBinding.ensureInitialized(); + engine = BleEngine( + onRecord: (sample, raw) async {}, + onState: (_) {}, + log: (_) {}, + ); + }); + + test('a backgrounded idle engine wants the cheap interval', () { + engine.setBackground(true); + expect(engine.linkPriorityForCurrentState(), LinkPriority.lowPower); + }); + + test('connect setup outranks being backgrounded', () { + engine.setBackground(true); + engine.debugBeginConnectSetup(); + expect( + engine.linkPriorityForCurrentState(), + LinkPriority.high, + reason: 'setup is immediately followed by the first flash drain', + ); + }); + + test('sendInit ends the setup boost', () async { + engine.setBackground(true); + engine.debugBeginConnectSetup(); + // No session on the host, so the writes fail — the point is that the + // flag is cleared in a `finally`, not only on the happy path. + await engine.sendInit(); + expect( + engine.linkPriorityForCurrentState(), + LinkPriority.lowPower, + reason: 'an idle background link must stop paying for setup speed', + ); + }); + }); +} diff --git a/test/reconnect_supervisor_test.dart b/test/reconnect_supervisor_test.dart new file mode 100644 index 0000000..c77f49b --- /dev/null +++ b/test/reconnect_supervisor_test.dart @@ -0,0 +1,159 @@ +// Issue #208: a band taken off for ten minutes, or carried out of range, never +// reconnects. The app shows "reconnecting", falls back to "disconnected", and +// stays there until the user forgets the band and re-pairs it. +// +// Two independent dead ends produced that, and both are terminal-by-design +// rather than flaky: +// +// 1. `_reconnect()` was EDGE-triggered — its only caller is the +// `connected → disconnected` transition — and the whole retry loop sat +// inside one try/catch. Any throw inside the loop abandoned it for good, +// after which the engine rests at 'disconnected' so the edge can never +// fire again. On Android the foreground service guarantees the process +// never restarts to clear it. +// 2. The bond-refusal pause was cleared ONLY inside the `createBond()` +// success branch, which lives inside the connect path that the pause +// prevents from running. Self-sealing. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/sync/sync_policy.dart'; + +void main() { + group('superviseReconnect', () { + ReconnectSupervisorAction call({ + bool paired = true, + bool keepAlive = true, + bool connected = false, + bool loopRunning = false, + bool paused = false, + bool connectInFlight = false, + Duration? runningFor, + }) => superviseReconnect( + paired: paired, + keepAlive: keepAlive, + connected: connected, + loopRunning: loopRunning, + autoReconnectPaused: paused, + connectInFlight: connectInFlight, + attemptRunningFor: runningFor, + ); + + test('disconnected with no loop running restarts the loop', () { + // The state the app was stuck in — nothing else would ever fire. + expect(call(), ReconnectSupervisorAction.start); + }); + + test('does nothing while connected', () { + expect(call(connected: true), ReconnectSupervisorAction.none); + }); + + test('does nothing when unpaired or when we do not want a link', () { + expect(call(paired: false), ReconnectSupervisorAction.none); + expect(call(keepAlive: false), ReconnectSupervisorAction.none); + }); + + test('never fights a healthy attempt', () { + // The Android OS-autoConnect branch waits up to 15 minutes for the band + // to reappear. That is one NORMAL attempt, and restarting it is actively + // harmful: the abandoned attempt's eventual disconnect() cancels the OS + // pending connect its replacement is waiting on. + expect( + call(loopRunning: true, runningFor: const Duration(minutes: 15)), + ReconnectSupervisorAction.none, + ); + expect( + call(loopRunning: true, runningFor: const Duration(minutes: 24)), + ReconnectSupervisorAction.none, + ); + expect( + call(loopRunning: true, runningFor: null), + ReconnectSupervisorAction.none, + ); + }); + + test('a loop running for hours is fine while its attempts turn over', () { + // A band left at home keeps the loop alive indefinitely; only an + // individual attempt that never returns is evidence of a wedge. + expect( + call(loopRunning: true, runningFor: const Duration(minutes: 3)), + ReconnectSupervisorAction.none, + ); + }); + + test('restarts an attempt wedged well past the autoConnect window', () { + // An await that never returns (a leaked band lease, a hung platform call) + // leaves the in-flight flag true forever; re-triggering cannot fix that, + // so the flag has to be treated as stale. + expect( + call(loopRunning: true, runningFor: const Duration(minutes: 25)), + ReconnectSupervisorAction.restartStale, + ); + }); + + test('stays out of the way of a user-initiated connect', () { + // `openSession` starts the supervisor before doing its own connect, and a + // first Android connect (bond dialog, discovery, INIT) can outlast a + // 60 s tick. Starting a loop underneath it makes two callers race the + // same peripheral and re-run the whole post-connect block. + expect(call(connectInFlight: true), ReconnectSupervisorAction.none); + }); + + test('respects an active bond-refusal pause', () { + // Not a dead end any more (see below), but while it IS in force the + // supervisor must not hammer a band that refuses to bond. + expect(call(paused: true), ReconnectSupervisorAction.none); + expect( + call(paused: true, loopRunning: true, runningFor: const Duration(hours: 1)), + ReconnectSupervisorAction.none, + ); + }); + }); + + group('BondRefusalGiveUp cooldown', () { + test('trips exactly once at the threshold, then pauses', () { + final g = BondRefusalGiveUp(giveUpThreshold: 3); + final t0 = DateTime(2026, 8, 8, 12); + expect(g.bondRefused(now: t0), isFalse); + expect(g.bondRefused(now: t0), isFalse); + expect(g.bondRefused(now: t0), isTrue, reason: 'crosses the threshold'); + expect(g.bondRefused(now: t0), isFalse, reason: 'only ever once'); + expect(g.stillPaused(t0), isTrue); + }); + + test('the pause expires after its cooldown', () { + final g = BondRefusalGiveUp( + giveUpThreshold: 1, + cooldown: const Duration(minutes: 30), + ); + final t0 = DateTime(2026, 8, 8, 12); + expect(g.bondRefused(now: t0), isTrue); + expect(g.stillPaused(t0.add(const Duration(minutes: 29))), isTrue); + expect( + g.stillPaused(t0.add(const Duration(minutes: 30))), + isFalse, + reason: 'previously nothing could ever clear this', + ); + // Expiry resets the streak, so a band that still refuses re-trips + // normally instead of being retried forever. + expect(g.consecutive, 0); + expect(g.gaveUp, isFalse); + expect(g.bondRefused(now: t0.add(const Duration(minutes: 31))), isTrue); + }); + + test('a successful bond clears the streak and the latch', () { + final g = BondRefusalGiveUp(giveUpThreshold: 2); + final t0 = DateTime(2026, 8, 8, 12); + g.bondRefused(now: t0); + g.bondRefused(now: t0); + expect(g.stillPaused(t0), isTrue); + g.bondSucceeded(); + expect(g.stillPaused(t0), isFalse); + expect(g.gaveUpAt, isNull); + }); + + test('an unpaused tracker is never reported as paused', () { + final g = BondRefusalGiveUp(); + expect(g.stillPaused(DateTime(2026, 8, 8)), isFalse); + }); + }); +} diff --git a/test/session_score_reconcile_test.dart b/test/session_score_reconcile_test.dart new file mode 100644 index 0000000..e839854 --- /dev/null +++ b/test/session_score_reconcile_test.dart @@ -0,0 +1,216 @@ +// Issue #206: a live session's strain is accumulated in RAM by the foreground +// app. Backgrounded (iOS suspends the 1 Hz timer) or killed mid-workout, that +// accumulator misses most of the workout — commonly leaving a handful of +// sub-resting minutes whose Banister TRIMP is exactly 0, which `strainScore` +// reports as a confident 0.0 next to a real duration and real HR. +// +// `reconcileSessionScore` merges that partial tally with a re-score of the same +// window from the 1 Hz substrate the band banked. These tests pin the merge +// rule (max, because both sides are lower bounds over subsets of one window's +// minutes) and the properties that make repeated application safe. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/compute/manual_session.dart'; + +ManualSessionStats _substrate({ + double? strain, + double? calories, + int? maxHr, + List zone = const [], + int samples = 1800, +}) => ManualSessionStats( + strain: strain, + calories: calories, + maxHr: maxHr, + zoneMinutes: zone, + hrSampleCount: samples, +); + +void main() { + test('a 0.0 live tally is replaced by the substrate score', () { + final r = reconcileSessionScore( + liveStrain: 0.0, // the app was awake only for sub-resting minutes + liveCalories: 3.0, + liveMaxHr: 71, + liveZoneMinutes: const [2, 0, 0, 0, 0], + substrate: _substrate( + strain: 11.4, + calories: 480, + maxHr: 168, + zone: const [4, 12, 20, 9, 1], + ), + ); + expect(r.strain, 11.4); + expect(r.calories, 480); + expect(r.maxHr, 168); + expect(r.zoneMinutes, const [4, 12, 20, 9, 1]); + expect(r.changed, isTrue); + }); + + test('an empty substrate leaves the live tally untouched', () { + // Right after a workout the band has not offloaded the window yet. The live + // tally is all the evidence there is — it must not be wiped to null. + final r = reconcileSessionScore( + liveStrain: 8.2, + liveCalories: 300, + liveMaxHr: 160, + liveZoneMinutes: const [1, 2, 3, 0, 0], + substrate: _substrate(samples: 0), + ); + expect(r.strain, 8.2); + expect(r.calories, 300); + expect(r.maxHr, 160); + expect(r.zoneMinutes, const [1, 2, 3, 0, 0]); + expect(r.changed, isFalse, reason: 'nothing improved — no write'); + }); + + test('a partially drained window never LOWERS a better live tally', () { + // The band has offloaded only the first few minutes so far. + final r = reconcileSessionScore( + liveStrain: 9.0, + liveCalories: 400, + liveMaxHr: 171, + liveZoneMinutes: const [1, 5, 10, 4, 0], + substrate: _substrate( + strain: 2.1, + calories: 90, + maxHr: 140, + zone: const [1, 2, 0, 0, 0], + samples: 300, + ), + ); + expect(r.strain, 9.0); + expect(r.calories, 400); + expect(r.maxHr, 171); + expect(r.zoneMinutes, const [1, 5, 10, 4, 0]); + expect(r.changed, isFalse); + }); + + test('absent stays absent — an unscored session never becomes 0.0', () { + final r = reconcileSessionScore( + liveStrain: null, // no profile anchor, so nothing was ever scored + liveCalories: null, + liveMaxHr: null, + liveZoneMinutes: const [], + substrate: _substrate(strain: null, calories: null, maxHr: 150), + ); + expect(r.strain, isNull); + expect(r.calories, isNull); + expect(r.maxHr, 150, reason: 'max HR is measurable without a profile'); + }); + + test('a null live strain is filled from the substrate', () { + final r = reconcileSessionScore( + liveStrain: null, + liveCalories: null, + liveMaxHr: null, + liveZoneMinutes: const [], + substrate: _substrate(strain: 6.5, calories: 210, maxHr: 155), + ); + expect(r.strain, 6.5); + expect(r.calories, 210); + expect(r.changed, isTrue); + }); + + test('a genuinely zero-load window stays 0.0 rather than being hidden', () { + // Sitting still for 30 minutes and calling it a workout IS zero strain. + // The fix must not turn every real zero into an absence. + final r = reconcileSessionScore( + liveStrain: 0.0, + liveCalories: 0.0, + liveMaxHr: 68, + liveZoneMinutes: const [], + substrate: _substrate(strain: 0.0, calories: 0.0, maxHr: 68), + ); + expect(r.strain, 0.0); + expect(r.changed, isFalse); + }); + + test('repeated application converges — the merge is monotone', () { + // Each pass sees more of the drained window; the value only ever rises and + // re-running on an already-merged row is a no-op. + var strain = 0.0; + for (final partial in [1.0, 4.4, 7.9, 11.2, 11.2]) { + final r = reconcileSessionScore( + liveStrain: strain, + liveCalories: null, + liveMaxHr: null, + liveZoneMinutes: const [], + substrate: _substrate(strain: partial), + ); + expect(r.strain! >= strain, isTrue, reason: 'never regresses'); + strain = r.strain!; + } + expect(strain, 11.2); + + final again = reconcileSessionScore( + liveStrain: strain, + liveCalories: null, + liveMaxHr: null, + liveZoneMinutes: const [], + substrate: _substrate(strain: 11.2), + ); + expect(again.changed, isFalse, reason: 'converged — stops writing'); + }); + + test('a complete substrate REPLACES the tally rather than maxing it', () { + // The max rule is only monotone while the scoring function is fixed, and it + // is not — strain depends on the trailing nightly resting HR, which moves. + // Maxing forever would ratchet a session up to the highest value any RHR + // the profile ever reported would have produced, with no way back down. + final r = reconcileSessionScore( + liveStrain: 14.0, // scored earlier against a lower resting HR + liveCalories: 700, + liveMaxHr: 190, + liveZoneMinutes: const [0, 0, 30, 0, 0], + substrate: _substrate( + strain: 11.4, + calories: 480, + maxHr: 168, + zone: const [4, 12, 20, 9, 1], + ), + substrateIsComplete: true, + ); + expect(r.strain, 11.4, reason: 'current anchors, not the historic peak'); + expect(r.calories, 480); + expect(r.maxHr, 168); + expect(r.zoneMinutes, const [4, 12, 20, 9, 1]); + expect(r.changed, isTrue); + }); + + test('completeness does not invent values the substrate lacks', () { + final r = reconcileSessionScore( + liveStrain: 9.0, + liveCalories: 250, + liveMaxHr: 170, + liveZoneMinutes: const [1, 2, 0, 0, 0], + // Zone minutes need a HRmax the profile may not carry, so an empty + // vector alongside complete coverage is a real case — and must not wipe + // the split that was already stored. + substrate: _substrate( + strain: null, + calories: null, + maxHr: null, + zone: const [], + ), + substrateIsComplete: true, + ); + expect(r.strain, 9.0, reason: 'no profile anchor ⇒ nothing to replace with'); + expect(r.calories, 250); + expect(r.maxHr, 170); + expect(r.zoneMinutes, const [1, 2, 0, 0, 0]); + }); + + test('zone minutes come from one source, never element-wise mixed', () { + // A per-element max would invent a total neither source observed. + final r = reconcileSessionScore( + liveStrain: null, + liveCalories: null, + liveMaxHr: null, + liveZoneMinutes: const [10, 0, 0, 0, 0], // 10 min total + substrate: _substrate(zone: const [0, 3, 9, 2, 0]), // 14 min total + ); + expect(r.zoneMinutes, const [0, 3, 9, 2, 0]); + expect(r.zoneMinutes.fold(0, (a, b) => a + b), 14); + }); +} diff --git a/test/workout_health_mapping_test.dart b/test/workout_health_mapping_test.dart new file mode 100644 index 0000000..0a2cf66 --- /dev/null +++ b/test/workout_health_mapping_test.dart @@ -0,0 +1,135 @@ +// Every workout type the app can start MUST map to an activity type the target +// platform's health store actually accepts. +// +// Issue #184: `strength` mapped to `HealthWorkoutActivityType.STRENGTH_TRAINING` +// on BOTH platforms. That value exists only in the plugin's Android set, so on +// iOS `writeWorkoutData` threw `HealthException` *before* the platform channel, +// the throw was swallowed by a `debugPrint`, and no strength workout ever +// reached Apple Health. The same latent bug existed for `swim`, which mapped to +// bare `SWIMMING` — an iOS-only value — and so was dropped on Android. +// +// The supported sets below are transcribed from `health: 11.1.1` +// (`lib/src/health_plugin.dart`, `_isOnIOS` / `_isOnAndroid`), restricted to the +// values `healthActivityForType` can actually emit. They are a PIN, not a +// mirror: on a `health` upgrade, re-check those two functions and update these +// sets deliberately. If a value silently leaves a platform's set upstream, this +// test is what catches it before another workout family goes missing for a +// release. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:health/health.dart'; +import 'package:openstrap_edge/health/health_export.dart'; +import 'package:openstrap_edge/ui/workouts/workout_types.dart'; + +/// Values `healthActivityForType` may emit that iOS (HealthKit) accepts. +const _iosSupported = { + HealthWorkoutActivityType.RUNNING, + HealthWorkoutActivityType.BIKING, + HealthWorkoutActivityType.WALKING, + HealthWorkoutActivityType.SWIMMING, + HealthWorkoutActivityType.TRADITIONAL_STRENGTH_TRAINING, + HealthWorkoutActivityType.YOGA, + HealthWorkoutActivityType.HIGH_INTENSITY_INTERVAL_TRAINING, + HealthWorkoutActivityType.OTHER, +}; + +/// Values `healthActivityForType` may emit that Android (Health Connect) accepts. +const _androidSupported = { + HealthWorkoutActivityType.RUNNING, + HealthWorkoutActivityType.BIKING, + HealthWorkoutActivityType.WALKING, + HealthWorkoutActivityType.SWIMMING_POOL, + HealthWorkoutActivityType.STRENGTH_TRAINING, + HealthWorkoutActivityType.YOGA, + HealthWorkoutActivityType.HIGH_INTENSITY_INTERVAL_TRAINING, + HealthWorkoutActivityType.OTHER, +}; + +/// Type strings that can reach the exporter but are not in [kWorkoutTypes]: +/// manual-start aliases and the auto-detector's own vocabulary. +const _extraTypeStrings = [ + 'running', + 'cycling', + 'bike', + 'biking', + 'walking', + 'swimming', + 'weights', + 'lifting', + 'autodetected', + 'autodetected_workout', + 'workout', + '', +]; + +void main() { + final allTypes = [ + ...kWorkoutTypes.map((e) => e.$1), + ..._extraTypeStrings, + null, + ]; + + group('healthActivityForType stays inside each platform supported set', () { + for (final type in allTypes) { + test('"${type ?? ''}" is writable on both platforms', () { + expect( + _iosSupported, + contains(healthActivityForType(type, ios: true)), + reason: + 'iOS would throw HealthException for "$type" and the workout ' + 'would never reach Apple Health (issue #184)', + ); + expect( + _androidSupported, + contains(healthActivityForType(type, ios: false)), + reason: + 'Health Connect would throw HealthException for "$type" and the ' + 'workout would never reach Android health', + ); + }); + } + }); + + test('strength maps to the platform-correct strength spelling', () { + expect( + healthActivityForType('strength', ios: true), + HealthWorkoutActivityType.TRADITIONAL_STRENGTH_TRAINING, + ); + expect( + healthActivityForType('strength', ios: false), + HealthWorkoutActivityType.STRENGTH_TRAINING, + ); + // The aliases the manual-start UI and older rows can carry. + for (final alias in ['weights', 'lifting', 'Strength', 'STRENGTH']) { + expect( + healthActivityForType(alias, ios: true), + HealthWorkoutActivityType.TRADITIONAL_STRENGTH_TRAINING, + reason: '"$alias" must land on the same iOS type as "strength"', + ); + } + }); + + test('swim maps to the platform-correct swim spelling', () { + expect( + healthActivityForType('swim', ios: true), + HealthWorkoutActivityType.SWIMMING, + ); + expect( + healthActivityForType('swim', ios: false), + HealthWorkoutActivityType.SWIMMING_POOL, + ); + }); + + test('an unknown type degrades to OTHER rather than an unwritable value', () { + for (final unknown in ['surfing', 'padel', 'autodetected', null]) { + expect( + healthActivityForType(unknown, ios: true), + HealthWorkoutActivityType.OTHER, + ); + expect( + healthActivityForType(unknown, ios: false), + HealthWorkoutActivityType.OTHER, + ); + } + }); +}