diff --git a/src/kaleidoscope/plugin/Qukeys.cpp b/src/kaleidoscope/plugin/Qukeys.cpp
index e9a9637e..67643acc 100644
--- a/src/kaleidoscope/plugin/Qukeys.cpp
+++ b/src/kaleidoscope/plugin/Qukeys.cpp
@@ -118,6 +118,14 @@ EventHandlerResult Qukeys::beforeReportingState() {
}
}
+ // Next, if there hasn't been a keypress in a while, update the prior keypress
+ // timestamp to avoid integer overflow issues:
+ if (Runtime.hasTimeExpired(prior_keypress_timestamp_,
+ minimum_prior_interval_)) {
+ prior_keypress_timestamp_ =
+ Runtime.millisAtCycleStart() - (minimum_prior_interval_ + 1);
+ }
+
// If any events get flushed from the queue, stop there; we can only safely
// send the one report per cycle.
if (processQueue()) {
@@ -134,14 +142,6 @@ EventHandlerResult Qukeys::beforeReportingState() {
queue_head_.primary_key : queue_head_.alternate_key;
flushEvent(event_key);
}
-
- // Last, if there hasn't been a keypress in a while, update the prior keypress
- // timestamp to avoid integer overflow issues:
- if (Runtime.hasTimeExpired(prior_keypress_timestamp_,
- minimum_prior_interval_)) {
- prior_keypress_timestamp_ =
- Runtime.millisAtCycleStart() - (minimum_prior_interval_ + 1);
- }
return EventHandlerResult::OK;
}
diff --git a/src/kaleidoscope/plugin/Qukeys.h b/src/kaleidoscope/plugin/Qukeys.h
index e4b5d320..78ad0e52 100644
--- a/src/kaleidoscope/plugin/Qukeys.h
+++ b/src/kaleidoscope/plugin/Qukeys.h
@@ -202,7 +202,10 @@ class Qukeys : public kaleidoscope::Plugin {
uint8_t minimum_prior_interval_{75};
// Timestamp of the keypress event immediately prior to the queue head event.
- uint16_t prior_keypress_timestamp_{0};
+ // The initial value is 256 to ensure that it won't trigger an error if a
+ // qukey is pressed before `minimum_prior_interval_` milliseconds after the
+ // keyboard powers on, and that value can only be as high as 255.
+ uint16_t prior_keypress_timestamp_{256};
// This is a guard against re-processing events when qukeys flushes them from
// its event queue. We can't just use an "injected" key state flag, because
diff --git a/tests/issues/970/common.h b/tests/issues/970/common.h
new file mode 100644
index 00000000..86c93412
--- /dev/null
+++ b/tests/issues/970/common.h
@@ -0,0 +1,32 @@
+// -*- mode: c++ -*-
+
+/* Kaleidoscope - Firmware for computer input devices
+ * Copyright (C) 2020 Keyboard.io, Inc.
+ *
+ * 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, version 3.
+ *
+ * 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 .
+ */
+
+#pragma once
+
+#include
+
+namespace kaleidoscope {
+namespace testing {
+
+constexpr uint16_t QUKEYS_HOLD_TIMEOUT = 200;
+constexpr uint8_t QUKEYS_OVERLAP_THRESHOLD = 90;
+constexpr uint8_t QUKEYS_MINIMUM_HOLD_TIME = 10;
+constexpr uint8_t QUKEYS_MIN_PRIOR_INTERVAL = 20;
+
+}
+}
diff --git a/tests/issues/970/sketch.ino b/tests/issues/970/sketch.ino
new file mode 100644
index 00000000..335586a5
--- /dev/null
+++ b/tests/issues/970/sketch.ino
@@ -0,0 +1,99 @@
+/* -*- mode: c++ -*-
+ * Copyright (C) 2020 Keyboard.io, Inc.
+ *
+ * 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, version 3.
+ *
+ * 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 .
+ */
+
+#include
+#include
+#include
+
+#include "./common.h"
+
+enum { MACRO_TOGGLE_QUKEYS };
+
+// *INDENT-OFF*
+KEYMAPS(
+ [0] = KEYMAP_STACKED
+ (
+ Key_NoKey, Key_1, Key_2, Key_3, Key_4, Key_5, Key_NoKey,
+ Key_Backtick, Key_Q, Key_W, Key_E, Key_R, Key_T, Key_Tab,
+ Key_PageUp, Key_A, Key_S, Key_D, Key_F, Key_G,
+ Key_PageDown, Key_Z, Key_X, Key_C, Key_V, Key_B, Key_Escape,
+
+ Key_LeftControl, Key_Backspace, Key_LeftGui, Key_LeftShift,
+ Key_Q,
+
+ M(MACRO_TOGGLE_QUKEYS), Key_6, Key_7, Key_8, Key_9, Key_0, Key_skip,
+ Key_Enter, Key_Y, Key_U, Key_I, Key_O, Key_P, Key_Equals,
+ Key_H, SFT_T(J), CTL_T(K), ALT_T(L), GUI_T(Semicolon), Key_Quote,
+ Key_skip, Key_N, Key_M, Key_Comma, Key_Period, Key_Slash, Key_Minus,
+
+ Key_RightShift, Key_RightAlt, Key_Spacebar, Key_RightControl,
+ LT(1,E)
+ ),
+ [1] = KEYMAP_STACKED
+ (
+ ___, Key_B, Key_C, Key_D, Key_E, Key_F, Key_G,
+ Key_A, Key_B, Key_C, Key_D, Key_E, Key_F, Key_G,
+ Key_A, Key_B, Key_C, Key_D, Key_E, Key_F,
+ Key_A, Key_B, Key_C, Key_D, Key_E, Key_F, Key_G,
+
+ Key_1, Key_2, Key_3, Key_4,
+ ___,
+
+
+ ___, Key_B, Key_C, Key_D, Key_E, Key_F, Key_G,
+ Key_A, Key_B, Key_C, Key_D, Key_E, Key_F, Key_G,
+ Key_A, Key_B, Key_C, Key_D, Key_E, Key_F,
+ Key_A, Key_B, Key_C, Key_D, Key_E, Key_F, Key_G,
+
+ Key_1, Key_2, Key_3, Key_4,
+ ___
+ ),
+)
+// *INDENT-ON*
+
+// Defining a macro (on the "any" key: see above) to toggle Qukeys on and off
+const macro_t *macroAction(uint8_t macro_index, uint8_t key_state) {
+ switch (macro_index) {
+ case MACRO_TOGGLE_QUKEYS:
+ if (keyToggledOn(key_state))
+ Qukeys.toggle();
+ break;
+ }
+ return MACRO_NONE;
+}
+
+// Use Qukeys
+KALEIDOSCOPE_INIT_PLUGINS(Qukeys, Macros);
+
+void setup() {
+ QUKEYS(
+ kaleidoscope::plugin::Qukey(0, KeyAddr(2, 1), Key_LeftGui), // A/cmd
+ kaleidoscope::plugin::Qukey(0, KeyAddr(2, 2), Key_LeftAlt), // S/alt
+ kaleidoscope::plugin::Qukey(0, KeyAddr(2, 3), Key_LeftControl), // D/ctrl
+ kaleidoscope::plugin::Qukey(0, KeyAddr(2, 4), Key_LeftShift), // F/shift
+ kaleidoscope::plugin::Qukey(0, KeyAddr(3, 6), ShiftToLayer(1)) // Q/layer-shift (on `fn`)
+ )
+ Qukeys.setHoldTimeout(kaleidoscope::testing::QUKEYS_HOLD_TIMEOUT);
+ Qukeys.setOverlapThreshold(kaleidoscope::testing::QUKEYS_OVERLAP_THRESHOLD);
+ Qukeys.setMinimumHoldTime(kaleidoscope::testing::QUKEYS_MINIMUM_HOLD_TIME);
+ Qukeys.setMinimumPriorInterval(kaleidoscope::testing::QUKEYS_MIN_PRIOR_INTERVAL);
+
+ Kaleidoscope.setup();
+}
+
+void loop() {
+ Kaleidoscope.loop();
+}
diff --git a/tests/issues/970/test.ktest b/tests/issues/970/test.ktest
new file mode 100644
index 00000000..6bb39c17
--- /dev/null
+++ b/tests/issues/970/test.ktest
@@ -0,0 +1,25 @@
+NAME Issue 970 Qukeys min prior interval overflow
+
+KEYSWITCH A 2 1
+
+PRESS A
+RUN 10 ms
+
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report Key_A # Report should contain only `A`
+RUN 1 cycle
+EXPECT keyboard-report empty # Report should be empty
+
+RUN 65536 ms
+
+PRESS A
+RUN 201 ms
+EXPECT keyboard-report Key_LeftGui # Report should contain only `LeftGui`
+
+RUN 10 ms
+
+RELEASE A
+# I'm not sure why it takes 2 cycles before the report is sent
+RUN 2 cycles
+EXPECT keyboard-report empty # Report should be empty
diff --git a/tests/plugins/Qukeys/basic/common.h b/tests/plugins/Qukeys/basic/common.h
index 71b91a3d..86c93412 100644
--- a/tests/plugins/Qukeys/basic/common.h
+++ b/tests/plugins/Qukeys/basic/common.h
@@ -26,6 +26,7 @@ namespace testing {
constexpr uint16_t QUKEYS_HOLD_TIMEOUT = 200;
constexpr uint8_t QUKEYS_OVERLAP_THRESHOLD = 90;
constexpr uint8_t QUKEYS_MINIMUM_HOLD_TIME = 10;
+constexpr uint8_t QUKEYS_MIN_PRIOR_INTERVAL = 20;
}
}
diff --git a/tests/plugins/Qukeys/basic/sketch.ino b/tests/plugins/Qukeys/basic/sketch.ino
index a1f3ab0a..335586a5 100644
--- a/tests/plugins/Qukeys/basic/sketch.ino
+++ b/tests/plugins/Qukeys/basic/sketch.ino
@@ -89,6 +89,7 @@ void setup() {
Qukeys.setHoldTimeout(kaleidoscope::testing::QUKEYS_HOLD_TIMEOUT);
Qukeys.setOverlapThreshold(kaleidoscope::testing::QUKEYS_OVERLAP_THRESHOLD);
Qukeys.setMinimumHoldTime(kaleidoscope::testing::QUKEYS_MINIMUM_HOLD_TIME);
+ Qukeys.setMinimumPriorInterval(kaleidoscope::testing::QUKEYS_MIN_PRIOR_INTERVAL);
Kaleidoscope.setup();
}
diff --git a/tests/plugins/Qukeys/basic/test/testcase.cpp b/tests/plugins/Qukeys/basic/test/testcase.cpp
index f9f1ede6..bfc1c38a 100644
--- a/tests/plugins/Qukeys/basic/test/testcase.cpp
+++ b/tests/plugins/Qukeys/basic/test/testcase.cpp
@@ -39,9 +39,6 @@ class QukeysBasic : public VirtualDeviceTest {
};
TEST_F(QukeysBasic, TapQukeyAlone) {
- // XXX Temporary workaround
- sim_.RunForMillis(1000);
-
// Press `A`
sim_.Press(key_addr_A);
@@ -74,8 +71,8 @@ TEST_F(QukeysBasic, TapQukeyAlone) {
}
TEST_F(QukeysBasic, HoldQukeyAlone) {
- // XXX Temporary workaround
- sim_.RunForMillis(1000);
+ // Prevent rapid typing suppression from affecting the test
+ sim_.RunForMillis(QUKEYS_MIN_PRIOR_INTERVAL);
// Press `A`
sim_.Press(key_addr_A);
@@ -125,8 +122,8 @@ TEST_F(QukeysBasic, HoldQukeyAlone) {
}
TEST_F(QukeysBasic, FullOverlap) {
- // XXX Temporary workaround
- sim_.RunForMillis(1000);
+ // Prevent rapid typing suppression from affecting the test
+ sim_.RunForMillis(QUKEYS_MIN_PRIOR_INTERVAL);
sim_.Press(key_addr_F);
sim_.RunForMillis(20);
@@ -172,8 +169,8 @@ TEST_F(QukeysBasic, FullOverlap) {
}
TEST_F(QukeysBasic, RolloverPrimary) {
- // XXX Temporary workaround
- sim_.RunForMillis(1000);
+ // Prevent rapid typing suppression from affecting the test
+ sim_.RunForMillis(QUKEYS_MIN_PRIOR_INTERVAL);
sim_.Press(key_addr_F);
sim_.RunForMillis(20);