From 3976e3dd5bb18376389dccbb22798e6ac9a57e6f Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Wed, 25 May 2022 19:26:17 -0500
Subject: [PATCH 1/4] Add MacroSupport plugin

This plugin provides a shared virtual keys array used by Macros and
DynamicMacros, along with some functions to interact with it (`press()`,
`release()`, `tap()`, `clear()`).

Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
---
 plugins/Kaleidoscope-MacroSupport/README.md   |  51 +++++++++
 .../library.properties                        |   7 ++
 .../src/Kaleidoscope-MacroSupport.h           |  19 ++++
 .../src/kaleidoscope/plugin/MacroSupport.cpp  | 105 ++++++++++++++++++
 .../src/kaleidoscope/plugin/MacroSupport.h    |  78 +++++++++++++
 5 files changed, 260 insertions(+)
 create mode 100644 plugins/Kaleidoscope-MacroSupport/README.md
 create mode 100644 plugins/Kaleidoscope-MacroSupport/library.properties
 create mode 100644 plugins/Kaleidoscope-MacroSupport/src/Kaleidoscope-MacroSupport.h
 create mode 100644 plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.cpp
 create mode 100644 plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.h

diff --git a/plugins/Kaleidoscope-MacroSupport/README.md b/plugins/Kaleidoscope-MacroSupport/README.md
new file mode 100644
index 00000000..bd285c26
--- /dev/null
+++ b/plugins/Kaleidoscope-MacroSupport/README.md
@@ -0,0 +1,51 @@
+# MacroSupport
+
+This plugin provides the supplemental key array used by the Macros and DynamicMacros plugins, and is necessary for the proper functioning of those plugins.
+
+## Using the plugin
+
+Any firmware sketch that uses either Macros or DynamicMacros automatically includes this plugin, so there's no need to add it explicitly.  If your sketch doesn't require either type of Macros key, however, you can still make use of the MacroSupport plugin's helper methods (`tap()`, `press()`, _et al_).  In that case, you should include the MacroSupport header file, and include it in `KALEIDOSCOPE_INIT_PLUGINS()`:
+
+```c++
+#include <Kaleidoscope.h>
+#include <Kaleidoscope-MacroSupport.h>
+
+// Other plugin code that calls `MacroSupport.press()` (for example)
+
+KALEIDOSCOPE_INIT_PLUGINS(
+  MacroSupport,
+  // Other plugin(s) that make use of MacroSupport
+);
+
+void setup() {
+  Kaleidoscope.setup ();
+}
+```
+
+## Plugin methods
+
+The plugin provides a `MacroSupport` object, which contains a supplemental array of virtual keys that it adds to USB Keyboard reports.  Other plugins and user code can interact with it via the following methods:
+
+### `.press(key)`
+
+> Sends a key press event for `key`, and will keep that virtual key active in
+> the supplemental virtual keys array.
+
+### `.release(key)`
+
+> Sends a key release event for `key`, and removes it from the supplemental
+> virtual keys array.
+
+### `.clear()`
+
+> Releases all active virtual keys held by MacroSupport.  This both empties the
+> supplemental keys array (see above) and sends a release event for each key
+> stored there.
+
+### `.tap(key)`
+
+> Sends an immediate press and release event for `key` with no delay, using an
+> invalid key address.  This method doesn't actually use the supplemental keys
+> array, but is provided here for convenience and simplicity.
+
+It is not necessary to use either the Macros (or DynamicMacros) to make use of MacroSupport.  When using it with custom code, however, please remember that the supplemental active keys array it provides will be shared by all clients (e.g. Macros, user-defined Leader or TapDance functions), so if you want more than one of those clients to be active simultaneously, be aware that calles to `MacroSupport.clear()` will affect all of them, not just the caller.
diff --git a/plugins/Kaleidoscope-MacroSupport/library.properties b/plugins/Kaleidoscope-MacroSupport/library.properties
new file mode 100644
index 00000000..68837366
--- /dev/null
+++ b/plugins/Kaleidoscope-MacroSupport/library.properties
@@ -0,0 +1,7 @@
+name=Kaleidoscope-MacroSupport
+version=0.0.0
+sentence=Macro keys for Kaleidoscope
+maintainer=Kaleidoscope's Developers <jesse@keyboard.io>
+url=https://github.com/keyboardio/Kaleidoscope
+author=Gergely Nagy, Jesse Vincent, Michael Richters
+paragraph=
diff --git a/plugins/Kaleidoscope-MacroSupport/src/Kaleidoscope-MacroSupport.h b/plugins/Kaleidoscope-MacroSupport/src/Kaleidoscope-MacroSupport.h
new file mode 100644
index 00000000..3d473086
--- /dev/null
+++ b/plugins/Kaleidoscope-MacroSupport/src/Kaleidoscope-MacroSupport.h
@@ -0,0 +1,19 @@
+/* Kaleidoscope-MacroSupport -- Macro keys for Kaleidoscope
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "kaleidoscope/plugin/MacroSupport.h"  // IWYU pragma: export
diff --git a/plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.cpp b/plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.cpp
new file mode 100644
index 00000000..bd453804
--- /dev/null
+++ b/plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.cpp
@@ -0,0 +1,105 @@
+/* Kaleidoscope-MacroSupport - Macros support functions for Kaleidoscope
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include "kaleidoscope/plugin/MacroSupport.h"
+
+#include <Arduino.h>                   // for pgm_read_byte, delay, F, PROGMEM, __F...
+#include <Kaleidoscope-FocusSerial.h>  // for Focus, FocusSerial
+#include <stdint.h>                    // for uint8_t
+
+#include "kaleidoscope/KeyAddr.h"               // for KeyAddr
+#include "kaleidoscope/KeyEvent.h"              // for KeyEvent
+#include "kaleidoscope/Runtime.h"               // for Runtime, Runtime_
+#include "kaleidoscope/event_handler_result.h"  // for EventHandlerResult, EventHandlerResul...
+#include "kaleidoscope/key_defs.h"              // for Key, LSHIFT, Key_NoKey, Key_0, Key_1
+#include "kaleidoscope/keyswitch_state.h"       // for INJECTED, IS_PRESSED, WAS_PRESSED
+
+// =============================================================================
+// `Macros` plugin code
+namespace kaleidoscope {
+namespace plugin {
+
+constexpr uint8_t press_state   = IS_PRESSED | INJECTED;
+constexpr uint8_t release_state = WAS_PRESSED | INJECTED;
+
+// -----------------------------------------------------------------------------
+// Public helper functions
+
+void MacroSupport::press(Key key) {
+  Runtime.handleKeyEvent(KeyEvent{KeyAddr::none(), press_state, key});
+  // This key may remain active for several subsequent events, so we need to
+  // store it in the active macro keys array.
+  for (Key &macro_key : active_macro_keys_) {
+    if (macro_key == Key_NoKey) {
+      macro_key = key;
+      break;
+    }
+  }
+}
+
+void MacroSupport::release(Key key) {
+  // Before sending the release event, we need to remove the key from the active
+  // macro keys array, or it will get inserted into the report anyway.
+  for (Key &macro_key : active_macro_keys_) {
+    if (macro_key == key) {
+      macro_key = Key_NoKey;
+    }
+  }
+  Runtime.handleKeyEvent(KeyEvent{KeyAddr::none(), release_state, key});
+}
+
+void MacroSupport::clear() {
+  // Clear the active macro keys array.
+  for (Key &macro_key : active_macro_keys_) {
+    if (macro_key == Key_NoKey)
+      continue;
+    Runtime.handleKeyEvent(KeyEvent{KeyAddr::none(), release_state, macro_key});
+    macro_key = Key_NoKey;
+  }
+}
+
+void MacroSupport::tap(Key key) const {
+  // No need to call `press()` & `release()`, because we're immediately
+  // releasing the key after pressing it. It is possible for some other plugin
+  // to insert an event in between, but very unlikely.
+  Runtime.handleKeyEvent(KeyEvent{KeyAddr::none(), press_state, key});
+  Runtime.handleKeyEvent(KeyEvent{KeyAddr::none(), release_state, key});
+}
+
+// -----------------------------------------------------------------------------
+// Event handlers
+
+EventHandlerResult MacroSupport::beforeReportingState(const KeyEvent &event) {
+  // Do this in beforeReportingState(), instead of `onAddToReport()` because
+  // `live_keys` won't get updated until after the macro sequence is played from
+  // the keypress. This could be changed by either updating `live_keys` manually
+  // ahead of time, or by executing the macro sequence on key release instead of
+  // key press. This is probably the simplest solution.
+  for (Key key : active_macro_keys_) {
+    if (key != Key_NoKey)
+      Runtime.addToReport(key);
+  }
+  return EventHandlerResult::OK;
+}
+
+EventHandlerResult MacroSupport::onNameQuery() {
+  return ::Focus.sendName(F("MacroSupport"));
+}
+
+}  // namespace plugin
+}  // namespace kaleidoscope
+
+kaleidoscope::plugin::MacroSupport MacroSupport;
diff --git a/plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.h b/plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.h
new file mode 100644
index 00000000..8358e37b
--- /dev/null
+++ b/plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.h
@@ -0,0 +1,78 @@
+/* Kaleidoscope-MacroSupport - Macros support functions for Kaleidoscope
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "kaleidoscope/KeyEvent.h"              // for KeyEvent
+#include "kaleidoscope/event_handler_result.h"  // for EventHandlerResult
+#include "kaleidoscope/key_defs.h"              // for Key
+#include "kaleidoscope/plugin.h"                // for Plugin
+
+// =============================================================================
+// The number of simultaneously-active `Key` values that a macro can have
+// running during a call to `Macros.play()`. I don't know if it's actually
+// possible to override this by defining it in a sketch before including
+// "Kaleidoscope-Macros.h", but probably not.
+#if !defined(MAX_CONCURRENT_MACRO_KEYS)
+#define MAX_CONCURRENT_MACRO_KEYS 8
+#endif
+
+namespace kaleidoscope {
+namespace plugin {
+
+class MacroSupport : public Plugin {
+ public:
+  /// Send a key press event from a Macro
+  ///
+  /// Generates a new `KeyEvent` and calls `Runtime.handleKeyEvent()` with the
+  /// specified `key`, then stores that `key` in an array of active macro key
+  /// values. This allows the macro to press one key and keep it active when a
+  /// subsequent key event is sent as part of the same macro sequence.
+  void press(Key key);
+
+  /// Send a key release event from a Macro
+  ///
+  /// Generates a new `KeyEvent` and calls `Runtime.handleKeyEvent()` with the
+  /// specified `key`, then removes that key from the array of active macro
+  /// keys (see `Macros.press()`).
+  void release(Key key);
+
+  /// Clear all virtual keys held by Macros
+  ///
+  /// This function clears the active macro keys array, sending a release event
+  /// for each key stored there.
+  void clear();
+
+  /// Send a key "tap event" from a Macro
+  ///
+  /// Generates two new `KeyEvent` objects, one each to press and release the
+  /// specified `key`, passing both in sequence to `Runtime.handleKeyEvent()`.
+  void tap(Key key) const;
+
+  // ---------------------------------------------------------------------------
+  // Event handlers
+  EventHandlerResult onNameQuery();
+  EventHandlerResult beforeReportingState(const KeyEvent &event);
+
+ private:
+  // An array of key values that are active while a macro sequence is playing
+  Key active_macro_keys_[MAX_CONCURRENT_MACRO_KEYS];
+};
+
+}  // namespace plugin
+}  // namespace kaleidoscope
+
+extern kaleidoscope::plugin::MacroSupport MacroSupport;

From 054dc9beb34858a75d6e39c21cbc79d650fb3f2c Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Wed, 25 May 2022 19:28:41 -0500
Subject: [PATCH 2/4] Convert Macros and DynamicMacros plugins to use
 MacroSupport plugin

Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
---
 plugins/Kaleidoscope-DynamicMacros/README.md  |  7 +-
 .../src/kaleidoscope/plugin/DynamicMacros.cpp | 42 +-------
 .../src/kaleidoscope/plugin/DynamicMacros.h   | 18 ++--
 plugins/Kaleidoscope-Macros/README.md         | 12 ++-
 .../src/kaleidoscope/plugin/Macros.cpp        | 96 ++++---------------
 .../src/kaleidoscope/plugin/Macros.h          | 36 +++----
 6 files changed, 58 insertions(+), 153 deletions(-)

diff --git a/plugins/Kaleidoscope-DynamicMacros/README.md b/plugins/Kaleidoscope-DynamicMacros/README.md
index ec667bab..b0a5eb80 100644
--- a/plugins/Kaleidoscope-DynamicMacros/README.md
+++ b/plugins/Kaleidoscope-DynamicMacros/README.md
@@ -19,9 +19,9 @@ except the amount of storage available on the keyboard.
 
 ## Using the plugin
 
-To use the plugin, we need to include the header, tell the firmware to `use` the
-plugin, and reserve storage space for the macros. This is best illustrated with
-an example:
+To use the plugin, we need to include the header, initialize the plugin with
+`KALEIDOSCOPE_INIT_PLUGINS()`, and reserve storage space for the macros. This is
+best illustrated with an example:
 
 ```c++
 #include <Kaleidoscope.h>
@@ -96,5 +96,6 @@ The plugin provides two Focus commands: `macros.map` and `macros.trigger`.
 
 ## Dependencies
 
+* [Kaleidoscope-MacroSupport](Kaleidoscope-MacroSupport.md)
 * [Kaleidoscope-EEPROM-Settings](Kaleidoscope-EEPROM-Settings.md)
 * [Kaleidoscope-FocusSerial](Kaleidoscope-FocusSerial.md)
diff --git a/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.cpp b/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.cpp
index d06a56fd..6fee6637 100644
--- a/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.cpp
+++ b/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.cpp
@@ -20,7 +20,6 @@
 #include <Kaleidoscope-FocusSerial.h>  // for Focus, FocusSerial
 #include <Kaleidoscope-Ranges.h>       // for DYNAMIC_MACRO_FIRST, DYNAMIC_MACRO_LAST
 
-#include "kaleidoscope/KeyAddr.h"                 // for KeyAddr
 #include "kaleidoscope/KeyEvent.h"                // for KeyEvent
 #include "kaleidoscope/Runtime.h"                 // for Runtime, Runtime_
 #include "kaleidoscope/device/device.h"           // for VirtualProps::Storage, Base<>::Storage
@@ -35,31 +34,6 @@ namespace kaleidoscope {
 namespace plugin {
 
 // =============================================================================
-// It might be possible to use Macros instead of reproducing it
-void DynamicMacros::press(Key key) {
-  Runtime.handleKeyEvent(KeyEvent(KeyAddr::none(), IS_PRESSED | INJECTED, key));
-  for (Key &mkey : active_macro_keys_) {
-    if (mkey == Key_NoKey) {
-      mkey = key;
-      break;
-    }
-  }
-}
-
-void DynamicMacros::release(Key key) {
-  for (Key &mkey : active_macro_keys_) {
-    if (mkey == key) {
-      mkey = Key_NoKey;
-    }
-  }
-  Runtime.handleKeyEvent(KeyEvent(KeyAddr::none(), WAS_PRESSED | INJECTED, key));
-}
-
-void DynamicMacros::tap(Key key) {
-  Runtime.handleKeyEvent(KeyEvent(KeyAddr::none(), IS_PRESSED | INJECTED, key));
-  Runtime.handleKeyEvent(KeyEvent(KeyAddr::none(), WAS_PRESSED | INJECTED, key));
-}
-
 uint8_t DynamicMacros::updateDynamicMacroCache() {
   uint16_t pos       = storage_base_;
   uint8_t current_id = 0;
@@ -222,6 +196,7 @@ bool isDynamicMacrosKey(Key key) {
 
 // -----------------------------------------------------------------------------
 EventHandlerResult DynamicMacros::onKeyEvent(KeyEvent &event) {
+  // Ignore everything except DynamicMacros keys
   if (!isDynamicMacrosKey(event.key))
     return EventHandlerResult::OK;
 
@@ -229,25 +204,12 @@ EventHandlerResult DynamicMacros::onKeyEvent(KeyEvent &event) {
     uint8_t macro_id = event.key.getRaw() - ranges::DYNAMIC_MACRO_FIRST;
     play(macro_id);
   } else {
-    for (Key key : active_macro_keys_) {
-      release(key);
-    }
+    clear();
   }
 
   return EventHandlerResult::EVENT_CONSUMED;
 }
 
-EventHandlerResult DynamicMacros::beforeReportingState(const KeyEvent &event) {
-  // Here we add keycodes to the HID report for keys held in a macro sequence.
-  // This is necessary because Kaleidoscope doesn't know about the supplemental
-  // `active_macro_keys_[]` array.
-  for (Key key : active_macro_keys_) {
-    if (key != Key_NoKey)
-      Runtime.addToReport(key);
-  }
-  return EventHandlerResult::OK;
-}
-
 EventHandlerResult DynamicMacros::onNameQuery() {
   return ::Focus.sendName(F("DynamicMacros"));
 }
diff --git a/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.h b/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.h
index 29e01a9c..e4767761 100644
--- a/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.h
+++ b/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.h
@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <Kaleidoscope-MacroSupport.h>
 #include <Kaleidoscope-Ranges.h>  // for DYNAMIC_MACRO_FIRST
 #include <stdint.h>               // for uint16_t, uint8_t
 
@@ -24,9 +25,7 @@
 #include "kaleidoscope/key_defs.h"              // for Key
 #include "kaleidoscope/plugin.h"                // for Plugin
 
-#define DM(n)                             ::kaleidoscope::plugin::DynamicMacrosKey(n)
-
-#define MAX_CONCURRENT_DYNAMIC_MACRO_KEYS 8
+#define DM(n) ::kaleidoscope::plugin::DynamicMacrosKey(n)
 
 namespace kaleidoscope {
 namespace plugin {
@@ -39,8 +38,10 @@ class DynamicMacros : public kaleidoscope::Plugin {
  public:
   EventHandlerResult onNameQuery();
   EventHandlerResult onKeyEvent(KeyEvent &event);
-  EventHandlerResult beforeReportingState(const KeyEvent &event);
   EventHandlerResult onFocusEvent(const char *command);
+  EventHandlerResult beforeReportingState(const KeyEvent &event) {
+    return ::MacroSupport.beforeReportingState(event);
+  }
 
   void reserve_storage(uint16_t size);
 
@@ -52,10 +53,11 @@ class DynamicMacros : public kaleidoscope::Plugin {
   uint16_t map_[32];
   uint8_t macro_count_;
   uint8_t updateDynamicMacroCache();
-  Key active_macro_keys_[MAX_CONCURRENT_DYNAMIC_MACRO_KEYS];
-  void press(Key key);
-  void release(Key key);
-  void tap(Key key);
+
+  inline void press(Key key) { ::MacroSupport.press(key); }
+  inline void release(Key key) { ::MacroSupport.release(key); }
+  inline void tap(Key key) const { ::MacroSupport.tap(key); }
+  inline void clear() { ::MacroSupport.clear(); }
 };
 
 }  // namespace plugin
diff --git a/plugins/Kaleidoscope-Macros/README.md b/plugins/Kaleidoscope-Macros/README.md
index 39c2062e..5249cb0d 100644
--- a/plugins/Kaleidoscope-Macros/README.md
+++ b/plugins/Kaleidoscope-Macros/README.md
@@ -14,10 +14,10 @@ In Kaleidoscope, macros are implemented via this plugin. You can define upto 256
 
 ## Using the plugin
 
-To use the plugin, we need to include the header, tell the firmware to `use` the
-plugin, place macros on the keymap, and create a special handler function
-(`macroAction`) that will tell the plugin what shall happen when macro keys are
-pressed. It is best illustrated with an example:
+To use the plugin, we need to include the header, initialize the plugins with
+`KALEIDOSCOPE_INIT_PLUGINS()`, place macros on the keymap, and create a special
+handler function (`macroAction()`) that will determine what happens when macro
+keys are pressed. It is best illustrated with an example:
 
 ```c++
 #include <Kaleidoscope.h>
@@ -210,3 +210,7 @@ Due to technical and practical reasons, `Macros.type()` assumes a QWERTY layout
 on the host side, and so do all other parts that work with keycodes. If your
 operating system is set to a different layout, the strings and keycodes will
 need to be adjusted accordingly.
+
+## Dependencies
+
+* [Kaleidoscope-MacroSupport](Kaleidoscope-MacroSupport.md)
diff --git a/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.cpp b/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.cpp
index 9b33c370..8f63a29a 100644
--- a/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.cpp
+++ b/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.cpp
@@ -1,5 +1,5 @@
 /* Kaleidoscope-Macros - Macro keys for Kaleidoscope.
- * Copyright (C) 2017-2021  Keyboard.io, Inc.
+ * Copyright (C) 2017-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
@@ -21,9 +21,7 @@
 #include <Kaleidoscope-Ranges.h>       // for MACRO_FIRST
 #include <stdint.h>                    // for uint8_t
 
-#include "kaleidoscope/KeyAddr.h"                   // for KeyAddr
 #include "kaleidoscope/KeyEvent.h"                  // for KeyEvent
-#include "kaleidoscope/Runtime.h"                   // for Runtime, Runtime_
 #include "kaleidoscope/event_handler_result.h"      // for EventHandlerResult, EventHandlerResul...
 #include "kaleidoscope/key_defs.h"                  // for Key, LSHIFT, Key_NoKey, Key_0, Key_1
 #include "kaleidoscope/keyswitch_state.h"           // for INJECTED, IS_PRESSED, WAS_PRESSED
@@ -42,56 +40,9 @@ macroAction(uint8_t macro_id, KeyEvent &event) {
 namespace kaleidoscope {
 namespace plugin {
 
-constexpr uint8_t press_state   = IS_PRESSED | INJECTED;
-constexpr uint8_t release_state = WAS_PRESSED | INJECTED;
-
-// Initialized to zeroes (i.e. `Key_NoKey`)
-Key Macros::active_macro_keys_[];
-
 // -----------------------------------------------------------------------------
 // Public helper functions
 
-void Macros::press(Key key) {
-  Runtime.handleKeyEvent(KeyEvent{KeyAddr::none(), press_state, key});
-  // This key may remain active for several subsequent events, so we need to
-  // store it in the active macro keys array.
-  for (Key &macro_key : active_macro_keys_) {
-    if (macro_key == Key_NoKey) {
-      macro_key = key;
-      break;
-    }
-  }
-}
-
-void Macros::release(Key key) {
-  // Before sending the release event, we need to remove the key from the active
-  // macro keys array, or it will get inserted into the report anyway.
-  for (Key &macro_key : active_macro_keys_) {
-    if (macro_key == key) {
-      macro_key = Key_NoKey;
-    }
-  }
-  Runtime.handleKeyEvent(KeyEvent{KeyAddr::none(), release_state, key});
-}
-
-void Macros::clear() {
-  // Clear the active macro keys array.
-  for (Key &macro_key : active_macro_keys_) {
-    if (macro_key == Key_NoKey)
-      continue;
-    Runtime.handleKeyEvent(KeyEvent{KeyAddr::none(), release_state, macro_key});
-    macro_key = Key_NoKey;
-  }
-}
-
-void Macros::tap(Key key) const {
-  // No need to call `press()` & `release()`, because we're immediately
-  // releasing the key after pressing it. It is possible for some other plugin
-  // to insert an event in between, but very unlikely.
-  Runtime.handleKeyEvent(KeyEvent{KeyAddr::none(), press_state, key});
-  Runtime.handleKeyEvent(KeyEvent{KeyAddr::none(), release_state, key});
-}
-
 void Macros::play(const macro_t *macro_p) {
   macro_t macro    = MACRO_ACTION_END;
   uint8_t interval = 0;
@@ -307,39 +258,24 @@ EventHandlerResult Macros::onKeyEvent(KeyEvent &event) {
   play(macro_ptr);
 
   if (keyToggledOff(event.state) || !isMacrosKey(event.key)) {
-    // If we toggled off or if the value of `event.key` has changed, send
-    // release events for any active macro keys. Simply clearing
-    // `active_macro_keys_` might be sufficient, but it's probably better to
-    // send the toggle off events so that other plugins get a chance to act on
-    // them.
+    // If a Macros key toggled off or if the value of `event.key` has been
+    // changed by the user-defined `macroAction()` function, we clear the array
+    // of active macro keys so that they won't get "stuck on".  There won't be a
+    // subsequent event that Macros will recognize as actionable, so we need to
+    // do it here.
     clear();
-
-    // Return `OK` to let Kaleidoscope finish processing this event as
-    // normal. This is so that, if the user-defined `macroAction(id, &event)`
-    // function changes the value of `event.key`, it will take effect properly.
-    return EventHandlerResult::OK;
   }
 
-  // No other plugin should be handling Macros keys, and there's nothing more
-  // for Kaleidoscope to do with a key press of a Macros key, so we return
-  // `EVENT_CONSUMED`, causing Kaleidoscope to update `live_keys[]` correctly,
-  // ensuring that the above block will clear `active_macro_keys_` on release,
-  // but not allowing any other plugins to change the `event.key` value, which
-  // would interfere.
-  //return EventHandlerResult::EVENT_CONSUMED;
-  return EventHandlerResult::OK;
-}
-
-EventHandlerResult Macros::beforeReportingState(const KeyEvent &event) {
-  // Do this in beforeReportingState(), instead of `onAddToReport()` because
-  // `live_keys` won't get updated until after the macro sequence is played from
-  // the keypress. This could be changed by either updating `live_keys` manually
-  // ahead of time, or by executing the macro sequence on key release instead of
-  // key press. This is probably the simplest solution.
-  for (Key key : active_macro_keys_) {
-    if (key != Key_NoKey)
-      Runtime.addToReport(key);
-  }
+  // Return `OK` to let Kaleidoscope finish processing this event as normal.
+  // This is so that, if the user-defined `macroAction(id, &event)` function
+  // changes the value of `event.key`, it will take effect properly.  Note that
+  // we're counting on other plugins to not subsequently change the value of
+  // `event.key` if a Macros key has toggled on, because that would leave any
+  // keys in the supplemental array "stuck on".  We could return
+  // `EVENT_CONSUMED` if `event.key` is still a Macros key, but that would lead
+  // to other undesirable plugin interactions (e.g. OneShot keys wouldn't be
+  // triggered to turn off when a Macros key toggles on, assuming that Macros
+  // comes first in `KALEIDOSCOPE_INIT_PLUGINS()`).
   return EventHandlerResult::OK;
 }
 
diff --git a/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.h b/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.h
index a85573f4..0de51309 100644
--- a/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.h
+++ b/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.h
@@ -1,5 +1,5 @@
 /* Kaleidoscope-Macros - Macro keys for Kaleidoscope.
- * Copyright (C) 2017-2021  Keyboard.io, Inc.
+ * Copyright (C) 2017-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
@@ -18,7 +18,8 @@
 
 #include <stdint.h>  // for uint8_t
 
-#include "Kaleidoscope-Ranges.h"                    // for MACRO_FIRST, MACRO_LAST
+#include <Kaleidoscope-MacroSupport.h>
+#include <Kaleidoscope-Ranges.h>                    // for MACRO_FIRST, MACRO_LAST
 #include "kaleidoscope/KeyEvent.h"                  // for KeyEvent
 #include "kaleidoscope/event_handler_result.h"      // for EventHandlerResult
 #include "kaleidoscope/key_defs.h"                  // for Key
@@ -29,14 +30,6 @@
 // Define this function in a Kaleidoscope sketch in order to trigger Macros.
 const macro_t *macroAction(uint8_t macro_id, KeyEvent &event);
 
-// The number of simultaneously-active `Key` values that a macro can have
-// running during a call to `Macros.play()`. I don't know if it's actually
-// possible to override this by defining it in a sketch before including
-// "Kaleidoscope-Macros.h", but probably not.
-#if !defined(MAX_CONCURRENT_MACRO_KEYS)
-#define MAX_CONCURRENT_MACRO_KEYS 8
-#endif
-
 namespace kaleidoscope {
 namespace plugin {
 
@@ -48,26 +41,34 @@ class Macros : public kaleidoscope::Plugin {
   /// specified `key`, then stores that `key` in an array of active macro key
   /// values. This allows the macro to press one key and keep it active when a
   /// subsequent key event is sent as part of the same macro sequence.
-  void press(Key key);
+  inline void press(Key key) {
+    ::MacroSupport.press(key);
+  }
 
   /// Send a key release event from a Macro
   ///
   /// Generates a new `KeyEvent` and calls `Runtime.handleKeyEvent()` with the
   /// specified `key`, then removes that key from the array of active macro
   /// keys (see `Macros.press()`).
-  void release(Key key);
+  inline void release(Key key) {
+    ::MacroSupport.release(key);
+  }
 
   /// Clear all virtual keys held by Macros
   ///
   /// This function clears the active macro keys array, sending a release event
   /// for each key stored there.
-  void clear();
+  inline void clear() {
+    ::MacroSupport.clear();
+  }
 
   /// Send a key "tap event" from a Macro
   ///
   /// Generates two new `KeyEvent` objects, one each to press and release the
   /// specified `key`, passing both in sequence to `Runtime.handleKeyEvent()`.
-  void tap(Key key) const;
+  inline void tap(Key key) const {
+    ::MacroSupport.tap(key);
+  }
 
   /// Play a macro sequence of key events
   void play(const macro_t *macro_ptr);
@@ -89,12 +90,11 @@ class Macros : public kaleidoscope::Plugin {
   // Event handlers
   EventHandlerResult onNameQuery();
   EventHandlerResult onKeyEvent(KeyEvent &event);
-  EventHandlerResult beforeReportingState(const KeyEvent &event);
+  EventHandlerResult beforeReportingState(const KeyEvent &event) {
+    return ::MacroSupport.beforeReportingState(event);
+  }
 
  private:
-  // An array of key values that are active while a macro sequence is playing
-  static Key active_macro_keys_[MAX_CONCURRENT_MACRO_KEYS];
-
   // Translate and ASCII character value to a corresponding `Key`
   Key lookupAsciiCode(uint8_t ascii_code) const;
 

From 7373882c88ab74a3397b956c67868b8a2661f0de Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Sun, 29 May 2022 13:30:33 -0500
Subject: [PATCH 3/4] Update examples and testcases for MacroSupport

Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
---
 examples/Keystrokes/LeaderPrefix/LeaderPrefix.ino |  4 ++--
 tests/issues/1042/1042.ino                        | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/examples/Keystrokes/LeaderPrefix/LeaderPrefix.ino b/examples/Keystrokes/LeaderPrefix/LeaderPrefix.ino
index 14ee4a33..b01144dc 100644
--- a/examples/Keystrokes/LeaderPrefix/LeaderPrefix.ino
+++ b/examples/Keystrokes/LeaderPrefix/LeaderPrefix.ino
@@ -17,7 +17,7 @@
 
 #include <Kaleidoscope.h>
 #include <Kaleidoscope-Leader.h>
-#include <Kaleidoscope-Macros.h>
+#include <Kaleidoscope-MacroSupport.h>
 
 #include <Kaleidoscope-Ranges.h>
 #include "kaleidoscope/KeyEventTracker.h"
@@ -176,7 +176,7 @@ void leaderTestPrefix(uint8_t seq_index) {
   uint8_t prefix_arg = LeaderPrefix.arg();
   // Use a Macros helper function to tap the `X` key repeatedly.
   while (prefix_arg-- > 0)
-    Macros.tap(Key_X);
+    MacroSupport.tap(Key_X);
 }
 
 static const kaleidoscope::plugin::Leader::dictionary_t leader_dictionary[] PROGMEM =
diff --git a/tests/issues/1042/1042.ino b/tests/issues/1042/1042.ino
index 9d961a20..5afb2dd2 100644
--- a/tests/issues/1042/1042.ino
+++ b/tests/issues/1042/1042.ino
@@ -17,7 +17,7 @@
 
 #include <Kaleidoscope.h>
 #include <Kaleidoscope-Leader.h>
-#include <Kaleidoscope-Macros.h>
+#include <Kaleidoscope-MacroSupport.h>
 
 #include <Kaleidoscope-Ranges.h>
 #include "kaleidoscope/KeyEventTracker.h"
@@ -114,19 +114,19 @@ kaleidoscope::plugin::LeaderPrefix LeaderPrefix;
 auto &serial_port = Kaleidoscope.serialPort();
 
 static void leaderTestX(uint8_t seq_index) {
-  Macros.tap(Key_A);
+  MacroSupport.tap(Key_A);
 }
 
 static void leaderTestXX(uint8_t seq_index) {
-  Macros.tap(Key_B);
+  MacroSupport.tap(Key_B);
 }
 
 void leaderTestPrefix(uint8_t seq_index) {
   uint8_t prefix_arg = LeaderPrefix.arg();
   // DUMP(prefix_arg);
-  Macros.tap(Key_Y);
+  MacroSupport.tap(Key_Y);
   while (prefix_arg-- > 0)
-    Macros.tap(Key_X);
+    MacroSupport.tap(Key_X);
 }
 
 static const kaleidoscope::plugin::Leader::dictionary_t leader_dictionary[] PROGMEM =

From 0f27253f905a84882932979e02d8770b94d83a0b Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Sun, 29 May 2022 14:06:38 -0500
Subject: [PATCH 4/4] Run IWYU on Macros, DynamicMacros, and MacroSupport code

Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
---
 .../src/kaleidoscope/plugin/DynamicMacros.cpp               | 2 +-
 .../src/kaleidoscope/plugin/DynamicMacros.h                 | 6 +++---
 .../src/kaleidoscope/plugin/MacroSupport.cpp                | 6 +++---
 .../Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.cpp  | 2 +-
 .../Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.h    | 6 +++---
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.cpp b/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.cpp
index 6fee6637..6efc813a 100644
--- a/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.cpp
+++ b/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.cpp
@@ -23,7 +23,7 @@
 #include "kaleidoscope/KeyEvent.h"                // for KeyEvent
 #include "kaleidoscope/Runtime.h"                 // for Runtime, Runtime_
 #include "kaleidoscope/device/device.h"           // for VirtualProps::Storage, Base<>::Storage
-#include "kaleidoscope/keyswitch_state.h"         // for INJECTED, IS_PRESSED, WAS_PRESSED
+#include "kaleidoscope/keyswitch_state.h"         // for keyToggledOn
 #include "kaleidoscope/plugin/EEPROM-Settings.h"  // for EEPROMSettings
 // This is a special exception to the rule of only including a plugin's
 // top-level header file, because DynamicMacros doesn't depend on the Macros
diff --git a/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.h b/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.h
index e4767761..93ff7600 100644
--- a/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.h
+++ b/plugins/Kaleidoscope-DynamicMacros/src/kaleidoscope/plugin/DynamicMacros.h
@@ -16,9 +16,9 @@
 
 #pragma once
 
-#include <Kaleidoscope-MacroSupport.h>
-#include <Kaleidoscope-Ranges.h>  // for DYNAMIC_MACRO_FIRST
-#include <stdint.h>               // for uint16_t, uint8_t
+#include <Kaleidoscope-MacroSupport.h>  // for MacroSupport
+#include <Kaleidoscope-Ranges.h>        // for DYNAMIC_MACRO_FIRST
+#include <stdint.h>                     // for uint16_t, uint8_t
 
 #include "kaleidoscope/KeyEvent.h"              // for KeyEvent
 #include "kaleidoscope/event_handler_result.h"  // for EventHandlerResult
diff --git a/plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.cpp b/plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.cpp
index bd453804..27b1722d 100644
--- a/plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.cpp
+++ b/plugins/Kaleidoscope-MacroSupport/src/kaleidoscope/plugin/MacroSupport.cpp
@@ -16,15 +16,15 @@
 
 #include "kaleidoscope/plugin/MacroSupport.h"
 
-#include <Arduino.h>                   // for pgm_read_byte, delay, F, PROGMEM, __F...
+#include <Arduino.h>                   // for F, __FlashStringHelper
 #include <Kaleidoscope-FocusSerial.h>  // for Focus, FocusSerial
 #include <stdint.h>                    // for uint8_t
 
 #include "kaleidoscope/KeyAddr.h"               // for KeyAddr
 #include "kaleidoscope/KeyEvent.h"              // for KeyEvent
 #include "kaleidoscope/Runtime.h"               // for Runtime, Runtime_
-#include "kaleidoscope/event_handler_result.h"  // for EventHandlerResult, EventHandlerResul...
-#include "kaleidoscope/key_defs.h"              // for Key, LSHIFT, Key_NoKey, Key_0, Key_1
+#include "kaleidoscope/event_handler_result.h"  // for EventHandlerResult, EventHandlerResult::OK
+#include "kaleidoscope/key_defs.h"              // for Key, Key_NoKey
 #include "kaleidoscope/keyswitch_state.h"       // for INJECTED, IS_PRESSED, WAS_PRESSED
 
 // =============================================================================
diff --git a/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.cpp b/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.cpp
index 8f63a29a..8101a85a 100644
--- a/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.cpp
+++ b/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.cpp
@@ -24,7 +24,7 @@
 #include "kaleidoscope/KeyEvent.h"                  // for KeyEvent
 #include "kaleidoscope/event_handler_result.h"      // for EventHandlerResult, EventHandlerResul...
 #include "kaleidoscope/key_defs.h"                  // for Key, LSHIFT, Key_NoKey, Key_0, Key_1
-#include "kaleidoscope/keyswitch_state.h"           // for INJECTED, IS_PRESSED, WAS_PRESSED
+#include "kaleidoscope/keyswitch_state.h"           // for keyToggledOff
 #include "kaleidoscope/plugin/Macros/MacroSteps.h"  // for macro_t, MACRO_NONE, MACRO_ACTION_END
 
 // =============================================================================
diff --git a/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.h b/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.h
index 0de51309..5ccc0c21 100644
--- a/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.h
+++ b/plugins/Kaleidoscope-Macros/src/kaleidoscope/plugin/Macros.h
@@ -16,10 +16,10 @@
 
 #pragma once
 
-#include <stdint.h>  // for uint8_t
+#include <Kaleidoscope-MacroSupport.h>  // for MacroSupport
+#include <Kaleidoscope-Ranges.h>        // for MACRO_FIRST, MACRO_LAST
+#include <stdint.h>                     // for uint8_t
 
-#include <Kaleidoscope-MacroSupport.h>
-#include <Kaleidoscope-Ranges.h>                    // for MACRO_FIRST, MACRO_LAST
 #include "kaleidoscope/KeyEvent.h"                  // for KeyEvent
 #include "kaleidoscope/event_handler_result.h"      // for EventHandlerResult
 #include "kaleidoscope/key_defs.h"                  // for Key