feat(app): allow users to rename their Omi device#4749
feat(app): allow users to rename their Omi device#4749eulicesl wants to merge 3 commits intoBasedHardware:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a useful feature for renaming devices. The implementation of the rename dialog and the logic to preserve the custom name on reconnection are well done. However, I've identified a couple of issues that should be addressed:
- Critical Bug: The 'Forget Device' functionality does not clear the custom device name, which is contrary to the feature's description and user expectations. This violates the rule about ensuring UI element actions are consistent with user expectations.
- Code Duplication: The logic for updating the device name on connection is repeated in five different places. This should be refactored into a single helper method to improve maintainability, aligning with the rule to prefer existing helper functions over inlining logic.
My detailed comments provide suggestions on how to fix these issues.
There was a problem hiding this comment.
Pull request overview
Adds end-user device renaming support and attempts to prevent automatic reconnections from overwriting a user-defined device name, with localization updates for the new UI strings.
Changes:
- Add a rename dialog in Device Settings (tap to rename, long-press to copy).
- Guard writes to
SharedPreferencesUtil().deviceNameinDeviceProvider/OnboardingProviderto preserve custom names. - Add new localization keys and regenerate localization outputs.
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| app/lib/providers/onboarding_provider.dart | Adds device-switch detection logic to avoid overwriting a stored custom device name during onboarding connections. |
| app/lib/providers/device_provider.dart | Adds guarded deviceName writes intended to preserve user-defined names across reconnects and reset on device switch. |
| app/lib/pages/settings/device_settings.dart | Adds rename dialog + tap/long-press handling; adjusts disconnect flow to avoid clearing deviceName. |
| app/lib/pages/home/device.dart | Adjusts disconnect flow to avoid clearing deviceName. |
| app/lib/l10n/app_en.arb | Adds 4 new English localization keys for the rename UI. |
| app/lib/l10n/app_localizations.dart | Adds new abstract localization getters and documentation for rename UI strings. |
| app/lib/l10n/app_localizations_ar.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_bg.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_ca.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_cs.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_da.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_de.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_el.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_en.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_es.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_et.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_fi.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_fr.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_hi.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_hu.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_id.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_it.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_ja.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_ko.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_lt.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_lv.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_ms.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_nl.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_no.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_pl.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_pt.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_ro.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_ru.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_sk.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_sv.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_th.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_tr.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_uk.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_vi.dart | Regenerated localization output including new keys. |
| app/lib/l10n/app_localizations_zh.dart | Regenerated localization output including new keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
795b175 to
ea81499
Compare
|
Reviewer context (guideline alignment):
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 345b1dd4ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
fd6c0d0 to
b02097b
Compare
|
Superseding this stale/conflicted PR with a fresh replacement based on current Replacement draft PR: The new PR keeps the scope tight around #2824:
I’m leaving the replacement as draft until CI/build checks pass. |
|
Closing this in favor of #5952. To keep the maintainer review path clean and unambiguous, I refreshed the device-rename work on a current branch and moved it to:
Why supersede this PR:
User-visible feature scope remains the same:
Closing this older thread to remove ambiguity for maintainers. |
|
Hey @eulicesl 👋 Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request. After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:
Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out. Thank you for being part of the Omi community! 💜 |
Closes #2824
Summary
updateDeviceNameOnConnect()intoSharedPreferencesUtilto centralize device-switch detection/name preservation and remove duplicated logicdeviceNameDeviceIdto track which device owns the custom name and avoid stale/overwrittenbtDevice.idedge casesdeviceNameanddeviceNameDeviceId)renameDevice,enterDeviceName,deviceNameCannotBeEmpty,deviceNameUpdatedScope (current)
+602 / -10)app_en.arb+flutter gen-l10ngenerated outputs)context.l10n.*usage and generated APIs in sync for merge/build safety.Demo
api.omiapi.com), so the STAGING banner is expected; rename-device behavior is backend-agnostic.ScreenRecording_03-12-2026.14-28-06_1.mp4
Changes
app/lib/backend/preferences.dartapp/lib/providers/device_provider.dartapp/lib/providers/onboarding_provider.dartapp/lib/pages/settings/device_settings.dartapp/lib/pages/home/device.dartapp/lib/l10n/app_en.arb+ generated localization files