fix R10 misdecode of short historical records with version byte 10 - #68
Conversation
recType 10 is both the R10 live-raw record tag and a valid but unmapped historical layout-version byte. decodeRecord/realtimeRr routed any recType==10 frame through the R10 field map with no length or plausibility check, so a short (~96-byte) 0x2F historical record whose version byte happened to be 10 got its unrelated v24-layout bytes read as hr/rr/imu instead of falling through to the archive path every other unrecognized version already takes. gate the R10 branch on pkt==0x2B (unambiguous at any length) or on a genuine-R10 length floor (~1920-byte records, incl. archived 0x2F R10 snapshots, which real hardware does send). full test suite still green.
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 5 hours by commenting @sourcery-ai review. Upgrade to get a review now.
|
Warning Review limit reachedNext included review available in 10 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 (1)
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 version-byte collisions by recognizing R10 only for unambiguous realtime raw packets or sufficiently long genuine R10 records, preserving support for full-length archived R10 snapshots while preventing short historical records from being misdecoded. Flow diagram for R10 record recognitionflowchart TD
A["Receive record"] --> B{recType == 10?}
B -->|No| C["Use existing record/archive handling"]
B -->|Yes| D{pkt is realtimeRawData?}
D -->|Yes| E["Decode as R10"]
D -->|No| F{length >= kR10MinLength?}
F -->|Yes| E
F -->|No| G["Return null / archive as unrecognized historical version"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
recType 10 is both the R10 live-raw record tag AND a valid-but-unmapped historical layout-version byte. decodeRecord/realtimeRr routed any frame with recType==10 through the R10 100Hz field map with no length or plausibility check, so a short (~96 byte) 0x2F historical record whose version byte happened to be 10 got its unrelated v24-layout bytes read as hr/rr/imu instead of falling through to the archive path every other unrecognized version already takes.
fix: gate the R10 branch on pkt==0x2B (unambiguous at any length, even truncated) OR a genuine-R10 length floor (~1920 bytes, kR10MinLength=685 for the accel arrays) — real hardware does send archived 0x2F R10 snapshots at full length, confirmed against the 2934-case parity fixture, so this can't just be a pkt-type check.
verified: patched a real v24 fixture's version byte to 10 at short length, decodeRecord/realtimeRr both correctly return null now (were misdecoding hr=98 + fabricating steps before). full dart test suite green (631 passed, 4 pre-existing skips) in a clean worktree off main.
Summary by Sourcery
Correct R10 record detection so short historical version-10 records are not interpreted using the live R10 layout.
Bug Fixes:
Enhancements: