Battery alert: stop re-firing the low-battery notification while charging (#80)#82
Merged
Conversation
…ing (#80) The low-battery alert re-fired repeatedly WHILE the strap was charging. The policy re-armed the low flag on `charging == true` but fired whenever `charging != true` (which includes the null/unknown state). The strap reports its charge bit only on BATTERY_LEVEL events (~every 8 min), so it flickers true→null between events: a `true` read re-armed the flag, then the `null` gap re-fired the alert — on every flicker, while plugged in. Fix (both platforms, identical): re-arm the low flag ONLY on genuine recovery (pct >= lowRearmAbove), not on the charge bit. `fireLow`'s `charging != true` guard is unchanged, so an explicit charging read still suppresses and a strap that never reports a charge bit (generic/FTMS, charging == null) still alerts. Charging still re-arms for the next cycle — via the recovered PERCENTAGE once it climbs past 25%, not the flickering bit. Tests: updated the "charging suppresses" case (charging must NOT re-arm) and added a true→null flicker regression on both iOS and Android. Android unit test green.
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
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.
Fixes #80 — the "charge your strap" low-battery notification fires repeatedly while the strap is charging.
Root cause (both platforms — identical policy)
BatteryAlertPolicyalready knew about charging, but the two conditions interacted badly:charging == true, yetcharging != true(which includes thenull/unknown state).The strap reports its charge bit only on BATTERY_LEVEL events (~every 8 min), so
chargingflickerstrue→nullbetween events:trueread → re-armslow = false(no fire).nullgap →!lowis true,pct ≤ 15,charging != trueis true → fires again.Every flicker re-armed then re-fired → repeated notifications while plugged in.
Fix
Re-arm the low flag only on genuine recovery (
pct >= lowRearmAbove), not on the charge bit:fireLow'scharging != trueguard is untouched, so:charging == trueread still suppresses;charging == null, e.g. generic/FTMS) still gets low alerts (no regression);Applied identically to iOS (
BatteryNotifier) and Android (BatteryAlertPolicy).Tests
Updated the "charging suppresses low" case (charging must not re-arm now) and added a true→null flicker regression on both platforms. Android unit test green (
testFullDebugUnitTest --tests BatteryAlertPolicyTest, exit 0). iOS is the byte-identical mirror (can't run on WSL — StrandTests → GRDBsqlite3.hwall — runs in CI).Files:
Strand/System/BatteryNotifier.swift,StrandTests/BatteryAlertPolicyTests.swift,android/.../BatteryAlertNotifier.kt,android/.../BatteryAlertPolicyTest.kt.