[Enhancement] Improvements for debounce test coverage + bug fixes for sym_defer_g and sym_eager_pr (#21667)

Co-authored-by: Nebuleon <2391500+Nebuleon@users.noreply.github.com>

11 files changed, 402 insertions, 9 deletions

diff --git a/quantum/debounce/tests/asym_eager_defer_pk_tests.cpp b/quantum/debounce/tests/asym_eager_defer_pk_tests.cpp
index 44b4fe1956..6737f499ab 100644
--- a/quantum/debounce/tests/asym_eager_defer_pk_tests.cpp
+++ b/quantum/debounce/tests/asym_eager_defer_pk_tests.cpp
@@ -392,3 +392,32 @@ TEST_F(DebounceTest, OneKeyDelayedScan8) {
     time_jumps_ = true;
     runEvents();
 }
+
+TEST_F(DebounceTest, AsyncTickOneKeyShort1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        /* Release key after 1ms delay */
+        {1, {{0, 1, UP}}, {}},
+
+        /*
+         * Until the eager timer on DOWN is observed to finish, the defer timer
+         * on UP can't start. There's no workaround for this because it's not
+         * possible to debounce an event that isn't being tracked.
+         *
+         * sym_defer_pk has the same problem but the test has to track that the
+         * key changed state so the DOWN timer is always allowed to finish
+         * before starting the UP timer.
+         */
+        {5, {}, {}},
+
+        {10, {}, {{0, 1, UP}}}, /* 5ms+5ms after DOWN at time 0 */
+        /* Press key again after 1ms delay */
+        {11, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+    });
+    /*
+     * Debounce implementations should never read the timer more than once per invocation
+     */
+    async_time_jumps_ = DEBOUNCE;
+    runEvents();
+}
diff --git a/quantum/debounce/tests/debounce_test_common.cpp b/quantum/debounce/tests/debounce_test_common.cpp
index b11378b286..fd4b6f01a6 100644
--- a/quantum/debounce/tests/debounce_test_common.cpp
+++ b/quantum/debounce/tests/debounce_test_common.cpp
@@ -26,8 +26,12 @@ extern "C" {
 #include "debounce.h"
 #include "timer.h"
 
-void set_time(uint32_t t);
-void advance_time(uint32_t ms);
+void     simulate_async_tick(uint32_t t);
+void     reset_access_counter(void);
+uint32_t current_access_counter(void);
+uint32_t timer_read_internal(void);
+void     set_time(uint32_t t);
+void     advance_time(uint32_t ms);
 }
 
 void DebounceTest::addEvents(std::initializer_list<DebounceTestEvent> events) {
@@ -58,6 +62,7 @@ void DebounceTest::runEventsInternal() {
     /* Initialise keyboard with start time (offset to avoid testing at 0) and all keys UP */
     debounce_init(MATRIX_ROWS);
     set_time(time_offset_);
+    simulate_async_tick(async_time_jumps_);
     std::fill(std::begin(input_matrix_), std::end(input_matrix_), 0);
     std::fill(std::begin(output_matrix_), std::end(output_matrix_), 0);
 
@@ -70,9 +75,9 @@ void DebounceTest::runEventsInternal() {
             advance_time(1);
         } else {
             /* Fast forward to the time for this event, calling debounce() with no changes */
-            ASSERT_LT((time_offset_ + event.time_) - timer_read_fast(), 60000) << "Test tries to advance more than 1 minute of time";
+            ASSERT_LT((time_offset_ + event.time_) - timer_read_internal(), 60000) << "Test tries to advance more than 1 minute of time";
 
-            while (timer_read_fast() != time_offset_ + event.time_) {
+            while (timer_read_internal() != time_offset_ + event.time_) {
                 runDebounce(false);
                 checkCookedMatrix(false, "debounce() modified cooked matrix");
                 advance_time(1);
@@ -124,14 +129,20 @@ void DebounceTest::runDebounce(bool changed) {
     std::copy(std::begin(input_matrix_), std::end(input_matrix_), std::begin(raw_matrix_));
     std::copy(std::begin(output_matrix_), std::end(output_matrix_), std::begin(cooked_matrix_));
 
+    reset_access_counter();
+
     bool cooked_changed = debounce(raw_matrix_, cooked_matrix_, MATRIX_ROWS, changed);
 
     if (!std::equal(std::begin(input_matrix_), std::end(input_matrix_), std::begin(raw_matrix_))) {
         FAIL() << "Fatal error: debounce() modified raw matrix at " << strTime() << "\ninput_matrix: changed=" << changed << "\n" << strMatrix(input_matrix_) << "\nraw_matrix:\n" << strMatrix(raw_matrix_);
     }
 
-    if (std::equal(std::begin(output_matrix_), std::end(output_matrix_), std::begin(cooked_matrix_)) && cooked_changed) {
-        FAIL() << "Fatal error: debounce() did detect a wrong cooked matrix change at " << strTime() << "\noutput_matrix: cooked_changed=" << cooked_changed << "\n" << strMatrix(output_matrix_) << "\ncooked_matrix:\n" << strMatrix(cooked_matrix_);
+    if (std::equal(std::begin(output_matrix_), std::end(output_matrix_), std::begin(cooked_matrix_)) == cooked_changed) {
+        FAIL() << "Fatal error: debounce() reported a wrong cooked matrix change result at " << strTime() << "\noutput_matrix: cooked_changed=" << cooked_changed << "\n" << strMatrix(output_matrix_) << "\ncooked_matrix:\n" << strMatrix(cooked_matrix_);
+    }
+
+    if (current_access_counter() > 1) {
+        FAIL() << "Fatal error: debounce() read the timer multiple times, which is not allowed, at " << strTime() << "\ntimer: access_count=" << current_access_counter() << "\noutput_matrix: cooked_changed=" << cooked_changed << "\n" << strMatrix(output_matrix_) << "\ncooked_matrix:\n" << strMatrix(cooked_matrix_);
     }
 }
 
@@ -144,7 +155,7 @@ void DebounceTest::checkCookedMatrix(bool changed, const std::string &error_mess
 std::string DebounceTest::strTime() {
     std::stringstream text;
 
-    text << "time " << (timer_read_fast() - time_offset_) << " (extra_iterations=" << extra_iterations_ << ", auto_advance_time=" << auto_advance_time_ << ")";
+    text << "time " << (timer_read_internal() - time_offset_) << " (extra_iterations=" << extra_iterations_ << ", auto_advance_time=" << auto_advance_time_ << ")";
 
     return text.str();
 }
diff --git a/quantum/debounce/tests/debounce_test_common.h b/quantum/debounce/tests/debounce_test_common.h
index 7319abfbf3..ebbe340c05 100644
--- a/quantum/debounce/tests/debounce_test_common.h
+++ b/quantum/debounce/tests/debounce_test_common.h
@@ -54,8 +54,9 @@ class DebounceTest : public ::testing::Test {
     void addEvents(std::initializer_list<DebounceTestEvent> events);
     void runEvents();
 
-    fast_timer_t time_offset_ = 7777;
-    bool         time_jumps_  = false;
+    fast_timer_t time_offset_      = 7777;
+    bool         time_jumps_       = false;
+    fast_timer_t async_time_jumps_ = 0;
 
    private:
     static bool        directionValue(Direction direction);
diff --git a/quantum/debounce/tests/none_tests.cpp b/quantum/debounce/tests/none_tests.cpp
new file mode 100644
index 0000000000..69fdd02101
--- /dev/null
+++ b/quantum/debounce/tests/none_tests.cpp
@@ -0,0 +1,256 @@
+/* Copyright 2021 Simon Arlott
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "gtest/gtest.h"
+
+#include "debounce_test_common.h"
+
+TEST_F(DebounceTest, OneKeyShort1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        {5, {}, {}},
+        /* 0ms delay (fast scan rate) */
+        {5, {{0, 1, UP}}, {{0, 1, UP}}},
+
+        {10, {}, {}},
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyShort2) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        {5, {}, {}},
+        /* 1ms delay */
+        {6, {{0, 1, UP}}, {{0, 1, UP}}},
+
+        {11, {}, {}},
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyShort3) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        {5, {}, {}},
+        /* 2ms delay */
+        {7, {{0, 1, UP}}, {{0, 1, UP}}},
+
+        {12, {}, {}},
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyTooQuick1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        /* Release key exactly on the debounce time */
+        {5, {{0, 1, UP}}, {{0, 1, UP}}},
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyTooQuick2) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        {5, {}, {}},
+        {6, {{0, 1, UP}}, {{0, 1, UP}}},
+
+        /* Press key exactly on the debounce time */
+        {11, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyBouncing1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {1, {{0, 1, UP}}, {{0, 1, UP}}},
+        {2, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {3, {{0, 1, UP}}, {{0, 1, UP}}},
+        {4, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {5, {{0, 1, UP}}, {{0, 1, UP}}},
+        {6, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {11, {{0, 1, UP}}, {{0, 1, UP}}}, /* 5ms after DOWN at time 7 */
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyBouncing2) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {5, {}, {}},
+        {6, {{0, 1, UP}}, {{0, 1, UP}}},
+        {7, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {8, {{0, 1, UP}}, {{0, 1, UP}}},
+        {9, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {10, {{0, 1, UP}}, {{0, 1, UP}}},
+        {15, {}, {}}, /* 5ms after UP at time 10 */
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyLong) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        {5, {}, {}},
+
+        {25, {{0, 1, UP}}, {{0, 1, UP}}},
+
+        {30, {}, {}},
+
+        {50, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        {55, {}, {}},
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, TwoKeysShort) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {1, {{0, 2, DOWN}}, {{0, 2, DOWN}}},
+
+        {6, {}, {}},
+
+        {7, {{0, 1, UP}}, {{0, 1, UP}}},
+        {8, {{0, 2, UP}}, {{0, 2, UP}}},
+
+        {13, {}, {}},
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, TwoKeysSimultaneous1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}, {0, 2, DOWN}}, {{0, 1, DOWN}, {0, 2, DOWN}}},
+
+        {5, {}, {}},
+        {6, {{0, 1, UP}, {0, 2, UP}}, {{0, 1, UP}, {0, 2, UP}}},
+
+        {11, {}, {}},
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, TwoKeysSimultaneous2) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {1, {{0, 2, DOWN}}, {{0, 2, DOWN}}},
+
+        {5, {}, {}},
+        {6, {}, {}},
+        {7, {{0, 1, UP}}, {{0, 1, UP}}},
+        {8, {{0, 2, UP}}, {{0, 2, UP}}},
+
+        {13, {}, {}},
+    });
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyDelayedScan1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        /* Processing is very late */
+        {300, {}, {}},
+        /* Immediately release key */
+        {300, {{0, 1, UP}}, {{0, 1, UP}}},
+
+        {305, {}, {}},
+    });
+    time_jumps_ = true;
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyDelayedScan2) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        /* Processing is very late */
+        {300, {}, {}},
+        /* Release key after 1ms */
+        {301, {{0, 1, UP}}, {{0, 1, UP}}},
+
+        {306, {}, {}},
+    });
+    time_jumps_ = true;
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyDelayedScan3) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        /* Release key before debounce expires */
+        {300, {{0, 1, UP}}, {{0, 1, UP}}},
+    });
+    time_jumps_ = true;
+    runEvents();
+}
+
+TEST_F(DebounceTest, OneKeyDelayedScan4) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        /* Processing is a bit late */
+        {50, {}, {}},
+        /* Release key after 1ms */
+        {51, {{0, 1, UP}}, {{0, 1, UP}}},
+
+        {56, {}, {}},
+    });
+    time_jumps_ = true;
+    runEvents();
+}
+
+TEST_F(DebounceTest, AsyncTickOneKeyShort1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+
+        {5, {}, {}},
+        /* 0ms delay (fast scan rate) */
+        {5, {{0, 1, UP}}, {{0, 1, UP}}},
+
+        {10, {}, {}},
+    });
+    /*
+     * Debounce implementations should never read the timer more than once per invocation
+     */
+    async_time_jumps_ = DEBOUNCE;
+    runEvents();
+}
diff --git a/quantum/debounce/tests/rules.mk b/quantum/debounce/tests/rules.mk
index 8318b1c668..bbc362d4c7 100644
--- a/quantum/debounce/tests/rules.mk
+++ b/quantum/debounce/tests/rules.mk
@@ -18,6 +18,11 @@ DEBOUNCE_COMMON_DEFS := -DMATRIX_ROWS=4 -DMATRIX_COLS=10 -DDEBOUNCE=5
 DEBOUNCE_COMMON_SRC := $(QUANTUM_PATH)/debounce/tests/debounce_test_common.cpp \
 	$(PLATFORM_PATH)/$(PLATFORM_KEY)/timer.c
 
+debounce_none_DEFS := $(DEBOUNCE_COMMON_DEFS)
+debounce_none_SRC := $(DEBOUNCE_COMMON_SRC) \
+	$(QUANTUM_PATH)/debounce/none.c \
+	$(QUANTUM_PATH)/debounce/tests/none_tests.cpp
+
 debounce_sym_defer_g_DEFS := $(DEBOUNCE_COMMON_DEFS)
 debounce_sym_defer_g_SRC := $(DEBOUNCE_COMMON_SRC) \
 	$(QUANTUM_PATH)/debounce/sym_defer_g.c \
diff --git a/quantum/debounce/tests/sym_defer_g_tests.cpp b/quantum/debounce/tests/sym_defer_g_tests.cpp
index 73d3d45e30..33e8b17852 100644
--- a/quantum/debounce/tests/sym_defer_g_tests.cpp
+++ b/quantum/debounce/tests/sym_defer_g_tests.cpp
@@ -236,3 +236,21 @@ TEST_F(DebounceTest, OneKeyDelayedScan4) {
     time_jumps_ = true;
     runEvents();
 }
+
+TEST_F(DebounceTest, AsyncTickOneKeyShort1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {}},
+
+        {5, {}, {{0, 1, DOWN}}},
+        /* 0ms delay (fast scan rate) */
+        {5, {{0, 1, UP}}, {}},
+
+        {10, {}, {{0, 1, UP}}},
+    });
+    /*
+     * Debounce implementations should never read the timer more than once per invocation
+     */
+    async_time_jumps_ = DEBOUNCE;
+    runEvents();
+}
diff --git a/quantum/debounce/tests/sym_defer_pk_tests.cpp b/quantum/debounce/tests/sym_defer_pk_tests.cpp
index 7542c2dad4..864b7afcc4 100644
--- a/quantum/debounce/tests/sym_defer_pk_tests.cpp
+++ b/quantum/debounce/tests/sym_defer_pk_tests.cpp
@@ -238,3 +238,21 @@ TEST_F(DebounceTest, OneKeyDelayedScan4) {
     time_jumps_ = true;
     runEvents();
 }
+
+TEST_F(DebounceTest, AsyncTickOneKeyShort1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {}},
+
+        {5, {}, {{0, 1, DOWN}}},
+        /* 0ms delay (fast scan rate) */
+        {5, {{0, 1, UP}}, {}},
+
+        {10, {}, {{0, 1, UP}}},
+    });
+    /*
+     * Debounce implementations should never read the timer more than once per invocation
+     */
+    async_time_jumps_ = DEBOUNCE;
+    runEvents();
+}
diff --git a/quantum/debounce/tests/sym_defer_pr_tests.cpp b/quantum/debounce/tests/sym_defer_pr_tests.cpp
index 417e1f4ca2..3ed360b966 100644
--- a/quantum/debounce/tests/sym_defer_pr_tests.cpp
+++ b/quantum/debounce/tests/sym_defer_pr_tests.cpp
@@ -236,3 +236,21 @@ TEST_F(DebounceTest, OneKeyDelayedScan4) {
     time_jumps_ = true;
     runEvents();
 }
+
+TEST_F(DebounceTest, AsyncTickOneKeyShort1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {}},
+
+        {5, {}, {{0, 1, DOWN}}},
+        /* 0ms delay (fast scan rate) */
+        {5, {{0, 1, UP}}, {}},
+
+        {10, {}, {{0, 1, UP}}},
+    });
+    /*
+     * Debounce implementations should never read the timer more than once per invocation
+     */
+    async_time_jumps_ = DEBOUNCE;
+    runEvents();
+}
diff --git a/quantum/debounce/tests/sym_eager_pk_tests.cpp b/quantum/debounce/tests/sym_eager_pk_tests.cpp
index d9a02fe33c..39d5b10d8e 100644
--- a/quantum/debounce/tests/sym_eager_pk_tests.cpp
+++ b/quantum/debounce/tests/sym_eager_pk_tests.cpp
@@ -251,3 +251,21 @@ TEST_F(DebounceTest, OneKeyDelayedScan6) {
     time_jumps_ = true;
     runEvents();
 }
+
+TEST_F(DebounceTest, AsyncTickOneKeyShort1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {1, {{0, 1, UP}}, {}},
+
+        {5, {}, {{0, 1, UP}}},
+        /* Press key again after 1ms delay (debounce has not yet finished) */
+        {6, {{0, 1, DOWN}}, {}},
+        {10, {}, {{0, 1, DOWN}}}, /* 5ms after UP at time 5 */
+    });
+    /*
+     * Debounce implementations should never read the timer more than once per invocation
+     */
+    async_time_jumps_ = DEBOUNCE;
+    runEvents();
+}
diff --git a/quantum/debounce/tests/sym_eager_pr_tests.cpp b/quantum/debounce/tests/sym_eager_pr_tests.cpp
index e91dd9cb87..9a94807a49 100644
--- a/quantum/debounce/tests/sym_eager_pr_tests.cpp
+++ b/quantum/debounce/tests/sym_eager_pr_tests.cpp
@@ -297,3 +297,21 @@ TEST_F(DebounceTest, OneKeyDelayedScan6) {
     time_jumps_ = true;
     runEvents();
 }
+
+TEST_F(DebounceTest, AsyncTickOneKeyShort1) {
+    addEvents({
+        /* Time, Inputs, Outputs */
+        {0, {{0, 1, DOWN}}, {{0, 1, DOWN}}},
+        {1, {{0, 1, UP}}, {}},
+
+        {5, {}, {{0, 1, UP}}},
+        /* Press key again after 1ms delay (debounce has not yet finished) */
+        {6, {{0, 1, DOWN}}, {}},
+        {10, {}, {{0, 1, DOWN}}}, /* 5ms after UP at time 5 */
+    });
+    /*
+     * Debounce implementations should never read the timer more than once per invocation
+     */
+    async_time_jumps_ = DEBOUNCE;
+    runEvents();
+}
diff --git a/quantum/debounce/tests/testlist.mk b/quantum/debounce/tests/testlist.mk
index f7bd520698..dd53633343 100644
--- a/quantum/debounce/tests/testlist.mk
+++ b/quantum/debounce/tests/testlist.mk
@@ -1,4 +1,5 @@
 TEST_LIST += \
+	debounce_none \
 	debounce_sym_defer_g \
 	debounce_sym_defer_pk \
 	debounce_sym_defer_pr \