Skip to content

Add configurable Omi button actions#7334

Open
rogierx wants to merge 2 commits into
BasedHardware:mainfrom
rogierx:bounty-2825-button-actions
Open

Add configurable Omi button actions#7334
rogierx wants to merge 2 commits into
BasedHardware:mainfrom
rogierx:bounty-2825-button-actions

Conversation

@rogierx
Copy link
Copy Markdown

@rogierx rogierx commented May 16, 2026

Summary

Fixes #2825.

  • Adds configurable single tap, double tap, and triple tap actions for Omi devices.
  • Defaults to single tap = Ask Omi, double tap = pause/resume recording, triple tap = end and process.
  • Lets users map each tap to Ask Omi, End and Process, Pause/Resume, Star, or Off from Device Settings.
  • Adds firmware triple-tap notification state 6 for both DevKit and CV1 button handlers.
  • Keeps legacy long-press/release voice-command handling for older firmware states.

Validation

  • flutter test test/unit/omi_button_action_test.dart passed 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.dart passed locally.
  • git diff --check passed on the final diff.
  • Firmware build was attempted with the repo Docker scripts. ghcr.io/zephyrproject-rtos/ci:latest has an incompatible Zephyr SDK for this NCS version, and the pinned zephyrprojectrtos/ci:v0.26.8-arm64 image gets to Zephyr configuration but Docker Desktop on macOS returns Resource deadlock avoided when 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.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 16, 2026

Greptile Summary

This PR adds configurable single-, double-, and triple-tap actions for Omi devices, backed by a new OmiButtonAction/OmiButtonPress enum model, per-press SharedPreferences keys, and firmware support for a TRIPLE_TAP (state 6) BLE notification on both DevKit and CV1 targets.

  • Flutter: CaptureProvider.streamButton replaces the old per-state if/else with a clean _runButtonAction dispatcher; device_settings.dart expands the single double-tap row into three independently configurable rows with a shared bottom sheet.
  • Firmware: Both button.c files introduce a btn_tap_count accumulator and a post-DOUBLE_TAP_WINDOW resolver that cleanly emits SINGLE_TAP (1), DOUBLE_TAP (2), or TRIPLE_TAP (6) after the multi-tap window expires; legacy long-press/release voice-command states (3/5) are preserved for older firmware.

Confidence Score: 4/5

Safe 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

Filename Overview
app/lib/models/omi_button_action.dart New enums OmiButtonAction and OmiButtonPress cleanly model configurable actions and BLE states; stable integer values, graceful fallback via fromValue/fromState.
app/lib/backend/preferences.dart Adds singleTapAction and tripleTapAction prefs with typed defaults; doubleTapPausesMuting compat shim updated correctly; btDevice null-coalescing drop is safe since getString returns non-nullable String.
app/lib/pages/settings/device_settings.dart Expands one double-tap row into three configurable rows; correctly calls outer setState so chip labels update after selection. '1x'/'3x' chip labels are hardcoded instead of using l10n.
app/lib/providers/capture_provider.dart Button handling refactored into _runButtonAction dispatch; _isProcessingButtonEvent guard preserved; analytics.omiDoubleTap is called for all three tap types, misattributing single- and triple-tap events.
omi/firmware/devkit/src/button.c Adds TRIPLE_TAP (6) state, btn_tap_count-based resolver, and notify_button_state helper; tap detection now waits DOUBLE_TAP_WINDOW before emitting any tap event, trading responsiveness for clean triple-tap disambiguation.
omi/firmware/omi/src/lib/core/button.c Identical triple-tap logic added for the CV1 target; same firmware considerations as devkit/src/button.c apply.
app/test/unit/omi_button_action_test.dart Covers stable enum values, default preference mapping, BLE state to OmiButtonPress mapping, and unknown-value fallback; good regression guard for serialisation stability.

Sequence Diagram

sequenceDiagram
    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
Loading

Comments Outside Diff (1)

  1. app/lib/providers/capture_provider.dart, line 511-543 (link)

    P2 All button actions log under the omiDoubleTap analytics event

    PlatformManager.instance.analytics.omiDoubleTap(...) is now called for actions triggered by single tap and triple tap as well as double tap. For example, triple-tap to endConversation records omiDoubleTap(feature: 'process_conversation') and single-tap to pauseResume records omiDoubleTap(feature: 'mute'). This permanently misattributes analytics events, making it impossible to distinguish which physical button gesture the user performed from the analytics data.

Reviews (1): Last reviewed commit: "Add configurable Omi button actions" | Re-trigger Greptile

Comment on lines +348 to +356
return '3x';
}
}

int _configuredActionValue(OmiButtonPress press) {
final preferences = SharedPreferencesUtil();
return switch (press) {
OmiButtonPress.singleTap => preferences.singleTapAction,
OmiButtonPress.doubleTap => preferences.doubleTapAction,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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)

@rogierx
Copy link
Copy Markdown
Author

rogierx commented May 16, 2026

Addressed the Greptile feedback in d4ce44f: single/triple tap labels now go through l10n, and button analytics now use a generic Omi Button Press event with a press property so single/double/triple gestures are not misattributed as double taps.

@rogierx rogierx force-pushed the bounty-2825-button-actions branch from 371f0df to d4ce44f Compare May 16, 2026 14:35
@rogierx
Copy link
Copy Markdown
Author

rogierx commented May 16, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

increased button functionality + customizable buttons ($500)

1 participant