From 31f05ab0572408c605b36e6108c373eab20cdca6 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Mon, 14 Sep 2026 09:34:31 +0000 Subject: [PATCH] fix(touch): stop LONG_PRESS repeating when the suppression deadline wraps (#11829) * fix(touch): stop LONG_PRESS repeating when the suppression deadline wraps TouchScreenBase::_start was one field doing two incompatible jobs. It held the press-down timestamp, and then the LONG_PRESS handler overwrote it with `millis() + 30000` to stop the event repeating for the rest of the hold. Every read was a hand-rolled signed subtraction on time_t, and suppression worked only because `time_t(millis()) - _start` came out around -30000. Where time_t is 64 bits - the portduino host - that uint32_t sum wraps to a small number while millis() is still just under 0xFFFFFFFF. The subtraction then goes hugely positive instead of negative, the threshold test passes on every 20ms poll, and each pass re-arms to another wrapped value. It keeps firing until millis() itself wraps, up to ~30 s later: about 1500 TOUCH_ACTION_LONG_PRESS events injected into InputBroker for one finger that never moved. Modelling the old expression across press-start offsets puts the worst case at exactly 1500 for a 60 s hold, where three is correct. On a 32-bit time_t build the signed wrap happens to keep suppressing, so this is host-and-variant dependent rather than universal. The zero-dodging helpers in src/UptimeClock.h are no use here: they map 0 to 1, and 1 reads as "long ago" exactly as 0 does. The defect is the overload, not the zero, so the field is split by what it is actually asked: _pressStartMs a past event time - how long has the finger been down _longPressSuppressed is repeat suppression armed _longPressSuppressUntilMs when it expires, read only while the bool is set Two fields for the suppression rather than one, for the reason Throttle.h's TODO(deadline-type) gives: armed has to stay a separate question from passed. No single value can stand in for "unarmed" here either, since deadlinePassed() reads 0 as long past below ~24.8 days of uptime and as far future above it. Nothing new uses 0 as a sentinel, so bin/lint-unset-sentinel-millis.sh needs no entry. All three comparisons now go through Throttle - hasElapsed() for the two elapsed-since-press questions, which also buys the full ~49.7 day range that a stored event time gets, and deadlinePassed() for the suppression window. Behaviour is preserved deliberately, including the part that is easy to miss: the old `+ 30000` made a held finger re-report LONG_PRESS once every 30 s, not once per touch. A bool latch would have been simpler and quietly narrowed that, so the window is kept as LONG_PRESS_REPEAT_SUPPRESS_MS. Old and new were compared across five wrap scenarios and agree everywhere except the wrap window the old code got wrong. The tap-on-release suppression the old write also provided is not needed: a hold long enough to reach here has duration >= TIME_LONG_PRESS, so the tap branch already takes its else and clears _tapped. One guard added while here. The RAK14014 deferred-tap window is TIME_LONG_PRESS - 50 and that subtraction is unsigned now, so a variant lowering TIME_LONG_PRESS below 50 would underflow it into a ~49.7 day wait and the deferred TAP would never fire. The only override in the tree is t5s3_epaper at 500; a static_assert fails the build instead of the touch panel. Co-Authored-By: Claude Opus 5 * style(touch): trim comments to the house limit --------- Co-authored-by: Claude Opus 5 Co-authored-by: nomdetom --- src/input/TouchScreenBase.cpp | 27 ++++++++++++++++++++------- src/input/TouchScreenBase.h | 13 +++++++++---- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/input/TouchScreenBase.cpp b/src/input/TouchScreenBase.cpp index 8512300f71..0770e7aec8 100644 --- a/src/input/TouchScreenBase.cpp +++ b/src/input/TouchScreenBase.cpp @@ -1,5 +1,6 @@ #include "TouchScreenBase.h" #include "main.h" +#include "mesh/Throttle.h" #if defined(RAK14014) && !defined(MESHTASTIC_EXCLUDE_CANNEDMESSAGES) #include "modules/CannedMessageModule.h" @@ -9,6 +10,12 @@ #define TIME_LONG_PRESS 400 #endif +// The deferred-tap window is `TIME_LONG_PRESS - 50`, unsigned: below 50 it underflows to ~49.7 days. +static_assert(TIME_LONG_PRESS >= 50, "TIME_LONG_PRESS must be at least 50ms: see the deferred-tap window below"); + +// How long a held finger stays suppressed after a LONG_PRESS is reported. +#define LONG_PRESS_REPEAT_SUPPRESS_MS 30000 + // Touch sampling cadence (milliseconds). // Can be overridden by board variants for faster touch panels. #ifndef TOUCH_POLL_INTERVAL_IDLE @@ -49,7 +56,8 @@ TouchScreenBase::TouchScreenBase(const char *name, uint16_t width, uint16_t height) : concurrency::OSThread(name), _display_width(width), _display_height(height), _first_x(0), _last_x(0), _first_y(0), - _last_y(0), _start(0), _lastTouchSeenMs(0), _tapped(false), _originName(name) + _last_y(0), _pressStartMs(0), _longPressSuppressed(false), _longPressSuppressUntilMs(0), _lastTouchSeenMs(0), + _tapped(false), _originName(name) { } @@ -95,12 +103,13 @@ int32_t TouchScreenBase::runOnce() if (touched) { hapticFeedback(); _state = TOUCH_EVENT_OCCURRED; - _start = millis(); + _pressStartMs = nowMs; + _longPressSuppressed = false; _first_x = x; _first_y = y; } else { _state = TOUCH_EVENT_CLEARED; - time_t duration = millis() - _start; + uint32_t duration = nowMs - _pressStartMs; x = _last_x; y = _last_y; this->setInterval(fastTapMode ? TOUCH_POLL_INTERVAL_RELEASE_FAST : TOUCH_POLL_INTERVAL_RELEASE); @@ -157,7 +166,7 @@ int32_t TouchScreenBase::runOnce() LOG_DEBUG("action TAP(%d/%d)", _last_x, _last_y); } } else { - if (_tapped && (time_t(millis()) - _start) > TIME_LONG_PRESS - 50) { + if (_tapped && Throttle::hasElapsed(_pressStartMs, TIME_LONG_PRESS - 50)) { _tapped = false; e.touchEvent = static_cast(TOUCH_ACTION_TAP); LOG_DEBUG("action TAP(%d/%d)", _last_x, _last_y); @@ -173,9 +182,13 @@ int32_t TouchScreenBase::runOnce() #endif // fire LONG_PRESS event without the need for release - if (allowLongPress && touched && (time_t(millis()) - _start) > TIME_LONG_PRESS) { - // tricky: prevent reoccurring events and another touch event when releasing - _start = millis() + 30000; + // Armed and expired are asked separately; folding the deadline into the press stamp repeated + // LONG_PRESS every poll across the wrap on 64-bit time_t hosts. + const bool longPressSuppressed = _longPressSuppressed && !Throttle::deadlinePassed(_longPressSuppressUntilMs); + if (allowLongPress && touched && !longPressSuppressed && Throttle::hasElapsed(_pressStartMs, TIME_LONG_PRESS)) { + // A finger held past the window re-reports LONG_PRESS once per window, as before. + _longPressSuppressed = true; + _longPressSuppressUntilMs = nowMs + LONG_PRESS_REPEAT_SUPPRESS_MS; e.touchEvent = static_cast(TOUCH_ACTION_LONG_PRESS); LOG_DEBUG("action LONG PRESS(%d/%d)", _last_x, _last_y); } diff --git a/src/input/TouchScreenBase.h b/src/input/TouchScreenBase.h index 91ec165ec7..67667232a2 100644 --- a/src/input/TouchScreenBase.h +++ b/src/input/TouchScreenBase.h @@ -50,10 +50,15 @@ class TouchScreenBase : public Observable, public concurrenc bool _touchedOld = false; // previous touch state int16_t _first_x, _last_x; // horizontal swipe direction int16_t _first_y, _last_y; // vertical swipe direction - time_t _start; // for LONG_PRESS - uint32_t _lastTouchSeenMs; // helps suppress brief touch-controller dropouts - bool _tapped; // for DOUBLE_TAP - uint32_t _lastRun = 0; // helps suppress too fast consecutive runOnce() executions + uint32_t _pressStartMs; // when the current touch began; read via Throttle::hasElapsed() + + // LONG_PRESS repeat suppression while one touch is held: the bool is the armed flag, the + // deadline is read only while it is set. No value of the deadline can mean "unarmed". + bool _longPressSuppressed; + uint32_t _longPressSuppressUntilMs; // meaningful only while _longPressSuppressed + uint32_t _lastTouchSeenMs; // helps suppress brief touch-controller dropouts + bool _tapped; // for DOUBLE_TAP + uint32_t _lastRun = 0; // helps suppress too fast consecutive runOnce() executions const char *_originName; };