From b1d89c9d4d931fac5afd6ca3ca943f4515583201 Mon Sep 17 00:00:00 2001 From: Maschell Date: Fri, 14 Sep 2018 13:02:06 +0200 Subject: [PATCH] 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 | 8 ++- loader/src/settings/ConfigSettings.h | 8 ++- src/config/WUPSConfigItemBoolean.cpp | 16 +++--- src/config/WUPSConfigItemMultipleValues.cpp | 53 ++++++++++--------- wups_include/wups/config/WUPSConfigItem.h | 11 ++++ .../wups/config/WUPSConfigItemBoolean.h | 6 ++- .../config/WUPSConfigItemMultipleValues.h | 6 ++- 7 files changed, 73 insertions(+), 35 deletions(-) diff --git a/loader/src/myutils/ConfigInformation.cpp b/loader/src/myutils/ConfigInformation.cpp index 6d51bd0..4dd0bce 100644 --- a/loader/src/myutils/ConfigInformation.cpp +++ b/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()); } } diff --git a/loader/src/settings/ConfigSettings.h b/loader/src/settings/ConfigSettings.h index b1a9774..57f8267 100644 --- a/loader/src/settings/ConfigSettings.h +++ b/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); diff --git a/src/config/WUPSConfigItemBoolean.cpp b/src/config/WUPSConfigItemBoolean.cpp index fff3b24..0fd7411 100644 --- a/src/config/WUPSConfigItemBoolean.cpp +++ b/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; +} diff --git a/src/config/WUPSConfigItemMultipleValues.cpp b/src/config/WUPSConfigItemMultipleValues.cpp index 7f2134b..c9682a2 100644 --- a/src/config/WUPSConfigItemMultipleValues.cpp +++ b/src/config/WUPSConfigItemMultipleValues.cpp @@ -18,6 +18,7 @@ #include #include #include +#include /* atoi */ WUPSConfigItemMultipleValues::WUPSConfigItemMultipleValues(std::string configID, std::string displayName, int32_t defaultValue, std::map 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 ""; + } + + 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; +} diff --git a/wups_include/wups/config/WUPSConfigItem.h b/wups_include/wups/config/WUPSConfigItem.h index fbfd663..08ed4b9 100644 --- a/wups_include/wups/config/WUPSConfigItem.h +++ b/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; diff --git a/wups_include/wups/config/WUPSConfigItemBoolean.h b/wups_include/wups/config/WUPSConfigItemBoolean.h index 0ac4eba..dd36df1 100644 --- a/wups_include/wups/config/WUPSConfigItemBoolean.h +++ b/wups_include/wups/config/WUPSConfigItemBoolean.h @@ -22,7 +22,9 @@ #include #include -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; diff --git a/wups_include/wups/config/WUPSConfigItemMultipleValues.h b/wups_include/wups/config/WUPSConfigItemMultipleValues.h index 6b80263..46fc339 100644 --- a/wups_include/wups/config/WUPSConfigItemMultipleValues.h +++ b/wups_include/wups/config/WUPSConfigItemMultipleValues.h @@ -22,7 +22,9 @@ #include #include -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;