From 4557e6c0bb5ca06d77b74d5330768c3776bbc3a0 Mon Sep 17 00:00:00 2001 From: andreas Date: Fri, 11 Mar 2022 14:50:48 +0100 Subject: [PATCH 1/3] optimize memory usage for config data, allow 4s timeout for setConfig --- lib/config/GWConfig.cpp | 13 +++++-------- lib/config/GWConfig.h | 1 - lib/config/GwConfigItem.h | 21 +++++++++++++++++++-- lib/queue/GwMessage.h | 1 + lib/webserver/GwWebServer.cpp | 2 +- src/main.cpp | 19 ++++++++++++++----- 6 files changed, 40 insertions(+), 17 deletions(-) diff --git a/lib/config/GWConfig.cpp b/lib/config/GWConfig.cpp index d8da398..9ed1181 100644 --- a/lib/config/GWConfig.cpp +++ b/lib/config/GWConfig.cpp @@ -66,13 +66,10 @@ bool GwConfigHandler::loadConfig(){ bool GwConfigHandler::saveConfig(){ prefs.begin(PREF_NAME,false); for (int i=0;iasString(); - auto it=changedValues.find(configs[i]->getName()); - if (it != changedValues.end()){ - val=it->second; + if (configs[i]->hasChangedValue){ + LOG_DEBUG(GwLog::LOG,"saving %s=%s",configs[i]->getName().c_str(),configs[i]->changedValue.c_str()); + prefs.putString(configs[i]->getName().c_str(),configs[i]->changedValue); } - LOG_DEBUG(GwLog::LOG,"saving %s=%s",configs[i]->getName().c_str(),val.c_str()); - prefs.putString(configs[i]->getName().c_str(),val); } prefs.end(); LOG_DEBUG(GwLog::LOG,"saved config"); @@ -87,14 +84,14 @@ bool GwConfigHandler::updateValue(String name, String value){ } else{ LOG_DEBUG(GwLog::LOG,"update config %s=>%s",name.c_str(),i->isSecret()?"***":value.c_str()); - changedValues[name]=value; + i->updateValue(value); } return true; } bool GwConfigHandler::reset(bool save){ LOG_DEBUG(GwLog::LOG,"reset config"); for (int i=0;igetName()]=configs[i]->getDefault(); + configs[i]->updateValue(configs[i]->getDefault(),true); } if (!save) return true; return saveConfig(); diff --git a/lib/config/GWConfig.h b/lib/config/GWConfig.h index 1319f48..1dcb28c 100644 --- a/lib/config/GWConfig.h +++ b/lib/config/GWConfig.h @@ -13,7 +13,6 @@ class GwConfigHandler: public GwConfigDefinitions{ Preferences prefs; GwLog *logger; typedef std::map StringMap; - StringMap changedValues; boolean allowChanges=true; public: public: diff --git a/lib/config/GwConfigItem.h b/lib/config/GwConfigItem.h index f97bbd5..01d5f52 100644 --- a/lib/config/GwConfigItem.h +++ b/lib/config/GwConfigItem.h @@ -7,11 +7,28 @@ class GwConfigHandler; class GwConfigInterface{ private: String name; - String initialValue; + const char * initialValue; String value; bool secret=false; + String changedValue; + bool hasChangedValue=false; + void updateValue(String value,bool cmpDefault=false){ + hasChangedValue=false; + if (cmpDefault){ + if (value != initialValue) { + changedValue=value; + hasChangedValue=true; + } + } + else{ + if (value != this->value) { + changedValue=value; + hasChangedValue=true; + } + } + } public: - GwConfigInterface(const String &name, const String initialValue, bool secret=false){ + GwConfigInterface(const String &name, const char * initialValue, bool secret=false){ this->name=name; this->initialValue=initialValue; this->value=initialValue; diff --git a/lib/queue/GwMessage.h b/lib/queue/GwMessage.h index 5c9a877..94dd50a 100644 --- a/lib/queue/GwMessage.h +++ b/lib/queue/GwMessage.h @@ -56,6 +56,7 @@ class GwRequestMessage : public GwMessage{ String getContentType(){ return contentType; } + virtual int getTimeout(){return 500;} }; diff --git a/lib/webserver/GwWebServer.cpp b/lib/webserver/GwWebServer.cpp index 10bb8cd..eb4dcef 100644 --- a/lib/webserver/GwWebServer.cpp +++ b/lib/webserver/GwWebServer.cpp @@ -68,7 +68,7 @@ GwWebServer::~GwWebServer(){ } void GwWebServer::handleAsyncWebRequest(AsyncWebServerRequest *request, GwRequestMessage *msg) { - GwRequestQueue::MessageSendStatus st=queue->sendAndWait(msg,500); + GwRequestQueue::MessageSendStatus st=queue->sendAndWait(msg,msg->getTimeout()); if (st == GwRequestQueue::MSG_ERR) { msg->unref(); //our diff --git a/src/main.cpp b/src/main.cpp index 03818d4..bb17036 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -460,7 +460,7 @@ class SetConfigRequest : public GwRequestMessage public: SetConfigRequest() : GwRequestMessage(F("application/json"),F("setConfig")){}; StringMap args; - + virtual int getTimeout(){return 4000;} protected: virtual void processRequest() { @@ -475,6 +475,10 @@ protected: result=JSON_INVALID_PASS; return; } + logger.logDebug(GwLog::DEBUG,"Heap free=%ld, minFree=%ld", + (long)xPortGetFreeHeapSize(), + (long)xPortGetMinimumEverFreeHeapSize() + ); for (StringMap::iterator it = args.begin(); it != args.end(); it++) { if (it->first.indexOf("_")>= 0) continue; @@ -489,12 +493,19 @@ protected: error += it->second; error += ","; } + logger.flush(); } if (ok) { result = JSON_OK; logger.logDebug(GwLog::ERROR,"update config and restart"); config.saveConfig(); + logger.flush(); + logger.logDebug(GwLog::DEBUG,"Heap free=%ld, minFree=%ld", + (long)xPortGetFreeHeapSize(), + (long)xPortGetMinimumEverFreeHeapSize() + ); + logger.flush(); delayedRestart(); } else @@ -632,13 +643,11 @@ void setup() { webserver.registerMainHandler("/api/setConfig", [](AsyncWebServerRequest *request)->GwRequestMessage * { - StringMap args; + SetConfigRequest *msg = new SetConfigRequest(); for (int i = 0; i < request->args(); i++) { - args[request->argName(i)] = request->arg(i); + msg->args[request->argName(i)] = request->arg(i); } - SetConfigRequest *msg = new SetConfigRequest(); - msg->args = args; return msg; }); webserver.registerMainHandler("/api/resetConfig", [](AsyncWebServerRequest *request)->GwRequestMessage * From 9d25ce8b7e30874b97359d30d0db39c2e00480ef Mon Sep 17 00:00:00 2001 From: andreas Date: Fri, 11 Mar 2022 15:18:38 +0100 Subject: [PATCH 2/3] further memory optimization of config update --- src/main.cpp | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index bb17036..582372e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -458,18 +458,21 @@ protected: class SetConfigRequest : public GwRequestMessage { public: - SetConfigRequest() : GwRequestMessage(F("application/json"),F("setConfig")){}; - StringMap args; + //we rely on the message living not longer then the request + AsyncWebServerRequest *request; + SetConfigRequest(AsyncWebServerRequest *rq) : GwRequestMessage(F("application/json"),F("setConfig")), + request(rq) + {}; virtual int getTimeout(){return 4000;} protected: virtual void processRequest() { bool ok = true; + const char * hashArg="_hash"; String error; String hash; - auto it=args.find("_hash"); - if (it != args.end()){ - hash=it->second; + if (request->hasArg(hashArg)){ + hash=request->arg(hashArg); } if (! checkPass(hash)){ result=JSON_INVALID_PASS; @@ -479,18 +482,19 @@ protected: (long)xPortGetFreeHeapSize(), (long)xPortGetMinimumEverFreeHeapSize() ); - for (StringMap::iterator it = args.begin(); it != args.end(); it++) - { - if (it->first.indexOf("_")>= 0) continue; - if (it->first == GwConfigDefinitions::apPassword && fixedApPass) continue; - bool rt = config.updateValue(it->first, it->second); + for (int i = 0; i < request->args(); i++){ + String name=request->argName(i); + String value=request->arg(i); + if (name.indexOf("_")>= 0) continue; + if (name == GwConfigDefinitions::apPassword && fixedApPass) continue; + bool rt = config.updateValue(name, value); if (!rt) { - logger.logDebug(GwLog::ERROR,"ERR: unable to update %s to %s", it->first.c_str(), it->second.c_str()); + logger.logDebug(GwLog::ERROR,"ERR: unable to update %s to %s", name.c_str(), value.c_str()); ok = false; - error += it->first; + error += name; error += "="; - error += it->second; + error += value; error += ","; } logger.flush(); @@ -523,6 +527,7 @@ public: ResetConfigRequest(String hash) : GwRequestMessage(F("application/json"),F("resetConfig")){ this->hash=hash; }; + virtual int getTimeout(){return 4000;} protected: virtual void processRequest() @@ -643,11 +648,7 @@ void setup() { webserver.registerMainHandler("/api/setConfig", [](AsyncWebServerRequest *request)->GwRequestMessage * { - SetConfigRequest *msg = new SetConfigRequest(); - for (int i = 0; i < request->args(); i++) - { - msg->args[request->argName(i)] = request->arg(i); - } + SetConfigRequest *msg = new SetConfigRequest(request); return msg; }); webserver.registerMainHandler("/api/resetConfig", [](AsyncWebServerRequest *request)->GwRequestMessage * From faadccd6cb952e2afa64c93913ab355fbc204ae2 Mon Sep 17 00:00:00 2001 From: andreas Date: Fri, 11 Mar 2022 15:39:29 +0100 Subject: [PATCH 3/3] correct factory reset handling after optimization --- lib/config/GWConfig.cpp | 2 +- lib/config/GwConfigItem.h | 23 +++++++++-------------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/lib/config/GWConfig.cpp b/lib/config/GWConfig.cpp index 9ed1181..f7316df 100644 --- a/lib/config/GWConfig.cpp +++ b/lib/config/GWConfig.cpp @@ -91,7 +91,7 @@ bool GwConfigHandler::updateValue(String name, String value){ bool GwConfigHandler::reset(bool save){ LOG_DEBUG(GwLog::LOG,"reset config"); for (int i=0;iupdateValue(configs[i]->getDefault(),true); + configs[i]->updateValue(configs[i]->getDefault()); } if (!save) return true; return saveConfig(); diff --git a/lib/config/GwConfigItem.h b/lib/config/GwConfigItem.h index 01d5f52..44f8a06 100644 --- a/lib/config/GwConfigItem.h +++ b/lib/config/GwConfigItem.h @@ -12,21 +12,16 @@ class GwConfigInterface{ bool secret=false; String changedValue; bool hasChangedValue=false; - void updateValue(String value,bool cmpDefault=false){ - hasChangedValue=false; - if (cmpDefault){ - if (value != initialValue) { - changedValue=value; - hasChangedValue=true; - } + void updateValue(String value) + { + hasChangedValue = false; + if (value != this->value) + { + changedValue = value; + hasChangedValue = true; } - else{ - if (value != this->value) { - changedValue=value; - hasChangedValue=true; - } - } - } + } + public: GwConfigInterface(const String &name, const char * initialValue, bool secret=false){ this->name=name;