fix(canned-messages): stop showing/writing deprecated enabled field (#2021)#2112
fix(canned-messages): stop showing/writing deprecated enabled field (#2021)#2112bruschill wants to merge 2 commits into
Conversation
…eshtastic#2021) CannedMessageConfig.enabled (enabled = 9) is deprecated upstream with no successor and removed from active use. The Apple app still showed an "Enabled" toggle, wrote it to the device on save, and persisted the inbound value locally. - Remove the "Enabled" toggle from the Canned Messages settings screen (plus its @State, onChange, and setCannedMessagesValues read). - Stop writing cmc.enabled when saving CannedMessageConfig. - Stop persisting config.enabled in UpdateSwiftData (both insert and update branches); inbound values from older firmware are still tolerated on read. - Retain the CannedMessageConfigEntity.enabled property (annotated) to avoid a SwiftData schema-version change; it is no longer surfaced or written.
bruschill
added
the
skip-docs-check
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe deprecated canned-message ChangesCanned message enabled retirement
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Meshtastic/Views/Settings/Config/Module/CannedMessagesConfig.swift (1)
43-47: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUpdate the user-facing settings documentation.
This removes a general settings control, so
docs/user/settings.mdshould remove the Enabled option and describe the field’s retirement.As per coding guidelines:
Meshtastic/Views/Settings/**/*.swiftrequires updatingdocs/user/settings.mdfor general settings changes.🤖 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 `@Meshtastic/Views/Settings/Config/Module/CannedMessagesConfig.swift` around lines 43 - 47, Update docs/user/settings.md to remove the documented Enabled option for the Canned Messages settings and note that the field has been retired, keeping the documentation aligned with the settings UI change in the CannedMessagesConfig view.Source: Coding guidelines
🧹 Nitpick comments (1)
Meshtastic/Views/Settings/Config/Module/CannedMessagesConfig.swift (1)
179-183: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a wire-level regression test for field omission.
AccessoryManager+ToRadio.swiftsends the entireModuleConfig.CannedMessageConfigobject. This change relies on protobuf serialization omittingenabledwhen it is not assigned; verify the serialized payload does not contain deprecated field 9, preventing older firmware from receiving an unintended value.🤖 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 `@Meshtastic/Views/Settings/Config/Module/CannedMessagesConfig.swift` around lines 179 - 183, Add a wire-level regression test covering the CannedMessageConfig serialization path used by AccessoryManager+ToRadio.swift. Construct the configuration without assigning the deprecated enabled property, serialize the complete ModuleConfig.CannedMessageConfig payload, and assert that protobuf field 9 is absent while the configured fields remain serialized.
🤖 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 `@Meshtastic/Model/ConfigModels.swift`:
- Around line 50-52: Update the enabled property documentation in the model to
state that it is no longer read or actively written by app code while remaining
stored in the SwiftData schema, preserving existing values. Add the
corresponding compatibility and persistence behavior to
docs/developer/swiftdata.md or docs/developer/architecture.md.
---
Outside diff comments:
In `@Meshtastic/Views/Settings/Config/Module/CannedMessagesConfig.swift`:
- Around line 43-47: Update docs/user/settings.md to remove the documented
Enabled option for the Canned Messages settings and note that the field has been
retired, keeping the documentation aligned with the settings UI change in the
CannedMessagesConfig view.
---
Nitpick comments:
In `@Meshtastic/Views/Settings/Config/Module/CannedMessagesConfig.swift`:
- Around line 179-183: Add a wire-level regression test covering the
CannedMessageConfig serialization path used by AccessoryManager+ToRadio.swift.
Construct the configuration without assigning the deprecated enabled property,
serialize the complete ModuleConfig.CannedMessageConfig payload, and assert that
protobuf field 9 is absent while the configured fields remain serialized.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a42fc637-6557-49ab-838f-c27665401894
📒 Files selected for processing (4)
Meshtastic/Model/ConfigModels.swiftMeshtastic/Persistence/CoreDataMigrationService.swiftMeshtastic/Persistence/UpdateSwiftData.swiftMeshtastic/Views/Settings/Config/Module/CannedMessagesConfig.swift
Address CodeRabbit review on meshtastic#2112: - Correct the enabled doc comment: it remains a stored SwiftData property (schema and existing values preserved) but is no longer surfaced, read, or actively written. - Align the deprecation comments in UpdateSwiftData/CoreDataMigrationService to say 'no longer written' rather than 'not persisted'. - Document the retain-for-compatibility pattern in docs/developer/swiftdata.md and regenerate the bundled HTML docs.
1 participant
What changed?
Removed the deprecated
CannedMessageConfig.enabledfield (enabled = 9) from the Canned Messages module config — it is deprecated upstream with no successor and removed from active use.CannedMessagesConfig.swift: removed the "Enabled" toggle from the Options section, plus its@State, its.onChange, and thesetCannedMessagesValues()read. Stopped writingcmc.enabledon save.UpdateSwiftData.swift: stopped persistingconfig.enabledin both the insert and update branches of the canned-message upsert. Inbound values from older firmware are still tolerated on read (the proto struct still decodes field 9), just never surfaced or stored.CoreDataMigrationService.swift: no longer migrates the legacyenabledvalue.ConfigModels.swift: retained theCannedMessageConfigEntity.enabledstored property (annotated as deprecated) to avoid a SwiftData schema-version change — the project uses aVersionedSchema+MeshtasticMigrationPlan, so removing the column would require a new migration stage for zero user benefit. The column is now inert (never surfaced, written, or persisted).Fixes #2021
Why did it change?
enabled = 9is[deprecated = true]inmodule_config.protowith no successor. Current firmware no longer gates the canned-message module on this proto field — enablement is driven by whether canned messages are configured (or a cardkb/matrix input is present). The app should therefore no longer show, write, or persist it.How is this tested?
CannedMessageModule.cppgates the module onsplitConfiguredMessages()/ cardkb / matrix input, not proto field 9 (the old compile-time toggle was removed upstream in firmware #9470). Dropping the write does not disable the module on supported firmware.cannedMessageConfig.enabledafter this change (verified absent from AppIntents, CarPlay, watch, export/import, and tests).hasChanges/save-button behavior verified: still driven by the remainingsendBell/rotary1Enabled/updown1Enabled/input-broker.onChangehandlers; no danglingenabledreference.Screenshots/Videos (when applicable)
N/A — the only visible change is the removal of the deprecated "Enabled" toggle from the Canned Messages settings screen.
Checklist
docs/user/ordocs/developer/. No doc update is needed —docs/user/settings.md's canned-messages entry is a high-level one-liner that does not document the removed toggle (so nothing becomes inaccurate), and theMeshtastic/Model/change is a comment only with no schema change.skip-docs-checklabel applies.Note for reviewers
Sibling of #2022 (which removes the deprecated
allow_input_sourcefrom the same screen). This PR is scoped toenabledonly and intentionally leaves theallowInputSourcesave block untouched. On truly ancient firmware that honored proto field 9 at runtime, the app can no longer toggle the module on/off — this is the intended behavior of #2021 and such firmware is outside the supported window.Summary by CodeRabbit