Improve the ConfigItem callback system.

- The callback is now only called when then config menu has been closed and value has actually changed (or an newer value has been read from SD). This increased the performance!
- The WUPSConfigItemBoolean and WUPSConfigItemMultipleValues callbacks now provide a pointer to the causes config item.

loader/src/myutils/ConfigInformation.cpp

@@ -82,6 +82,7 @@ bool ConfigInformation::loadValuesFromSD() {
             if(prevValue.compare(loadedValue) != 0) {
                 //DEBUG_FUNCTION_LINE("Call loadValue\n");
                 curItem->loadValue(loadedValue);
+                // calling the callback here is _NOT_ needed. It will be called when the menu is closed.
             }
         }
     }
@@ -105,7 +106,12 @@ bool ConfigInformation::updateConfigSettings() {
         for (auto & curItem : curCat->getItems()) {
             std::string configID = curItem->getConfigID();
             std::string newValue = curItem->persistValue();
-            this->configSettings->setValueAsString(this->configSettings->getIdByName(configID), newValue);
+            if(this->configSettings->setValueAsString(this->configSettings->getIdByName(configID), newValue)){
+                // When the value has changed, call the callback.
+                DEBUG_FUNCTION_LINE("Called callback. Reason. Menu was closed and value has changed\n");
+                curItem->callCallback();
+            }
+
             //DEBUG_FUNCTION_LINE("Set %s(%d) to %s\n",configID.c_str(),this->configSettings->getIdByName(configID), newValue.c_str());
         }
     }

loader/src/settings/ConfigSettings.h

@@ -189,17 +189,23 @@ public:
         }
     }
 
-    void setValueAsString(int32_t idx, const std::string & val) {
+    bool setValueAsString(int32_t idx, const std::string & val) {
         if(isValidId(idx) && settingsValues[idx].dataType == TypeString && settingsValues[idx].strValue) {
             if(val.compare(*(settingsValues[idx].strValue)) != 0) { // Only update if the value changed.
                 *(settingsValues[idx].strValue) = val;
                 bChanged = true;
+                return true;
             }
         }
+        return false;
     }
 
     int32_t getIdByName(std::string configID);
 
+    bool hasChanged(){
+        return bChanged;
+    }
+
 protected:
     bool ValidVersion(const std::string & versionString);
 

src/config/WUPSConfigItemBoolean.cpp

@@ -82,19 +82,21 @@ void WUPSConfigItemBoolean::loadValue(std::string persistedValue) {
     if(persistedValue.compare("1") == 0) {
         newValue = true;
     }
-
-    if(newValue != value) {
-        toggleValue();
-    }
+    value = newValue;
 }
 
 void WUPSConfigItemBoolean::toggleValue() {
     value = !value;
-    if(callback != NULL) {
-        callback(value);
-    }
 }
 
 void WUPSConfigItemBoolean::restoreDefault() {
     this->value = this->defaultValue;
 }
+
+bool WUPSConfigItemBoolean::callCallback(){
+    if(callback != NULL) {
+        callback(this, this->value);
+        return true;
+    }
+    return false;
+}

src/config/WUPSConfigItemMultipleValues.cpp

@@ -18,6 +18,7 @@
 #include <string>
 #include <vector>
 #include <wups/config/WUPSConfigItemMultipleValues.h>
+#include <stdlib.h>     /* atoi */
 
 WUPSConfigItemMultipleValues::WUPSConfigItemMultipleValues(std::string configID, std::string displayName, int32_t defaultValue, std::map<int32_t,std::string> values_, MultipleValuesChangedCallback callback) : WUPSConfigItem(configID,displayName)  {
     if(values_.size() == 0) {
@@ -62,7 +63,12 @@ std::string WUPSConfigItemMultipleValues::getCurrentValueDisplay() {
 }
 
 std::string WUPSConfigItemMultipleValues::getCurrentValueSelectedDisplay() {
-    uint32_t index_max = values.size()-1;
+    if(values.size() == 0) {
+        return "<ERROR>";
+    }
+
+    uint32_t index_max = values.size() -1;
+
     uint32_t index = 0;
     for (auto& kv : values) {
         if(index == valueIndex) {
@@ -89,7 +95,6 @@ void WUPSConfigItemMultipleValues::onSelected(bool isSelected) {
 }
 
 void WUPSConfigItemMultipleValues::onButtonPressed(WUPSConfigButtons buttons) {
-    uint32_t previousValue = valueIndex;
     if(buttons & WUPS_CONFIG_BUTTON_LEFT) {
         if(valueIndex != 0){
             valueIndex--;
@@ -99,17 +104,6 @@ void WUPSConfigItemMultipleValues::onButtonPressed(WUPSConfigButtons buttons) {
         valueIndex++;
         if(valueIndex > values.size()-1) {
             valueIndex = values.size()-1;
-
-        }
-    }
-    if(previousValue != valueIndex) {
-        uint32_t index = 0;
-        for (auto& kv : values) {
-            if(index == valueIndex) {
-                callback(kv.first);
-                break;
-            }
-            index++;
         }
     }
 }
@@ -123,20 +117,31 @@ std::string WUPSConfigItemMultipleValues::persistValue() {
 }
 
 void WUPSConfigItemMultipleValues::loadValue(std::string persistedValue) {
-    uint32_t newValueIndex = std::stoi(persistedValue);
-    if(newValueIndex != valueIndex) {
-        uint32_t index = 0;
-        for (auto& kv : values) {
-            if(index == newValueIndex) {
-                valueIndex = newValueIndex;
-                callback(kv.first);
-                break;
-            }
-            index++;
-        }
+    // Note: std::stoi would throw an exception on error. atoi leads to an undefined behavior, but we
+    // check if the result is in range anyway.
+    uint32_t newValueIndex = atoi(persistedValue.c_str());
+    if(newValueIndex >= 0 && (newValueIndex +1) <= this->values.size()) {
+        valueIndex = newValueIndex;
+    }else{
+        valueIndex = 0;
     }
 }
 
 void WUPSConfigItemMultipleValues::restoreDefault() {
     this->valueIndex = 0;
 }
+
+bool WUPSConfigItemMultipleValues::callCallback() {
+    if(callback == NULL) {
+        return false;
+    }
+    uint32_t index = 0;
+    for (auto& kv : values) {
+        if(index == valueIndex) {
+            callback(this, kv.first);
+            return true;
+        }
+        index++;
+    }
+    return false;
+}

wups_include/wups/config/WUPSConfigItem.h

@@ -115,6 +115,16 @@ public:
     **/
     virtual void restoreDefault() = 0;
 
+
+    /**
+        Call callback with with current value.
+        This function will be called whenever this item should call it's (optional) given
+        callback with the current value.
+        Returns true if a valid callback could be called
+        Returns false if no callback was called (e.g. callback was NULL)
+    **/
+    virtual bool callCallback() = 0;
+
     WUPSConfigItem(std::string configID, std::string displayName){
         this->configID = configID;
         this->displayName = displayName;
@@ -122,6 +132,7 @@ public:
 
     virtual ~WUPSConfigItem(){
     }
+
 private:
     std::string displayName;
 

wups_include/wups/config/WUPSConfigItemBoolean.h

@@ -22,7 +22,9 @@
 #include <vector>
 #include <wups/config/WUPSConfigItem.h>
 
-typedef void (*BooleanValueChangedCallback)(bool);
+class WUPSConfigItemBoolean;
+
+typedef void (*BooleanValueChangedCallback)(WUPSConfigItemBoolean *, bool);
 
 class WUPSConfigItemBoolean : public WUPSConfigItem {
 public:
@@ -64,6 +66,8 @@ public:
 
     virtual void restoreDefault();
 
+    virtual bool callCallback();
+
 private:
     BooleanValueChangedCallback callback = NULL;
     bool value;

wups_include/wups/config/WUPSConfigItemMultipleValues.h

@@ -22,7 +22,9 @@
 #include <map>
 #include <wups/config/WUPSConfigItem.h>
 
-typedef void (*MultipleValuesChangedCallback)(int32_t);
+class WUPSConfigItemMultipleValues;
+
+typedef void (*MultipleValuesChangedCallback)(WUPSConfigItemMultipleValues *, int32_t);
 
 class WUPSConfigItemMultipleValues : public WUPSConfigItem {
 public:
@@ -46,6 +48,8 @@ public:
 
     virtual void restoreDefault();
 
+    virtual bool callCallback();
+
 private:
     MultipleValuesChangedCallback callback = NULL;
     int32_t defaultValue;