Fixed Issue #43

Fixed issue #43 Saving Presets doesn't survive power off. Fix is not ideal, but gets things back to working.
bkshepherd · Dec 28, 2024 · 8dda5f2 · 8dda5f2
1 parent db73a0e
commit 8dda5f2
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 13 deletions.
diff --git a/Software/GuitarPedal/guitar_pedal_storage.cpp b/Software/GuitarPedal/guitar_pedal_storage.cpp
@@ -9,8 +9,6 @@ extern BaseEffectModule **availableEffects;
 extern int activeEffectID;
 extern BaseEffectModule *activeEffect;
 
-uint32_t DSY_SDRAM_BSS globalEffectsSettings[SETTINGS_ABSOLUTE_MAX_PARAM_COUNT];
-
 uint32_t GetDefaultTotalIdxOfGlobalSettingsBlock() {
     uint32_t tempSize = 0;
 
@@ -30,7 +28,7 @@ void InitPersistantStorage() {
     defaultSettings.globalMidiThrough = true;
     defaultSettings.globalRelayBypassEnabled = false;
     defaultSettings.globalSplitMonoInputToStereo = true;
-    defaultSettings.globalEffectsSettings = globalEffectsSettings;
+
     // All Effect Params in the settings should be zero'd
     for (int i = 0; i < SETTINGS_ABSOLUTE_MAX_PARAM_COUNT; i++) {
         defaultSettings.globalEffectsSettings[i] = 0;
@@ -39,13 +37,15 @@ void InitPersistantStorage() {
     uint32_t globalEffectsSettingMemIdx = 0U;
     defaultSettings.globalEffectsSettings[globalEffectsSettingMemIdx] = GetDefaultTotalIdxOfGlobalSettingsBlock();
     ++globalEffectsSettingMemIdx;
+
     // Override any defaults with effect specific default settings
     for (int effectID = 0; effectID < availableEffectsCount; effectID++) {
         int paramCount = availableEffects[effectID]->GetParameterCount();
         // Change first word of each effect such that this is total number of presets
         // Default value: 1
         defaultSettings.globalEffectsSettings[globalEffectsSettingMemIdx] = 1U;
         ++globalEffectsSettingMemIdx;
+
         // Next word is going to be the number of parameters
         defaultSettings.globalEffectsSettings[globalEffectsSettingMemIdx] = paramCount;
         ++globalEffectsSettingMemIdx;
@@ -55,14 +55,13 @@ void InitPersistantStorage() {
 
             if (availableEffects[effectID]->GetParameterType(paramID) == ParameterValueType::FloatMagnitude) {
                 uint32_t tmp;
-                float f = defaultSettings.globalEffectsSettings[globalEffectsSettingMemIdx] =
-                    availableEffects[effectID]->GetParameterAsFloat(paramID);
+                float f = defaultSettings.globalEffectsSettings[globalEffectsSettingMemIdx] = availableEffects[effectID]->GetParameterAsFloat(paramID);
                 std::memcpy(&tmp, &f, sizeof(float));
                 defaultSettings.globalEffectsSettings[globalEffectsSettingMemIdx] = tmp;
             } else {
-                defaultSettings.globalEffectsSettings[globalEffectsSettingMemIdx] =
-                    availableEffects[effectID]->GetParameterRaw(paramID);
+                defaultSettings.globalEffectsSettings[globalEffectsSettingMemIdx] = availableEffects[effectID]->GetParameterRaw(paramID);
             }
+
             ++globalEffectsSettingMemIdx;
         }
     }
@@ -230,6 +229,7 @@ void SaveEffectSettingsToPersitantStorageForEffectID(int effectID, uint32_t pres
     bool canWriteNewPreset = true;
     Settings &settings = storage.GetSettings();
     uint32_t globalEffectsMaxIdx = settings.globalEffectsSettings[0U];
+
     // Save Effect Parameters to Persistant Storage based on values from the specified active effect
     if (effectID >= 0 && effectID < availableEffectsCount) {
         int paramCount = availableEffects[effectID]->GetParameterCount();

diff --git a/Software/GuitarPedal/guitar_pedal_storage.h b/Software/GuitarPedal/guitar_pedal_storage.h
@@ -3,11 +3,15 @@
 #define GUITAR_PEDAL_STORAGE_H
 
 // Peristant Storage Settings
-#define SETTINGS_FILE_FORMAT_VERSION 2
+#define SETTINGS_FILE_FORMAT_VERSION 3
 
-// Absolute maximum on current system, arbitrarily limiting this to 64KB
-#define SETTINGS_ABSOLUTE_MAX_PARAM_COUNT 16000
+// Arbitrarily limiting this to 4KB of stored presets since this sits in DTCMRAM which is limited to 128KB.
+// TODO: In the future it would be better if this worked with the QSPI directly instead of using
+// the PersistentStorage class as an abstraction since that only lets you store a fixed struct size.
+// then it would be possible to not have to pre-allocate a fixed size in DTCMRAM for the preset save data.
+#define SETTINGS_ABSOLUTE_MAX_PARAM_COUNT 1024
 #define ERR_VALUE_MAX 0xffffffff
+
 // Save System Variables
 struct Settings {
     int fileFormatVersion;
@@ -17,7 +21,12 @@ struct Settings {
     int globalMidiChannel;
     bool globalRelayBypassEnabled;
     bool globalSplitMonoInputToStereo;
-    uint32_t *globalEffectsSettings; // Set aside a block of memory for individual effect params
+
+    // Set aside a block of memory for individual effect params. 
+    // Please note this MUST be a fixed amount of memory in the struct and cannot be a pointer to dynamic memory!
+    // If you try to use a pointer it will only save the pointer address to QSPI storage and not any of the contents
+    // of that dynamic memory.  This is a limitation of the way the PersistantStorage helper class works.
+    uint32_t globalEffectsSettings[SETTINGS_ABSOLUTE_MAX_PARAM_COUNT];
 
     bool operator==(const Settings &rhs) {
         if (fileFormatVersion != rhs.fileFormatVersion || globalActiveEffectID != rhs.globalActiveEffectID ||
@@ -27,7 +36,7 @@ struct Settings {
             return false;
         }
 
-        for (uint32_t i = 0; i < globalEffectsSettings[0]; i++) {
+        for (uint32_t i = 0; i < SETTINGS_ABSOLUTE_MAX_PARAM_COUNT; i++) {
             if (globalEffectsSettings[i] != rhs.globalEffectsSettings[i]) {
                 return false;
             }
@@ -42,7 +51,6 @@ struct Settings {
 void InitPersistantStorage();
 void LoadEffectSettingsFromPersistantStorage();
 void SaveEffectSettingsToPersitantStorageForEffectID(int effectID, uint32_t presetID);
-uint32_t GetSettingsParameterValueForEffect(int effectID, int paramID);
 void SetSettingsParameterValueForEffect(int effectID, int paramID, uint32_t paramValue, uint32_t startIdx);
 void LoadPresetFromPersistentStorage(uint32_t effectID, uint32_t presetID);
 void FactoryReset(void *context);