activity: invalidate raw HR metric when the HRM reports off-wrist - #1960
Open
Dreamkeeper wants to merge 1 commit into
Open
activity: invalidate raw HR metric when the HRM reports off-wrist#1960Dreamkeeper wants to merge 1 commit into
Dreamkeeper wants to merge 1 commit into
Conversation
Off-wrist BPM events never update hr.metrics.current_bpm - the storage behind ActivityMetricHeartRateRawBPM / HealthMetricHeartRateRawBPM - so health_service_peek_current_value() keeps serving the last on-wrist reading indefinitely. The health event payload already reports current_bpm 0 for off-wrist, but apps cannot read event payloads; peeking the metric is their only path. Measured on a Pebble Time 2: after taking the watch off, apps peeking HealthMetricHeartRateRawBPM on each HeartRateUpdate event saw the last on-wrist value (bit-identical, e.g. 82) with ~1 Hz event cadence for 9+ minutes across table/face-down/fabric placements. For apps that infer wear state from the raw metric (safety monitors in particular), off-wrist is indistinguishable from a wearer with a steady pulse. Invalidate the peekable metric on off-wrist events (bpm 0, quality OffWrist, fresh timestamp), and exclude off-wrist events from the metrics/median path entirely: an off-wrist event may still carry a nonzero bpm (driver-dependent - the existing bpm=120/OffWrist unit test input), which previously would have overwritten the invalidation via the range-only validity check. Regression assertions added to the existing off-wrist sequence in test_activity. Field-validated on a retail Time 2 (obelix@pvt): the peeked value drops to 0 within ~30 s of removal; worn readings unchanged across a 450-sample six-condition sensor sweep. Signed-off-by: Dmitry Kvasnikov <dkvasnikov@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Off-wrist BPM events never update
hr.metrics.current_bpm— the storage behindActivityMetricHeartRateRawBPM/HealthMetricHeartRateRawBPM— sohealth_service_peek_current_value()keeps serving the last on-wrist reading indefinitely. The health event payload already reportscurrent_bpm = 0for off-wrist, but apps cannot read event payloads; peeking the metric is their only path.Measured on a Pebble Time 2: after taking the watch off, an app peeking
HealthMetricHeartRateRawBPMon eachHeartRateUpdateevent saw the last on-wrist value (bit-identical, e.g. 82) at ~1 Hz event cadence for 9+ minutes across table / face-down / on-fabric placements. For apps that infer wear state from the raw metric (safety monitors in particular), off-wrist is indistinguishable from a wearer with a steady pulse.Root cause
Off-wrist, the gh3x2x driver zero-initializes
HRMData(bpm = 0,quality = HRMQuality_OffWrist). Inprv_hrm_subscription_cb, bpm 0 fails theACTIVITY_DEFAULT_MIN_HRrange check, so the metrics writer never runs — and nothing else ever invalidates the stored raw metric.Fix
activity_metrics_prv_set_raw_hr_offwrist()invalidates the peekable metric on off-wrist events (bpm 0, qualityOffWrist, fresh timestamp), mirroring the adjacent locking/write patterns.valid_hr_reading, so a driver-dependent nonzero bpm on an off-wrist event (the existingbpm=120/OffWristunit-test input) cannot overwrite the invalidation through the range-only check.Possibly related to the existing
TODO PBL-40784about special-casingHRMQuality_OffWrist.Testing
test_activity.c: raw BPM/quality afterbpm=120/OffWrist, afterbpm=0/OffWrist, and restoration on the next valid on-wrist reading. Full unit test suite passes.obelix@pvt, sideloaded dev build): the peeked value drops to 0 within ~30 s of removal; worn readings unchanged across a 450-sample, six-condition sensor sweep. Raw data available on request.Disclosure
Developed with AI assistance (Claude). I reviewed the diff, built it locally, and field-tested it on real hardware; the use case is a safety application (unresponsiveness monitor) where wear-state honesty matters.
🤖 Generated with Claude Code