Support direct heart-rate broadcasting on WHOOP 4.0 - #2400
Conversation
ryanbr
left a comment
ryanbr
left a comment
There was a problem hiding this comment.
Thanks @don86nl. The shape is right: opcode 14 is free in CommandNumber (the DOUBLE_TAP(14) clash is in the event enum, a different space), the feature is opt-in and reversible, Swift and Kotlin match, and the reconnect restore sits inside runConnectHandshake, which is WHOOP4-only, so it cannot re-write the 5/MG config flag every connect or collide with the ECG gate's shared 121 read-back.
One thing to change before merge.
The 4.0 path claims a result it did not check
The 5/MG branch right below it writes the flag, reads it back on opcode 121, and its own comment says the write ack is never trusted, only the read-back decides. It then warns that even a confirmed read-back does not prove the strap advertises 0x180D, because some firmware stores the flag and ignores it.
The new path is:
send(.toggleGenericHRProfile, payload: [on ? 0x01 : 0x00])
log("Broadcast HR: WHOOP 4 \(on ? "enable" : "disable") command sent (14).")You verified 14 on your strap, and I believe it. But this was tested on one 4.0 on one firmware, the toggle reflects a local pref rather than the strap, and a 4.0 that does not implement 14 gives the user a switch that stays on, a log line that reads like success, and no HR broadcast.
Please make the log say what it knows: sent, not confirmed. If command 14 draws a COMMAND_RESPONSE, logging that would be better still.
Smaller
testWhoop4EnableFrameMatchesHardwareTestedCommandpins the bytes the code builds, which is worth having, but the hardware evidence is in the PR description and no test can assert what the strap did. The name claims the stronger thing.- Where 14 came from is not cited.
ABORT_HISTORICAL_TRANSMITStwo lines up names its evidence; this one names only the issue.
Before it can be judged green
CI has run nothing: fork workflows are parked awaiting approval, which the API reports as zero checks rather than a failure. The base is also 039dd9392, now eight merges behind, two of which (#2403, #2402) rework WhoopBleClient.kt around this code. Rebase and I will approve the run.
c0dd5e5 to
ec3e80f
Compare
|
Addressed in
Verification rerun after the rebase:
The Swift app target still was not locally compiled because this development host is Windows. |
ec3e80f to
420433a
Compare
#2400 flipped "Strap broadcasts its own HR" to a green Yes for the 4.0. That is the one place the feature asserts the effect works. Every log line on the same path deliberately refuses to: the strap is asked over TOGGLE_GENERIC_HR_PROFILE and answers, but nothing reads back whether it then advertises 0x180D, and the opcode is confirmed on one strap on one firmware. The comparison screen is where a 4.0 owner decides whether to rely on this, so it must not claim more than the code does. Partly, in the warning tone, with the description saying what is and is not known, which is the convention every other Partly row on this screen already follows. The 5/MG column keeps Yes: its device-config write is read back on opcode 121.
2 participants
Summary
TOGGLE_GENERIC_HR_PROFILE(14) path alongside the existing 5.0/MG device-config pathRefs #159
Verification
./gradlew :app:compileFullDebugKotlin./gradlew :app:testFullDebugUnitTest --tests com.noop.protocol.BroadcastHrConfigTest