diff --git a/src/JsonSettings.cpp b/src/JsonSettings.cpp index fe41a7a1..2156f170 100644 --- a/src/JsonSettings.cpp +++ b/src/JsonSettings.cpp @@ -4,6 +4,8 @@ #include String JsonSettings::getString(const char *key) { + Guard guard(mutex); + preferences.begin(name, true); String value = preferences.getString(key, this->find(key).strDefault); preferences.end(); @@ -11,6 +13,8 @@ String JsonSettings::getString(const char *key) { } int JsonSettings::getInt(const char *key) { + Guard guard(mutex); + preferences.begin(name, true); int value = preferences.getInt(key, this->find(key).intDefault); preferences.end(); @@ -18,6 +22,8 @@ int JsonSettings::getInt(const char *key) { } float JsonSettings::getFloat(const char *key) { + Guard guard(mutex); + preferences.begin(name, true); float value = preferences.getFloat(key, this->find(key).floatDefault); preferences.end(); @@ -25,6 +31,8 @@ float JsonSettings::getFloat(const char *key) { } std::vector JsonSettings::getIntVector(const char *key) { + Guard guard(mutex); + preferences.begin(name, true); String value = preferences.getString(key, this->find(key).strDefault); preferences.end(); @@ -45,18 +53,24 @@ std::vector JsonSettings::getIntVector(const char *key) { } void JsonSettings::putString(const char *key, String value) { + Guard guard(mutex); + preferences.begin(name, false); preferences.putString(key, value); preferences.end(); } void JsonSettings::putInt(const char *key, int value) { + Guard guard(mutex); + preferences.begin(name, false); preferences.putInt(key, value); preferences.end(); } void JsonSettings::putFloat(const char *key, float value) { + Guard guard(mutex); + preferences.begin(name, false); preferences.putFloat(key, value); preferences.end(); @@ -74,6 +88,8 @@ void JsonSettings::putIntVector(const char *key, std::vector value) { } JsonDocument JsonSettings::toJson() { + Guard guard(mutex); + JsonDocument settings; preferences.begin(name, true); @@ -99,15 +115,30 @@ JsonDocument JsonSettings::toJson() { } bool JsonSettings::fromJson(JsonDocument settings) { + Guard guard(mutex); + preferences.begin(name, false); for (JsonPair kv : settings.as()) { const char *key = kv.key().c_str(); + + // Skip keys we do not know about instead of letting find() throw. This + // body runs on the web server task, where an uncaught exception panics + // the whole device - and a browser still holding a page from a + // different firmware version will happily post keys we have never + // heard of. + if (this->map.find(key) == this->map.end()) { + Serial.print("Ignoring unknown setting: "); + Serial.println(key); + continue; + } + JsonSetting setting = this->find(key); if (! setting.validate(kv.value().as())) { lastValidationError = setting.getLastValidationError(); lastValidationKey = String(key); + preferences.end(); // do not leak the nvs handle on the error path return false; } @@ -125,6 +156,8 @@ bool JsonSettings::fromJson(JsonDocument settings) { } bool JsonSettings::reset() { + Guard guard(mutex); + preferences.begin("config", false); preferences.clear(); preferences.end(); diff --git a/src/JsonSettings.h b/src/JsonSettings.h index 2de9f973..d33b7c1b 100644 --- a/src/JsonSettings.h +++ b/src/JsonSettings.h @@ -5,11 +5,14 @@ #include #include #include +#include +#include #include class JsonSettings { public: - JsonSettings(const char *name, std::map map) : name(name), map(map) {} + JsonSettings(const char *name, std::map map) + : name(name), map(map), mutex(xSemaphoreCreateRecursiveMutex()) {} String getString(const char *key); int getInt(const char *key); @@ -37,5 +40,31 @@ class JsonSettings { JsonSetting find(const char *key); + // Every accessor below opens and closes NVS on the single `preferences` + // instance. The Arduino loop task reads settings (getMode() runs on every + // pass through loop()) while the AsyncTCP task writes them from the HTTP + // handlers, so without this lock one task's end() closes the handle the + // other is mid-way through using: reads silently fall back to the compiled + // default and writes are lost while the request still answers "success". + // + // Recursive because reset() calls fromJson(toJson()), and both take it. + SemaphoreHandle_t mutex; + + class Guard { + public: + explicit Guard(SemaphoreHandle_t mutex) : mutex(mutex) { + xSemaphoreTakeRecursive(mutex, portMAX_DELAY); + } + ~Guard() { + xSemaphoreGiveRecursive(mutex); + } + + Guard(const Guard &) = delete; + Guard &operator=(const Guard &) = delete; + + private: + SemaphoreHandle_t mutex; + }; + Preferences preferences; };