Introduce a new event: onLayerChange #363
Conversation
|
I plan to use this in |
|
In theory, this make sense. :) I'd love to know it it works in practice. |
|
Tested with colormap, and... it doesn't buy us much there. The code becomes a little simpler, but speed is about the same (except I see higher latencies with the Might be easier if |
|
Actually, scratch that. I see the spikes in both cases, so it is unrelated to |
|
keyboardio/Kaleidoscope-Colormap@a7f6ebff1be373544743e79743267daf65b9b398 shows how Colormap would be ported to I expect |
|
keyboardio/Kaleidoscope-LED-ActiveModColor@ac3527b6e7c85e1b99fc3487689572d51bac3cac is the That one buys us ~0.7ms in the idle case, and ~0.3ms with a bunch of modifiers pressed, at the cost of 138 bytes of PROGMEM, and 17 bytes of RAM. I think the speedup is worth this much. Without |
|
There are currently no other plugins (under the keyboardio umbrella) that would benefit from this event. |
|
Another use case for this is user code, if someone wants to implement their own coloring logic, for example, then this makes it to do that in a simple way. It can be used to send back events to the host, which can then display a notification or somesuch. I'll rebase this PR on top of current master. |
algernon
force-pushed
the
f/onLayerChange
branch
3 times, most recently
from
6b26a90
to
900aa9f
Compare
|
The PR now includes the Colormap and ActiveModColor changes too. |
|
(At this point, do you propose this for merge?)
ᐧ
…On Fri, Nov 16, 2018 at 6:39 AM Gergely Nagy ***@***.***> wrote:
The PR now includes the Colormap and ActiveModColor changes too.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#363 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACxaPnsq5JCRa5jLTcX2izel00mqYxMks5uvs4igaJpZM4XNFtg>
.
|
|
As soon as I rebased and fixed the conflicts - yes. |
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 <algernon@keyboard.io>
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 <algernon@keyboard.io>
…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 <algernon@keyboard.io>
algernon
force-pushed
the
f/onLayerChange
branch
from
900aa9f
to
38681b9
Compare
|
Rebased, ready to merge in my opinion. |
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.