diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 5383f743ea..02ed3b46df 100644 --- a/docs/UPGRADING.md +++ b/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 diff --git a/examples/Devices/Keyboardio/Atreus/Atreus.ino b/examples/Devices/Keyboardio/Atreus/Atreus.ino index 81c1801ac8..5942d85979 100644 --- a/examples/Devices/Keyboardio/Atreus/Atreus.ino +++ b/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. diff --git a/examples/Features/EEPROM/EEPROM-Keymap-Programmer/EEPROM-Keymap-Programmer.ino b/examples/Features/EEPROM/EEPROM-Keymap-Programmer/EEPROM-Keymap-Programmer.ino index e614907baa..6270f7c9e4 100644 --- a/examples/Features/EEPROM/EEPROM-Keymap-Programmer/EEPROM-Keymap-Programmer.ino +++ b/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(); } diff --git a/plugins/Kaleidoscope-EEPROM-Keymap-Programmer/README.md b/plugins/Kaleidoscope-EEPROM-Keymap-Programmer/README.md index 18c7244ebc..3c2fa8676c 100644 --- a/plugins/Kaleidoscope-EEPROM-Keymap-Programmer/README.md +++ b/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(); } ``` diff --git a/plugins/Kaleidoscope-EEPROM-Keymap/README.md b/plugins/Kaleidoscope-EEPROM-Keymap/README.md index 6cafeffcf2..cc53c11165 100644 --- a/plugins/Kaleidoscope-EEPROM-Keymap/README.md +++ b/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 diff --git a/plugins/Kaleidoscope-EEPROM-Keymap/src/kaleidoscope/plugin/EEPROM-Keymap.cpp b/plugins/Kaleidoscope-EEPROM-Keymap/src/kaleidoscope/plugin/EEPROM-Keymap.cpp index c887dbfc4a..5dd90ce911 100644 --- a/plugins/Kaleidoscope-EEPROM-Keymap/src/kaleidoscope/plugin/EEPROM-Keymap.cpp +++ b/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; } diff --git a/plugins/Kaleidoscope-EEPROM-Keymap/src/kaleidoscope/plugin/EEPROM-Keymap.h b/plugins/Kaleidoscope-EEPROM-Keymap/src/kaleidoscope/plugin/EEPROM-Keymap.h index 125e59cdf3..4c755e3eee 100644 --- a/plugins/Kaleidoscope-EEPROM-Keymap/src/kaleidoscope/plugin/EEPROM-Keymap.h +++ b/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 diff --git a/plugins/Kaleidoscope-EEPROM-Settings/README.md b/plugins/Kaleidoscope-EEPROM-Settings/README.md index cc86df152f..6dd6d4a493 100644 --- a/plugins/Kaleidoscope-EEPROM-Settings/README.md +++ b/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` diff --git a/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.cpp b/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.cpp index 0424caff90..c9af222a68 100644 --- a/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.cpp +++ b/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()); diff --git a/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.h b/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.h index fdf4bceb94..745ab8032d 100644 --- a/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.h +++ b/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_; };