Fix Qukeys mod flag rollover

This is a short-term solution for the problem of qukeys rollover from
mod-flagged keys. When flushing its queue, Qukeys stashes a copy of the
current keyboard HID report, and bases the new (usually mid-cycle) report on
the previous one, because it's complete, unlike the partial current report. This
prevents bugs where it would cause undesired repeats of keys scanned later in
the cycle, but results in a bypassing of the system in the HIDAdaptor that masks
modifier flags when rolling over to a key without those flags.

By first storing a copy of the previous key report's modifiers byte and then
restoring it after the report for a released qukey is sent, we remove any
modifier flags it added to that report, so the next key in the queue (if any)
won't base it's report on those modifiers (which would normally be masked by the
HIDAdaptor).

This is a temporary fix. The real fix will probably involve changes to both
KeyboardioHID and the HIDAdaptor, and will allow Qukeys to stop accessing the
HID reports, which really ought to be private member variables of the Keyboard
class.

Fixes #619.

Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>

src/kaleidoscope/plugin/Qukeys.cpp

@@ -173,11 +173,12 @@ bool Qukeys::flushKey(bool qukey_state, uint8_t keyswitch_state) {
   // have a full HID report, and we don't want to accidentally turn
   // off keys that the scan hasn't reached yet, so we force the
   // current report to be the same as the previous one, then proceed
-  HID_KeyboardReport_Data_t hid_report;
+  HID_KeyboardReport_Data_t curr_hid_report;
-  // First, save the current report
+  // First, save the current report & previous report's modifiers
-  memcpy(hid_report.allkeys, Keyboard.keyReport.allkeys, sizeof(hid_report));
+  memcpy(&curr_hid_report, &Keyboard.keyReport, sizeof(curr_hid_report));
+  byte prev_hid_report_modifiers = Keyboard.lastKeyReport.modifiers;
   // Next, copy the old report
-  memcpy(Keyboard.keyReport.allkeys, Keyboard.lastKeyReport.allkeys, sizeof(Keyboard.keyReport));
+  memcpy(&Keyboard.keyReport, &Keyboard.lastKeyReport, sizeof(Keyboard.keyReport));
   // Instead of just calling pressKey here, we start processing the
   // key again, as if it was just pressed, and mark it as injected, so
   // we can ignore it and don't start an infinite loop. It would be
@@ -188,11 +189,32 @@ bool Qukeys::flushKey(bool qukey_state, uint8_t keyswitch_state) {
   hid::sendKeyboardReport();
 
   // Next, we restore the current state of the report
-  memcpy(Keyboard.keyReport.allkeys, hid_report.allkeys, sizeof(hid_report));
+  memcpy(&Keyboard.keyReport, &curr_hid_report, sizeof(curr_hid_report));
 
   // Last, if the key is still down, add its code back in
-  if (keyswitch_state & IS_PRESSED)
+  if (keyswitch_state & IS_PRESSED) {
     handleKeyswitchEvent(keycode, row, col, IS_PRESSED | WAS_PRESSED);
+  } else {
+    // If this is the key that was released, send that release event now
+    handleKeyswitchEvent(Key_NoKey, row, col, WAS_PRESSED);
+    // ...and if there's another key in the queue that's about to also be
+    // flushed, we need to do something to clear this one's modifier flags (if
+    // any) from the previous report
+    if (key_queue_length_ > 1) {
+      // Restore the previous report; whatever was added by this key flush
+      // should not appear in the next one, because this key has now been
+      // released.  This is necessary to handle the case where a qukey's primary
+      // key value has a modifier flag.  Because we copy the last report
+      // directly, we're bypassing the mod-flag rollover protection offered by
+      // the HIDAdapter.  Unfortunately, this does not help if we're rolling
+      // over multiple keys, and one of the unreleased ones has a mod flag.
+      // That's probably rare enough that it won't be noticed, however.  THIS IS
+      // AN UGLY HACK, AND IT SHOULD BE FIXED WITH SOMETHING BETTER EVENTUALLY.
+      // Doing it right will most likely involve either major changes in
+      // KeyboardioHID or Kaleidoscope itself.
+      Keyboard.lastKeyReport.modifiers = prev_hid_report_modifiers;
+    }
+  }
 
   // Now that we're done sending the report(s), Qukeys can process events again:
   flushing_queue_ = false;
@@ -217,11 +239,7 @@ void Qukeys::flushQueue(int8_t index) {
       return;
     flushKey(QUKEY_STATE_ALTERNATE, IS_PRESSED | WAS_PRESSED);
   }
-  if (isQukey(key_queue_[0].addr)) {
+  flushKey(QUKEY_STATE_PRIMARY, WAS_PRESSED);
-    flushKey(QUKEY_STATE_PRIMARY, IS_PRESSED | WAS_PRESSED);
-  } else {
-    flushKey(QUKEY_STATE_PRIMARY, WAS_PRESSED);
-  }
 }
 
 // Flush all the non-qukey keys from the front of the queue