hello battery offset + v24 rr slot cap - #74
Open
abdulsaheel wants to merge 2 commits into
Open
abdulsaheel wants to merge 2 commits into
abdulsaheel wants to merge 2 commits into
Conversation
… trim labrador comments
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
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 9 hours by commenting @sourcery-ai review. Upgrade to get a review now.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe decoder now starts battery scanning at payload offset 3 and limits historical v24/v12 R-R slots to 4. Regression tests cover both guards. Comments and test descriptions were also revised. ChangesDecode guards and protocol notes
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The PR adjusts HELLO battery parsing and historical R-R slot handling, with regression tests for both paths. No merge-blocking risk is evident; it appears ready for normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 GuideFixes HELLO battery detection by starting after the fixed prefix byte, caps v24/v12 R-R parsing at their four available slots while retaining the R10 cap of eight, and trims Labrador/MG comments to concise field-level facts. Flow diagram for HELLO battery field detectionflowchart LR
A[HELLO payload] --> B["payload[3..9] scan"]
B --> C{u16 value 10..1000?}
C -->|yes| D[Battery percentage]
C -->|no| B
E["payload[2] fixed 0x04 prefix"] -. skipped .-> B
Flow diagram for version-specific R-R slot capsflowchart LR
A[Record type] --> B{v24/v12?}
B -->|yes| C[_kV24RrSlots = 4]
B -->|no R10| D[kMaxRrPerRecord = 8]
C --> E{Declared count within cap?}
D --> E
E -->|yes| F[Read available R-R intervals]
E -->|no| G[Return no R-R intervals]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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
1 participant
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.
hello battery scan started at payload[2], which is a fixed 0x04 byte. with a battery low byte of 1-3 it read 0x04|lo<<8 as 26.0/51.6/77.2% and never got to the real field at [3]. scan starts at 3 now.
v24/v12 records only have 4 rr slots (19..25) but the count was allowed up to 8, so a count of 5-8 read [27] and ppg_green as beats. capped at 4 there, 8 stays for r10.
also trimmed the labrador/mg comments down to the field facts.
Summary by Sourcery
Fix HELLO battery parsing and constrain v24/v12 R-R decoding to the slots available in each record.
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit