From d4698c0a37998fe486d20ffa9326f04a083f627b Mon Sep 17 00:00:00 2001
From: Max Prokhorov <prokhorov.max@outlook.com>
Date: Tue, 1 Oct 2019 01:50:40 +0300
Subject: [PATCH] light: fix inconsistent transitions (#1901, #1923)

* light: fix inconsistent transitions

- capture step variable for the current transition
- use one-shot timer, restart from the timer callback

* schedule inside provider func, not transition
---
 code/espurna/light.ino | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/code/espurna/light.ino b/code/espurna/light.ino
index 9eb8c009..037d0077 100644
--- a/code/espurna/light.ino
+++ b/code/espurna/light.ino
@@ -52,7 +52,6 @@ bool _light_provider_update = false;
 
 bool _light_use_transitions = false;
 unsigned int _light_transition_time = LIGHT_TRANSITION_TIME;
-unsigned long _light_steps_left = 1;
 
 bool _light_dirty = false;
 bool _light_state = false;
@@ -499,33 +498,25 @@ unsigned int _toPWM(unsigned char id) {
     return _toPWM(_light_channel[id].current, useGamma, _light_channel[id].reverse);
 }
 
-void _transition() {
+void _lightTransition(unsigned long step) {
 
-    // Update transition ticker
-    _light_steps_left--;
-    if (_light_steps_left == 0) _light_transition_ticker.detach();
-
-    // Transitions
-    for (unsigned int i=0; i < _light_channel.size(); i++) {
-
-        if (_light_steps_left == 0) {
-            _light_channel[i].current = _light_channel[i].target;
+    // Transitions based on current step. If step == 0, then it is the last transition
+    for (auto& channel : _light_channel) {
+        if (!step) {
+            channel.current = channel.target;
         } else {
-            double difference = (double) (_light_channel[i].target - _light_channel[i].current) / (_light_steps_left + 1);
-            _light_channel[i].current = _light_channel[i].current + difference;
+            channel.current += (double) (channel.target - channel.current) / (step + 1);
         }
-
     }
 
 }
 
-void _lightProviderUpdate() {
+void _lightProviderUpdate(unsigned long steps) {
 
     if (_light_provider_update) return;
-
     _light_provider_update = true;
 
-    _transition();
+    _lightTransition(--steps);
 
     #if LIGHT_PROVIDER == LIGHT_PROVIDER_MY92XX
 
@@ -546,12 +537,15 @@ void _lightProviderUpdate() {
 
     #endif
 
+    // This is not the final value, update again
+    if (steps) _light_transition_ticker.once_ms(LIGHT_TRANSITION_STEP, _lightProviderScheduleUpdate, steps);
+
     _light_provider_update = false;
 
 }
 
-void _lightProviderDoUpdate() {
-    schedule_function(_lightProviderUpdate);
+void _lightProviderScheduleUpdate(unsigned long steps) {
+    schedule_function(std::bind(_lightProviderUpdate, steps));
 }
 
 // -----------------------------------------------------------------------------
@@ -833,9 +827,10 @@ void lightUpdate(bool save, bool forward, bool group_forward) {
         //DEBUG_MSG_P("[LIGHT] Channel #%u target value: %u\n", i, _light_channel[i].target);
     }
 
-    // Configure color transition
-    _light_steps_left = _light_use_transitions ? _light_transition_time / LIGHT_TRANSITION_STEP : 1;
-    _light_transition_ticker.attach_ms(LIGHT_TRANSITION_STEP, _lightProviderDoUpdate);
+    // Channel transition will be handled by the provider function
+    // User can configure total transition time, step time is a fixed value
+    unsigned long steps = _light_use_transitions ? _light_transition_time / LIGHT_TRANSITION_STEP : 1;
+    _light_transition_ticker.once_ms(LIGHT_TRANSITION_STEP, _lightProviderScheduleUpdate, steps);
 
     // Delay every communication 100ms to avoid jamming
     unsigned char mask = 0;