cmdGetDataRange: take a profile like its siblings - #66
abdulsaheel wants to merge 1 commit into
Conversation
control.dart already decodes GET_DATA_RANGE as dual-profile but the
builder always framed gen4. folds cmdGetDataRangeGen5 into the same
{profile} pattern as the other builders instead of keeping a separate
gen5-only function.
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 21 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 guide (collapsed on small PRs)Reviewer's GuideMakes the public cmdGetDataRange builder profile-aware, preserving the gen4 default while producing the gen5 envelope with an empty payload; removes the duplicate gen5 API and updates coverage to verify the unified implementation. Sequence diagram for profile-aware GET_DATA_RANGE framingsequenceDiagram
participant Caller
participant cmdGetDataRange
participant buildCommand
participant Strap
Caller->>cmdGetDataRange: cmdGetDataRange(seq, profile)
alt profile is gen5
cmdGetDataRange->>buildCommand: buildCommand(seq, Cmd.getDataRange, [], profile)
buildCommand-->>Strap: Gen5 envelope with empty payload
else profile is gen4
cmdGetDataRange->>buildCommand: buildCommand(seq, Cmd.getDataRange, [0x00], profile)
buildCommand-->>Strap: Gen4 envelope with 0x00 payload
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
subsumed by #65 (already merged), which covers cmdGetDataRange's gen5 profile param as part of the broader gen5-framing sweep. closing, not a real gap. |
control.dart already decodes GET_DATA_RANGE (0x22) as dual-profile (
!profile.isGen5 || status == 1) but the request-side builder never got the treatment the sibling opcodes got in #55-#63 — it always framed gen4 (8-byte crc8), so a gen5 caller of the primarycmdGetDataRangewould build a frame the strap can't parse.there was already a separate
cmdGetDataRangeGen5doing the right thing with the gen5 envelope + empty payload, duplicating the profile-param pattern into a second function instead of extending the primary one like every sibling did. folded it in and deleted the dupe.edge doesn't call either builder (it frames GET_DATA_RANGE itself via
_send), so no runtime blast radius, this is purely fixing the protocol package's public API.cmdGetDataRange(seq, {profile})now branches payload/envelope by profile, default unchanged (gen4,[0x00]body)cmdGetDataRangeGen5+ its exportdoc_conformance_test.dart's existing gen5 GET_DATA_RANGE assertion to go through the primary builderdart testgreen (631 passed, 4 skipped, same skip count as main).Summary by Sourcery
Update GET_DATA_RANGE command construction to support profile-specific framing through the primary API.
Bug Fixes:
Enhancements:
Tests: