From 0e6bd743a6ce2da4bedbfd88e914c8c01e7fea4d Mon Sep 17 00:00:00 2001 From: MarekZegare4 Date: Mon, 29 Jun 2026 18:47:51 +0200 Subject: [PATCH] refactor(companion): unify screen save-on-exit via UITask::savePrefsIfDirty() The "_dirty bool, then if (_dirty) the_mesh.savePrefs() on exit" pattern was duplicated across 9 screens with subtle divergence: some cleared the flag after saving, some left it set and relied on onShow() to reset. Replace all 12 exit sites with a single _task->savePrefsIfDirty(flag) helper that saves once only if dirty and always clears the flag, so the "did we touch flash?" answer lives in one place and the reset is consistent. Edit sites still mark the flag manually (inherent to change tracking); only the persist-on-exit boilerplate is centralised. Co-Authored-By: Claude Opus 4.8 --- examples/companion_radio/ui-new/AutoAdvertScreen.h | 2 +- examples/companion_radio/ui-new/BotScreen.h | 2 +- examples/companion_radio/ui-new/DashboardConfigScreen.h | 2 +- examples/companion_radio/ui-new/LiveShareScreen.h | 2 +- examples/companion_radio/ui-new/LocatorScreen.h | 2 +- examples/companion_radio/ui-new/QuickMsgScreen.h | 8 ++------ examples/companion_radio/ui-new/RepeaterScreen.h | 2 +- examples/companion_radio/ui-new/SettingsScreen.h | 2 +- examples/companion_radio/ui-new/TrailScreen.h | 6 +++--- examples/companion_radio/ui-new/UITask.cpp | 7 +++++++ examples/companion_radio/ui-new/UITask.h | 5 +++++ 11 files changed, 24 insertions(+), 16 deletions(-) diff --git a/examples/companion_radio/ui-new/AutoAdvertScreen.h b/examples/companion_radio/ui-new/AutoAdvertScreen.h index 22aa4cc0..87e032fd 100644 --- a/examples/companion_radio/ui-new/AutoAdvertScreen.h +++ b/examples/companion_radio/ui-new/AutoAdvertScreen.h @@ -51,7 +51,7 @@ public: bool handleInput(char c) override { if (c == KEY_CANCEL || c == KEY_CONTEXT_MENU) { - if (_dirty) the_mesh.savePrefs(); + _task->savePrefsIfDirty(_dirty); _task->gotoToolsScreen(); return true; } diff --git a/examples/companion_radio/ui-new/BotScreen.h b/examples/companion_radio/ui-new/BotScreen.h index c43441ea..d8fa90aa 100644 --- a/examples/companion_radio/ui-new/BotScreen.h +++ b/examples/companion_radio/ui-new/BotScreen.h @@ -141,7 +141,7 @@ public: } if (cancel) { - if (_dirty) the_mesh.savePrefs(); + _task->savePrefsIfDirty(_dirty); _task->gotoToolsScreen(); return true; } diff --git a/examples/companion_radio/ui-new/DashboardConfigScreen.h b/examples/companion_radio/ui-new/DashboardConfigScreen.h index 88278d5a..e2f307ae 100644 --- a/examples/companion_radio/ui-new/DashboardConfigScreen.h +++ b/examples/companion_radio/ui-new/DashboardConfigScreen.h @@ -65,7 +65,7 @@ public: bool handleInput(char c) override { if (c == KEY_CANCEL || c == KEY_CONTEXT_MENU) { - if (_dirty) the_mesh.savePrefs(); + _task->savePrefsIfDirty(_dirty); _task->gotoHomeScreen(); return true; } diff --git a/examples/companion_radio/ui-new/LiveShareScreen.h b/examples/companion_radio/ui-new/LiveShareScreen.h index 40e73b33..a7d8cf63 100644 --- a/examples/companion_radio/ui-new/LiveShareScreen.h +++ b/examples/companion_radio/ui-new/LiveShareScreen.h @@ -161,7 +161,7 @@ public: bool handleInput(char c) override { if (c == KEY_CANCEL || c == KEY_CONTEXT_MENU) { - if (_dirty) { the_mesh.savePrefs(); _dirty = false; } + _task->savePrefsIfDirty(_dirty); _task->gotoToolsScreen(); return true; } diff --git a/examples/companion_radio/ui-new/LocatorScreen.h b/examples/companion_radio/ui-new/LocatorScreen.h index 7034b3ce..09b5d65c 100644 --- a/examples/companion_radio/ui-new/LocatorScreen.h +++ b/examples/companion_radio/ui-new/LocatorScreen.h @@ -293,7 +293,7 @@ public: return true; } if (c == KEY_CANCEL || c == KEY_CONTEXT_MENU) { - if (_dirty) { the_mesh.savePrefs(); _dirty = false; } // engine re-seeded per edit + _task->savePrefsIfDirty(_dirty); // engine re-seeded per edit _task->gotoToolsScreen(); return true; } diff --git a/examples/companion_radio/ui-new/QuickMsgScreen.h b/examples/companion_radio/ui-new/QuickMsgScreen.h index 0f8317ea..f3bcc109 100644 --- a/examples/companion_radio/ui-new/QuickMsgScreen.h +++ b/examples/companion_radio/ui-new/QuickMsgScreen.h @@ -1606,9 +1606,7 @@ public: // sel == 1 (Notif) and sel == 2 (Melody): already cycled via LEFT/RIGHT; ENTER just closes. } } - if (res != PopupMenu::NONE && _ctx_dirty) { - the_mesh.savePrefs(); _ctx_dirty = false; - } + if (res != PopupMenu::NONE) _task->savePrefsIfDirty(_ctx_dirty); return true; } if (c == KEY_CANCEL) { _room_mode = false; _phase = MODE_SELECT; return true; } @@ -1718,9 +1716,7 @@ public: } // sel 1/2/3 already handled by LEFT/RIGHT; ENTER just closes. } - if (res != PopupMenu::NONE && _ctx_dirty) { - the_mesh.savePrefs(); _ctx_dirty = false; - } + if (res != PopupMenu::NONE) _task->savePrefsIfDirty(_ctx_dirty); return true; } if (c == KEY_CANCEL) { _phase = MODE_SELECT; return true; } diff --git a/examples/companion_radio/ui-new/RepeaterScreen.h b/examples/companion_radio/ui-new/RepeaterScreen.h index 66619bf7..ba8e5d40 100644 --- a/examples/companion_radio/ui-new/RepeaterScreen.h +++ b/examples/companion_radio/ui-new/RepeaterScreen.h @@ -211,7 +211,7 @@ public: } if (c == KEY_CANCEL || c == KEY_CONTEXT_MENU) { - if (_dirty) the_mesh.savePrefs(); + _task->savePrefsIfDirty(_dirty); _task->gotoToolsScreen(); return true; } diff --git a/examples/companion_radio/ui-new/SettingsScreen.h b/examples/companion_radio/ui-new/SettingsScreen.h index 32cb17b4..e6405ec7 100644 --- a/examples/companion_radio/ui-new/SettingsScreen.h +++ b/examples/companion_radio/ui-new/SettingsScreen.h @@ -754,7 +754,7 @@ public: } if (c == KEY_CANCEL) { - if (_dirty) the_mesh.savePrefs(); + _task->savePrefsIfDirty(_dirty); _task->gotoHomeScreen(); return true; } diff --git a/examples/companion_radio/ui-new/TrailScreen.h b/examples/companion_radio/ui-new/TrailScreen.h index 15709c82..543682d9 100644 --- a/examples/companion_radio/ui-new/TrailScreen.h +++ b/examples/companion_radio/ui-new/TrailScreen.h @@ -192,16 +192,16 @@ public: case ACT_EXPORT: handleExport(); break; case ACT_EXPORT_SAVED: handleExportSaved(); break; } - if (_cfg_dirty) { the_mesh.savePrefs(); _cfg_dirty = false; } + _task->savePrefsIfDirty(_cfg_dirty); } else if (res == PopupMenu::CANCELLED) { - if (_cfg_dirty) { the_mesh.savePrefs(); _cfg_dirty = false; } + _task->savePrefsIfDirty(_cfg_dirty); if (_menu_level != ML_MAIN) buildMainMenu(); // Cancel backs out of a submenu } return true; } if (c == KEY_CANCEL) { - if (_cfg_dirty) { the_mesh.savePrefs(); _cfg_dirty = false; } + _task->savePrefsIfDirty(_cfg_dirty); if (_return_home) _task->gotoHomeScreen(); else _task->gotoToolsScreen(); return true; diff --git a/examples/companion_radio/ui-new/UITask.cpp b/examples/companion_radio/ui-new/UITask.cpp index e848d171..d17a813e 100644 --- a/examples/companion_radio/ui-new/UITask.cpp +++ b/examples/companion_radio/ui-new/UITask.cpp @@ -1758,6 +1758,13 @@ void UITask::setCurrScreen(UIScreen* c) { _next_refresh = 100; } +bool UITask::savePrefsIfDirty(bool& dirty) { + if (!dirty) return false; + the_mesh.savePrefs(); + dirty = false; + return true; +} + /* hardware-agnostic pre-shutdown activity should be done here */ diff --git a/examples/companion_radio/ui-new/UITask.h b/examples/companion_radio/ui-new/UITask.h index 510f30d3..00514644 100644 --- a/examples/companion_radio/ui-new/UITask.h +++ b/examples/companion_radio/ui-new/UITask.h @@ -355,6 +355,11 @@ public: void applyPowerSave(); // hardware duty-cycle RX on/off from prefs void applyApc(); // Adaptive Power Control on/off from prefs void applyRadioParams(); // freq/bw/sf/cr from prefs (radio preset change) + // Save-on-exit helper for the screen `_dirty` pattern: persists NodePrefs once + // only if `dirty`, then clears the flag. Standardises the screens' exit paths + // (some used to leave the flag set, relying on onShow() to reset it) and keeps + // the "did we touch flash?" answer in one place. Returns whether it saved. + bool savePrefsIfDirty(bool& dirty); void applyFont(); void applyRotation(); void applyFullRefreshInterval();