Merge pull request #1013 from gedankenexperimenter/f/core.floating-base-layer

Allow base layer to float

src/kaleidoscope/layers.cpp

@@ -59,14 +59,17 @@ void Layer_::setup() {
 
 void Layer_::handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) {
   if (keymapEntry.getKeyCode() >= LAYER_MOVE_OFFSET) {
+    // MoveToLayer()
     if (keyToggledOn(keyState)) {
       move(keymapEntry.getKeyCode() - LAYER_MOVE_OFFSET);
     }
   } else if (keymapEntry.getKeyCode() >= LAYER_SHIFT_OFFSET) {
+    // layer shift keys
     uint8_t target = keymapEntry.getKeyCode() - LAYER_SHIFT_OFFSET;
 
     switch (target) {
     case KEYMAP_NEXT:
+      // Key_KeymapNext_Momentary
       if (keyToggledOn(keyState))
         activateNext();
       else if (keyToggledOff(keyState))
@@ -74,6 +77,7 @@ void Layer_::handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) {
       break;
 
     case KEYMAP_PREVIOUS:
+      // Key_KeymapPrevious_Momentary
       if (keyToggledOn(keyState))
         deactivateMostRecent();
       else if (keyToggledOff(keyState))
@@ -81,6 +85,7 @@ void Layer_::handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) {
       break;
 
     default:
+      // ShiftToLayer()
       /* 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.
@@ -104,8 +109,10 @@ void Layer_::handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) {
       break;
     }
   } else if (keyToggledOn(keyState)) {
+    // LockLayer()/UnlockLayer()
+    uint8_t target = keymapEntry.getKeyCode();
     // switch keymap and stay there
-    if (Layer.isActive(keymapEntry.getKeyCode()) && keymapEntry.getKeyCode())
+    if (Layer.isActive(target))
       deactivate(keymapEntry.getKeyCode());
     else
       activate(keymapEntry.getKeyCode());
@@ -130,22 +137,28 @@ void Layer_::updateLiveCompositeKeymap(KeyAddr key_addr) {
 }
 
 void Layer_::updateActiveLayers(void) {
+  // First, set every entry in the active layer keymap to point to the default
+  // layer (layer 0).
   memset(active_layer_keymap_, 0, Runtime.device().numKeys());
+
+  // For each key address, set its entry in the active layer keymap to the value
+  // of the top active layer that has a non-transparent entry for that address.
   for (auto key_addr : KeyAddr::all()) {
-    int8_t layer_index = active_layer_count_;
+    for (uint8_t i = active_layer_count_; i > 0; --i) {
-    while (layer_index > 0) {
+      uint8_t layer = active_layers_[i - 1];
-      uint8_t layer = active_layers_[layer_index - 1];
+      Key key = (*getKey)(layer, key_addr);
-      if (Layer.isActive(layer)) {
+
-        Key mappedKey = (*getKey)(layer, key_addr);
+      if (key != Key_Transparent) {
-
+        active_layer_keymap_[key_addr.toInt()] = layer;
-        if (mappedKey != Key_Transparent) {
+        break;
-          active_layer_keymap_[key_addr.toInt()] = layer;
-          break;
-        }
       }
-      layer_index--;
     }
   }
+  // Even if there are no active layers (a situation that should be prevented by
+  // `deactivate()`), each key will be mapped from the base layer (layer
+  // 0). Likewise, for any address where all active layers have a transparent
+  // entry, that key will be mapped from the base layer, even if the base layer
+  // has been deactivated.
 }
 
 void Layer_::move(uint8_t layer) {
@@ -177,7 +190,8 @@ void Layer_::activate(uint8_t layer) {
   if (isActive(layer))
     return;
 
-  // Otherwise, turn on its bit in layer_state_
+  // Otherwise, turn on its bit in layer_state_, and push it onto the active
+  // layer stack
   bitSet(layer_state_, layer);
   active_layers_[active_layer_count_++] = layer;
 
@@ -191,22 +205,28 @@ void Layer_::activate(uint8_t layer) {
 // Deactivate a given layer
 void Layer_::deactivate(uint8_t layer) {
   // If the target layer was already off, return
-  if (!bitRead(layer_state_, layer))
+  if (!isActive(layer))
     return;
 
+  // If the sole active layer is being deactivated, turn on the base layer and
+  // return so we always have at least one layer active.
+  if (active_layer_count_ <= 1) {
+    move(0);
+    return;
+  }
+
   // Turn off its bit in layer_state_
   bitClear(layer_state_, layer);
 
-  // Rearrange the activation order array...
+  // Remove the target layer from the active layer stack, and shift any layers
-  uint8_t idx = 0;
+  // above it down to fill in the gap
-  for (uint8_t i = active_layer_count_; i > 0; i--) {
+  for (uint8_t i = 0; i < active_layer_count_; ++i) {
     if (active_layers_[i] == layer) {
-      idx = i;
+      memmove(&active_layers_[i], &active_layers_[i + 1], active_layer_count_ - i);
+      --active_layer_count_;
       break;
     }
   }
-  memmove(&active_layers_[idx], &active_layers_[idx + 1], active_layer_count_ - idx);
-  active_layer_count_--;
 
   // Update the keymap cache (but not live_composite_keymap_; that gets
   // updated separately, when keys toggle on or off. See layers.h)

tests/features/layers/floating-base/floating-base.ino

@@ -0,0 +1,80 @@
+/* -*- mode: c++ -*-
+ * Copyright (C) 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
+ * Foundation, version 3.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <Kaleidoscope.h>
+
+// *INDENT-OFF*
+KEYMAPS(
+    [0] = KEYMAP_STACKED
+    (
+        Key_A, ___, ___, ___, ___, ___, ___,
+        ShiftToLayer(0), ShiftToLayer(1), ShiftToLayer(2), ___, ___, ___, ___,
+        LockLayer(0), LockLayer(1), LockLayer(2), ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___,
+        ___,
+
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___,
+        ___
+    ),
+    [1] = KEYMAP_STACKED
+    (
+        Key_B, Key_X, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___,
+        ___,
+
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___,
+        ___
+    ),
+    [2] = KEYMAP_STACKED
+    (
+        Key_C, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___,
+        ___,
+
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___, ___, ___, ___,
+        ___, ___, ___, ___,
+        ___
+    ),
+)
+// *INDENT-ON*
+
+//KALEIDOSCOPE_INIT_PLUGINS();
+
+void setup() {
+  Kaleidoscope.setup();
+}
+
+void loop() {
+  Kaleidoscope.loop();
+}

tests/features/layers/floating-base/sketch.json

@@ -0,0 +1,6 @@
+{
+  "cpu": {
+    "fqbn": "keyboardio:virtual:model01",
+    "port": ""
+  }
+}

tests/features/layers/floating-base/test.ktest

@@ -0,0 +1,165 @@
+VERSION 1
+
+KEYSWITCH A              0 0
+KEYSWITCH X              0 1
+
+KEYSWITCH LAYER_SHIFT_0  1 0
+KEYSWITCH LAYER_SHIFT_1  1 1
+KEYSWITCH LAYER_SHIFT_2  1 2
+
+KEYSWITCH LAYER_LOCK_0   2 0
+KEYSWITCH LAYER_LOCK_1   2 1
+KEYSWITCH LAYER_LOCK_2   2 2
+
+# ==============================================================================
+# Previously, if only layer 0 was active, and the user pressed and then released
+# `ShiftToLayer(0)`, the layer count would go to zero. This test would still
+# pass, however, because Kaleidoscope falls back on layer 0 values anyway.
+NAME Base layer stays active
+
+RUN 5 cycles
+
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_A # Report should contain only `A`
+RUN 5 ms
+
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty # Report should be empty
+RUN 5 ms
+
+PRESS LAYER_SHIFT_0
+RUN 5 ms
+RELEASE LAYER_SHIFT_0
+RUN 5 ms
+
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_A # Report should contain only `A`
+RUN 5 ms
+
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty # Report should be empty
+RUN 5 ms
+
+# ==============================================================================
+NAME Lock layer 1
+
+# Activate Layer 1
+PRESS LAYER_LOCK_1
+RUN 5 ms
+RELEASE LAYER_LOCK_1
+RUN 5 ms
+
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_B # Report should contain only `B`
+RUN 5 ms
+
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty # Report should be empty
+RUN 5 ms
+
+# ==============================================================================
+NAME Stack layer 0 on top of layer 1
+
+# Deactivate Layer 0: Now the layer stack should contain only layer 1.
+PRESS LAYER_LOCK_0
+RUN 5 ms
+RELEASE LAYER_LOCK_0
+RUN 5 ms
+
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_B # Report should contain only `B`
+RUN 5 ms
+
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty # Report should be empty
+RUN 5 ms
+
+# Activate Layer 0: There should now be two layers in the stack, with layer 0 on
+# top of layer 1.
+PRESS LAYER_LOCK_0
+RUN 5 ms
+RELEASE LAYER_LOCK_0
+RUN 5 ms
+
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_A # Report should contain only `A`
+RUN 5 ms
+
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty # Report should be empty
+RUN 5 ms
+
+# Deactivate Layer 0: Back to just layer 1 active.
+PRESS LAYER_LOCK_0
+RUN 5 ms
+RELEASE LAYER_LOCK_0
+RUN 5 ms
+
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_B # Report should contain only `B`
+RUN 5 ms
+
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty # Report should be empty
+RUN 5 ms
+
+# Hold Layer 0: Now the same test, but with a layer shift instead of a layer
+# lock. The stack should now have layer 0 on top of layer 1.
+PRESS LAYER_SHIFT_0
+RUN 5 ms
+
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_A # Report should contain only `A`
+RUN 5 ms
+
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty # Report should be empty
+RUN 5 ms
+
+# Release Layer 0 shift: Back to just layer 1 active.
+RELEASE LAYER_SHIFT_0
+RUN 5 ms
+
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_B # Report should contain only `B`
+RUN 5 ms
+
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty # Report should be empty
+RUN 5 ms
+
+# ==============================================================================
+NAME Default to layer 0
+
+# Unlock layer 1: This was the only active layer; layer 0 should activate
+# automatically in response.
+PRESS LAYER_LOCK_1
+RUN 5 ms
+RELEASE LAYER_LOCK_1
+RUN 5 ms
+
+PRESS A
+RUN 1 cycle
+EXPECT keyboard-report Key_A # Report should contain only `A`
+RUN 5 ms
+
+RELEASE A
+RUN 1 cycle
+EXPECT keyboard-report empty # Report should be empty
+RUN 5 ms