fallback keymap

docs/UPGRADING.md

@@ -442,6 +442,13 @@ As a developer, one can continue using `millis()`, but migrating to `Kaleidoscop
 
 ## Breaking changes
 
+### `EEPROMSettings` key layout fallback
+
+When detecting stored settings that might be incompatible with the current firmware, `EEPROMSettings` will attempt to fall back to a usable key layout by temporarily forcing `settings.defaultLayer 0` and `keymap.onlyCustom 0`.
+These have the effect of loading the hardcoded firmware key layout, regardless of what the previous settings were.
+This should reduce the incidence of scrambled key layouts when using Arduino to upgrade to custom firmware from Chrysalis firmware, for example.
+
+This fallback can be a breaking change for people who have relied on being able to use non-Chrysalis means to upgrade to firmware that has a different, but compatible layout.
 ### Sketch preprocssing system
 
 We used to support the ability to amend all compiled sketches by

examples/Devices/Keyboardio/Atreus/Atreus.ino

@@ -151,8 +151,6 @@ void setup() {
 
   LayerNames.reserve_storage(63);
 
-  Layer.move(EEPROMSettings.default_layer());
-
   // To avoid any surprises, SpaceCadet is turned off by default. However, it
   // can be permanently enabled via Chrysalis, so we should only disable it if
   // no configuration exists.

examples/Features/EEPROM/EEPROM-Keymap-Programmer/EEPROM-Keymap-Programmer.ino

@@ -62,7 +62,7 @@ void setup() {
 
   Layer.getKey = EEPROMKeymap.getKey;
 
-  EEPROMKeymap.max_layers(1);
+  EEPROMKeymap.setup(1);
   EEPROMSettings.seal();
 }
 

plugins/Kaleidoscope-EEPROM-Keymap-Programmer/README.md

@@ -74,7 +74,7 @@ void setup() {
 
   Layer.getKey = EEPROMKeymap.getKey;
 
-  EEPROMKeymap.max_layers(1);
+  EEPROMKeymap.setup(1);
   EEPROMSettings.seal();
 }
 ```

plugins/Kaleidoscope-EEPROM-Keymap/README.md

@@ -6,7 +6,7 @@ In short, this plugin allows us to change our keymaps, without having to compile
 
  [plugin:focusSerial]: Kaleidoscope-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](Kaleidoscope-EEPROM-Settings.md) documentation for more information).
+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.ignoreHardcodedLayers` (see the [EEPROMSettings](Kaleidoscope-EEPROM-Settings.md) documentation for more information).
 
 ## Using the plugin
 
@@ -39,7 +39,7 @@ The plugin provides the `EEPROMKeymap` object, which has the following method:
 
 ## Focus commands
 
-The plugin provides three Focus commands: `keymap.default`, `keymap.custom`, and `keymap.useCustom`.
+The plugin provides three Focus commands: `keymap.default`, `keymap.custom`, and `keymap.onlyCustom`.
 
 ### `keymap.default`
 
@@ -58,6 +58,8 @@ The plugin provides three Focus commands: `keymap.default`, `keymap.custom`, and
 > 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.
+>
+> This setting is forced to `0` (but not written to EEPROM) if invalid settings are detected.
 
 ## Dependencies
 

plugins/Kaleidoscope-EEPROM-Keymap/src/kaleidoscope/plugin/EEPROM-Keymap.cpp

@@ -34,9 +34,9 @@ namespace plugin {
 uint16_t EEPROMKeymap::keymap_base_;
 uint8_t EEPROMKeymap::max_layers_;
 uint8_t EEPROMKeymap::progmem_layers_;
+bool EEPROMKeymap::sealed_;
 
 EventHandlerResult EEPROMKeymap::onSetup() {
-  ::EEPROMSettings.onSetup();
   progmem_layers_ = layer_count;
   return EventHandlerResult::OK;
 }
@@ -45,20 +45,33 @@ EventHandlerResult EEPROMKeymap::onNameQuery() {
   return ::Focus.sendName(F("EEPROMKeymap"));
 }
 
-void EEPROMKeymap::setup(uint8_t max) {
-  layer_count = max;
-  if (::EEPROMSettings.ignoreHardcodedLayers()) {
+inline void EEPROMKeymap::set_layers(bool ignore_hardcoded) {
+  layer_count = max_layers_;
+  if (ignore_hardcoded) {
     Layer.getKey = getKey;
   } else {
     layer_count += progmem_layers_;
     Layer.getKey = getKeyExtended;
   }
-  max_layers(max);
 }
 
-void EEPROMKeymap::max_layers(uint8_t max) {
+void EEPROMKeymap::setup(uint8_t max) {
   max_layers_  = max;
   keymap_base_ = ::EEPROMSettings.requestSlice(max_layers_ * Runtime.device().numKeys() * 2);
+  set_layers(::EEPROMSettings.ignoreHardcodedLayers());
+}
+
+/*
+ * EEPROMSettings.ignoreHardcodedLayers() might change, for example,
+ * if EEPROMSettings.seal() detects corruption. Force hardcoded layers
+ * to avoid scrambled keymaps in some firmware update situations.
+ */
+EventHandlerResult EEPROMKeymap::beforeEachCycle() {
+  if (!sealed_) {
+    sealed_ = true;
+    set_layers(::EEPROMSettings.ignoreHardcodedLayers());
+  }
+  return EventHandlerResult::OK;
 }
 
 Key EEPROMKeymap::getKey(uint8_t layer, KeyAddr key_addr) {
@@ -117,14 +130,7 @@ EventHandlerResult EEPROMKeymap::onFocusEvent(const char *input) {
 
       ::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;
-      }
+      set_layers(v);
     }
     return EventHandlerResult::EVENT_CONSUMED;
   }

plugins/Kaleidoscope-EEPROM-Keymap/src/kaleidoscope/plugin/EEPROM-Keymap.h

@@ -36,11 +36,10 @@ class EEPROMKeymap : public kaleidoscope::Plugin {
   EventHandlerResult onSetup();
   EventHandlerResult onNameQuery();
   EventHandlerResult onFocusEvent(const char *input);
+  EventHandlerResult beforeEachCycle();
 
   static void setup(uint8_t max);
 
-  static void max_layers(uint8_t max);
-
   static uint16_t keymap_base();
 
   static Key getKey(uint8_t layer, KeyAddr key_addr);
@@ -52,10 +51,12 @@ class EEPROMKeymap : public kaleidoscope::Plugin {
   static uint16_t keymap_base_;
   static uint8_t max_layers_;
   static uint8_t progmem_layers_;
+  static bool sealed_;
 
   static Key parseKey();
   static void printKey(Key key);
   static void dumpKeymap(uint8_t layers, Key (*getkey)(uint8_t, KeyAddr));
+  static inline void set_layers(bool ignore_hardcoded);
 };
 
 }  // namespace plugin

plugins/Kaleidoscope-EEPROM-Settings/README.md

@@ -65,6 +65,8 @@ The plugin provides the `EEPROMSettings` object, which has the following methods
 > any.
 >
 > Setting it to `126` or anything higher disables the automatic switching.
+>
+> This setting is forced to `0` (but not written to EEPROM) if invalid settings are detected, to ensure a usable key layout.
 
 ### `ignoreHardcodedLayers([true|false])`
 
@@ -80,6 +82,8 @@ The plugin provides the `EEPROMSettings` object, which has the following methods
 > implemented by the [EEPROM-Keymap][EEPROM-Keymap.md] plugin.
 >
 > Defaults to `false`.
+>
+> This setting is forced to `false` (but not written to EEPROM) if invalid settings are detected, to ensure a usable key layout.
 
 ### `seal()`
 
@@ -118,6 +122,11 @@ The plugin provides the `EEPROMSettings` object, which has the following methods
 >
 > This is for internal use only, end-users should not need to care about it.
 
+### `accept_invalid()`
+
+> Accepts and stores the calculated settings CRC as valid, even if different from the previously stored CRC.
+> Use this to allow a user to accept existing invalid settings, even if possibly corrupted by an EEPROM layout change.
+
 ### `crc()`
 
 > Returns the CRC checksum of the layout. Should only be used after calling
@@ -146,6 +155,8 @@ following commands:
 >
 > This is the Focus counterpart of the `default_layer()` method documented
 > above.
+>
+> This setting is forced to `0` (but not stored) if invalid settings are detected, to ensure a usable key layout.
 
 ### `settings.crc`
 
@@ -155,6 +166,9 @@ following commands:
 
 > Returns either `true` or `false`, depending on whether the sealed settings are
 > to be considered valid or not.
+>
+> When given an argument of `1`, accepts the current settings as valid, regardless of whether the settings layout matches.
+> Accepting also stores the current checksum, so it will be considered valid in the future.
 
 ### `settings.version`
 

plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.cpp

@@ -30,6 +30,11 @@
 namespace kaleidoscope {
 namespace plugin {
 
+inline void EEPROMSettings::fallback_layers() {
+  settings_.ignore_hardcoded_layers = false;
+  settings_.default_layer           = 0;
+}
+
 EventHandlerResult EEPROMSettings::onSetup() {
   Runtime.storage().get(0, settings_);
 
@@ -41,8 +46,7 @@ EventHandlerResult EEPROMSettings::onSetup() {
          and setting sensible defaults is safe. If either of them is not at it's
          uninitialized state, we do not override them, to avoid overwriting user
          settings. */
-      settings_.ignore_hardcoded_layers = false;
-      settings_.default_layer           = 0;
+      fallback_layers();
     }
 
     /* If the version is undefined, we'll set it to our current one. */
@@ -107,18 +111,17 @@ void EEPROMSettings::seal() {
 
   if (settings_.version != VERSION_CURRENT) {
     is_valid_ = false;
-    return;
+  } else if (settings_.crc == 0xffff) {
+    accept_invalid();
+  } else if (settings_.crc == CRCCalculator.crc) {
+    is_valid_ = true;
   }
 
-  if (settings_.crc == 0xffff) {
-    settings_.crc = CRCCalculator.crc;
-    update();
-  } else if (settings_.crc != CRCCalculator.crc) {
-    return;
+  // Fall back to hardcoded layers if settings might be corrupt
+  if (!is_valid_) {
+    fallback_layers();
   }
 
-  is_valid_ = true;
-
   /* If we have a default layer set, switch to it.
    *
    * We check if the layer is smaller than IGNORE_HARDCODED_LAYER (0x7e),
@@ -147,6 +150,12 @@ void EEPROMSettings::invalidate() {
   is_valid_ = false;
 }
 
+// Accept possibly corrupt settings, hopefully after user review
+void EEPROMSettings::accept_invalid() {
+  settings_.crc = ::CRCCalculator.crc;
+  update();
+}
+
 uint16_t EEPROMSettings::used() {
   return next_start_;
 }
@@ -165,6 +174,7 @@ EventHandlerResult FocusSettingsCommand::onFocusEvent(const char *input) {
     GET_VERSION,
     GET_CRC,
   } sub_command;
+  uint8_t v;
 
   const char *cmd_defaultLayer = PSTR("settings.defaultLayer");
   const char *cmd_isValid      = PSTR("settings.valid?");
@@ -190,14 +200,20 @@ EventHandlerResult FocusSettingsCommand::onFocusEvent(const char *input) {
     if (::Focus.isEOL()) {
       ::Focus.send(::EEPROMSettings.default_layer());
     } else {
-      uint8_t layer;
-      ::Focus.read(layer);
-      ::EEPROMSettings.default_layer(layer);
+      ::Focus.read(v);
+      ::EEPROMSettings.default_layer(v);
     }
     break;
   }
   case IS_VALID:
-    ::Focus.send(::EEPROMSettings.isValid());
+    // `isEOL()` not needed, because `read()` will store 0 on early EOL
+    ::Focus.read(v);
+    if (v == 1) {
+      // Accept possibly corrupt settings, hopefully after user review
+      ::EEPROMSettings.accept_invalid();
+    } else {
+      ::Focus.send(::EEPROMSettings.isValid());
+    }
     break;
   case GET_VERSION:
     ::Focus.send(::EEPROMSettings.version());

plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.h

@@ -59,6 +59,7 @@ class EEPROMSettings : public kaleidoscope::Plugin {
   uint8_t version() {
     return settings_.version;
   }
+  void accept_invalid();
 
   uint16_t requestSlice(uint16_t size);
   void seal();
@@ -80,6 +81,7 @@ class EEPROMSettings : public kaleidoscope::Plugin {
   uint16_t next_start_ = sizeof(EEPROMSettings::Settings);
   bool is_valid_;
   bool sealed_;
+  inline void fallback_layers();
 
   Settings settings_;
 };