From 21cf917906eccf66abbead3f3eb5357e5f94e4bc Mon Sep 17 00:00:00 2001
From: Nicu Hodos <nicu@hodos.ro>
Date: Mon, 10 Feb 2025 16:02:17 +0100
Subject: [PATCH 1/4] extract timer and generic callback in dedicated files

---
 include/devices.h |  12 ++---
 include/display.h | 127 ++++++++++++++--------------------------------
 include/helper.h  |  12 +++++
 include/timer.h   |  31 +++++++++++
 4 files changed, 88 insertions(+), 94 deletions(-)
 create mode 100644 include/helper.h
 create mode 100644 include/timer.h

diff --git a/include/devices.h b/include/devices.h
index 9c1e90c..562fdad 100644
--- a/include/devices.h
+++ b/include/devices.h
@@ -49,7 +49,7 @@ namespace Devices {
     Number* displayTimerMqtt = Builder<Number>::instance(new Number{ "Timer duration", "timer_duration",
             [](const char* msg) {
                 auto value = String{ msg }.toInt();
-                Display::Timer::timer = value;
+                timer = value;
                 displayTimerMqtt->updateState(value);
             }
         }).withMin(0).withMax(90).withStep(5).restoreStateFromCommand().build();
@@ -58,10 +58,10 @@ namespace Devices {
     Switch* timerMqtt = Builder<Switch>::instance(new Switch{"Timer", "timer",
             [](const char* msg) {
                 if (strcmp("ON", msg) == 0) {
-                    Display::Timer::start();
+                    Display::tTimer.restart();
                     timerMqtt->updateState(true);
                  } else {
-                    Display::Timer::stop();
+                    Display::tTimer.disable();
                     timerMqtt->updateState(false);
                  }
             }
@@ -97,8 +97,8 @@ namespace Devices {
         .addSecondary(
             Builder<Button>::instance(new Button{"Display remaining timer", "display_remaining_timer",
                 [](const char* msg) {
-                    if (strcmp("PRESS", msg) == 0 && !Display::Timer::tDisplayTimer.isEnabled()) {
-                        Display::Timer::tDisplayTimer.restart();
+                    if (strcmp("PRESS", msg) == 0 && !Display::tDisplayTimer.isEnabled()) {
+                        Display::tDisplayTimer.restart();
                     }
                 }
             }).build()
@@ -137,7 +137,7 @@ namespace Devices {
         Display::brightness.registerCallback([]{
             brightnessMqtt->updateState(Display::brightness);
         });
-        Display::Timer::timer.registerCallback([](int8 current){
+        timer.registerCallback([](int8 current){
             timerRemainingMqtt->updateState(to_string(current).c_str());
         });
     }
diff --git a/include/display.h b/include/display.h
index 6e798e4..0fff91d 100644
--- a/include/display.h
+++ b/include/display.h
@@ -6,6 +6,7 @@
 #include <SPI.h>
 #include "ntp_time.h"
 #include "bme.h"
+#include "timer.h"
 
 #define DISPLAY_ADDRESS 0x70
 #define MILLISECONDS(value) value*TASK_MILLISECOND
@@ -14,19 +15,11 @@
 #define DISPLAY_SENSOR_ITERATIONS 2+1
 #define DISPLAY_DELAY (SECONDS(2))
 
-typedef void (*callback_t)();
-
-template <typename T>
-struct CallbackAware {
-    void registerCallback(T f) {
-        callback = f;
-    }
-protected:
-    T callback;
-};
-
 namespace Display {
 
+    // Create display object
+    Adafruit_7segment clockDisplay = Adafruit_7segment();
+
     void displayTime();
     void displayDate();
     void displayTemp();
@@ -61,6 +54,41 @@ namespace Display {
         []() {
             tDisplayTime.enable();
         });
+    Task tDisplayTimer(SECONDS(10), TASK_ONCE + 1, 
+        []{
+            if (timer >= 0) {
+                clockDisplay.print(timer, DEC);
+                clockDisplay.writeDisplay();
+            }
+        }, &ts, false,
+        []{
+            if (!tDisplayTime.isEnabled()) return false;
+            tDisplayTime.disable();
+            return true;
+        },
+        []{
+            tDisplayTimer.setIterations(TASK_ONCE + 1);
+            tDisplayTime.enable();
+        });
+    Task tTimer(MINUTES(1), TASK_FOREVER, 
+        []{
+            static constexpr uint8 threshold = 16;
+            timer.decrease();
+            if (timer.atBeginning()) {
+                if (timer <= threshold) tDisplayTimer.setIterations(TASK_FOREVER);
+                tDisplayTimer.restart();
+            } else if (timer == threshold) {
+                tDisplayTimer.setIterations(TASK_FOREVER);
+                tDisplayTimer.restart();
+            }
+        }, &ts, false, 
+        []{
+            timer.start();
+            return true;
+        },
+        []{
+            tDisplayTimer.disable();
+        });
 
     struct HourFormat : public CallbackAware<callback_t> {
 
@@ -77,9 +105,6 @@ namespace Display {
         bool format24 = false;
     } hourFormat24;
 
-    // Create display object
-    Adafruit_7segment clockDisplay = Adafruit_7segment();
-
     struct Brightness : public CallbackAware<callback_t> {
         static constexpr uint8 MIN = 0;
         static constexpr uint8 MAX = 15;
@@ -100,80 +125,6 @@ namespace Display {
         uint8 current = NIGHT;
     } brightness;
 
-    namespace Timer {
-        typedef void (*remaining_callback_t)(int8);
-        struct : public CallbackAware<remaining_callback_t> {
-
-            void operator=(int8 value) {
-                initial = remaining = value;
-            }
-
-            operator int8() {
-                return remaining;
-            }
-
-            void start() {
-                remaining = initial + 1;
-            }
-
-            void decrease() {
-                remaining--;
-                if (callback) callback(remaining);
-            }
-
-            bool atBeginning() {
-                return initial == remaining;
-            }
-        
-        private:
-            int8 initial = 0, remaining = 0;
-        } timer;
-
-        Task tDisplayTimer(SECONDS(10), TASK_ONCE + 1, 
-            []{
-                if (timer >= 0) {
-                    clockDisplay.print(timer, DEC);
-                    clockDisplay.writeDisplay();
-                }
-            }, &ts, false,
-            []{
-                if (!tDisplayTime.isEnabled()) return false;
-                tDisplayTime.disable();
-                return true;
-            },
-            []{
-                tDisplayTimer.setIterations(TASK_ONCE + 1);
-                tDisplayTime.enable();
-            });
-        Task tTimer(MINUTES(1), TASK_FOREVER, 
-            []{
-                static constexpr uint8 threshold = 16;
-                timer.decrease();
-                if (timer.atBeginning()) {
-                    if (timer <= threshold) tDisplayTimer.setIterations(TASK_FOREVER);
-                    tDisplayTimer.restart();
-                } else if (timer == threshold) {
-                    tDisplayTimer.setIterations(TASK_FOREVER);
-                    tDisplayTimer.restart();
-                }
-            }, &ts, false, 
-            []{
-                timer.start();
-                return true;
-            },
-            []{
-                tDisplayTimer.disable();
-            });
-
-        void start() {
-            tTimer.restart();
-        }
-
-        void stop() {
-            tTimer.disable();
-        }
-    }
-
     void drawTime() {
         int displayHour = hourFormat24 ? hour() : hourFormat12();
         int displayMinute = minute();
diff --git a/include/helper.h b/include/helper.h
new file mode 100644
index 0000000..c84a4ad
--- /dev/null
+++ b/include/helper.h
@@ -0,0 +1,12 @@
+#pragma once
+
+typedef void (*callback_t)();
+
+template <typename T>
+struct CallbackAware {
+    void registerCallback(T f) {
+        callback = f;
+    }
+protected:
+    T callback;
+};
diff --git a/include/timer.h b/include/timer.h
new file mode 100644
index 0000000..864883d
--- /dev/null
+++ b/include/timer.h
@@ -0,0 +1,31 @@
+#pragma once
+
+#include "helper.h"
+
+typedef void (*remaining_callback_t)(int8);
+struct : public CallbackAware<remaining_callback_t> {
+
+    void operator=(int8 value) {
+        initial = remaining = value;
+    }
+
+    operator int8() {
+        return remaining;
+    }
+
+    void start() {
+        remaining = initial + 1;
+    }
+
+    void decrease() {
+        remaining--;
+        if (callback) callback(remaining);
+    }
+
+    bool atBeginning() {
+        return initial == remaining;
+    }
+
+private:
+    int8 initial = 0, remaining = 0;
+} timer;

From 985bcd1c092eac6b1c80e18f41c52876131daa24 Mon Sep 17 00:00:00 2001
From: Nicu Hodos <nicu@hodos.ro>
Date: Wed, 12 Feb 2025 08:14:51 +0100
Subject: [PATCH 2/4] bring back functionality where displaying various info
 can be overwritten by adding a display controller class

---
 include/devices.h |  6 ++---
 include/display.h | 62 ++++++++++++++++++++++++++++++-----------------
 src/esp_clock.cpp |  4 +--
 3 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/include/devices.h b/include/devices.h
index 562fdad..b4dd440 100644
--- a/include/devices.h
+++ b/include/devices.h
@@ -80,7 +80,7 @@ namespace Devices {
                     if (strcmp("PRESS", msg) == 0 && !Display::tDisplaySensor.isEnabled()) {
                         Bme::data.readAll();
                         Display::tDisplaySensor.setIterations(DISPLAY_SENSOR_ITERATIONS);
-                        Display::tDisplaySensor.restart();
+                        Display::displayTask.activate(Display::tDisplaySensor);
                     };
                 }
             }).build()
@@ -89,7 +89,7 @@ namespace Devices {
             Builder<Button>::instance(new Button{"Display date", "display_date",
                 [](const char* msg) {
                     if (strcmp("PRESS", msg) == 0 && !Display::tDisplayDate.isEnabled()) {
-                        Display::tDisplayDate.restart();
+                        Display::displayTask.activate(Display::tDisplayDate);
                     };
                 }
             }).build()
@@ -98,7 +98,7 @@ namespace Devices {
             Builder<Button>::instance(new Button{"Display remaining timer", "display_remaining_timer",
                 [](const char* msg) {
                     if (strcmp("PRESS", msg) == 0 && !Display::tDisplayTimer.isEnabled()) {
-                        Display::tDisplayTimer.restart();
+                        Display::displayTask.activate(Display::tDisplayTimer);
                     }
                 }
             }).build()
diff --git a/include/display.h b/include/display.h
index 0fff91d..8457455 100644
--- a/include/display.h
+++ b/include/display.h
@@ -20,6 +20,36 @@ namespace Display {
     // Create display object
     Adafruit_7segment clockDisplay = Adafruit_7segment();
 
+    struct {
+        operator Task&() {
+            return *perm;
+        }
+
+        Task& activate(Task& task) {
+            if (temp) temp->cancel();
+            if (perm) perm->cancel();
+            if (task.getIterations() == TASK_FOREVER) {
+                task.enable();
+                perm = &task;
+            } else {
+                task.restart();
+                temp = &task;
+            }
+            return task;
+        }
+
+        void restorePerm() {
+            if (temp && temp->isEnabled()) return;  // don't enable perm when triggered by another temp on top of exsting temp
+            if (perm) perm->enable();
+        }
+
+        bool isPerm(Task& task) {
+            return perm == &task;
+        }
+    private:
+        Task *temp = nullptr, *perm = nullptr;
+    } displayTask;
+
     void displayTime();
     void displayDate();
     void displayTemp();
@@ -36,23 +66,16 @@ namespace Display {
         });
     Task tDisplaySensor(SECONDS(5), DISPLAY_SENSOR_ITERATIONS, nullptr, &ts, false,
         []() {
-            if (!tDisplayTime.isEnabled()) return false;
-            tDisplayTime.disable();
             tDisplaySensor.setCallback(displayTemp);
             return true;
         },
         []() {
             tDisplaySensor.setIterations((DISPLAY_SENSOR_ITERATIONS - 1) * 2 + 1);
-            tDisplayTime.enable();
+            displayTask.restorePerm();
         });
-    Task tDisplayDate(SECONDS(5), TASK_ONCE + 1, displayDate, &ts, false,
+    Task tDisplayDate(SECONDS(5), TASK_ONCE + 1, displayDate, &ts, false, nullptr,
         []() {
-            if (!tDisplayTime.isEnabled()) return false;
-            tDisplayTime.disable();
-            return true;
-        },
-        []() {
-            tDisplayTime.enable();
+            displayTask.restorePerm();
         });
     Task tDisplayTimer(SECONDS(10), TASK_ONCE + 1, 
         []{
@@ -60,15 +83,10 @@ namespace Display {
                 clockDisplay.print(timer, DEC);
                 clockDisplay.writeDisplay();
             }
-        }, &ts, false,
-        []{
-            if (!tDisplayTime.isEnabled()) return false;
-            tDisplayTime.disable();
-            return true;
-        },
+        }, &ts, false, nullptr,
         []{
             tDisplayTimer.setIterations(TASK_ONCE + 1);
-            tDisplayTime.enable();
+            if (!displayTask.isPerm(tDisplayTimer)) displayTask.restorePerm();
         });
     Task tTimer(MINUTES(1), TASK_FOREVER, 
         []{
@@ -76,10 +94,10 @@ namespace Display {
             timer.decrease();
             if (timer.atBeginning()) {
                 if (timer <= threshold) tDisplayTimer.setIterations(TASK_FOREVER);
-                tDisplayTimer.restart();
+                displayTask.activate(tDisplayTimer);
             } else if (timer == threshold) {
                 tDisplayTimer.setIterations(TASK_FOREVER);
-                tDisplayTimer.restart();
+                displayTask.activate(tDisplayTimer);
             }
         }, &ts, false, 
         []{
@@ -87,7 +105,7 @@ namespace Display {
             return true;
         },
         []{
-            tDisplayTimer.disable();
+            displayTask.activate(tDisplayTime);
         });
 
     struct HourFormat : public CallbackAware<callback_t> {
@@ -98,7 +116,7 @@ namespace Display {
 
         void operator=(bool value) {
             format24 = value;
-            tDisplayTime.restart();
+            if (displayTask.isPerm(tDisplayTime)) drawTime();
             if (callback) callback();
         }
     private:
@@ -225,6 +243,6 @@ namespace Display {
     void setup() {
         clockDisplay.begin(DISPLAY_ADDRESS);
         clockDisplay.setBrightness(brightness);
-        tDisplayTime.enable();
+        displayTask.activate(tDisplayTime);
     }
 }
diff --git a/src/esp_clock.cpp b/src/esp_clock.cpp
index 10ca0e9..3d800a5 100644
--- a/src/esp_clock.cpp
+++ b/src/esp_clock.cpp
@@ -31,7 +31,7 @@ Task tButton(TASK_IMMEDIATE, TASK_ONCE,
         if (Display::tDisplaySensor.isEnabled()) return;
         Bme::data.readAll();
         Display::tDisplaySensor.setIterations(DISPLAY_SENSOR_ITERATIONS);
-        Display::tDisplaySensor.restart();
+        Display::displayTask.activate(Display::tDisplaySensor);
     }, &ts);
 Task tLed(TASK_IMMEDIATE, TASK_ONCE,
     []() {
@@ -46,7 +46,7 @@ Task tReadBme(TASK_MINUTE, TASK_FOREVER, []() {
     Devices::publishBme280();
     if (abs(lastTemp - Bme::data.temp) > 0.2) {
         lastTemp = Bme::data.temp;
-        Display::tDisplaySensor.restart();
+        Display::displayTask.activate(Display::tDisplaySensor);
     }
     }, &ts);
 

From ca31851afe698b172c6c267ebd99ea08d9a796d6 Mon Sep 17 00:00:00 2001
From: Nicu Hodos <nicu@hodos.ro>
Date: Wed, 12 Feb 2025 12:05:30 +0100
Subject: [PATCH 3/4] simplify reading/displaying sensor values

---
 include/devices.h | 1 -
 include/display.h | 2 +-
 src/esp_clock.cpp | 5 +++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/devices.h b/include/devices.h
index b4dd440..21ee6c1 100644
--- a/include/devices.h
+++ b/include/devices.h
@@ -79,7 +79,6 @@ namespace Devices {
                 [](const char* msg) {
                     if (strcmp("PRESS", msg) == 0 && !Display::tDisplaySensor.isEnabled()) {
                         Bme::data.readAll();
-                        Display::tDisplaySensor.setIterations(DISPLAY_SENSOR_ITERATIONS);
                         Display::displayTask.activate(Display::tDisplaySensor);
                     };
                 }
diff --git a/include/display.h b/include/display.h
index 8457455..c85ccc0 100644
--- a/include/display.h
+++ b/include/display.h
@@ -70,7 +70,7 @@ namespace Display {
             return true;
         },
         []() {
-            tDisplaySensor.setIterations((DISPLAY_SENSOR_ITERATIONS - 1) * 2 + 1);
+            tDisplaySensor.setIterations(DISPLAY_SENSOR_ITERATIONS);
             displayTask.restorePerm();
         });
     Task tDisplayDate(SECONDS(5), TASK_ONCE + 1, displayDate, &ts, false, nullptr,
diff --git a/src/esp_clock.cpp b/src/esp_clock.cpp
index 3d800a5..81e27ca 100644
--- a/src/esp_clock.cpp
+++ b/src/esp_clock.cpp
@@ -30,7 +30,6 @@ Task tButton(TASK_IMMEDIATE, TASK_ONCE,
     []() {
         if (Display::tDisplaySensor.isEnabled()) return;
         Bme::data.readAll();
-        Display::tDisplaySensor.setIterations(DISPLAY_SENSOR_ITERATIONS);
         Display::displayTask.activate(Display::tDisplaySensor);
     }, &ts);
 Task tLed(TASK_IMMEDIATE, TASK_ONCE,
@@ -46,6 +45,7 @@ Task tReadBme(TASK_MINUTE, TASK_FOREVER, []() {
     Devices::publishBme280();
     if (abs(lastTemp - Bme::data.temp) > 0.2) {
         lastTemp = Bme::data.temp;
+        Display::tDisplaySensor.setIterations((DISPLAY_SENSOR_ITERATIONS - 1) * 2 + 1);
         Display::displayTask.activate(Display::tDisplaySensor);
     }
     }, &ts);
@@ -89,7 +89,8 @@ void setup() {
     attachInterrupt(digitalPinToInterrupt(BUTTON), onButtonPressed, FALLING);
     attachInterrupt(digitalPinToInterrupt(LED_BUILTIN), onLed, CHANGE);
 
-    tReadBme.enable();
+    tReadBme.enableDelayed();
+    tButton.restart();
 }
 
 void loop() {

From 598638239ff75ba71e1662f488f4c13ad94def25 Mon Sep 17 00:00:00 2001
From: Nicu Hodos <nicu@hodos.ro>
Date: Wed, 12 Feb 2025 12:05:32 +0100
Subject: [PATCH 4/4] move logic for deciding enabling a task inside the
 controller

---
 include/devices.h | 6 +++---
 include/display.h | 8 ++++----
 src/esp_clock.cpp | 4 +---
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/include/devices.h b/include/devices.h
index 21ee6c1..ae027c9 100644
--- a/include/devices.h
+++ b/include/devices.h
@@ -77,7 +77,7 @@ namespace Devices {
         .addSecondary(
             Builder<Button>::instance(new Button{"Display sensor data", "display_sensor_data",
                 [](const char* msg) {
-                    if (strcmp("PRESS", msg) == 0 && !Display::tDisplaySensor.isEnabled()) {
+                    if (strcmp("PRESS", msg) == 0) {
                         Bme::data.readAll();
                         Display::displayTask.activate(Display::tDisplaySensor);
                     };
@@ -87,7 +87,7 @@ namespace Devices {
         .addSecondary(
             Builder<Button>::instance(new Button{"Display date", "display_date",
                 [](const char* msg) {
-                    if (strcmp("PRESS", msg) == 0 && !Display::tDisplayDate.isEnabled()) {
+                    if (strcmp("PRESS", msg) == 0) {
                         Display::displayTask.activate(Display::tDisplayDate);
                     };
                 }
@@ -96,7 +96,7 @@ namespace Devices {
         .addSecondary(
             Builder<Button>::instance(new Button{"Display remaining timer", "display_remaining_timer",
                 [](const char* msg) {
-                    if (strcmp("PRESS", msg) == 0 && !Display::tDisplayTimer.isEnabled()) {
+                    if (strcmp("PRESS", msg) == 0) {
                         Display::displayTask.activate(Display::tDisplayTimer);
                     }
                 }
diff --git a/include/display.h b/include/display.h
index c85ccc0..e0adbd5 100644
--- a/include/display.h
+++ b/include/display.h
@@ -25,17 +25,17 @@ namespace Display {
             return *perm;
         }
 
-        Task& activate(Task& task) {
+        bool activate(Task& task) {
+            if (task.isEnabled()) return false;
             if (temp) temp->cancel();
             if (perm) perm->cancel();
             if (task.getIterations() == TASK_FOREVER) {
-                task.enable();
                 perm = &task;
+                return task.enable();
             } else {
-                task.restart();
                 temp = &task;
+                return task.restart();
             }
-            return task;
         }
 
         void restorePerm() {
diff --git a/src/esp_clock.cpp b/src/esp_clock.cpp
index 81e27ca..58ac444 100644
--- a/src/esp_clock.cpp
+++ b/src/esp_clock.cpp
@@ -28,9 +28,7 @@ void turnLed(bool on = true) {
 
 Task tButton(TASK_IMMEDIATE, TASK_ONCE,
     []() {
-        if (Display::tDisplaySensor.isEnabled()) return;
-        Bme::data.readAll();
-        Display::displayTask.activate(Display::tDisplaySensor);
+        if (Display::displayTask.activate(Display::tDisplaySensor)) Bme::data.readAll();
     }, &ts);
 Task tLed(TASK_IMMEDIATE, TASK_ONCE,
     []() {