fix: wire up gen5 framing on remaining unwired command builders - #65
Conversation
cmdLinkValid, cmdGetHello, cmdGetHelloModern, cmdReportVersionInfo, cmdToggleHr, cmdSendR10R11, cmdEnableOptical had no [profile] param at all, so a gen5 caller always got gen4's crc8 header - unparseable on a real gen5 strap. same bug already fixed on cmdGetBattery, cmdSetClock, cmdEnterHighFreqSync etc, just missed these. also folded the gen5 payload branch (empty body vs gen4's [0x00]) into cmdGetDataRange and cmdSendHistorical directly instead of keeping separate Gen5 twins, since nothing called the twins anywhere - deleted cmdGetDataRangeGen5/cmdSendHistoricalGen5. nothing in edge/lib calls any of these yet so this is latent, not a live fix. semantics on cmdSendR10R11/cmdEnableOptical stay unresolved per their existing doc comments - this only fixes framing.
There was a problem hiding this comment.
Sorry @abdulsaheel, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 6 days and 7 hours by commenting @sourcery-ai review. Upgrade to get a review now.
|
Warning Review limit reachedNext included review available in 22 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
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 |
Reviewer's GuideThe PR makes the remaining command builders profile-aware so callers can generate parseable gen5 frames while preserving gen4 defaults, folds gen5 payload differences into the shared range and historical-data builders, removes unused gen5-specific APIs, and adds dual-profile conformance tests. Sequence diagram for profile-aware command framingsequenceDiagram
participant Caller
participant Builder as CommandBuilder
participant Framer as buildCommand
participant Device
Caller->>Builder: cmdLinkValid(seq, profile)
Builder->>Framer: buildCommand(seq, command, payload, profile)
Framer->>Framer: Select gen4 or gen5 wire envelope
Framer->>Device: Profile-compatible framed command
Device-->>Caller: Parseable command
Flow diagram for profile-specific command payloadsflowchart LR
A[cmdGetDataRange or cmdSendHistorical] --> B{profile.isGen5}
B -->|yes| C[Build with empty payload]
B -->|no| D[Build with 0x00 payload]
C --> E[buildCommand with profile]
D --> E
E --> F[Gen5 or gen4 framed command]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
cmdLinkValid, cmdGetHello, cmdGetHelloModern, cmdReportVersionInfo, cmdToggleHr, cmdSendR10R11, cmdEnableOptical took no [profile] param, so buildCommand always framed them gen4 (crc8 header) - a real gen5 strap can't parse that at all. same class of bug we already fixed on cmdGetBattery/cmdSetClock/cmdEnterHighFreqSync/cmdToggleImu etc, these eight just got missed.
also folded the gen5 payload branching for GET_DATA_RANGE/SEND_HISTORICAL_DATA (empty body on gen5 vs gen4's [0x00]) directly into cmdGetDataRange/cmdSendHistorical, and deleted cmdGetDataRangeGen5/cmdSendHistoricalGen5 since nothing called them (grepped edge/lib, ml/, and this repo's own tests/lib).
latent, not live - nothing in edge/lib calls any of these eight yet. semantics on cmdSendR10R11 (gen5 ignores 0x3F) and cmdEnableOptical (opcodes reversed on gen5) are still open per their own doc comments, this PR only fixes the wire envelope.
tests: added gen5+gen4 framing assertions for all 8 builders in doc_conformance_test.dart / whoop_protocol_update_test.dart.
dart test-> 630 passed, 4 pre-existing skips.Summary by Sourcery
Support gen5 framing across the remaining command builders and align command payloads with each hardware generation.
Bug Fixes:
Enhancements:
Tests: