From b1cab867ed9616563260f8cd96c995eb1082ef03 Mon Sep 17 00:00:00 2001 From: Holger Eiboeck Date: Mon, 10 Aug 2026 11:01:51 +0200 Subject: [PATCH 1/2] =?UTF-8?q?Place=20=20=20=20=20=E2=94=82=20=E2=94=82?= =?UTF-8?q?=20ESP8266=20ISR=20paths=20in=20IRAM?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/ClickEncoder.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/ClickEncoder.cpp b/src/ClickEncoder.cpp index 3684db3..e6660da 100644 --- a/src/ClickEncoder.cpp +++ b/src/ClickEncoder.cpp @@ -10,6 +10,12 @@ // ---------------------------------------------------------------------------- #include "ClickEncoder.h" +#if defined(ESP8266) +#define CLICK_ENCODER_ISR_ATTR IRAM_ATTR +#else +#define CLICK_ENCODER_ISR_ATTR +#endif + // ---------------------------------------------------------------------------- // Button configuration (values for 1ms timer service calls) @@ -112,7 +118,7 @@ AnalogButton::AnalogButton(int8_t BTN, int16_t rangeLow, int16_t rangeHigh) : Cl // ---------------------------------------------------------------------------- // call this every 1 millisecond via timer ISR // -void ClickEncoder::service(void) +void CLICK_ENCODER_ISR_ATTR ClickEncoder::service(void) { bool moved = false; @@ -299,7 +305,7 @@ ClickEncoder::Button ClickEncoder::getButton(void) return ret; } -bool ClickEncoder::getPinState() { +bool CLICK_ENCODER_ISR_ATTR ClickEncoder::getPinState() { bool pinState; if (analogInput) { int16_t pinValue = analogRead(pinBTN); @@ -311,3 +317,4 @@ bool ClickEncoder::getPinState() { } #endif +#undef CLICK_ENCODER_ISR_ATTR From 6a3dd27562212be7abca1b752f5bf566e316f116 Mon Sep 17 00:00:00 2001 From: Holger Eiboeck Date: Mon, 17 Aug 2026 17:18:03 +0200 Subject: [PATCH 2/2] Harden ESP8266 analog button ISR handling --- src/ClickEncoder.cpp | 51 +++++++++++++------- src/ClickEncoder.h | 1 + test/esp8266_analog_button_test.cpp | 74 +++++++++++++++++++++++++++++ test/support/Arduino.h | 21 ++++++++ 4 files changed, 129 insertions(+), 18 deletions(-) create mode 100644 test/esp8266_analog_button_test.cpp create mode 100644 test/support/Arduino.h diff --git a/src/ClickEncoder.cpp b/src/ClickEncoder.cpp index e6660da..c16e099 100644 --- a/src/ClickEncoder.cpp +++ b/src/ClickEncoder.cpp @@ -11,9 +11,15 @@ #include "ClickEncoder.h" #if defined(ESP8266) -#define CLICK_ENCODER_ISR_ATTR IRAM_ATTR +# if defined(IRAM_ATTR) +# define CLICK_ENCODER_ISR_ATTR IRAM_ATTR +# elif defined(ICACHE_RAM_ATTR) +# define CLICK_ENCODER_ISR_ATTR ICACHE_RAM_ATTR +# else +# error "ClickEncoder requires IRAM_ATTR or ICACHE_RAM_ATTR on ESP8266" +# endif #else -#define CLICK_ENCODER_ISR_ATTR +# define CLICK_ENCODER_ISR_ATTR #endif @@ -176,24 +182,29 @@ void CLICK_ENCODER_ISR_ATTR ClickEncoder::service(void) } } } - // handle button - // #ifndef WITHOUT_BUTTON +#if defined(ESP8266) + if (!analogInput) { + serviceButton(); + } +#else + serviceButton(); +#endif +#endif + +} + +#ifndef WITHOUT_BUTTON +void CLICK_ENCODER_ISR_ATTR ClickEncoder::serviceButton(void) +{ unsigned long currentMillis = millis(); unsigned long millisSinceLastCheck = currentMillis - lastButtonCheck; - if ((pinBTN > 0 || (pinBTN == 0 && buttonOnPinZeroEnabled)) // check button only, if a pin has been provided - && (millisSinceLastCheck >= ENC_BUTTONINTERVAL)) // checking button is sufficient every 10-30ms - { + if ((pinBTN > 0 || (pinBTN == 0 && buttonOnPinZeroEnabled)) + && (millisSinceLastCheck >= ENC_BUTTONINTERVAL)) + { lastButtonCheck = currentMillis; bool pinRead = getPinState(); - - - - - - - if (pinRead == !pinsActive) { // key is now up if (keyDownTicks > 1) { //Make sure key was down through 1 complete tick to prevent random transients from registering as click @@ -217,14 +228,14 @@ void CLICK_ENCODER_ISR_ATTR ClickEncoder::service(void) keyDownTicks = 0; } - + if (pinRead == pinsActive) { // key is down if ((keyDownTicks > (buttonHoldTime)) && (buttonHeldEnabled)) { button = Held; } keyDownTicks += millisSinceLastCheck; } - + if (doubleClickTicks > 0) { doubleClickTicks -= (uint16_t)constrain(min(millisSinceLastCheck, (unsigned long)doubleClickTicks), 0, 65536); if (doubleClickTicks == 0) { @@ -232,9 +243,8 @@ void CLICK_ENCODER_ISR_ATTR ClickEncoder::service(void) } } } -#endif // WITHOUT_BUTTON - } +#endif // ---------------------------------------------------------------------------- @@ -295,6 +305,11 @@ void ClickEncoder::resetEncoder(void) #ifndef WITHOUT_BUTTON ClickEncoder::Button ClickEncoder::getButton(void) { +#if defined(ESP8266) + if (analogInput) { + serviceButton(); + } +#endif noInterrupts(); ClickEncoder::Button ret = button; if (button != ClickEncoder::Held && ret != ClickEncoder::Open) { diff --git a/src/ClickEncoder.h b/src/ClickEncoder.h index 9bbe856..e82b9d9 100644 --- a/src/ClickEncoder.h +++ b/src/ClickEncoder.h @@ -173,6 +173,7 @@ class ClickEncoder unsigned long lastButtonCheck = 0; int16_t anlogActiveRangeLow = 0; int16_t anlogActiveRangeHigh = 0; + void serviceButton(void); bool getPinState(); #endif }; diff --git a/test/esp8266_analog_button_test.cpp b/test/esp8266_analog_button_test.cpp new file mode 100644 index 0000000..324bc33 --- /dev/null +++ b/test/esp8266_analog_button_test.cpp @@ -0,0 +1,74 @@ +#include + +#include "ClickEncoder.h" + +static unsigned long fakeMillis; +static int fakeAnalogValue; +static int fakeDigitalValue; +static unsigned int analogReadCalls; +static unsigned int digitalReadCalls; + +unsigned long millis(void) +{ + return fakeMillis; +} + +int analogRead(uint8_t) +{ + ++analogReadCalls; + return fakeAnalogValue; +} + +int digitalRead(uint8_t) +{ + ++digitalReadCalls; + return fakeDigitalValue; +} + +void pinMode(uint8_t, uint8_t) +{ +} + +void noInterrupts(void) +{ +} + +void interrupts(void) +{ +} + +int main(void) +{ + fakeAnalogValue = 150; + AnalogButton analogButton(1, 100, 200); + +#if defined(ESP8266) + fakeMillis = 10; + assert(analogButton.getButton() == ClickEncoder::Open); + assert(analogReadCalls == 1); + + fakeMillis = 20; + analogButton.service(); + assert(analogReadCalls == 1); + + fakeAnalogValue = 900; + assert(analogButton.getButton() == ClickEncoder::Open); + assert(analogReadCalls == 2); + + fakeMillis = 410; + assert(analogButton.getButton() == ClickEncoder::Clicked); + assert(analogReadCalls == 3); + + fakeDigitalValue = LOW; + DigitalButton digitalButton(1); + fakeMillis = 10; + digitalButton.service(); + assert(digitalReadCalls == 1); +#else + fakeMillis = 10; + analogButton.service(); + assert(analogReadCalls == 1); +#endif + + return 0; +} diff --git a/test/support/Arduino.h b/test/support/Arduino.h new file mode 100644 index 0000000..e32fc98 --- /dev/null +++ b/test/support/Arduino.h @@ -0,0 +1,21 @@ +#ifndef TEST_SUPPORT_ARDUINO_H +#define TEST_SUPPORT_ARDUINO_H + +#include + +#define LOW 0x0 +#define HIGH 0x1 +#define INPUT 0x0 +#define INPUT_PULLUP 0x2 + +unsigned long millis(void); +int analogRead(uint8_t pin); +int digitalRead(uint8_t pin); +void pinMode(uint8_t pin, uint8_t mode); +void noInterrupts(void); +void interrupts(void); + +#define min(a, b) ((a) < (b) ? (a) : (b)) +#define constrain(value, lower, upper) ((value) < (lower) ? (lower) : ((value) > (upper) ? (upper) : (value))) + +#endif