Change default value of `KeyEvent` from `Key_NoKey` to `Key_Undefined`

This lets us remove some awkward code from `handleKeyswitchEvent()`, because as
long as the default value (which triggers a keymap lookup) was the same as
`Key_Masked`, it wasn't sufficient for an `onKeyswitchEvent()` handler to change
`event.key` to `Key_Masked`, because that would be interpreted by
`handleKeyEvent()` as a signal to do a keymap lookup.

This also makes it more consistent with other parts of the code.  The values
`Key_Undefined`, `Key_Inactive`, and `Key_Transparent` are all the same, and
with this change they are each interpreted the same way by code that encounters
them.  In a keymap lookup, if an active layer has a `Key_Transparent` value, we
continue searching for a different value on another layer.  In the live keys
array, if we find a `Key_Inactive` entry, we look for a value from the keymap.
And with this change, when processing a key event, if it has a `Key_Undefined`
value, we look for a value from `active_keys` (and then from the keymap layers,
if nothing is found there).

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

src/kaleidoscope/KeyAddrEventQueue.h

@@ -147,7 +147,7 @@ class KeyAddrEventQueue {
 
   KeyEvent event(uint8_t i) const {
     uint8_t state = isRelease(i) ? WAS_PRESSED : IS_PRESSED;
-    return KeyEvent{addr(i), state, Key_NoKey, id(i)};
+    return KeyEvent{addr(i), state, Key_Undefined, id(i)};
   }
 
   // Only call this after `EventTracker::shouldIgnore()` returns `true`.

src/kaleidoscope/KeyEvent.h

@@ -32,7 +32,7 @@ struct KeyEvent {
  public:
   // Constructor for plugin use when regenerating an event with specific ID:
   KeyEvent(KeyAddr addr, uint8_t state,
-           Key key = Key_NoKey, KeyEventId id = last_id_)
+           Key key = Key_Undefined, KeyEventId id = last_id_)
     : addr(addr), state(state), key(key), id_(id) {}
 
   KeyEvent() : id_(last_id_) {}
@@ -40,7 +40,7 @@ struct KeyEvent {
   // For use by keyscanner creating a new event from a physical keyswitch toggle
   // on or off.
   static KeyEvent next(KeyAddr addr, uint8_t state) {
-    return KeyEvent(addr, state, Key_NoKey, ++last_id_);
+    return KeyEvent(addr, state, Key_Undefined, ++last_id_);
   }
 
   KeyEventId id() const {
@@ -54,7 +54,7 @@ struct KeyEvent {
 
   KeyAddr addr = KeyAddr::none();
   uint8_t state = 0;
-  Key key = Key_NoKey;
+  Key key = Key_Undefined;
 
  private:
   // serial number of the event:

src/kaleidoscope/Runtime.cpp

@@ -107,7 +107,7 @@ Runtime_::handleKeyswitchEvent(KeyEvent event) {
       live_keys.clear(event.addr);
       return;
     }
-  } else if (event.key == Key_NoKey) {
+  } else if (event.key == Key_Undefined) {
     // When a key toggles on, unless the event already has a key value (i.e. we
     // were called by a plugin rather than `actOnMatrixScan()`), we look up the
     // value from the current keymap (overridden by `live_keys`).
@@ -117,24 +117,6 @@ Runtime_::handleKeyswitchEvent(KeyEvent event) {
   // Run the plugin event handlers
   auto result = Hooks::onKeyswitchEvent(event);
 
-  // If an event handler changed `event.key` to `Key_Masked` in order to mask
-  // that keyswitch, we need to propagate that, but since `handleKeyEvent()`
-  // will recognize that value as the signal to do a fresh lookup, so we need to
-  // set that value in `live_keys` now. The alternative would be changing it to
-  // some other sentinel value, and have `handleKeyEvent()` change it back to
-  // `Key_Masked`, but I think this makes more sense.
-  //
-  // Note: It is still important to let events with `Key_Masked` fall through to
-  // `handleKeyEvent()`, because some plugins might still care about the event
-  // regardless of its `Key` value, and more importantly, that's where we clear
-  // masked keys that have toggled off. Alternatively, we could call
-  // `live_keys.clear(addr)` for toggle-off events here, and `mask(addr)` for
-  // toggle-on events, then return, short-cutting the call to
-  // `handleKeyEvent()`. It should work, but some plugins might be able to use
-  // that information.
-  if (event.key == Key_Masked)
-    live_keys.mask(event.addr);
-
   // Now we check the result from the plugin event handlers, and stop processing
   // if it was anything other than `OK`.
   if (result != EventHandlerResult::OK)
@@ -152,7 +134,7 @@ Runtime_::handleKeyEvent(KeyEvent event) {
   // For events that didn't begin with `handleKeyswitchEvent()`, we need to look
   // up the `Key` value from the keymap (maybe overridden by `live_keys`).
   if (event.addr.isValid()) {
-    if (keyToggledOff(event.state) || event.key == Key_NoKey) {
+    if (keyToggledOff(event.state) || event.key == Key_Undefined) {
       event.key = lookupKey(event.addr);
     }
   }
@@ -177,6 +159,7 @@ Runtime_::handleKeyEvent(KeyEvent event) {
   if (result != EventHandlerResult::OK ||
       event.key == Key_Masked ||
       event.key == Key_NoKey ||
+      event.key == Key_Undefined ||
       event.key == Key_Transparent)
     return;
 
@@ -213,6 +196,7 @@ Runtime_::handleKeyEvent(KeyEvent event) {
   if (old_result != EventHandlerResult::OK ||
       event.key == Key_Masked ||
       event.key == Key_NoKey ||
+      event.key == Key_Undefined ||
       event.key == Key_Transparent)
     return;
 #endif

src/kaleidoscope/key_defs.h

@@ -293,6 +293,10 @@ typedef kaleidoscope::Key Key_;
 #define Key_Inactive Key_Transparent
 #define Key_Masked Key_NoKey
 
+// The default value for new events.  Used to signal that a keymap lookup should
+// be done.
+#define Key_Undefined Key_Transparent
+
 #define KEY_BACKLIGHT_DOWN 0xf1
 #define KEY_BACKLIGHT_UP 0xf2
 #define Key_BacklightDown Key(KEY_BACKLIGHT_DOWN, KEY_FLAGS)