From 445687634e791ee02eeabc3b44b75511df6fa492 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 8 Oct 2018 16:40:18 +0200 Subject: [PATCH 1/3] Introduce a new event: onLayerChange The intent is to make it easier for plugins to detect layer changes and schedule work accordingly. There event receives no arguments, the current state can always be queried with `Layer.getLayerState()`, and if a plugin requires the old state too, they can track that on their own. Signed-off-by: Gergely Nagy --- NEWS.md | 4 ++++ src/kaleidoscope/event_handlers.h | 6 ++++++ src/kaleidoscope/hooks.h | 4 ++++ src/kaleidoscope/layers.cpp | 4 ++++ 4 files changed, 18 insertions(+) diff --git a/NEWS.md b/NEWS.md index abe27afdee..7f2c97480a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -79,6 +79,10 @@ The [Cycle](doc/plugin/Cycle.md) plugin has much better support for cycling thro There are situations where one would like to disable sending a report after each and every step of a macro, and rather have direct control over when reports are sent. The new `WITH_EXPLICIT_REPORT`, `WITH_IMPLICIT_REPORT` and `SEND_REPORT` steps help with that. Please see the [Macros](doc/plugin/Macros.md) documentation for more information. +### Events now trigger on layer changes + +Changing layers now triggers the `onLayerChange` event - but only if there was real change (thus, calling `Layer.on(SOME_LAYER)` multiple times in a row will only trigger one event). This event was introduced to help plugins that depend on layer state schedule their work better. + ## New hardware support Kaleidoscope has been ported to the following devices: diff --git a/src/kaleidoscope/event_handlers.h b/src/kaleidoscope/event_handlers.h index 45c9feac00..39f12a817f 100644 --- a/src/kaleidoscope/event_handlers.h +++ b/src/kaleidoscope/event_handlers.h @@ -71,6 +71,12 @@ (const char *command), __NL__ \ (command), ##__VA_ARGS__) __NL__ \ __NL__ \ + /* Called when the layer state changes. Which layes changed are */ __NL__ \ + /* not passed as arguments. If one needs that info, they should */ __NL__ \ + /* track Layer.getState() themselves. */ __NL__ \ + OPERATION(onLayerChange, __NL__ \ + _NOT_ABORTABLE, __NL__ \ + (), (), ##__VA_ARGS__) __NL__ \ /* Called before reporting our state to the host. This is the */ __NL__ \ /* last point in a cycle where a plugin can alter what gets */ __NL__ \ /* reported to the host. */ __NL__ \ diff --git a/src/kaleidoscope/hooks.h b/src/kaleidoscope/hooks.h index d83bb29307..9fd57ead57 100644 --- a/src/kaleidoscope/hooks.h +++ b/src/kaleidoscope/hooks.h @@ -32,6 +32,9 @@ extern void handleKeyswitchEvent(kaleidoscope::Key mappedKey, byte row, byte col namespace kaleidoscope { +// Forward declaration to enable friend declarations. +class Layer_; + // The reason why the hook routing entry point functions live within // class Hooks and not directly within a namespace is, that we want // to restrict who is allowed to trigger hooks, mainly to prevent @@ -49,6 +52,7 @@ class Hooks { // Kaleidoscope_ calls Hooks::onSetup, Hooks::beforeReportingState // and Hooks::afterEachCycle. friend class Kaleidoscope_; + friend class ::kaleidoscope::Layer_; // ::handleKeyswitchEvent(...) calls Hooks::onKeyswitchEvent. friend void ::handleKeyswitchEvent(kaleidoscope::Key mappedKey, diff --git a/src/kaleidoscope/layers.cpp b/src/kaleidoscope/layers.cpp index 72e4a7e3b3..2c6fd0b55c 100644 --- a/src/kaleidoscope/layers.cpp +++ b/src/kaleidoscope/layers.cpp @@ -168,6 +168,8 @@ void Layer_::on(uint8_t layer) { // Update the keymap cache (but not liveCompositeKeymap; that gets // updated separately, when keys toggle on or off. See layers.h) updateActiveLayers(); + + kaleidoscope::Hooks::onLayerChange(); } // Deactivate a given layer @@ -187,6 +189,8 @@ void Layer_::off(uint8_t layer) { // Update the keymap cache (but not liveCompositeKeymap; that gets // updated separately, when keys toggle on or off. See layers.h) updateActiveLayers(); + + kaleidoscope::Hooks::onLayerChange(); } boolean Layer_::isOn(uint8_t layer) { From 7214cc0d4731d77579a3f4cca1c00c1fd60c66df Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 10 Oct 2018 08:31:27 +0200 Subject: [PATCH 2/3] Colormap: Migrate to using `onLayerChange` Instead of tracking layer changes ourselves, use the new `onLayerChange` event to do that for us. This makes the code a tiny bit easier to follow. Signed-off-by: Gergely Nagy --- src/kaleidoscope/plugin/Colormap.cpp | 23 +++++++++++------------ src/kaleidoscope/plugin/Colormap.h | 4 ++-- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/kaleidoscope/plugin/Colormap.cpp b/src/kaleidoscope/plugin/Colormap.cpp index 577909314b..ac7b3144b1 100644 --- a/src/kaleidoscope/plugin/Colormap.cpp +++ b/src/kaleidoscope/plugin/Colormap.cpp @@ -28,7 +28,7 @@ namespace plugin { uint16_t ColormapEffect::map_base_; uint8_t ColormapEffect::max_layers_; -uint8_t ColormapEffect::last_highest_layer_; +uint8_t ColormapEffect::top_layer_; void ColormapEffect::max_layers(uint8_t max_) { if (map_base_ != 0) @@ -42,21 +42,20 @@ void ColormapEffect::onActivate(void) { if (!Kaleidoscope.has_leds) return; - last_highest_layer_ = Layer.top(); - if (last_highest_layer_ <= max_layers_) - ::LEDPaletteTheme.updateHandler(map_base_, last_highest_layer_); + top_layer_ = Layer.top(); + if (top_layer_ <= max_layers_) + ::LEDPaletteTheme.updateHandler(map_base_, top_layer_); } -void ColormapEffect::update(void) { - if (!Kaleidoscope.has_leds || Layer.top() == last_highest_layer_) - return; - - onActivate(); +void ColormapEffect::refreshAt(byte row, byte col) { + if (top_layer_ <= max_layers_) + ::LEDPaletteTheme.refreshAt(map_base_, top_layer_, row, col); } -void ColormapEffect::refreshAt(byte row, byte col) { - if (last_highest_layer_ <= max_layers_) - ::LEDPaletteTheme.refreshAt(map_base_, last_highest_layer_, row, col); +EventHandlerResult ColormapEffect::onLayerChange() { + if (::LEDControl.get_mode() == this) + onActivate(); + return EventHandlerResult::OK; } EventHandlerResult ColormapEffect::onFocusEvent(const char *command) { diff --git a/src/kaleidoscope/plugin/Colormap.h b/src/kaleidoscope/plugin/Colormap.h index 03ca53d059..7837a8c354 100644 --- a/src/kaleidoscope/plugin/Colormap.h +++ b/src/kaleidoscope/plugin/Colormap.h @@ -28,15 +28,15 @@ class ColormapEffect : public LEDMode { void max_layers(uint8_t max_); + EventHandlerResult onLayerChange(); EventHandlerResult onFocusEvent(const char *command); protected: void onActivate(void) final; - void update(void) final; void refreshAt(byte row, byte col) final; private: - static uint8_t last_highest_layer_; + static uint8_t top_layer_; static uint8_t max_layers_; static uint16_t map_base_; }; From 38681b9681d5bcf2a7ca94e0c16d78a6be9b5da0 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Tue, 9 Oct 2018 11:15:55 +0200 Subject: [PATCH 3/3] ActiveModColor: Improve latency by caching interesting keys on layer change Keys normally only change when switching layers, so instead of going through every key in every cycle to look for modifiers, do that once, when layers change (using the new `onLayerChange` event), and store the coordinates of keys we consider modifiers in an array (currently limited to 16 items). Then, when we want to highlight them, go over this array only, significantly reducing the work we need to do. In a typical case, on a full-size keyboard, one would have eight modifiers and a few layer keys, so instead of going through a hundred keys, we go through sixteen at most, but usually considerably less. This fixes #403, at the cost of noticeably higher PROGMEM and RAM use. Signed-off-by: Gergely Nagy --- .../plugin/LED-ActiveModColor.cpp | 72 ++++++++++++------- src/kaleidoscope/plugin/LED-ActiveModColor.h | 7 ++ 2 files changed, 55 insertions(+), 24 deletions(-) diff --git a/src/kaleidoscope/plugin/LED-ActiveModColor.cpp b/src/kaleidoscope/plugin/LED-ActiveModColor.cpp index d6df13f98c..e0dfb324dd 100644 --- a/src/kaleidoscope/plugin/LED-ActiveModColor.cpp +++ b/src/kaleidoscope/plugin/LED-ActiveModColor.cpp @@ -22,41 +22,30 @@ namespace kaleidoscope { namespace plugin { +uint8_t ActiveModColorEffect::mod_keys_[MAX_MODS_PER_LAYER]; +uint8_t ActiveModColorEffect::mod_key_count_; + cRGB ActiveModColorEffect::highlight_color = (cRGB) { 0xff, 0xff, 0xff }; cRGB ActiveModColorEffect::sticky_color = CRGB(0xff, 0x00, 0x00); -EventHandlerResult ActiveModColorEffect::beforeReportingState() { +EventHandlerResult ActiveModColorEffect::onLayerChange() { if (!Kaleidoscope.has_leds) return EventHandlerResult::OK; + mod_key_count_ = 0; + for (byte r = 0; r < ROWS; r++) { for (byte c = 0; c < COLS; c++) { - Key k = Layer.lookupOnActiveLayer(r, c); - - if (::OneShot.isOneShotKey(k)) { - if (::OneShot.isSticky(k)) - ::LEDControl.setCrgbAt(r, c, sticky_color); - else if (::OneShot.isActive(k)) - ::LEDControl.setCrgbAt(r, c, highlight_color); - else - ::LEDControl.refreshAt(r, c); - } else if (k.raw >= Key_LeftControl.raw && k.raw <= Key_RightGui.raw) { - if (hid::isModifierKeyActive(k)) - ::LEDControl.setCrgbAt(r, c, highlight_color); - else - ::LEDControl.refreshAt(r, c); - } else if (k.flags == (SYNTHETIC | SWITCH_TO_KEYMAP)) { - uint8_t layer = k.keyCode; - if (layer >= LAYER_SHIFT_OFFSET) - layer -= LAYER_SHIFT_OFFSET; - - if (Layer.isOn(layer)) - ::LEDControl.setCrgbAt(r, c, highlight_color); - else - ::LEDControl.refreshAt(r, c); + Key k = Layer.lookup(r, c); + + if (::OneShot.isOneShotKey(k) || + (k.raw >= Key_LeftControl.raw && k.raw <= Key_RightGui.raw) || + (k.flags == (SYNTHETIC | SWITCH_TO_KEYMAP))) { + uint8_t coords = r * COLS + c; + mod_keys_[mod_key_count_++] = coords; } } } @@ -64,6 +53,41 @@ EventHandlerResult ActiveModColorEffect::beforeReportingState() { return EventHandlerResult::OK; } +EventHandlerResult ActiveModColorEffect::beforeReportingState() { + for (uint8_t i = 0; i < mod_key_count_; i++) { + uint8_t coords = mod_keys_[i]; + byte c = coords % COLS; + byte r = (coords - c) / COLS; + + Key k = Layer.lookup(r, c); + + if (::OneShot.isOneShotKey(k)) { + if (::OneShot.isSticky(k)) + ::LEDControl.setCrgbAt(r, c, sticky_color); + else if (::OneShot.isActive(k)) + ::LEDControl.setCrgbAt(r, c, highlight_color); + else + ::LEDControl.refreshAt(r, c); + } else if (k.raw >= Key_LeftControl.raw && k.raw <= Key_RightGui.raw) { + if (hid::isModifierKeyActive(k)) + ::LEDControl.setCrgbAt(r, c, highlight_color); + else + ::LEDControl.refreshAt(r, c); + } else if (k.flags == (SYNTHETIC | SWITCH_TO_KEYMAP)) { + uint8_t layer = k.keyCode; + if (layer >= LAYER_SHIFT_OFFSET) + layer -= LAYER_SHIFT_OFFSET; + + if (Layer.isOn(layer)) + ::LEDControl.setCrgbAt(r, c, highlight_color); + else + ::LEDControl.refreshAt(r, c); + } + } + + return EventHandlerResult::OK; +} + } } diff --git a/src/kaleidoscope/plugin/LED-ActiveModColor.h b/src/kaleidoscope/plugin/LED-ActiveModColor.h index 6beb7889c0..ab2b94f582 100644 --- a/src/kaleidoscope/plugin/LED-ActiveModColor.h +++ b/src/kaleidoscope/plugin/LED-ActiveModColor.h @@ -20,6 +20,8 @@ #include #include +#define MAX_MODS_PER_LAYER 16 + namespace kaleidoscope { namespace plugin { class ActiveModColorEffect : public kaleidoscope::Plugin { @@ -30,6 +32,11 @@ class ActiveModColorEffect : public kaleidoscope::Plugin { static cRGB sticky_color; EventHandlerResult beforeReportingState(); + EventHandlerResult onLayerChange(); + + private: + static uint8_t mod_keys_[MAX_MODS_PER_LAYER]; + static uint8_t mod_key_count_; }; } }