From b218143faf5975b5e5b061a3172575338cf75c36 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Sun, 13 Aug 2017 11:15:16 +0200 Subject: [PATCH 1/3] When a momentary layer is held, reactivate the layer if it turns off If we have two keys on our keymap that momentarily go to the same layer (which is the case for the factory firmware), we hold both, and release one, we want the layer to remain active still. To this effect, in `handleKeymapKeyswitchEvent` we will handle the case when a momentary layer key is pressed, but not toggled on (that is, it is held): if it is not a next/previous switch, we re-activate the layer if it wasn't on. This fixes #154, thanks to @ToyKeeper for the report. Signed-off-by: Gergely Nagy --- src/layers.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/layers.cpp b/src/layers.cpp index 765615e1..0e046eab 100644 --- a/src/layers.cpp +++ b/src/layers.cpp @@ -20,6 +20,11 @@ static void handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) { } else { Layer.on(target); } + } else if (keyIsPressed(keyState) && + target != KEYMAP_NEXT && + target != KEYMAP_PREVIOUS) { + if (!Layer.isOn(target)) + Layer.on(target); } if (keyToggledOff(keyState)) { if (target == KEYMAP_NEXT) { From c88062a2432ec49694522cf37f8574486c13c4e8 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Sun, 13 Aug 2017 11:24:25 +0200 Subject: [PATCH 2/3] layers: Code refactoring Refactor the momentary layer handling part of `handleKeymapKeyswitchEvent`. Instead of a bunch of ifs that are increasingly hard to follow, use a switch based on the target layer, and branch out depending on `keyState` from there. Makes it easier to follow what happens. Signed-off-by: Gergely Nagy --- src/layers.cpp | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/layers.cpp b/src/layers.cpp index 0e046eab..e453b51a 100644 --- a/src/layers.cpp +++ b/src/layers.cpp @@ -12,32 +12,32 @@ static void handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) { if (keymapEntry.keyCode >= MOMENTARY_OFFSET) { uint8_t target = keymapEntry.keyCode - MOMENTARY_OFFSET; - if (keyToggledOn(keyState)) { - if (target == KEYMAP_NEXT) { + switch (target) { + case KEYMAP_NEXT: + if (keyToggledOn(keyState)) Layer.next(); - } else if (target == KEYMAP_PREVIOUS) { + else if (keyToggledOff(keyState)) Layer.previous(); - } else { - Layer.on(target); - } - } else if (keyIsPressed(keyState) && - target != KEYMAP_NEXT && - target != KEYMAP_PREVIOUS) { - if (!Layer.isOn(target)) - Layer.on(target); - } - if (keyToggledOff(keyState)) { - if (target == KEYMAP_NEXT) { + break; + + case KEYMAP_PREVIOUS: + if (keyToggledOn(keyState)) Layer.previous(); - } else if (target == KEYMAP_PREVIOUS) { + else if (keyToggledOff(keyState)) Layer.next(); - } else { + break; + + default: + if (keyIsPressed(keyState)) { + if (!Layer.isOn(target)) + Layer.on(target); + } else if (keyToggledOff(keyState)) { Layer.off(target); } + break; } - - // switch keymap and stay there } else if (keyToggledOn(keyState)) { + // switch keymap and stay there if (Layer.isOn(keymapEntry.keyCode) && keymapEntry.keyCode) Layer.off(keymapEntry.keyCode); else From 33dc5931510aae29e10abec0929e1a36d9fad368 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Sun, 13 Aug 2017 19:04:26 +0200 Subject: [PATCH 3/3] layers: Add a comment about the momentary layer handling Signed-off-by: Gergely Nagy --- src/layers.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/layers.cpp b/src/layers.cpp index e453b51a..369dafcc 100644 --- a/src/layers.cpp +++ b/src/layers.cpp @@ -28,6 +28,20 @@ static void handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) { break; default: + /* The default case is when we are switching to a layer by its number, and + * is a bit more complicated than switching there when the key toggles on, + * and away when it toggles off. + * + * We want to handle the case where we have more than one momentary layer + * key on our keymap that point to the same target layer, and we hold + * both, and release one. In this case, the layer should remain active, + * because the second momentary key is still held. + * + * To do this, we turn the layer back on if the switcher key is still + * held, not only when it toggles on. So when one of them is released, + * that does turn the layer off, but with the other still being held, the + * layer will toggle back on in the same cycle. + */ if (keyIsPressed(keyState)) { if (!Layer.isOn(target)) Layer.on(target);