gate get_alarm_time on status like getclock/getdatarange - #60
Conversation
gen5 GET_ALARM_TIME reply was decoded off the form byte alone, no status check. a failure/deferred reply leaves the body stale, so old alarm bytes could get reported as the current alarm. same fix as the getclock/ getdatarange/getbatterypackinfo status gates already in this file, gen4 stays ungated for the same unconfirmed-status-byte reason.
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 17 hours by commenting @sourcery-ai review. Upgrade to get a review now.
|
Warning Review limit reachedNext included review available in 9 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 (2)
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 GuideUpdates GET_ALARM_TIME parsing to ignore potentially stale body data for non-success gen5 responses while retaining the existing ungated gen4 behavior, with focused regression tests for both profiles and status outcomes. Sequence diagram for status-gated GET_ALARM_TIME parsingsequenceDiagram
participant Response
participant Parser
participant Decoded
Response->>Parser: parseCommandResponse(inner, ...)
Parser->>Parser: status = response status
alt gen5 and status != 1
Parser-->>Decoded: no alarm_epoch or active field
else gen5 status == 1
Parser->>Parser: inspect form and payload length
Parser-->>Decoded: decode alarm_epoch and active
else gen4
Parser->>Parser: inspect form and payload length
Parser-->>Decoded: decode alarm_epoch and active
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
gen5 GET_ALARM_TIME reply was decoded off the form byte alone, no status check. same bug class as getclock/getdatarange/getbatterypackinfo which already got this fix in this file - a failure/deferred reply leaves the body stale, so leftover bytes from a prior read could get reported as a confident current alarm epoch/active flag.
fix: gate the decode on
!profile.isGen5 || status == 1, same pattern as the sibling opcodes. gen4 stays ungated (unconfirmed status convention there, same caveat as the other three).added tests: gen5 non-success (0/2/3) emits neither field, gen5 success still decodes, gen4 stays ungated on failure.
Summary by Sourcery
Gate gen5 alarm-time decoding on a successful response status while retaining the existing gen4 behavior.
Bug Fixes:
Tests: