Merge 5ae10a7eb6 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
- do not boot-loop on bad string values
- (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 6213378 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,4 +49,12 @@ 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,20 +29,16 @@ void SettingDescriptor::setValue(CrossPointSettings& settings, uint8_t value) co
 
 void SettingDescriptor::resetToDefault(CrossPointSettings& settings) const {
   if (type == SettingType::STRING) {
-    char* str = getStringPtr(settings);
-    if (str && stringData.defaultString) {
-      strncpy(str, stringData.defaultString, stringData.maxSize - 1);
-      str[stringData.maxSize - 1] = '\0';
-    }
+    strncpy(stringPtr, stringData.defaultString, stringData.maxSize - 1);
+    stringPtr[stringData.maxSize - 1] = '\0';
     return;
   }
-  settings.*(memberPtr) = defaultValue;
+  setValue(settings, defaultValue);
 }
 
 void SettingDescriptor::save(FsFile& file, const CrossPointSettings& settings) const {
   if (type == SettingType::STRING) {
-    const char* str = getStringPtr(settings);
-    serialization::writeString(file, std::string(str ? str : ""));
+    serialization::writeString(file, std::string(stringPtr));
     return;
   }
   serialization::writePod(file, settings.*(memberPtr));
@@ -50,13 +46,7 @@ void SettingDescriptor::save(FsFile& file, const CrossPointSettings& settings) c
 
 void SettingDescriptor::load(FsFile& file, CrossPointSettings& settings) const {
   if (type == SettingType::STRING) {
-    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';
-    }
+    serialization::readString(file, stringPtr, stringData.maxSize);
     return;
   }
   uint8_t value;
@@ -96,8 +86,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, offsetof(CrossPointSettings, member), defStr, \
+#define makeStringDescriptor(name, member, defStr)                                          \
+  SettingDescriptor(name, SettingType::STRING, CrossPointSettings::instance.member, defStr, \
                     sizeof(CrossPointSettings::member))
 }  // namespace
 
@@ -157,10 +147,9 @@ bool CrossPointSettings::saveToFile() const {
   }
 
   serialization::writePod(outputFile, SETTINGS_FILE_VERSION);
-  serialization::writePod(outputFile, CrossPointSettings::DESCRIPTOR_COUNT);
+  serialization::writePod(outputFile, static_cast<uint8_t>(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);
@@ -193,7 +182,6 @@ 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;
 
@@ -203,20 +191,15 @@ 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,7 +30,10 @@ struct ValueRange {
 
 // Concrete descriptor for uint8_t settings (constexpr-compatible)
 struct SettingDescriptor : public SettingDescriptorBase {
-  uint8_t CrossPointSettings::* memberPtr;  // Member pointer (unused for STRING type)
+  union {
+    uint8_t CrossPointSettings::* memberPtr;  // For TOGGLE/ENUM/VALUE types
+    char* stringPtr;                          // For STRING type
+  };
   uint8_t defaultValue;
   SettingValidator validator;  // Optional validator function
 
@@ -46,7 +49,6 @@ 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;
@@ -72,12 +74,12 @@ struct SettingDescriptor : public SettingDescriptorBase {
         valueRange(valRange) {}
 
   // STRING constructor
-  constexpr SettingDescriptor(const char* name_, SettingType type_, size_t offset, const char* defStr, size_t maxSz)
+  constexpr SettingDescriptor(const char* name_, SettingType type_, char* strPtr, const char* defStr, size_t maxSz)
       : SettingDescriptorBase{name_, type_},
-        memberPtr(nullptr),  // Not used for strings
+        stringPtr(strPtr),
         defaultValue(0),
         validator(nullptr),
-        stringData{offset, defStr, maxSz} {}
+        stringData{defStr, maxSz} {}
 
   bool validate(const CrossPointSettings& settings) const;
   uint8_t getValue(const CrossPointSettings& settings) const;
@@ -93,17 +95,6 @@ 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)