Rework button ISR management

Signed-off-by: Martin Mihálek <[email protected]>

skarsta/lib/buttons/Buttons.cpp

@@ -1,86 +1,130 @@
 #include "Buttons.h"
-#include <PinChangeInt.h>
 #include <Service.h>
 
+#define NO_PIN_NUMBER
+#define DISABLE_PCINT_MULTI_SERVICE
+
+#include <PinChangeInt.h>
+
 #define BUTTON_ISR(i) []() { if (buttons[i]) { buttons[i]->isr(); } }
 static Button *buttons[MAX_BUTTONS] = {nullptr};
 static const PCIntvoidFuncPtr buttonsIsr[MAX_BUTTONS] = {
         BUTTON_ISR(0), BUTTON_ISR(1), BUTTON_ISR(2), BUTTON_ISR(3), BUTTON_ISR(4),
         BUTTON_ISR(5), BUTTON_ISR(6), BUTTON_ISR(7), BUTTON_ISR(8), BUTTON_ISR(9)};
 
-static void registerIsr(uint8_t button, Button *buttonI) {
-    PCIntvoidFuncPtr isr = nullptr;
+static uint8_t registerIsr(uint8_t button, Button *buttonI) {
     for (uint8_t i = 0; i < MAX_BUTTONS; i++) {
         if (buttons[i] == nullptr) {
             buttons[i] = buttonI;
-            isr = buttonsIsr[i];
-
+            pinMode(button, INPUT_PULLUP);
+            PCintPort::attachInterrupt(button, buttonsIsr[i], CHANGE);
 #ifdef __DEBUG__
             Serial.print("register isr: ");
             Serial.print(i);
             Serial.println();
 #endif
-            break;
+            return i;
         }
     }
-    pinMode(button, INPUT_PULLUP);
-    PCintPort::attachInterrupt(button, isr, CHANGE);
+    return MAX_BUTTONS;
 }
 
-ToggleButton::ToggleButton(uint8_t button, void (*on)(), void (*off)()) {
+Button::Button(uint8_t button) {
     this->button = button;
+    this->isrIndex = registerIsr(button, this);
+}
+
+void Button::isr() {
+    bool state = PCintPort::pinState == 0;
+    _isr(state);
+    this->state = state;
+}
+
+Button::~Button() {
+    if (this->isrIndex == MAX_BUTTONS) {
+        return;
+    }
+
+    PCintPort::detachInterrupt(button);
+    buttons[this->isrIndex] = nullptr;
+}
+
+ToggleButton::ToggleButton(uint8_t button, void (*on)(), void (*off)()) : Button(button) {
     this->on = on;
     this->off = off;
-    registerIsr(button, this);
 }
 
-TimedButton::TimedButton(uint8_t button, unsigned int delay, void (*short_press)(), void (*long_press)(),
-                         void (*on)()) {
-    this->button = button;
+TimedButton::TimedButton(uint8_t button, unsigned int delay, void (*short_press)(),
+                         void (*long_press)(), void (*on)()) : Button(button) {
     this->delay = delay;
     this->long_press = long_press;
     this->short_press = short_press;
     this->on = on;
-    registerIsr(button, this);
 }
 
-void ToggleButton::isr() {
-    bool state = digitalRead(button) == LOW;
-
+void ToggleButton::_isr(bool state) {
     if (this->state == state) return;
     if (state) {
+#ifdef __DEBUG__
+        Serial.print("b: on: ");
+        Serial.print(button);
+        Serial.println();
+#endif
         this->on();
     } else {
+#ifdef __DEBUG__
+        Serial.print("b: off: ");
+        Serial.print(button);
+        Serial.println();
+#endif
         this->off();
     }
-    this->state = state;
 }
 
 bool TimedButton::get_state() {
-    return button_state;
+    return state;
 }
 
 bool TimedButton::is_short() {
     return get_period(msg_time, millis()) < delay;
 }
 
-void TimedButton::isr() {
-    button_state = digitalRead(button) == LOW;
-    if (button_state) {
+bool TimedButton::debounce() {
+    return get_period(msg_time, millis()) < DEBOUNCE;
+}
+
+void TimedButton::_isr(bool state) {
+    if (state) {
         if (this->on) this->on();
         msg_time = millis();
         this->state = true;
         return;
     } else if (!this->state) {
-        button_state = false;
         return;
     }
 
-    if (is_short()) {
-        this->short_press();
+    if (!debounce()) {
+        if (is_short()) {
+#ifdef __DEBUG__
+            Serial.print("b: short: ");
+            Serial.print(button);
+            Serial.println();
+#endif
+            this->short_press();
+        } else {
+#ifdef __DEBUG__
+            Serial.print("b: long: ");
+            Serial.print(button);
+            Serial.println();
+#endif
+            this->long_press();
+        }
     } else {
-        this->long_press();
+#ifdef __DEBUG__
+        Serial.print("b: debouncing: ");
+        Serial.print(button);
+        Serial.println();
+#endif
     }
-    this->state = false;
     msg_time = millis();
-}
+}

skarsta/lib/buttons/Buttons.h

@@ -4,43 +4,52 @@
 #include <Arduino.h>
 
 #define MAX_BUTTONS 10
+#define DEBOUNCE 5
 
 class Button {
 protected:
     volatile bool state = false;
+    uint8_t button, isrIndex = MAX_BUTTONS;
+
+    virtual void _isr(bool state) = 0;
+
 public:
-    virtual void isr() = 0;
+    explicit Button(uint8_t button);
+
+    virtual void isr();
 
-    virtual ~Button() = default;
+    virtual ~Button();
 };
 
 class ToggleButton : Button {
 private:
-    uint8_t button;
-
     void (*on)();
 
     void (*off)();
 
+protected:
+    void _isr(bool state) override;
+
 public:
     ToggleButton(uint8_t button, void (*on)(), void (*off)());
-
-    void isr() override;
 };
 
 class TimedButton : Button {
 private:
     volatile unsigned long msg_time = 0;
-    volatile bool button_state = false;
     unsigned int delay;
-    uint8_t button;
 
     void (*on)();
 
     void (*short_press)();
 
     void (*long_press)();
 
+protected:
+    void _isr(bool state) override;
+
+    bool debounce();
+
 public:
     TimedButton(uint8_t button, unsigned int delay, void (*short_press)(), void (*long_press)())
             : TimedButton(button, delay, short_press, long_press, nullptr) {}
@@ -50,8 +59,6 @@ class TimedButton : Button {
     bool get_state();
 
     bool is_short();
-
-    void isr() override;
 };
 
 #endif //ARDUINO_PROJECTS_ROOT_BUTTONS_H