From 33618fb088ec396dc8b791cfbbaac98bcd1b42bf Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 11 Nov 2020 11:37:00 -0600 Subject: [PATCH 1/4] Update Qukeys basic testcase for issue #970 Signed-off-by: Michael Richters --- tests/plugins/Qukeys/basic/common.h | 1 + tests/plugins/Qukeys/basic/sketch.ino | 1 + tests/plugins/Qukeys/basic/test/testcase.cpp | 15 ++++++--------- 3 files changed, 8 insertions(+), 9 deletions(-) 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); From ae0a9975b673a98b76619dc8404ea5ba3a137df4 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 11 Nov 2020 11:51:01 -0600 Subject: [PATCH 2/4] Add testcase for issue #970 Signed-off-by: Michael Richters --- tests/issues/970/common.h | 32 ++++++++ tests/issues/970/sketch.ino | 99 +++++++++++++++++++++++ tests/issues/970/test/testcase.cpp | 126 +++++++++++++++++++++++++++++ 3 files changed, 257 insertions(+) create mode 100644 tests/issues/970/common.h create mode 100644 tests/issues/970/sketch.ino create mode 100644 tests/issues/970/test/testcase.cpp 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/testcase.cpp b/tests/issues/970/test/testcase.cpp new file mode 100644 index 00000000..f00cd535 --- /dev/null +++ b/tests/issues/970/test/testcase.cpp @@ -0,0 +1,126 @@ +/* -*- 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 "testing/setup-googletest.h" + +#include "../common.h" + +SETUP_GOOGLETEST(); + +namespace kaleidoscope { +namespace testing { +namespace { + +constexpr KeyAddr key_addr_A{2, 1}; +constexpr KeyAddr key_addr_S{2, 2}; +constexpr KeyAddr key_addr_D{2, 3}; +constexpr KeyAddr key_addr_F{2, 4}; +constexpr KeyAddr key_addr_X{3, 2}; + +using ::testing::IsEmpty; + +class QukeysBasic : public VirtualDeviceTest { + protected: + std::set expected_keycodes_ = {}; + std::unique_ptr state_ = nullptr; +}; + +TEST_F(QukeysBasic, TapQukeyAlone) { + // Press `A` + sim_.Press(key_addr_A); + + state_ = RunCycle(); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0) + << "There should be no HID report after the qukey is pressed"; + sim_.RunForMillis(20); + // Release `A` + sim_.Release(key_addr_A); + + sim_.RunCycles(2); + state_ = RunCycle(); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 2) + << "There should be two HID reports after the release of a tapped qukey"; + + expected_keycodes_.insert(Key_A.getKeyCode()); + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The first report should include only `A`"; + + expected_keycodes_.erase(Key_A.getKeyCode()); + EXPECT_THAT(state_->HIDReports()->Keyboard(1).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The second report should be empty"; + + state_ = RunCycle(); + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0); +} + +TEST_F(QukeysBasic, MinPriorIntervalOverflow) { + // Wait until integer overflow causes the timestamp to catch up + sim_.RunForMillis(65535); + + // Press `A` + sim_.Press(key_addr_A); + + state_ = RunCycle(); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0); + + uint32_t t0 = Kaleidoscope.millisAtCycleStart(); + + // To test the hold timeout, we check after every cycle to see if there's new + // HID report. We can't just call `RunForMillis()` because we care about when + // that report was sent. + do { + state_ = RunCycle(); + } while (state_->HIDReports()->Keyboard().size() == 0 && + (Kaleidoscope.millisAtCycleStart() - t0 < QUKEYS_HOLD_TIMEOUT + 1)); + + uint32_t t1 = Kaleidoscope.millisAtCycleStart(); + + EXPECT_THAT(t1 - t0, ::testing::Ge(QUKEYS_HOLD_TIMEOUT)) + << "The HID report should be sent after the hold timeout has elapsed"; + + expected_keycodes_.insert(Key_LeftGui.getKeyCode()); + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) + << "There should be only one HID report"; + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The HID report should contain just `Key_LeftGui`"; + + sim_.RunForMillis(100); + + sim_.Release(key_addr_A); + sim_.RunCycles(2); + state_ = RunCycle(); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) + << "There should be a HID report immediately after the key is released"; + + expected_keycodes_.erase(Key_LeftGui.getKeyCode()); + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The HID report should be empty at this point"; + + state_ = RunCycle(); + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0); +} + +} // namespace +} // namespace testing +} // namespace kaleidoscope From 9e1aaac3d83fb7d96dc2c06a4db224b6a7e79253 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 11 Nov 2020 12:04:53 -0600 Subject: [PATCH 3/4] Convert issue #970 testcase to ktest Signed-off-by: Michael Richters --- tests/issues/970/test.ktest | 25 ++++++ tests/issues/970/test/testcase.cpp | 126 ----------------------------- 2 files changed, 25 insertions(+), 126 deletions(-) create mode 100644 tests/issues/970/test.ktest delete mode 100644 tests/issues/970/test/testcase.cpp 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/issues/970/test/testcase.cpp b/tests/issues/970/test/testcase.cpp deleted file mode 100644 index f00cd535..00000000 --- a/tests/issues/970/test/testcase.cpp +++ /dev/null @@ -1,126 +0,0 @@ -/* -*- 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 "testing/setup-googletest.h" - -#include "../common.h" - -SETUP_GOOGLETEST(); - -namespace kaleidoscope { -namespace testing { -namespace { - -constexpr KeyAddr key_addr_A{2, 1}; -constexpr KeyAddr key_addr_S{2, 2}; -constexpr KeyAddr key_addr_D{2, 3}; -constexpr KeyAddr key_addr_F{2, 4}; -constexpr KeyAddr key_addr_X{3, 2}; - -using ::testing::IsEmpty; - -class QukeysBasic : public VirtualDeviceTest { - protected: - std::set expected_keycodes_ = {}; - std::unique_ptr state_ = nullptr; -}; - -TEST_F(QukeysBasic, TapQukeyAlone) { - // Press `A` - sim_.Press(key_addr_A); - - state_ = RunCycle(); - - ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0) - << "There should be no HID report after the qukey is pressed"; - sim_.RunForMillis(20); - // Release `A` - sim_.Release(key_addr_A); - - sim_.RunCycles(2); - state_ = RunCycle(); - - ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 2) - << "There should be two HID reports after the release of a tapped qukey"; - - expected_keycodes_.insert(Key_A.getKeyCode()); - EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), - ::testing::ElementsAreArray(expected_keycodes_)) - << "The first report should include only `A`"; - - expected_keycodes_.erase(Key_A.getKeyCode()); - EXPECT_THAT(state_->HIDReports()->Keyboard(1).ActiveKeycodes(), - ::testing::ElementsAreArray(expected_keycodes_)) - << "The second report should be empty"; - - state_ = RunCycle(); - ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0); -} - -TEST_F(QukeysBasic, MinPriorIntervalOverflow) { - // Wait until integer overflow causes the timestamp to catch up - sim_.RunForMillis(65535); - - // Press `A` - sim_.Press(key_addr_A); - - state_ = RunCycle(); - - ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0); - - uint32_t t0 = Kaleidoscope.millisAtCycleStart(); - - // To test the hold timeout, we check after every cycle to see if there's new - // HID report. We can't just call `RunForMillis()` because we care about when - // that report was sent. - do { - state_ = RunCycle(); - } while (state_->HIDReports()->Keyboard().size() == 0 && - (Kaleidoscope.millisAtCycleStart() - t0 < QUKEYS_HOLD_TIMEOUT + 1)); - - uint32_t t1 = Kaleidoscope.millisAtCycleStart(); - - EXPECT_THAT(t1 - t0, ::testing::Ge(QUKEYS_HOLD_TIMEOUT)) - << "The HID report should be sent after the hold timeout has elapsed"; - - expected_keycodes_.insert(Key_LeftGui.getKeyCode()); - ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) - << "There should be only one HID report"; - EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), - ::testing::ElementsAreArray(expected_keycodes_)) - << "The HID report should contain just `Key_LeftGui`"; - - sim_.RunForMillis(100); - - sim_.Release(key_addr_A); - sim_.RunCycles(2); - state_ = RunCycle(); - - ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) - << "There should be a HID report immediately after the key is released"; - - expected_keycodes_.erase(Key_LeftGui.getKeyCode()); - EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), - ::testing::ElementsAreArray(expected_keycodes_)) - << "The HID report should be empty at this point"; - - state_ = RunCycle(); - ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0); -} - -} // namespace -} // namespace testing -} // namespace kaleidoscope From 0d788bb718ea46576da6f47e714e426519784e71 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 11 Nov 2020 11:38:05 -0600 Subject: [PATCH 4/4] Fix Qukeys minimum prior interval problem The code for guarding against integer overflow on the prior interval timestamp was in the wrong place, and wouldn't get executed on cycles when the keyboard was idle, leading to a very slim chance of getting the wrong qukey value if all keys were idle long enough (65 seconds). Also fixed the same problem in the first quarter-second after the keyboard power on. Not likely to ever be observed, but costs nothing extra to fix. Signed-off-by: Michael Richters --- src/kaleidoscope/plugin/Qukeys.cpp | 16 ++++++++-------- src/kaleidoscope/plugin/Qukeys.h | 5 ++++- 2 files changed, 12 insertions(+), 9 deletions(-) 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