gen5: record HELLO revision instead of gating parsing - #35
Conversation
The official 5.458.0 parser records body[0] but reads the fixed revision-1 offsets regardless of its value; only the short-body rejection is a real parse failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DCrELi9LMQr55gW9hCJoeN
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesGen5 Hello Revision Handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized parser change removes an overly strict revision gate while retaining the length check and regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ntics; flag the pending OpenStrap#35 repin Three follow-ups on the bootstrap review: A Memfault chunk is real inbound traffic on the link, so it now advances _lastRx like every other notification — a strap volunteering crash data must not look silent to the staleness fuse and get its link bounced. New regression test for the SET_CLOCK result split the contract implies: a FAILURE result inside a non-null response object still satisfies readiness (only a null result fails, one write, no resend) — but the strap did NOT take the write, so the phone-suspect history deferral computed off the pre-correction reading stays live; clearing it is reserved for an accepted correction. The protocol pin stays at 4ce8f02 (OpenStrap#33's tree, whose parser still nulls any non-1 hello revision — under the mandatory hello that means a future revision bump cannot connect). OpenStrap/protocol#35 lifts that gate; the pending repin is now documented at BOTH pin locations — pubspec.yaml's ref and kProtocolPin in derivation_engine.dart — with the rule that every pin location moves together to the main merge commit (the pin-equality test fails a partial repin) and the kAlgoVersion no-bump reasoning to re-verify against the actual merge diff.
…ision gate is gone All three pin locations move together to protocol main @ 6664854, the OpenStrap/protocol#35 merge commit. The old pin's parser returned null for any hello body whose revision byte was not 1; under this branch's mandatory-hello bootstrap that made a future firmware revision bump unable to connect. OpenStrap#35 records the byte instead of gating on it. NO kAlgoVersion bump, verified against the full 4ce8f02..6664854 diff: OpenStrap#35's one code change widens which HELLO bodies PARSE (hello feeds connection identity/state, not the derivation pipeline); OpenStrap#34, also in the hop, only ADDS files (oura + generic-HRS wire formats nothing here imports); the rest is comment rewording. No decoder for a persisted record moves, so no stored number can.
2 participants
Why
Gen5HelloInfo.parsecurrently rejects a complete WHOOP 5 HELLO response whenever its revision byte is not exactly1.The official WHOOP Android 5.458.0 parser does not use that byte as an equality gate. It records the revision and reads the established fixed-offset HELLO layout through byte 103. Consequently, the current check can discard an otherwise compatible HELLO response and prevent connection readiness after a firmware revision change.
This aligns the parser with the behavior recovered from the official client while retaining a defensive length check for independent implementations.
What changed
helloRevision == 1parser requirement.helloRevision.Compatibility and safety
Evidence
The inspected official parser records the revision without an explicit
revision == 1gate and reads the fixed offsets through byte 103.Validation
dart analyze— no issuesdart test— 374 passedwhoop_hist.jsonlis not present beside the repositorygit diff --check origin/main...HEAD— cleanSummary by CodeRabbit