Add configurable Omi button actions#7334
Conversation
Greptile SummaryThis PR adds configurable single-, double-, and triple-tap actions for Omi devices, backed by a new
Confidence Score: 4/5Safe to merge; the core tap-dispatch logic, preference storage, and firmware resolver are sound with two non-blocking style issues. The button action dispatch, preference serialisation stability, and firmware tap-count accumulator are all well-structured and tested. The two findings are cosmetic/observability issues: '1x'/'3x' labels bypass the l10n system, and every action regardless of which physical button was tapped is logged under the omiDoubleTap analytics event, making per-tap breakdown impossible in analytics. Neither issue breaks functionality. app/lib/pages/settings/device_settings.dart (hardcoded l10n strings) and app/lib/providers/capture_provider.dart (analytics event misattribution) Important Files Changed
Sequence DiagramsequenceDiagram
participant FW as Firmware (button.c)
participant BLE as BLE GATT Notify
participant CP as CaptureProvider
participant Prefs as SharedPreferences
participant UI as Device UI
FW->>FW: "Accumulate btn_tap_count on each release < TAP_THRESHOLD"
FW->>FW: Wait DOUBLE_TAP_WINDOW (600 ms) after last tap
FW->>BLE: notify state 1 / 2 / 6
BLE->>CP: onButtonReceived(value)
CP->>CP: OmiButtonPress.fromState(buttonState)
CP->>Prefs: read singleTapAction / doubleTapAction / tripleTapAction
CP->>CP: _runButtonAction(action, deviceId, source)
alt askQuestion
CP->>CP: _toggleVoiceQuestionSession()
else pauseResume
CP->>CP: pauseDeviceRecording() / resumeDeviceRecording()
else starConversation
CP->>CP: markConversationForStarring()
else endConversation
CP->>CP: forceProcessingCurrentConversation()
else noAction
CP->>CP: log and return
end
UI->>Prefs: read tap action prefs
UI->>UI: _buildCustomizationSection() shows chip labels
|
| return '3x'; | ||
| } | ||
| } | ||
|
|
||
| int _configuredActionValue(OmiButtonPress press) { | ||
| final preferences = SharedPreferencesUtil(); | ||
| return switch (press) { | ||
| OmiButtonPress.singleTap => preferences.singleTapAction, | ||
| OmiButtonPress.doubleTap => preferences.doubleTapAction, |
There was a problem hiding this comment.
Hardcoded user-facing strings bypass l10n
'1x' and '3x' are shown directly in the UI as both the chip label on the settings page and as the sheet title in _showButtonActionSheet, without going through context.l10n. The adjacent OmiButtonPress.doubleTap case already uses context.l10n.doubleTap, so this inconsistency means only the single- and triple-tap labels will not be translated. New l10n keys (e.g. singleTap, tripleTap) should be added and used here.
Context Used: Flutter localization - all user-facing strings mus... (source)
|
Addressed the Greptile feedback in d4ce44f: single/triple tap labels now go through l10n, and button analytics now use a generic |
371f0df to
d4ce44f
Compare
|
Quick status update for maintainer review: the Greptile feedback has been addressed in d4ce44f, the PR branch is mergeable, and GitHub reports no checks on the branch.\n\nCould @aaravgarg or @beastoin review/test this when you have a chance? I left maintainer edits enabled, and the only remaining blocker I can see is maintainer review/hardware validation. |
Summary
Fixes #2825.
6for both DevKit and CV1 button handlers.Validation
flutter test test/unit/omi_button_action_test.dartpassed locally.flutter analyze lib/models/omi_button_action.dart lib/backend/preferences.dart lib/providers/capture_provider.dart lib/pages/settings/device_settings.dart test/unit/omi_button_action_test.dartpassed locally.git diff --checkpassed on the final diff.ghcr.io/zephyrproject-rtos/ci:latesthas an incompatible Zephyr SDK for this NCS version, and the pinnedzephyrprojectrtos/ci:v0.26.8-arm64image gets to Zephyr configuration but Docker Desktop on macOS returnsResource deadlock avoidedwhen reading mounted firmware source files. This should still be validated on maintainer hardware/build environment.Notes
The firmware tap resolver intentionally waits for the multi-tap window before emitting single/double/triple tap events so triple tap can be distinguished cleanly.