From 3b8b389408d6967aee71773f0312be816134f986 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 15:38:37 +0200 Subject: [PATCH 01/10] Refactor Settings usage --- src/SettingsList.h | 16 ++-- src/activities/reader/EpubReaderActivity.cpp | 4 +- .../reader/EpubReaderMenuActivity.cpp | 49 +++++++---- .../settings/ClockSettingsActivity.cpp | 1 + .../settings/KOReaderSettingsActivity.cpp | 5 +- src/activities/settings/SettingInfo.cpp | 10 +-- src/activities/settings/SettingInfo.h | 61 +++++++++---- src/activities/settings/SettingsActivity.cpp | 88 +++++++++++-------- .../weather/WeatherSettingsActivity.cpp | 1 + src/network/CrossPointWebServer.cpp | 8 +- 10 files changed, 151 insertions(+), 92 deletions(-) diff --git a/src/SettingsList.h b/src/SettingsList.h index 439539ca..071c28db 100644 --- a/src/SettingsList.h +++ b/src/SettingsList.h @@ -151,22 +151,22 @@ inline const std::vector list = { // --- KOReader Sync (web-only, uses KOReaderCredentialStore) --- SettingInfo::DynamicString( - StrId::STR_SYNC_SERVER_URL, [] { return KOREADER_STORE.getServerUrl(); }, - [](const std::string& v) { + StrId::STR_SYNC_SERVER_URL, [](void*) { return KOREADER_STORE.getServerUrl(); }, + [](void*, const std::string& v) { KOREADER_STORE.setServerUrl(v); KOREADER_STORE.saveToFile(); }, "koServerUrl", StrId::STR_KOREADER_SYNC), SettingInfo::DynamicString( - StrId::STR_KOREADER_USERNAME, [] { return KOREADER_STORE.getUsername(); }, - [](const std::string& v) { + StrId::STR_KOREADER_USERNAME, [](void*) { return KOREADER_STORE.getUsername(); }, + [](void*, const std::string& v) { KOREADER_STORE.setCredentials(v, KOREADER_STORE.getPassword()); KOREADER_STORE.saveToFile(); }, "koUsername", StrId::STR_KOREADER_SYNC), SettingInfo::DynamicString( - StrId::STR_KOREADER_PASSWORD, [] { return KOREADER_STORE.getPassword(); }, - [](const std::string& v) { + StrId::STR_KOREADER_PASSWORD, [](void*) { return KOREADER_STORE.getPassword(); }, + [](void*, const std::string& v) { KOREADER_STORE.setCredentials(KOREADER_STORE.getUsername(), v); KOREADER_STORE.saveToFile(); }, @@ -174,8 +174,8 @@ inline const std::vector list = { .withObfuscated(), SettingInfo::DynamicEnum( StrId::STR_DOCUMENT_MATCHING, {StrId::STR_FILENAME, StrId::STR_BINARY}, - [] { return static_cast(KOREADER_STORE.getMatchMethod()); }, - [](uint8_t v) { + [](void*) { return static_cast(KOREADER_STORE.getMatchMethod()); }, + [](void*, uint8_t v) { KOREADER_STORE.setMatchMethod(static_cast(v)); KOREADER_STORE.saveToFile(); }, diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 5490e76e..30133953 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -33,7 +33,7 @@ namespace { // pagesPerRefresh now comes from SETTINGS.getRefreshFrequency() constexpr unsigned long skipChapterMs = 700; // pages per minute, first item is 1 to prevent division by zero if accessed -const std::vector PAGE_TURN_LABELS = {1, 1, 3, 6, 12}; +constexpr int PAGE_TURN_LABELS[] = {1, 1, 3, 6, 12}; void logReaderMemSnapshot(const char* stage) { const uint32_t freeHeap = esp_get_free_heap_size(); @@ -756,7 +756,7 @@ void EpubReaderActivity::applyTextDarkness(const uint8_t textDarkness) { } void EpubReaderActivity::toggleAutoPageTurn(const uint8_t selectedPageTurnOption) { - if (selectedPageTurnOption == 0 || selectedPageTurnOption >= PAGE_TURN_LABELS.size()) { + if (selectedPageTurnOption == 0 || selectedPageTurnOption >= std::size(PAGE_TURN_LABELS)) { automaticPageTurnActive = false; return; } diff --git a/src/activities/reader/EpubReaderMenuActivity.cpp b/src/activities/reader/EpubReaderMenuActivity.cpp index d6a3610a..7d7558f4 100644 --- a/src/activities/reader/EpubReaderMenuActivity.cpp +++ b/src/activities/reader/EpubReaderMenuActivity.cpp @@ -50,44 +50,57 @@ void EpubReaderMenuActivity::buildMenuItems(bool hasFootnotes, bool hasStarredPa // --- Appearance --- menuItems.push_back(SettingInfo::Separator(StrId::STR_READER_APPEARANCE)); + auto* self = this; + // Embedded style: cycles default(-1) -> ON(1) -> OFF(0) via DynamicEnum indices 0/1/2 - menuItems.push_back(SettingInfo::DynamicEnum( - StrId::STR_EMBEDDED_STYLE, {StrId::STR_DEFAULT_VALUE, StrId::STR_STATE_ON, StrId::STR_STATE_OFF}, - [this]() -> uint8_t { - if (pendingEmbeddedStyleOverride < 0) return 0; - if (pendingEmbeddedStyleOverride > 0) return 1; + menuItems.push_back(SettingInfo::DynamicEnumCtx( + StrId::STR_EMBEDDED_STYLE, {StrId::STR_DEFAULT_VALUE, StrId::STR_STATE_ON, StrId::STR_STATE_OFF}, self, + [](void* ctx) -> uint8_t { + auto* s = static_cast(ctx); + if (s->pendingEmbeddedStyleOverride < 0) return 0; + if (s->pendingEmbeddedStyleOverride > 0) return 1; return 2; }, - [this](uint8_t v) { + [](void* ctx, uint8_t v) { + auto* s = static_cast(ctx); if (v == 0) - pendingEmbeddedStyleOverride = -1; + s->pendingEmbeddedStyleOverride = -1; else if (v == 1) - pendingEmbeddedStyleOverride = 1; + s->pendingEmbeddedStyleOverride = 1; else - pendingEmbeddedStyleOverride = 0; + s->pendingEmbeddedStyleOverride = 0; })); // Image rendering: cycles default(-1) -> display(0) -> placeholder(1) -> suppress(2) - menuItems.push_back(SettingInfo::DynamicEnum( + menuItems.push_back(SettingInfo::DynamicEnumCtx( StrId::STR_IMAGES, {StrId::STR_DEFAULT_VALUE, StrId::STR_IMAGES_DISPLAY, StrId::STR_IMAGES_PLACEHOLDER, StrId::STR_IMAGES_SUPPRESS}, - [this]() -> uint8_t { return (pendingImageRenderingOverride < 0) ? 0 : (pendingImageRenderingOverride + 1); }, - [this](uint8_t v) { pendingImageRenderingOverride = (v == 0) ? -1 : static_cast(v - 1); })); + self, + [](void* ctx) -> uint8_t { + auto* s = static_cast(ctx); + return (s->pendingImageRenderingOverride < 0) ? 0 : (s->pendingImageRenderingOverride + 1); + }, + [](void* ctx, uint8_t v) { + auto* s = static_cast(ctx); + s->pendingImageRenderingOverride = (v == 0) ? -1 : static_cast(v - 1); + })); // Text darkness: straightforward 0-3 cycle - menuItems.push_back(SettingInfo::DynamicEnum( - StrId::STR_TEXT_DARKNESS, {StrId::STR_NORMAL, StrId::STR_DARK, StrId::STR_EXTRA_DARK, StrId::STR_MAX_DARK}, - [this]() -> uint8_t { return pendingTextDarkness; }, [this](uint8_t v) { pendingTextDarkness = v; })); + menuItems.push_back(SettingInfo::DynamicEnumCtx( + StrId::STR_TEXT_DARKNESS, {StrId::STR_NORMAL, StrId::STR_DARK, StrId::STR_EXTRA_DARK, StrId::STR_MAX_DARK}, self, + [](void* ctx) -> uint8_t { return static_cast(ctx)->pendingTextDarkness; }, + [](void* ctx, uint8_t v) { static_cast(ctx)->pendingTextDarkness = v; })); // Helper functions, reading ruler, auto page turn, orientation menuItems.push_back(SettingInfo::Separator(StrId::STR_READER_UTILS)); // Auto page turn: ACTION type with custom cycling in onActionSelected menuItems.push_back(SettingInfo::Action(StrId::STR_AUTO_TURN_PAGES_PER_MIN, SettingAction::None)); // Orientation: straightforward 0-3 cycle - menuItems.push_back(SettingInfo::DynamicEnum( + menuItems.push_back(SettingInfo::DynamicEnumCtx( StrId::STR_ORIENTATION, - {StrId::STR_PORTRAIT, StrId::STR_LANDSCAPE_CW, StrId::STR_INVERTED, StrId::STR_LANDSCAPE_CCW}, - [this]() -> uint8_t { return pendingOrientation; }, [this](uint8_t v) { pendingOrientation = v; })); + {StrId::STR_PORTRAIT, StrId::STR_LANDSCAPE_CW, StrId::STR_INVERTED, StrId::STR_LANDSCAPE_CCW}, self, + [](void* ctx) -> uint8_t { return static_cast(ctx)->pendingOrientation; }, + [](void* ctx, uint8_t v) { static_cast(ctx)->pendingOrientation = v; })); // --- Synchronisation (only if credentials are set) --- if (KOREADER_STORE.hasCredentials()) { diff --git a/src/activities/settings/ClockSettingsActivity.cpp b/src/activities/settings/ClockSettingsActivity.cpp index 854b2347..ca9c2aeb 100644 --- a/src/activities/settings/ClockSettingsActivity.cpp +++ b/src/activities/settings/ClockSettingsActivity.cpp @@ -19,6 +19,7 @@ const StrId timeZoneNames[CrossPointSettings::TIMEZONE_COUNT] = { } // namespace void ClockSettingsActivity::buildMenuItems() { + menuItems.reserve(6); menuItems.push_back(SettingInfo::Separator(StrId::STR_SETTINGS_TITLE)); menuItems.push_back( SettingInfo::Toggle(StrId::STR_USE_CLOCK, &CrossPointSettings::useClock, "useClock", StrId::STR_CAT_SYSTEM)); diff --git a/src/activities/settings/KOReaderSettingsActivity.cpp b/src/activities/settings/KOReaderSettingsActivity.cpp index 49f8ab61..ff56e73d 100644 --- a/src/activities/settings/KOReaderSettingsActivity.cpp +++ b/src/activities/settings/KOReaderSettingsActivity.cpp @@ -16,6 +16,7 @@ KOReaderSettingsActivity::KOReaderSettingsActivity(GfxRenderer& renderer, Mapped } void KOReaderSettingsActivity::buildMenuItems() { + menuItems.reserve(6); // Username, Password, Server URL: ACTION items with custom value display menuItems.push_back(SettingInfo::Action(StrId::STR_SYNC_SERVER_URL, SettingAction::None) .withSubcategory(StrId::STR_MENU_KOSYNC_SERVER)); @@ -25,8 +26,8 @@ void KOReaderSettingsActivity::buildMenuItems() { // Document matching: DynamicEnum toggling between Filename and Binary menuItems.push_back(SettingInfo::DynamicEnum( StrId::STR_DOCUMENT_MATCHING, {StrId::STR_FILENAME, StrId::STR_BINARY}, - [] { return static_cast(KOREADER_STORE.getMatchMethod()); }, - [](uint8_t v) { + [](void*) { return static_cast(KOREADER_STORE.getMatchMethod()); }, + [](void*, uint8_t v) { KOREADER_STORE.setMatchMethod(static_cast(v)); KOREADER_STORE.saveToFile(); })); diff --git a/src/activities/settings/SettingInfo.cpp b/src/activities/settings/SettingInfo.cpp index 7525c1ff..f7f40825 100644 --- a/src/activities/settings/SettingInfo.cpp +++ b/src/activities/settings/SettingInfo.cpp @@ -19,7 +19,7 @@ std::string SettingInfo::getDisplayValue() const { if (valuePtr) value = SETTINGS.*(valuePtr); else if (valueGetter) - value = valueGetter(); + value = callValueGetter(); else return {}; return std::string(value ? tr(STR_STATE_ON) : tr(STR_STATE_OFF)); @@ -29,7 +29,7 @@ std::string SettingInfo::getDisplayValue() const { if (valuePtr) value = SETTINGS.*(valuePtr); else if (valueGetter) - value = valueGetter(); + value = callValueGetter(); else return {}; if (value < enumValues.size()) return std::string(I18N.get(enumValues[value])); @@ -37,7 +37,7 @@ std::string SettingInfo::getDisplayValue() const { } case SettingType::VALUE: { if (valuePtr) return std::to_string(SETTINGS.*(valuePtr)); - if (valueGetter) return std::to_string(valueGetter()); + if (valueGetter) return std::to_string(callValueGetter()); return {}; } case SettingType::ACTION: @@ -56,7 +56,7 @@ void SettingInfo::toggleValue() const { if (valuePtr) { SETTINGS.*(valuePtr) = !(SETTINGS.*(valuePtr)); } else if (valueGetter && valueSetter) { - valueSetter(!valueGetter()); + callValueSetter(!callValueGetter()); } break; @@ -66,7 +66,7 @@ void SettingInfo::toggleValue() const { if (valuePtr) { SETTINGS.*(valuePtr) = (SETTINGS.*(valuePtr) + 1) % count; } else if (valueGetter && valueSetter) { - valueSetter((valueGetter() + 1) % count); + callValueSetter((callValueGetter() + 1) % count); } break; } diff --git a/src/activities/settings/SettingInfo.h b/src/activities/settings/SettingInfo.h index 108a6e34..498cbb6d 100644 --- a/src/activities/settings/SettingInfo.h +++ b/src/activities/settings/SettingInfo.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include @@ -49,11 +48,25 @@ struct SettingInfo { size_t stringOffset = 0; size_t stringMaxLen = 0; - // Dynamic accessors (for settings stored outside CrossPointSettings, e.g. KOReaderCredentialStore) - std::function valueGetter; - std::function valueSetter; - std::function stringGetter; - std::function stringSetter; + // Dynamic accessors (for settings stored outside CrossPointSettings, e.g. KOReaderCredentialStore). + // Function pointers + opaque context avoid the heap allocation of std::function. Stateless + // lambdas pass ctx=nullptr; captures must be hand-written as trampoline functions. See + // DynamicEnumCtx / DynamicStringCtx factories below. + using ValueGetterFn = uint8_t (*)(void*); + using ValueSetterFn = void (*)(void*, uint8_t); + using StringGetterFn = std::string (*)(void*); + using StringSetterFn = void (*)(void*, const std::string&); + + void* accessorCtx = nullptr; + ValueGetterFn valueGetter = nullptr; + ValueSetterFn valueSetter = nullptr; + StringGetterFn stringGetter = nullptr; + StringSetterFn stringSetter = nullptr; + + uint8_t callValueGetter() const { return valueGetter(accessorCtx); } + void callValueSetter(uint8_t v) const { valueSetter(accessorCtx, v); } + std::string callStringGetter() const { return stringGetter(accessorCtx); } + void callStringSetter(const std::string& v) const { stringSetter(accessorCtx, v); } SettingInfo& withObfuscated() { obfuscated = true; @@ -115,33 +128,49 @@ struct SettingInfo { return s; } - static SettingInfo DynamicEnum(StrId nameId, std::vector values, std::function getter, - std::function setter, const char* key = nullptr, - StrId category = StrId::STR_NONE_OPT) { + // Stateless variant — getter/setter are free/static functions with no captured state. + static SettingInfo DynamicEnum(StrId nameId, std::vector values, ValueGetterFn getter, ValueSetterFn setter, + const char* key = nullptr, StrId category = StrId::STR_NONE_OPT) { SettingInfo s; s.nameId = nameId; s.type = SettingType::ENUM; s.enumValues = std::move(values); - s.valueGetter = std::move(getter); - s.valueSetter = std::move(setter); + s.valueGetter = getter; + s.valueSetter = setter; s.key = key; s.category = category; return s; } - static SettingInfo DynamicString(StrId nameId, std::function getter, - std::function setter, const char* key = nullptr, - StrId category = StrId::STR_NONE_OPT) { + // Context-carrying variant — trampolines receive `ctx` as first argument and cast it back to + // their concrete owner type. + static SettingInfo DynamicEnumCtx(StrId nameId, std::vector values, void* ctx, ValueGetterFn getter, + ValueSetterFn setter, const char* key = nullptr, + StrId category = StrId::STR_NONE_OPT) { + SettingInfo s = DynamicEnum(nameId, std::move(values), getter, setter, key, category); + s.accessorCtx = ctx; + return s; + } + + static SettingInfo DynamicString(StrId nameId, StringGetterFn getter, StringSetterFn setter, + const char* key = nullptr, StrId category = StrId::STR_NONE_OPT) { SettingInfo s; s.nameId = nameId; s.type = SettingType::STRING; - s.stringGetter = std::move(getter); - s.stringSetter = std::move(setter); + s.stringGetter = getter; + s.stringSetter = setter; s.key = key; s.category = category; return s; } + static SettingInfo DynamicStringCtx(StrId nameId, void* ctx, StringGetterFn getter, StringSetterFn setter, + const char* key = nullptr, StrId category = StrId::STR_NONE_OPT) { + SettingInfo s = DynamicString(nameId, getter, setter, key, category); + s.accessorCtx = ctx; + return s; + } + static SettingInfo Separator(StrId nameId) { SettingInfo s; s.nameId = nameId; diff --git a/src/activities/settings/SettingsActivity.cpp b/src/activities/settings/SettingsActivity.cpp index 37f74be7..91b9051d 100644 --- a/src/activities/settings/SettingsActivity.cpp +++ b/src/activities/settings/SettingsActivity.cpp @@ -30,16 +30,22 @@ void SettingsActivity::onEnter() { controlsSettings.clear(); systemSettings.clear(); submenuData.clear(); + displaySettings.reserve(20); + readerSettings.reserve(30); + controlsSettings.reserve(8); + systemSettings.reserve(20); + submenuData.reserve(4); StrId lastDisplaySub = StrId::STR_NONE_OPT; StrId lastReaderSub = StrId::STR_NONE_OPT; StrId lastControlsSub = StrId::STR_NONE_OPT; StrId lastSystemSub = StrId::STR_NONE_OPT; - auto addTo = [this](std::vector& vec, StrId& lastSub, SettingInfo s) { + // Shared placement logic — locates submenu target or inserts separator. + // Returns the vector the caller should push into (either `vec` or a submenu's items). + auto locateTarget = [this](std::vector& vec, StrId& lastSub, + const SettingInfo& s) -> std::vector* { if (s.submenu != StrId::STR_NONE_OPT) { - // Item belongs to a submenu — collect it and insert a placeholder in the main - // list the first time this submenu ID is encountered. auto it = std::find_if(submenuData.begin(), submenuData.end(), [&s](const SubmenuData& d) { return d.id == s.submenu; }); if (it == submenuData.end()) { @@ -47,14 +53,21 @@ void SettingsActivity::onEnter() { submenuData.push_back({s.submenu, {}}); it = submenuData.end() - 1; } - it->items.push_back(std::move(s)); - return; + return &it->items; } if (s.subcategory != StrId::STR_NONE_OPT && s.subcategory != lastSub) { vec.push_back(SettingInfo::Separator(s.subcategory)); lastSub = s.subcategory; } - vec.push_back(std::move(s)); + return &vec; + }; + + auto addTo = [&locateTarget](std::vector& vec, StrId& lastSub, const SettingInfo& s) { + locateTarget(vec, lastSub, s)->push_back(s); + }; + auto addToMoved = [&locateTarget](std::vector& vec, StrId& lastSub, SettingInfo&& s) { + auto* target = locateTarget(vec, lastSub, s); + target->push_back(std::move(s)); }; for (const auto& setting : getSettingsList()) { @@ -80,34 +93,34 @@ void SettingsActivity::onEnter() { controlsSettings.insert(controlsSettings.begin(), SettingInfo::Action(StrId::STR_REMAP_FRONT_BUTTONS, SettingAction::RemapFrontButtons)); - addTo(readerSettings, lastReaderSub, - SettingInfo::Action(StrId::STR_CUSTOMISE_STATUS_BAR, SettingAction::CustomiseStatusBar)); + addToMoved(readerSettings, lastReaderSub, + SettingInfo::Action(StrId::STR_CUSTOMISE_STATUS_BAR, SettingAction::CustomiseStatusBar)); - addTo(systemSettings, lastSystemSub, SettingInfo::Action(StrId::STR_LANGUAGE, SettingAction::Language)); - addTo(systemSettings, lastSystemSub, - SettingInfo::Action(StrId::STR_WIFI_NETWORKS, SettingAction::Network) - .withSubcategory(StrId::STR_MENU_SYS_NETWORK)); - addTo(systemSettings, lastSystemSub, - SettingInfo::Action(StrId::STR_KOREADER_SYNC, SettingAction::KOReaderSync) - .withSubcategory(StrId::STR_MENU_SYS_NETWORK)); - addTo(systemSettings, lastSystemSub, - SettingInfo::Action(StrId::STR_OPDS_BROWSER, SettingAction::OPDSBrowser) - .withSubcategory(StrId::STR_MENU_SYS_NETWORK)); - addTo(systemSettings, lastSystemSub, - SettingInfo::Action(StrId::STR_CLOCK_SETTINGS, SettingAction::ClockSettings) - .withSubcategory(StrId::STR_MENU_SYS_TOOLS)); - addTo(systemSettings, lastSystemSub, - SettingInfo::Action(StrId::STR_WEATHER_SETTINGS, SettingAction::Weather) - .withSubcategory(StrId::STR_MENU_SYS_TOOLS)); - addTo(systemSettings, lastSystemSub, - SettingInfo::Action(StrId::STR_CLEAR_READING_CACHE, SettingAction::ClearCache) - .withSubcategory(StrId::STR_MENU_SYS_SYSTEM)); - addTo(systemSettings, lastSystemSub, - SettingInfo::Action(StrId::STR_CHECK_UPDATES, SettingAction::CheckForUpdates) - .withSubcategory(StrId::STR_MENU_SYS_SYSTEM)); - addTo(systemSettings, lastSystemSub, - SettingInfo::Action(StrId::STR_SYSTEM_INFO, SettingAction::SystemInfo) - .withSubcategory(StrId::STR_MENU_SYS_SYSTEM)); + addToMoved(systemSettings, lastSystemSub, SettingInfo::Action(StrId::STR_LANGUAGE, SettingAction::Language)); + addToMoved(systemSettings, lastSystemSub, + std::move(SettingInfo::Action(StrId::STR_WIFI_NETWORKS, SettingAction::Network) + .withSubcategory(StrId::STR_MENU_SYS_NETWORK))); + addToMoved(systemSettings, lastSystemSub, + std::move(SettingInfo::Action(StrId::STR_KOREADER_SYNC, SettingAction::KOReaderSync) + .withSubcategory(StrId::STR_MENU_SYS_NETWORK))); + addToMoved(systemSettings, lastSystemSub, + std::move(SettingInfo::Action(StrId::STR_OPDS_BROWSER, SettingAction::OPDSBrowser) + .withSubcategory(StrId::STR_MENU_SYS_NETWORK))); + addToMoved(systemSettings, lastSystemSub, + std::move(SettingInfo::Action(StrId::STR_CLOCK_SETTINGS, SettingAction::ClockSettings) + .withSubcategory(StrId::STR_MENU_SYS_TOOLS))); + addToMoved(systemSettings, lastSystemSub, + std::move(SettingInfo::Action(StrId::STR_WEATHER_SETTINGS, SettingAction::Weather) + .withSubcategory(StrId::STR_MENU_SYS_TOOLS))); + addToMoved(systemSettings, lastSystemSub, + std::move(SettingInfo::Action(StrId::STR_CLEAR_READING_CACHE, SettingAction::ClearCache) + .withSubcategory(StrId::STR_MENU_SYS_SYSTEM))); + addToMoved(systemSettings, lastSystemSub, + std::move(SettingInfo::Action(StrId::STR_CHECK_UPDATES, SettingAction::CheckForUpdates) + .withSubcategory(StrId::STR_MENU_SYS_SYSTEM))); + addToMoved(systemSettings, lastSystemSub, + std::move(SettingInfo::Action(StrId::STR_SYSTEM_INFO, SettingAction::SystemInfo) + .withSubcategory(StrId::STR_MENU_SYS_SYSTEM))); // Reset selection to first category selectedCategoryIndex = 0; @@ -212,11 +225,12 @@ void SettingsActivity::toggleCurrentSetting() { auto resultHandler = [this](const ActivityResult&) { SETTINGS.saveToFile(); }; if (setting.action == SettingAction::Submenu) { - const auto it = std::find_if(submenuData.cbegin(), submenuData.cend(), - [&setting](const SubmenuData& d) { return d.id == setting.nameId; }); - if (it != submenuData.cend()) { + auto it = std::find_if(submenuData.begin(), submenuData.end(), + [&setting](const SubmenuData& d) { return d.id == setting.nameId; }); + if (it != submenuData.end()) { startActivityForResult( - std::make_unique(renderer, mappedInput, setting.nameId, it->items), resultHandler); + std::make_unique(renderer, mappedInput, setting.nameId, std::move(it->items)), + resultHandler); } } else { auto activity = createActivityForAction(setting.action, renderer, mappedInput); diff --git a/src/activities/weather/WeatherSettingsActivity.cpp b/src/activities/weather/WeatherSettingsActivity.cpp index 32e938a4..371eb3c6 100644 --- a/src/activities/weather/WeatherSettingsActivity.cpp +++ b/src/activities/weather/WeatherSettingsActivity.cpp @@ -14,6 +14,7 @@ #include "fontIds.h" void WeatherSettingsActivity::buildMenuItems() { + menuItems.reserve(10); menuItems.push_back(SettingInfo::Separator(StrId::STR_SETTINGS_TITLE)); menuItems.push_back(SettingInfo::Toggle(StrId::STR_USE_WEATHER, &CrossPointSettings::useWeather, "useWeather", StrId::STR_CAT_SYSTEM)); diff --git a/src/network/CrossPointWebServer.cpp b/src/network/CrossPointWebServer.cpp index 71f83812..77049ff3 100644 --- a/src/network/CrossPointWebServer.cpp +++ b/src/network/CrossPointWebServer.cpp @@ -1240,7 +1240,7 @@ void CrossPointWebServer::handleGetSettings() const { if (s.valuePtr) { doc["value"] = static_cast(SETTINGS.*(s.valuePtr)); } else if (s.valueGetter) { - doc["value"] = static_cast(s.valueGetter()); + doc["value"] = static_cast(s.callValueGetter()); } JsonArray options = doc["options"].to(); for (const auto& opt : s.enumValues) { @@ -1261,7 +1261,7 @@ void CrossPointWebServer::handleGetSettings() const { case SettingType::STRING: { doc["type"] = "string"; if (s.stringGetter) { - doc["value"] = s.stringGetter(); + doc["value"] = s.callStringGetter(); } else if (s.stringMaxLen > 0) { doc["value"] = reinterpret_cast(&SETTINGS) + s.stringOffset; } @@ -1326,7 +1326,7 @@ void CrossPointWebServer::handlePostSettings() { if (s.valuePtr) { SETTINGS.*(s.valuePtr) = static_cast(val); } else if (s.valueSetter) { - s.valueSetter(static_cast(val)); + s.callValueSetter(static_cast(val)); } applied++; } @@ -1345,7 +1345,7 @@ void CrossPointWebServer::handlePostSettings() { case SettingType::STRING: { const std::string val = doc[s.key].as(); if (s.stringSetter) { - s.stringSetter(val); + s.callStringSetter(val); } else if (s.stringMaxLen > 0) { char* ptr = reinterpret_cast(&SETTINGS) + s.stringOffset; strncpy(ptr, val.c_str(), s.stringMaxLen - 1); From 2b17872c16a3375dc386a1374a6cf29dec6eea05 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 15:39:02 +0200 Subject: [PATCH 02/10] Refactor spaghetti code --- src/activities/home/HomeActivity.cpp | 149 +++++++++++---------------- src/activities/home/HomeActivity.h | 33 ++++-- 2 files changed, 85 insertions(+), 97 deletions(-) diff --git a/src/activities/home/HomeActivity.cpp b/src/activities/home/HomeActivity.cpp index e8cd0d91..554830af 100644 --- a/src/activities/home/HomeActivity.cpp +++ b/src/activities/home/HomeActivity.cpp @@ -99,21 +99,25 @@ int getHomeCoverRenderHeight(const HomeScreenLayout& layout) { } } // namespace -int HomeActivity::getMenuItemCount() const { - int count = 4; // File Browser, Recents, File transfer, Settings - if (SETTINGS.useWeather) { - count++; - } - if (!recentBooks.empty()) { - count += recentBooks.size(); +// Builds the menu entry list in display order. Single source of truth for both loop() (which +// dispatches Confirm based on action) and render() (which draws labels/icons). +void HomeActivity::rebuildMenuEntries() { + menuEntries.clear(); + menuEntries.reserve(7); + + menuEntries.push_back({MenuAction::FileBrowser, StrId::STR_BROWSE_FILES, Folder}); + menuEntries.push_back({MenuAction::Recents, StrId::STR_MENU_RECENT_BOOKS, Recent}); + if (!GLOBAL_BOOKMARKS.isEmpty()) { + menuEntries.push_back({MenuAction::GlobalBookmarks, StrId::STR_GLOBAL_BOOKMARKS, Book}); } if (hasOpdsUrl) { - count++; + menuEntries.push_back({MenuAction::OpdsBrowser, StrId::STR_OPDS_BROWSER, Library}); } - if (!GLOBAL_BOOKMARKS.isEmpty()) { - count++; + menuEntries.push_back({MenuAction::FileTransfer, StrId::STR_FILE_TRANSFER, Transfer}); + if (SETTINGS.useWeather) { + menuEntries.push_back({MenuAction::Weather, StrId::STR_WEATHER, Weather}); } - return count; + menuEntries.push_back({MenuAction::Settings, StrId::STR_SETTINGS_TITLE, Settings}); } void HomeActivity::loadRecentBooks(int maxBooks) { @@ -260,57 +264,37 @@ void HomeActivity::freeCoverBuffer() { } void HomeActivity::loop() { + rebuildMenuEntries(); + const int totalItems = static_cast(recentBooks.size() + menuEntries.size()); + if (firstRenderDone && !recentsLoaded && !recentsLoading) { const auto& metrics = UITheme::getInstance().getMetrics(); const Rect contentRect = UITheme::getContentRect(renderer, true, false); - const int menuItemCount = getMenuItemCount(); - const HomeScreenLayout layout = computeHomeScreenLayout(metrics, contentRect.height, menuItemCount); + const HomeScreenLayout layout = + computeHomeScreenLayout(metrics, contentRect.height, static_cast(menuEntries.size())); loadRecentCovers(getHomeCoverRenderHeight(layout)); return; } - const int menuCount = getMenuItemCount(); - - buttonNavigator.onNext([this, menuCount] { - selectorIndex = ButtonNavigator::nextIndex(selectorIndex, menuCount); + buttonNavigator.onNext([this, totalItems] { + selectorIndex = ButtonNavigator::nextIndex(selectorIndex, totalItems); requestUpdate(); }); - buttonNavigator.onPrevious([this, menuCount] { - selectorIndex = ButtonNavigator::previousIndex(selectorIndex, menuCount); + buttonNavigator.onPrevious([this, totalItems] { + selectorIndex = ButtonNavigator::previousIndex(selectorIndex, totalItems); requestUpdate(); }); if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { - // Calculate dynamic indices based on which options are available - int idx = 0; - int menuSelectedIndex = selectorIndex - static_cast(recentBooks.size()); - const bool hasGlobalBookmarks = !GLOBAL_BOOKMARKS.isEmpty(); - const bool hasWeather = SETTINGS.useWeather; - const int fileBrowserIdx = idx++; - const int recentsIdx = idx++; - const int globalBookmarksIdx = hasGlobalBookmarks ? idx++ : -1; - const int opdsLibraryIdx = hasOpdsUrl ? idx++ : -1; - const int fileTransferIdx = idx++; - const int weatherIdx = hasWeather ? idx++ : -1; - const int settingsIdx = idx; - - if (selectorIndex < recentBooks.size()) { + const int recentsCount = static_cast(recentBooks.size()); + if (selectorIndex < recentsCount) { onSelectBook(recentBooks[selectorIndex].path); - } else if (menuSelectedIndex == fileBrowserIdx) { - onFileBrowserOpen(); - } else if (menuSelectedIndex == recentsIdx) { - onRecentsOpen(); - } else if (menuSelectedIndex == globalBookmarksIdx) { - onGlobalBookmarksOpen(); - } else if (menuSelectedIndex == opdsLibraryIdx) { - onOpdsBrowserOpen(); - } else if (menuSelectedIndex == weatherIdx) { - onWeatherOpen(); - } else if (menuSelectedIndex == fileTransferIdx) { - onFileTransferOpen(); - } else if (menuSelectedIndex == settingsIdx) { - onSettingsOpen(); + } else { + const int menuIdx = selectorIndex - recentsCount; + if (menuIdx >= 0 && menuIdx < static_cast(menuEntries.size())) { + dispatchMenuAction(menuEntries[menuIdx].action); + } } } } @@ -324,37 +308,14 @@ void HomeActivity::render(RenderLock&&) { GUI.drawHeader(renderer, Rect{contentRect.x, metrics.topPadding, contentRect.width, metrics.homeTopPadding}, nullptr); - // Build menu items dynamically - const char* weatherMenuLabel = SETTINGS.useWeather ? tr(STR_WEATHER) : tr(STR_SETTINGS_TITLE); - const UIIcon weatherMenuIcon = SETTINGS.useWeather ? Weather : Settings; + rebuildMenuEntries(); - std::vector menuItems = {tr(STR_BROWSE_FILES), tr(STR_MENU_RECENT_BOOKS), tr(STR_FILE_TRANSFER), - weatherMenuLabel, tr(STR_SETTINGS_TITLE)}; - std::vector menuIcons = {Folder, Recent, Transfer, weatherMenuIcon, Settings}; - - if (!SETTINGS.useWeather) { - menuItems.erase(menuItems.begin() + 3); - menuIcons.erase(menuIcons.begin() + 3); - } - - int insertAfterRecents = 2; - if (!GLOBAL_BOOKMARKS.isEmpty()) { - menuItems.insert(menuItems.begin() + insertAfterRecents, tr(STR_GLOBAL_BOOKMARKS)); - menuIcons.insert(menuIcons.begin() + insertAfterRecents, Book); - insertAfterRecents++; - } - - if (hasOpdsUrl) { - menuItems.insert(menuItems.begin() + insertAfterRecents, tr(STR_OPDS_BROWSER)); - menuIcons.insert(menuIcons.begin() + insertAfterRecents, Library); - } - - const int totalItems = static_cast(recentBooks.size() + menuItems.size()); + const int totalItems = static_cast(recentBooks.size() + menuEntries.size()); if (selectorIndex >= totalItems) { selectorIndex = std::max(0, totalItems - 1); } - const int menuCount = static_cast(menuItems.size()); + const int menuCount = static_cast(menuEntries.size()); const HomeScreenLayout layout = computeHomeScreenLayout(metrics, contentRect.height, menuCount); GUI.drawRecentBookCover(renderer, @@ -367,8 +328,8 @@ void HomeActivity::render(RenderLock&&) { Rect{contentRect.x, metrics.homeTopPadding + layout.recentTileHeight + layout.recentToMenuGap, contentRect.width, layout.menuHeight}, menuCount, selectorIndex - static_cast(recentBooks.size()), - [&menuItems](int index) { return std::string(menuItems[index]); }, - [&menuIcons](int index) { return menuIcons[index]; }); + [this](int index) { return std::string(I18N.get(menuEntries[index].label)); }, + [this](int index) { return menuEntries[index].icon; }); const auto labels = mappedInput.mapLabels("", tr(STR_SELECT), tr(STR_DIR_UP), tr(STR_DIR_DOWN)); GUI.drawButtonHints(renderer, labels.btn1, labels.btn2, labels.btn3, labels.btn4); @@ -383,16 +344,28 @@ void HomeActivity::render(RenderLock&&) { void HomeActivity::onSelectBook(const std::string& path) { activityManager.pushReader(path); } -void HomeActivity::onFileBrowserOpen() { activityManager.goToFileBrowser(); } - -void HomeActivity::onRecentsOpen() { activityManager.goToRecentBooks(); } - -void HomeActivity::onGlobalBookmarksOpen() { activityManager.goToGlobalBookmarks(); } - -void HomeActivity::onSettingsOpen() { activityManager.goToSettings(); } - -void HomeActivity::onFileTransferOpen() { activityManager.goToFileTransfer(); } - -void HomeActivity::onOpdsBrowserOpen() { activityManager.goToBrowser(); } - -void HomeActivity::onWeatherOpen() { activityManager.goToWeather(); } +void HomeActivity::dispatchMenuAction(MenuAction action) { + switch (action) { + case MenuAction::FileBrowser: + activityManager.goToFileBrowser(); + break; + case MenuAction::Recents: + activityManager.goToRecentBooks(); + break; + case MenuAction::GlobalBookmarks: + activityManager.goToGlobalBookmarks(); + break; + case MenuAction::OpdsBrowser: + activityManager.goToBrowser(); + break; + case MenuAction::FileTransfer: + activityManager.goToFileTransfer(); + break; + case MenuAction::Weather: + activityManager.goToWeather(); + break; + case MenuAction::Settings: + activityManager.goToSettings(); + break; + } +} diff --git a/src/activities/home/HomeActivity.h b/src/activities/home/HomeActivity.h index 8bf7ce7e..692fbd69 100644 --- a/src/activities/home/HomeActivity.h +++ b/src/activities/home/HomeActivity.h @@ -5,12 +5,31 @@ #include "../Activity.h" #include "./FileBrowserActivity.h" +#include "components/UITheme.h" #include "util/ButtonNavigator.h" struct RecentBook; struct Rect; class HomeActivity final : public Activity { + public: + enum class MenuAction { + FileBrowser, + Recents, + GlobalBookmarks, + OpdsBrowser, + FileTransfer, + Weather, + Settings, + }; + + private: + struct MenuEntry { + MenuAction action; + StrId label; + UIIcon icon; + }; + ButtonNavigator buttonNavigator; int selectorIndex = 0; bool recentsLoading = false; @@ -22,16 +41,12 @@ class HomeActivity final : public Activity { size_t nextRecentCoverIndex = 0; uint8_t* coverBuffer = nullptr; // HomeActivity's own buffer for cover image std::vector recentBooks; - void onSelectBook(const std::string& path); - void onFileBrowserOpen(); - void onRecentsOpen(); - void onGlobalBookmarksOpen(); - void onSettingsOpen(); - void onFileTransferOpen(); - void onOpdsBrowserOpen(); - void onWeatherOpen(); + std::vector menuEntries; - int getMenuItemCount() const; + void onSelectBook(const std::string& path); + void dispatchMenuAction(MenuAction action); + + void rebuildMenuEntries(); bool storeCoverBuffer(); // Store frame buffer for cover image bool restoreCoverBuffer(); // Restore frame buffer from stored cover void freeCoverBuffer(); // Free the stored cover buffer From 106e9d31e60952e376342586c3a3dfd58e582c31 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 15:53:57 +0200 Subject: [PATCH 03/10] Reduce heap pressure --- src/JsonSettingsIO.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/JsonSettingsIO.cpp b/src/JsonSettingsIO.cpp index 3ed29dfd..d941f645 100644 --- a/src/JsonSettingsIO.cpp +++ b/src/JsonSettingsIO.cpp @@ -68,7 +68,7 @@ void applyLegacyStatusBarSettings(CrossPointSettings& settings) { // ---- CrossPointState ---- bool JsonSettingsIO::saveState(const CrossPointState& s, const char* path) { - JsonDocument doc; + StaticJsonDocument<4096> doc; doc["openEpubPath"] = s.openEpubPath; doc["lastSleepImage"] = s.lastSleepImage; doc["readerActivityLoadCount"] = s.readerActivityLoadCount; @@ -95,9 +95,14 @@ bool JsonSettingsIO::saveState(const CrossPointState& s, const char* path) { jump["spineIndex"] = s.pendingBookmarkJump.spineIndex; jump["pageNumber"] = s.pendingBookmarkJump.pageNumber; - String json; - serializeJson(doc, json); - return Storage.writeFile(path, json); + FsFile file; + if (!Storage.openFileForWrite("CPS", path, file)) { + return false; + } + + const size_t written = serializeJson(doc, file); + file.close(); + return written > 0; } bool JsonSettingsIO::loadState(CrossPointState& s, const char* json) { From 87b9d333f76e4bd46a5b1ebbca5bfa80aa951d88 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 15:54:08 +0200 Subject: [PATCH 04/10] Review changes --- src/activities/home/HomeActivity.cpp | 8 +++++++- src/activities/home/HomeActivity.h | 1 + src/activities/settings/SettingInfo.h | 21 ++++++++++++++++---- src/activities/settings/SettingsActivity.cpp | 3 +-- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/activities/home/HomeActivity.cpp b/src/activities/home/HomeActivity.cpp index 554830af..4f3450a3 100644 --- a/src/activities/home/HomeActivity.cpp +++ b/src/activities/home/HomeActivity.cpp @@ -211,6 +211,7 @@ void HomeActivity::onEnter() { } // Trigger first update + menuEntriesDirty = true; requestUpdate(); } @@ -308,7 +309,9 @@ void HomeActivity::render(RenderLock&&) { GUI.drawHeader(renderer, Rect{contentRect.x, metrics.topPadding, contentRect.width, metrics.homeTopPadding}, nullptr); - rebuildMenuEntries(); + if (menuEntriesDirty) { + rebuildMenuEntries(); + } const int totalItems = static_cast(recentBooks.size() + menuEntries.size()); if (selectorIndex >= totalItems) { @@ -367,5 +370,8 @@ void HomeActivity::dispatchMenuAction(MenuAction action) { case MenuAction::Settings: activityManager.goToSettings(); break; + default: + LOG_ERR("HOME", "Unexpected menu action: %d", static_cast(action)); + break; } } diff --git a/src/activities/home/HomeActivity.h b/src/activities/home/HomeActivity.h index 692fbd69..c4fad72a 100644 --- a/src/activities/home/HomeActivity.h +++ b/src/activities/home/HomeActivity.h @@ -42,6 +42,7 @@ class HomeActivity final : public Activity { uint8_t* coverBuffer = nullptr; // HomeActivity's own buffer for cover image std::vector recentBooks; std::vector menuEntries; + bool menuEntriesDirty = true; void onSelectBook(const std::string& path); void dispatchMenuAction(MenuAction action); diff --git a/src/activities/settings/SettingInfo.h b/src/activities/settings/SettingInfo.h index 498cbb6d..9d8f1696 100644 --- a/src/activities/settings/SettingInfo.h +++ b/src/activities/settings/SettingInfo.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -63,10 +64,22 @@ struct SettingInfo { StringGetterFn stringGetter = nullptr; StringSetterFn stringSetter = nullptr; - uint8_t callValueGetter() const { return valueGetter(accessorCtx); } - void callValueSetter(uint8_t v) const { valueSetter(accessorCtx, v); } - std::string callStringGetter() const { return stringGetter(accessorCtx); } - void callStringSetter(const std::string& v) const { stringSetter(accessorCtx, v); } + uint8_t callValueGetter() const { + assert(valueGetter && "SettingInfo::callValueGetter requires a non-null valueGetter"); + return valueGetter(accessorCtx); + } + void callValueSetter(uint8_t v) const { + assert(valueSetter && "SettingInfo::callValueSetter requires a non-null valueSetter"); + valueSetter(accessorCtx, v); + } + std::string callStringGetter() const { + assert(stringGetter && "SettingInfo::callStringGetter requires a non-null stringGetter"); + return stringGetter(accessorCtx); + } + void callStringSetter(const std::string& v) const { + assert(stringSetter && "SettingInfo::callStringSetter requires a non-null stringSetter"); + stringSetter(accessorCtx, v); + } SettingInfo& withObfuscated() { obfuscated = true; diff --git a/src/activities/settings/SettingsActivity.cpp b/src/activities/settings/SettingsActivity.cpp index 91b9051d..789b38e2 100644 --- a/src/activities/settings/SettingsActivity.cpp +++ b/src/activities/settings/SettingsActivity.cpp @@ -229,8 +229,7 @@ void SettingsActivity::toggleCurrentSetting() { [&setting](const SubmenuData& d) { return d.id == setting.nameId; }); if (it != submenuData.end()) { startActivityForResult( - std::make_unique(renderer, mappedInput, setting.nameId, std::move(it->items)), - resultHandler); + std::make_unique(renderer, mappedInput, setting.nameId, it->items), resultHandler); } } else { auto activity = createActivityForAction(setting.action, renderer, mappedInput); From af018af03af70126e82817602ede9a1b15e76f9c Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 15:59:58 +0200 Subject: [PATCH 05/10] style fixes --- src/activities/home/HomeActivity.cpp | 2 +- src/activities/reader/EpubReaderMenuActivity.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/activities/home/HomeActivity.cpp b/src/activities/home/HomeActivity.cpp index 4f3450a3..f9950571 100644 --- a/src/activities/home/HomeActivity.cpp +++ b/src/activities/home/HomeActivity.cpp @@ -293,7 +293,7 @@ void HomeActivity::loop() { onSelectBook(recentBooks[selectorIndex].path); } else { const int menuIdx = selectorIndex - recentsCount; - if (menuIdx >= 0 && menuIdx < static_cast(menuEntries.size())) { + if (menuIdx < static_cast(menuEntries.size())) { dispatchMenuAction(menuEntries[menuIdx].action); } } diff --git a/src/activities/reader/EpubReaderMenuActivity.cpp b/src/activities/reader/EpubReaderMenuActivity.cpp index 7d7558f4..e7d66f84 100644 --- a/src/activities/reader/EpubReaderMenuActivity.cpp +++ b/src/activities/reader/EpubReaderMenuActivity.cpp @@ -56,7 +56,7 @@ void EpubReaderMenuActivity::buildMenuItems(bool hasFootnotes, bool hasStarredPa menuItems.push_back(SettingInfo::DynamicEnumCtx( StrId::STR_EMBEDDED_STYLE, {StrId::STR_DEFAULT_VALUE, StrId::STR_STATE_ON, StrId::STR_STATE_OFF}, self, [](void* ctx) -> uint8_t { - auto* s = static_cast(ctx); + const auto* s = static_cast(ctx); if (s->pendingEmbeddedStyleOverride < 0) return 0; if (s->pendingEmbeddedStyleOverride > 0) return 1; return 2; @@ -77,7 +77,7 @@ void EpubReaderMenuActivity::buildMenuItems(bool hasFootnotes, bool hasStarredPa {StrId::STR_DEFAULT_VALUE, StrId::STR_IMAGES_DISPLAY, StrId::STR_IMAGES_PLACEHOLDER, StrId::STR_IMAGES_SUPPRESS}, self, [](void* ctx) -> uint8_t { - auto* s = static_cast(ctx); + const auto* s = static_cast(ctx); return (s->pendingImageRenderingOverride < 0) ? 0 : (s->pendingImageRenderingOverride + 1); }, [](void* ctx, uint8_t v) { From a8816b2964646f8a249896cd5e862baef3258bc3 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 16:16:27 +0200 Subject: [PATCH 06/10] Remove outdated static variant --- src/JsonSettingsIO.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/JsonSettingsIO.cpp b/src/JsonSettingsIO.cpp index d941f645..eb6b711d 100644 --- a/src/JsonSettingsIO.cpp +++ b/src/JsonSettingsIO.cpp @@ -68,7 +68,7 @@ void applyLegacyStatusBarSettings(CrossPointSettings& settings) { // ---- CrossPointState ---- bool JsonSettingsIO::saveState(const CrossPointState& s, const char* path) { - StaticJsonDocument<4096> doc; + JsonDocument doc; doc["openEpubPath"] = s.openEpubPath; doc["lastSleepImage"] = s.lastSleepImage; doc["readerActivityLoadCount"] = s.readerActivityLoadCount; From e0fb3e0160d0e7310e235d0a36977a888370f342 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 16:25:29 +0200 Subject: [PATCH 07/10] More comments --- src/JsonSettingsIO.cpp | 14 ++++++++++++-- src/activities/home/HomeActivity.cpp | 5 ++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/JsonSettingsIO.cpp b/src/JsonSettingsIO.cpp index eb6b711d..4dc6811e 100644 --- a/src/JsonSettingsIO.cpp +++ b/src/JsonSettingsIO.cpp @@ -100,9 +100,19 @@ bool JsonSettingsIO::saveState(const CrossPointState& s, const char* path) { return false; } + if (doc.overflowed()) { + LOG_ERR("CPS", "JSON document overflowed while building state"); + file.close(); + return false; + } + + const size_t expected = measureJson(doc); const size_t written = serializeJson(doc, file); - file.close(); - return written > 0; + file.flush(); + if (!file.close()) { + return false; + } + return written == expected; } bool JsonSettingsIO::loadState(CrossPointState& s, const char* json) { diff --git a/src/activities/home/HomeActivity.cpp b/src/activities/home/HomeActivity.cpp index f9950571..52ec7658 100644 --- a/src/activities/home/HomeActivity.cpp +++ b/src/activities/home/HomeActivity.cpp @@ -118,6 +118,7 @@ void HomeActivity::rebuildMenuEntries() { menuEntries.push_back({MenuAction::Weather, StrId::STR_WEATHER, Weather}); } menuEntries.push_back({MenuAction::Settings, StrId::STR_SETTINGS_TITLE, Settings}); + menuEntriesDirty = false; } void HomeActivity::loadRecentBooks(int maxBooks) { @@ -265,7 +266,9 @@ void HomeActivity::freeCoverBuffer() { } void HomeActivity::loop() { - rebuildMenuEntries(); + if (menuEntriesDirty) { + rebuildMenuEntries(); + } const int totalItems = static_cast(recentBooks.size() + menuEntries.size()); if (firstRenderDone && !recentsLoaded && !recentsLoading) { From 8b6db63788e87695cdef4e715896ccfe48b3a40b Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 16:33:15 +0200 Subject: [PATCH 08/10] Add guard --- src/JsonSettingsIO.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/JsonSettingsIO.cpp b/src/JsonSettingsIO.cpp index 4dc6811e..b09692d0 100644 --- a/src/JsonSettingsIO.cpp +++ b/src/JsonSettingsIO.cpp @@ -95,14 +95,13 @@ bool JsonSettingsIO::saveState(const CrossPointState& s, const char* path) { jump["spineIndex"] = s.pendingBookmarkJump.spineIndex; jump["pageNumber"] = s.pendingBookmarkJump.pageNumber; - FsFile file; - if (!Storage.openFileForWrite("CPS", path, file)) { + if (doc.overflowed()) { + LOG_ERR("CPS", "JSON document overflowed while building state"); return false; } - if (doc.overflowed()) { - LOG_ERR("CPS", "JSON document overflowed while building state"); - file.close(); + FsFile file; + if (!Storage.openFileForWrite("CPS", path, file)) { return false; } From efbe8adf43276ef84182b6ef55aa40f79cfa0c17 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 16:41:24 +0200 Subject: [PATCH 09/10] cppcheck finding --- src/activities/reader/EpubReaderMenuActivity.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/activities/reader/EpubReaderMenuActivity.cpp b/src/activities/reader/EpubReaderMenuActivity.cpp index e7d66f84..7716b60c 100644 --- a/src/activities/reader/EpubReaderMenuActivity.cpp +++ b/src/activities/reader/EpubReaderMenuActivity.cpp @@ -55,7 +55,7 @@ void EpubReaderMenuActivity::buildMenuItems(bool hasFootnotes, bool hasStarredPa // Embedded style: cycles default(-1) -> ON(1) -> OFF(0) via DynamicEnum indices 0/1/2 menuItems.push_back(SettingInfo::DynamicEnumCtx( StrId::STR_EMBEDDED_STYLE, {StrId::STR_DEFAULT_VALUE, StrId::STR_STATE_ON, StrId::STR_STATE_OFF}, self, - [](void* ctx) -> uint8_t { + [](const void* ctx) -> uint8_t { const auto* s = static_cast(ctx); if (s->pendingEmbeddedStyleOverride < 0) return 0; if (s->pendingEmbeddedStyleOverride > 0) return 1; @@ -76,7 +76,7 @@ void EpubReaderMenuActivity::buildMenuItems(bool hasFootnotes, bool hasStarredPa StrId::STR_IMAGES, {StrId::STR_DEFAULT_VALUE, StrId::STR_IMAGES_DISPLAY, StrId::STR_IMAGES_PLACEHOLDER, StrId::STR_IMAGES_SUPPRESS}, self, - [](void* ctx) -> uint8_t { + [](const void* ctx) -> uint8_t { const auto* s = static_cast(ctx); return (s->pendingImageRenderingOverride < 0) ? 0 : (s->pendingImageRenderingOverride + 1); }, From 4bada707b34315e225c3a6d6f03e7eb43c9d8bf4 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 16:46:48 +0200 Subject: [PATCH 10/10] Fix getter --- src/activities/reader/EpubReaderMenuActivity.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/activities/reader/EpubReaderMenuActivity.cpp b/src/activities/reader/EpubReaderMenuActivity.cpp index 7716b60c..e7d66f84 100644 --- a/src/activities/reader/EpubReaderMenuActivity.cpp +++ b/src/activities/reader/EpubReaderMenuActivity.cpp @@ -55,7 +55,7 @@ void EpubReaderMenuActivity::buildMenuItems(bool hasFootnotes, bool hasStarredPa // Embedded style: cycles default(-1) -> ON(1) -> OFF(0) via DynamicEnum indices 0/1/2 menuItems.push_back(SettingInfo::DynamicEnumCtx( StrId::STR_EMBEDDED_STYLE, {StrId::STR_DEFAULT_VALUE, StrId::STR_STATE_ON, StrId::STR_STATE_OFF}, self, - [](const void* ctx) -> uint8_t { + [](void* ctx) -> uint8_t { const auto* s = static_cast(ctx); if (s->pendingEmbeddedStyleOverride < 0) return 0; if (s->pendingEmbeddedStyleOverride > 0) return 1; @@ -76,7 +76,7 @@ void EpubReaderMenuActivity::buildMenuItems(bool hasFootnotes, bool hasStarredPa StrId::STR_IMAGES, {StrId::STR_DEFAULT_VALUE, StrId::STR_IMAGES_DISPLAY, StrId::STR_IMAGES_PLACEHOLDER, StrId::STR_IMAGES_SUPPRESS}, self, - [](const void* ctx) -> uint8_t { + [](void* ctx) -> uint8_t { const auto* s = static_cast(ctx); return (s->pendingImageRenderingOverride < 0) ? 0 : (s->pendingImageRenderingOverride + 1); },