Merge 265bbea7db into 3ce11f14ce

Multiple goals are achieved with this change:
- make descriptions of settings close to their definitions
- settings validation on loading with reset to default in case of bad
  value
- more constexpr to reduce RAM usage
- less hardcoded values
- more maintainable

RAM Usage:
 From
RAM:   [===       ]  32.4% (used 106084 bytes from 327680 bytes)
Flash: [========= ]  92.8% (used 6082238 bytes from 6553600 bytes)
 To
RAM:   [===       ]  32.2% (used 105436 bytes from 327680 bytes)
Flash: [========= ]  92.8% (used 6078912 bytes from 6553600 bytes)

Boot config validation:
[1186] [SD] SD card detected
[1192] [CPS] Invalid value for Status Bar, resetting to default
[1192] [CPS] Settings loaded from file

src/CrossPointSettings.cpp

@@ -20,11 +20,17 @@ bool SettingDescriptor::validate(const CrossPointSettings& settings) const {
   return validator(value);
 }
 
-uint8_t SettingDescriptor::getValue(const CrossPointSettings& settings) const { return settings.*(memberPtr); }
+uint8_t SettingDescriptor::getValue(const CrossPointSettings& settings) const {
+  return settings.*(memberPtr);
+}
 
-void SettingDescriptor::setValue(CrossPointSettings& settings, uint8_t value) const { settings.*(memberPtr) = value; }
+void SettingDescriptor::setValue(CrossPointSettings& settings, uint8_t value) const {
+  settings.*(memberPtr) = value;
+}
 
-void SettingDescriptor::resetToDefault(CrossPointSettings& settings) const { settings.*(memberPtr) = defaultValue; }
+void SettingDescriptor::resetToDefault(CrossPointSettings& settings) const {
+  settings.*(memberPtr) = defaultValue;
+}
 
 void SettingDescriptor::save(FsFile& file, const CrossPointSettings& settings) const {
   serialization::writePod(file, settings.*(memberPtr));
@@ -36,10 +42,10 @@ void SettingDescriptor::load(FsFile& file, CrossPointSettings& settings) const {
   settings.*(memberPtr) = value;
 }
 
+
 namespace {
 constexpr uint8_t SETTINGS_FILE_VERSION = 1;
 // Increment this when adding new persisted settings fields
-constexpr uint8_t SETTINGS_COUNT = CrossPointSettings::DESCRIPTOR_COUNT + 1;  // descriptors + opdsServerUrl string
 constexpr char SETTINGS_FILE[] = "/.crosspoint/settings.bin";
 }  // namespace
 
@@ -49,8 +55,7 @@ constexpr const char* sleepScreenValues[] = {"Dark", "Light", "Custom", "Cover",
 constexpr const char* shortPwrBtnValues[] = {"Ignore", "Sleep", "Page Turn"};
 constexpr const char* statusBarValues[] = {"None", "No Progress", "Full"};
 constexpr const char* orientationValues[] = {"Portrait", "Landscape CW", "Inverted", "Landscape CCW"};
-constexpr const char* frontButtonLayoutValues[] = {"Back/Confirm/Left/Right", "Left/Right/Back/Confirm",
+constexpr const char* frontButtonLayoutValues[] = {"Back/Confirm/Left/Right", "Left/Right/Back/Confirm", "Left/Back/Confirm/Right"};
-                                                   "Left/Back/Confirm/Right"};
 constexpr const char* sideButtonLayoutValues[] = {"Prev/Next", "Next/Prev"};
 constexpr const char* fontFamilyValues[] = {"Bookerly", "Noto Sans", "Open Dyslexic"};
 constexpr const char* fontSizeValues[] = {"Small", "Medium", "Large", "X Large"};
@@ -62,53 +67,45 @@ constexpr const char* sleepScreenCoverModeValues[] = {"Fit", "Crop"};
 constexpr const char* hideBatteryPercentageValues[] = {"Never", "In Reader", "Always"};
 
 // Helper function template to deduce array size automatically
-template <size_t N>
+template<size_t N>
-constexpr SettingDescriptor makeEnumDescriptor(const char* name, uint8_t CrossPointSettings::* ptr,
+constexpr SettingDescriptor makeEnumDescriptor(
-                                               uint8_t defaultValue, const char* const (&enumValues)[N]) {
+    const char* name,
-  return SettingDescriptor(name, SettingType::ENUM, ptr, defaultValue, validateEnum<N>, enumValues, N);
+    uint8_t CrossPointSettings::* ptr,
+    uint8_t defaultValue,
+    const char* const (&enumValues)[N]
+) {
+    return SettingDescriptor(name, SettingType::ENUM, ptr, defaultValue,
+                            validateEnum<N>, enumValues, N);
+}
 }
-}  // namespace
 
 // Define static constexpr members (required in C++14 and earlier)
 constexpr size_t CrossPointSettings::DESCRIPTOR_COUNT;
 
 // Define the static constexpr array of all setting descriptors
 // Order must match current serialization order for file format compatibility!
-const std::array<SettingDescriptor, CrossPointSettings::DESCRIPTOR_COUNT> CrossPointSettings::descriptors = {
+const std::array<SettingDescriptor, CrossPointSettings::DESCRIPTOR_COUNT> CrossPointSettings::descriptors = {{
-    {makeEnumDescriptor("Sleep Screen", &CrossPointSettings::sleepScreen, CrossPointSettings::DARK, sleepScreenValues),
+    makeEnumDescriptor("Sleep Screen", &CrossPointSettings::sleepScreen, CrossPointSettings::DARK, sleepScreenValues),
-     {"Extra Paragraph Spacing", SettingType::TOGGLE, &CrossPointSettings::extraParagraphSpacing, 1, validateToggle,
+    {"Extra Paragraph Spacing", SettingType::TOGGLE, &CrossPointSettings::extraParagraphSpacing, 1, validateToggle, nullptr, 0},
-      nullptr, 0},
+    makeEnumDescriptor("Short Power Button Click", &CrossPointSettings::shortPwrBtn, CrossPointSettings::IGNORE, shortPwrBtnValues),
-     makeEnumDescriptor("Short Power Button Click", &CrossPointSettings::shortPwrBtn, CrossPointSettings::IGNORE,
+    makeEnumDescriptor("Status Bar", &CrossPointSettings::statusBar, CrossPointSettings::FULL, statusBarValues),
-                        shortPwrBtnValues),
+    makeEnumDescriptor("Reading Orientation", &CrossPointSettings::orientation, CrossPointSettings::PORTRAIT, orientationValues),
-     makeEnumDescriptor("Status Bar", &CrossPointSettings::statusBar, CrossPointSettings::FULL, statusBarValues),
+    makeEnumDescriptor("Front Button Layout", &CrossPointSettings::frontButtonLayout, CrossPointSettings::BACK_CONFIRM_LEFT_RIGHT, frontButtonLayoutValues),
-     makeEnumDescriptor("Reading Orientation", &CrossPointSettings::orientation, CrossPointSettings::PORTRAIT,
+    makeEnumDescriptor("Side Button Layout", &CrossPointSettings::sideButtonLayout, CrossPointSettings::PREV_NEXT, sideButtonLayoutValues),
-                        orientationValues),
+    makeEnumDescriptor("Reader Font Family", &CrossPointSettings::fontFamily, CrossPointSettings::BOOKERLY, fontFamilyValues),
-     makeEnumDescriptor("Front Button Layout", &CrossPointSettings::frontButtonLayout,
+    makeEnumDescriptor("Reader Font Size", &CrossPointSettings::fontSize, CrossPointSettings::MEDIUM, fontSizeValues),
-                        CrossPointSettings::BACK_CONFIRM_LEFT_RIGHT, frontButtonLayoutValues),
+    makeEnumDescriptor("Reader Line Spacing", &CrossPointSettings::lineSpacing, CrossPointSettings::NORMAL, lineSpacingValues),
-     makeEnumDescriptor("Side Button Layout", &CrossPointSettings::sideButtonLayout, CrossPointSettings::PREV_NEXT,
+    makeEnumDescriptor("Reader Paragraph Alignment", &CrossPointSettings::paragraphAlignment, CrossPointSettings::JUSTIFIED, paragraphAlignmentValues),
-                        sideButtonLayoutValues),
+    makeEnumDescriptor("Time to Sleep", &CrossPointSettings::sleepTimeout, CrossPointSettings::SLEEP_10_MIN, sleepTimeoutValues),
-     makeEnumDescriptor("Reader Font Family", &CrossPointSettings::fontFamily, CrossPointSettings::BOOKERLY,
+    makeEnumDescriptor("Refresh Frequency", &CrossPointSettings::refreshFrequency, CrossPointSettings::REFRESH_15, refreshFrequencyValues),
-                        fontFamilyValues),
+    {"Reader Screen Margin", SettingType::VALUE, &CrossPointSettings::screenMargin, 5, validateRange<5, 40>, ValueRange{5, 40, 5}},
-     makeEnumDescriptor("Reader Font Size", &CrossPointSettings::fontSize, CrossPointSettings::MEDIUM, fontSizeValues),
+    makeEnumDescriptor("Sleep Screen Cover Mode", &CrossPointSettings::sleepScreenCoverMode, CrossPointSettings::FIT, sleepScreenCoverModeValues),
-     makeEnumDescriptor("Reader Line Spacing", &CrossPointSettings::lineSpacing, CrossPointSettings::NORMAL,
+    // Note: opdsServerUrl (string) at position 15 is handled separately in serialization
-                        lineSpacingValues),
+    {"Text Anti-Aliasing", SettingType::TOGGLE, &CrossPointSettings::textAntiAliasing, 1, validateToggle, nullptr, 0},
-     makeEnumDescriptor("Reader Paragraph Alignment", &CrossPointSettings::paragraphAlignment,
+    makeEnumDescriptor("Hide Battery %", &CrossPointSettings::hideBatteryPercentage, CrossPointSettings::HIDE_NEVER, hideBatteryPercentageValues),
-                        CrossPointSettings::JUSTIFIED, paragraphAlignmentValues),
+    {"Long-press Chapter Skip", SettingType::TOGGLE, &CrossPointSettings::longPressChapterSkip, 1, validateToggle, nullptr, 0},
-     makeEnumDescriptor("Time to Sleep", &CrossPointSettings::sleepTimeout, CrossPointSettings::SLEEP_10_MIN,
+    {"Hyphenation", SettingType::TOGGLE, &CrossPointSettings::hyphenationEnabled, 0, validateToggle, nullptr, 0}
-                        sleepTimeoutValues),
+}};
-     makeEnumDescriptor("Refresh Frequency", &CrossPointSettings::refreshFrequency, CrossPointSettings::REFRESH_15,
-                        refreshFrequencyValues),
-     {"Reader Screen Margin", SettingType::VALUE, &CrossPointSettings::screenMargin, 5, validateRange<5, 40>,
-      ValueRange{5, 40, 5}},
-     makeEnumDescriptor("Sleep Screen Cover Mode", &CrossPointSettings::sleepScreenCoverMode, CrossPointSettings::FIT,
-                        sleepScreenCoverModeValues),
-     // Note: opdsServerUrl (string) at position 15 is handled separately in serialization
-     {"Text Anti-Aliasing", SettingType::TOGGLE, &CrossPointSettings::textAntiAliasing, 1, validateToggle, nullptr, 0},
-     makeEnumDescriptor("Hide Battery %", &CrossPointSettings::hideBatteryPercentage, CrossPointSettings::HIDE_NEVER,
-                        hideBatteryPercentageValues),
-     {"Long-press Chapter Skip", SettingType::TOGGLE, &CrossPointSettings::longPressChapterSkip, 1, validateToggle,
-      nullptr, 0},
-     {"Hyphenation", SettingType::TOGGLE, &CrossPointSettings::hyphenationEnabled, 0, validateToggle, nullptr, 0}}};
 
 bool CrossPointSettings::saveToFile() const {
   // Make sure the directory exists
@@ -120,19 +117,20 @@ bool CrossPointSettings::saveToFile() const {
   }
 
   serialization::writePod(outputFile, SETTINGS_FILE_VERSION);
-  serialization::writePod(outputFile, SETTINGS_COUNT);
+  serialization::writePod(outputFile, CrossPointSettings::DESCRIPTOR_COUNT);
 
   // Use descriptors to automatically serialize all uint8_t settings
-  // opdsServerUrl string is written at position 15 (between descriptors 14 and 15)
+  uint8_t settingsWritten = 0;
-  uint8_t descriptorIndex = 0;
+
   for (const auto& desc : descriptors) {
-    // Write opdsServerUrl string before descriptor 15 (at position 15)
+    // Special handling for opdsServerUrl (string field at position 15)
-    if (descriptorIndex == 15) {
+    if (settingsWritten == 15) {
       serialization::writeString(outputFile, std::string(opdsServerUrl));
+      settingsWritten++;
     }
 
     desc.save(outputFile, *this);
-    descriptorIndex++;
+    settingsWritten++;
   }
 
   outputFile.close();
@@ -142,10 +140,8 @@ bool CrossPointSettings::saveToFile() const {
 }
 
 bool CrossPointSettings::loadFromFile() {
-  Serial.printf("[%lu] [CPS] Loading settings from file\n", millis());
   FsFile inputFile;
   if (!SdMan.openFileForRead("CPS", SETTINGS_FILE, inputFile)) {
-    Serial.printf("[%lu] [CPS] Deserialization failed: Could not open settings file\n", millis());
     return false;
   }
 
@@ -161,31 +157,29 @@ bool CrossPointSettings::loadFromFile() {
   serialization::readPod(inputFile, fileSettingsCount);
 
   // Use descriptors to automatically deserialize all uint8_t settings
-  // opdsServerUrl string is at position 15 (between descriptors 14 and 15)
+  uint8_t settingsRead = 0;
-  uint8_t descriptorIndex = 0;
-  uint8_t filePosition = 0;
 
   for (const auto& desc : descriptors) {
-    if (filePosition >= fileSettingsCount) {
+    if (settingsRead >= fileSettingsCount) {
       break;  // File has fewer settings than current version
     }
 
-    // Read opdsServerUrl string at position 15 (before descriptor 15)
+    // Special handling for opdsServerUrl (string field at position 15)
-    if (descriptorIndex == 15) {
+    if (settingsRead == 15) {
-      std::string urlStr;
+      if (settingsRead < fileSettingsCount) {
-      serialization::readString(inputFile, urlStr);
+        std::string urlStr;
-      strncpy(opdsServerUrl, urlStr.c_str(), sizeof(opdsServerUrl) - 1);
+        serialization::readString(inputFile, urlStr);
-      opdsServerUrl[sizeof(opdsServerUrl) - 1] = '\0';
+        strncpy(opdsServerUrl, urlStr.c_str(), sizeof(opdsServerUrl) - 1);
-      filePosition++;
+        opdsServerUrl[sizeof(opdsServerUrl) - 1] = '\0';
-
+        settingsRead++;
-      if (filePosition >= fileSettingsCount) {
+      }
+      if (settingsRead >= fileSettingsCount) {
         break;
       }
     }
 
     desc.load(inputFile, *this);
-    descriptorIndex++;
+    settingsRead++;
-    filePosition++;
   }
 
   inputFile.close();
@@ -193,8 +187,7 @@ bool CrossPointSettings::loadFromFile() {
   // Validate each setting and reset invalid values to defaults
   for (const auto& desc : descriptors) {
     if (!desc.validate(*this)) {
-      Serial.printf("[%lu] [CPS] Invalid value (0x%X) for %s, resetting to default\n", millis(), desc.getValue(*this),
+      Serial.printf("[%lu] [CPS] Invalid value for %s, resetting to default\n", millis(), desc.name);
-                    desc.name);
       desc.resetToDefault(*this);
     }
   }

src/CrossPointSettings.h

@@ -1,15 +1,15 @@
 #pragma once
-#include <array>
 #include <cstdint>
 #include <iosfwd>
-#include <string>
+#include <array>
 #include <vector>
+#include <string>
 
 // Setting descriptor infrastructure
 enum class SettingType { TOGGLE, ENUM, VALUE };
 
 // Validator function pointer (not std::function to save memory)
-using SettingValidator = bool (*)(uint8_t);
+using SettingValidator = bool(*)(uint8_t);
 
 // Forward declare for descriptors
 class CrossPointSettings;
@@ -19,77 +19,72 @@ class FsFile;
 
 // Base descriptor for all settings (non-virtual for constexpr)
 struct SettingDescriptorBase {
-  const char* name;  // Display name
+    const char* name;              // Display name
-  SettingType type;
+    SettingType type;
 };
 
 // Value range for VALUE type settings
 struct ValueRange {
-  uint8_t min, max, step;
+    uint8_t min, max, step;
 };
 
 // Concrete descriptor for uint8_t settings (constexpr-compatible)
 struct SettingDescriptor : public SettingDescriptorBase {
-  uint8_t CrossPointSettings::* memberPtr;  // Member pointer
+    uint8_t CrossPointSettings::* memberPtr;  // Member pointer
-  uint8_t defaultValue;
+    uint8_t defaultValue;
-  SettingValidator validator;  // Optional validator function
+    SettingValidator validator;  // Optional validator function
 
-  union {
+    union {
-    // For ENUM types
+        // For ENUM types
-    struct {
+        struct {
-      const char* const* values;
+            const char* const* values;
-      uint8_t count;
+            uint8_t count;
-    } enumData;
+        } enumData;
 
-    // For VALUE types
+        // For VALUE types
-    ValueRange valueRange;
+        ValueRange valueRange;
-  };
+    };
 
-  // Constexpr constructors for different setting types
+    // Constexpr constructors for different setting types
-  // TOGGLE/ENUM constructor
+    // TOGGLE/ENUM constructor
-  constexpr SettingDescriptor(const char* name_, SettingType type_, uint8_t CrossPointSettings::* ptr, uint8_t defVal,
+    constexpr SettingDescriptor(const char* name_, SettingType type_,
-                              SettingValidator val, const char* const* enumVals, uint8_t enumCnt)
+                                 uint8_t CrossPointSettings::* ptr, uint8_t defVal, SettingValidator val,
-      : SettingDescriptorBase{name_, type_},
+                                 const char* const* enumVals, uint8_t enumCnt)
-        memberPtr(ptr),
+        : SettingDescriptorBase{name_, type_},
-        defaultValue(defVal),
+          memberPtr(ptr), defaultValue(defVal), validator(val),
-        validator(val),
+          enumData{enumVals, enumCnt} {}
-        enumData{enumVals, enumCnt} {}
 
-  // VALUE constructor
+    // VALUE constructor
-  constexpr SettingDescriptor(const char* name_, SettingType type_, uint8_t CrossPointSettings::* ptr, uint8_t defVal,
+    constexpr SettingDescriptor(const char* name_, SettingType type_,
-                              SettingValidator val, ValueRange valRange)
+                                 uint8_t CrossPointSettings::* ptr, uint8_t defVal, SettingValidator val,
-      : SettingDescriptorBase{name_, type_},
+                                 ValueRange valRange)
-        memberPtr(ptr),
+        : SettingDescriptorBase{name_, type_},
-        defaultValue(defVal),
+          memberPtr(ptr), defaultValue(defVal), validator(val),
-        validator(val),
+          valueRange(valRange) {}
-        valueRange(valRange) {}
 
-  bool validate(const CrossPointSettings& settings) const;
+    bool validate(const CrossPointSettings& settings) const;
-  uint8_t getValue(const CrossPointSettings& settings) const;
+    uint8_t getValue(const CrossPointSettings& settings) const;
-  void setValue(CrossPointSettings& settings, uint8_t value) const;
+    void setValue(CrossPointSettings& settings, uint8_t value) const;
-  void resetToDefault(CrossPointSettings& settings) const;
+    void resetToDefault(CrossPointSettings& settings) const;
-  void save(FsFile& file, const CrossPointSettings& settings) const;
+    void save(FsFile& file, const CrossPointSettings& settings) const;
-  void load(FsFile& file, CrossPointSettings& settings) const;
+    void load(FsFile& file, CrossPointSettings& settings) const;
 
-  // Helper to get enum value as string
+    // Helper to get enum value as string
-  const char* getEnumValueString(uint8_t index) const {
+    const char* getEnumValueString(uint8_t index) const {
-    if (index < enumData.count && enumData.values) {
+        if (index < enumData.count && enumData.values) {
-      return enumData.values[index];
+            return enumData.values[index];
+        }
+        return "";
     }
-    return "";
-  }
 };
 
+
 // Validator functions (constexpr for compile-time optimization)
 constexpr bool validateToggle(uint8_t v) { return v <= 1; }
-template <uint8_t MAX>
+template<uint8_t MAX>
-constexpr bool validateEnum(uint8_t v) {
+constexpr bool validateEnum(uint8_t v) { return v < MAX; }
-  return v < MAX;
+template<uint8_t MIN, uint8_t MAX>
-}
+constexpr bool validateRange(uint8_t v) { return v >= MIN && v <= MAX; }
-template <uint8_t MIN, uint8_t MAX>
-constexpr bool validateRange(uint8_t v) {
-  return v >= MIN && v <= MAX;
-}
 
 class CrossPointSettings {
  private:

src/activities/settings/CategorySettingsActivity.cpp

@@ -203,10 +203,6 @@ void CategorySettingsActivity::render() const {
     renderer.drawText(UI_10_FONT_ID, 20, settingY, action.name, !isSelected);
   }
 
-  // Draw version text above button hints
-  renderer.drawText(SMALL_FONT_ID, pageWidth - 20 - renderer.getTextWidth(SMALL_FONT_ID, CROSSPOINT_VERSION),
-                    pageHeight - 60, CROSSPOINT_VERSION);
-
   // Draw help text
   const auto labels = mappedInput.mapLabels("« Back", "Toggle", "", "");
   renderer.drawButtonHints(UI_10_FONT_ID, labels.btn1, labels.btn2, labels.btn3, labels.btn4);

src/activities/settings/SettingsActivity.cpp

@@ -3,8 +3,6 @@
 #include <GfxRenderer.h>
 #include <HardwareSerial.h>
 
-#include <algorithm>
-
 #include "CategorySettingsActivity.h"
 #include "CrossPointSettings.h"
 #include "MappedInputManager.h"
@@ -14,9 +12,12 @@ const char* SettingsActivity::categoryNames[categoryCount] = {"Display", "Reader
 
 // Helper function to find descriptor by member pointer
 static const SettingDescriptor* findDescriptor(uint8_t CrossPointSettings::* memberPtr) {
-  auto it = std::find_if(CrossPointSettings::descriptors.begin(), CrossPointSettings::descriptors.end(),
+  for (const auto& desc : CrossPointSettings::descriptors) {
-                         [memberPtr](const SettingDescriptor& desc) { return desc.memberPtr == memberPtr; });
+    if (desc.memberPtr == memberPtr) {
-  return (it != CrossPointSettings::descriptors.end()) ? &(*it) : nullptr;
+      return &desc;
+    }
+  }
+  return nullptr;
 }
 
 void SettingsActivity::taskTrampoline(void* param) {
@@ -131,10 +132,10 @@ void SettingsActivity::enterCategory(int categoryIndex) {
   xSemaphoreTake(renderingMutex, portMAX_DELAY);
   exitActivity();
   enterNewActivity(new CategorySettingsActivity(renderer, mappedInput, categoryNames[categoryIndex], descriptors,
-                                                actionItems, [this] {
+                                                 actionItems, [this] {
-                                                  exitActivity();
+                                                   exitActivity();
-                                                  updateRequired = true;
+                                                   updateRequired = true;
-                                                }));
+                                                 }));
   xSemaphoreGive(renderingMutex);
 }
 
@@ -168,6 +169,7 @@ void SettingsActivity::render() const {
 
     // Draw category name
     renderer.drawText(UI_10_FONT_ID, 20, categoryY, categoryNames[i], i != selectedCategoryIndex);
+
   }
 
   // Draw version text above button hints