feat(notify): buzz the strap for incoming phone calls - #95
Conversation
The notification relay can never see a ringing phone: dialers post the incoming-call notification as ONGOING (which the relay rightly skips as 'not a ping'), and the dialer itself is a system app the picker hides (installed_apps excludeSystemApps). So calls get their own path — a tiny native telephony bridge (CallStateBridge) streaming ringing/offhook/idle over an EventChannel, registered on the long-lived engine so it works backgrounded, and a CallBuzzer on the Dart side that buzzes the band on a fixed cadence (immediately, then every 4 s, capped at ~one 30 s ring) until the call is answered, declined, or rings out. Needs READ_PHONE_STATE (runtime). Only the call state is read — numbers never are (no READ_CALL_LOG). Android-only like the relay; iOS has no ringer-observation API, so the feature is inert and hidden there. UI: an 'Buzz on incoming calls' card on the Band-notifications screen, independent of the app-relay master toggle, with inline permission grant. Cadence covered by fakeAsync tests. Closes OpenStrap#92
📝 WalkthroughWalkthroughAdds Android phone-state permission and a native telephony bridge, introduces Flutter incoming-call buzzing with persisted settings and bounded haptic cadence, wires it into application state, and adds settings controls plus cadence tests. ChangesIncoming call buzzing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Settings
participant CallBuzzer
participant CallStateBridge
participant TelephonyManager
participant BLEEngine
User->>Settings: Enable incoming-call buzzing
Settings->>CallBuzzer: requestPermission
CallBuzzer->>CallStateBridge: requestPermission
CallStateBridge->>User: Show phone-state permission dialog
User->>CallStateBridge: Grant permission
CallBuzzer->>CallStateBridge: Subscribe to call-state events
CallStateBridge->>TelephonyManager: Register listener
TelephonyManager->>CallStateBridge: Emit ringing
CallStateBridge->>CallBuzzer: Forward ringing
CallBuzzer->>BLEEngine: Buzz on cadence
TelephonyManager->>CallStateBridge: Emit idle/offhook
CallStateBridge->>CallBuzzer: Stop buzzing
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/notify/call_buzzer.dart`:
- Around line 160-177: Update _startRinging so reaching maxBuzzes cancels and
clears only _ringTimer without resetting the ringing state (_buzzCount); reserve
_stopRinging for idle/offhook handling so duplicate ringing events after the cap
do not restart the buzz cadence. Add a regression test covering a duplicate
ringing event after maxBuzzes without an intervening terminal state.
In `@lib/state/app_state.dart`:
- Around line 123-129: Update AppState.dispose() to call callBuzzer.dispose()
and release the CallBuzzer’s observer, subscription, and timer when AppState is
destroyed.
In `@lib/ui/profile/notification_relay_section.dart`:
- Around line 33-44: Update the subtitle branching in the notification relay
status logic to check calls.enabled && !calls.permissionGranted before the
existing calls.active-based statuses, and display “Needs phone permission” for
that case. Preserve the current relay and app-selection statuses when phone
permission is granted or calls are disabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 09218478-ddab-42b4-bec5-712ed9cb6d1e
📒 Files selected for processing (8)
android/app/src/main/AndroidManifest.xmlandroid/app/src/main/kotlin/wtf/openstrap/openstrap_edge/CallStateBridge.ktandroid/app/src/main/kotlin/wtf/openstrap/openstrap_edge/EdgeApplication.ktandroid/app/src/main/kotlin/wtf/openstrap/openstrap_edge/MainActivity.ktlib/notify/call_buzzer.dartlib/state/app_state.dartlib/ui/profile/notification_relay_section.darttest/call_buzzer_test.dart
- Keep the call marked as ringing after the buzz cap, so a duplicate RINGING event for the same call can't start a second cadence; only a terminal state (idle/offhook) clears it. Regression test added. - Dispose the owned CallBuzzer in AppState.dispose(). - Profile card: surface 'Needs phone permission' when the calls toggle is on but the grant is missing, instead of reporting Off.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/call_buzzer_test.dart (1)
67-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider verifying the cap-counting claim in the disconnected-band test.
The test name asserts "ticks count toward the cap but nothing buzzes," but only the "nothing buzzes" half is checked. To fully validate the name, consider also confirming that the internal cap was reached — e.g., after elapsing past the cap while disconnected, reconnect and verify no further ticks fire, or transition to
idlethenringingand confirm a fresh immediate buzz (proving the prior cap was consumed).💚 Suggested addition
test('band disconnected: ticks count toward the cap but nothing buzzes', () { fakeAsync((async) { var buzzes = 0; final cb = make(() => buzzes++, connected: false); cb.handleStateEvent('ringing'); async.elapse(const Duration(minutes: 1)); expect(buzzes, 0, reason: 'no link — no writes'); + // Cap was still counting internally — reconnecting should not produce + // retroactive buzzes, and a fresh ring after idle should reset cleanly. + cb.handleStateEvent('idle'); + final cb2 = make(() => buzzes++, connected: true); + cb2.handleStateEvent('ringing'); + expect(buzzes, 1, reason: 'fresh ring on a new buzzer buzzes immediately'); }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/call_buzzer_test.dart` around lines 67 - 75, Extend the disconnected-band test around make and handleStateEvent to verify that elapsed ticks consume the cap, not just that buzzes remain zero: after advancing beyond the cap while disconnected, reconnect or reset through the existing state transitions and assert behavior proving no further ticks occur until a fresh ringing cycle starts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/call_buzzer_test.dart`:
- Around line 67-75: Extend the disconnected-band test around make and
handleStateEvent to verify that elapsed ticks consume the cap, not just that
buzzes remain zero: after advancing beyond the cap while disconnected, reconnect
or reset through the existing state transitions and assert behavior proving no
further ticks occur until a fresh ringing cycle starts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a8111e6-cf74-42c6-afc4-8c082b10f228
📒 Files selected for processing (4)
lib/notify/call_buzzer.dartlib/state/app_state.dartlib/ui/profile/notification_relay_section.darttest/call_buzzer_test.dart
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/state/app_state.dart
- lib/ui/profile/notification_relay_section.dart
- lib/notify/call_buzzer.dart
|
I actually think the Tasker integration makes this unnecessary now. |
the relay itself never stopped working — app_state still bootstraps it and the manifest still declares BIND_NOTIFICATION_LISTENER_SERVICE for it. what got deleted was every control, so we've been shipping a notification-listener permission with no way to reach the feature it's there for. that's the part that matters: a reviewer reading the manifest sees an unexplained permission. the app list is apps that have actually notified you while the listener was running, not the installed set. enumerating installed apps needs QUERY_ALL_PACKAGES, which the sweep pulled out of the manifest with tools:node=remove and called the most policy-expensive permission there is — that stands. it's also the better list: the dozen apps that interrupt you instead of two hundred to scroll. cost is it starts empty and fills over the first few minutes, which the empty state says out loud. names come off the package (the real label is behind the permission we're not asking for); the icon comes off the notification itself and is the thing you actually recognise. no telephony call-buzz here — pr OpenStrap#95 never merged, there's no READ_PHONE_STATE and nothing in history.
Closes #92.
Why the relay can't do this
Two separate blockers mean incoming calls can never ride the existing notification relay:
InstalledApps.getInstalledApps(excludeSystemApps: true), and the dialer (Google Phone, Samsung Phone, AOSP Dialer) is a system app on essentially every device — so there's no 'Phone' entry to select (exactly what Buzz Notifications - Missing phone call app #92 reports).NotificationRelay._onNotificationskips ongoing notifications (if (e.hasRemoved || e.onGoing) return;) — correctly, to avoid media players and foreground services — and Android posts incoming-call notifications as ongoing full-screen-intent notifications, so ringing is precisely the kind of notification that filter swallows.What this does instead
Calls get their own telephony-based path:
CallStateBridge.kt— a small native bridge streamingringing/offhook/idlefromTelephonyManagerover an EventChannel (TelephonyCallbackon API 31+,PhoneStateListenerbelow). Registered on the long-lived engine alongsideNativeChannels, so it keeps working while backgrounded/headless. Permission request borrows the ActivityCompanionBridgealready tracks; the result is forwarded fromMainActivity.onRequestPermissionsResult.CallBuzzer(Dart) — same persisted-ChangeNotifier idiom asNotificationRelay. While RINGING it buzzes the band immediately and then every 4 s — a cadence on the wrist reads unmistakably as 'your phone', where a single pulse is easy to miss — and stops the instant the call is answered, declined, or rings out. A cap of 8 buzzes (~one standard 30 s ring) guards against OEMs that never emit IDLE after a missed call.Permissions
Adds
READ_PHONE_STATE(runtime, requested only when the toggle is switched on). Only the call state is read — numbers never are (noREAD_CALL_LOG), and the manifest comment says so. iOS is untouched: no ringer-observation API exists there, so the feature is inert and hidden, matching the relay's pattern.Testing
flutter analyzeclean for the touched files;flutter test— the newcall_buzzer_test.dartcovers the cadence (immediate buzz, repeat interval, stop on offhook/idle, stuck-RINGING cap, duplicate-event and re-ring behaviour) underfakeAsync. The two pre-existingderivation_pipeline_test.dartfailures on main are unrelated (git-dep drift) and unaffected.Summary by CodeRabbit