Repository navigation
v20 optical: Kotlin RawImu gate parity + restore the corroborating RE evidence (#546 follow-up) - #577
Merged
Merged
Conversation
…ce (#546 follow-up) Two points from the #546 review, now that it's merged. 1. Whoop5RawImu length-gate parity. #546 tightened the Swift decoder's gate from `>=` to `== bufferLength` so the 1244-B IMU and 2140-B optical decoders can't cross-misfire, but the Kotlin twin still read `f.size < bufferLength` (accepting anything >= 1244). Its own KDoc already claimed "exact length", so the code now matches both the doc and Swift: `f.size != bufferLength`. The only caller (PuffinDeepBufferLog) already gates exact length, so this is a no-op on real frames — it just closes the same cross-misfire the Swift change did. Also adds the Kotlin twin of #546's Swift over-length test (a valid buffer + 1 trailing byte must be rejected) — it pins the fix and fails on the old `<` gate. 2. Restore the RE evidence #546 dropped. The superseded decoder cited issue #423's config-header fields — an LED-current @28 and per-photodiode offset DACs @38/@45 — which #546 removed while strengthening "inferred to be optical" into "is optical". Those notes CORROBORATE the new paired-block model: under the 21-byte split, @28 falls in a block's shared bytes (@27..32) and @38/@45 fall in its two 7-byte channel descriptors (@33..39, @40..46) — one LED per block, one detector per channel, which aligns with "two readout channels share one measurement config". Re-added to both Whoop5RawOptical docs (Swift + Kotlin twin), hedged as a non-authoritative line of evidence for the detector/readout reading rather than two wavelengths. Not included: the `sensor_channel_samples` = max(block counts) naming point (review item 3). Tested (fully — pure package + Kotlin, no app-target/CI gap): swift test (WhoopProtocol) -> 289/289 ./gradlew testFullDebugUnitTest --tests "com.noop.protocol.*" -> 164/164
ryanbr
force-pushed
the
fix/v20-optical-parity-and-evidence
branch
from
July 18, 2026 21:22
b6e395f to
6df5b6c
Compare
DX23876
pushed a commit
to DX23876/noop
that referenced
this pull request
Jul 22, 2026
…ce (ryanbr#546 follow-up) (ryanbr#577) Two points from the ryanbr#546 review, now that it's merged. 1. Whoop5RawImu length-gate parity. ryanbr#546 tightened the Swift decoder's gate from `>=` to `== bufferLength` so the 1244-B IMU and 2140-B optical decoders can't cross-misfire, but the Kotlin twin still read `f.size < bufferLength` (accepting anything >= 1244). Its own KDoc already claimed "exact length", so the code now matches both the doc and Swift: `f.size != bufferLength`. The only caller (PuffinDeepBufferLog) already gates exact length, so this is a no-op on real frames — it just closes the same cross-misfire the Swift change did. Also adds the Kotlin twin of ryanbr#546's Swift over-length test (a valid buffer + 1 trailing byte must be rejected) — it pins the fix and fails on the old `<` gate. 2. Restore the RE evidence ryanbr#546 dropped. The superseded decoder cited issue ryanbr#423's config-header fields — an LED-current @28 and per-photodiode offset DACs @38/@45 — which ryanbr#546 removed while strengthening "inferred to be optical" into "is optical". Those notes CORROBORATE the new paired-block model: under the 21-byte split, @28 falls in a block's shared bytes (@27..32) and @38/@45 fall in its two 7-byte channel descriptors (@33..39, @40..46) — one LED per block, one detector per channel, which aligns with "two readout channels share one measurement config". Re-added to both Whoop5RawOptical docs (Swift + Kotlin twin), hedged as a non-authoritative line of evidence for the detector/readout reading rather than two wavelengths. Not included: the `sensor_channel_samples` = max(block counts) naming point (review item 3). Tested (fully — pure package + Kotlin, no app-target/CI gap): swift test (WhoopProtocol) -> 289/289 ./gradlew testFullDebugUnitTest --tests "com.noop.protocol.*" -> 164/164
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…ce (ryanbr#546 follow-up) (ryanbr#577) Two points from the ryanbr#546 review, now that it's merged. 1. Whoop5RawImu length-gate parity. ryanbr#546 tightened the Swift decoder's gate from `>=` to `== bufferLength` so the 1244-B IMU and 2140-B optical decoders can't cross-misfire, but the Kotlin twin still read `f.size < bufferLength` (accepting anything >= 1244). Its own KDoc already claimed "exact length", so the code now matches both the doc and Swift: `f.size != bufferLength`. The only caller (PuffinDeepBufferLog) already gates exact length, so this is a no-op on real frames — it just closes the same cross-misfire the Swift change did. Also adds the Kotlin twin of ryanbr#546's Swift over-length test (a valid buffer + 1 trailing byte must be rejected) — it pins the fix and fails on the old `<` gate. 2. Restore the RE evidence ryanbr#546 dropped. The superseded decoder cited issue ryanbr#423's config-header fields — an LED-current @28 and per-photodiode offset DACs @38/@45 — which ryanbr#546 removed while strengthening "inferred to be optical" into "is optical". Those notes CORROBORATE the new paired-block model: under the 21-byte split, @28 falls in a block's shared bytes (@27..32) and @38/@45 fall in its two 7-byte channel descriptors (@33..39, @40..46) — one LED per block, one detector per channel, which aligns with "two readout channels share one measurement config". Re-added to both Whoop5RawOptical docs (Swift + Kotlin twin), hedged as a non-authoritative line of evidence for the detector/readout reading rather than two wavelengths. Not included: the `sensor_channel_samples` = max(block counts) naming point (review item 3). Tested (fully — pure package + Kotlin, no app-target/CI gap): swift test (WhoopProtocol) -> 289/289 ./gradlew testFullDebugUnitTest --tests "com.noop.protocol.*" -> 164/164
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-up to #546 (merged). Addresses the two actionable points from its review; unlike the recent app-target PRs this is the pure WhoopProtocol package + Kotlin, so it's fully tested on both platforms (Swift 289/289, Kotlin 164/164) — no verification gap.
1.
Whoop5RawImulength-gate parity (Kotlin)#546 tightened the Swift decoder's gate
>= bufferLength→== bufferLengthso the 1244-B IMU and 2140-B optical decoders can't cross-misfire. The Kotlin twin still readf.size < bufferLength(i.e. accepted anything ≥ 1244), and its own KDoc already claimed "exact length" — so the code now matches both the doc and Swift:The only caller (
PuffinDeepBufferLog) already gates exact length, so this is a no-op on real frames — it just closes the same cross-misfire on Android that #546 closed on Apple.Whoop5RawImuTeststays 3/3.2. Restore the RE evidence #546 dropped — because it corroborates the new model
#546 removed the superseded decoder's citation of issue #423's config-header fields (LED-current
@28, per-photodiode offset DACs@38/@45) while upgrading "inferred to be optical" to "is optical". But those notes are the strongest independent support for the paired-block model this PR introduced. Under the 21-byte split:@28@27..32@38@33..39@45@40..46One LED per block, one detector per channel — exactly what "two readout channels share one measurement config, not two wavelengths" predicts. Re-added to both
Whoop5RawOpticaldocs (Swift + Kotlin twin) as the evidence for the detector/readout reading, so the model is self-justifying rather than asserted.Not included
The
sensor_channel_samples=max(block counts)naming point (review item 3) — a judgement call on a field name with no consumers. Left for the maintainer rather than bundled here.Verification
swift test(WhoopProtocol) → 289/289./gradlew testFullDebugUnitTest --tests "com.noop.protocol.*"→ 164/164Swift change is comment-only; the Kotlin change is a stricter length gate with zero test regressions.