diff --git a/src/kaleidoscope/Runtime.cpp b/src/kaleidoscope/Runtime.cpp index ec353aef..0f91b954 100644 --- a/src/kaleidoscope/Runtime.cpp +++ b/src/kaleidoscope/Runtime.cpp @@ -1,5 +1,5 @@ /* Kaleidoscope - Firmware for computer input devices - * Copyright (C) 2013-2018 Keyboard.io, Inc. + * Copyright (C) 2013-2021 Keyboard.io, Inc. * * This program is free software: you can redistribute it and/or modify it under * the terms of the GNU General Public License as published by the Free Software @@ -22,10 +22,12 @@ namespace kaleidoscope { uint32_t Runtime_::millis_at_cycle_start_; +KeyAddr Runtime_::last_addr_toggled_on_ = KeyAddr::none(); Runtime_::Runtime_(void) { } +// ---------------------------------------------------------------------------- void Runtime_::setup(void) { // We are explicitly initializing the Serial port as early as possible to @@ -49,20 +51,294 @@ Runtime_::setup(void) { Layer.setup(); } +// ---------------------------------------------------------------------------- void Runtime_::loop(void) { millis_at_cycle_start_ = millis(); kaleidoscope::Hooks::beforeEachCycle(); +#ifndef NDEPRECATED + // For backwards compatibility. Some plugins rely on the handler for + // `beforeReportingState()` being called every cycle. In most cases, they can + // simply switch to using `afterEachCycle()`, but we don't want to simply + // break those plugins. + kaleidoscope::Hooks::beforeReportingState(); + // Also for backwards compatibility. If user code calls any code that directly + // changes the HID report(s) at any point between an event being detected and + // the end of `handleKeyEvent()` (most likely from `beforeReportingState()`), + // we need to make sure that report doesn't just get discarded. + hid().keyboard().sendReport(); +#endif + + // Next, we scan the keyswitches. Any toggle-on or toggle-off events will + // trigger a call to `handleKeyswitchEvent()`, which in turn will + // (conditionally) result in a HID report. Note that each event gets handled + // (and any resulting HID report(s) sent) as soon as it is detected. It is + // possible for more than one event to be handled like this in any given + // cycle, resulting in multiple HID reports, but guaranteeing that only one + // event is being handled at a time. device().scanMatrix(); - kaleidoscope::Hooks::beforeReportingState(); + kaleidoscope::Hooks::afterEachCycle(); +} - device().hid().keyboard().sendReport(); +// ---------------------------------------------------------------------------- +void +Runtime_::handleKeyswitchEvent(KeyEvent event) { + + // This function strictly handles physical key events. Any event without a + // valid `KeyAddr` gets ignored. + if (!event.addr.isValid()) + return; + + // Ignore any (non-)event that's not a state change. This check should be + // unnecessary, as we shouldn't call this function otherwise. + if (!(keyToggledOn(event.state) || keyToggledOff(event.state))) + return; + + // Set the `Key` value for this event. + if (keyToggledOff(event.state)) { + // When a key toggles off, set the event's key value to whatever the key's + // current value is in the live keys state array. + event.key = live_keys[event.addr]; + } else if (event.key == Key_NoKey) { + // 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`). + event.key = lookupKey(event.addr); + } + + // 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) + return; + + // If all the plugin handlers returned OK, we proceed to the next step in + // processing the event. + handleKeyEvent(event); +} + +// ---------------------------------------------------------------------------- +void +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) { + event.key = lookupKey(event.addr); + } + } + + // If any `onKeyEvent()` handler returns `ABORT`, we return before updating + // the Live Keys state array; as if the event didn't happen. + auto result = Hooks::onKeyEvent(event); + if (result == EventHandlerResult::ABORT) + return; + + // Update the live keys array based on the new event. + if (event.addr.isValid()) { + if (keyToggledOff(event.state)) { + live_keys.clear(event.addr); + } else { + live_keys.activate(event.addr, event.key); + } + } + + // If any `onKeyEvent()` handler returned a value other than `OK`, stop + // processing now. Likewise if the event's `Key` value is a no-op. + if (result != EventHandlerResult::OK || + event.key == Key_Masked || + event.key == Key_NoKey || + event.key == Key_Transparent) + return; + + // If it's a built-in Layer key, we handle it here, and skip sending report(s) + if (event.key.isLayerKey()) { + Layer.handleKeymapKeyswitchEvent(event.key, event.state); + //return; + } + + // The System Control HID report contains only one keycode, and gets sent + // immediately on `pressSystemControl()` or `releaseSystemControl()`. This is + // significantly different from the way the other HID reports work, where held + // keys remain in effect for subsequent reports. + if (event.key.isSystemControlKey()) { + if (keyToggledOn(event.state)) { + hid().keyboard().pressSystemControl(event.key); + } else { /* if (keyToggledOff(key_state)) */ + hid().keyboard().releaseSystemControl(event.key); + } + return; + } + + // Until this point, the old report was still valid. Now we construct the new + // one, based on the contents of the `live_keys` state array. + prepareKeyboardReport(event); + +#ifndef NDEPRECATED + // Deprecated handlers might depend on values in the report, so we wait until + // the new report is otherwise complete before calling them. + auto old_result = Hooks::onKeyswitchEvent(event.key, event.addr, event.state); + if (old_result == EventHandlerResult::ABORT) + return; + + if (old_result != EventHandlerResult::OK || + event.key == Key_Masked || + event.key == Key_NoKey || + event.key == Key_Transparent) + return; +#endif + + // Finally, send the new keyboard report + sendKeyboardReport(event); +} + +// ---------------------------------------------------------------------------- +void +Runtime_::prepareKeyboardReport(const KeyEvent &event) { + // before building the new report, start clean device().hid().keyboard().releaseAllKeys(); - kaleidoscope::Hooks::afterEachCycle(); + // Build report from composite keymap cache. This can be much more efficient + // with a bitfield. What we should be doing here is going through the array + // and checking for HID values (Keyboard, Consumer, System) and directly + // adding them to their respective reports. This comes before the old plugin + // hooks are called for the new event so that the report will be full complete + // except for that new event. + for (KeyAddr key_addr : KeyAddr::all()) { + // Skip this event's key addr; we will deal with that later. This is most + // important in the case of a key release, because we can't safely remove + // any keycode(s) added to the report later. + if (key_addr == event.addr) + continue; + + Key key = live_keys[key_addr]; + + // If the key is idle or masked, we can ignore it. + if (key == Key_Inactive || key == Key_Masked) + continue; + +#ifndef NDEPRECATED + // Only run hooks for plugin keys. If a plugin needs to do something every + // cycle, it can use one of the every-cycle hooks and search for active keys + // of interest. + auto result = Hooks::onKeyswitchEvent(key, key_addr, IS_PRESSED | WAS_PRESSED); + if (result == EventHandlerResult::ABORT) + continue; + + if (key_addr == event.addr) { + // update active keys cache? + if (keyToggledOn(event.state)) { + live_keys.activate(event.addr, key); + } else { + live_keys.clear(event.addr); + } + } +#endif + + addToReport(key); + } +} + +// ---------------------------------------------------------------------------- +void +Runtime_::addToReport(Key key) { + // First, call any relevant plugin handlers, to give them a chance to add + // other values to the HID report directly and/or to abort the automatic + // adding of keycodes below. + auto result = Hooks::onAddToReport(key); + if (result == EventHandlerResult::ABORT) + return; + + if (key.isKeyboardKey()) { + // The only incidental Keyboard modifiers that are allowed are the ones on + // the key that generated the event, so we strip any others before adding + // them. This might turn out to be too simple to cover all the corner cases, + // but the OS should be helpful and do most of the masking we want for us. + if (!key.isKeyboardModifier()) + key.setFlags(0); + + hid().keyboard().pressKey(key); + return; + } + + if (key.isConsumerControlKey()) { + hid().keyboard().pressConsumerControl(key); + return; + } + + // System Control keys and Layer keys are not handled here because they only + // take effect on toggle-on or toggle-off events, they don't get added to HID + // reports when held. +} + +// ---------------------------------------------------------------------------- +void +Runtime_::sendKeyboardReport(const KeyEvent &event) { + // If the keycode for this key is already in the report, we need to send an + // extra report without that keycode in order to correctly process the + // rollover. It might be better to exempt modifiers from this rule, but it's + // not clear that would be better. + if (keyToggledOn(event.state) && + event.key.isKeyboardKey()) { + // last keyboard key toggled on + last_addr_toggled_on_ = event.addr; + if (hid().keyboard().isKeyPressed(event.key)) { + // The keycode (flags ignored) for `event.key` is active in the current + // report. Should this be `wasKeyPressed()` instead? I don't think so, + // because (if I'm right) the new event hasn't been added yet. + hid().keyboard().releaseKey(event.key); + hid().keyboard().sendReport(); + } + if (event.key.getFlags() != 0) { + hid().keyboard().pressModifiers(event.key); + hid().keyboard().sendReport(); + } + } else if (event.addr != last_addr_toggled_on_) { + // (not a keyboard key OR toggled off) AND not last keyboard key toggled on + Key last_key = live_keys[last_addr_toggled_on_]; + if (last_key.isKeyboardKey()) { + hid().keyboard().pressModifiers(last_key); + } + } + if (keyToggledOn(event.state)) { + addToReport(event.key); + } + +#ifndef NDEPRECATED + // Call old pre-report handlers: + Hooks::beforeReportingState(); +#endif + + // Call new pre-report handlers: + if (Hooks::beforeReportingState(event) == EventHandlerResult::ABORT) + return; + + // Finally, send the report: + device().hid().keyboard().sendReport(); } Runtime_ Runtime; diff --git a/src/kaleidoscope/Runtime.h b/src/kaleidoscope/Runtime.h index 7a4c0218..7ebff605 100644 --- a/src/kaleidoscope/Runtime.h +++ b/src/kaleidoscope/Runtime.h @@ -19,6 +19,9 @@ #include "kaleidoscope_internal/device.h" #include "kaleidoscope/event_handler_result.h" #include "kaleidoscope/hooks.h" +#include "kaleidoscope/KeyEvent.h" +#include "kaleidoscope/LiveKeys.h" +#include "kaleidoscope/layers.h" namespace kaleidoscope { @@ -128,8 +131,82 @@ class Runtime_ { return kaleidoscope::Hooks::onFocusEvent(command); } + /** Handle a physical keyswitch event + * + * This method is called in response to physical keyswitch state changes. Its + * job is to call the `onKeyswitchEvent()` plugin handler functions, used by + * plugins that are particularly concerned about the timing of those + * events. It takes only one parameter, of type `KeyEvent`, which encapsulates + * the information about that event: + * + * - `event.key_addr`: The address of the key that was pressed or released. + * - `event.state`: The state of the keyswitch event (toggled on or off). + * - `event.key`: The `Key` value for this event. + * - `event.id`: A semi-unique ID value for the event. + * + * The ID value is used to help plugins that delay events to coordinate with + * each other so that they can avoid re-processing the same event, possibly + * causing endless loops. + */ + void handleKeyswitchEvent(KeyEvent event); + + /** Handle a logical key event + * + * This method triggers the handling of a logical "key event". Ususally that + * event is the result of a call to `handleKeyswitchEvent()`, but it can also + * be called by plugins that need to generate extra events without a 1:1 + * mapping to physical keyswitch state transitions. + */ + void handleKeyEvent(KeyEvent event); + + /** Prepare a new set of USB HID reports + * + * This method gets called when a key event results in at least one new HID + * report being sent to the host, usually as a result of a call to + * `handleKeyEvent()`. It clears the keyboard report (after plugins have + * already responded to the new event that triggered the forthcoming report), + * then populates the new report based on the values stored in the `live_keys` + * state array. + */ + void prepareKeyboardReport(const KeyEvent &event); + + /** Add keycode(s) to a USB HID report + * + * This method gets called from `prepareKeyboardReport()` to add keycodes + * corresponding to active keys in the `live_keys` state array to the Keyboard + * & Consumer Control HID reports. It calls the `onAddToReport()` plugin + * handlers first to give them a chance to abort. + */ + void addToReport(Key key); + + /** Send the new USB HID report(s) + * + * This method is called by `handleKeyEvent()` after `prepareKeyboardReport()` + * is done. It uses the information about the new event to guard against + * modifier and mod-flags rollover issues, and calls the + * `beforeReportingState()` plugin handler functions before sending the + * complete Keyboard and Consumer Control HID reports. + */ + void sendKeyboardReport(const KeyEvent &event); + + /** Get the current value of a keymap entry + * + * Returns the `Key` value for a given `KeyAddr` entry in the current keymap, + * overridden by any active entry in the `live_keys` array. + */ + Key lookupKey(KeyAddr key_addr) { + // First, check for an active key value in the `live_keys` array. + Key key = live_keys[key_addr]; + // If that entry is clear, look up the entry from the active keymap layers. + if (key == Key_Transparent) { + key = Layer.lookupOnActiveLayer(key_addr); + } + return key; + } + private: static uint32_t millis_at_cycle_start_; + static KeyAddr last_addr_toggled_on_; }; extern kaleidoscope::Runtime_ Runtime; diff --git a/src/kaleidoscope/device/virtual/Virtual.cpp b/src/kaleidoscope/device/virtual/Virtual.cpp index 11123f9d..82fef177 100644 --- a/src/kaleidoscope/device/virtual/Virtual.cpp +++ b/src/kaleidoscope/device/virtual/Virtual.cpp @@ -24,7 +24,6 @@ #include "kaleidoscope/keyswitch_state.h" #include "kaleidoscope/MatrixAddr.h" #include "kaleidoscope/key_defs.h" -#include "kaleidoscope/key_events.h" #include "HIDReportObserver.h" diff --git a/src/kaleidoscope/driver/hid/base/Keyboard.h b/src/kaleidoscope/driver/hid/base/Keyboard.h index 8ebeb894..0b2a02cb 100644 --- a/src/kaleidoscope/driver/hid/base/Keyboard.h +++ b/src/kaleidoscope/driver/hid/base/Keyboard.h @@ -135,64 +135,14 @@ class Keyboard { } void sendReport() __attribute__((noinline)) { - // Before sending the report, we add any modifier flags that are currently - // allowed, based on the latest keypress: - pressModifiers(requested_modifier_flags & modifier_flag_mask); - - // If a key has been toggled on in this cycle, we might need to send an extra - // HID report to the host, because that key might have the same keycode as - // another key that was already in the report on the previous cycle. For - // example, a user could have two `Key_E` keys in their keymap, in order to - // avoid repeating the same key with one finger. Or one might have a - // `LCTRL(Key_S)` and a plain `Key_S`, and have a reason to press them in - // rapid succession. In order to make this work, we need to call `release()` & - // `sendReport()` to send a release event to the host so that its normal - // repeat-rate-limiting behaviour won't effectively mask the second keypress. - // Then we call `press()` to add the keycode back in before sending the normal - // report. - // - // In most cases, this won't result in any difference from the previous report - // (because the newly-toggled-on keycode won't be in the previous report), so - // no extra report will be sent (because we suppress duplicate reports in - // KeyboardioHID). If there is a difference in the modifiers byte, an extra - // report would be sent later, regardless (also in KeyboardioHID). - // - // Furthermore, we need to send a report without the keycode for the - // newly-toggled-on key, but with any masked modifiers from flags removed. For - // example, if we roll over from `LSHIFT(Key_A)` to `Key_B`, we need to first - // send a report without the `shift`, then a second report with the `B`. If - // both of those bits are changed in the same report, at least one major OS - // processes the `B` keypress first, and we end up with `AB` instead of `Ab` - // in the output. - if (boot_keyboard_.getProtocol() == HID_BOOT_PROTOCOL) { - if (last_keycode_toggled_on) { - boot_keyboard_.release(last_keycode_toggled_on); - boot_keyboard_.sendReport(); - boot_keyboard_.press(last_keycode_toggled_on); - last_keycode_toggled_on = 0; - } boot_keyboard_.sendReport(); return; } - - if (last_keycode_toggled_on) { - // It would be good if KeyboardioHID's Keyboard object offered a way to - // compare the modifiers bytes of the current and previous key reports. That - // would allow us to only send these extra reports when either - // `last_keycode_toggled_on` was already held, or the modifiers byte - // changed. Likewise for BootKeyboard above. - nkro_keyboard_.release(last_keycode_toggled_on); - nkro_keyboard_.sendReport(); - nkro_keyboard_.press(last_keycode_toggled_on); - last_keycode_toggled_on = 0; - } - nkro_keyboard_.sendReport(); consumer_control_.sendReport(); } void releaseAllKeys() __attribute__((noinline)) { - resetModifierTracking(); if (boot_keyboard_.getProtocol() == HID_BOOT_PROTOCOL) { boot_keyboard_.releaseAll(); } else { @@ -218,71 +168,30 @@ class Keyboard { } } - // pressKey takes a Key, as well as optional boolean 'toggledOn' which defaults - // to 'true' - - // If toggled_on is not set to false, this routine adds the modifier flags on - // this key to the bitmask of modifiers that are allowed to be added to the - // upcoming report. We do this so that when we roll over from a key with a - // modifier flag to one without it, that modifier flag won't affect the new - // keypress. - - // If the key we're processing is a modifier key, any modifier flags attached to - // it are added directly to the report along with the modifier from its keycode - // byte. - // - // (A 'modifier key' is one of the eight modifier keys defined by the HID - // standard: left and right variants of Control, Shift, Alt, and GUI.) - - // Eventually it calls pressRawKey. - - void pressKey(Key pressed_key, boolean toggled_on = true) __attribute__((noinline)) { - if (toggled_on) { - // If two keys are toggled on during the same USB report, we would ideally - // send an extra USB report to help the host handle each key correctly, but - // this is problematic. - - // If we simply allow modifiers associated with the second newly-pressed - // key, it is possible to drop a modifier before the report is sent. - // Instead, we send modifiers associated with any newly-pressed keys. - - // The downside of this behavior is that in cases where the user presses - // down keys with conflicting modifiers at the exact same moment, they may - // get unexpected behavior. - - // If this is the first 'new' keycode being pressed in this cycle, reset the - // bitmask of modifiers we're willing to attach to USB HID keyboard reports - if (!last_keycode_toggled_on) { - modifier_flag_mask = 0; - } - - // Add any modifiers attached to this key to the bitmask of modifiers we're - // willing to attach to USB HID keyboard reports - modifier_flag_mask |= pressed_key.getFlags(); - - if (!isModifierKey(pressed_key)) { - last_keycode_toggled_on = pressed_key.getKeyCode(); - } - } - - - if (isModifierKey(pressed_key)) { - // If the key is a modifier key with additional modifiers attached to it as - // flags (as one might when creating a 'Hyper' key or a "Control Alt" key, - // we assume that all those modifiers are intended to modify other keys - // pressed while this key is held, so they are never masked out. - pressModifiers(pressed_key.getFlags()); - } else { - // If, instead, the modifiers are attached to a 'printable' or non-modifier - // key, we assume that they're not intended to modify other keys, so we add - // them to requested_modifier_flags, and only allow them to affect the report if - // the most recent keypress includes those modifiers. - requestModifiers(pressed_key.getFlags()); - } + DEPRECATED(HID_KEYBOARD_PRESSKEY_TOGGLEDON) + void pressKey(Key pressed_key, bool toggled_on) __attribute__((noinline)) { + pressKey(pressed_key); + } + void pressKey(Key pressed_key) __attribute__((noinline)) { + pressModifiers(pressed_key); pressRawKey(pressed_key); } + void pressModifiers(Key pressed_key) { + pressModifiers(pressed_key.getFlags()); + } + void releaseModifiers(Key released_key) { + releaseModifiers(released_key.getFlags()); + } + void clearModifiers() { + releaseRawKey(Key_LeftShift); + releaseRawKey(Key_LeftControl); + releaseRawKey(Key_LeftAlt); + releaseRawKey(Key_RightAlt); + releaseRawKey(Key_LeftGui); + } + // pressRawKey takes a Key object and calles KeyboardioHID's ".press" method // with its keycode. It does no processing of any flags or modifiers on the key void pressRawKey(Key pressed_key) { @@ -304,29 +213,17 @@ class Keyboard { } void releaseKey(Key released_key) { - // Remove any modifiers attached to this key from the bitmask of modifiers we're - // willing to attach to USB HID keyboard reports - modifier_flag_mask &= ~(released_key.getFlags()); - - if (!isModifierKey(released_key)) { - - // TODO(anyone): this code is incomplete, but is better than nothing - // If we're toggling off the most recently toggled on key, clear - // last_keycode_toggled_on - if (last_keycode_toggled_on == released_key.getKeyCode()) { - last_keycode_toggled_on = 0; - } - - // If the modifiers are attached to a 'printable' or non-modifier - // key, we need to clean up after the key press which would have requested - // the modifiers be pressed if the most recent keypress includes those modifiers. - cancelModifierRequest(released_key.getFlags()); - } - releaseModifiers(released_key.getFlags()); releaseRawKey(released_key); } + bool isKeyPressed(Key key) { + if (boot_keyboard_.getProtocol() == HID_BOOT_PROTOCOL) { + return boot_keyboard_.isKeyPressed(key.getKeyCode()); + } + return nkro_keyboard_.isKeyPressed(key.getKeyCode()); + } + boolean isModifierKeyActive(Key modifier_key) { if (boot_keyboard_.getProtocol() == HID_BOOT_PROTOCOL) { return boot_keyboard_.isModifierActive(modifier_key.getKeyCode()); @@ -384,70 +281,6 @@ class Keyboard { // not be a valid System Control keycode. uint8_t last_system_control_keycode_ = 0; - // modifier_flag_mask is a bitmask of modifiers that we found attached to - // keys that were newly pressed down during the most recent cycle with any new - // keypresses. - - // This is used to determine which modifier flags will be allowed to be added to - // the current keyboard HID report. It gets set during any cycle where one or - // more new keys is toggled on and presists until the next cycle with a newly - // detected keypress. - - uint8_t modifier_flag_mask = 0; - - // The functions in this namespace are primarily to solve the problem of - // rollover from a key with a modifier flag (e.g. `LSHIFT(Key_T)`) to one - // without (e.g. `Key_H`), which used to result in the mod flag being applied to - // keys other than the one with the flag. By using `modifier_flag_mask`, we can - // mask out any modifier flags that aren't attached to modifier keys or keys - // pressed or held in the most recent cycle, mitigating the rollover problem, - // and getting the intended `The` instead of `THe`. - - // requested_modifier_flags is bitmap of the modifiers attached to any non-modifier - // key found to be pressed during the most recent cycle. For example, it would - // include modifiers attached to Key_A, but not modifiers attached to - // Key_LeftControl - - uint8_t requested_modifier_flags = 0; - - // last_keycode_toggled_on is the keycode of the key most recently toggled on - // for this report. This is set when a keypress is first detected and cleared - // after the report is sent. If multiple keys are toggled on during a single - // cycle, this contains the most recently handled one. - - uint8_t last_keycode_toggled_on = 0; - - void resetModifierTracking(void) { - last_keycode_toggled_on = 0; - requested_modifier_flags = 0; - } - - // isModifierKey takes a Key and returns true if the key is a - // keyboard key corresponding to a modifier like Control, Alt or Shift - // TODO(anyone): This function should be lifted to the Kaleidoscope core, somewhere. - - bool isModifierKey(Key key) { - // If it's not a keyboard key, return false - if (key.getFlags() & (SYNTHETIC | RESERVED)) return false; - - return (key.getKeyCode() >= HID_KEYBOARD_FIRST_MODIFIER && - key.getKeyCode() <= HID_KEYBOARD_LAST_MODIFIER); - } - - // requestModifiers takes a bitmap of modifiers that might apply - // to the next USB HID report and adds them to a bitmap of all such modifiers. - - void requestModifiers(byte flags) { - requested_modifier_flags |= flags; - } - - // cancelModifierRequest takes a bitmap of modifiers that should no longer apply - // to the next USB HID report and removes them from the bitmap of all such modifiers. - - void cancelModifierRequest(byte flags) { - requested_modifier_flags &= ~flags; - } - // pressModifiers takes a bitmap of modifier keys that must be included in // the upcoming USB HID report and passes them through to KeyboardioHID // immediately diff --git a/src/kaleidoscope/driver/hid/keyboardio/Keyboard.h b/src/kaleidoscope/driver/hid/keyboardio/Keyboard.h index 8023eee2..6db99718 100644 --- a/src/kaleidoscope/driver/hid/keyboardio/Keyboard.h +++ b/src/kaleidoscope/driver/hid/keyboardio/Keyboard.h @@ -69,6 +69,9 @@ class BootKeyboardWrapper { BootKeyboard.releaseAll(); } + bool isKeyPressed(uint8_t code) { + return BootKeyboard.isKeyPressed(code); + } bool isModifierActive(uint8_t code) { return BootKeyboard.isModifierActive(code); } @@ -108,6 +111,9 @@ class NKROKeyboardWrapper { Keyboard.releaseAll(); } + bool isKeyPressed(uint8_t code) { + return Keyboard.isKeyPressed(code); + } bool isModifierActive(uint8_t code) { return Keyboard.isModifierActive(code); } diff --git a/src/kaleidoscope/driver/keyscanner/Base_Impl.h b/src/kaleidoscope/driver/keyscanner/Base_Impl.h index e2f17c23..fb958e0f 100644 --- a/src/kaleidoscope/driver/keyscanner/Base_Impl.h +++ b/src/kaleidoscope/driver/keyscanner/Base_Impl.h @@ -19,15 +19,30 @@ #include "kaleidoscope/driver/keyscanner/Base.h" #include "kaleidoscope/device/device.h" +#include "kaleidoscope/keyswitch_state.h" +#include "kaleidoscope/KeyEvent.h" +#include "kaleidoscope/Runtime.h" namespace kaleidoscope { namespace driver { namespace keyscanner { template<> -void Base::handleKeyswitchEvent(Key mappedKey, kaleidoscope::Device::Props::KeyScannerProps::KeyAddr key_addr, uint8_t keyState) { - ::handleKeyswitchEvent(mappedKey, key_addr, keyState); +void Base::handleKeyswitchEvent( + Key key __attribute__((unused)), + kaleidoscope::Device::Props::KeyScannerProps::KeyAddr key_addr, + uint8_t key_state) { + + // Because the `KeyEvent` constructor invoked below assigns a new `EventId` + // each time it's called, and plugins that implement `onKeyswitchEvent()` and + // use those event ID numbers to determine whether or not an event is new, + // it's critical that we do the test for keyswitches toggling on or off first. + if (keyToggledOn(key_state) || keyToggledOff(key_state)) { + auto event = KeyEvent::next(key_addr, key_state); + kaleidoscope::Runtime.handleKeyswitchEvent(event); + } } + } // namespace keyscanner } // namespace driver } // namespace kaleidoscope diff --git a/src/kaleidoscope/event_handlers.h b/src/kaleidoscope/event_handlers.h index 4f7f630f..ae24ab01 100644 --- a/src/kaleidoscope/event_handlers.h +++ b/src/kaleidoscope/event_handlers.h @@ -139,6 +139,7 @@ class SignatureCheckDummy {}; (),(),(), /* non template */ __NL__ \ (), (), ##__VA_ARGS__) __NL__ \ __NL__ \ + /* DEPRECATED */ __NL__ \ /* Function called for every non-idle key, every cycle, so it */ __NL__ \ /* can decide what to do with it. It can modify the key (which is */ __NL__ \ /* passed by reference for this reason), and decide whether */ __NL__ \ @@ -148,7 +149,7 @@ class SignatureCheckDummy {}; /* will stop processing there. */ __NL__ \ OPERATION(onKeyswitchEvent, __NL__ \ 1, __NL__ \ - _CURRENT_IMPLEMENTATION, __NL__ \ + DEPRECATED(ON_KEYSWITCH_EVENT_V1), __NL__ \ _ABORTABLE, __NL__ \ (),(),(), /* non template */ __NL__ \ (Key &mappedKey, KeyAddr key_addr, uint8_t keyState), __NL__ \ @@ -238,7 +239,7 @@ class SignatureCheckDummy {}; /* reported to the host. */ __NL__ \ OPERATION(beforeReportingState, __NL__ \ 1, __NL__ \ - _CURRENT_IMPLEMENTATION, __NL__ \ + DEPRECATED(BEFORE_REPORTING_STATE_V1), __NL__ \ _NOT_ABORTABLE, __NL__ \ (),(),(), /* non template */ __NL__ \ (),(),##__VA_ARGS__) __NL__ \ diff --git a/src/kaleidoscope/hooks.h b/src/kaleidoscope/hooks.h index d59ffccf..cda1710e 100644 --- a/src/kaleidoscope/hooks.h +++ b/src/kaleidoscope/hooks.h @@ -23,13 +23,16 @@ class Key; } #include "kaleidoscope/KeyAddr.h" +#include "kaleidoscope/KeyEvent.h" #include "kaleidoscope/plugin.h" #include "kaleidoscope/event_handlers.h" // Forward declaration required to enable friend declarations // in class Hooks. class kaleidoscope_; +#ifndef NDEPRECATED extern void handleKeyswitchEvent(kaleidoscope::Key mappedKey, KeyAddr key_addr, uint8_t keyState); +#endif namespace kaleidoscope { namespace plugin { @@ -67,9 +70,11 @@ class Hooks { friend class ::kaleidoscope::plugin::LEDControl; friend void ::kaleidoscope::sketch_exploration::pluginsExploreSketch(); +#ifndef NDEPRECATED // ::handleKeyswitchEvent(...) calls Hooks::onKeyswitchEvent. friend void ::handleKeyswitchEvent(kaleidoscope::Key mappedKey, KeyAddr key_addr, uint8_t keyState); +#endif private: diff --git a/src/kaleidoscope/key_events.cpp b/src/kaleidoscope/key_events.cpp index 55987049..eae4a95b 100644 --- a/src/kaleidoscope/key_events.cpp +++ b/src/kaleidoscope/key_events.cpp @@ -14,115 +14,21 @@ * this program. If not, see . */ +#ifndef NDEPRECATED #include "kaleidoscope/Runtime.h" #include "kaleidoscope/LiveKeys.h" #include "kaleidoscope/hooks.h" #include "kaleidoscope/keyswitch_state.h" #include "kaleidoscope/layers.h" +#include "kaleidoscope/event_handler_result.h" -static bool handleSyntheticKeyswitchEvent(Key mappedKey, uint8_t keyState) { - if (mappedKey.getFlags() & RESERVED) - return false; - - if (!(mappedKey.getFlags() & SYNTHETIC)) - return false; - - using kaleidoscope::Runtime; - - if (mappedKey.getFlags() & IS_CONSUMER) { - if (keyIsPressed(keyState)) - Runtime.hid().keyboard().pressConsumerControl(mappedKey); - } else if (mappedKey.getFlags() & IS_SYSCTL) { - if (keyToggledOn(keyState)) { - Runtime.hid().keyboard().pressSystemControl(mappedKey); - } else if (keyToggledOff(keyState)) { - Runtime.hid().keyboard().releaseSystemControl(mappedKey); - } - } else if (mappedKey.getFlags() & IS_INTERNAL) { - return false; - } else if (mappedKey.getFlags() & SWITCH_TO_KEYMAP) { - Layer.handleKeymapKeyswitchEvent(mappedKey, keyState); - } - - return true; -} - -static bool handleKeyswitchEventDefault(Key mappedKey, KeyAddr key_addr, uint8_t keyState) { - using kaleidoscope::Runtime; - //for every newly pressed button, figure out what logical key it is and send a key down event - // for every newly released button, figure out what logical key it is and send a key up event - - if (mappedKey.getFlags() & SYNTHETIC) { - handleSyntheticKeyswitchEvent(mappedKey, keyState); - } else if (keyToggledOn(keyState)) { - Runtime.hid().keyboard().pressKey(mappedKey); - } else if (keyIsPressed(keyState)) { - Runtime.hid().keyboard().pressKey(mappedKey, false); - } else if (keyToggledOff(keyState) && (keyState & INJECTED)) { - Runtime.hid().keyboard().releaseKey(mappedKey); - } - return true; -} - -void handleKeyswitchEvent(Key mappedKey, KeyAddr key_addr, uint8_t keyState) { - - using kaleidoscope::Runtime; - - /* These first steps are only done for keypresses that have a valid key_addr. - * In particular, doing them for keypresses with out-of-bounds key_addr - * would cause out-of-bounds array accesses in Layer.lookup(), - * Layer.updateLiveCompositeKeymap(), etc. - * Note that many INJECTED keypresses use UnknownKeyswitchLocation - * which gives us an invalid key_addr here. Therefore, it's legitimate that - * we may have keypresses with out-of-bounds key_addr. - * However, some INJECTED keypresses do have valid key_addr if they are - * injecting an event tied to a physical keyswitch - and we want them to go - * through this lookup. - * So we can't just test for INJECTED here, we need to test the key_addr - * directly. - * Note also that this key_addr test avoids out-of-bounds accesses in *core*, - * but doesn't guarantee anything about event handlers - event handlers may - * still receive out-of-bounds key_addr, and handling that properly is on - * them. - */ - if (key_addr.isValid()) { - // If the caller did not supply a `Key` value, get it from the keymap - // cache. If that value is transparent, look it up from the active layer for - // that key address. - if (mappedKey == Key_NoKey) { - // Note: If the next line returns `Key_NoKey`, that will effectively mask - // the key. - mappedKey = Layer.lookup(key_addr); - } - - } // key_addr valid - - // Keypresses with out-of-bounds key_addr start here in the processing chain - auto result = kaleidoscope::Hooks::onKeyswitchEvent(mappedKey, key_addr, keyState); - - // If any plugin returns `ABORT`, stop here and don't update the active keys - // cache entry. +// Deprecated. See `Runtime.handleKeyEvent()` +void handleKeyswitchEvent(Key key, KeyAddr key_addr, uint8_t key_state) { + // Perhaps we should call deprecated plugin event handlers here? + auto result = kaleidoscope::Hooks::onKeyswitchEvent(key, key_addr, key_state); if (result == kaleidoscope::EventHandlerResult::ABORT) return; - - // Update the keyboard state array - if (key_addr.isValid()) { - if (keyToggledOn(keyState)) { - kaleidoscope::live_keys.activate(key_addr, mappedKey); - } else if (keyToggledOff(keyState)) { - kaleidoscope::live_keys.clear(key_addr); - } - } - - // Only continue if all plugin handlers returned `OK`. - if (result != kaleidoscope::EventHandlerResult::OK) - return; - - // If the key has been masked (i.e. `Key_NoKey`), or it's a plugin-specific - // key (`RESERVED`), don't bother continuing. - if (mappedKey == Key_NoKey || (mappedKey.getFlags() & RESERVED) != 0) - return; - - // Handle all built-in key types. - handleKeyswitchEventDefault(mappedKey, key_addr, keyState); + if (keyIsPressed(key_state)) + kaleidoscope::Runtime.addToReport(key); } +#endif diff --git a/src/kaleidoscope/key_events.h b/src/kaleidoscope/key_events.h index 45d7ca8e..3a0ade75 100644 --- a/src/kaleidoscope/key_events.h +++ b/src/kaleidoscope/key_events.h @@ -65,4 +65,10 @@ * currentState may be flagged INJECTED, which signals that the event was * injected, and is not a direct result of a keypress, coming from the scanner. */ + +#ifndef NDEPRECATED + +DEPRECATED(HANDLE_KEYSWITCH_EVENT) void handleKeyswitchEvent(Key mappedKey, kaleidoscope::Device::Props::KeyScannerProps::KeyAddr key_addr, uint8_t keyState); + +#endif diff --git a/src/kaleidoscope/layers.h b/src/kaleidoscope/layers.h index d12fbdf1..e2265a92 100644 --- a/src/kaleidoscope/layers.h +++ b/src/kaleidoscope/layers.h @@ -20,12 +20,15 @@ #include "kaleidoscope/key_defs.h" #include "kaleidoscope/keymaps.h" #include "kaleidoscope/device/device.h" -#include "kaleidoscope/LiveKeys.h" #include "kaleidoscope_internal/device.h" #include "kaleidoscope_internal/sketch_exploration/sketch_exploration.h" #include "kaleidoscope_internal/shortname.h" #include "kaleidoscope_internal/deprecations.h" +#ifndef NDEPRECATED +#include "kaleidoscope/LiveKeys.h" +#endif + #define START_KEYMAPS __NL__ \ constexpr Key keymaps_linear[][kaleidoscope_internal::device.matrix_rows * kaleidoscope_internal::device.matrix_columns] PROGMEM = { @@ -54,35 +57,30 @@ class Layer_ { void setup(); - /* There are two lookup functions, because we have two caches, and different - * parts of the firmware will want to use either this or that (or perhaps - * both, in rare cases). - * - * First of all, we use caches because looking up a key through all the layers - * is costy, and the cost increases dramatically the more layers we have. - * - * Then, we have the `liveCompositeKeymap`, because to have layer behaviours - * we want, that is, if you hold a key on a layer, release the layer key but - * continue holding the other, we want for the layered keycode to continue - * repeating. - * - * At the same time, we want other keys to not be affected by the - * now-turned-off layer. So we update the keycode in the cache on-demand, when - * the key is pressed. (see the top of `handleKeyswitchEvent`). - * - * On the other hand, we also have plugins that scan the whole keymap, and do - * things based on that information, such as highlighting keys that changed - * between layers. These need to be able to look at a state of where the - * keymap *should* be, not necessarily where it is. The `liveCompositeKeymap` - * is not useful here. So we use `activeLayers` which we update whenever - * layers change (see `Layer.on` and `Layer.off`), and it updates the cache to - * show how the keymap should look, without the `liveCompositeKeymap`-induced - * behaviour. - * - * Thus, if we are curious about what a given key will do, use `lookup`. If we - * are curious what the active layer state describes the key as, use - * `lookupOnActiveLayer`. - */ + // There are two lookup functions here, for historical reasons. Previously, + // Kaleidoscope would need to look up a value for each active keyswitch in + // every cycle, and pass that value on to the "event" handlers. Most of these + // lookups were for keys that were being held, not toggled on or off. Because + // these lookups were so frequent, a cache was used to speed them up. + // + // We no longer need to look up these values every cycle for keys that are + // held, because Kaleidoscope now only acts on key events that are actual + // toggle-on or toggle-off events, so the speed of the lookups here is not so + // critical. However, the old "live composite keymap" cache was also used by + // some plugins (and certain parts of Kaleidoscope itself) to override values + // in the keymap, and these plugins might use calls to `Layer.lookup()`, + // expecting to get the override values. + // + // Therefore, the `lookup()` function below first checks the `live_keys` array + // (the keyboard state array that has replaced the keymap cache). This should + // allow old code to continue working, until all the associated code (mostly + // the `onKeyswitchEvent()` handlers) is replaced, at which point we can + // remove dependence on `live_keys` entirely from this class. + // + // The `Runtime.lookupKey()` function replaces this one, for plugins that + // still want to do this same check. + + DEPRECATED(LAYER_LOOKUP) static Key lookup(KeyAddr key_addr) { // First check the keyboard state array Key key = live_keys[key_addr]; diff --git a/src/kaleidoscope_internal/deprecations.h b/src/kaleidoscope_internal/deprecations.h index 5cae97ee..93dcebe3 100644 --- a/src/kaleidoscope_internal/deprecations.h +++ b/src/kaleidoscope_internal/deprecations.h @@ -38,3 +38,37 @@ #define _DEPRECATED_MESSAGE_LAYER_EVENTHANDLER __NL__ \ "`Layer.eventHandler()` is deprecated.\n" __NL__ \ "Please use `Layer.handleKeymapKeyswitchEvent()` instead." + +#define _DEPRECATED_MESSAGE_LAYER_LOOKUP __NL__ \ + "`Layer.lookup(key_addr)` is deprecated.\n" __NL__ \ + "Please use `Runtime.lookupKey(key_addr)` instead. Alternatively, if you\n" __NL__ \ + "need to look up the current keymap entry without regard to current live\n" __NL__ \ + "key state(s) (i.e. the `live_keys` array, which normally overrides the\n" __NL__ \ + "keymap), you can use `Layer.lookupOnActiveLayer(key_addr)`." + +#define _DEPRECATED_MESSAGE_HANDLE_KEYSWITCH_EVENT __NL__ \ + "`handleKeyswitchEvent()` has been deprecated.\n" __NL__ \ + "Please use `Runtime.handleKeyEvent()` instead." + +#define _DEPRECATED_MESSAGE_ON_KEYSWITCH_EVENT_V1 __NL__ \ + "The `onKeyswitchEvent()` event handler is deprecated.\n" __NL__ \ + "Please replace it with an `onKeyEvent()` handler. See the documentation\n" __NL__ \ + "in UPGRADING.md and docs/api-reference/event-handler-hooks.md for more\n" __NL__ \ + "information on what changes are needed to adapt old plugins to the new\n" __NL__ \ + "event handler API." + +#define _DEPRECATED_MESSAGE_BEFORE_REPORTING_STATE_V1 __NL__ \ + "This `beforeReportingState()` event handler version is deprecated.\n" __NL__ \ + "There is a new `beforeReportingState(KeyEvent)` handler that can be used\n" __NL__ \ + "instead, for plugins that need to execute code before each new HID\n" __NL__ \ + "report is sent. However, the new handler does not run every cycle, but\n" __NL__ \ + "only in response to key events. If you have code that is intended to run\n" __NL__ \ + "every scan cycle, it should be moved to the `afterEachCycle()` event\n" __NL__ \ + "handler instead." + +#define _DEPRECATED_MESSAGE_HID_KEYBOARD_PRESSKEY_TOGGLEDON __NL__ \ + "The `Keyboard::pressKey(key, toggled_on)` function is deprecated.\n" __NL__ \ + "Please use `Keyboard::pressKey(key)` without the second argument\n" __NL__ \ + "instead. The version with two arguments handled rollover events, and\n" __NL__ \ + "this is now handled more completely by the event handling functions in\n" __NL__ \ + "`Runtime`."