diff --git a/examples/Keystrokes/AutoShift/AutoShift.ino b/examples/Keystrokes/AutoShift/AutoShift.ino index 4cc2ce88..333e9d58 100644 --- a/examples/Keystrokes/AutoShift/AutoShift.ino +++ b/examples/Keystrokes/AutoShift/AutoShift.ino @@ -46,6 +46,9 @@ const macro_t *macroAction(uint8_t macro_id, KeyEvent &event) { return MACRO_NONE; } +// This sketch uses the AutoShiftConfig plugin, which enables run-time +// configuration of AutoShift configuration settings. All of the plugins marked +// "for AutoShiftConfig" are optional; AutoShift itself will work without them. KALEIDOSCOPE_INIT_PLUGINS( EEPROMSettings, // for AutoShiftConfig EEPROMKeymap, // for AutoShiftConfig diff --git a/plugins/Kaleidoscope-AutoShift/README.md b/plugins/Kaleidoscope-AutoShift/README.md index 650c23ce..f16b72f7 100644 --- a/plugins/Kaleidoscope-AutoShift/README.md +++ b/plugins/Kaleidoscope-AutoShift/README.md @@ -24,8 +24,8 @@ the output will be unshifted. The `AutoShift` object provides three methods for turning itself on and off: -- To turn the plugin off, call `AutoShift.enable()`. -- To turn the plugin on, call `AutoShift.disable()`. +- To turn the plugin on, call `AutoShift.enable()`. +- To turn the plugin off, call `AutoShift.disable()`. - To toggle the plugin's state, call `AutoShift.toggle()`. Note: Disabling the AutoShift plugin does not affect which `Key` categories it @@ -78,6 +78,20 @@ As you can see, this method takes a `Key` as its input and returns either `true` (for keys eligible to be auto-shifted) or `false` (for keys AutoShift will leave alone). +## Plugin compatibility + +If you're using AutoShift in a sketch that also includes the Qukeys and/or +SpaceCadet plugins, make sure to register AutoShift after those other plugins in +order to prevent auto-shifts from getting clobbered. The recommended order is +as follows: + +```c++ +KALEIDOSCOPE_INIT_PLUGINS(Qukeys, SpaceCadet, AutoShift) +``` + +It's not generally recommended to use AutoShift on the same key(s) handled by +either Qukeys or SpaceCadet, as this can result in confusing behaviour. + ## Further reading Starting from the [example][plugin:example] is the recommended way of getting diff --git a/plugins/Kaleidoscope-AutoShift/src/kaleidoscope/plugin/AutoShift.cpp b/plugins/Kaleidoscope-AutoShift/src/kaleidoscope/plugin/AutoShift.cpp index 2c0c4df4..206d4c7c 100644 --- a/plugins/Kaleidoscope-AutoShift/src/kaleidoscope/plugin/AutoShift.cpp +++ b/plugins/Kaleidoscope-AutoShift/src/kaleidoscope/plugin/AutoShift.cpp @@ -137,19 +137,26 @@ EventHandlerResult AutoShift::onKeyswitchEvent(KeyEvent &event) { if (!queue_.isEmpty()) { // There's an unresolved AutoShift key press. - if (queue_.isFull()) { - flushQueue(); - } else if (event.addr == queue_.addr(0)) { + if (keyToggledOn(event.state) || + event.addr == queue_.addr(0) || + queue_.isFull()) { + // If a new key toggled on, the unresolved key toggled off (it was a + // "tap"), or if the queue is full, we clear the queue, and the key event + // does not get modified. flushEvent(false); flushQueue(); + } else { + // Otherwise, add the release event to the queue. We do this so that + // rollover from a modifier to an auto-shifted key will result in the + // modifier being applied to the key. + queue_.append(event); + return EventHandlerResult::ABORT; } - if (queue_.isEmpty()) - return EventHandlerResult::OK; - queue_.append(event); - return EventHandlerResult::ABORT; } if (keyToggledOn(event.state) && isAutoShiftable(event.key)) { + // The key is eligible to be auto-shifted, so we add it to the queue and + // defer processing of the event. queue_.append(event); return EventHandlerResult::ABORT; } diff --git a/tests/issues/1074/1074.ino b/tests/issues/1074/1074.ino new file mode 100644 index 00000000..606b09f6 --- /dev/null +++ b/tests/issues/1074/1074.ino @@ -0,0 +1,50 @@ +/* -*- mode: c++ -*- + * Copyright (C) 2021 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_LeftShift, Key_RightShift, ___, ___, ___, ___, ___, + Key_A, Key_B, Key_C, Key_D, Key_E, ___, ___, + ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, + ___, + + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, + ___ + ), +) +// *INDENT-ON* + +KALEIDOSCOPE_INIT_PLUGINS(AutoShift); + +void setup() { + Kaleidoscope.setup(); + AutoShift.setTimeout(20); +} + +void loop() { + Kaleidoscope.loop(); +} diff --git a/tests/issues/1074/sketch.json b/tests/issues/1074/sketch.json new file mode 100644 index 00000000..8cc86922 --- /dev/null +++ b/tests/issues/1074/sketch.json @@ -0,0 +1,6 @@ +{ + "cpu": { + "fqbn": "keyboardio:virtual:model01", + "port": "" + } +} diff --git a/tests/issues/1074/test.ktest b/tests/issues/1074/test.ktest new file mode 100644 index 00000000..ff4608e5 --- /dev/null +++ b/tests/issues/1074/test.ktest @@ -0,0 +1,54 @@ +VERSION 1 + +KEYSWITCH LSHIFT 0 0 +KEYSWITCH RSHIFT 0 1 +KEYSWITCH A 1 0 +KEYSWITCH B 1 1 +KEYSWITCH C 1 2 +KEYSWITCH D 1 3 +KEYSWITCH E 1 4 + +# ============================================================================== +NAME AutoShift overflow + +RUN 4 ms +PRESS A +RUN 1 cycle + +RUN 4 ms +PRESS B +RUN 1 cycle +EXPECT keyboard-report Key_A # report: { 4 } + +RUN 4 ms +PRESS C +RUN 1 cycle +EXPECT keyboard-report Key_A Key_B # report: { 4 5 } + +RUN 4 ms +PRESS D +RUN 1 cycle +EXPECT keyboard-report Key_A Key_B Key_C # report: { 4 5 6 } + +RUN 4 ms +RELEASE A +RUN 1 cycle + +RUN 4 ms +RELEASE B +RUN 1 cycle + +RUN 4 ms +RELEASE C +RUN 1 cycle + +RUN 4 ms +RELEASE D +RUN 1 cycle +EXPECT keyboard-report Key_A Key_B Key_C Key_D # report: { 4 5 6 7 } +EXPECT keyboard-report Key_B Key_C Key_D # report: { 5 6 7 } +EXPECT keyboard-report Key_C Key_D # report: { 6 7 } +EXPECT keyboard-report Key_D # report: { 7 } +EXPECT keyboard-report empty + +RUN 5 ms