EEPROMKeymap redesign

Instead of having a single `keymap.map` Focus command that tries to be smart
about what layers it presents, we should have two separate commands: one to
query the defaults (PROGMEM), and one to get/set the custom keymap (EEPROM).
This makes `keymap.map` and `keymap.roLayers` obsolete, and they're now removed.

Furthermore, having to specify whether the EEPROM keymap extends the built-in
one or not proved to be unflexible. So we re-purposed the highest bit of the
first EEPROM byte, to signal whether we should use EEPROM layers only or not.

Signed-off-by: Gergely Nagy <algernon@keyboard.io>

NEWS.md

@@ -140,6 +140,10 @@ The [Redial](doc/plugin/Redial.md) plugin was simplified, one no longer needs to
 
 The [LED-Palette-Theme](doc/plugin/LED-Palette-Theme.md) had to be changed to store the palette colors in reverse. This change had to be made in order to not default to a bright white palette, that would draw so much power that most operating systems would disconnect the keyboard due to excessive power usage. With inverting the colors, we now default to a black palette instead. This sadly breaks existing palettes, and you will have to re-set the colors.
 
+### EEPROM-Keymap changed Focus commands
+
+The [EEPROMKeymap](doc/plugin/EEPROM-Keymap.md) plugin was changed to treat built-in (default) and EEPROM-stored (custom) layers separately, because that's less surprising, and easier to work with from Chrysalis. The old `keymap.map` and `keymap.roLayers` commands are gone, the new `keymap.default` and `keymap.custom` commands should be used instead.
+
 ## Bugfixes
 
 We fixed way too many issues to list here, so we're going to narrow it down to the most important, most visible ones.

UPGRADING.md

@@ -20,6 +20,7 @@ If any of this does not make sense to you, or you have trouble updating your .in
     - [Removal of Layer.defaultLayer](#removal-of-layerdefaultlayer)
     - [More clarity in Layer method names](#more-clarity-in-layer-method-names)
     - [Finer OneShot stickability control](#finer-oneshot-stickability-control)
+    - [EEPROMKeymap mode](#eepromkeymap-mode)
     - [Source code and namespace rearrangement](#source-code-and-namespace-rearrangement)
 * [Removed APIs](#removed-apis)
 
@@ -454,6 +455,10 @@ The goal was to have a method name that is a verb, because these are actions we
 
 The [OneShot plugin](doc/plugin/OneShot.md) has much improved stickability control. Instead of only being able to control if one-shot layers should be stickable too, or disabling the sticky feature in general, it is now possible to control stickiness on a per-key basis with the new `OneShot.enableStickability()` and `OneShot.disableStickablity()` methods.
 
+### EEPROMKeymap mode
+
+The [EEPROM-Keymap](doc/plugin/EEPROM-Keymap.md) plugin had its `setup()` method changed, the formerly optional `method` argument is now obsolete and unused. It can be safely removed. Supplying a second argument will continue to work until its scheduled removal by **2019-04-30**.
+
 ### Source code and namespace rearrangement
 
 With the move towards a monorepo-based source, some headers have moved to a new location, and plenty of plugins moved to a new namespace (`kaleidoscope::plugin`). This means that the old headers, and some old names are deprecated. The old names no longer work.

doc/plugin/EEPROM-Keymap.md

@@ -6,6 +6,8 @@ In short, this plugin allows us to change our keymaps, without having to compile
 
  [plugin:focusSerial]: FocusSerial.md
 
+By default, the plugin extends the keymap in PROGMEM: it will only look for keys in EEPROM if looking up from a layer that's higher than the last one in PROGMEM. This behaviour can be changed either via `Focus` (see below), or by calling `EEPROMSettings.use_eeprom_layers_only` (see the [EEPROMSettings](EEPROM-Settings.md) documentation for more information).
+
 ## Using the plugin
 
 Using the plugin is reasonably simple: after including the header, enable the plugin, and configure how many layers at most we want to store in `EEPROM`. There are other settings one can tweak, but these two steps are enough to get started with.
@@ -31,27 +33,31 @@ void setup() {
 
 The plugin provides the `EEPROMKeymap` object, which has the following method:
 
-### `.setup(layers[, mode])`
+### `.setup(layers)`
 
-> Reserve space in EEPROM for up to `layers` layers, and set things up to work according to the specified `mode` (see below for a list of supported modes). To be called from the `setup` method of one's sketch.
+> Reserve space in EEPROM for up to `layers` layers, and set up the key lookup mechanism.
->
-> Supported modes are:
-> - `EEPROMKeymap.Mode::EXTEND`: Extend the keymap with layers from EEPROM, treating them as extensions of the main keymap embedded in the firmware. The first layer in EEPROM will have a number one higher than the last layer in PROGMEM. In this case, the total number of layers will be the number of them in PROGMEM plus `layers`.
-> - `EEPROMKeymap.Mode::CUSTOM`: For advanced use cases where the `EXTEND` mode is not appropriate. In this case, the plugin merely reserves a slice of EEPROM for the requested amount of layers, but does no other configuration - that's entirely up to the Sketch.
 
 ## Focus commands
 
-The plugin provides the `keymap.map` and a `keymap.roLayers` commands.
+The plugin provides three Focus commands: `keymap.default`, `keymap.custom`, and `keymap.useCustom`.
+
+### `keymap.default`
 
-### `keymap.map [codes...]`
+> Display the default keymap from PROGMEM. Each key is printed as its raw, 16-bit keycode.
+>
+> Unlike `keymap.custom`, this does not support updating, because PROGMEM is read-only.
+
+### `keymap.custom [codes...]`
 
-> Without arguments, displays the keymap currently in effect. Each key is printed as its raw, 16-bit keycode.
+> Without arguments, display the custom keymap stored in EEPROM. Each key is printed as its raw, 16-bit keycode.
 >
-> With arguments, it stores as many keys as given. One does not need to set all keys, on all layers: the command will start from the first key on the first layer, and go on as long as it has input. It will not go past the total amount of layers (that is, `layer_count`).
+> With arguments, it updates as many keys as given. One does not need to set all keys, on all layers: the command will start from the first key on the first layer (in EEPROM, which might be different than the first layer!), and go on as long as it has input. It will not go past the number of layers in EEPROM.
 
-### `keymap.roLayers`
+### `keymap.onlyCustom [0|1]`
 
-> Returns the number of read-only layers. This only makes sense for the `EEPROMKeymap.Mode::EXTEND` mode, where it returns the number of layers in PROGMEM. In any other case, it doesn't return anything, doing so is left for another event handler that understands what the correct value would be.
+> Without arguments, returns whether the firmware uses both the default and the custom layers (the default, `0`) or custom (EEPROM-stored) layers only (`1`).
+>
+> With an argument, sets whether to use custom layers only, or extend the built-in layers instead.
 
 ## Dependencies
 

doc/plugin/EEPROM-Settings.md

@@ -64,8 +64,22 @@ The plugin provides the `EEPROMSettings` object, which has the following methods
 > keyboard boots up, it will automatically switch to the configured layer - if
 > any.
 >
-> This is the Focus counterpart of the `default_layer()` method documented
+> Setting it to `126` or anything higher disables the automatic switching.
-> above.
+
+### `ignoreHardcodedLayers([true|false])`
+
+> Controls whether the hardcoded layers (in `PROGMEM`) are ignored or not.
+>
+> When not ignored, the custom layes (in `EEPROM`) extend the hardcoded ones.
+> When ignored, they replace the hardcoded set.
+>
+> Returns the setting if called without arguments, changes it to the desired
+> value if called with a boolean flag.
+>
+> This setting is exposed to Focus via the `keymap.onlyCustom` command
+> implemented by the [EEPROM-Keymap][EEPROM-Keymap.md] plugin.
+>
+> Defaults to `false`.
 
 ### `seal()`
 
@@ -129,7 +143,10 @@ following commands:
 
 > Sets or returns (if called without arguments) the ID of the default layer. If
 > set, the keyboard will automatically switch to the given layer when connected.
-> Setting it to `255` disables the automatic switching.
+> Setting it to `126` or anything higher disables the automatic switching.
+>
+> This is the Focus counterpart of the `default_layer()` method documented
+> above.
 
 ### `settings.crc`
 

src/kaleidoscope/plugin/EEPROM-Keymap.cpp

@@ -1,6 +1,6 @@
 /* -*- mode: c++ -*-
  * Kaleidoscope-EEPROM-Keymap -- EEPROM-based keymap support.
- * Copyright (C) 2017, 2018  Keyboard.io, Inc
+ * Copyright (C) 2017, 2018, 2019  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
@@ -21,7 +21,6 @@
 
 namespace kaleidoscope {
 namespace plugin {
-EEPROMKeymap::Mode EEPROMKeymap::mode_;
 uint16_t EEPROMKeymap::keymap_base_;
 uint8_t EEPROMKeymap::max_layers_;
 uint8_t EEPROMKeymap::progmem_layers_;
@@ -32,16 +31,14 @@ EventHandlerResult EEPROMKeymap::onSetup() {
   return EventHandlerResult::OK;
 }
 
-void EEPROMKeymap::setup(uint8_t max, Mode mode) {
+void EEPROMKeymap::setup(uint8_t max) {
-  switch (mode) {
+  layer_count = max;
-  case Mode::CUSTOM:
+  if (::EEPROMSettings.ignoreHardcodedLayers()) {
-    break;
+    Layer.getKey = getKey;
-  case Mode::EXTEND:
+  } else {
-    layer_count = progmem_layers_ + max;
+    layer_count += progmem_layers_;
     Layer.getKey = getKeyExtended;
-    break;
   }
-  mode_ = mode;
   max_layers(max);
 }
 
@@ -85,51 +82,63 @@ void EEPROMKeymap::updateKey(uint16_t base_pos, Key key) {
   EEPROM.update(keymap_base_ + base_pos * 2 + 1, key.keyCode);
 }
 
+void EEPROMKeymap::dumpKeymap(uint8_t layers, Key(*getkey)(uint8_t, byte, byte)) {
+  for (uint8_t layer = 0; layer < layers; layer++) {
+    for (uint8_t row = 0; row < ROWS; row++) {
+      for (uint8_t col = 0; col < COLS; col++) {
+        Key k = (*getkey)(layer, row, col);
+
+        ::Focus.send(k);
+      }
+    }
+  }
+}
+
 EventHandlerResult EEPROMKeymap::onFocusEvent(const char *command) {
-  const char *cmd = PSTR("keymap.map");
+  if (::Focus.handleHelp(command, PSTR("keymap.custom\nkeymap.default\nkeymap.onlyCustom")))
-  if (::Focus.handleHelp(command, PSTR("keymap.map\nkeymap.roLayers")))
     return EventHandlerResult::OK;
 
   if (strncmp_P(command, PSTR("keymap."), 7) != 0)
     return EventHandlerResult::OK;
 
-  if (strcmp_P(command + 7, PSTR("roLayers")) == 0) {
+  if (strcmp_P(command + 7, PSTR("onlyCustom")) == 0) {
-    if (mode_ != Mode::EXTEND)
+    if (::Focus.isEOL()) {
-      return EventHandlerResult::OK;
+      ::Focus.send((uint8_t)::EEPROMSettings.ignoreHardcodedLayers());
-    ::Focus.send(progmem_layers_);
+    } else {
+      bool v;
+
+      ::Focus.read((uint8_t &)v);
+      ::EEPROMSettings.ignoreHardcodedLayers(v);
+
+      layer_count = max_layers_;
+      if (v) {
+        Layer.getKey = getKey;
+      } else {
+        layer_count += progmem_layers_;
+        Layer.getKey = getKeyExtended;
+      }
+    }
+    return EventHandlerResult::EVENT_CONSUMED;
+  }
+
+  if (strcmp_P(command + 7, PSTR("default")) == 0) {
+    dumpKeymap(progmem_layers_, Layer.getKeyFromPROGMEM);
     return EventHandlerResult::EVENT_CONSUMED;
   }
 
-  if (strcmp_P(command + 7, PSTR("map")) != 0)
+  if (strcmp_P(command + 7, PSTR("custom")) != 0)
     return EventHandlerResult::OK;
 
   if (::Focus.isEOL()) {
-    for (uint8_t layer = 0; layer < layer_count; layer++) {
+    dumpKeymap(max_layers_, getKey);
-      for (uint8_t row = 0; row < ROWS; row++) {
-        for (uint8_t col = 0; col < COLS; col++) {
-          Key k = Layer.getKey(layer, row, col);
-
-          ::Focus.send(k);
-        }
-      }
-    }
   } else {
     uint16_t i = 0;
-    uint8_t layers = layer_count;
+
-    if (layers > 0)
+    while (!::Focus.isEOL() && (i < ROWS * COLS * max_layers_)) {
-      layers--;
-    while (!::Focus.isEOL() && (i < ROWS * COLS * layers)) {
       Key k;
 
       ::Focus.read(k);
-
+      updateKey(i, k);
-      if (mode_ == Mode::EXTEND) {
-        uint8_t layer = i / (ROWS * COLS);
-        if (layer >= progmem_layers_)
-          updateKey(i - (progmem_layers_ * ROWS * COLS), k);
-      } else {
-        updateKey(i, k);
-      }
       i++;
     }
   }

src/kaleidoscope/plugin/EEPROM-Keymap.h

@@ -1,6 +1,6 @@
 /* -*- mode: c++ -*-
  * Kaleidoscope-EEPROM-Keymap -- EEPROM-based keymap support.
- * Copyright (C) 2017, 2018  Keyboard.io, Inc
+ * Copyright (C) 2017, 2018, 2019  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
@@ -20,13 +20,17 @@
 #include <Kaleidoscope.h>
 #include <Kaleidoscope-EEPROM-Settings.h>
 
+#define _DEPRECATED_MESSAGE_EEPROM_KEYMAP_SETUP_MODE                       \
+  "The `mode` argument of EEPROMKeymap.setup() is deprecated and is not\n" \
+  "used anymore. You can remove it safely."
+
 namespace kaleidoscope {
 namespace plugin {
 class EEPROMKeymap : public kaleidoscope::Plugin {
  public:
   enum class Mode {
     CUSTOM,
-    EXTEND,
+    EXTEND
   };
 
   EEPROMKeymap(void) {}
@@ -34,7 +38,10 @@ class EEPROMKeymap : public kaleidoscope::Plugin {
   EventHandlerResult onSetup();
   EventHandlerResult onFocusEvent(const char *command);
 
-  static void setup(uint8_t max, Mode mode = Mode::EXTEND);
+  static void setup(uint8_t max);
+  static void setup(uint8_t max, Mode mode) DEPRECATED(EEPROM_KEYMAP_SETUP_MODE) {
+    setup(max);
+  }
 
   static void max_layers(uint8_t max);
 
@@ -49,10 +56,10 @@ class EEPROMKeymap : public kaleidoscope::Plugin {
   static uint16_t keymap_base_;
   static uint8_t max_layers_;
   static uint8_t progmem_layers_;
-  static Mode mode_;
 
   static Key parseKey(void);
   static void printKey(Key key);
+  static void dumpKeymap(uint8_t layers, Key(*getkey)(uint8_t, byte, byte));
 };
 }
 }

src/kaleidoscope/plugin/EEPROM-Settings.cpp

@@ -1,6 +1,6 @@
 /* -*- mode: c++ -*-
  * Kaleidoscope-EEPROM-Settings -- Basic EEPROM settings plugin for Kaleidoscope.
- * Copyright (C) 2017, 2018  Keyboard.io, Inc
+ * Copyright (C) 2017, 2018, 2019  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
@@ -54,21 +54,44 @@ uint8_t EEPROMSettings::default_layer(uint8_t layer) {
     Layer.move(layer);
     settings_.default_layer = layer;
   }
-  if (layer == 0xff) {
+
-    settings_.default_layer = layer;
+  /*
+   * We set default_layer to IGNORE_HARDCODED_LAYER_MASK (instead of `value`)
+   * because due to compatibility reasons, we might get passed 0xff (which
+   * conveniently matches our mask), yet, we want to set a different value to
+   * signal an explicit "no default".
+   */
+  if (layer & IGNORE_HARDCODED_LAYER_MASK) {
+    settings_.default_layer = IGNORE_HARDCODED_LAYER_MASK;
   }
   update();
   return settings_.default_layer;
 }
 
+void EEPROMSettings::ignoreHardcodedLayers(bool value) {
+  settings_.ignore_hardcoded_layers = value;
+  if (settings_.default_layer & IGNORE_HARDCODED_LAYER_MASK)
+    settings_.default_layer = IGNORE_HARDCODED_LAYER_MASK;
+  update();
+}
+
 void EEPROMSettings::seal(void) {
   sealed_ = true;
 
   CRC.finalize();
 
-  /* If we have a default layer set, switch to it. As 0xff is the default EEPROM
+  /* If we have a default layer set, switch to it.
-   * value, treat it as not having a default layer set. */
+   *
-  if (settings_.default_layer != 0xff)
+   * We use IGNORE_HARDCODED_LAYER_MASK, because we want to avoid setting a
+   * default layer in two cases:
+   *
+   * - When the EEPROM is uninitialized (0x7f)
+   * - When such layer switching is explicitly turned off (0x7e)
+   *
+   * In both cases, the bits in IGNORE_HARDCODED_LAYER_MASK are set, and the
+   * remaining bit is not important.
+   */
+  if (!(settings_.default_layer & IGNORE_HARDCODED_LAYER_MASK))
     Layer.move(settings_.default_layer);
 
   /* Until we set a version, consider the EEPROM contents flexible, and always

src/kaleidoscope/plugin/EEPROM-Settings.h

@@ -1,6 +1,6 @@
 /* -*- mode: c++ -*-
  * Kaleidoscope-EEPROM-Settings -- Basic EEPROM settings plugin for Kaleidoscope.
- * Copyright (C) 2017, 2018  Keyboard.io, Inc
+ * Copyright (C) 2017, 2018, 2019  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
@@ -44,14 +44,20 @@ class EEPROMSettings : public kaleidoscope::Plugin {
   static uint8_t default_layer() {
     return settings_.default_layer;
   }
+  static void ignoreHardcodedLayers(bool value);
+  static bool ignoreHardcodedLayers() {
+    return settings_.ignore_hardcoded_layers;
+  }
 
  private:
+  static constexpr uint8_t IGNORE_HARDCODED_LAYER_MASK = 0b1111110;
   static uint16_t next_start_;
   static bool is_valid_;
   static bool sealed_;
 
   static struct settings {
-    uint8_t default_layer;
+    uint8_t default_layer: 7;
+    bool ignore_hardcoded_layers: 1;
     uint8_t version;
     uint16_t crc;
   } settings_;