From d67358522425551d6966fcc792febb8d14e3cdb4 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Mon, 9 Nov 2020 13:39:56 -0600 Subject: [PATCH 1/4] Add a few simple testcases for Macros These testcases demonstrate that the macros defined in the keymap still work as they did before the change from `IS_MACRO` to using `kaleidoscope::ranges`. Signed-off-by: Michael Richters --- tests/plugins/Macros/basic/sketch.ino | 69 ++++++++++ tests/plugins/Macros/basic/test/testcase.cpp | 136 +++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 tests/plugins/Macros/basic/sketch.ino create mode 100644 tests/plugins/Macros/basic/test/testcase.cpp diff --git a/tests/plugins/Macros/basic/sketch.ino b/tests/plugins/Macros/basic/sketch.ino new file mode 100644 index 00000000..59c552ff --- /dev/null +++ b/tests/plugins/Macros/basic/sketch.ino @@ -0,0 +1,69 @@ +/* -*- 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 + +// *INDENT-OFF* +KEYMAPS( + [0] = KEYMAP_STACKED + ( + M(0), M(1), M(255), ___, ___, ___, ___, + Key_X, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + + ___, ___, ___, ___, + ___, + + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, + ___, ___, ___, ___, ___, ___, ___, + + ___, ___, ___, ___, + ___ + ), +) +// *INDENT-ON* + +const macro_t *macroAction(uint8_t index, uint8_t key_state) { + if (keyToggledOn(key_state)) { + switch (index) { + case 0: + Kaleidoscope.hid().keyboard().pressKey(Key_A); + break; + case 1: + Kaleidoscope.hid().keyboard().pressKey(Key_B); + break; + case 255: + Kaleidoscope.hid().keyboard().pressKey(Key_C); + break; + } + } + return MACRO_NONE; +} + +// Use Redial +KALEIDOSCOPE_INIT_PLUGINS(Macros); + +void setup() { + Kaleidoscope.setup(); +} + +void loop() { + Kaleidoscope.loop(); +} diff --git a/tests/plugins/Macros/basic/test/testcase.cpp b/tests/plugins/Macros/basic/test/testcase.cpp new file mode 100644 index 00000000..e23c5d52 --- /dev/null +++ b/tests/plugins/Macros/basic/test/testcase.cpp @@ -0,0 +1,136 @@ +/* -*- 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" + +SETUP_GOOGLETEST(); + +namespace kaleidoscope { +namespace testing { +namespace { + +constexpr KeyAddr addr_macro_A{0, 0}; +constexpr KeyAddr addr_macro_B{0, 1}; +constexpr KeyAddr addr_macro_C{0, 2}; +constexpr KeyAddr addr_macro_X{0, 3}; +constexpr KeyAddr addr_X{1, 0}; + +class MacrosBasic : public VirtualDeviceTest { + protected: + std::set expected_keycodes_ = {}; + std::unique_ptr state_ = nullptr; +}; + +TEST_F(MacrosBasic, MacroIndex_0) { + + sim_.Press(addr_macro_A); + state_ = RunCycle(); + expected_keycodes_.insert(Key_A.getKeyCode()); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) + << "There should be one HID report"; + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The report should include only `A`"; + + sim_.Release(addr_macro_A); + state_ = RunCycle(); + expected_keycodes_.erase(Key_A.getKeyCode()); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) + << "There should be one report after letter key release"; + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The report should be empty"; +} + +TEST_F(MacrosBasic, MacroIndex_1) { + + sim_.Press(addr_macro_B); + state_ = RunCycle(); + expected_keycodes_.insert(Key_B.getKeyCode()); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) + << "There should be one HID report"; + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The report should include only `B`"; + + sim_.Release(addr_macro_B); + state_ = RunCycle(); + expected_keycodes_.erase(Key_B.getKeyCode()); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) + << "There should be one report after letter key release"; + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The report should be empty"; +} + +TEST_F(MacrosBasic, MacroIndex_255) { + + sim_.Press(addr_macro_C); + state_ = RunCycle(); + expected_keycodes_.insert(Key_C.getKeyCode()); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) + << "There should be one HID report"; + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The report should include only `C`"; + + state_ = RunCycle(); + expected_keycodes_.erase(Key_C.getKeyCode()); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) + << "There should be one report in the next cycle"; + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The report should be empty"; + + sim_.Release(addr_macro_C); + state_ = RunCycle(); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0) + << "There should be no report after release"; +} + +TEST_F(MacrosBasic, NonMacrosKey) { + + sim_.Press(addr_X); + state_ = RunCycle(); + expected_keycodes_.insert(Key_X.getKeyCode()); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) + << "There should be one HID report"; + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The report should include only `X`"; + + sim_.Release(addr_X); + state_ = RunCycle(); + expected_keycodes_.erase(Key_X.getKeyCode()); + + ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1) + << "There should be one report after letter key release"; + EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(), + ::testing::ElementsAreArray(expected_keycodes_)) + << "The report should be empty"; +} + +} // namespace +} // namespace testing +} // namespace kaleidoscope From ef126a267efd52501e68edc01fd3a4935242180e Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Mon, 9 Nov 2020 10:30:55 -0600 Subject: [PATCH 2/4] Add Macros.isMacroKey() function This just isolates the test for a Macros plugin key to its own function. Signed-off-by: Michael Richters --- src/kaleidoscope/plugin/Macros.cpp | 8 +++++++- src/kaleidoscope/plugin/Macros.h | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/kaleidoscope/plugin/Macros.cpp b/src/kaleidoscope/plugin/Macros.cpp index 23ade38f..3b7c0a9f 100644 --- a/src/kaleidoscope/plugin/Macros.cpp +++ b/src/kaleidoscope/plugin/Macros.cpp @@ -243,8 +243,14 @@ const macro_t *Macros_::type(const char *string) { return MACRO_NONE; } +bool Macros_::isMacroKey(Key key) { + if (key.getFlags() == (SYNTHETIC | IS_MACRO)) + return true; + return false; +} + EventHandlerResult Macros_::onKeyswitchEvent(Key &mappedKey, KeyAddr key_addr, uint8_t keyState) { - if (mappedKey.getFlags() != (SYNTHETIC | IS_MACRO)) + if (! isMacroKey(mappedKey)) return EventHandlerResult::OK; addActiveMacroKey(mappedKey.getKeyCode(), key_addr.toInt(), keyState); diff --git a/src/kaleidoscope/plugin/Macros.h b/src/kaleidoscope/plugin/Macros.h index ca4aaf03..e0a3d0cf 100644 --- a/src/kaleidoscope/plugin/Macros.h +++ b/src/kaleidoscope/plugin/Macros.h @@ -80,6 +80,7 @@ class Macros_ : public kaleidoscope::Plugin { private: Key lookupAsciiCode(uint8_t ascii_code); + bool isMacroKey(Key key); }; } From ec7cdbcecbb4fc7fd3d30e26d0dc75227ee22710 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Mon, 9 Nov 2020 10:45:35 -0600 Subject: [PATCH 3/4] Standardize Macros to use Kaleidoscope-Ranges Macros was still using its own bit in the `Key.flags_` byte to define Macros keys, unlike all the other plugins that define their own special `Key` values. This standardizes Macros to make it more like other plugins. Signed-off-by: Michael Richters --- src/Kaleidoscope-Ranges.h | 2 ++ src/kaleidoscope/plugin/Macros.cpp | 5 +++-- src/kaleidoscope/plugin/Macros/MacroKeyDefs.h | 7 +++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Kaleidoscope-Ranges.h b/src/Kaleidoscope-Ranges.h index d0135173..dfa4a62b 100644 --- a/src/Kaleidoscope-Ranges.h +++ b/src/Kaleidoscope-Ranges.h @@ -49,6 +49,8 @@ enum : uint16_t { SC_LAST, REDIAL, TURBO, + MACRO_FIRST, + MACRO_LAST = MACRO_FIRST + 255, DYNAMIC_MACRO_FIRST, DYNAMIC_MACRO_LAST = DYNAMIC_MACRO_FIRST + 31, diff --git a/src/kaleidoscope/plugin/Macros.cpp b/src/kaleidoscope/plugin/Macros.cpp index 3b7c0a9f..eb83f648 100644 --- a/src/kaleidoscope/plugin/Macros.cpp +++ b/src/kaleidoscope/plugin/Macros.cpp @@ -244,7 +244,7 @@ const macro_t *Macros_::type(const char *string) { } bool Macros_::isMacroKey(Key key) { - if (key.getFlags() == (SYNTHETIC | IS_MACRO)) + if (key >= ranges::MACRO_FIRST && key <= ranges::MACRO_LAST) return true; return false; } @@ -253,7 +253,8 @@ EventHandlerResult Macros_::onKeyswitchEvent(Key &mappedKey, KeyAddr key_addr, u if (! isMacroKey(mappedKey)) return EventHandlerResult::OK; - addActiveMacroKey(mappedKey.getKeyCode(), key_addr.toInt(), keyState); + uint8_t macro_index = mappedKey.getRaw() - ranges::MACRO_FIRST; + addActiveMacroKey(macro_index, key_addr.toInt(), keyState); return EventHandlerResult::EVENT_CONSUMED; } diff --git a/src/kaleidoscope/plugin/Macros/MacroKeyDefs.h b/src/kaleidoscope/plugin/Macros/MacroKeyDefs.h index a12cd038..ede4ce7a 100644 --- a/src/kaleidoscope/plugin/Macros/MacroKeyDefs.h +++ b/src/kaleidoscope/plugin/Macros/MacroKeyDefs.h @@ -16,9 +16,12 @@ #pragma once -#define IS_MACRO B00100000 +#include "Kaleidoscope-Ranges.h" + +constexpr Key M(uint8_t n) { + return Key(kaleidoscope::ranges::MACRO_FIRST + n); +} -#define M(n) Key(n, KEY_FLAGS | SYNTHETIC | IS_MACRO) #define Key_macroKey1 M(1) #define Key_macroKey2 M(2) #define Key_macroKey3 M(3) From 3ccbd1ceeb7263090ff5d2d3526362633fcdd627 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Mon, 9 Nov 2020 13:10:38 -0600 Subject: [PATCH 4/4] Remove stale commented definition of IS_MACRO Signed-off-by: Michael Richters --- src/kaleidoscope/key_defs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kaleidoscope/key_defs.h b/src/kaleidoscope/key_defs.h index 16f88a37..1b363287 100644 --- a/src/kaleidoscope/key_defs.h +++ b/src/kaleidoscope/key_defs.h @@ -160,7 +160,7 @@ typedef kaleidoscope::Key Key_; #define RALT_HELD B00000100 #define SHIFT_HELD B00001000 #define GUI_HELD B00010000 -// #define IS_MACRO B00100000 // defined in Kaleidoscope-Macros/src/MacroKeyDefs.h + #define SYNTHETIC B01000000 #define RESERVED B10000000