From 6fee2f1f9fd6ff4fe0f921050a2d1c3404b319d0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Xose=20P=C3=A9rez?= <xose.perez@gmail.com>
Date: Sun, 7 Oct 2018 20:35:30 +0200
Subject: [PATCH] Avoid EEPROM commits on callbacks (#1214)

---
 code/espurna/config/hardware.h |  6 ++---
 code/espurna/mqtt.ino          |  2 +-
 code/espurna/relay.ino         | 45 +++++++++++++++++++++++++++++-----
 code/espurna/utils.ino         |  6 ++---
 4 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/code/espurna/config/hardware.h b/code/espurna/config/hardware.h
index 2603fce9..2d17193e 100644
--- a/code/espurna/config/hardware.h
+++ b/code/espurna/config/hardware.h
@@ -70,7 +70,7 @@
     // Buttons
     #define BUTTON1_PIN         0
     #define BUTTON1_MODE        BUTTON_PUSHBUTTON | BUTTON_DEFAULT_HIGH
-    #define BUTTON1_RELAY           1
+    #define BUTTON1_RELAY       1
 
     // Hidden button will enter AP mode if dblclick and reset the device when long-long-clicked
     #define RELAY1_PIN          12
@@ -1943,11 +1943,11 @@
     #define LED1_PIN			0  // red
     #define LED1_PIN_INVERSE	1
     #define LED1_MODE           LED_MODE_WIFI
-    
+
     #define LED2_PIN			15  // blue
     #define LED2_PIN_INVERSE	1
     #define LED2_MODE           LED_MODE_RELAY
-    
+
     // HLW8012
     #ifndef HLW8012_SUPPORT
     #define HLW8012_SUPPORT     1
diff --git a/code/espurna/mqtt.ino b/code/espurna/mqtt.ino
index 017240c2..bbd0e3d9 100644
--- a/code/espurna/mqtt.ino
+++ b/code/espurna/mqtt.ino
@@ -291,7 +291,7 @@ unsigned long _mqttNextMessageId() {
         EEPROMr.write(EEPROM_MESSAGE_ID + 1, (id >> 16) & 0xFF);
         EEPROMr.write(EEPROM_MESSAGE_ID + 2, (id >>  8) & 0xFF);
         EEPROMr.write(EEPROM_MESSAGE_ID + 3, (id >>  0) & 0xFF);
-        EEPROMr.commit();
+        saveSettings();
     }
 
     id++;
diff --git a/code/espurna/relay.ino b/code/espurna/relay.ino
index 3cfab0c9..135be43f 100644
--- a/code/espurna/relay.ino
+++ b/code/espurna/relay.ino
@@ -173,11 +173,19 @@ void _relayProcess(bool mode) {
         #endif
 
         if (!_relayRecursive) {
+
             relayPulse(id);
-            _relaySaveTicker.once_ms(RELAY_SAVE_DELAY, relaySave);
+
+            // We will trigger a commit only if
+            // we care about current relay status on boot
+            unsigned char boot_mode = getSetting("relayBoot", id, RELAY_BOOT_MODE).toInt();
+            bool do_commit = ((RELAY_BOOT_SAME == boot_mode) || (RELAY_BOOT_TOGGLE == boot_mode));
+            _relaySaveTicker.once_ms(RELAY_SAVE_DELAY, relaySave, do_commit);
+
             #if WEB_SUPPORT
                 wsSend(_relayWebSocketUpdate);
             #endif
+
         }
 
         #if DOMOTICZ_SUPPORT
@@ -383,16 +391,41 @@ void relaySync(unsigned char id) {
 
 }
 
-void relaySave() {
+void relaySave(bool do_commit) {
+
+    // Relay status is stored in a single byte
+    // This means that, atm,
+    // we are only storing the status of the first 8 relays.
     unsigned char bit = 1;
     unsigned char mask = 0;
-    for (unsigned int i=0; i < _relays.size(); i++) {
+    unsigned char count = _relays.size();
+    if (count > 8) count = 8;
+    for (unsigned int i=0; i < count; i++) {
         if (relayStatus(i)) mask += bit;
         bit += bit;
     }
+
     EEPROMr.write(EEPROM_RELAY_STATUS, mask);
-    DEBUG_MSG_P(PSTR("[RELAY] Saving mask: %d\n"), mask);
-    EEPROMr.commit();
+    DEBUG_MSG_P(PSTR("[RELAY] Setting relay mask: %d\n"), mask);
+
+    // The 'do_commit' flag controls wether we are commiting this change or not.
+    // It is useful to set it to 'false' if the relay change triggering the
+    // save involves a relay whose boot mode is independent from current mode,
+    // thus storing the last relay value is not absolutely necessary.
+    // Nevertheless, we store the value in the EEPROM buffer so it will be written
+    // on the next commit.
+    if (do_commit) {
+
+        // We are actually enqueuing the commit so it will be
+        // executed on the main loop, in case this is called from a callback
+        saveSettings();
+
+    }
+
+}
+
+void relaySave() {
+    relaySave(true);
 }
 
 void relayToggle(unsigned char id, bool report, bool group_report) {
@@ -508,7 +541,7 @@ void _relayBoot() {
     // Save if there is any relay in the RELAY_BOOT_TOGGLE mode
     if (trigger_save) {
         EEPROMr.write(EEPROM_RELAY_STATUS, mask);
-        EEPROMr.commit();
+        saveSettings();
     }
 
     _relayRecursive = false;
diff --git a/code/espurna/utils.ino b/code/espurna/utils.ino
index 96f6fbea..0c0c8847 100644
--- a/code/espurna/utils.ino
+++ b/code/espurna/utils.ino
@@ -468,13 +468,13 @@ void resetReason(unsigned char reason) {
     EEPROMr.commit();
 }
 
-void reset(unsigned char reason) {
-    resetReason(reason);
+void reset() {
     ESP.restart();
 }
 
 void deferredReset(unsigned long delay, unsigned char reason) {
-    _defer_reset.once_ms(delay, reset, reason);
+    resetReason(reason);
+    _defer_reset.once_ms(delay, reset);
 }
 
 // -----------------------------------------------------------------------------