From 8818ad51b738a17ac500713880d724efaa41f57d Mon Sep 17 00:00:00 2001
From: Michael Richters <gedankenexperimenter@gmail.com>
Date: Mon, 5 Feb 2018 13:15:59 -0600
Subject: [PATCH] Fixed scan order bug

Store any macros key events and play them after the event handler pass has finished, so we
don't have a problem when holding other keys that are handled after the macro key in a
pass. This fixes the problem where held modifiers wouldn't be applied to macros, and also
fast repeating of printing characters.

This change does introduce a limit (default: 8) on the number of concurrent macros that
can be played.
---
 src/Kaleidoscope-Macros.cpp | 34 ++++++++++++++++++++++++++++------
 src/Kaleidoscope-Macros.h   | 25 +++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/src/Kaleidoscope-Macros.cpp b/src/Kaleidoscope-Macros.cpp
index 39fa7dcd..276b952e 100644
--- a/src/Kaleidoscope-Macros.cpp
+++ b/src/Kaleidoscope-Macros.cpp
@@ -6,6 +6,8 @@ const macro_t *macroAction(uint8_t macroIndex, uint8_t keyState) {
   return MACRO_NONE;
 }
 
+MacroKeyEvent Macros_::active_macros[];
+byte Macros_::active_macro_count;
 byte Macros_::row, Macros_::col;
 
 void playMacroKeyswitchEvent(Key key, uint8_t flags) {
@@ -181,24 +183,44 @@ const macro_t *Macros_::type(const char *string) {
   return MACRO_NONE;
 }
 
-static Key handleMacroEvent(Key mappedKey, byte row, byte col, uint8_t keyState) {
+Key Macros_::handleMacroEvent(Key mappedKey, byte row, byte col, uint8_t keyState) {
   if (mappedKey.flags != (SYNTHETIC | IS_MACRO))
     return mappedKey;
 
-  Macros_::row = row;
-  Macros_::col = col;
-  const macro_t *m = macroAction(mappedKey.keyCode, keyState);
-
-  Macros.play(m);
+  byte key_id = (row * COLS) + col;
+  addActiveMacroKey(mappedKey.keyCode, key_id, keyState);
   return Key_NoKey;
 }
 
+void Macros_::loopHook(bool post_clear) {
+  if (post_clear) {
+    active_macro_count = 0;
+    return;
+  }
+
+  for (byte i = 0; i < active_macro_count; ++i) {
+    if (active_macros[i].key_id == 0xFF) {
+      // i.e. UNKNOWN_KEYSWITCH_LOCATION
+      row = 0xFF;
+      col = 0xFF;
+    } else {
+      row = active_macros[i].key_id / COLS;
+      col = active_macros[i].key_id % COLS;
+    }
+    const macro_t *m = macroAction(active_macros[i].key_code,
+                                   active_macros[i].key_state);
+    Macros.play(m);
+  }
+}
+
 Macros_::Macros_(void) {
 }
 
 void
 Macros_::begin(void) {
+  active_macro_count = 0;
   Kaleidoscope.useEventHandlerHook(handleMacroEvent);
+  Kaleidoscope.useLoopHook(loopHook);
 }
 
 Macros_ Macros;
diff --git a/src/Kaleidoscope-Macros.h b/src/Kaleidoscope-Macros.h
index d750f996..5c6d2658 100644
--- a/src/Kaleidoscope-Macros.h
+++ b/src/Kaleidoscope-Macros.h
@@ -7,12 +7,37 @@
 
 const macro_t *macroAction(uint8_t macroIndex, uint8_t keyState);
 
+#if !defined(MAX_CONCURRENT_MACROS)
+#define MAX_CONCURRENT_MACROS 8
+#endif
+
+struct MacroKeyEvent {
+  byte key_code;
+  byte key_id;
+  byte key_state;
+};
+
 class Macros_ : public KaleidoscopePlugin {
  public:
   Macros_(void);
 
   void begin(void) final;
 
+  static MacroKeyEvent active_macros[MAX_CONCURRENT_MACROS];
+  static byte active_macro_count;
+  static void addActiveMacroKey(byte key_code, byte key_id, byte key_state) {
+    // If we've got too many active macros, give up:
+    if (active_macro_count >= MAX_CONCURRENT_MACROS) {
+      return;
+    }
+    active_macros[active_macro_count].key_code = key_code;
+    active_macros[active_macro_count].key_id = key_id;
+    active_macros[active_macro_count].key_state = key_state;
+    ++active_macro_count;
+  }
+  static Key handleMacroEvent(Key mappedKey, byte row, byte col, uint8_t keyState);
+  static void loopHook(bool post_clear);
+
   void play(const macro_t *macro_p);
 
   /* What follows below, is a bit of template magic that allows us to use