Small tweak to fix potential softlock near millis() boundary

src/Button.cpp

@@ -9,7 +9,7 @@
 #include <stdint.h>
 
 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,19 +24,19 @@ 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
 	}
 
 	// pin has changed
 	else if (digitalRead(_pin) != _state)
 	{
-		_ignore_until = millis() + _delay;
+		_ignore_start = millis();
 		_state = !_state;
 		_has_changed = true;
 	}

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;
 };
 

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,39 +141,66 @@ 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());
 }
 
 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();
 }