decodeFrame historical hr + garmin mlr fixes - #73
abdulsaheel wants to merge 2 commits into
Conversation
![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 12 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 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 changes update historical R10 frame routing, Garmin MLR decoding, and the byte layout of CLOSE_ALL requests. Tests cover the R10 frame cases, bare-handle MLR data, and Garmin request fields. ChangesR10 Frame Routing
Garmin Protocol
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The changes fix short historical heart-rate records being misread as realtime data, accept Garmin bare-handle data frames, and reorder the close-all request fields. No concrete merge-blocking risk was identified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The changes are narrowly scoped byte-decoding and encoding corrections, with no demonstrated new authorization bypass. However, accepting additional Garmin handle formats and correcting the reset command depend on session controls outside the supplied code, so downstream safety remains unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Hardening Proposals
🚥 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 GuideThe PR hardens frame decoding by preventing short gen4 historical records from entering realtime-HR heuristics, while keeping valid archived R10 packets supported. It also fixes Garmin MLR handling for bare handle-addressed data and aligns CLOSE_ALL serialization with the protocol’s client-ID offset. Flow diagram for historical and realtime frame decodingflowchart TD
A["Decode data record"] --> B{"Record is historical 0x2F?"}
B -->|Yes| C{"R10 at valid minimum length?"}
B -->|No| D{"R10 realtime or valid length?"}
C -->|Yes| E["parseR10Lite"]
C -->|No| F["Skip realtime-HR heuristics"]
D -->|Yes| E
D -->|No| G{"Packet length less than 64?"}
G -->|Yes| H["Compact realtime parsing"]
G -->|No| I["Other record decoding"]
Flow diagram for Garmin MLR notification decodingflowchart TD
A["garminDecodeMlr"] --> B{"Flagged handle byte?"}
B -->|Yes| C["Decode flagged handle data"]
B -->|No| D{"Bare nonzero handle byte?"}
D -->|Yes| E["Decode bare handle data"]
D -->|No| F{"Control packet type"}
F --> G["Decode Garmin control response"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
1 participant
decodeFrame was still turning short gen4 0x2F records into realtime_hr (version byte 10 hit the r10 path, anything under 64 bytes read a timestamp byte as bpm). same gate as live.dart now.
garmin: a watch sends data on a bare handle byte, not just the flagged form, so those were all getting dropped. also close-all had the client id 2 bytes late, it goes right after the type like register-ml.
Summary by Sourcery
Fix historical heart-rate detection and Garmin MLR handle and close-all request handling.
Bug Fixes:
Tests:
Summary by CodeRabbit