Merge pull request #929 from gedankenexperimenter/qukeys-rollover-925

Improve Qukeys protection against unintended modifiers

docs/NEWS.md

@@ -12,6 +12,18 @@ See [UPGRADING.md](UPGRADING.md) for more detailed instructions about upgrading
 
 ## New features
 
+### Better protection against unintended modifiers from Qukeys
+
+Qukeys has two new configuration options for preventing unintended modifiers in
+the output, particularly when typing fast:
+
+- `Qukeys.setMinimumHoldTime(ms)` sets the minimum duration of a qukey press
+  required for it to be eligible to take on its alternate (modifier) value.
+- `Qukeys.setMinimumPriorInterval(ms)` sets the minimum interval between the
+  previous printable (letters, numbers, and punctuation) key press and the press
+  of the qukey required to make the qukey eligible to take on its alternate
+  (modifier) value.
+
 ### KALEIDOSCOPE_API_VERSION bump
 
 `KALEIDOSCOPE_API_VERSION` has been bumped to **2** due to the plugin API

docs/plugins/Qukeys.md

@@ -113,6 +113,15 @@ likely to generate errors and out-of-order events.
 >
 > Defaults to `50` (milliseconds).
 
+### `.setMinimumPriorInterval(min_interval)`
+
+> Sets the minimum amount of time (in milliseconds) that must pass between the
+> press event of a prior (non-modifier) key and the press of a qukey required to
+> make that qukey eligible to take on it's alternate state. This is another
+> measure that can be taken to prevent unintended modifiers while typing fast.
+>
+> Defaults to `75` (milliseconds).
+
 ### `.activate()`
 ### `.deactivate()`
 ### `.toggle()`

examples/Keystrokes/Qukeys/Qukeys.ino

@@ -73,6 +73,7 @@ void setup() {
   Qukeys.setHoldTimeout(1000);
   Qukeys.setOverlapThreshold(50);
   Qukeys.setMinimumHoldTime(100);
+  Qukeys.setMinimumPriorInterval(80);
 
   Kaleidoscope.setup();
 }

src/kaleidoscope/plugin/Qukeys.cpp

@@ -134,6 +134,14 @@ EventHandlerResult Qukeys::beforeReportingState() {
                     queue_head_.primary_key : queue_head_.alternate_key;
     flushEvent(event_key);
   }
+
+  // Last, if there hasn't been a keypress in a while, update the prior keypress
+  // timestamp to avoid integer overflow issues:
+  if (Runtime.hasTimeExpired(prior_keypress_timestamp_,
+                             minimum_prior_interval_)) {
+    prior_keypress_timestamp_ =
+      Runtime.millisAtCycleStart() - (minimum_prior_interval_ + 1);
+  }
   return EventHandlerResult::OK;
 }
 
@@ -185,6 +193,14 @@ bool Qukeys::processQueue() {
   // key, so we don't need to do it repeatedly later.
   bool qukey_is_spacecadet = isModifierKey(queue_head_.primary_key);
 
+  // If the qukey press is followed a printable key too closely, it's not
+  // eligible to take on its alternate value unless it's a SpaceCadet-type key.
+  if (!Runtime.hasTimeExpired(prior_keypress_timestamp_,
+                              minimum_prior_interval_) &&
+      !qukey_is_spacecadet) {
+    flushEvent(queue_head_.primary_key);
+  }
+
   // Now we search the queue for events that will let us decide if the qukey
   // should be flushed (and if so, in which of its two states). We start with
   // the second event in the queue (index 1).
@@ -283,6 +299,16 @@ void Qukeys::flushEvent(Key event_key) {
   KeyAddr queue_head_addr = event_queue_.addr(0);
   uint8_t keyswitch_state = event_queue_.isRelease(0) ? WAS_PRESSED : IS_PRESSED;
 
+  // If the flushed event is a keypress of a printable symbol, record its
+  // timestamp. This lets us suppress some unintended alternate values seen by
+  // fast typists by requiring a minimum interval between this keypress and the
+  // next qukey press in order for that qukey to become alternate-eligible.
+  if (!event_queue_.isRelease(0) &&
+      ((event_key >= Key_A && event_key <= Key_0) ||
+       (event_key >= Key_Minus && event_key <= Key_Slash))) {
+    prior_keypress_timestamp_ = event_queue_.timestamp(0);
+  }
+
   // Remove the head event from the queue:
   event_queue_.shift();
   // This ensures that the flushed event will be ignored by the event handler hook:

src/kaleidoscope/plugin/Qukeys.h

@@ -130,6 +130,12 @@ class Qukeys : public kaleidoscope::Plugin {
     minimum_hold_time_ = min_hold_time;
   }
 
+  // Set the minimum interval between the previous keypress and the qukey press
+  // to make the qukey eligible to become its alternate keycode.
+  void setMinimumPriorInterval(uint8_t min_interval) {
+    minimum_prior_interval_ = min_interval;
+  }
+
   // Function for defining the array of qukeys data (in PROGMEM). It's a
   // template function that takes as its sole argument an array reference of
   // size `_qukeys_count`, so there's no need to use `sizeof` to calculate the
@@ -190,6 +196,14 @@ class Qukeys : public kaleidoscope::Plugin {
   // for very fast typists).
   uint8_t minimum_hold_time_{50};
 
+  // The minimum interval in milliseconds between the previous keypress and the
+  // press of a qukey required to make the qukey eligible to take on its
+  // alternate value.
+  uint8_t minimum_prior_interval_{75};
+
+  // Timestamp of the keypress event immediately prior to the queue head event.
+  uint16_t prior_keypress_timestamp_{0};
+
   // This is a guard against re-processing events when qukeys flushes them from
   // its event queue. We can't just use an "injected" key state flag, because
   // that would cause other plugins to also ignore the event.