From 7b1a2939f21be890ea858d1cad7d14c5882d0721 Mon Sep 17 00:00:00 2001 From: Kemal Hadimli Date: Sun, 31 May 2026 23:08:06 +0100 Subject: [PATCH 1/2] =?UTF-8?q?RAK3401:=20reliable=20AIN1=20button=20?= =?UTF-8?q?=E2=80=94=20interrupt=20latch,=20debounce,=20digital=20read,=20?= =?UTF-8?q?instant=20screen-on?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RAK3401 companion build drives its only user button (AIN1) by polling analogRead() once per main-loop iteration, with no interrupt. Presses are dropped whenever the loop briefly stalls (LoRa RX/TX, BLE, flash writes), and a single click waits out the full multi-click window before the screen wakes. MomentaryButton (generic): - Configurable multi-click window on the analog constructor (default unchanged). - reset(): abandon the in-flight gesture (used to swallow a screen-wake press). - enableInterrupt(): attach a permanent FALLING-edge GPIOTE interrupt that latches presses so they survive loop stalls (and can wake the MCU where the platform sleeps on interrupts). Debounced by counting one press per press and only re-arming after check() has seen a clean release — rejects press/release contact bounce. Skipped for analog/SAADC-threshold buttons (GPIOTE and analogRead conflict on one pin). companion_radio UITask: wake the display instantly on the press edge when it is off (instead of after the multi-click window), and arm the button interrupt at startup for PIN_USER_BTN_ANA boards. variants/rak3401: read the AIN1 button digitally (active-low, internal pull-up) instead of via SAADC. digitalRead coexists with the GPIOTE edge interrupt where analogRead does not (analogRead reads stuck-low once the interrupt is attached). Multi-click retained: single=NEXT / double=PREV / triple=SELECT / long=ENTER. Co-Authored-By: Claude Opus 4.8 --- examples/companion_radio/ui-new/UITask.cpp | 29 +++++--- src/helpers/ui/MomentaryButton.cpp | 79 ++++++++++++++++++++-- src/helpers/ui/MomentaryButton.h | 18 ++++- variants/rak3401/target.cpp | 7 +- 4 files changed, 118 insertions(+), 15 deletions(-) diff --git a/examples/companion_radio/ui-new/UITask.cpp b/examples/companion_radio/ui-new/UITask.cpp index 6f363d7f96..4fcabddc54 100644 --- a/examples/companion_radio/ui-new/UITask.cpp +++ b/examples/companion_radio/ui-new/UITask.cpp @@ -564,6 +564,9 @@ void UITask::begin(DisplayDriver* display, SensorManager* sensors, NodePrefs* no #endif #if defined(PIN_USER_BTN_ANA) analog_btn.begin(); + // Arm a permanent edge interrupt so presses are never lost when the main loop stalls + // (radio/BLE/flash) and the poll misses the edge. This also wakes the MCU from light sleep. + analog_btn.enableInterrupt(); #endif _node_prefs = node_prefs; @@ -750,15 +753,23 @@ void UITask::loop() { #endif #if defined(PIN_USER_BTN_ANA) if (abs(millis() - _analogue_pin_read_millis) > 10) { - int ev = analog_btn.check(); - if (ev == BUTTON_EVENT_CLICK) { - c = checkDisplayOn(KEY_NEXT); - } else if (ev == BUTTON_EVENT_LONG_PRESS) { - c = handleLongPress(KEY_ENTER); - } else if (ev == BUTTON_EVENT_DOUBLE_CLICK) { - c = handleDoubleClick(KEY_PREV); - } else if (ev == BUTTON_EVENT_TRIPLE_CLICK) { - c = handleTripleClick(KEY_SELECT); + if (_display != NULL && !_display->isOn() && analog_btn.isPressed()) { + // Instant screen-wake: light the display on the press edge instead of waiting out + // the ~280ms multi-click window, then abandon the gesture so this press only wakes + // the screen and doesn't also navigate. + checkDisplayOn(0); + analog_btn.reset(); + } else { + int ev = analog_btn.check(); + if (ev == BUTTON_EVENT_CLICK) { + c = checkDisplayOn(KEY_NEXT); + } else if (ev == BUTTON_EVENT_LONG_PRESS) { + c = handleLongPress(KEY_ENTER); + } else if (ev == BUTTON_EVENT_DOUBLE_CLICK) { + c = handleDoubleClick(KEY_PREV); + } else if (ev == BUTTON_EVENT_TRIPLE_CLICK) { + c = handleTripleClick(KEY_SELECT); + } } _analogue_pin_read_millis = millis(); } diff --git a/src/helpers/ui/MomentaryButton.cpp b/src/helpers/ui/MomentaryButton.cpp index 9d01e5b011..390f7dbbf7 100644 --- a/src/helpers/ui/MomentaryButton.cpp +++ b/src/helpers/ui/MomentaryButton.cpp @@ -1,8 +1,33 @@ #include "MomentaryButton.h" -#define MULTI_CLICK_WINDOW_MS 280 +#define IRQ_DEBOUNCE_MS 30 // ignore edges closer than this (contact bounce) -MomentaryButton::MomentaryButton(int8_t pin, int long_press_millis, bool reverse, bool pulldownup, bool multiclick) { +MomentaryButton* MomentaryButton::_irq_self = nullptr; + +void MomentaryButton::_onIrq() { + MomentaryButton* self = _irq_self; + if (!self) return; + // Count at most one press until check() re-arms us after a clean release. This rejects the + // press- and release-bounce falling edges that would otherwise latch a phantom second press. + if (!self->_irq_armed) return; + unsigned long now = millis(); + if (now - self->_irq_last_ms < IRQ_DEBOUNCE_MS) return; // also coalesce very fast chatter + self->_irq_last_ms = now; + self->_irq_armed = false; + if (self->_irq_events < 200) self->_irq_events++; // latch the press for check() to consume +} + +void MomentaryButton::enableInterrupt() { + if (_pin < 0) return; + // Edge interrupts need the digital input buffer; on an analog-read (SAADC threshold) pin the + // GPIOTE channel and analogRead() conflict and the pin reads stuck-low. Digital buttons only. + if (_threshold > 0) return; + _irq_self = this; // single button per board; last caller wins + pinMode(_pin, INPUT_PULLUP); // active-low button + attachInterrupt(digitalPinToInterrupt(_pin), _onIrq, FALLING); +} + +MomentaryButton::MomentaryButton(int8_t pin, int long_press_millis, bool reverse, bool pulldownup, bool multiclick) { _pin = pin; _reverse = reverse; _pull = pulldownup; @@ -15,9 +40,14 @@ MomentaryButton::MomentaryButton(int8_t pin, int long_press_millis, bool reverse _last_click_time = 0; _multi_click_window = multiclick ? MULTI_CLICK_WINDOW_MS : 0; _pending_click = false; + _irq_events = 0; + _irq_last_ms = 0; + _irq_armed = true; // first press counts; ISR disarms until a clean release re-arms + _irq_held = false; + _irq_release_ms = 0; } -MomentaryButton::MomentaryButton(int8_t pin, int long_press_millis, int analog_threshold) { +MomentaryButton::MomentaryButton(int8_t pin, int long_press_millis, int analog_threshold, int multi_click_window) { _pin = pin; _reverse = false; _pull = false; @@ -28,8 +58,13 @@ MomentaryButton::MomentaryButton(int8_t pin, int long_press_millis, int analog_t _threshold = analog_threshold; _click_count = 0; _last_click_time = 0; - _multi_click_window = MULTI_CLICK_WINDOW_MS; + _multi_click_window = multi_click_window; // 0 = single click acts immediately (no double/triple-click) _pending_click = false; + _irq_events = 0; + _irq_last_ms = 0; + _irq_armed = true; // first press counts; ISR disarms until a clean release re-arms + _irq_held = false; + _irq_release_ms = 0; } void MomentaryButton::begin() { @@ -51,6 +86,23 @@ void MomentaryButton::cancelClick() { _pending_click = false; } +void MomentaryButton::reset() { + // Resync 'prev' to the current physical level and drop all gesture state. With the + // button still held, prev becomes the pressed level and down_at stays 0, so the eventual + // release records no click (check() guards the click on down_at > 0) — the gesture is + // fully abandoned, including the click that would otherwise fire a multi-click window + // after release. Used to swallow the press that only wakes the display. + prev = _threshold > 0 ? (analogRead(_pin) < _threshold) : digitalRead(_pin); + down_at = 0; + cancel = 0; + _click_count = 0; + _last_click_time = 0; + _pending_click = false; + _irq_events = 0; // drop latched presses too: the wake-press shouldn't queue navigation + _irq_armed = true; + _irq_held = false; +} + bool MomentaryButton::isPressed(int level) const { if (_threshold > 0) { return level; @@ -67,9 +119,20 @@ int MomentaryButton::check(bool repeat_click) { int event = BUTTON_EVENT_NONE; int btn = _threshold > 0 ? (analogRead(_pin) < _threshold) : digitalRead(_pin); + + // Re-arm the IRQ latch only once the button has been cleanly released for the debounce + // period. This is the debounce: the ISR can't count a new press (bounce or real) until here. + if (isPressed(btn)) { + _irq_held = true; + } else { + if (_irq_held) { _irq_held = false; _irq_release_ms = millis(); } + if (!_irq_armed && (millis() - _irq_release_ms) >= IRQ_DEBOUNCE_MS) _irq_armed = true; + } + if (btn != prev) { if (isPressed(btn)) { down_at = millis(); + if (_irq_events > 0) _irq_events--; // this live press was already latched by the ISR; claim it } else { // button UP if (_long_millis > 0) { @@ -141,5 +204,13 @@ int MomentaryButton::check(bool repeat_click) { _pending_click = false; } + // Flush a press the live poll never saw (loop stalled through its entire down→up window): + // the ISR latched it but no edge was detected here. Only when idle (released, nothing + // pending) so we never interfere with an in-progress long-press or multi-click. + if (event == BUTTON_EVENT_NONE && _irq_events > 0 && !isPressed(btn) && down_at == 0 && !_pending_click) { + _irq_events--; + event = BUTTON_EVENT_CLICK; + } + return event; } \ No newline at end of file diff --git a/src/helpers/ui/MomentaryButton.h b/src/helpers/ui/MomentaryButton.h index 358a343b0f..1bc491ea8e 100644 --- a/src/helpers/ui/MomentaryButton.h +++ b/src/helpers/ui/MomentaryButton.h @@ -2,6 +2,8 @@ #include +#define MULTI_CLICK_WINDOW_MS 280 // delay before a single click acts, used to detect double/triple-click + #define BUTTON_EVENT_NONE 0 #define BUTTON_EVENT_CLICK 1 #define BUTTON_EVENT_LONG_PRESS 2 @@ -20,14 +22,28 @@ class MomentaryButton { int _multi_click_window; bool _pending_click; + // Interrupt-latch: presses caught by the edge ISR even when the main loop stalls (radio/ + // BLE/flash) and the poll misses them. check() reconciles these with live polling. + // Debounce: the ISR counts at most one press, and only re-arms after check() has seen the + // button cleanly released for the debounce period — so press/release bounce can't double-count. + volatile uint8_t _irq_events; + volatile bool _irq_armed; + unsigned long _irq_last_ms; + bool _irq_held; + unsigned long _irq_release_ms; + static MomentaryButton* _irq_self; + static void _onIrq(); + bool isPressed(int level) const; public: MomentaryButton(int8_t pin, int long_press_mills=0, bool reverse=false, bool pulldownup=false, bool multiclick=true); - MomentaryButton(int8_t pin, int long_press_mills, int analog_threshold); + MomentaryButton(int8_t pin, int long_press_mills, int analog_threshold, int multi_click_window = MULTI_CLICK_WINDOW_MS); void begin(); + void enableInterrupt(); // attach a permanent edge IRQ so presses are never lost to a stalled loop int check(bool repeat_click=false); // returns one of BUTTON_EVENT_* void cancelClick(); // suppress next BUTTON_EVENT_CLICK (if already in DOWN state) + void reset(); // abandon the in-flight gesture: resync to current level, drop any pending click uint8_t getPin() { return _pin; } bool isPressed() const; }; diff --git a/variants/rak3401/target.cpp b/variants/rak3401/target.cpp index 5309e6b22b..3be9757854 100644 --- a/variants/rak3401/target.cpp +++ b/variants/rak3401/target.cpp @@ -13,7 +13,12 @@ RAK3401Board board; MomentaryButton user_btn(PIN_USER_BTN, 1000, true, true); #if defined(PIN_USER_BTN_ANA) - MomentaryButton analog_btn(PIN_USER_BTN_ANA, 1000, 20); + // Read the button digitally (not via SAADC/analogRead): it's a clean active-low input + // (internal pull-up + button to GND), and digitalRead coexists with the latch/wake edge + // interrupt (enableInterrupt) where analogRead does not (GPIOTE vs SAADC conflict). + // reverse=true (active-low), pulldownup=true (INPUT_PULLUP), multiclick=true enables the + // MULTI_CLICK_WINDOW_MS (280ms) window: single=NEXT / double=PREV / triple=SELECT / long=ENTER. + MomentaryButton analog_btn(PIN_USER_BTN_ANA, 1000, true, true, true); #endif #endif From 60938ee59087f8d358f461230b5dfd07fa1e31ac Mon Sep 17 00:00:00 2001 From: Kemal Hadimli Date: Mon, 1 Jun 2026 10:43:02 +0100 Subject: [PATCH 2/2] RAK3401: AIN button SYSTEMOFF wake via LPCOMP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brings #2642's RAK3401 board pieces into this branch so the AIN1 button story is self-contained, using LPCOMP (not GPIO SENSE) as the hibernate wake source. AIN1 (P0.31) is wired as an analog button (pressed == analogRead() < threshold). GPIO SENSE can't wake on it: the digital input buffer reads the *released* idle level as LOW even though analogRead reports ~VDD, so arming SENSE_Low latches DETECT the instant we enter SYSTEMOFF — sd_power_system_off() returns immediately and the fall-through reset reboots ("instant wake"). LATCH/EVENTS_PORT clearing and release debounce don't help, because the released level itself reads LOW digitally. LPCOMP operates in the analog domain and sees the idle level correctly. Arm a DOWN crossing at ~1/2 VDD: released idles above threshold, a press pulls the pin below -> downward crossing -> wake. Wait for a confirmed release (analogRead above threshold) before arming, bounded by a 5s timeout so a stuck/low reading can't wedge shutdown. - NRF52Board::configureVoltageWake() gains a detect_down param (default false = UP crossing for voltage recovery; true = DOWN crossing for a button press), selecting ANADETECT Down/Up and INTENSET DOWN/UP accordingly. - RAK3401Board.h: powerOff() override routing through initiateShutdown(USER). - RAK3401Board::initiateShutdown() arms LPCOMP on AIN7 at REFSEL 3 (~1/2 VDD) for a USER shutdown when PIN_USER_BTN_ANA is defined. Channel/threshold overridable via PWRMGT_BTN_LPCOMP_AIN / PWRMGT_BTN_LPCOMP_REFSEL. Ports the fix from #2642. Built: RAK_3401_companion_radio_ble. Co-Authored-By: Claude Opus 4.8 --- src/helpers/NRF52Board.cpp | 10 +++++--- src/helpers/NRF52Board.h | 5 +++- variants/rak3401/RAK3401Board.cpp | 42 +++++++++++++++++++++++++++++++ variants/rak3401/RAK3401Board.h | 4 +++ 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/helpers/NRF52Board.cpp b/src/helpers/NRF52Board.cpp index 2c8753d464..66823b0eb2 100644 --- a/src/helpers/NRF52Board.cpp +++ b/src/helpers/NRF52Board.cpp @@ -177,7 +177,7 @@ void NRF52Board::enterSystemOff(uint8_t reason) { NVIC_SystemReset(); } -void NRF52Board::configureVoltageWake(uint8_t ain_channel, uint8_t refsel) { +void NRF52Board::configureVoltageWake(uint8_t ain_channel, uint8_t refsel, bool detect_down) { // LPCOMP is not managed by SoftDevice - direct register access required // Halt and disable before reconfiguration NRF_LPCOMP->TASKS_STOP = 1; @@ -189,8 +189,10 @@ void NRF52Board::configureVoltageWake(uint8_t ain_channel, uint8_t refsel) { // Reference: REFSEL (0-6=1/8..7/8, 7=ARef, 8-15=1/16..15/16) NRF_LPCOMP->REFSEL = ((uint32_t)refsel << LPCOMP_REFSEL_REFSEL_Pos) & LPCOMP_REFSEL_REFSEL_Msk; - // Detect UP events (voltage rises above threshold for battery recovery) - NRF_LPCOMP->ANADETECT = LPCOMP_ANADETECT_ANADETECT_Up; + // Crossing direction: UP for voltage recovery (rises above threshold), + // DOWN for an analog button press (pin pulled below threshold). + NRF_LPCOMP->ANADETECT = detect_down ? LPCOMP_ANADETECT_ANADETECT_Down + : LPCOMP_ANADETECT_ANADETECT_Up; // Enable 50mV hysteresis for noise immunity NRF_LPCOMP->HYST = LPCOMP_HYST_HYST_Hyst50mV; @@ -202,7 +204,7 @@ void NRF52Board::configureVoltageWake(uint8_t ain_channel, uint8_t refsel) { NRF_LPCOMP->EVENTS_CROSS = 0; NRF_LPCOMP->INTENCLR = 0xFFFFFFFF; - NRF_LPCOMP->INTENSET = LPCOMP_INTENSET_UP_Msk; + NRF_LPCOMP->INTENSET = detect_down ? LPCOMP_INTENSET_DOWN_Msk : LPCOMP_INTENSET_UP_Msk; // Enable LPCOMP NRF_LPCOMP->ENABLE = LPCOMP_ENABLE_ENABLE_Enabled; diff --git a/src/helpers/NRF52Board.h b/src/helpers/NRF52Board.h index c9f1e071b8..65fe897266 100644 --- a/src/helpers/NRF52Board.h +++ b/src/helpers/NRF52Board.h @@ -40,7 +40,10 @@ class NRF52Board : public mesh::MainBoard { bool checkBootVoltage(const PowerMgtConfig* config); void enterSystemOff(uint8_t reason); - void configureVoltageWake(uint8_t ain_channel, uint8_t refsel); + // Arm LPCOMP as a SYSTEMOFF wake source on the given analog channel. + // detect_down=false wakes on an upward crossing (voltage recovery); + // detect_down=true wakes on a downward crossing (e.g. an analog button press). + void configureVoltageWake(uint8_t ain_channel, uint8_t refsel, bool detect_down = false); virtual void initiateShutdown(uint8_t reason); #endif diff --git a/variants/rak3401/RAK3401Board.cpp b/variants/rak3401/RAK3401Board.cpp index cbf7c1087d..07e28fcbe5 100644 --- a/variants/rak3401/RAK3401Board.cpp +++ b/variants/rak3401/RAK3401Board.cpp @@ -4,6 +4,17 @@ #include "RAK3401Board.h" #ifdef NRF52_POWER_MANAGEMENT +#ifdef PIN_USER_BTN_ANA +// LPCOMP wake config for the AIN user button. Defaults assume PIN_USER_BTN_ANA +// is pin 31 (P0.31 = AIN7); override via build flags if the button moves. +#ifndef PWRMGT_BTN_LPCOMP_AIN + #define PWRMGT_BTN_LPCOMP_AIN 7 +#endif +#ifndef PWRMGT_BTN_LPCOMP_REFSEL + #define PWRMGT_BTN_LPCOMP_REFSEL 3 // 4/8 VDD (~1.5V) threshold +#endif +#endif + // Static configuration for power management // Values set in variant.h defines const PowerMgtConfig power_config = { @@ -24,6 +35,37 @@ void RAK3401Board::initiateShutdown(uint8_t reason) { configureVoltageWake(power_config.lpcomp_ain_channel, power_config.lpcomp_refsel); } +#ifdef PIN_USER_BTN_ANA + // Wake-from-SYSTEMOFF on the AIN user button (P0.31 = AIN7). + // + // This pin is wired as an *analog* button (see MomentaryButton in target.cpp: + // pressed == analogRead() < threshold). GPIO SENSE can't be used as the wake + // source: the digital input buffer reads this line as LOW even at the released + // idle level (verified on hardware — analogRead reports ~VDD while NRF_GPIO->IN + // reads 0 and SENSE_Low latches immediately), so a GPIO SENSE arm wakes the + // chip the instant we enter SYSTEMOFF and it can never stay off. + // + // LPCOMP works in the analog domain, so it sees the idle level correctly. Arm + // it for a DOWN crossing at ~1/2 VDD: released idles near VDD (above), a press + // pulls the pin toward 0V (below) -> downward crossing -> wake. The LPCOMP is + // otherwise unused for a USER shutdown (voltage wake is only armed for the + // low-voltage / boot-protect reasons handled above), so there is no conflict. + // + // Wait for release first so LPCOMP is armed while the level is above the + // threshold — otherwise the initial press generates no new downward crossing. + // Bounded by a timeout so a stuck/low reading can never wedge shutdown. + const int BTN_RELEASED_ADC = 1024; // well above the press threshold + uint32_t t0 = millis(); + int released_streak = 0; + while (released_streak < 5 && (millis() - t0) < 5000) { + if (analogRead(PIN_USER_BTN_ANA) > BTN_RELEASED_ADC) released_streak++; + else released_streak = 0; + delay(10); + } + + configureVoltageWake(PWRMGT_BTN_LPCOMP_AIN, PWRMGT_BTN_LPCOMP_REFSEL, /*detect_down=*/true); +#endif + enterSystemOff(reason); } #endif diff --git a/variants/rak3401/RAK3401Board.h b/variants/rak3401/RAK3401Board.h index 3a080d5e2c..6b1d398735 100644 --- a/variants/rak3401/RAK3401Board.h +++ b/variants/rak3401/RAK3401Board.h @@ -20,6 +20,10 @@ class RAK3401Board : public NRF52BoardDCDC { RAK3401Board() : NRF52Board("RAK3401_OTA") {} void begin(); +#ifdef NRF52_POWER_MANAGEMENT + void powerOff() override { initiateShutdown(SHUTDOWN_REASON_USER); } +#endif + #define BATTERY_SAMPLES 8 uint16_t getBattMilliVolts() override {