From cbd7a7d8c3050c623354e68a2d8d2616b8a1554b Mon Sep 17 00:00:00 2001 From: Todd Herbert Date: Wed, 10 Apr 2024 15:22:58 +1200 Subject: [PATCH 1/8] Make ButtonThread instance extern Previously was a static local instance in setup(). Now declared in ButtonThread.cpp, accessible via extern declaration in ButtonThread. --- src/ButtonThread.cpp | 1 + src/ButtonThread.h | 2 ++ src/main.cpp | 3 --- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ButtonThread.cpp b/src/ButtonThread.cpp index 069a92308c..0d30741953 100644 --- a/src/ButtonThread.cpp +++ b/src/ButtonThread.cpp @@ -23,6 +23,7 @@ using namespace concurrency; +ButtonThread *buttonThread; // Declared extern in header volatile ButtonThread::ButtonEventType ButtonThread::btnEvent = ButtonThread::BUTTON_EVENT_NONE; ButtonThread::ButtonThread() : OSThread("Button") diff --git a/src/ButtonThread.h b/src/ButtonThread.h index 3f177302d3..0f0b54add9 100644 --- a/src/ButtonThread.h +++ b/src/ButtonThread.h @@ -54,3 +54,5 @@ class ButtonThread : public concurrency::OSThread static void userButtonPressedLongStop(); static void touchPressedLongStart() { btnEvent = BUTTON_EVENT_TOUCH_LONG_PRESSED; } }; + +extern ButtonThread *buttonThread; diff --git a/src/main.cpp b/src/main.cpp index 47f3e2c22d..0f2ef7e67f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -188,9 +188,6 @@ uint32_t timeLastPowered = 0; static Periodic *ledPeriodic; static OSThread *powerFSMthread; -#if HAS_BUTTON || defined(ARCH_PORTDUINO) -static OSThread *buttonThread; -#endif static OSThread *accelerometerThread; static OSThread *ambientLightingThread; SPISettings spiSettings(4000000, MSBFIRST, SPI_MODE0); From 79d60fa4020d3a2b53eed2a1a236c5f574b84c89 Mon Sep 17 00:00:00 2001 From: Todd Herbert Date: Wed, 10 Apr 2024 15:40:15 +1200 Subject: [PATCH 2/8] Extract attachInterrupt() calls to public method; create matching method for detachInterrupt() --- src/ButtonThread.cpp | 77 +++++++++++++++++++++++++++++++++----------- src/ButtonThread.h | 4 ++- 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/ButtonThread.cpp b/src/ButtonThread.cpp index 0d30741953..66dfc5d3cb 100644 --- a/src/ButtonThread.cpp +++ b/src/ButtonThread.cpp @@ -24,14 +24,14 @@ using namespace concurrency; ButtonThread *buttonThread; // Declared extern in header +OneButton ButtonThread::userButton; // Get reference to static member volatile ButtonThread::ButtonEventType ButtonThread::btnEvent = ButtonThread::BUTTON_EVENT_NONE; ButtonThread::ButtonThread() : OSThread("Button") { -#if defined(ARCH_PORTDUINO) || defined(BUTTON_PIN) #if defined(ARCH_PORTDUINO) if (settingsMap.count(user) != 0 && settingsMap[user] != RADIOLIB_NC) { - userButton = OneButton(settingsMap[user], true, true); + this->userButton = OneButton(settingsMap[user], true, true); LOG_DEBUG("Using GPIO%02d for button\n", settingsMap[user]); } #elif defined(BUTTON_PIN) @@ -54,21 +54,7 @@ ButtonThread::ButtonThread() : OSThread("Button") userButton.attachLongPressStart(userButtonPressedLongStart); userButton.attachLongPressStop(userButtonPressedLongStop); #endif -#if defined(ARCH_PORTDUINO) - if (settingsMap.count(user) != 0 && settingsMap[user] != RADIOLIB_NC) - wakeOnIrq(settingsMap[user], FALLING); -#else - static OneButton *pBtn = &userButton; // only one instance of ButtonThread is created, so static is safe - attachInterrupt( - pin, - []() { - BaseType_t higherWake = 0; - mainDelay.interruptFromISR(&higherWake); - pBtn->tick(); - }, - CHANGE); -#endif -#endif + #ifdef BUTTON_PIN_ALT userButtonAlt = OneButton(BUTTON_PIN_ALT, true, true); #ifdef INPUT_PULLUP_SENSE @@ -82,15 +68,15 @@ ButtonThread::ButtonThread() : OSThread("Button") userButtonAlt.attachDoubleClick(userButtonDoublePressed); userButtonAlt.attachLongPressStart(userButtonPressedLongStart); userButtonAlt.attachLongPressStop(userButtonPressedLongStop); - wakeOnIrq(BUTTON_PIN_ALT, FALLING); #endif #ifdef BUTTON_PIN_TOUCH userButtonTouch = OneButton(BUTTON_PIN_TOUCH, true, true); userButtonTouch.setPressMs(400); userButtonTouch.attachLongPressStart(touchPressedLongStart); // Better handling with longpress than click? - wakeOnIrq(BUTTON_PIN_TOUCH, FALLING); #endif + + attachButtonInterrupts(); } int32_t ButtonThread::runOnce() @@ -221,6 +207,59 @@ int32_t ButtonThread::runOnce() return 50; } +/* + * Attach (or re-attach) hardware interrupts for buttons + * Public method. Used outside class when waking from MCU sleep + */ +void ButtonThread::attachButtonInterrupts() +{ +#if defined(ARCH_PORTDUINO) + if (settingsMap.count(user) != 0 && settingsMap[user] != RADIOLIB_NC) + wakeOnIrq(settingsMap[user], FALLING); +#elif defined(BUTTON_PIN) + // Interrupt for user button, during normal use. Improves responsiveness. + attachInterrupt( + config.device.button_gpio ? config.device.button_gpio : BUTTON_PIN, + []() { + LOG_DEBUG("Interrupt: user button\n"); + BaseType_t higherWake = 0; + mainDelay.interruptFromISR(&higherWake); + ButtonThread::userButton.tick(); + }, + CHANGE); +#endif + +#ifdef BUTTON_PIN_ALT + wakeOnIrq(BUTTON_PIN_ALT, FALLING); +#endif + +#ifdef BUTTON_PIN_TOUCH + wakeOnIrq(BUTTON_PIN_TOUCH, FALLING); +#endif +} + +/* + * Detach the "normal" button interrupts. + * Public method. Used before attaching a "wake-on-button" interrupt for MCU sleep + */ +void ButtonThread::detachButtonInterrupts() +{ +#if defined(ARCH_PORTDUINO) + if (settingsMap.count(user) != 0 && settingsMap[user] != RADIOLIB_NC) + detachInterrupt(settingsMap[user]); +#elif defined(BUTTON_PIN) + detachInterrupt(config.device.button_gpio ? config.device.button_gpio : BUTTON_PIN); +#endif + +#ifdef BUTTON_PIN_ALT + detachInterrupt(BUTTON_PIN_ALT); +#endif + +#ifdef BUTTON_PIN_TOUCH + detachInterrupt(BUTTON_PIN_TOUCH); +#endif +} + /** * Watch a GPIO and if we get an IRQ, wake the main thread. * Use to add wake on button press diff --git a/src/ButtonThread.h b/src/ButtonThread.h index 0f0b54add9..3c2f49daed 100644 --- a/src/ButtonThread.h +++ b/src/ButtonThread.h @@ -22,11 +22,13 @@ class ButtonThread : public concurrency::OSThread ButtonThread(); int32_t runOnce() override; + void attachButtonInterrupts(); + void detachButtonInterrupts(); void storeClickCount(); private: #ifdef BUTTON_PIN - OneButton userButton; + static OneButton userButton; // Static - accessed from an interrupt #endif #ifdef BUTTON_PIN_ALT OneButton userButtonAlt; From ddfe9ad25409b3a92efb157a3a7b2964e1d03ce7 Mon Sep 17 00:00:00 2001 From: Todd Herbert Date: Wed, 10 Apr 2024 15:47:09 +1200 Subject: [PATCH 3/8] Change suspension of button interrupts for light-sleep --- src/sleep.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/sleep.cpp b/src/sleep.cpp index 0d36112c1b..5fbb733e6e 100644 --- a/src/sleep.cpp +++ b/src/sleep.cpp @@ -4,6 +4,7 @@ #include "GPS.h" #endif +#include "ButtonThread.h" #include "MeshRadio.h" #include "MeshService.h" #include "NodeDB.h" @@ -337,7 +338,10 @@ esp_sleep_wakeup_cause_t doLightSleep(uint64_t sleepMsec) // FIXME, use a more r #ifdef BUTTON_PIN // The enableLoraInterrupt() method is using ext0_wakeup, so we are forced to use GPIO wakeup gpio_num_t pin = (gpio_num_t)(config.device.button_gpio ? config.device.button_gpio : BUTTON_PIN); - gpio_intr_disable(pin); + + // Have to *fully* detach the normal button-interrupts first + buttonThread->detachButtonInterrupts(); + gpio_wakeup_enable(pin, GPIO_INTR_LOW_LEVEL); esp_sleep_enable_gpio_wakeup(); #endif @@ -364,9 +368,9 @@ esp_sleep_wakeup_cause_t doLightSleep(uint64_t sleepMsec) // FIXME, use a more r assert(res == ESP_OK); #ifdef BUTTON_PIN + // Disable wake-on-button interrupt. Re-attach normal button-interrupts gpio_wakeup_disable(pin); - // Would have thought that need gpio_intr_enable() here, but nope.. - // Works fine without it; crashes with it. + buttonThread->attachButtonInterrupts(); #endif esp_sleep_wakeup_cause_t cause = esp_sleep_get_wakeup_cause(); From a3065defe7d31bedb33f92f675d26e2c706e138b Mon Sep 17 00:00:00 2001 From: Todd Herbert Date: Wed, 10 Apr 2024 19:58:28 +1200 Subject: [PATCH 4/8] Fix declaration for ARCH_PORTDUINO --- src/ButtonThread.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ButtonThread.h b/src/ButtonThread.h index 3c2f49daed..07c7ccff7e 100644 --- a/src/ButtonThread.h +++ b/src/ButtonThread.h @@ -27,7 +27,7 @@ class ButtonThread : public concurrency::OSThread void storeClickCount(); private: -#ifdef BUTTON_PIN +#if defined(BUTTON_PIN) || defined(ARCH_PORTDUINO) static OneButton userButton; // Static - accessed from an interrupt #endif #ifdef BUTTON_PIN_ALT @@ -36,9 +36,6 @@ class ButtonThread : public concurrency::OSThread #ifdef BUTTON_PIN_TOUCH OneButton userButtonTouch; #endif -#if defined(ARCH_PORTDUINO) - OneButton userButton; -#endif // set during IRQ static volatile ButtonEventType btnEvent; From af80a96fddea62278e366790e82b27451df3fb90 Mon Sep 17 00:00:00 2001 From: Todd Herbert Date: Wed, 10 Apr 2024 20:01:58 +1200 Subject: [PATCH 5/8] Remove LOG_DEBUG used during testing --- src/ButtonThread.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ButtonThread.cpp b/src/ButtonThread.cpp index 66dfc5d3cb..cbb16477ca 100644 --- a/src/ButtonThread.cpp +++ b/src/ButtonThread.cpp @@ -221,7 +221,6 @@ void ButtonThread::attachButtonInterrupts() attachInterrupt( config.device.button_gpio ? config.device.button_gpio : BUTTON_PIN, []() { - LOG_DEBUG("Interrupt: user button\n"); BaseType_t higherWake = 0; mainDelay.interruptFromISR(&higherWake); ButtonThread::userButton.tick(); From 89acfa80cca9273052a1564c18b03cb3f0256f2e Mon Sep 17 00:00:00 2001 From: Todd Herbert Date: Wed, 10 Apr 2024 20:26:28 +1200 Subject: [PATCH 6/8] Don't assume device has a button.. --- src/ButtonThread.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ButtonThread.cpp b/src/ButtonThread.cpp index cbb16477ca..1345b2625a 100644 --- a/src/ButtonThread.cpp +++ b/src/ButtonThread.cpp @@ -23,10 +23,13 @@ using namespace concurrency; -ButtonThread *buttonThread; // Declared extern in header -OneButton ButtonThread::userButton; // Get reference to static member +ButtonThread *buttonThread; // Declared extern in header volatile ButtonThread::ButtonEventType ButtonThread::btnEvent = ButtonThread::BUTTON_EVENT_NONE; +#if defined(BUTTON_PIN) || defined(ARCH_PORTDUINO) +OneButton ButtonThread::userButton; // Get reference to static member +#endif + ButtonThread::ButtonThread() : OSThread("Button") { #if defined(ARCH_PORTDUINO) @@ -44,6 +47,8 @@ ButtonThread::ButtonThread() : OSThread("Button") // Some platforms (nrf52) have a SENSE variant which allows wake from sleep - override what OneButton did pinMode(pin, INPUT_PULLUP_SENSE); #endif + +#if defined(BUTTON_PIN) || defined(ARCH_PORTDUINO) userButton.attachClick(userButtonPressed); userButton.setClickMs(250); userButton.setPressMs(c_longPressTime); @@ -54,6 +59,7 @@ ButtonThread::ButtonThread() : OSThread("Button") userButton.attachLongPressStart(userButtonPressedLongStart); userButton.attachLongPressStop(userButtonPressedLongStop); #endif +#endif #ifdef BUTTON_PIN_ALT userButtonAlt = OneButton(BUTTON_PIN_ALT, true, true); From 14119f66775186f564945c8d401c1f81d153ad67 Mon Sep 17 00:00:00 2001 From: Todd Herbert Date: Wed, 10 Apr 2024 20:49:04 +1200 Subject: [PATCH 7/8] Guard entire constructor code --- src/ButtonThread.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ButtonThread.cpp b/src/ButtonThread.cpp index 1345b2625a..d1e451f0e8 100644 --- a/src/ButtonThread.cpp +++ b/src/ButtonThread.cpp @@ -32,13 +32,15 @@ OneButton ButtonThread::userButton; // Get reference to static member ButtonThread::ButtonThread() : OSThread("Button") { +#if defined(BUTTON_PIN) || defined(ARCH_PORTDUINO) + int pin = config.device.button_gpio ? config.device.button_gpio : BUTTON_PIN; // Resolved button pin + #if defined(ARCH_PORTDUINO) if (settingsMap.count(user) != 0 && settingsMap[user] != RADIOLIB_NC) { this->userButton = OneButton(settingsMap[user], true, true); LOG_DEBUG("Using GPIO%02d for button\n", settingsMap[user]); } #elif defined(BUTTON_PIN) - int pin = config.device.button_gpio ? config.device.button_gpio : BUTTON_PIN; this->userButton = OneButton(pin, true, true); LOG_DEBUG("Using GPIO%02d for button\n", pin); #endif @@ -83,6 +85,7 @@ ButtonThread::ButtonThread() : OSThread("Button") #endif attachButtonInterrupts(); +#endif } int32_t ButtonThread::runOnce() From 5758374463557fa3ab11533b5e51daa876aad58c Mon Sep 17 00:00:00 2001 From: Todd Herbert Date: Wed, 10 Apr 2024 20:55:40 +1200 Subject: [PATCH 8/8] Don't use BUTTON_PIN with ARCH_PORTDUINO --- src/ButtonThread.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ButtonThread.cpp b/src/ButtonThread.cpp index d1e451f0e8..206bb72395 100644 --- a/src/ButtonThread.cpp +++ b/src/ButtonThread.cpp @@ -33,7 +33,6 @@ OneButton ButtonThread::userButton; // Get reference to static member ButtonThread::ButtonThread() : OSThread("Button") { #if defined(BUTTON_PIN) || defined(ARCH_PORTDUINO) - int pin = config.device.button_gpio ? config.device.button_gpio : BUTTON_PIN; // Resolved button pin #if defined(ARCH_PORTDUINO) if (settingsMap.count(user) != 0 && settingsMap[user] != RADIOLIB_NC) { @@ -41,6 +40,7 @@ ButtonThread::ButtonThread() : OSThread("Button") LOG_DEBUG("Using GPIO%02d for button\n", settingsMap[user]); } #elif defined(BUTTON_PIN) + int pin = config.device.button_gpio ? config.device.button_gpio : BUTTON_PIN; // Resolved button pin this->userButton = OneButton(pin, true, true); LOG_DEBUG("Using GPIO%02d for button\n", pin); #endif