Merge 89aa47e6c8 into da4d3b5ea5

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
- (de)serialization is based on a single list of descriptors: single point
  of truth
- possibly to have default values for the String configuration type
- more constexpr to reduce RAM usage
- less hardcoded values
- more maintainable

RAM Usage:
 From
RAM:   [===       ]  32.5% (used 106516 bytes from 327680 bytes)
Flash: [========= ]  94.9% (used 6217240 bytes from 6553600 bytes)
 To
RAM:   [===       ]  32.3% (used 105844 bytes from 327680 bytes)
Flash: [========= ]  94.8% (used 6213636 bytes from 6553600 bytes)

Boot config validation with a test where the status bar config is wrong:
[1256] [SD] SD card detected
[1256] [CPS] Loading settings from file
[1265] [CPS] Invalid value (0x3) for Status Bar, resetting to default
[1265] [CPS] Settings loaded from file

lib/Serialization/Serialization.h

@@ -49,12 +49,4 @@ static void readString(FsFile& file, std::string& s) {
   s.resize(len);
   file.read(&s[0], len);
 }
-
-static void readString(FsFile& file, char* buffer, size_t maxLen) {
-  uint32_t len;
-  readPod(file, len);
-  const uint32_t bytesToRead = (len < maxLen - 1) ? len : (maxLen - 1);
-  file.read(reinterpret_cast<uint8_t*>(buffer), bytesToRead);
-  buffer[bytesToRead] = '\0';
-}
 }  // namespace serialization

src/CrossPointSettings.cpp

@@ -29,16 +29,20 @@ void SettingDescriptor::setValue(CrossPointSettings& settings, uint8_t value) co
 
 void SettingDescriptor::resetToDefault(CrossPointSettings& settings) const {
   if (type == SettingType::STRING) {
-    strncpy(stringPtr, stringData.defaultString, stringData.maxSize - 1);
-    stringPtr[stringData.maxSize - 1] = '\0';
+    char* str = getStringPtr(settings);
+    if (str && stringData.defaultString) {
+      strncpy(str, stringData.defaultString, stringData.maxSize - 1);
+      str[stringData.maxSize - 1] = '\0';
+    }
     return;
   }
-  setValue(settings, defaultValue);
+  settings.*(memberPtr) = defaultValue;
 }
 
 void SettingDescriptor::save(FsFile& file, const CrossPointSettings& settings) const {
   if (type == SettingType::STRING) {
-    serialization::writeString(file, std::string(stringPtr));
+    const char* str = getStringPtr(settings);
+    serialization::writeString(file, std::string(str ? str : ""));
     return;
   }
   serialization::writePod(file, settings.*(memberPtr));
@@ -46,7 +50,13 @@ void SettingDescriptor::save(FsFile& file, const CrossPointSettings& settings) c
 
 void SettingDescriptor::load(FsFile& file, CrossPointSettings& settings) const {
   if (type == SettingType::STRING) {
-    serialization::readString(file, stringPtr, stringData.maxSize);
+    char* str = getStringPtr(settings);
+    if (str) {
+      std::string tempStr;
+      serialization::readString(file, tempStr);
+      strncpy(str, tempStr.c_str(), stringData.maxSize - 1);
+      str[stringData.maxSize - 1] = '\0';
+    }
     return;
   }
   uint8_t value;
@@ -86,8 +96,8 @@ constexpr SettingDescriptor makeEnumDescriptor(const char* name, uint8_t CrossPo
 }
 
 // Helper macro to create STRING descriptors without repetition
-#define makeStringDescriptor(name, member, defStr)                                          \
-  SettingDescriptor(name, SettingType::STRING, CrossPointSettings::instance.member, defStr, \
+#define makeStringDescriptor(name, member, defStr)                                           \
+  SettingDescriptor(name, SettingType::STRING, offsetof(CrossPointSettings, member), defStr, \
                     sizeof(CrossPointSettings::member))
 }  // namespace
 
@@ -147,9 +157,10 @@ bool CrossPointSettings::saveToFile() const {
   }
 
   serialization::writePod(outputFile, SETTINGS_FILE_VERSION);
-  serialization::writePod(outputFile, static_cast<uint8_t>(CrossPointSettings::DESCRIPTOR_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;
   for (const auto& desc : descriptors) {
     desc.save(outputFile, *this);
@@ -182,6 +193,7 @@ 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;
 
@@ -191,15 +203,20 @@ bool CrossPointSettings::loadFromFile() {
     }
 
     desc.load(inputFile, *this);
+    descriptorIndex++;
+    filePosition++;
+  }
+
+  inputFile.close();
+
+  // 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);
       desc.resetToDefault(*this);
     }
-    descriptorIndex++;
-    filePosition++;
   }
-  inputFile.close();
 
   Serial.printf("[%lu] [CPS] Settings loaded from file\n", millis());
   return true;

src/CrossPointSettings.h

@@ -30,10 +30,7 @@ struct ValueRange {
 
 // Concrete descriptor for uint8_t settings (constexpr-compatible)
 struct SettingDescriptor : public SettingDescriptorBase {
-  union {
-    uint8_t CrossPointSettings::* memberPtr;  // For TOGGLE/ENUM/VALUE types
-    char* stringPtr;                          // For STRING type
-  };
+  uint8_t CrossPointSettings::* memberPtr;  // Member pointer (unused for STRING type)
   uint8_t defaultValue;
   SettingValidator validator;  // Optional validator function
 
@@ -49,6 +46,7 @@ struct SettingDescriptor : public SettingDescriptorBase {
 
     // For STRING types
     struct {
+      size_t offset;              // Offset of the string member in CrossPointSettings
       const char* defaultString;  // Default string value
       size_t maxSize;             // Max size of the string buffer
     } stringData;
@@ -74,12 +72,12 @@ struct SettingDescriptor : public SettingDescriptorBase {
         valueRange(valRange) {}
 
   // STRING constructor
-  constexpr SettingDescriptor(const char* name_, SettingType type_, char* strPtr, const char* defStr, size_t maxSz)
+  constexpr SettingDescriptor(const char* name_, SettingType type_, size_t offset, const char* defStr, size_t maxSz)
       : SettingDescriptorBase{name_, type_},
-        stringPtr(strPtr),
+        memberPtr(nullptr),  // Not used for strings
         defaultValue(0),
         validator(nullptr),
-        stringData{defStr, maxSz} {}
+        stringData{offset, defStr, maxSz} {}
 
   bool validate(const CrossPointSettings& settings) const;
   uint8_t getValue(const CrossPointSettings& settings) const;
@@ -95,6 +93,17 @@ struct SettingDescriptor : public SettingDescriptorBase {
     }
     return "";
   }
+
+  // Helpers for STRING type - get pointer to string member using offset
+  char* getStringPtr(CrossPointSettings& settings) const {
+    if (type != SettingType::STRING) return nullptr;
+    return reinterpret_cast<char*>(reinterpret_cast<uint8_t*>(&settings) + stringData.offset);
+  }
+
+  const char* getStringPtr(const CrossPointSettings& settings) const {
+    if (type != SettingType::STRING) return nullptr;
+    return reinterpret_cast<const char*>(reinterpret_cast<const uint8_t*>(&settings) + stringData.offset);
+  }
 };
 
 // Validator functions (constexpr for compile-time optimization)