Repository navigation
alarm: let a strap be asked what alarm it has stored - #1710
Merged
Merged
Conversation
Follow-on to #1706, which now persists the raw readback frame but had no way to obtain one. On Android the GET_ALARM_TIME readback was reachable only by ARMING, so an install with the alarm switched off could not produce the evidence that would explain what its strap reports — which is exactly the state of the account whose 2045 readback prompted the issue. iOS has getStrapAlarm for this, but it has never had a caller on either platform, so porting the function alone would only have added dead code to a second platform. So: the port, plus a way to press it. A "Check what the strap has stored" button on the strap firmware-alarm card, shown when a strap is connected and the 5/MG branch has not claimed the card. The reply lands on the same GET_ALARM_TIME branch an arm's follow-up read uses, so it persists the reported epoch and the raw bytes for the debug export. Deliberately not family-gated, unlike armStrapAlarm. Only the 4.0 branch DECODES the reply, but a 5/MG still logs whatever frame comes back with its raw hex, and on a strap whose readback layout is undocumented that raw frame is the whole point of the button. Refusing to send would throw away the one thing worth capturing. Both strings are hand-authored, so they are named without the l10n_ prefix that in this file marks a key the extractor generated with a content hash. Verified: 4,725 Android tests, 0 failures; i18n audit clean; lintVitalFullRelease clean, run on its own because combining it with the debug test tasks trips a task-wiring validation bug that reproduces on unmodified main.
Re-review. The doc defended leaving getStrapAlarm ungated by saying a 5/MG still logs whatever frame comes back and that the raw frame is the whole point of the button. But the button renders only in the non-5/MG branch of the strap-alarm card, so the method can never be invoked on a 5/MG: the rationale described an unreachable path. Now says what is true. Ungated is defensive rather than a feature — send already no-ops when nothing is connected and only the 4.0 branch decodes the reply, so an errant call costs one ignored write — and it states outright that nothing captures the 5/MG frame today, with a note on where a caller would go if that layout ever needs recording. Verified: 4,725 Android tests, 0 failures.
Parity. Before this branch neither platform had a caller for getStrapAlarm, which was symmetric even if useless. Adding the Android button alone made it a feature-level gap, and on the wrong side: the alarm.rejectStreak warning is iOS-only, so iOS users are the ones more likely to meet a confusing readback with no way to capture the frame behind it. Same card, same copy, same command. One deliberate difference, commented at the call site: the Android button hides until bonded, this one is always shown. Gating is free on Android because that card already observes LiveState; SmartAlarmView does not, and this codebase keeps `live` out of parent views so a 1 Hz HR tick cannot re-render them. getStrapAlarm no-ops and logs when nothing is connected, so the cost of showing it is one ignored write rather than a per-second re-render of the whole alarm screen. Both strings carry all ten iOS locales; pl is ratcheted, so a missing one hard-fails the audit. The catalogue was edited as TEXT, not re-serialized. Re-serializing with json.dumps(indent=2) expands the file's hand-compacted entries and turns a two-string addition into a 3,426-line diff — the first attempt here did exactly that. The kept version is six added lines in the neighbours' own style, re-parsed to prove it still loads. Verified: i18n audit clean; 4,725 Android tests still green. SmartAlarmView is app-target, so the Swift half needs app-build.
7 of 12 tasks
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.
Follow-on to #1706, which now persists the raw readback frame but gave no way to obtain one.
The gap
The
GET_ALARM_TIMEreadback was reachable only by arming. An install with the alarm switched off could not produce the evidence that would explain what its strap reports — which is exactly the state of the account whose2045-06-10readback prompted the issue.iOS had
getStrapAlarm()for this, but it had never had a caller on either platform. So porting the function alone would only have added dead code to a second platform. This is the function plus a way to press it.What this adds, both platforms
A "Check what the strap has stored" button on the strap firmware-alarm card. The reply lands on the same
GET_ALARM_TIMEbranch an arm's follow-up read uses, so it persists the reported epoch and the raw bytes for the debug export.StrapAlarmCardSmartAlarmView, same card and copyWhoopBleClient.getStrapAlarm()BLEManager.getStrapAlarm()(existing)The gating difference is deliberate and commented at the call site. Gating is free on Android because that card already observes
LiveState.SmartAlarmViewdoes not, and this codebase deliberately keepsliveout of parent views so a 1 Hz HR tick cannot re-render them.getStrapAlarm()no-ops and logs when nothing is connected, so always showing it costs one ignored write rather than a per-second re-render of the whole alarm screen.Not family-gated
Unlike
armStrapAlarm, which refuses on a 5/MG — but that is defensive, not a feature.send()already no-ops when disconnected and only the 4.0 branch decodes the reply. Both buttons live in the 4.0 branch of their card, so nothing captures a 5/MG frame today; a caller could be added there if that layout ever needs recording, without changing the method.An earlier draft of the doc claimed the 5/MG raw frame was "the whole point of the button" — a path its only caller cannot reach. The doc now says what the code does.
Practical note for #1706
The reporting account has a 5/MG active and a 4.0 paired, and the
2045readback came from the 4.0. The button appears in the 4.0 branch, so that user needs to be connected to the 4.0 for it to show. It is "connect your 4.0, then press this", not "press this".Strings
Two strings. Android: all seven
values*/strings.xml, named without thel10n_prefix — in that file the prefix marks a key the extractor generated with a content hash, so a hand-written key wearing it would look machine-extracted. iOS: all ten locales in the catalogue (plis ratcheted, so a missing one hard-fails the audit).The catalogue was edited as text, not re-serialized.
json.dumps(indent=2)expands that file's hand-compacted entries and turns a two-string addition into a 3,426-line diff — the first attempt did exactly that, and the audit still passed on it, which is what made it dangerous. The kept version is six added lines in the neighbours' own style, re-parsed to prove it still loads.Verified
--rerun-tasks --no-build-cacheTools/i18n_audit.py --ciclean — it correctly rejected an earlier hardcoded Android help string, now localizedlintVitalFullReleasecleanSmartAlarmViewOne note for whoever runs the build
lintVitalFullReleasemust be invoked separately from the debug test tasks. Combining them trips a Gradle task-wiring validation error —syncRoomSchemaSnapshotconsumeskspFullReleaseKotlin's output without declaring the dependency. I confirmed it reproduces on unmodified main with the same command in an isolated worktree, so it is pre-existing and unrelated to this change. Probably worth its own issue; CI evidently uses separate invocations, which is why it has not surfaced.