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",
-                                                   "Left/Back/Confirm/Right"};
+constexpr const char* frontButtonLayoutValues[] = {"Back/Confirm/Left/Right", "Left/Right/Back/Confirm", "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"};
@@ -63,52 +68,44 @@ constexpr const char* hideBatteryPercentageValues[] = {"Never", "In Reader", "Al
 
 // Helper function template to deduce array size automatically
 template<size_t N>
-constexpr SettingDescriptor makeEnumDescriptor(const char* name, uint8_t CrossPointSettings::* ptr,
-                                               uint8_t defaultValue, const char* const (&enumValues)[N]) {
-  return SettingDescriptor(name, SettingType::ENUM, ptr, defaultValue, validateEnum<N>, enumValues, N);
+constexpr SettingDescriptor makeEnumDescriptor(
+    const char* name,
+    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 = {
-    {makeEnumDescriptor("Sleep Screen", &CrossPointSettings::sleepScreen, CrossPointSettings::DARK, sleepScreenValues),
-     {"Extra Paragraph Spacing", SettingType::TOGGLE, &CrossPointSettings::extraParagraphSpacing, 1, validateToggle,
-      nullptr, 0},
-     makeEnumDescriptor("Short Power Button Click", &CrossPointSettings::shortPwrBtn, CrossPointSettings::IGNORE,
-                        shortPwrBtnValues),
+const std::array<SettingDescriptor, CrossPointSettings::DESCRIPTOR_COUNT> CrossPointSettings::descriptors = {{
+    makeEnumDescriptor("Sleep Screen", &CrossPointSettings::sleepScreen, CrossPointSettings::DARK, sleepScreenValues),
+    {"Extra Paragraph Spacing", SettingType::TOGGLE, &CrossPointSettings::extraParagraphSpacing, 1, validateToggle, nullptr, 0},
+    makeEnumDescriptor("Short Power Button Click", &CrossPointSettings::shortPwrBtn, CrossPointSettings::IGNORE, shortPwrBtnValues),
     makeEnumDescriptor("Status Bar", &CrossPointSettings::statusBar, CrossPointSettings::FULL, statusBarValues),
-     makeEnumDescriptor("Reading Orientation", &CrossPointSettings::orientation, CrossPointSettings::PORTRAIT,
-                        orientationValues),
-     makeEnumDescriptor("Front Button Layout", &CrossPointSettings::frontButtonLayout,
-                        CrossPointSettings::BACK_CONFIRM_LEFT_RIGHT, frontButtonLayoutValues),
-     makeEnumDescriptor("Side Button Layout", &CrossPointSettings::sideButtonLayout, CrossPointSettings::PREV_NEXT,
-                        sideButtonLayoutValues),
-     makeEnumDescriptor("Reader Font Family", &CrossPointSettings::fontFamily, CrossPointSettings::BOOKERLY,
-                        fontFamilyValues),
+    makeEnumDescriptor("Reading Orientation", &CrossPointSettings::orientation, CrossPointSettings::PORTRAIT, orientationValues),
+    makeEnumDescriptor("Front Button Layout", &CrossPointSettings::frontButtonLayout, CrossPointSettings::BACK_CONFIRM_LEFT_RIGHT, frontButtonLayoutValues),
+    makeEnumDescriptor("Side Button Layout", &CrossPointSettings::sideButtonLayout, CrossPointSettings::PREV_NEXT, sideButtonLayoutValues),
+    makeEnumDescriptor("Reader Font Family", &CrossPointSettings::fontFamily, CrossPointSettings::BOOKERLY, fontFamilyValues),
     makeEnumDescriptor("Reader Font Size", &CrossPointSettings::fontSize, CrossPointSettings::MEDIUM, fontSizeValues),
-     makeEnumDescriptor("Reader Line Spacing", &CrossPointSettings::lineSpacing, CrossPointSettings::NORMAL,
-                        lineSpacingValues),
-     makeEnumDescriptor("Reader Paragraph Alignment", &CrossPointSettings::paragraphAlignment,
-                        CrossPointSettings::JUSTIFIED, paragraphAlignmentValues),
-     makeEnumDescriptor("Time to Sleep", &CrossPointSettings::sleepTimeout, CrossPointSettings::SLEEP_10_MIN,
-                        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),
+    makeEnumDescriptor("Reader Line Spacing", &CrossPointSettings::lineSpacing, CrossPointSettings::NORMAL, lineSpacingValues),
+    makeEnumDescriptor("Reader Paragraph Alignment", &CrossPointSettings::paragraphAlignment, CrossPointSettings::JUSTIFIED, paragraphAlignmentValues),
+    makeEnumDescriptor("Time to Sleep", &CrossPointSettings::sleepTimeout, CrossPointSettings::SLEEP_10_MIN, 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}}};
+    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 descriptorIndex = 0;
+  uint8_t settingsWritten = 0;
+
   for (const auto& desc : descriptors) {
-    // Write opdsServerUrl string before descriptor 15 (at position 15)
-    if (descriptorIndex == 15) {
+    // Special handling for opdsServerUrl (string field at position 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 descriptorIndex = 0;
-  uint8_t filePosition = 0;
+  uint8_t settingsRead = 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)
-    if (descriptorIndex == 15) {
+    // Special handling for opdsServerUrl (string field at position 15)
+    if (settingsRead == 15) {
+      if (settingsRead < fileSettingsCount) {
         std::string urlStr;
         serialization::readString(inputFile, urlStr);
         strncpy(opdsServerUrl, urlStr.c_str(), sizeof(opdsServerUrl) - 1);
         opdsServerUrl[sizeof(opdsServerUrl) - 1] = '\0';
-      filePosition++;
-
-      if (filePosition >= fileSettingsCount) {
+        settingsRead++;
+      }
+      if (settingsRead >= fileSettingsCount) {
         break;
       }
     }
 
     desc.load(inputFile, *this);
-    descriptorIndex++;
-    filePosition++;
+    settingsRead++;
   }
 
   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),
-                    desc.name);
+      Serial.printf("[%lu] [CPS] Invalid value for %s, resetting to default\n", millis(), desc.name);
       desc.resetToDefault(*this);
     }
   }

src/CrossPointSettings.h

@@ -1,9 +1,9 @@
 #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 };
@@ -47,21 +47,19 @@ struct SettingDescriptor : public SettingDescriptorBase {
 
     // Constexpr constructors for different setting types
     // TOGGLE/ENUM constructor
-  constexpr SettingDescriptor(const char* name_, SettingType type_, uint8_t CrossPointSettings::* ptr, uint8_t defVal,
-                              SettingValidator val, const char* const* enumVals, uint8_t enumCnt)
+    constexpr SettingDescriptor(const char* name_, SettingType type_,
+                                 uint8_t CrossPointSettings::* ptr, uint8_t defVal, SettingValidator val,
+                                 const char* const* enumVals, uint8_t enumCnt)
         : SettingDescriptorBase{name_, type_},
-        memberPtr(ptr),
-        defaultValue(defVal),
-        validator(val),
+          memberPtr(ptr), defaultValue(defVal), validator(val),
           enumData{enumVals, enumCnt} {}
 
     // VALUE constructor
-  constexpr SettingDescriptor(const char* name_, SettingType type_, uint8_t CrossPointSettings::* ptr, uint8_t defVal,
-                              SettingValidator val, ValueRange valRange)
+    constexpr SettingDescriptor(const char* name_, SettingType type_,
+                                 uint8_t CrossPointSettings::* ptr, uint8_t defVal, SettingValidator val,
+                                 ValueRange valRange)
         : SettingDescriptorBase{name_, type_},
-        memberPtr(ptr),
-        defaultValue(defVal),
-        validator(val),
+          memberPtr(ptr), defaultValue(defVal), validator(val),
           valueRange(valRange) {}
 
     bool validate(const CrossPointSettings& settings) const;
@@ -80,16 +78,13 @@ struct SettingDescriptor : public SettingDescriptorBase {
     }
 };
 
+
 // Validator functions (constexpr for compile-time optimization)
 constexpr bool validateToggle(uint8_t v) { return v <= 1; }
 template<uint8_t MAX>
-constexpr bool validateEnum(uint8_t v) {
-  return v < MAX;
-}
+constexpr bool validateEnum(uint8_t v) { return v < MAX; }
 template<uint8_t MIN, uint8_t MAX>
-constexpr bool validateRange(uint8_t v) {
-  return v >= MIN && v <= 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(),
-                         [memberPtr](const SettingDescriptor& desc) { return desc.memberPtr == memberPtr; });
-  return (it != CrossPointSettings::descriptors.end()) ? &(*it) : nullptr;
+  for (const auto& desc : CrossPointSettings::descriptors) {
+    if (desc.memberPtr == memberPtr) {
+      return &desc;
+    }
+  }
+  return nullptr;
 }
 
 void SettingsActivity::taskTrampoline(void* param) {
@@ -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