From 20a380aa3fe29b1d72b6e54dbaf2bfaf93aec9e6 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Mon, 16 Nov 2020 09:31:32 -0600 Subject: [PATCH 1/3] Generate expected report check unconditionally Even if a testcase shouldn't generate any HID reports, we should verify that it didn't. If it does generate a report when there are none expected, that test should fail. Signed-off-by: Michael Richters --- testing/bin/ktest-to-cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/testing/bin/ktest-to-cxx b/testing/bin/ktest-to-cxx index 16623a39..68853570 100644 --- a/testing/bin/ktest-to-cxx +++ b/testing/bin/ktest-to-cxx @@ -221,9 +221,7 @@ sub generate_start_new_test { cxx("ClearState(); // Clear any state from previous tests"); } sub generate_end_test { - if ($reports_expected) { - generate_check_expected_reports(); - } + generate_check_expected_reports(); outdent(); cxx("} // TEST_F"); cxx(''); From 2b6e6f437cc13af4e200c20e5b624ad0f3bfcfff Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Mon, 16 Nov 2020 08:54:28 -0600 Subject: [PATCH 2/3] Add testcase for idle key events in simulator Signed-off-by: Michael Richters --- tests/issues/978/common.h | 27 ++++++++++++++ tests/issues/978/sketch.ino | 72 +++++++++++++++++++++++++++++++++++++ tests/issues/978/test.ktest | 12 +++++++ 3 files changed, 111 insertions(+) create mode 100644 tests/issues/978/common.h create mode 100644 tests/issues/978/sketch.ino create mode 100644 tests/issues/978/test.ktest diff --git a/tests/issues/978/common.h b/tests/issues/978/common.h new file mode 100644 index 00000000..dcfcc35b --- /dev/null +++ b/tests/issues/978/common.h @@ -0,0 +1,27 @@ +// -*- 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 { + +} // namespace testing +} // namespace kaleidoscope diff --git a/tests/issues/978/sketch.ino b/tests/issues/978/sketch.ino new file mode 100644 index 00000000..a9e0b0e7 --- /dev/null +++ b/tests/issues/978/sketch.ino @@ -0,0 +1,72 @@ +/* -*- 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 "./common.h" + +#undef min +#undef max +#include + +// *INDENT-OFF* +KEYMAPS( + [0] = KEYMAP_STACKED + ( + Key_A, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, + ___, + + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, + ___ + ), +) +// *INDENT-ON* + +namespace kaleidoscope { +namespace plugin { + +class IdleKeyDetector : public kaleidoscope::Plugin { + public: + EventHandlerResult onKeyswitchEvent(Key &key, KeyAddr key_addr, uint8_t key_state) { + if (key_addr == KeyAddr{0, 0} && key_state == 0) { + handleKeyswitchEvent(Key_X, key_addr, IS_PRESSED | WAS_PRESSED); + } + return EventHandlerResult::OK; + } +}; + +} // namespace plugin +} // namespace kaleidoscope + +kaleidoscope::plugin::IdleKeyDetector IdleKeyDetector; + +KALEIDOSCOPE_INIT_PLUGINS(IdleKeyDetector); + +void setup() { + Kaleidoscope.setup(); +} + +void loop() { + Kaleidoscope.loop(); +} diff --git a/tests/issues/978/test.ktest b/tests/issues/978/test.ktest new file mode 100644 index 00000000..ebbad75e --- /dev/null +++ b/tests/issues/978/test.ktest @@ -0,0 +1,12 @@ +VERSION 1 + +KEYSWITCH A 0 0 + +# ============================================================================== +# This testcase shouldn't generate any HID reports. The sketch includes a plugin +# that produces `Key_X` if the key at (0,0) is idle, so if the virtual device is +# calling `handleKeyswitchEvent()` when `key_state` has neither `IS_PRESSED` nor +# `WAS_PRESSED` bits set, it will fail. +NAME Idle key detector + +RUN 3 cycles From a393f4778e09e3dcf6527c4d50c11a474024ac59 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Mon, 16 Nov 2020 01:32:48 -0600 Subject: [PATCH 3/3] Suppress idle keyswitch events from virtual hardware device The virtual hardware device, unlike others, was calling `handleKeyswitchEvent()` for every keyswitch, every cycle. It should suppress calls corresponding to idle keyswitches, just like the other devices. Signed-off-by: Michael Richters --- src/kaleidoscope/device/virtual/Virtual.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/kaleidoscope/device/virtual/Virtual.cpp b/src/kaleidoscope/device/virtual/Virtual.cpp index 387719d8..68ca3835 100644 --- a/src/kaleidoscope/device/virtual/Virtual.cpp +++ b/src/kaleidoscope/device/virtual/Virtual.cpp @@ -260,7 +260,8 @@ void VirtualKeyScanner::actOnMatrixScan() { break; } - handleKeyswitchEvent(Key_NoKey, key_addr, key_state); + if (key_state != 0) + handleKeyswitchEvent(Key_NoKey, key_addr, key_state); keystates_prev_[key_addr.toInt()] = keystates_[key_addr.toInt()]; if (keystates_[key_addr.toInt()] == KeyState::Tap) {