Add advanced repeater settings controls to repeater screen#238
Add advanced repeater settings controls to repeater screen#238Specter242 wants to merge 4 commits intozjs81:mainfrom
Conversation
Add firmware tuning controls to repeater settings screen: - Interference Threshold (int.thresh) with dB input - AGC Reset Interval (agc.reset.interval) in seconds - Flood Max Hops (flood.max) - Multi ACKs toggle (multi.acks) All settings use CLI commands via buildSendCliCommandFrame. Each field has individual refresh and a section-level refresh. Add 15 localization keys for labels, helpers, and tooltips. Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23d043db67
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f38c9f358
ℹ️ 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".
| final intThresh = int.tryParse(_intThreshController.text.trim()); | ||
| if (intThresh != null) { | ||
| commands.add('set int.thresh $intThresh'); |
There was a problem hiding this comment.
_intThreshController and _floodMaxController are initialized with defaults (14 and 4), and _saveSettings unconditionally appends set int.thresh/set flood.max whenever parsing succeeds; this means changing an unrelated field (like name/password) will also overwrite any existing device-specific advanced values if the user has not first refreshed or edited these controls. Please gate these commands behind a hydrated/touched check (as done for multi.acks) or leave the controllers unset until fetched.
Useful? React with 👍 / 👎.
just-stuff-tm
left a comment
just-stuff-tm
left a comment
There was a problem hiding this comment.
Findings
_intThreshControllerdefaults to14and_floodMaxControllerdefaults to4.
_saveSettings()always sends them if they are parseable. Opening settings and saving unrelated fields can unintentionally reset device values.
- Every refresh clears and reuses the single
_fetchedSettingsmap.
If the user triggers multiple refresh actions quickly, one refresh can clear data while another is still collecting.
-
Save-path validation for new numeric advanced fields is weaker than fetch-path validation.
Fetch validation enforces a non-negative integer format, but save only performs
int.tryParseand sends the value directly. Invalid or negative pasted values can still be transmitted.
- Regenerate Localization dart files after running translate.py
Please fix these so upstream can merge... Thank You
| final TextEditingController _agcResetIntervalController = | ||
| TextEditingController(); | ||
| final TextEditingController _floodMaxController = TextEditingController( | ||
| text: '4', |
There was a problem hiding this comment.
Blocking: _saveSettings() always emits set int.thresh and set flood.max because the controllers are pre-seeded (14/4). This can overwrite existing repeater values even when user did not touch advanced settings. Please gate these like multi.acks (hydrate/touched flags) or initialize empty and only send when user changed or after successful fetch.
| "repeater_refreshInterferenceThreshold": "Refresh interference threshold", | ||
| "repeater_refreshAgcResetInterval": "Refresh AGC reset interval", | ||
| "repeater_refreshFloodMaxHops": "Refresh flood max hops", | ||
| "repeater_refreshMultiAcks": "Refresh multi ACKs", |
There was a problem hiding this comment.
Please regenerate locale translations before merge. The generated locale files currently contain English fallback text for new repeater advanced settings keys.
Steps to fix:
- Ensure Ollama is running and pull the expected model:
ollama pull translategemma:latest- Translate missing keys from
app_en.arbinto all locale ARBs:
python3 tools/translate.py --in lib/l10n/app_en.arb --l10n-dir lib/l10n --missing-only- Regenerate localization Dart files:
flutter gen-l10n
|
We need to modify the repeater settings so it only sends modified fields. |
|
I'll work on this in the next few days. I'm on a job for the moment. |
|
Addressed the requested follow-ups in 8954379. Changes included:
Verification:
|
|
The remaining failed check is the workflow's test step, and it appears to be an existing upstream failure rather than something introduced by this branch. Current PR failure:
Same failure is already present on :
So the PR-specific changes are pushed and green on build targets, but the shared TCP test failure is still blocking the check. |
|
The remaining failed check is the Current PR failure:
Same failure is already present on
So the PR-specific changes are pushed and green on build targets, but the shared TCP test failure is still blocking the |
3 participants
Summary
Adds an advanced settings section to the repeater settings screen and wires refresh actions for the new controls.
Scope
lib/screens/repeater_settings_screen.dartlib/l10n/app_en.arblib/l10n/app_localizations*.dartIncluded Controls
Validation
flutter analyze lib/screens/repeater_settings_screen.dart lib/l10n/app_localizations.dart lib/l10n/app_localizations_zh.dart(pass)Issue
Refs #237