Repository navigation
ringconn: pair + drain, raw archive only - #344
Conversation
adds a BandLink.read() seam (ringconn's auth needs one GATT read that nothing else did) and a per-entry pairTier so a second zero-signal notify-class band can't inherit ble_hrs's beat-to-beat tag.
Reviewer's GuideIntroduces an experimental, raw-archive-only RingConn integration for Gen 2, Gen 2 Air, and Gen 3: it pairs and authenticates using the standard System ID, drains sleep and awake history through a new GATT-read-capable link, persists every frame verbatim without decoding, and integrates lifecycle, UI, background sync, and replay/database coverage. The implementation depends on the pending pinned openstrap_protocol PR. Sequence diagram for RingConn pairing and raw history syncsequenceDiagram
actor User
participant UI as Pairing UI
participant HrsLink
participant RingConnLink
participant Ring as RingConn
participant DB as Local DB
User->>UI: Select RingConn
UI->>HrsLink: pairNotifySensor(kRingConn)
HrsLink->>Ring: Connect and discover services
HrsLink->>DB: Store paired device
RingConnLink->>Ring: Read System ID
Ring-->>RingConnLink: EUI-64
RingConnLink->>Ring: write status command
Ring-->>RingConnLink: Authentication challenge
RingConnLink->>Ring: write SM3 auth response
Ring-->>RingConnLink: Authentication confirmation
RingConnLink->>Ring: Open sleep channel at now
Ring-->>RingConnLink: Sleep history pages
RingConnLink->>Ring: ACK page
RingConnLink->>Ring: Open awake channel at now
Ring-->>RingConnLink: Awake history pages
RingConnLink->>Ring: ACK page
RingConnLink->>DB: Archive every frame verbatim
Flow diagram for RingConn authentication and dual-channel drainflowchart TD
Start[Start sync] --> Read[Read System ID]
Read --> MAC[Recover BLE MAC]
MAC --> Auth[Authenticate with challenge and SM3 response]
Auth -->|success| Sleep[Drain sleep channel]
Auth -->|failure| ArchiveAuth[Archive received auth frames]
Sleep --> Awake[Drain awake channel]
Awake --> Archive[Archive all frames verbatim]
ArchiveAuth --> End[End sync]
Archive --> End
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds RingConn BLE support for pairing, authentication, raw history synchronization, archive storage, device management, and background synchronization. It also adds GATT read support and updates the protocol dependency pin. ChangesRingConn BLE contracts
RingConn adapter
Connection and storage
Application integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DeviceScreen
participant RingConnLink
participant BluetoothDevice
participant RingConnAdapter
participant ArchiveStore
DeviceScreen->>RingConnLink: request sync
RingConnLink->>BluetoothDevice: connect and discover services
RingConnLink->>RingConnAdapter: run synchronization
RingConnAdapter->>BluetoothDevice: authenticate and drain history
RingConnAdapter->>ArchiveStore: commit raw frames
RingConnLink->>BluetoothDevice: disconnect
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="lib/ble/adapters/ringconn.dart" line_range="216-244" />
<code_context>
+ final rec = await inbox.next(replyTimeout);
</code_context>
<issue_to_address>
**issue (bug_risk):** The page loop silently stops after 512 pages even if the ring still reports more pages or has not emitted a burst-end response, leaving the remainder of a valid history burst undrained while still returning a successful session.
**Triggers:** When a channel has more than 512 pages queued in one burst.
**Suggested fix:** Surface the ceiling as an incomplete-drain failure or continue using a protocol-defined completion condition rather than treating the cap as a normal successful end.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the unverified RingConn protocol or archive handling is wrong, the sync can persist incorrect or unusable raw records and leave paired-device state behind after a revert; those effects are bounded and can be removed or repaired, but they do not disappear automatically when the code is reverted.
Blocking findings: lib/ble/adapters/ringconn.dart:244
PR Reviewer Guide 🔍(Review updated until commit 02ac02e)Here are some key observations to aid the review process:
|
a burst that never signals done and just runs out the 512-page cap fell through with no log line, unlike every other early exit here.
PR Code Suggestions ✨No code suggestions found for the PR. |
|
Persistent review updated to latest commit b149aa2 |
PR Code Suggestions ✨Explore these optional code suggestions:
|
Wire the ring into the devices screen's manual sync affordance (was hardcoded to Oura only) and repin openstrap_protocol to feat/ringconn's head so the branch actually compiles in CI instead of only through a gitignored local override. Yield each drained frame to the host as it arrives instead of accumulating a whole channel's burst for one yield at the end, so a page reaches the host's buffer before the ack that advances the ring's own resume pointer, not after the whole burst finishes. Drop anything left over in the inbox between the two channels so a straggler from the first can't get mistaken for the second's sync-open reply.
|
Triaging the reviewer-guide findings from the PR-Agent pass (issue comments, no comment_id to thread a reply on): Missing Pin Update — confirmed, and it was breaking CI ( Data Loss Window — confirmed. State Machine Desync — confirmed as a real race, but I didn't take the suggested Also replied inline on the 512-page-cap comment (declined, with reason). |
A pub get run with pubspec_overrides.yaml present after the repin rewrote the lock back to the local worktree path, which is exactly what the CI guard script exists to catch.
|
Persistent review updated to latest commit e274793 |
|
Persistent review updated to latest commit 02ac02e |
|
Failed to generate code suggestions for PR |
1 similar comment
|
Failed to generate code suggestions for PR |
The page ack is what advances the ring's own resume pointer, and until now the adapter sent it directly after yielding the page — the yield only buffers the raw bytes in the host's memory, durable commit is a separate periodic flush. A crash between the two would have let the ring move past a page nothing ever wrote to disk. Route the ack through OffloadCheckpoint's confirm callback, same contract oura.dart already uses, so the host commits before the ack goes out.
|
Data-durability ordering finding — confirmed, fixed in 9d479ab. The previous round's fix (yield each page before its ack, not accumulated for the whole burst) only got the raw bytes into the host's in-memory buffer; the actual DB commit was still on the periodic flush timer, so a crash between the ack and that flush could let the ring's resume pointer move past a page nothing had written to disk yet. Routed the ack through |
|
Failed to generate code suggestions for PR |
|
Failed to generate code suggestions for PR |
1 participant
User description
pairs RingConn (Gen 2 / Gen 2 Air / Gen 3), authenticates, drains both history channels (sleep + awake), banks every frame verbatim. experimental — no signals declared, nothing decoded, pairing UI wired same as oura.
needs the protocol PR (OpenStrap/protocol#48) merged and pinned before this can merge — using a local pubspec_overrides.yaml in the meantime.
Summary by Sourcery
Integrate experimental RingConn support for authenticated raw-history synchronization across pairing, background sync, and device management.
New Features:
Bug Fixes:
Enhancements:
Tests:
PR Type
Enhancement
Description
Adds experimental RingConn pairing and authentication.
Drains history channels into
raw_archivewithout decoding.Adds
read()toBandLinkfor GATT reads.Prevents notify-only sensors from inheriting incorrect tiers.
Integrates RingConn into background headless sync flow.
Diagram Walkthrough
flowchart LR UI["UI (devices.dart)"] -- "Pair RingConn" --> RingConnLink["RingConnLink"] BgSync["Background Sync"] -- "Trigger Sync" --> RingConnLink RingConnLink -- "Drive Session" --> RingConnAdapter["RingConnAdapter"] RingConnAdapter -- "Read/Write/Notify" --> GattLink["GattBandLink"] RingConnAdapter -- "Bank Frames" --> RawArchive[("raw_archive DB")]File Walkthrough
1 files
Add RingConn constants and pairTier to BandEntry6 files
Add read method to BandLink interfaceImplement read method for GATT characteristic readsImplement RingConnAdapter for auth and channel drainingImplement RingConnLink host for device sessionsAdd RingConn sync to headless background syncAdd RingConn to device pairing UI1 files
Use pairTier and route RingConn unpairing4 files
Add RingConn to adapter signals testTest RingConn adapter state machineUpdate registry tests to include RingConnTest RingConn host session and raw archivingSummary by CodeRabbit
New Features
Bug Fixes