Merge pull request #966 from gedankenexperimenter/b/macros.key-range

Standardize Macros key definitions

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,
 

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
 

src/kaleidoscope/plugin/Macros.cpp

@@ -243,11 +243,18 @@ const macro_t *Macros_::type(const char *string) {
   return MACRO_NONE;
 }
 
+bool Macros_::isMacroKey(Key key) {
+  if (key >= ranges::MACRO_FIRST && key <= ranges::MACRO_LAST)
+    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);
+  uint8_t macro_index = mappedKey.getRaw() - ranges::MACRO_FIRST;
+  addActiveMacroKey(macro_index, key_addr.toInt(), keyState);
 
   return EventHandlerResult::EVENT_CONSUMED;
 }

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);
 };
 
 }

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)

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 <http://www.gnu.org/licenses/>.
+ */
+
+#include <Kaleidoscope.h>
+#include <Kaleidoscope-Macros.h>
+
+// *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();
+}

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 <http://www.gnu.org/licenses/>.
+ */
+
+#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<uint8_t> expected_keycodes_ = {};
+  std::unique_ptr<State> 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