Repository navigation
hello battery offset + v24 rr slot cap - #74
Conversation
… trim labrador comments
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. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 32 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (9)
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
|
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 decoding and constrain v24/v12 R-R parsing to the records' available slots.
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit