fix: step calibration counts 0 on Android — sticky standard-HR fallback suppressed the IMU stream - #103
Conversation
Once the marginal-radio or frame-corruption detector tripped, the standard-HR fallback flag was never reset and enableLiveStreams honoured it silently — so every later calibration walk and workout counted 0 steps for the rest of the process lifetime (the R10/IMU toggles were never sent). Android trips the detectors far more readily than iOS, which is why the 0-step calibration reports were Android-only. - BleEngine.retryFullLiveStreams(): clears the fallback + resets both detectors, then re-arms the full live set. If the radio genuinely can't sustain the flood the detectors re-trip within seconds. - startStepCalibration/startWorkout call it: an explicit foreground user action whose feature needs the 100 Hz stream is exactly the moment to retry (spot check + breathing only need 0x28, which the fallback still delivers, so they're unchanged). - Calibration screen: if the fallback re-trips mid-walk, show a 'Bluetooth can't keep up' card with a retry action instead of 'Keep walking…' over a count that will never move.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an explicit full live-stream retry API, invokes it during step calibration and workout startup after degraded streaming, and displays a retry action when calibration detects Bluetooth radio degradation. ChangesFull live stream retry
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant StepCalibrationScreen
participant AppState
participant BleEngine
participant LiveStreams
User->>StepCalibrationScreen: Select “Retry stream”
StepCalibrationScreen->>AppState: Restart calibration
AppState->>BleEngine: retryFullLiveStreams()
BleEngine->>BleEngine: Clear fallback and reset detectors
BleEngine->>LiveStreams: enableLiveStreams()
LiveStreams-->>AppState: Re-enable full live stream set
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/state/app_state.dart`:
- Around line 2926-2932: Update the workout-start retry condition around
engine.retryFullLiveStreams() to run for connected devices when
!engine.liveEnabled, engine.liveHrOnly, or device.standardHrFallback is true.
Preserve the existing unawaited retry behavior so every workout explicitly
re-arms the full live stream set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 12ed35e0-b7a7-471c-8ba6-b768a484f76a
📒 Files selected for processing (3)
lib/ble/ble_engine.dartlib/state/app_state.dartlib/ui/today/step_calibration_screen.dart
|
Tested this on the affected Pixel with a side-by-side debug build of this branch — calibration now counts properly on a device where it previously sat at 0 (the fallback had tripped and was being cleared on walk start, as designed). |
Symptom
Android users (myself included) run the step-calibration walk and the gauge never moves off 0 of 250, no matter how far they walk. iOS users don't hit it. The same starvation affects the workout screen's live step count.
Root cause — traced on a real device
I debugged this on a Pixel with a field bug report. At the exact moment calibration started, the app logged:
That's the early-out at the top of
enableLiveStreams: whenstate.standardHrFallbackis set, it arms realtime HR (0x28) only and silently skips the R10/R11 + IMU (0x33) toggles. The calibration pedometer counts exclusively over those high-rate accel frames, so it has literally nothing to count — while the screen keeps saying "Keep walking…".The flag is set by two radio-health detectors (
MarginalRadioDetector: 2 consecutive arm→quick-timeout cycles;FrameCorruptionDetector: ≥20% CRC failures over a 50-frame window). The design flaw is the combination of three properties:standardHrFallbackshort of killing the app process.enableLiveStreamscallers can't tell they got the degraded set.So one rough patch of radio early in a session permanently kills step calibration and workout step counts until the next app restart. The fallback itself is doing its job — protecting a struggling link from a flood it can't sustain — but it's over-scoped: it also vetoes a deliberate, foreground, user-initiated action where the flood is the entire point.
The fix
BleEngine.retryFullLiveStreams()— clearsstandardHrFallback, resets both detectors, and re-arms the full live set. If the radio genuinely can't sustain it, the detectors re-trip (and re-downgrade) within seconds — the protection isn't weakened, it just gets re-evaluated when the user explicitly asks for the stream.startStepCalibrationandstartWorkoutuse it — these are the two features that need the 100 Hz accel. Spot check and breathing only consume 0x28 RR frames, which the fallback still delivers, so they're deliberately unchanged.Testing
Verified the mechanism end-to-end on-device via logs (fallback line firing at calibration start on v0.9.15, Pixel). I couldn't run the app from this machine, so the changes are intentionally minimal and pattern-matched to the surrounding code: both detectors already had
reset(), the engine already mutates+broadcastsstatethe same way on the trip path, and the screen reuses the existingStateCard. Would appreciate a maintainer smoke-test of: trip the fallback (walk out of range mid-live twice), then start a calibration — pre-patch it counts 0, post-patch it should re-arm and count.Happy to split the UI card into a separate PR if you'd rather keep the engine change alone.
Summary by CodeRabbit
New Features
Bug Fixes