From 502434abc432069b7432f3c01f7ac166f1c4d35e Mon Sep 17 00:00:00 2001
From: Klaus K Wilting <klaus.k.wilting@deutschebahn.com>
Date: Sun, 4 Oct 2020 13:15:17 +0200
Subject: [PATCH] configmanager: improved error handling

---
 include/configmanager.h |  2 +-
 src/configmanager.cpp   | 52 +++++++++++++++++++++++++++--------------
 src/main.cpp            |  2 +-
 3 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/include/configmanager.h b/include/configmanager.h
index 8da2e83c..e7d26438 100644
--- a/include/configmanager.h
+++ b/include/configmanager.h
@@ -5,7 +5,7 @@
 #include "globals.h"
 
 void saveConfig(bool erase = false);
-void loadConfig(void);
+bool loadConfig(void);
 void eraseConfig(void);
 
 #endif
\ No newline at end of file
diff --git a/src/configmanager.cpp b/src/configmanager.cpp
index 6c2efc4b..b74f4e0d 100644
--- a/src/configmanager.cpp
+++ b/src/configmanager.cpp
@@ -16,11 +16,12 @@ static const char TAG[] = __FILE__;
 
 Preferences nvram;
 
-static const size_t cfgLen = sizeof(cfg);
-static char buffer[cfgLen];
+static const char cfgMagicBytes[] = {0x21, 0x76, 0x87, 0x32, 0xf3};
+static const size_t cfgLen = sizeof(cfg), cfgLen2 = sizeof(cfgMagicBytes);
+static char buffer[cfgLen + cfgLen2];
 
 // populate runtime config with factory settings
-void defaultConfig(configData_t *myconfig) {
+static void defaultConfig(configData_t *myconfig) {
   char version[10];
   snprintf(version, 10, "%-10s", PROGVERSION);
 
@@ -56,45 +57,60 @@ void defaultConfig(configData_t *myconfig) {
 
 // save current configuration from RAM to NVRAM
 void saveConfig(bool erase) {
-  ESP_LOGI(TAG, "Storing settings in NVRAM");
+  ESP_LOGI(TAG, "Storing settings to NVRAM...");
 
   nvram.begin(DEVCONFIG, false);
 
   if (erase) {
-    ESP_LOGI(TAG, "Resetting NVRAM to factory settings");
+    ESP_LOGI(TAG, "Resetting device to factory settings");
     nvram.clear();
     defaultConfig(&cfg);
   }
 
-  // Copy device runtime config cfg to byte array
+  // Copy device runtime config cfg to byte array, padding it with magicBytes
   memcpy(buffer, &cfg, cfgLen);
+  memcpy(buffer + cfgLen, &cfgMagicBytes, cfgLen2);
+
+  // save byte array to NVRAM, padding with cfg magicbyes
+  if (nvram.putBytes(DEVCONFIG, buffer, cfgLen + cfgLen2))
+    ESP_LOGI(TAG, "Device settings saved");
+  else
+    ESP_LOGE(TAG, "NVRAM Error, device settings not saved");
 
-  // save byte array to NVRAM
-  nvram.putBytes(DEVCONFIG, buffer, cfgLen);
   nvram.end();
 }
 
 // load configuration from NVRAM into RAM and make it current
-void loadConfig() {
+bool loadConfig() {
 
-  ESP_LOGI(TAG, "Loading runtime settings from NVS");
+  ESP_LOGI(TAG, "Loading device runtime configuration from NVRAM...");
 
   if (!nvram.begin(DEVCONFIG, true)) {
-    ESP_LOGI(TAG, "Initializing NVRAM");
+    ESP_LOGW(TAG, "NVRAM initialized, device starts with factory settings");
     eraseConfig();
+    return true;
+
   } else {
     // simple check that runtime config data matches
-    if (nvram.getBytesLength(DEVCONFIG) != cfgLen) {
-      ESP_LOGW(TAG, "NVRAM settings invalid");
-      eraseConfig();
+    if (nvram.getBytesLength(DEVCONFIG) != (cfgLen + cfgLen2)) {
+      ESP_LOGE(TAG, "configuration invalid");
+      return false;
+
     } else {
-
       // load device runtime config from nvram and copy it to byte array
-      nvram.getBytes(DEVCONFIG, buffer, cfgLen);
+      nvram.getBytes(DEVCONFIG, buffer, cfgLen + cfgLen2);
       nvram.end();
+      // validate configuration by checking magic bytes at end of array
+      if (memcmp(buffer + cfgLen, &cfgMagicBytes, cfgLen2) != 0) {
+        ESP_LOGW(TAG, "No configuration found");
+        return false;
 
-      // copy the byte array into runtime cfg struct
-      memcpy(&cfg, buffer, cfgLen);
+      } else {
+        // copy byte array into runtime cfg struct
+        memcpy(&cfg, buffer, cfgLen);
+        ESP_LOGI(TAG, "Runtime configuration loaded");
+        return true;
+      }
     }
   }
 }
diff --git a/src/main.cpp b/src/main.cpp
index 175e78f0..e31b2b59 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -196,7 +196,7 @@ void setup() {
 #endif
 
   // read (and initialize on first run) runtime settings from NVRAM
-  loadConfig(); // includes initialize if necessary
+  assert(loadConfig()); // includes initialize if necessary
 
   // now that we are powered, we scan i2c bus for devices
   i2c_scan();