lights: refactor providers

- make sure we have custom provider before calling the update,
replace old check in the update function
- fix race condition in the provider function, causing complete lock-up of
lights controls (triggered by sending light updates very fast. for example,
using arrow keys with the webui sliders; ref. #2424)
- rework state change functions to work using loop() func instead of a
pair of scheduled functions. we should not go out-of-sync now, and it
also avoids forcibly creating std::func object each invocation
- rework other providers to be more in line with custom callbacks -
introduce state, value and update calls as a common way to handle lights
- correctly handle state change when turning OFF and ON
- update tuya to store the last state, never send channel values when OFF (ref. #2424)

code/espurna/compat.h

@@ -97,18 +97,25 @@ using std::isnan;
 #endif
 
 // -----------------------------------------------------------------------------
-// std::make_unique backport for C++11, since we still use it
+// std::make_unique & std::clamp backports for C++11, since we still use it
 // -----------------------------------------------------------------------------
-#if 201103L >= __cplusplus
+#if __cplusplus <= 201103L
 
 #include <memory>
 namespace std {
-    template<typename T, typename... Args>
-    std::unique_ptr<T> make_unique(Args&&... args) {
-        return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
-    }
+
+template<typename T, typename... Args>
+std::unique_ptr<T> make_unique(Args&&... args) {
+    return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
+}
+
+template <typename T>
+constexpr const T& clamp(const T& value, const T& low, const T& high) {
+    return (value < low) ? low : (high < value) ? high : value;
 }
 
+} // namespace std
+
 #endif
 
 // -----------------------------------------------------------------------------

code/espurna/libs/OnceFlag.h

@@ -0,0 +1,37 @@
+/*
+
+Helper class to set the boolean exactly when setting multiple times in a row
+(as an alternative to checking input every time before setting)
+
+*/
+
+#pragma once
+
+struct OnceFlag {
+    OnceFlag() = default;
+    OnceFlag(const OnceFlag&) = delete;
+    OnceFlag(OnceFlag&&) = delete;
+
+    explicit operator bool() const {
+        return _value;
+    }
+
+    OnceFlag& operator=(bool value) {
+        if (!_value) {
+            _value = value;
+        }
+        return *this;
+    }
+
+    void set() {
+        _value = true;
+    }
+
+    bool get() const {
+        return _value;
+    }
+
+private:
+    bool _value { false };
+};
+

code/espurna/light.cpp

@@ -17,6 +17,7 @@ Copyright (C) 2016-2019 by Xose Pérez <xose dot perez at gmail dot com>
 #include "rpc.h"
 #include "rtcmem.h"
 #include "ws.h"
+#include "libs/OnceFlag.h"
 
 #include "light_config.h"
 
@@ -26,7 +27,7 @@ Copyright (C) 2016-2019 by Xose Pérez <xose dot perez at gmail dot com>
 #include <vector>
 
 extern "C" {
-    #include "libs/fs_math.h"
+#include "libs/fs_math.h"
 }
 
 #if LIGHT_PROVIDER == LIGHT_PROVIDER_DIMMER
@@ -34,7 +35,7 @@ extern "C" {
 // default is 8, we only need up to 5
 #define PWM_CHANNEL_NUM_MAX Light::ChannelsMax
 extern "C" {
-    #include "libs/pwm.h"
+#include "libs/pwm.h"
 }
 
 #endif
@@ -95,10 +96,10 @@ struct channel_t {
 
     bool state { true };                // is the channel ON
 
-    unsigned char inputValue { 0 };     // raw value, without the brightness
-    unsigned char value { 0 };          // normalized value, including brightness
-    unsigned char target { 0 };         // target value
-    float current { 0.0f };             // transition value
+    unsigned char inputValue { Light::VALUE_MIN };     // raw value, without the brightness
+    unsigned char value { Light::VALUE_MIN };          // normalized value, including brightness
+    unsigned char target { Light::VALUE_MIN };         // target value
+    float current { Light::VALUE_MIN };             // transition value
 };
 
 std::vector<channel_t> _light_channels;
@@ -117,7 +118,6 @@ bool _light_use_white = false;
 bool _light_use_cct = false;
 bool _light_use_gamma = false;
 
-bool _light_dirty = false;
 bool _light_state = false;
 unsigned char _light_brightness = Light::BRIGHTNESS_MAX;
 
@@ -131,9 +131,10 @@ long _light_warm_kelvin = (1000000L / _light_warm_mireds);
 
 long _light_mireds = lround((_light_cold_mireds + _light_warm_mireds) / 2L);
 
-using light_brightness_func_t = void(*)();
+using light_brightness_func_t = bool(*)();
 light_brightness_func_t _light_brightness_func = nullptr;
 
+bool _light_state_changed = false;
 LightStateListener _light_state_listener = nullptr;
 
 #if LIGHT_PROVIDER == LIGHT_PROVIDER_MY92XX
@@ -187,14 +188,17 @@ static_assert(Light::VALUE_MAX <= sizeof(_light_gamma_table), "Out-of-bounds arr
 // UTILS
 // -----------------------------------------------------------------------------
 
-void _setValue(const unsigned char id, unsigned int value) {
+bool _setValue(unsigned char, unsigned int) __attribute__((warn_unused_result));
+bool _setValue(unsigned char id, unsigned int value) {
     if (_light_channels[id].value != value) {
         _light_channels[id].value = value;
-        _light_dirty = true;
+        return true;
     }
+
+    return false;
 }
 
-void _setInputValue(const unsigned char id, unsigned int value) {
+void _setInputValue(unsigned char id, unsigned int value) {
     _light_channels[id].inputValue = value;
 }
 
@@ -209,27 +213,29 @@ void _setCCTInputValue(unsigned char warm, unsigned char cold) {
     _setInputValue(1, constrain(cold, Light::VALUE_MIN, Light::VALUE_MAX));
 }
 
-void _lightApplyBrightness(size_t channels = lightChannels()) {
-
+bool _lightApplyBrightness(size_t channels = lightChannels()) {
     double brightness = static_cast<double>(_light_brightness) / static_cast<double>(Light::BRIGHTNESS_MAX);
 
     channels = std::min(channels, lightChannels());
 
+    OnceFlag changed;
     for (unsigned char i=0; i < lightChannels(); i++) {
         if (i >= channels) brightness = 1;
-        _setValue(i, _light_channels[i].inputValue * brightness);
+        changed = _setValue(i, _light_channels[i].inputValue * brightness);
     }
 
+    return changed.get();
 }
 
-void _lightApplyBrightnessColor() {
+bool _lightApplyBrightnessColor() {
+    OnceFlag changed;
 
     double brightness = static_cast<double>(_light_brightness) / static_cast<double>(Light::BRIGHTNESS_MAX);
 
     // Substract the common part from RGB channels and add it to white channel. So [250,150,50] -> [200,100,0,50]
     unsigned char white = std::min(_light_channels[0].inputValue, std::min(_light_channels[1].inputValue, _light_channels[2].inputValue));
     for (unsigned int i=0; i < 3; i++) {
-        _setValue(i, _light_channels[i].inputValue - white);
+        changed = _setValue(i, _light_channels[i].inputValue - white);
     }
 
     // Split the White Value across 2 White LED Strips.
@@ -240,14 +246,14 @@ void _lightApplyBrightnessColor() {
 
         // set cold white
         _light_channels[3].inputValue = 0;
-        _setValue(3, lround(((double) 1.0 - miredFactor) * white));
+        changed = _setValue(3, lround(((double) 1.0 - miredFactor) * white));
 
         // set warm white
         _light_channels[4].inputValue = 0;
-        _setValue(4, lround(miredFactor * white));
+        changed = _setValue(4, lround(miredFactor * white));
     } else {
         _light_channels[3].inputValue = 0;
-        _setValue(3, white);
+        changed = _setValue(3, white);
     }
 
     // Scale up to equal input values. So [250,150,50] -> [200,100,0,50] -> [250, 125, 0, 63]
@@ -261,20 +267,21 @@ void _lightApplyBrightnessColor() {
 
     double factor = (max_out > 0) ? (double) (max_in / max_out) : 0;
     for (unsigned char i=0; i < channelSize; i++) {
-        _setValue(i, lround((double) _light_channels[i].value * factor * brightness));
+        changed = _setValue(i, lround((double) _light_channels[i].value * factor * brightness));
     }
 
     // Scale white channel to match brightness
     for (unsigned char i=3; i < channelSize; i++) {
-        _setValue(i, constrain(static_cast<unsigned int>(_light_channels[i].value * LIGHT_WHITE_FACTOR), Light::BRIGHTNESS_MIN, Light::BRIGHTNESS_MAX));
+        changed = _setValue(i, constrain(static_cast<unsigned int>(_light_channels[i].value * LIGHT_WHITE_FACTOR), Light::BRIGHTNESS_MIN, Light::BRIGHTNESS_MAX));
     }
 
     // For the rest of channels, don't apply brightness, it is already in the inputValue
     // i should be 4 when RGBW and 5 when RGBWW
     for (unsigned char i=channelSize; i < _light_channels.size(); i++) {
-        _setValue(i, _light_channels[i].inputValue);
+        changed = _setValue(i, _light_channels[i].inputValue);
     }
 
+    return changed.get();
 }
 
 String lightDesc(unsigned char id) {
@@ -632,20 +639,6 @@ void _lightAdjustMireds(const String& payload) {
 // PROVIDER
 // -----------------------------------------------------------------------------
 
-unsigned int _toPWM(unsigned int value, bool gamma, bool inverse) {
-    value = constrain(value, Light::VALUE_MIN, Light::VALUE_MAX);
-    if (gamma) value = pgm_read_byte(_light_gamma_table + value);
-    if (Light::VALUE_MAX != Light::PWM_LIMIT) value = _lightMap(value, Light::VALUE_MIN, Light::VALUE_MAX, Light::PWM_MIN, Light::PWM_LIMIT);
-    if (inverse) value = LIGHT_LIMIT_PWM - value;
-    return value;
-}
-
-// Returns a PWM value for the given channel ID
-unsigned int _toPWM(unsigned char id) {
-    bool useGamma = _light_use_gamma && _light_has_color && (id < 3);
-    return _toPWM(_light_channels[id].current, useGamma, _light_channels[id].inverse);
-}
-
 namespace {
 
 class LightTransitionHandler {
@@ -665,29 +658,32 @@ public:
     };
 
     explicit LightTransitionHandler(Channels& channels, bool state, LightTransition transition) :
+        _state(state),
         _time(transition.time),
         _step(transition.step)
     {
+        OnceFlag delayed;
         for (auto& channel : channels) {
-            prepare(channel, state);
+            delayed = prepare(channel, state);
         }
 
         // if nothing to do, ignore transition step & time and just schedule as soon as possible
-        if (!transitions()) {
+        if (!delayed) {
             reset();
             return;
         }
 
-        DEBUG_MSG_P(PSTR("[LIGHT] Scheduled transition every %ums (total %ums)\n"), _step, _time);
+        DEBUG_MSG_P(PSTR("[LIGHT] Scheduled transition for %u (ms) every %u (ms)\n"), _time, _step);
     }
 
-    void prepare(channel_t& channel, bool state) {
-        channel.target = (state && channel.state) ? channel.value : 0;
+    bool prepare(channel_t& channel, bool state) {
+        bool target_state = state && channel.state;
+        channel.target = target_state ? channel.value : Light::VALUE_MIN;
 
         float diff = static_cast<float>(channel.target) - channel.current;
-        if (!_time || (_step >= _time) || (std::abs(diff) <= std::numeric_limits<float>::epsilon())) {
-            channel.current = channel.target;
-            return;
+        if (isImmediateTransition(target_state, diff)) {
+            _transitions.push_back(Transition{channel.current, channel.target, diff, 1});
+            return false;
         }
 
         float step = (diff > 0.0) ? 1.0f : -1.0f;
@@ -703,6 +699,8 @@ public:
         transition.debug();
 
         _transitions.push_back(transition);
+
+        return true;
     }
 
     void reset() {
@@ -710,23 +708,43 @@ public:
         _time = 10;
     }
 
-    bool next() {
-        bool result { false };
+    template <typename StateFunc, typename ValueFunc, typename UpdateFunc>
+    bool run(StateFunc&& state, ValueFunc&& value, UpdateFunc&& update) {
+        bool next { false };
 
-        for (auto& transition : _transitions) {
+        if (!_state_notified && _state) {
+            _state_notified = true;
+            state(_state);
+        }
+
+        for (unsigned char index = 0; index < _transitions.size(); ++index) {
+            auto& transition = _transitions[index];
             if (!transition.count) {
                 continue;
             }
 
             if (--transition.count) {
                 transition.value += transition.step;
-                result = true;
+                next = true;
             } else {
                 transition.value = transition.target;
             }
+
+            value(index, transition.value);
         }
 
-        return result;
+        if (!_state_notified && !next && !_state) {
+            _state_notified = true;
+            state(_state);
+        }
+
+        update();
+
+        return next;
+    }
+
+    bool state() const {
+        return _state;
     }
 
     unsigned long step() const {
@@ -737,18 +755,77 @@ public:
         return _time;
     }
 
-    size_t transitions() const {
-        return _transitions.size();
+private:
+    bool isImmediateTransition(bool state, float diff) {
+        return (!_time || (_step >= _time) || (std::abs(diff) <= std::numeric_limits<float>::epsilon())
+                || (!state && (diff > 0.0)) || (state && (diff < 0.0)));
     }
 
-private:
     std::vector<Transition> _transitions;
+    bool _state_notified { false };
+
+    bool _state;
     unsigned long _time;
     unsigned long _step;
 };
 
 } // namespace
 
+struct LightUpdateHandler {
+    LightUpdateHandler() = default;
+
+    explicit operator bool() {
+        return _run;
+    }
+
+    void lock() {
+        _lock = true;
+    }
+
+    void unlock() {
+        _lock = false;
+    }
+
+    void reset() {
+        _lock = false;
+        _run = false;
+    }
+
+    void set(bool save, LightTransition transition, int report) {
+        if (_lock) {
+            panic();
+        }
+
+        _run = true;
+
+        _save = save;
+        _transition = transition;
+        _report = report;
+    }
+
+    template <typename T>
+    void run(T&& callback) {
+        if (!_run) {
+            panic();
+        }
+
+        lock();
+        callback(_save, _transition, _report);
+        reset();
+    }
+
+private:
+    bool _save;
+    LightTransition _transition;
+    int _report;
+
+    bool _run { false };
+    bool _lock { false };
+};
+
+LightUpdateHandler _light_update;
+bool _light_provider_update = false;
+
 std::unique_ptr<LightTransitionHandler> _light_transition;
 
 Ticker _light_transition_ticker;
@@ -756,51 +833,83 @@ bool _light_use_transitions = false;
 unsigned long _light_transition_time = LIGHT_TRANSITION_TIME;
 unsigned long _light_transition_step = LIGHT_TRANSITION_STEP;
 
-bool _light_provider_update = false;
-
 void _lightProviderSchedule(unsigned long ms);
 
-void _lightProviderUpdate() {
+#if (LIGHT_PROVIDER == LIGHT_PROVIDER_DIMMER) || (LIGHT_PROVIDER == LIGHT_PROVIDER_MY92XX)
 
-    if (_light_provider_update) return;
-    _light_provider_update = true;
+// there is no PWM stop and it seems my92xx version is fine by just setting 0 values for channels
+void _lightProviderHandleState(bool) {
+}
 
-    if (!_light_transition) return;
-    auto next = _light_transition->next();
+// both require original values to be scaled into a PWM frequency
+void _lightProviderHandleValue(unsigned char channel, float value) {
+    // TODO: strict rule in the transition itself?
+    if (value < 0.0) {
+        return;
+    }
 
-    #if LIGHT_PROVIDER == LIGHT_PROVIDER_MY92XX
+    // TODO: have 'red', 'green' or 'blue' tag instead of using hard-coded index offset?
+    auto gamma = _light_use_gamma && _light_has_color && (channel < 3);
+    auto inverse = _light_channels[channel].inverse;
 
-        for (unsigned char i=0; i<_light_channels.size(); i++) {
-            _my92xx->setChannel(_light_channel_map[i], _toPWM(i));
-        }
-        _my92xx->setState(true);
-        _my92xx->update();
+    auto rounded = std::lround(value);
+    if (gamma) {
+        rounded = pgm_read_byte(_light_gamma_table + rounded);
+    }
 
-    #endif
+    if (Light::VALUE_MAX != Light::PWM_LIMIT) {
+        rounded = _lightMap(rounded, Light::VALUE_MIN, Light::VALUE_MAX, Light::PWM_MIN, Light::PWM_LIMIT);
+    }
 
-    #if LIGHT_PROVIDER == LIGHT_PROVIDER_DIMMER
+    if (inverse) {
+       rounded = Light::PWM_LIMIT - rounded;
+    }
 
-        for (unsigned char i=0; i < _light_channels.size(); i++) {
-            pwm_set_duty(_toPWM(i), i);
-        }
-        pwm_start();
+#if LIGHT_PROVIDER == LIGHT_PROVIDER_DIMMER
+    pwm_set_duty(rounded, channel);
+#elif LIGHT_PROVIDER == LIGHT_PROVIDER_MY92XX
+    _my92xx->setChannel(_light_channel_map[channel], rounded);
+#endif
+}
 
-    #endif
+void _lightProviderHandleUpdate() {
+#if LIGHT_PROVIDER == LIGHT_PROVIDER_DIMMER
+    pwm_start();
+#elif LIGHT_PROVIDER == LIGHT_PROVIDER_MY92XX
+    _my92xx->setState(true);
+    _my92xx->update();
+#endif
+}
 
-    #if LIGHT_PROVIDER == LIGHT_PROVIDER_CUSTOM
+#elif LIGHT_PROVIDER == LIGHT_PROVIDER_CUSTOM
 
-        if (_light_provider) {
-            for (unsigned char i=0; i < _light_channels.size(); i++) {
-                _light_provider->channel(i, _light_channels[i].current);
-            }
-            _light_provider->update();
-        }
+void _lightProviderHandleState(bool state) {
+    _light_provider->state(state);
+}
 
-        if (!next) {
-            _light_provider->state(_light_state);
-        }
+void _lightProviderHandleValue(unsigned char channel, float value) {
+    _light_provider->channel(channel, value);
+}
 
-    #endif
+void _lightProviderHandleUpdate() {
+    _light_provider->update();
+}
+
+#endif
+
+void _lightProviderUpdate() {
+    if (!_light_provider_update) {
+        return;
+    }
+
+    if (!_light_transition) {
+        _light_provider_update = false;
+    }
+
+    auto next = _light_transition->run(
+        _lightProviderHandleState,
+        _lightProviderHandleValue,
+        _lightProviderHandleUpdate);
 
     if (next) {
         _lightProviderSchedule(_light_transition->step());
@@ -809,11 +918,12 @@ void _lightProviderUpdate() {
     }
 
     _light_provider_update = false;
-
 }
 
 void _lightProviderSchedule(unsigned long ms) {
-    _light_transition_ticker.once_ms_scheduled(ms, _lightProviderUpdate);
+    _light_transition_ticker.once_ms(ms, []() {
+        _light_provider_update = true;
+    });
 }
 
 // -----------------------------------------------------------------------------
@@ -1067,7 +1177,6 @@ void _lightMqttSetup() {
 }
 
 void lightMQTT() {
-
     char buffer[20];
 
     if (_light_has_color) {
@@ -1084,31 +1193,24 @@ void lightMQTT() {
         mqttSend(MQTT_TOPIC_COLOR_HSV, buffer);
 
     }
-    
+
     if (_light_has_color || _light_use_cct) {
-      
-      // Mireds
-      snprintf_P(buffer, sizeof(buffer), PSTR("%d"), _light_mireds);
-      mqttSend(MQTT_TOPIC_MIRED, buffer);
-    
+        snprintf_P(buffer, sizeof(buffer), PSTR("%d"), _light_mireds);
+        mqttSend(MQTT_TOPIC_MIRED, buffer);
     }
 
-    // Channels
     for (unsigned int i=0; i < _light_channels.size(); i++) {
         itoa(_light_channels[i].target, buffer, 10);
         mqttSend(MQTT_TOPIC_CHANNEL, i, buffer);
     }
 
-    // Brightness
     snprintf_P(buffer, sizeof(buffer), PSTR("%d"), _light_brightness);
     mqttSend(MQTT_TOPIC_BRIGHTNESS, buffer);
 
-    // Global
     if (!_light_has_controls) {
         snprintf_P(buffer, sizeof(buffer), "%c", _light_state ? '1' : '0');
         mqttSend(MQTT_TOPIC_LIGHT, buffer);
     }
-
 }
 
 void lightMQTTGroup() {
@@ -1495,37 +1597,50 @@ void _lightReport(Light::Report report) {
     _lightReport(static_cast<int>(report));
 }
 
-void lightUpdate(bool save, LightTransition transition, int report) {
-    // Calculate values based on inputs and brightness
-    // Update only if the values had actually changed
-    _light_brightness_func();
+// Called in the loop() when we received lightUpdate(...) values
 
-    if (!_light_channels.size()) {
+void _lightUpdate() {
+    if (!_light_update) {
         return;
     }
 
-    if (!_light_dirty) {
+#if LIGHT_PROVIDER == LIGHT_PROVIDER_CUSTOM
+    if (!_light_provider) {
+        return;
+    }
+#endif
+
+    auto changed = _light_brightness_func();
+    if (!_light_state_changed && !changed) {
+        _light_update.reset();
         return;
     }
-    _light_dirty = false;
 
-    // Channel output values will be set by the handler class and the specified provider
-    // We either set the values immediately or schedule an ongoing transition
-    _light_transition = std::make_unique<LightTransitionHandler>(_light_channels, _light_state, transition);
-    _lightProviderSchedule(_light_transition->step());
+    _light_state_changed = false;
 
-    // Send current state to all available 'report' targets
-    // (make sure to delay the report, in case lightUpdate is called repeatedly)
-    _light_report_ticker.once_ms(_light_report_delay, [report]() {
-        _lightReport(report);
+    _light_update.run([](bool save, LightTransition transition, int report) {
+        // Channel output values will be set by the handler class and the specified provider
+        // We either set the values immediately or schedule an ongoing transition
+        _light_transition = std::make_unique<LightTransitionHandler>(_light_channels, _light_state, transition);
+        _lightProviderSchedule(_light_transition->step());
+
+        // Send current state to all available 'report' targets
+        // (make sure to delay the report, in case lightUpdate is called repeatedly)
+        _light_report_ticker.once_ms(_light_report_delay, [report]() {
+            _lightReport(report);
+        });
+
+        // Always save to RTCMEM, optionally preserve the state in the settings storage
+        _lightSaveRtcmem();
+        if (save) {
+            _light_save_ticker.once_ms(_light_save_delay, _lightSaveSettings);
+        }
     });
+}
 
-    // Always save to RTCMEM, optionally preserve the state in the settings storage
-    _lightSaveRtcmem();
-    if (save) {
-        _light_save_ticker.once_ms(_light_save_delay, _lightSaveSettings);
-    }
-};
+void lightUpdate(bool save, LightTransition transition, int report) {
+    _light_update.set(save, transition, report);
+}
 
 void lightUpdate(bool save, LightTransition transition, Light::Report report) {
     lightUpdate(save, transition, static_cast<int>(report));
@@ -1547,7 +1662,7 @@ void lightState(unsigned char id, bool state) {
     if (id >= _light_channels.size()) return;
     if (_light_channels[id].state != state) {
         _light_channels[id].state = state;
-        _light_dirty = true;
+        _light_state_changed = true;
     }
 }
 
@@ -1558,10 +1673,11 @@ bool lightState(unsigned char id) {
 
 void lightState(bool state) {
     if (_light_state != state) {
-        if (_light_state_listener)
-            _light_state_listener(state);
         _light_state = state;
-        _light_dirty = true;
+        if (_light_state_listener) {
+            _light_state_listener(state);
+        }
+        _light_state_changed = true;
     }
 }
 
@@ -1708,10 +1824,10 @@ void _lightConfigure() {
         if (_light_use_white) {
             _light_brightness_func = _lightApplyBrightnessColor;
         } else {
-            _light_brightness_func = []() { _lightApplyBrightness(3); };
+            _light_brightness_func = []() { return _lightApplyBrightness(3); };
         }
     } else {
-        _light_brightness_func = []() { _lightApplyBrightness(); };
+        _light_brightness_func = []() { return _lightApplyBrightness(); };
     }
 
     _light_use_cct = getSetting("useCCT", 1 == LIGHT_USE_CCT);
@@ -1893,6 +2009,10 @@ void lightSetup() {
     #endif
 
     espurnaRegisterReload(_lightConfigure);
+    espurnaRegisterLoop([]() {
+        _lightUpdate();
+        _lightProviderUpdate();
+    });
 
 }
 

code/espurna/tuya.cpp

@@ -17,6 +17,8 @@ Copyright (C) 2019-2021 by Maxim Prokhorov <prokhorov dot max at outlook dot com
 #include "relay.h"
 #include "rpc.h"
 
+#include "libs/OnceFlag.h"
+
 #include <functional>
 #include <queue>
 #include <forward_list>
@@ -121,41 +123,42 @@ namespace tuya {
             _channels(channels)
         {}
 
-        explicit TuyaLightProvider(const DpMap& channels, StateId* state) :
+        explicit TuyaLightProvider(const DpMap& channels, StateId* stateId) :
             _channels(channels),
-            _state(state)
+            _stateId(stateId)
         {}
 
         void update() override {
         }
 
-        void state(bool status) override {
-            if (_state && *_state) {
-                _state->filter(false);
-                if (!status) {
-                    send(_state->id(), status);
-                }
+        // Channel values > 0 will switch the lights ON anyway
+        void state(bool value) override {
+            _last_state = value;
+            if (*_stateId && !value) {
+                send(_stateId->id(), value);
             }
         }
 
         void channel(unsigned char channel, double value) override {
+            // XXX: can't handle channel values when OFF, and will turn the lights ON
+            if (!_last_state) {
+                return;
+            }
+
             auto* entry = _channels.find_local(channel);
             if (!entry) {
                 return;
             }
 
-            // tuya dimmer is precious, and can't handle 0...some-kind-of-threshold
-            // just ignore it, the associated switch will handle turning it off
             auto rounded = static_cast<unsigned int>(value);
-            if (rounded <= 0x10) {
-                return;
-            }
 
-            // Filtering for incoming data
-            // ref. https://github.com/xoseperez/espurna/issues/2222
-            // TODO: should be fixed when relay & channel transition states are implemented as transactions?
-            if (_state && *_state) {
-                _state->filter(true);
+            // input dimmer channel value when lights are OFF is 16
+            // for the same reason as above, don't send OFF values
+            constexpr unsigned int Low { 16u };
+            constexpr unsigned int High { 255u };
+            rounded = std::clamp(rounded, Low, High);
+            if (rounded == 16u) {
+                return;
             }
 
             send(entry->dp_id, rounded);
@@ -163,7 +166,8 @@ namespace tuya {
 
     private:
         const DpMap& _channels;
-        StateId* _state { nullptr };
+        bool _last_state { false };
+        StateId* _stateId { nullptr };
     };
 
 #endif

code/espurna/tuya_util.h

@@ -8,8 +8,8 @@ Copyright (C) 2019 by Maxim Prokhorov <prokhorov dot max at outlook dot com>
 
 #pragma once
 
-#include <cstdint>
 #include <algorithm>
+#include <cstdint>
 #include <vector>
 
 #include "tuya_types.h"
@@ -61,34 +61,6 @@ private:
     bool _filter { false };
 };
 
-struct OnceFlag {
-    OnceFlag() = default;
-    OnceFlag(const OnceFlag&) = delete;
-    OnceFlag(OnceFlag&&) = delete;
-
-    explicit operator bool() const {
-        return _value;
-    }
-
-    OnceFlag& operator=(bool value) {
-        if (!_value) {
-            _value = value;
-        }
-        return *this;
-    }
-
-    void set() {
-        _value = true;
-    }
-
-    bool get() const {
-        return _value;
-    }
-
-private:
-    bool _value { false };
-};
-
 struct Dp {
     Type type;
     uint8_t id;