diff --git a/src/Button.cpp b/src/Button.cpp index 7f79e3f..8688b14 100644 --- a/src/Button.cpp +++ b/src/Button.cpp @@ -9,7 +9,7 @@ #include Button::Button(uint8_t pin, uint16_t debounce_ms) - : _pin(pin), _delay(debounce_ms), _state(HIGH), _ignore_until(0), _has_changed(false) + : _pin(pin), _delay(debounce_ms), _state(HIGH), _ignore_start(0), _has_changed(false) { } @@ -24,11 +24,11 @@ void Button::begin() bool Button::read() { - // ignore pin changes until after this delay time. The subtraction is done - // in modular (wraparound-safe) arithmetic so debouncing keeps working across - // the ~49-day millis() rollover: a negative signed difference means we have - // not yet reached _ignore_until. - if ((int32_t)((uint32_t)millis() - _ignore_until) < 0) + // ignore pin changes for _delay ms after the last accepted change. Elapsed + // time is measured with unsigned (modular) subtraction, so debouncing stays + // correct across the ~49-day millis() rollover and can never soft-lock: the + // ignore window is only ever _delay milliseconds wide. + if ((uint32_t)(millis() - _ignore_start) < _delay) { // ignore any changes during this period } @@ -36,7 +36,7 @@ bool Button::read() // pin has changed else if (digitalRead(_pin) != _state) { - _ignore_until = millis() + _delay; + _ignore_start = millis(); _state = !_state; _has_changed = true; } diff --git a/src/Button.h b/src/Button.h index 9f0fa09..a6b56c2 100644 --- a/src/Button.h +++ b/src/Button.h @@ -26,7 +26,7 @@ class Button uint8_t _pin; uint16_t _delay; bool _state; - uint32_t _ignore_until; + uint32_t _ignore_start; bool _has_changed; }; diff --git a/test/test_button/test_main.cpp b/test/test_button/test_main.cpp index 72ebb3e..38355ba 100644 --- a/test/test_button/test_main.cpp +++ b/test/test_button/test_main.cpp @@ -49,11 +49,26 @@ void test_starts_released(void) TEST_ASSERT_EQUAL(Button::RELEASED, button.read()); } +// Changes within the first _delay ms after construction are debounced away +// (power-on transients are ignored); the button becomes responsive afterwards. +void test_ignores_power_on_window(void) +{ + Button button(2, 100); + + _mock_millis = 50; // inside the initial debounce window + _mock_pin_state = LOW; // already held at boot + TEST_ASSERT_FALSE(button.pressed()); + + _mock_millis = 100; // window elapsed + TEST_ASSERT_TRUE(button.pressed()); +} + // pressed() fires exactly once per press. void test_pressed_fires_once(void) { Button button(2); + _mock_millis = 1000; // past the power-on debounce window _mock_pin_state = LOW; // button pushed down TEST_ASSERT_TRUE(button.pressed()); TEST_ASSERT_FALSE(button.pressed()); // no new edge @@ -64,10 +79,11 @@ void test_released_fires_once(void) { Button button(2); + _mock_millis = 1000; _mock_pin_state = LOW; - button.pressed(); // register the press (debounce window: 0..100) + button.pressed(); // register the press (ignore window: 1000..1100) - _mock_millis = 150; // past the debounce window + _mock_millis = 1150; // past the debounce window _mock_pin_state = HIGH; TEST_ASSERT_TRUE(button.released()); TEST_ASSERT_FALSE(button.released()); @@ -78,10 +94,11 @@ void test_debounce_ignores_bounce(void) { Button button(2); - _mock_pin_state = LOW; // press at t=0, ignore changes until t=100 + _mock_millis = 1000; // press at t=1000, ignore changes until t=1100 + _mock_pin_state = LOW; TEST_ASSERT_TRUE(button.pressed()); - _mock_millis = 50; // still inside the window + _mock_millis = 1050; // still inside the window _mock_pin_state = HIGH; // a contact bounce back up TEST_ASSERT_FALSE(button.released()); TEST_ASSERT_EQUAL(Button::PRESSED, button.read()); // state unchanged @@ -92,11 +109,12 @@ void test_toggled_on_each_change(void) { Button button(2); + _mock_millis = 1000; // past the power-on debounce window _mock_pin_state = LOW; // pressed TEST_ASSERT_TRUE(button.toggled()); TEST_ASSERT_FALSE(button.toggled()); - _mock_millis = 200; // past debounce window + _mock_millis = 1200; // past debounce window _mock_pin_state = HIGH; // released TEST_ASSERT_TRUE(button.toggled()); } @@ -106,14 +124,15 @@ void test_custom_debounce_time(void) { Button button(2, 500); + _mock_millis = 1000; _mock_pin_state = LOW; - TEST_ASSERT_TRUE(button.pressed()); // ignore changes until t=500 + TEST_ASSERT_TRUE(button.pressed()); // ignore changes until t=1500 - _mock_millis = 400; // a real release, but still inside the 500ms window + _mock_millis = 1400; // a real release, but still inside the 500ms window _mock_pin_state = HIGH; TEST_ASSERT_FALSE(button.released()); - _mock_millis = 500; // window elapsed + _mock_millis = 1500; // window elapsed TEST_ASSERT_TRUE(button.released()); } @@ -122,27 +141,52 @@ void test_debounce_survives_millis_wraparound(void) { Button button(2, 100); - // Prime with a normal press so _ignore_until tracks recent activity, the way - // it always does in real operation (never far behind millis()). - _mock_millis = 0x7FFFFFFF; + // Prime with a normal press just below the top of the range. + _mock_millis = 0xFFFFFF00; _mock_pin_state = LOW; TEST_ASSERT_TRUE(button.pressed()); - // Release at the very top of the range: the debounce deadline (millis + 100) - // overflows and wraps to a tiny value (~99). - _mock_millis = 0xFFFFFFFF; + // Release just before the rollover. The elapsed time since the press is + // computed with modular subtraction, so it is correct across the wrap. + _mock_millis = 0xFFFFFFF0; _mock_pin_state = HIGH; TEST_ASSERT_TRUE(button.released()); - // A bounce at the same (large) millis is still inside the debounce window, - // even though the deadline has wrapped to a small number. The old absolute - // comparison (deadline > millis) would wrongly treat the window as expired - // and let this bounce through. + // A bounce just after millis() wraps to zero is still inside the debounce + // window (only ~16 ms elapsed since the release) and must be ignored. + _mock_millis = 0x00000000; _mock_pin_state = LOW; TEST_ASSERT_FALSE(button.pressed()); - // Once millis() wraps past the (wrapped) deadline, a genuine press registers. - _mock_millis = 0x70; + // Once the window has elapsed across the wrap, a genuine press registers. + _mock_millis = 0x00000080; + TEST_ASSERT_TRUE(button.pressed()); +} + +// Reproduces the reported soft-lock: with the old code a change registered near +// the top of the range left a debounce deadline of ~millis_max; after millis() +// wrapped to a small value, (deadline > millis()) stayed true for ~49 days and +// locked out every read. Measuring elapsed time since the change instead means +// the window expires normally across the wrap. +void test_no_softlock_after_overflow(void) +{ + Button button(2, 100); + + // Prime with a normal press, as in normal operation. + _mock_millis = 0x7FFFFFFF; + _mock_pin_state = LOW; + TEST_ASSERT_TRUE(button.pressed()); + + // Release at (max - delay): the old deadline (millis + delay) would be max, + // the exact reported trigger. + _mock_millis = 0xFFFFFFFF - 100; + _mock_pin_state = HIGH; + TEST_ASSERT_TRUE(button.released()); + + // millis() rolls over to a small value. A genuine press must still register; + // the old code would ignore it forever (until millis climbed back to max). + _mock_millis = 5; + _mock_pin_state = LOW; TEST_ASSERT_TRUE(button.pressed()); } @@ -150,11 +194,13 @@ int main(int, char **) { UNITY_BEGIN(); RUN_TEST(test_starts_released); + RUN_TEST(test_ignores_power_on_window); RUN_TEST(test_pressed_fires_once); RUN_TEST(test_released_fires_once); RUN_TEST(test_debounce_ignores_bounce); RUN_TEST(test_toggled_on_each_change); RUN_TEST(test_custom_debounce_time); RUN_TEST(test_debounce_survives_millis_wraparound); + RUN_TEST(test_no_softlock_after_overflow); return UNITY_END(); }