fix(device-config): surface and migrate deprecated Repeater role (#2025) - #2109
fix(device-config): surface and migrate deprecated Repeater role (#2025)#2109bruschill wants to merge 4 commits into
Conversation
…htastic#2025) Existing nodes reporting the deprecated REPEATER role (rawValue 4) had no DeviceRoles case, so DeviceRoles(rawValue: 4) returned nil — the role could not be displayed and the node could not be safely reconfigured. - Add a deprecated .repeater case so reads resolve everywhere (node detail, node list, settings) as "Repeater (Deprecated)". - Add DeviceRoles.isDeprecated; hide deprecated roles from new selection in the Device Config picker, but keep the current role visible so an existing repeater node can be seen and switched to a supported role. - Show a migration hint guiding users to a Router-based role. - Update enum tests for the new case and deprecation flag.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds the deprecated ChangesRepeater role migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ExistingNode
participant DeviceRoles
participant DeviceConfig
participant User
ExistingNode->>DeviceRoles: Reports role raw value 4
DeviceRoles->>DeviceConfig: Decodes as .repeater
DeviceConfig->>DeviceRoles: Filters deprecated roles
DeviceRoles-->>DeviceConfig: Keeps current repeater role visible
DeviceConfig->>User: Shows deprecated warning and router migration guidance
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 |
…ater-role-2025 # Conflicts: # Meshtastic/Resources/docs/index.json
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Meshtastic/Resources/docs/index.json (1)
398-429: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winIndex the repeater migration terms.
The
settingsentry does not containrepeater,deprecated,router, orrole. SinceDocBundle.retrievePagesscores pages only by keyword matches, queries about the newly documented migration can give this page a score of zero. Regenerate the index fromdocs/user/settings.md, retaining these terms within the 30-keyword limit.As per coding guidelines, bundled documentation under
Meshtastic/Resources/docs/**must stay in sync with the source markdown underdocs/.🤖 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/Resources/docs/index.json` around lines 398 - 429, Regenerate the `settings` entry in the bundled docs index from `docs/user/settings.md` so its 30-keyword list includes `repeater`, `deprecated`, `router`, and `role`. Preserve the existing index format and ensure the generated bundle remains synchronized with the source documentation.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@Meshtastic/Resources/docs/index.json`:
- Around line 398-429: Regenerate the `settings` entry in the bundled docs index
from `docs/user/settings.md` so its 30-keyword list includes `repeater`,
`deprecated`, `router`, and `role`. Preserve the existing index format and
ensure the generated bundle remains synchronized with the source documentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 381216ba-0d5a-4694-87f7-78b605d52180
📒 Files selected for processing (5)
Meshtastic/Resources/docs/index.jsonMeshtastic/Resources/docs/markdown/user/settings.mdMeshtastic/Resources/docs/user/settings.htmlMeshtasticTests/LoraDeviceEnumTests.swiftdocs/user/settings.md
🚧 Files skipped from review as they are similar to previous changes (3)
- Meshtastic/Resources/docs/user/settings.html
- docs/user/settings.md
- Meshtastic/Resources/docs/markdown/user/settings.md
…ater-role-2025 # Conflicts: # Meshtastic/Resources/docs/index.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/Resources/docs/index.json`:
- Around line 401-429: Update the Settings entry’s indexed term list to include
repeater, deprecated, and router, while keeping it within the 30-term cap by
removing low-signal terms such as from, only, or later. Preserve the existing
charCount and all other relevant migration terms.
🪄 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: 9cce725b-49fb-4786-963b-8e6439f6a79e
📒 Files selected for processing (5)
Meshtastic/Resources/docs/index.jsonMeshtastic/Resources/docs/markdown/user/settings.mdMeshtastic/Resources/docs/user/settings.htmlMeshtasticTests/LoraDeviceEnumTests.swiftdocs/user/settings.md
🚧 Files skipped from review as they are similar to previous changes (4)
- Meshtastic/Resources/docs/user/settings.html
- docs/user/settings.md
- Meshtastic/Resources/docs/markdown/user/settings.md
- MeshtasticTests/LoraDeviceEnumTests.swift
| "device", | ||
| "from", | ||
| "firmware", | ||
| "device", | ||
| "settings", | ||
| "tag", | ||
| "region", | ||
| "position", | ||
| "mesh", | ||
| "language", | ||
| "nodes", | ||
| "nfc", | ||
| "traffic", | ||
| "translation", | ||
| "set", | ||
| "rate", | ||
| "only", | ||
| "contact", | ||
| "connected", | ||
| "configure", | ||
| "channel", | ||
| "translated", | ||
| "set", | ||
| "only", | ||
| "management", | ||
| "later", | ||
| "ios", | ||
| "hardware", | ||
| "enable", | ||
| "documentation", | ||
| "display", | ||
| "default", | ||
| "connected", | ||
| "unknown", | ||
| "setting", | ||
| "preset", | ||
| "pages", | ||
| "packets" | ||
| "documentation" | ||
| ], | ||
| "charCount": 9637 | ||
| "charCount": 11204 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Index the Repeater migration terms.
The updated Settings entry omits repeater, deprecated, and router, despite documenting that migration. Since this index drives in-app doc retrieval and is capped at 30 terms, replace low-signal terms such as from, only, or later so searches for the deprecated role find the migration guidance.
🤖 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/Resources/docs/index.json` around lines 401 - 429, Update the
Settings entry’s indexed term list to include repeater, deprecated, and router,
while keeping it within the 30-term cap by removing low-signal terms such as
from, only, or later. Preserve the existing charCount and all other relevant
migration terms.
Source: Coding guidelines
…ater-role-2025 # Conflicts: # Meshtastic/Resources/docs/index.json
1 participant
What changed?
Added handling for the deprecated
REPEATERdevice role (Config.DeviceConfig.Role.repeater, rawValue4), which previously had noDeviceRolesenum case — soDeviceRoles(rawValue: 4)returnednileverywhere.DeviceEnums.swift: added a deprecatedcase repeater = 4(name "Repeater (Deprecated)", migration-oriented description,wifi.routericon,protoEnumValue()→.repeater) and a computedvar isDeprecated.DeviceConfig.swift: the role picker now iteratesavailableDeviceRoles=allCases.filter { !$0.isDeprecated || $0.rawValue == deviceRole }, hiding deprecated roles from new selection while keeping the currently-set role visible so an existing repeater node can be seen and reconfigured. Added an orange warning label guiding the user to a Router-based role.docs/user/settings.mdand regenerated the bundled docs.allCases.count11 → 12 in both suites) and added coverage for the.repeatercase /isDeprecated.Fixes #2025
Why did it change?
Existing nodes still reporting
REPEATERcould not be displayed (role showed as "Unknown") and could not be safely reconfigured — and opening their Device Config and saving would crash on the force-unwrap in the save path. This surfaces the role and provides a migration path to a Router-based role, mirroring Android (filtered from new selection, but visible for existing nodes).How is this tested?
EnumComprehensiveTests.swiftandLoraDeviceEnumTests.swift(case count, rawValue,isDeprecated, and that repeater is the only deprecated role).Screenshots/Videos (when applicable)
N/A
Checklist
docs/user/ordocs/developer/, and updated accordingly — added a deprecated-Repeater note todocs/user/settings.mdand regenerated the bundled docs underMeshtastic/Resources/docs/.Summary by CodeRabbit
New Features
Documentation
Tests