Relay: use relative time comparisons, runtime settings for flood window and delays (#1962)

* relay: respect relay delay time when using NONE_OR_ONE sync

* fixup! relay: respect relay delay time when using NONE_OR_ONE sync

* calculate delay from flood window + existing delay

* configure flood max changes too

* oops, s -> ms

* count other way around

* interpret delay differently with sync_one/none_or_one

* instead of accumulation, ensure that max delay time is selected (if it is there at all)

* global interlock delay setting, allow to set change_delay before relayStatus(id, status) changes it

* fixup! global interlock delay setting, allow to set change_delay before relayStatus(id, status) changes it

* (finally) use lock attr to avoid user changing state while target != current state

* fix building with 2.3.0

* postpone wspost after all relays are processed, remove change_start refresh, snapshot relaySync value in configure

* fix warning

* add another timer for ONE

code/espurna/config/defaults.h

@@ -473,6 +473,10 @@
 #define RELAY8_DELAY_OFF      0
 #endif
 
+#ifndef RELAY_DELAY_INTERLOCK
+#define RELAY_DELAY_INTERLOCK       0
+#endif
+
 // -----------------------------------------------------------------------------
 // LEDs
 // -----------------------------------------------------------------------------

code/espurna/config/prototypes.h

@@ -1,6 +1,7 @@
 #include <Arduino.h>
 #include <ArduinoJson.h>
 #include <functional>
+#include <algorithm>
 #include <vector>
 #include <memory>
 

code/espurna/relay.ino

@@ -31,7 +31,8 @@ typedef struct {
     unsigned char lock;         // Holds the value of target status, that cannot be changed afterwards. (0 for false, 1 for true, 2 to disable)
     unsigned long fw_start;     // Flood window start time
     unsigned char fw_count;     // Number of changes within the current flood window
-    unsigned long change_time;  // Scheduled time to change
+    unsigned long change_start;      // Time when relay was scheduled to change
+    unsigned long change_delay;      // Delay until the next change
     bool report;                // Whether to report to own topic
     bool group_report;          // Whether to report to group topic
 
@@ -42,7 +43,22 @@ typedef struct {
 } relay_t;
 std::vector<relay_t> _relays;
 bool _relayRecursive = false;
-Ticker _relaySaveTicker;
+
+unsigned long _relay_flood_window = (1000 * RELAY_FLOOD_WINDOW);
+unsigned long _relay_flood_changes = RELAY_FLOOD_CHANGES;
+
+unsigned long _relay_delay_interlock;
+unsigned char _relay_sync_mode = RELAY_SYNC_ANY;
+bool _relay_sync_locked = false;
+
+Ticker _relay_save_timer;
+Ticker _relay_sync_timer;
+
+#if WEB_SUPPORT
+
+bool _relay_report_ws = false;
+
+#endif // WEB_SUPPORT
 
 #if MQTT_SUPPORT
 
@@ -82,6 +98,69 @@ RelayStatus _relayStatusTyped(unsigned char id) {
     return (status) ? RelayStatus::ON : RelayStatus::OFF;
 }
 
+void _relayLockAll() {
+    for (auto& relay : _relays) {
+        relay.lock = relay.target_status;
+    }
+    _relay_sync_locked = true;
+}
+
+void _relayUnlockAll() {
+    for (auto& relay : _relays) {
+        relay.lock = RELAY_LOCK_DISABLED;
+    }
+    _relay_sync_locked = false;
+}
+
+bool _relayStatusLock(unsigned char id, bool status) {
+    if (_relays[id].lock != RELAY_LOCK_DISABLED) {
+        bool lock = _relays[id].lock == RELAY_LOCK_ON;
+        if ((lock != status) || (lock != _relays[id].target_status)) {
+            _relays[id].target_status = lock;
+            _relays[id].change_delay = 0;
+            return false;
+        }
+    }
+
+    return true;
+}
+
+// https://github.com/xoseperez/espurna/issues/1510#issuecomment-461894516
+// completely reset timing on the other relay to sync with this one
+// to ensure that they change state sequentially
+void _relaySyncRelaysDelay(unsigned char first, unsigned char second) {
+    _relays[second].fw_start = _relays[first].change_start;
+    _relays[second].fw_count = 1;
+    _relays[second].change_delay = std::max({
+        _relay_delay_interlock,
+        _relays[first].change_delay,
+        _relays[second].change_delay
+    });
+}
+
+void _relaySyncUnlock() {
+    bool unlock = true;
+    bool all_off = true;
+    for (const auto& relay : _relays) {
+        unlock = unlock && (relay.current_status == relay.target_status);
+        if (!unlock) break;
+        all_off = all_off && !relay.current_status;
+    }
+
+    if (!unlock) return;
+
+    auto action = []() {
+        _relayUnlockAll();
+        _relay_report_ws = true;
+    };
+
+    if (all_off) {
+        _relay_sync_timer.once_ms(_relay_delay_interlock, action);
+    } else {
+        action();
+    }
+}
+
 // -----------------------------------------------------------------------------
 // RELAY PROVIDERS
 // -----------------------------------------------------------------------------
@@ -201,7 +280,7 @@ void _relayProviderStatus(unsigned char id, bool status) {
  */
 void _relayProcess(bool mode) {
 
-    unsigned long current_time = millis();
+    bool changed = false;
 
     for (unsigned char id = 0; id < _relays.size(); id++) {
 
@@ -213,25 +292,12 @@ void _relayProcess(bool mode) {
         // Only process the relays we have to change to the requested mode
         if (target != mode) continue;
 
-        // Only process the relays that can be changed
-        switch (_relays[id].lock) {
-            case RELAY_LOCK_ON:
-            case RELAY_LOCK_OFF:
-                {
-                    bool lock = _relays[id].lock == 1;
-                    if (lock != _relays[id].target_status) {
-                        _relays[id].target_status = lock;
-                        continue;
-                    }
-                    break;
-                }
-            case RELAY_LOCK_DISABLED:
-            default:
-                break;
-        }
+        // Only process if the change delay has expired
+        if (millis() - _relays[id].change_start < _relays[id].change_delay) continue;
 
-        // Only process if the change_time has arrived
-        if (current_time < _relays[id].change_time) continue;
+        // Purge existing delay in case of cancelation
+        _relays[id].change_delay = 0;
+        changed = true;
 
         DEBUG_MSG_P(PSTR("[RELAY] #%d set to %s\n"), id, target ? "ON" : "OFF");
 
@@ -248,6 +314,10 @@ void _relayProcess(bool mode) {
             relayMQTT(id);
         #endif
 
+        #if WEB_SUPPORT
+            _relay_report_ws = true;
+        #endif
+
         if (!_relayRecursive) {
 
             relayPulse(id);
@@ -256,11 +326,7 @@ void _relayProcess(bool mode) {
             // we care about current relay status on boot
             unsigned char boot_mode = getSetting("relayBoot", id, RELAY_BOOT_MODE).toInt();
             bool save_eeprom = ((RELAY_BOOT_SAME == boot_mode) || (RELAY_BOOT_TOGGLE == boot_mode));
-            _relaySaveTicker.once_ms(RELAY_SAVE_DELAY, relaySave, save_eeprom);
-
-            #if WEB_SUPPORT
-                wsPost(_relayWebSocketUpdate);
-            #endif
+            _relay_save_timer.once_ms(RELAY_SAVE_DELAY, relaySave, save_eeprom);
 
         }
 
@@ -269,6 +335,12 @@ void _relayProcess(bool mode) {
 
     }
 
+    // Whenever we are using sync modes and any relay had changed the state, check if we can unlock
+    const bool needs_unlock = ((_relay_sync_mode == RELAY_SYNC_NONE_OR_ONE) || (_relay_sync_mode == RELAY_SYNC_ONE));
+    if (_relay_sync_locked && needs_unlock && changed) {
+        _relaySyncUnlock();
+    }
+
 }
 
 #if defined(ITEAD_SONOFF_IFAN02)
@@ -337,6 +409,13 @@ bool relayStatus(unsigned char id, bool status, bool report, bool group_report)
 
     if (id >= _relays.size()) return false;
 
+    if (!_relayStatusLock(id, status)) {
+        DEBUG_MSG_P(PSTR("[RELAY] #%d is locked to %s\n"), id, _relays[id].current_status ? "ON" : "OFF");
+        _relays[id].report = true;
+        _relays[id].group_report = true;
+        return false;
+    }
+
     bool changed = false;
 
     if (_relays[id].current_status == status) {
@@ -346,6 +425,7 @@ bool relayStatus(unsigned char id, bool status, bool report, bool group_report)
             _relays[id].target_status = status;
             _relays[id].report = false;
             _relays[id].group_report = false;
+            _relays[id].change_delay = 0;
             changed = true;
         }
 
@@ -360,27 +440,29 @@ bool relayStatus(unsigned char id, bool status, bool report, bool group_report)
     } else {
 
         unsigned long current_time = millis();
-        unsigned long fw_end = _relays[id].fw_start + 1000 * RELAY_FLOOD_WINDOW;
-        unsigned long delay = status ? _relays[id].delay_on : _relays[id].delay_off;
+        unsigned long change_delay = status ? _relays[id].delay_on : _relays[id].delay_off;
 
         _relays[id].fw_count++;
-        _relays[id].change_time = current_time + delay;
+        _relays[id].change_start = current_time;
+        _relays[id].change_delay = std::max(_relays[id].change_delay, change_delay);
 
         // If current_time is off-limits the floodWindow...
-        if (current_time < _relays[id].fw_start || fw_end <= current_time) {
+        const auto fw_diff = current_time - _relays[id].fw_start;
+        if (fw_diff > _relay_flood_window) {
 
             // We reset the floodWindow
             _relays[id].fw_start = current_time;
             _relays[id].fw_count = 1;
 
         // If current_time is in the floodWindow and there have been too many requests...
-        } else if (_relays[id].fw_count >= RELAY_FLOOD_CHANGES) {
+        } else if (_relays[id].fw_count >= _relay_flood_changes) {
 
             // We schedule the changes to the end of the floodWindow
             // unless it's already delayed beyond that point
-            if (fw_end - delay > current_time) {
-                _relays[id].change_time = fw_end;
-            }
+            _relays[id].change_delay = std::max(change_delay, _relay_flood_window - fw_diff);
+
+            // Another option is to always move it forward, starting from current time
+            //_relays[id].fw_start = current_time;
 
         }
 
@@ -391,8 +473,8 @@ bool relayStatus(unsigned char id, bool status, bool report, bool group_report)
         relaySync(id);
 
         DEBUG_MSG_P(PSTR("[RELAY] #%d scheduled %s in %u ms\n"),
-                id, status ? "ON" : "OFF",
-                (_relays[id].change_time - current_time));
+            id, status ? "ON" : "OFF", _relays[id].change_delay
+        );
 
         changed = true;
 
@@ -427,37 +509,44 @@ void relaySync(unsigned char id) {
     // Flag sync mode
     _relayRecursive = true;
 
-    byte relaySync = getSetting("relaySync", RELAY_SYNC).toInt();
     bool status = _relays[id].target_status;
 
     // If RELAY_SYNC_SAME all relays should have the same state
-    if (relaySync == RELAY_SYNC_SAME) {
+    if (_relay_sync_mode == RELAY_SYNC_SAME) {
         for (unsigned short i=0; i<_relays.size(); i++) {
             if (i != id) relayStatus(i, status);
         }
 
     // If RELAY_SYNC_FIRST all relays should have the same state as first if first changes
-    } else if (relaySync == RELAY_SYNC_FIRST) {
+    } else if (_relay_sync_mode == RELAY_SYNC_FIRST) {
         if (id == 0) {
             for (unsigned short i=1; i<_relays.size(); i++) {
                 relayStatus(i, status);
             }
         }
 
-    // If NONE_OR_ONE or ONE and setting ON we should set OFF all the others
-    } else if (status) {
-        if (relaySync != RELAY_SYNC_ANY) {
-            for (unsigned short i=0; i<_relays.size(); i++) {
-                if (i != id) relayStatus(i, false);
+    } else if ((_relay_sync_mode == RELAY_SYNC_NONE_OR_ONE) || (_relay_sync_mode == RELAY_SYNC_ONE)) {
+        // If NONE_OR_ONE or ONE and setting ON we should set OFF all the others
+        if (status) {
+            if (_relay_sync_mode != RELAY_SYNC_ANY) {
+                for (unsigned short other_id=0; other_id<_relays.size(); other_id++) {
+                    if (other_id != id) {
+                        relayStatus(other_id, false);
+                        if (relayStatus(other_id)) {
+                            _relaySyncRelaysDelay(other_id, id);
+                        }
+                    }
+                }
+            }
+        // If ONLY_ONE and setting OFF we should set ON the other one
+        } else {
+            if (_relay_sync_mode == RELAY_SYNC_ONE) {
+                unsigned char other_id = (id + 1) % _relays.size();
+                _relaySyncRelaysDelay(id, other_id);
+                relayStatus(other_id, true);
             }
         }
-
-    // If ONLY_ONE and setting OFF we should set ON the other one
-    } else {
-        if (relaySync == RELAY_SYNC_ONE) {
-            unsigned char i = (id + 1) % _relays.size();
-            relayStatus(i, true);
-        }
+        _relayLockAll();
     }
 
     // Unflag sync mode
@@ -619,10 +708,12 @@ void _relayBoot() {
 
         _relays[i].current_status = !status;
         _relays[i].target_status = status;
+        _relays[i].change_start = millis();
+
         #if RELAY_PROVIDER == RELAY_PROVIDER_STM
-            _relays[i].change_time = millis() + 3000 + 1000 * i;
-        #else
-            _relays[i].change_time = millis();
+            // XXX hack for correctly restoring relay state on boot
+            // because of broken stm relay firmware
+            _relays[i].change_delay = 3000 + 1000 * i;
         #endif
 
         _relays[i].lock = lock;
@@ -641,11 +732,40 @@ void _relayBoot() {
 
 }
 
+constexpr const unsigned long _relayDelayOn(unsigned char index) {
+    return (
+        (index == 0) ? RELAY1_DELAY_ON :
+        (index == 1) ? RELAY2_DELAY_ON :
+        (index == 2) ? RELAY3_DELAY_ON :
+        (index == 3) ? RELAY4_DELAY_ON :
+        (index == 4) ? RELAY5_DELAY_ON :
+        (index == 5) ? RELAY6_DELAY_ON :
+        (index == 6) ? RELAY7_DELAY_ON :
+        (index == 7) ? RELAY8_DELAY_ON : 0
+    );
+}
+
+constexpr const unsigned long _relayDelayOff(unsigned char index) {
+    return (
+        (index == 0) ? RELAY1_DELAY_OFF :
+        (index == 1) ? RELAY2_DELAY_OFF :
+        (index == 2) ? RELAY3_DELAY_OFF :
+        (index == 3) ? RELAY4_DELAY_OFF :
+        (index == 4) ? RELAY5_DELAY_OFF :
+        (index == 5) ? RELAY6_DELAY_OFF :
+        (index == 6) ? RELAY7_DELAY_OFF :
+        (index == 7) ? RELAY8_DELAY_OFF : 0
+    );
+}
+
 void _relayConfigure() {
     for (unsigned int i=0; i<_relays.size(); i++) {
         _relays[i].pulse = getSetting("relayPulse", i, RELAY_PULSE_MODE).toInt();
         _relays[i].pulse_ms = 1000 * getSetting("relayTime", i, RELAY_PULSE_MODE).toFloat();
 
+        _relays[i].delay_on = getSetting("relayDelayOn", i, _relayDelayOn(i)).toInt();
+        _relays[i].delay_off = getSetting("relayDelayOff", i, _relayDelayOff(i)).toInt();
+
         if (GPIO_NONE == _relays[i].pin) continue;
 
         pinMode(_relays[i].pin, OUTPUT);
@@ -658,6 +778,12 @@ void _relayConfigure() {
         }
     }
 
+    _relay_flood_window = (1000 * getSetting("relayFloodTime", RELAY_FLOOD_WINDOW).toInt());
+    _relay_flood_changes = getSetting("relayFloodChanges", RELAY_FLOOD_CHANGES).toInt();
+
+    _relay_delay_interlock = getSetting("relayDelayInterlock", RELAY_DELAY_INTERLOCK).toInt();
+    _relay_sync_mode = getSetting("relaySync", RELAY_SYNC).toInt();
+
     #if MQTT_SUPPORT
         settingsProcessConfig({
             {_relay_mqtt_payload_on,     "relayPayloadOn",     RELAY_MQTT_ON},
@@ -1141,6 +1267,25 @@ void _relayInitCommands() {
         terminalOK();
     });
 
+    #if 0
+    terminalRegisterCommand(F("RELAY.INFO"), [](Embedis* e) {
+        DEBUG_MSG_P(PSTR("    cur tgt pin type reset lock  delay_on   delay_off  pulse  pulse_ms\n"));
+        DEBUG_MSG_P(PSTR("    --- --- --- ---- ----- ---- ---------- ----------- ----- ----------\n"));
+        for (unsigned char index = 0; index < _relays.size(); ++index) {
+            const auto& relay = _relays.at(index);
+            DEBUG_MSG_P(PSTR("%3u %3s %3s %3u %4u %5u %4u %10u %11u %5u %10u\n"),
+                index,
+                relay.current_status ? "ON" : "OFF",
+                relay.target_status ? "ON" : "OFF",
+                relay.pin, relay.type, relay.reset_pin,
+                relay.lock,
+                relay.delay_on, relay.delay_off,
+                relay.pulse, relay.pulse_ms
+            );
+        }
+    });
+    #endif
+
 }
 
 #endif // TERMINAL_SUPPORT
@@ -1152,41 +1297,47 @@ void _relayInitCommands() {
 void _relayLoop() {
     _relayProcess(false);
     _relayProcess(true);
+    #if WEB_SUPPORT
+        if (_relay_report_ws) {
+            wsPost(_relayWebSocketUpdate);
+            _relay_report_ws = false;
+        }
+    #endif
 }
 
 void relaySetup() {
 
     // Ad-hoc relays
     #if RELAY1_PIN != GPIO_NONE
-        _relays.push_back((relay_t) { RELAY1_PIN, RELAY1_TYPE, RELAY1_RESET_PIN, RELAY1_DELAY_ON, RELAY1_DELAY_OFF });
+        _relays.push_back((relay_t) { RELAY1_PIN, RELAY1_TYPE, RELAY1_RESET_PIN });
     #endif
     #if RELAY2_PIN != GPIO_NONE
-        _relays.push_back((relay_t) { RELAY2_PIN, RELAY2_TYPE, RELAY2_RESET_PIN, RELAY2_DELAY_ON, RELAY2_DELAY_OFF });
+        _relays.push_back((relay_t) { RELAY2_PIN, RELAY2_TYPE, RELAY2_RESET_PIN });
     #endif
     #if RELAY3_PIN != GPIO_NONE
-        _relays.push_back((relay_t) { RELAY3_PIN, RELAY3_TYPE, RELAY3_RESET_PIN, RELAY3_DELAY_ON, RELAY3_DELAY_OFF });
+        _relays.push_back((relay_t) { RELAY3_PIN, RELAY3_TYPE, RELAY3_RESET_PIN });
     #endif
     #if RELAY4_PIN != GPIO_NONE
-        _relays.push_back((relay_t) { RELAY4_PIN, RELAY4_TYPE, RELAY4_RESET_PIN, RELAY4_DELAY_ON, RELAY4_DELAY_OFF });
+        _relays.push_back((relay_t) { RELAY4_PIN, RELAY4_TYPE, RELAY4_RESET_PIN });
     #endif
     #if RELAY5_PIN != GPIO_NONE
-        _relays.push_back((relay_t) { RELAY5_PIN, RELAY5_TYPE, RELAY5_RESET_PIN, RELAY5_DELAY_ON, RELAY5_DELAY_OFF });
+        _relays.push_back((relay_t) { RELAY5_PIN, RELAY5_TYPE, RELAY5_RESET_PIN });
     #endif
     #if RELAY6_PIN != GPIO_NONE
-        _relays.push_back((relay_t) { RELAY6_PIN, RELAY6_TYPE, RELAY6_RESET_PIN, RELAY6_DELAY_ON, RELAY6_DELAY_OFF });
+        _relays.push_back((relay_t) { RELAY6_PIN, RELAY6_TYPE, RELAY6_RESET_PIN });
     #endif
     #if RELAY7_PIN != GPIO_NONE
-        _relays.push_back((relay_t) { RELAY7_PIN, RELAY7_TYPE, RELAY7_RESET_PIN, RELAY7_DELAY_ON, RELAY7_DELAY_OFF });
+        _relays.push_back((relay_t) { RELAY7_PIN, RELAY7_TYPE, RELAY7_RESET_PIN });
     #endif
     #if RELAY8_PIN != GPIO_NONE
-        _relays.push_back((relay_t) { RELAY8_PIN, RELAY8_TYPE, RELAY8_RESET_PIN, RELAY8_DELAY_ON, RELAY8_DELAY_OFF });
+        _relays.push_back((relay_t) { RELAY8_PIN, RELAY8_TYPE, RELAY8_RESET_PIN });
     #endif
 
     // Dummy relays for AI Light, Magic Home LED Controller, H801, Sonoff Dual and Sonoff RF Bridge
     // No delay_on or off for these devices to easily allow having more than
     // 8 channels. This behaviour will be recovered with v2.
     for (unsigned char i=0; i < DUMMY_RELAY_COUNT; i++) {
-        _relays.push_back((relay_t) {GPIO_NONE, RELAY_TYPE_NORMAL, 0, 0, 0});
+        _relays.push_back((relay_t) { GPIO_NONE, RELAY_TYPE_NORMAL, GPIO_NONE });
     }
 
     _relayBackwards();

code/espurna/scheduler.ino

@@ -256,7 +256,7 @@ void _schLoop() {
     if (!ntpSynced()) return;
 
     if (_sch_restore == 0) {
-        for (int i = 0; i < _relays.size(); i++){
+        for (unsigned char i = 0; i < relayCount(); i++){
             if (getSetting("relayLastSch", i, SCHEDULER_RESTORE_LAST_SCHEDULE).toInt() == 1)
                 _schCheck(i, 0);
         }