smaq2oss: pairs and banks raw data, decodes nothing yet - #352
smaq2oss: pairs and banks raw data, decodes nothing yet#352abdulsaheel wants to merge 6 commits into
Conversation
Reviewer's GuideAdds an explicitly experimental SMA-Q2-OSS adapter that discovers a configured GATT service, listens without sending commands or decoding signals, archives notifications verbatim during bounded foreground/background sync sessions, and exposes the device through pairing and profile UI. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reachedNext included review available in 51 minutes. View limit detailsLimit details: You’ve used all 4 included reviews 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (6)
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/smaq2oss_link.dart" line_range="64-76" />
<code_context>
+ }
+
+ Future<bool> _sync() async {
+ final row = await pairedRow();
+ if (row == null) return false;
+ final deviceId = row['id'] as String?;
</code_context>
<issue_to_address>
**issue (bug_risk):** `sync()` calls `pairedRow()` before entering the `_sync` try/catch, so a failure from `LocalDb.deviceRows()` escapes despite the method's contract stating that it never throws. The profile sync action does not catch this exception and reports an unhandled failure to the UI.
**Triggers:** When the database read used to locate the paired watch fails.
**Suggested fix:** Move the paired-row lookup and row field validation inside the existing try/catch, or catch and convert those failures to `false`.
```suggestion
Future<bool> _sync() async {
try {
final row = await pairedRow();
if (row == null) return false;
final deviceId = row['id'] as String?;
final remoteId = row['remote_id'] as String?;
if (deviceId == null || remoteId == null || remoteId.isEmpty) return false;
if (deviceId == LocalDb.kPrimaryDeviceId) {
debugPrint('[smaq2oss] refusing to sync: the row claims the primary '
'device id — re-pair it with a minted id.');
return false;
}
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if this is wrong, the background and manual sync paths can persist incorrect or unwanted raw notification frames in the archive, and reverting the code would not remove those rows. The stored data is bounded and can be deleted or repaired, so the consequence is recoverable rather than an irreversible external action.
Blocking findings: lib/ble/smaq2oss_link.dart:76
|
Failed to generate code suggestions for PR |
|
Failed to generate code suggestions for PR |
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
Failed to generate code suggestions for PR |
|
@coderabbitai review |
Rate Limit Exceeded
|
|
Failed to generate code suggestions for PR |
|
@coderabbitai review |
|
|
Failed to generate code suggestions for PR |
l is never null in a running app, so l?.devicesSyncNowSub always won and the family branch under it never ran.
|
Failed to generate code suggestions for PR |
1 participant
pairs-only adapter for the SMA-Q2-OSS watch. no signals declared, no commands sent.
Summary by Sourcery
Add experimental SMA-Q2-OSS support that pairs with the watch and archives its notifications as un decoded raw frames.
New Features:
Enhancements:
Tests: