From 559858db8af100462895cb5104fdfb8a4f7930b5 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Tue, 15 Aug 2017 22:45:31 +0200 Subject: [PATCH 1/3] Updated to use the new LEDMode/LEDControl API Signed-off-by: Gergely Nagy --- src/Kaleidoscope/LED-ActiveModColor.cpp | 32 ++++++++++++++++--------- src/Kaleidoscope/LED-ActiveModColor.h | 6 ++--- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/Kaleidoscope/LED-ActiveModColor.cpp b/src/Kaleidoscope/LED-ActiveModColor.cpp index 132a0c31..9d959567 100644 --- a/src/Kaleidoscope/LED-ActiveModColor.cpp +++ b/src/Kaleidoscope/LED-ActiveModColor.cpp @@ -26,14 +26,11 @@ cRGB ActiveModColorEffect::highlight_color = (cRGB) { 0xff, 0xff, 0xff }; -ActiveModColorEffect::ActiveModColorEffect(void) { -} - void ActiveModColorEffect::begin(void) { Kaleidoscope.useLoopHook(loopHook); } -bool ActiveModColorEffect::isModifierActive(Key key) { +uint8_t ActiveModColorEffect::isModifierKeyActive(Key key) { if (key.raw >= ranges::OSM_FIRST && key.raw <= ranges::OSM_LAST) { uint8_t idx = key.raw - ranges::OSM_FIRST; key.flags = 0; @@ -41,12 +38,15 @@ bool ActiveModColorEffect::isModifierActive(Key key) { } if (key.raw < Key_LeftControl.raw || key.raw > Key_RightGui.raw) - return false; + return 0; - return hid::isModifierKeyActive(key); + if (hid::isModifierKeyActive(key)) + return 2; + else + return 1; } -bool ActiveModColorEffect::isLayerKeyActive(Key key) { +uint8_t ActiveModColorEffect::isLayerKeyActive(Key key) { uint8_t layer = 255; if (key.raw >= ranges::OSL_FIRST && key.raw <= ranges::OSL_LAST) { @@ -58,9 +58,12 @@ bool ActiveModColorEffect::isLayerKeyActive(Key key) { } if (layer == 255) - return false; + return 0; - return Layer.isOn(layer); + if (Layer.isOn(layer)) + return 2; + else + return 1; } void ActiveModColorEffect::loopHook(bool is_post_clear) { @@ -70,9 +73,16 @@ void ActiveModColorEffect::loopHook(bool is_post_clear) { for (byte r = 0; r < ROWS; r++) { for (byte c = 0; c < COLS; c++) { Key k = Layer.lookupOnActiveLayer(r, c); + uint8_t is_mod = isModifierKeyActive(k); + uint8_t is_layer = isLayerKeyActive(k); + + if (!is_mod && !is_layer) // Neither mod, nor layer key + continue; - if (isModifierActive(k) || isLayerKeyActive(k)) - LEDControl.setCrgbAt(r, c, highlight_color); + if (is_mod == 2 || is_mod == 2) + ::LEDControl.setCrgbAt(r, c, highlight_color); + else + ::LEDControl.refreshAt(r, c); } } } diff --git a/src/Kaleidoscope/LED-ActiveModColor.h b/src/Kaleidoscope/LED-ActiveModColor.h index 6da5ac7c..c38eb604 100644 --- a/src/Kaleidoscope/LED-ActiveModColor.h +++ b/src/Kaleidoscope/LED-ActiveModColor.h @@ -24,15 +24,15 @@ namespace kaleidoscope { class ActiveModColorEffect : public KaleidoscopePlugin { public: - ActiveModColorEffect(void); + ActiveModColorEffect(void) {} void begin(void) final; static cRGB highlight_color; private: - static bool isModifierActive(Key key); - static bool isLayerKeyActive(Key key); + static uint8_t isModifierKeyActive(Key key); + static uint8_t isLayerKeyActive(Key key); static void loopHook(bool is_post_clear); }; } From e15aaed8c6839908b5c22f6907d73f0c56d10f72 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 16 Aug 2017 01:52:37 +0200 Subject: [PATCH 2/3] loopHook: Correct a thinko Thanks @obra for catching it! Signed-off-by: Gergely Nagy --- src/Kaleidoscope/LED-ActiveModColor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Kaleidoscope/LED-ActiveModColor.cpp b/src/Kaleidoscope/LED-ActiveModColor.cpp index 9d959567..3349f207 100644 --- a/src/Kaleidoscope/LED-ActiveModColor.cpp +++ b/src/Kaleidoscope/LED-ActiveModColor.cpp @@ -79,7 +79,7 @@ void ActiveModColorEffect::loopHook(bool is_post_clear) { if (!is_mod && !is_layer) // Neither mod, nor layer key continue; - if (is_mod == 2 || is_mod == 2) + if (is_mod == 2 || is_layer == 2) ::LEDControl.setCrgbAt(r, c, highlight_color); else ::LEDControl.refreshAt(r, c); From 8375838c81de81662c4246c7bad79f4e29ffdb64 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 16 Aug 2017 09:55:35 +0200 Subject: [PATCH 3/3] Redo the logic that checks if we need to highlight Instead of using `Kaleidoscope-Ranges` and custom helper functions with magic constants to decide whether we need to highlight a key, refresh it, or leave it alone, use an if-else chain and inner ifs for activity. Leverages the new `OneShot.isOneShotKey(key)` and `OneShot.isActive(key)` methods. The net result is slightly cleaner code (though it can still be improved), and about 0.2ms saved, along with some PROGMEM space. Signed-off-by: Gergely Nagy --- README.md | 2 +- src/Kaleidoscope/LED-ActiveModColor.cpp | 67 ++++++++----------------- src/Kaleidoscope/LED-ActiveModColor.h | 2 - 3 files changed, 22 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 6996c802..3f8b7512 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ property: ## Dependencies * [Kaleidoscope-LEDControl](https://github.com/keyboardio/Kaleidoscope-LEDControl) -* [Kaleidoscope-Ranges](https://github.com/keyboardio/Kaleidoscope-Ranges) +* [Kaleidoscope-OneShot](https://github.com/keyboardio/Kaleidoscope-OneShot) ## Further reading diff --git a/src/Kaleidoscope/LED-ActiveModColor.cpp b/src/Kaleidoscope/LED-ActiveModColor.cpp index 3349f207..e47d16c5 100644 --- a/src/Kaleidoscope/LED-ActiveModColor.cpp +++ b/src/Kaleidoscope/LED-ActiveModColor.cpp @@ -17,7 +17,7 @@ */ #include -#include +#include #include namespace kaleidoscope { @@ -30,42 +30,6 @@ void ActiveModColorEffect::begin(void) { Kaleidoscope.useLoopHook(loopHook); } -uint8_t ActiveModColorEffect::isModifierKeyActive(Key key) { - if (key.raw >= ranges::OSM_FIRST && key.raw <= ranges::OSM_LAST) { - uint8_t idx = key.raw - ranges::OSM_FIRST; - key.flags = 0; - key.keyCode = Key_LeftControl.keyCode + idx; - } - - if (key.raw < Key_LeftControl.raw || key.raw > Key_RightGui.raw) - return 0; - - if (hid::isModifierKeyActive(key)) - return 2; - else - return 1; -} - -uint8_t ActiveModColorEffect::isLayerKeyActive(Key key) { - uint8_t layer = 255; - - if (key.raw >= ranges::OSL_FIRST && key.raw <= ranges::OSL_LAST) { - layer = key.raw - ranges::OSL_FIRST; - } else if (key.flags == (SYNTHETIC | SWITCH_TO_KEYMAP)) { - layer = key.keyCode; - if (layer >= MOMENTARY_OFFSET) - layer -= MOMENTARY_OFFSET; - } - - if (layer == 255) - return 0; - - if (Layer.isOn(layer)) - return 2; - else - return 1; -} - void ActiveModColorEffect::loopHook(bool is_post_clear) { if (is_post_clear) return; @@ -73,16 +37,27 @@ void ActiveModColorEffect::loopHook(bool is_post_clear) { for (byte r = 0; r < ROWS; r++) { for (byte c = 0; c < COLS; c++) { Key k = Layer.lookupOnActiveLayer(r, c); - uint8_t is_mod = isModifierKeyActive(k); - uint8_t is_layer = isLayerKeyActive(k); - - if (!is_mod && !is_layer) // Neither mod, nor layer key - continue; - if (is_mod == 2 || is_layer == 2) - ::LEDControl.setCrgbAt(r, c, highlight_color); - else - ::LEDControl.refreshAt(r, c); + if (::OneShot.isOneShotKey(k)) { + 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 >= MOMENTARY_OFFSET) + layer -= MOMENTARY_OFFSET; + + if (Layer.isOn(layer)) + ::LEDControl.setCrgbAt(r, c, highlight_color); + else + ::LEDControl.refreshAt(r, c); + } } } } diff --git a/src/Kaleidoscope/LED-ActiveModColor.h b/src/Kaleidoscope/LED-ActiveModColor.h index c38eb604..2d0a05b6 100644 --- a/src/Kaleidoscope/LED-ActiveModColor.h +++ b/src/Kaleidoscope/LED-ActiveModColor.h @@ -31,8 +31,6 @@ class ActiveModColorEffect : public KaleidoscopePlugin { static cRGB highlight_color; private: - static uint8_t isModifierKeyActive(Key key); - static uint8_t isLayerKeyActive(Key key); static void loopHook(bool is_post_clear); }; }