diff --git a/src/helpers/RadioActivityWindow.h b/src/helpers/RadioActivityWindow.h index 36032b75d..ddd8547cb 100644 --- a/src/helpers/RadioActivityWindow.h +++ b/src/helpers/RadioActivityWindow.h @@ -22,6 +22,10 @@ // as a stale or (after a 49-day rollover) nonsensical age. #define RADIO_ACTIVITY_MAX_AGE_MS (100UL * RADIO_ACTIVITY_BUCKET_MS) +// How far a timestamp may run behind the previous one and still count as an +// out-of-order reading rather than a very long forward gap. +#define RADIO_ACTIVITY_BACKSTEP_TOLERANCE_MS 5000UL + struct RadioActivitySnapshot { uint32_t packets; uint32_t wire_bytes; @@ -166,16 +170,22 @@ private: uint8_t _head; // ring index of the current minute bool _ever_received; - // Accumulates the delta since the previous call, which is correct across one - // millis() wrap. Read as signed so a caller handing back a slightly older - // reading counts as no time passing, rather than as a ~49-day leap forward - // that would expire the whole ring. Successive calls must therefore be less - // than 2^31 ms (~24.8 days) apart - guaranteed while the tracker is being - // serviced, and when it is not the ring is empty anyway. + // Accumulates the unsigned delta since the previous call, which is correct + // across one millis() wrap. + // + // A delta past the halfway mark is ambiguous: it is either a slightly stale + // reading or a genuine gap of more than ~24.8 days. recordPacket() and + // snapshot() sample millis() microseconds apart, so a real stale reading is + // tiny - anything larger is treated as the long gap it is, which matters + // because rejecting it outright would freeze the ring and leave a + // month-old packet looking recently received. void tick(uint32_t now_ms) { - int32_t delta = (int32_t)(now_ms - _last_input_ms); - if (delta <= 0) return; // stale or repeated reading: no time has passed - _now_ms += (uint32_t)delta; + uint32_t delta = now_ms - _last_input_ms; + if (delta > 0x80000000u && + (uint32_t)(_last_input_ms - now_ms) <= RADIO_ACTIVITY_BACKSTEP_TOLERANCE_MS) { + return; // out-of-order reading: no time has passed + } + _now_ms += delta; _last_input_ms = now_ms; } diff --git a/src/helpers/ui/CHSC6XTouch.h b/src/helpers/ui/CHSC6XTouch.h index fc8f119fc..a34e9100d 100644 --- a/src/helpers/ui/CHSC6XTouch.h +++ b/src/helpers/ui/CHSC6XTouch.h @@ -19,16 +19,12 @@ #define CHSC6X_READ_LEN 5 -// Bounds a single read. The shared bus can wedge, and the ESP32 default lets a -// failed transfer burn ~1 s inside the UI loop. -#ifndef CHSC6X_I2C_TIMEOUT_MS -#define CHSC6X_I2C_TIMEOUT_MS 10 -#endif - class CHSC6XTouch { public: - // Probes the bus. Returns false (and disables itself) when nothing answers, - // so a board without the touch panel simply carries on without it. + // Probes the bus for diagnostics only. The result must NOT gate polling: this + // controller NACKs its address whenever it has nothing to report, so a single + // idle probe at boot is indistinguishable from absent hardware. checkTap() + // keeps polling either way, and a NACK there costs one quiet, fast bus cycle. bool begin(TwoWire& wire = Wire) { _wire = &wire; _wire->beginTransmission((uint8_t)CHSC6X_I2C_ADDR); @@ -58,8 +54,12 @@ public: // True exactly once per new touch. bool checkTap(uint32_t now_ms) { - if (!_present) return false; - return _detector.update(now_ms, readPressed()); + bool pressed = readPressed(); + if (pressed && !_present) { + _present = true; // answered late; the boot probe caught it mid-idle + Serial.println("Touch: CHSC6X responding"); + } + return _detector.update(now_ms, pressed); } private: @@ -75,15 +75,12 @@ private: _wire->beginTransmission((uint8_t)CHSC6X_I2C_ADDR); if (_wire->endTransmission() != 0) return false; - const uint16_t prev_timeout = _wire->getTimeOut(); - _wire->setTimeOut(CHSC6X_I2C_TIMEOUT_MS); uint8_t got = _wire->requestFrom((uint8_t)CHSC6X_I2C_ADDR, (uint8_t)CHSC6X_READ_LEN); uint8_t buf[CHSC6X_READ_LEN]; for (uint8_t i = 0; i < CHSC6X_READ_LEN; i++) { buf[i] = i < got ? (uint8_t)_wire->read() : 0xFF; } while (_wire->available()) _wire->read(); // drain a short read - _wire->setTimeOut(prev_timeout); if (got != CHSC6X_READ_LEN) { logRaw(got, NULL); diff --git a/src/helpers/ui/TouchTapDetector.h b/src/helpers/ui/TouchTapDetector.h index b2a610f4c..b7de09766 100644 --- a/src/helpers/ui/TouchTapDetector.h +++ b/src/helpers/ui/TouchTapDetector.h @@ -9,8 +9,12 @@ // elapsed-time comparisons are unsigned subtractions, so millis() rollover is // a non-event. +// Must exceed the caller's poll interval, or a state change is confirmed by the +// very next sample and the debounce does nothing. At the 50 ms touch poll this +// requires two consecutive consistent reads, so one NACK or short read during a +// continuous touch cannot fake a release (and therefore a second tap). #ifndef TOUCH_TAP_DEBOUNCE_MS -#define TOUCH_TAP_DEBOUNCE_MS 40 +#define TOUCH_TAP_DEBOUNCE_MS 80 #endif // Ignores a second tap arriving this soon after an accepted one, so a bouncy diff --git a/test/test_radio_activity_window/test_radio_activity_window.cpp b/test/test_radio_activity_window/test_radio_activity_window.cpp index 65f953e4d..2628e81c3 100644 --- a/test/test_radio_activity_window/test_radio_activity_window.cpp +++ b/test/test_radio_activity_window/test_radio_activity_window.cpp @@ -355,6 +355,23 @@ TEST(RadioActivityWindow, AnOlderTimestampDoesNotExpireTheWindow) { EXPECT_EQ(10000u, s.window_ms); } +TEST(RadioActivityWindow, AGapLongerThanHalfTheMillisRangeStillExpiresTheRing) { + // Display off and no traffic for ~25 days: the elapsed time passes the signed + // halfway mark, which must not be mistaken for an out-of-order reading, or a + // month-old packet would still be sitting in the "last 20 minutes". + RadioActivityWindow w; + w.reset(0); + recordTypical(w, 1000); + ASSERT_EQ(1u, snapshotAt(w, 2000).packets); + + const uint32_t twenty_five_days = 25UL * 24 * 3600 * 1000; + ASSERT_GT(twenty_five_days, 0x80000000u) << "gap must cross the halfway mark"; + + RadioActivitySnapshot s = snapshotAt(w, twenty_five_days); + EXPECT_TRUE(s.isEmpty()); + EXPECT_FALSE(s.has_last_packet); +} + TEST(RadioActivityWindow, AveragesHandleNegativeSnrAndMixedSigns) { RadioActivityWindow w; w.reset(0); diff --git a/test/test_touch_tap_detector/test_touch_tap_detector.cpp b/test/test_touch_tap_detector/test_touch_tap_detector.cpp index 656b5b42d..538bdf666 100644 --- a/test/test_touch_tap_detector/test_touch_tap_detector.cpp +++ b/test/test_touch_tap_detector/test_touch_tap_detector.cpp @@ -43,7 +43,7 @@ TEST(TouchTapDetector, HoldingAFingerDownDoesNotRepeat) { uint32_t now = 1000; d.reset(now); - EXPECT_EQ(1, hold(d, now, true, 100)); + EXPECT_EQ(1, hold(d, now, true, 200)); EXPECT_EQ(0, hold(d, now, true, 10000)) << "a long press must not toggle repeatedly"; } @@ -79,9 +79,9 @@ TEST(TouchTapDetector, SecondTapTooSoonIsSuppressed) { uint32_t now = 1000; d.reset(now); - EXPECT_EQ(1, hold(d, now, true, 100)); - EXPECT_EQ(0, hold(d, now, false, 100)); - EXPECT_EQ(0, hold(d, now, true, 100)) << "inside TOUCH_TAP_MIN_GAP_MS"; + EXPECT_EQ(1, hold(d, now, true, 200)); + EXPECT_EQ(0, hold(d, now, false, 150)); // release long enough to be confirmed + EXPECT_EQ(0, hold(d, now, true, 200)) << "inside TOUCH_TAP_MIN_GAP_MS"; } TEST(TouchTapDetector, DeliberateSecondTapIsAccepted) { @@ -89,9 +89,25 @@ TEST(TouchTapDetector, DeliberateSecondTapIsAccepted) { uint32_t now = 1000; d.reset(now); - EXPECT_EQ(1, hold(d, now, true, 100)); + EXPECT_EQ(1, hold(d, now, true, 200)); EXPECT_EQ(0, hold(d, now, false, 600)); // past the min gap - EXPECT_EQ(1, hold(d, now, true, 100)); + EXPECT_EQ(1, hold(d, now, true, 200)); +} + +TEST(TouchTapDetector, ASingleBadSampleDuringATouchDoesNotFakeARelease) { + // A NACK or short read reports "not pressed" for that poll. One of those must + // not confirm a release, or recovery reads as a second tap and the display + // toggles twice on one touch. + TouchTapDetector d; + uint32_t now = 1000; + d.reset(now); + EXPECT_EQ(1, hold(d, now, true, 200)); + + EXPECT_FALSE(d.update(now, false)); + now += POLL; + EXPECT_TRUE(d.isTouched()) << "one bad sample must not confirm a release"; + + EXPECT_EQ(0, hold(d, now, true, 2000)) << "and must not produce a second tap"; } TEST(TouchTapDetector, SurvivesMillisRollover) { @@ -100,10 +116,10 @@ TEST(TouchTapDetector, SurvivesMillisRollover) { uint32_t now = start; d.reset(now); - EXPECT_EQ(1, hold(d, now, true, 200)); // the touch itself crosses the wrap + EXPECT_EQ(1, hold(d, now, true, 250)); // the touch itself crosses the wrap ASSERT_LT(now, start) << "test setup must actually wrap"; EXPECT_EQ(0, hold(d, now, false, 600)); - EXPECT_EQ(1, hold(d, now, true, 200)); + EXPECT_EQ(1, hold(d, now, true, 250)); } TEST(TouchTapDetector, ResetClearsPendingState) {