From 7a38368494b7daa45a5326fd4bd74b46e142f8e8 Mon Sep 17 00:00:00 2001 From: whywilson Date: Mon, 16 Jun 2025 23:18:45 +0800 Subject: [PATCH 1/3] Optimize key event processing and add debounce logic. --- src/input/UpDownInterruptBase.cpp | 24 ++++++++++++++++-------- src/input/UpDownInterruptBase.h | 6 ++++++ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/input/UpDownInterruptBase.cpp b/src/input/UpDownInterruptBase.cpp index 9a95323fe..44eae5dbb 100644 --- a/src/input/UpDownInterruptBase.cpp +++ b/src/input/UpDownInterruptBase.cpp @@ -33,16 +33,25 @@ int32_t UpDownInterruptBase::runOnce() { InputEvent e; e.inputEvent = INPUT_BROKER_NONE; - + unsigned long now = millis(); if (this->action == UPDOWN_ACTION_PRESSED) { - LOG_DEBUG("GPIO event Press"); - e.inputEvent = this->_eventPressed; + if (now - lastPressKeyTime >= PRESS_DEBOUNCE_MS) { + lastPressKeyTime = now; + LOG_DEBUG("GPIO event Press"); + e.inputEvent = this->_eventPressed; + } } else if (this->action == UPDOWN_ACTION_UP) { - LOG_DEBUG("GPIO event Up"); - e.inputEvent = this->_eventUp; + if (now - lastUpKeyTime >= UPDOWN_DEBOUNCE_MS) { + lastUpKeyTime = now; + LOG_DEBUG("GPIO event Up"); + e.inputEvent = this->_eventUp; + } } else if (this->action == UPDOWN_ACTION_DOWN) { - LOG_DEBUG("GPIO event Down"); - e.inputEvent = this->_eventDown; + if (now - lastDownKeyTime >= UPDOWN_DEBOUNCE_MS) { + lastDownKeyTime = now; + LOG_DEBUG("GPIO event Down"); + e.inputEvent = this->_eventDown; + } } if (e.inputEvent != INPUT_BROKER_NONE) { @@ -52,7 +61,6 @@ int32_t UpDownInterruptBase::runOnce() } this->action = UPDOWN_ACTION_NONE; - return 100; } diff --git a/src/input/UpDownInterruptBase.h b/src/input/UpDownInterruptBase.h index 4e9f591b9..57e42a76a 100644 --- a/src/input/UpDownInterruptBase.h +++ b/src/input/UpDownInterruptBase.h @@ -27,4 +27,10 @@ class UpDownInterruptBase : public Observable, public concur input_broker_event _eventUp = INPUT_BROKER_NONE; input_broker_event _eventPressed = INPUT_BROKER_NONE; const char *_originName; + + unsigned long lastUpKeyTime = 0; + unsigned long lastDownKeyTime = 0; + unsigned long lastPressKeyTime = 0; + const unsigned long UPDOWN_DEBOUNCE_MS = 300; + const unsigned long PRESS_DEBOUNCE_MS = 500; }; From 8ba98ae8733556af45c741835b1a0f6284e9736a Mon Sep 17 00:00:00 2001 From: whywilson Date: Tue, 17 Jun 2025 06:05:45 +0800 Subject: [PATCH 2/3] Add a debounce time parameter and use it in the runOnce method to debounce the key. --- src/input/UpDownInterruptBase.cpp | 8 ++++---- src/input/UpDownInterruptBase.h | 6 +++--- src/input/UpDownInterruptImpl1.cpp | 3 ++- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/input/UpDownInterruptBase.cpp b/src/input/UpDownInterruptBase.cpp index 44eae5dbb..c66eb13d0 100644 --- a/src/input/UpDownInterruptBase.cpp +++ b/src/input/UpDownInterruptBase.cpp @@ -8,7 +8,7 @@ UpDownInterruptBase::UpDownInterruptBase(const char *name) : concurrency::OSThre void UpDownInterruptBase::init(uint8_t pinDown, uint8_t pinUp, uint8_t pinPress, input_broker_event eventDown, input_broker_event eventUp, input_broker_event eventPressed, void (*onIntDown)(), - void (*onIntUp)(), void (*onIntPress)()) + void (*onIntUp)(), void (*onIntPress)(), unsigned long updownDebounceMs) { this->_pinDown = pinDown; this->_pinUp = pinUp; @@ -35,19 +35,19 @@ int32_t UpDownInterruptBase::runOnce() e.inputEvent = INPUT_BROKER_NONE; unsigned long now = millis(); if (this->action == UPDOWN_ACTION_PRESSED) { - if (now - lastPressKeyTime >= PRESS_DEBOUNCE_MS) { + if (now - lastPressKeyTime >= pressDebounceMs) { lastPressKeyTime = now; LOG_DEBUG("GPIO event Press"); e.inputEvent = this->_eventPressed; } } else if (this->action == UPDOWN_ACTION_UP) { - if (now - lastUpKeyTime >= UPDOWN_DEBOUNCE_MS) { + if (now - lastUpKeyTime >= updownDebounceMs) { lastUpKeyTime = now; LOG_DEBUG("GPIO event Up"); e.inputEvent = this->_eventUp; } } else if (this->action == UPDOWN_ACTION_DOWN) { - if (now - lastDownKeyTime >= UPDOWN_DEBOUNCE_MS) { + if (now - lastDownKeyTime >= updownDebounceMs) { lastDownKeyTime = now; LOG_DEBUG("GPIO event Down"); e.inputEvent = this->_eventDown; diff --git a/src/input/UpDownInterruptBase.h b/src/input/UpDownInterruptBase.h index 57e42a76a..d4a39a0e4 100644 --- a/src/input/UpDownInterruptBase.h +++ b/src/input/UpDownInterruptBase.h @@ -8,7 +8,7 @@ class UpDownInterruptBase : public Observable, public concur public: explicit UpDownInterruptBase(const char *name); void init(uint8_t pinDown, uint8_t pinUp, uint8_t pinPress, input_broker_event eventDown, input_broker_event eventUp, - input_broker_event eventPressed, void (*onIntDown)(), void (*onIntUp)(), void (*onIntPress)()); + input_broker_event eventPressed, void (*onIntDown)(), void (*onIntUp)(), void (*onIntPress)(), unsigned long updownDebounceMs = 300); void intPressHandler(); void intDownHandler(); void intUpHandler(); @@ -31,6 +31,6 @@ class UpDownInterruptBase : public Observable, public concur unsigned long lastUpKeyTime = 0; unsigned long lastDownKeyTime = 0; unsigned long lastPressKeyTime = 0; - const unsigned long UPDOWN_DEBOUNCE_MS = 300; - const unsigned long PRESS_DEBOUNCE_MS = 500; + unsigned long updownDebounceMs = 300; + const unsigned long pressDebounceMs = 500; }; diff --git a/src/input/UpDownInterruptImpl1.cpp b/src/input/UpDownInterruptImpl1.cpp index 761b92348..847724ec7 100644 --- a/src/input/UpDownInterruptImpl1.cpp +++ b/src/input/UpDownInterruptImpl1.cpp @@ -21,8 +21,9 @@ bool UpDownInterruptImpl1::init() input_broker_event eventUp = INPUT_BROKER_UP; input_broker_event eventPressed = INPUT_BROKER_SELECT; + unsigned long debounceMs = moduleConfig.canned_message.rotary1_enabled ? 100 : 300; UpDownInterruptBase::init(pinDown, pinUp, pinPress, eventDown, eventUp, eventPressed, UpDownInterruptImpl1::handleIntDown, - UpDownInterruptImpl1::handleIntUp, UpDownInterruptImpl1::handleIntPressed); + UpDownInterruptImpl1::handleIntUp, UpDownInterruptImpl1::handleIntPressed, debounceMs); inputBroker->registerSource(this); return true; } From e1df4e19e5c9c1cd5670123a58b8d292af6ee237 Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Sat, 21 Jun 2025 20:47:11 -0500 Subject: [PATCH 3/3] Default to very short updownDebounce values --- src/input/UpDownInterruptBase.h | 9 +++++---- src/input/UpDownInterruptImpl1.cpp | 3 +-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/input/UpDownInterruptBase.h b/src/input/UpDownInterruptBase.h index d4a39a0e4..789ba2310 100644 --- a/src/input/UpDownInterruptBase.h +++ b/src/input/UpDownInterruptBase.h @@ -8,7 +8,8 @@ class UpDownInterruptBase : public Observable, public concur public: explicit UpDownInterruptBase(const char *name); void init(uint8_t pinDown, uint8_t pinUp, uint8_t pinPress, input_broker_event eventDown, input_broker_event eventUp, - input_broker_event eventPressed, void (*onIntDown)(), void (*onIntUp)(), void (*onIntPress)(), unsigned long updownDebounceMs = 300); + input_broker_event eventPressed, void (*onIntDown)(), void (*onIntUp)(), void (*onIntPress)(), + unsigned long updownDebounceMs = 50); void intPressHandler(); void intDownHandler(); void intUpHandler(); @@ -27,10 +28,10 @@ class UpDownInterruptBase : public Observable, public concur input_broker_event _eventUp = INPUT_BROKER_NONE; input_broker_event _eventPressed = INPUT_BROKER_NONE; const char *_originName; - + unsigned long lastUpKeyTime = 0; unsigned long lastDownKeyTime = 0; unsigned long lastPressKeyTime = 0; - unsigned long updownDebounceMs = 300; - const unsigned long pressDebounceMs = 500; + unsigned long updownDebounceMs; + const unsigned long pressDebounceMs = 200; }; diff --git a/src/input/UpDownInterruptImpl1.cpp b/src/input/UpDownInterruptImpl1.cpp index 847724ec7..761b92348 100644 --- a/src/input/UpDownInterruptImpl1.cpp +++ b/src/input/UpDownInterruptImpl1.cpp @@ -21,9 +21,8 @@ bool UpDownInterruptImpl1::init() input_broker_event eventUp = INPUT_BROKER_UP; input_broker_event eventPressed = INPUT_BROKER_SELECT; - unsigned long debounceMs = moduleConfig.canned_message.rotary1_enabled ? 100 : 300; UpDownInterruptBase::init(pinDown, pinUp, pinPress, eventDown, eventUp, eventPressed, UpDownInterruptImpl1::handleIntDown, - UpDownInterruptImpl1::handleIntUp, UpDownInterruptImpl1::handleIntPressed, debounceMs); + UpDownInterruptImpl1::handleIntUp, UpDownInterruptImpl1::handleIntPressed); inputBroker->registerSource(this); return true; }