From ba662f6ba1fae0da28e20acd51759a6a9d2393f3 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 25 May 2022 10:59:04 -0500 Subject: [PATCH 1/4] Add testcase for Turbo sticky mode Signed-off-by: Michael Richters --- tests/plugins/Turbo/sticky/sketch.json | 6 ++ tests/plugins/Turbo/sticky/sticky.ino | 52 ++++++++++ tests/plugins/Turbo/sticky/test.ktest | 138 +++++++++++++++++++++++++ 3 files changed, 196 insertions(+) create mode 100644 tests/plugins/Turbo/sticky/sketch.json create mode 100644 tests/plugins/Turbo/sticky/sticky.ino create mode 100644 tests/plugins/Turbo/sticky/test.ktest diff --git a/tests/plugins/Turbo/sticky/sketch.json b/tests/plugins/Turbo/sticky/sketch.json new file mode 100644 index 00000000..8cc86922 --- /dev/null +++ b/tests/plugins/Turbo/sticky/sketch.json @@ -0,0 +1,6 @@ +{ + "cpu": { + "fqbn": "keyboardio:virtual:model01", + "port": "" + } +} diff --git a/tests/plugins/Turbo/sticky/sticky.ino b/tests/plugins/Turbo/sticky/sticky.ino new file mode 100644 index 00000000..2285a56c --- /dev/null +++ b/tests/plugins/Turbo/sticky/sticky.ino @@ -0,0 +1,52 @@ +/* -*- mode: c++ -*- + * Copyright (C) 2022 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 + +// *INDENT-OFF* +KEYMAPS( + [0] = KEYMAP_STACKED + ( + Key_Turbo, Key_RightGui, Key_LeftShift, ___, ___, ___, ___, + Key_A, Key_B, Key_C, Key_D, ___, ___, ___, + ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, + ___, + + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, + ___ + ), +) +// *INDENT-ON* + +KALEIDOSCOPE_INIT_PLUGINS(Turbo); + +void setup() { + Kaleidoscope.setup(); + + Turbo.interval(20); + Turbo.sticky(true); +} + +void loop() { + Kaleidoscope.loop(); +} diff --git a/tests/plugins/Turbo/sticky/test.ktest b/tests/plugins/Turbo/sticky/test.ktest new file mode 100644 index 00000000..07295450 --- /dev/null +++ b/tests/plugins/Turbo/sticky/test.ktest @@ -0,0 +1,138 @@ +VERSION 1 + +KEYSWITCH TURBO 0 0 +KEYSWITCH R_GUI 0 1 +KEYSWITCH L_SHIFT 0 2 +KEYSWITCH A 1 0 +KEYSWITCH B 1 1 +KEYSWITCH C 1 2 +KEYSWITCH D 1 3 + +# ============================================================================== +NAME Turbo no regression + +RUN 4 ms +PRESS A +RUN 1 cycle +EXPECT keyboard-report Key_A # report should contain `A` (0x04) + +RUN 4 ms +RELEASE A +RUN 1 cycle +EXPECT keyboard-report empty # report should be empty + +RUN 5 ms +EXPECT no keyboard-report # expect no more reports + +# ============================================================================== +NAME Turbo second + +RUN 4 ms +PRESS A +RUN 1 cycle +EXPECT keyboard-report Key_A # report should contain `A` (0x04) + +RUN 4 ms +PRESS TURBO +RUN 1 cycle +EXPECT keyboard-report empty # report should be empty +EXPECT keyboard-report Key_A # report should contain `A` (0x04) + +RUN 20 ms +EXPECT keyboard-report empty # report should be empty +EXPECT keyboard-report Key_A # report should contain `A` (0x04) + +RUN 10 ms +RELEASE TURBO +RUN 10 ms +EXPECT keyboard-report empty # report should be empty +EXPECT keyboard-report Key_A # report should contain `A` (0x04) + +RUN 20 ms +EXPECT keyboard-report empty # report should be empty +EXPECT keyboard-report Key_A # report should contain `A` (0x04) + +RUN 20 ms +EXPECT keyboard-report empty # report should be empty +EXPECT keyboard-report Key_A # report should contain `A` (0x04) + +RUN 9 ms +RELEASE A +RUN 1 cycle +EXPECT keyboard-report empty # report should be empty + +RUN 20 ms +EXPECT no keyboard-report + +RUN 4 ms +PRESS B +RUN 1 cycle +EXPECT keyboard-report Key_B # report should contain `B` (0x05) + +RUN 5 ms +EXPECT keyboard-report empty # report should be empty +EXPECT keyboard-report Key_B # report should contain `B` (0x05) + +RUN 20 ms +EXPECT keyboard-report empty # report should be empty +EXPECT keyboard-report Key_B # report should contain `B` (0x05) + +RUN 4 ms +PRESS TURBO +RUN 1 cycle + +RUN 4 ms +RELEASE TURBO +RUN 1 cycle + +RUN 20 ms +RELEASE B +RUN 1 cycle +EXPECT keyboard-report empty # report should be empty + +RUN 5 ms +EXPECT no keyboard-report # expect no more reports + +# ============================================================================== +NAME Turbo first + +RUN 4 ms +PRESS TURBO +RUN 1 cycle + +RUN 4 ms +RELEASE TURBO +RUN 1 cycle + +RUN 5 ms + +RUN 4 ms +PRESS A +RUN 1 cycle +EXPECT keyboard-report Key_A # report should contain `A` (0x04) + +RUN 5 ms +EXPECT keyboard-report empty # report should be empty +EXPECT keyboard-report Key_A # report should contain `A` (0x04) + +RUN 20 ms +EXPECT keyboard-report empty # report should be empty +EXPECT keyboard-report Key_A # report should contain `A` (0x04) + +RUN 4 ms +RELEASE A +RUN 1 cycle +EXPECT keyboard-report empty # report should be empty + +RUN 25 ms + +RUN 4 ms +PRESS TURBO +RUN 1 cycle + +RUN 4 ms +RELEASE TURBO +RUN 1 cycle + +RUN 5 ms +EXPECT no keyboard-report # expect no more reports From ab52a6761d50ef77a61cb5b28509ef6b2d5c0e18 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 25 May 2022 11:03:14 -0500 Subject: [PATCH 2/4] Fix Turbo sticky mode Previously, the "sticky" state was simply ignored. Now it's handled properly, leaving the "sticky" active Turbo key in the live keys array. Signed-off-by: Michael Richters --- .../src/kaleidoscope/plugin/Turbo.cpp | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/plugins/Kaleidoscope-Turbo/src/kaleidoscope/plugin/Turbo.cpp b/plugins/Kaleidoscope-Turbo/src/kaleidoscope/plugin/Turbo.cpp index ae8c9fb9..bbf67331 100644 --- a/plugins/Kaleidoscope-Turbo/src/kaleidoscope/plugin/Turbo.cpp +++ b/plugins/Kaleidoscope-Turbo/src/kaleidoscope/plugin/Turbo.cpp @@ -1,6 +1,7 @@ /* -*- mode: c++ -*- * Kaleidoscope-Turbo * Copyright (C) 2018 ash lea + * Copyright (C) 2022 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 @@ -82,22 +83,37 @@ void Turbo::activeColor(cRGB newVal) { } EventHandlerResult Turbo::onKeyEvent(KeyEvent &event) { + // If any key toggles off, reset its LED to normal. if (active_ && flash_ && keyToggledOff(event.state)) { if (event.key.isKeyboardKey()) LEDControl::refreshAt(event.addr); } + // Ignore any non-Turbo key events. if (event.key != Key_Turbo) return EventHandlerResult::OK; - if (keyToggledOn(event.state)) { - active_ = true; - start_time_ = Runtime.millisAtCycleStart() - interval_; - } else { + + if (active_) { + // If Turbo is active, and in "sticky" mode, we abort the event when a Turbo + // key toggles off, leaving it in the active keys array. This means that a + // layer change won't hide an active Turbo key. + if (sticky_ && keyToggledOff(event.state)) { + return EventHandlerResult::ABORT; + } + // If not in "sticky" mode and a Turbo key toggles off, or if in "sticky" + // mode and a Turbo key toggles on, we deactivate Turbo. active_ = false; if (flash_) LEDControl::refreshAll(); + + } else if (keyToggledOn(event.state)) { + // If Turbo is inactive, turn it on when a Turbo key is pressed. + active_ = true; + start_time_ = Runtime.millisAtCycleStart() - interval_; } + + // We assume that other plugins don't need to know about Turbo key events. return EventHandlerResult::EVENT_CONSUMED; } From 919cb39ac910c506bc62b009ca5e413f3ac23fad Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 25 May 2022 11:03:36 -0500 Subject: [PATCH 3/4] Remove unnecessarily cautious guard against Turbo getting "stuck on" I think it's reasonable to assume that other plugins won't be bad actors and remove an active Turbo key from the live keys array unceremoniously, so this check is really unnecessary. Signed-off-by: Michael Richters --- .../Kaleidoscope-Turbo/src/kaleidoscope/plugin/Turbo.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/plugins/Kaleidoscope-Turbo/src/kaleidoscope/plugin/Turbo.cpp b/plugins/Kaleidoscope-Turbo/src/kaleidoscope/plugin/Turbo.cpp index bbf67331..96cc9970 100644 --- a/plugins/Kaleidoscope-Turbo/src/kaleidoscope/plugin/Turbo.cpp +++ b/plugins/Kaleidoscope-Turbo/src/kaleidoscope/plugin/Turbo.cpp @@ -129,16 +129,9 @@ EventHandlerResult Turbo::afterEachCycle() { // Send the empty report to register the release of all the held keys. Runtime.hid().keyboard().sendReport(); - // Just in case the Turbo key has been wiped from `live_keys[]` without - // `onKeyEvent()` being called with a toggle-off: - active_ = false; - // Go through the `live_keys[]` array and add any Keyboard HID keys to the // new report. for (Key key : live_keys.all()) { - if (key == Key_Turbo) { - active_ = true; - } if (key.isKeyboardKey()) { Runtime.addToReport(key); } From 0a34e034d76705cd78e1a190932bcb4da709aaf3 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Wed, 25 May 2022 11:05:31 -0500 Subject: [PATCH 4/4] Fix Turbo docs to use `sticky()` instead of `toggle()` Signed-off-by: Michael Richters --- plugins/Kaleidoscope-Turbo/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/Kaleidoscope-Turbo/README.md b/plugins/Kaleidoscope-Turbo/README.md index cfbe1141..0c18b22e 100644 --- a/plugins/Kaleidoscope-Turbo/README.md +++ b/plugins/Kaleidoscope-Turbo/README.md @@ -24,7 +24,7 @@ void setup() { Kaleidoscope.setup(); Turbo.interval(30); - Turbo.toggle(true); + Turbo.sticky(true); Turbo.flash(true); Turbo.flashInterval(80); Turbo.activeColor(CRGB(0x64, 0x96, 0xed)); @@ -50,7 +50,6 @@ The `Turbo` object has the following user-configurable properties: ### `.sticky([bool])` > This method makes the Turbo functionality sticky, so it remains in effect not only while -> > it is held, but after it is released too, until it is toggled off with another tap. Without > arguments, the method enables the sticky functionality. Passing a boolean argument > sets stickiness to the given value.