From 2cb5a1c17292555e088d8b9501b47fae463253e4 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Tue, 1 Jun 2021 21:00:08 -0500 Subject: [PATCH] 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 --- src/kaleidoscope/KeyAddrEventQueue.h | 2 +- src/kaleidoscope/KeyEvent.h | 6 +++--- src/kaleidoscope/Runtime.cpp | 24 ++++-------------------- src/kaleidoscope/key_defs.h | 4 ++++ 4 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/kaleidoscope/KeyAddrEventQueue.h b/src/kaleidoscope/KeyAddrEventQueue.h index 48338e4d..326b84e5 100644 --- a/src/kaleidoscope/KeyAddrEventQueue.h +++ b/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`. diff --git a/src/kaleidoscope/KeyEvent.h b/src/kaleidoscope/KeyEvent.h index 0cc97ab8..eb852bb7 100644 --- a/src/kaleidoscope/KeyEvent.h +++ b/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: diff --git a/src/kaleidoscope/Runtime.cpp b/src/kaleidoscope/Runtime.cpp index 82b0df83..3218a64a 100644 --- a/src/kaleidoscope/Runtime.cpp +++ b/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 diff --git a/src/kaleidoscope/key_defs.h b/src/kaleidoscope/key_defs.h index 4d2a3c14..c357faf2 100644 --- a/src/kaleidoscope/key_defs.h +++ b/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)