From b1517a4e81807043321512fbcc054bfbda7075df Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 6 Mar 2024 11:57:33 -0800 Subject: [PATCH 1/2] Refactor the previous refactoring to how we load eeprom slices and data, hopefully making the new code's behavior closer to the original code. --- .../src/kaleidoscope/plugin/AutoShiftConfig.cpp | 2 +- .../kaleidoscope/plugin/DefaultLEDModeConfig.cpp | 4 ++-- .../src/kaleidoscope/plugin/EEPROM-Settings.cpp | 15 +++++++++++---- .../src/kaleidoscope/plugin/EEPROM-Settings.h | 2 +- .../kaleidoscope/plugin/Escape-OneShot-Config.cpp | 4 ++-- .../src/kaleidoscope/plugin/IdleLEDs.cpp | 5 ++--- .../kaleidoscope/plugin/LEDBrightnessConfig.cpp | 8 +++----- .../src/kaleidoscope/plugin/MouseKeysConfig.cpp | 6 ++---- .../src/kaleidoscope/plugin/OneShotConfig.cpp | 5 ++--- .../src/kaleidoscope/plugin/PersistentLEDMode.cpp | 5 ++--- .../src/kaleidoscope/plugin/SpaceCadetConfig.cpp | 2 +- .../src/kaleidoscope/plugin/TypingBreaks.cpp | 5 ++--- 12 files changed, 31 insertions(+), 32 deletions(-) diff --git a/plugins/Kaleidoscope-AutoShift/src/kaleidoscope/plugin/AutoShiftConfig.cpp b/plugins/Kaleidoscope-AutoShift/src/kaleidoscope/plugin/AutoShiftConfig.cpp index 72078938bf..da92af24bd 100644 --- a/plugins/Kaleidoscope-AutoShift/src/kaleidoscope/plugin/AutoShiftConfig.cpp +++ b/plugins/Kaleidoscope-AutoShift/src/kaleidoscope/plugin/AutoShiftConfig.cpp @@ -33,7 +33,7 @@ namespace plugin { // AutoShift configurator EventHandlerResult AutoShiftConfig::onSetup() { - settings_base_ = ::EEPROMSettings.requestSliceAndData(&::AutoShift.settings_, sizeof(AutoShift::settings_)); + ::EEPROMSettings.requestSliceAndLoadData(sizeof(::AutoShift.settings_), &settings_base_, &::AutoShift.settings_); return EventHandlerResult::OK; } diff --git a/plugins/Kaleidoscope-DefaultLEDModeConfig/src/kaleidoscope/plugin/DefaultLEDModeConfig.cpp b/plugins/Kaleidoscope-DefaultLEDModeConfig/src/kaleidoscope/plugin/DefaultLEDModeConfig.cpp index 2a5356cad9..2da101a2e0 100644 --- a/plugins/Kaleidoscope-DefaultLEDModeConfig/src/kaleidoscope/plugin/DefaultLEDModeConfig.cpp +++ b/plugins/Kaleidoscope-DefaultLEDModeConfig/src/kaleidoscope/plugin/DefaultLEDModeConfig.cpp @@ -34,8 +34,8 @@ uint16_t DefaultLEDModeConfig::settings_base_; struct DefaultLEDModeConfig::settings DefaultLEDModeConfig::settings_; EventHandlerResult DefaultLEDModeConfig::onSetup() { - settings_base_ = ::EEPROMSettings.requestSliceAndData(&settings_, sizeof(settings_)); - if (::EEPROMSettings.isSliceValid(settings_base_, sizeof(settings_))) { + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(settings_), &settings_base_, &settings_); + if (success) { ::LEDControl.set_mode(settings_.default_mode_index); } return EventHandlerResult::OK; 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 8c09b5238b..240c49f900 100644 --- a/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.cpp +++ b/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.cpp @@ -161,15 +161,22 @@ void EEPROMSettings::update() { } // get a settings slice from the storage and stick it in the untyped struct -// Returns the slice start address or 0 if the if the EEPROM is not initialized or the settings are invalid) -uint16_t EEPROMSettings::requestSliceAndData(void *data, size_t size) { +// startAddress is the address of the start of the slice +// Returns true if the load was successful and false otherwise. +// Takes the size of storage we want, a pointer to the start address, and a pointer to the data structure for settings + +bool EEPROMSettings::requestSliceAndLoadData(size_t size, uint16_t *startAddress, void *data) { // Request the slice for the struct from storage uint16_t start = requestSlice(size); - if (isSliceValid(start, size)) { + *startAddress = start; + + if (!Runtime.storage().isSliceUninitialized(start, size)) { Runtime.storage().get(start, data); + return true; // Data was loaded successfully + } else { } - return start; + return false; // Data was not loaded } bool EEPROMSettings::isSliceValid(uint16_t start, size_t size) { 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 01d0e13c66..d6a06ab2b7 100644 --- a/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.h +++ b/plugins/Kaleidoscope-EEPROM-Settings/src/kaleidoscope/plugin/EEPROM-Settings.h @@ -73,8 +73,8 @@ class EEPROMSettings : public kaleidoscope::Plugin { bool ignoreHardcodedLayers() { return settings_.ignore_hardcoded_layers; } + bool requestSliceAndLoadData(size_t size, uint16_t *startAddress, void *data); - uint16_t requestSliceAndData(void *data, size_t size); bool isSliceValid(uint16_t start, size_t size); private: diff --git a/plugins/Kaleidoscope-Escape-OneShot/src/kaleidoscope/plugin/Escape-OneShot-Config.cpp b/plugins/Kaleidoscope-Escape-OneShot/src/kaleidoscope/plugin/Escape-OneShot-Config.cpp index 8ce716e782..e72f943633 100644 --- a/plugins/Kaleidoscope-Escape-OneShot/src/kaleidoscope/plugin/Escape-OneShot-Config.cpp +++ b/plugins/Kaleidoscope-Escape-OneShot/src/kaleidoscope/plugin/Escape-OneShot-Config.cpp @@ -30,8 +30,8 @@ namespace kaleidoscope { namespace plugin { EventHandlerResult EscapeOneShotConfig::onSetup() { - settings_base_ = ::EEPROMSettings.requestSliceAndData(&::EscapeOneShot.settings_, sizeof(::EscapeOneShot.settings_)); - if (!::EEPROMSettings.isSliceValid(settings_base_, sizeof(::EscapeOneShot.settings_))) { + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(::EscapeOneShot.settings_), &settings_base_, &::EscapeOneShot.settings_); + if (!success) { // If our slice is uninitialized, set sensible defaults. Runtime.storage().put(settings_base_, ::EscapeOneShot.settings_); Runtime.storage().commit(); diff --git a/plugins/Kaleidoscope-IdleLEDs/src/kaleidoscope/plugin/IdleLEDs.cpp b/plugins/Kaleidoscope-IdleLEDs/src/kaleidoscope/plugin/IdleLEDs.cpp index 6eb5bdb41a..7e85319f26 100644 --- a/plugins/Kaleidoscope-IdleLEDs/src/kaleidoscope/plugin/IdleLEDs.cpp +++ b/plugins/Kaleidoscope-IdleLEDs/src/kaleidoscope/plugin/IdleLEDs.cpp @@ -77,12 +77,11 @@ EventHandlerResult PersistentIdleLEDs::onNameQuery() { EventHandlerResult PersistentIdleLEDs::onSetup() { uint16_t idle_time; - settings_base_ = ::EEPROMSettings.requestSliceAndData(&idle_time, sizeof(idle_time)); - + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(idle_time), &settings_base_, &idle_time); // If idleTime is max, assume that EEPROM is uninitialized, and store the // defaults. - if (idle_time == 0xffff) { + if (idle_time == 0xffff || !success) { idle_time = idle_time_limit / 1000; } setIdleTimeoutSeconds(idle_time); diff --git a/plugins/Kaleidoscope-LEDBrightnessConfig/src/kaleidoscope/plugin/LEDBrightnessConfig.cpp b/plugins/Kaleidoscope-LEDBrightnessConfig/src/kaleidoscope/plugin/LEDBrightnessConfig.cpp index 04b1b3b22c..f772371575 100644 --- a/plugins/Kaleidoscope-LEDBrightnessConfig/src/kaleidoscope/plugin/LEDBrightnessConfig.cpp +++ b/plugins/Kaleidoscope-LEDBrightnessConfig/src/kaleidoscope/plugin/LEDBrightnessConfig.cpp @@ -34,11 +34,9 @@ uint16_t LEDBrightnessConfig::settings_base_; struct LEDBrightnessConfig::settings LEDBrightnessConfig::settings_; EventHandlerResult LEDBrightnessConfig::onSetup() { - settings_base_ = ::EEPROMSettings.requestSliceAndData(&settings_, sizeof(settings_)); - - // We do not need to treat uninitialized slices in any special way, because - // uninitialized defaults to `255`, which happens to be our desired default - // brightness, too. + if (!::EEPROMSettings.requestSliceAndLoadData(sizeof(settings_), &settings_base_, &settings_)) { + settings_.brightness = 255; + } ::LEDControl.setBrightness(settings_.brightness); return EventHandlerResult::OK; diff --git a/plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp b/plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp index a50617fd87..67c7b4ff3a 100644 --- a/plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp +++ b/plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp @@ -33,11 +33,9 @@ namespace plugin { // MouseKeys configurator EventHandlerResult MouseKeysConfig::onSetup() { - settings_base_ = ::EEPROMSettings.requestSliceAndData(&::MouseKeys.settings_, sizeof(MouseKeys::settings_)); + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(::MouseKeys::settings_), &settings_base_, &::MouseKeys.settings_); - - // If the EEPROM is empty, store the default settings. - if (!::EEPROMSettings.isSliceValid(settings_base_, sizeof(::MouseKeys.settings_))) { + if (!success) { Runtime.storage().put(settings_base_, ::MouseKeys.settings_); Runtime.storage().commit(); } diff --git a/plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShotConfig.cpp b/plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShotConfig.cpp index 2b93a8a564..e9e87ea5c1 100644 --- a/plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShotConfig.cpp +++ b/plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShotConfig.cpp @@ -30,10 +30,9 @@ namespace kaleidoscope { namespace plugin { EventHandlerResult OneShotConfig::onSetup() { - settings_base_ = ::EEPROMSettings.requestSliceAndData(&::OneShot.settings_, sizeof(::OneShot.settings_)); + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(::OneShot::settings_), &settings_base_, &::OneShot.settings_); - // If the EEPROM is empty, store the default settings. - if (!::EEPROMSettings.isSliceValid(settings_base_, sizeof(::OneShot.settings_))) { + if (!success) { Runtime.storage().put(settings_base_, ::OneShot.settings_); Runtime.storage().commit(); } diff --git a/plugins/Kaleidoscope-PersistentLEDMode/src/kaleidoscope/plugin/PersistentLEDMode.cpp b/plugins/Kaleidoscope-PersistentLEDMode/src/kaleidoscope/plugin/PersistentLEDMode.cpp index 200b3535fb..eca57e9c55 100644 --- a/plugins/Kaleidoscope-PersistentLEDMode/src/kaleidoscope/plugin/PersistentLEDMode.cpp +++ b/plugins/Kaleidoscope-PersistentLEDMode/src/kaleidoscope/plugin/PersistentLEDMode.cpp @@ -35,10 +35,9 @@ uint16_t PersistentLEDMode::settings_base_; struct PersistentLEDMode::settings PersistentLEDMode::settings_; EventHandlerResult PersistentLEDMode::onSetup() { - settings_base_ = ::EEPROMSettings.requestSliceAndData(&settings_, sizeof(settings_)); + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(settings_), &settings_base_, &settings_); - // If the EEPROM is empty, store the default settings. - if (::EEPROMSettings.isSliceValid(settings_base_, sizeof(settings_))) { + if (success) { ::LEDControl.set_mode(settings_.default_mode_index); } diff --git a/plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp b/plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp index 95c8d7bae1..980f5fdb39 100644 --- a/plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp +++ b/plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp @@ -31,7 +31,7 @@ namespace kaleidoscope { namespace plugin { EventHandlerResult SpaceCadetConfig::onSetup() { - settings_base_ = ::EEPROMSettings.requestSliceAndData(&::SpaceCadet.settings_, sizeof(SpaceCadet::settings_)); + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(::SpaceCadet::settings_), &settings_base_, &::SpaceCadet.settings_); return EventHandlerResult::OK; } diff --git a/plugins/Kaleidoscope-TypingBreaks/src/kaleidoscope/plugin/TypingBreaks.cpp b/plugins/Kaleidoscope-TypingBreaks/src/kaleidoscope/plugin/TypingBreaks.cpp index 31b948f03f..ee3f19416c 100644 --- a/plugins/Kaleidoscope-TypingBreaks/src/kaleidoscope/plugin/TypingBreaks.cpp +++ b/plugins/Kaleidoscope-TypingBreaks/src/kaleidoscope/plugin/TypingBreaks.cpp @@ -118,10 +118,9 @@ EventHandlerResult TypingBreaks::onNameQuery() { } EventHandlerResult TypingBreaks::onSetup() { - settings_base_ = ::EEPROMSettings.requestSliceAndData(&settings, sizeof(settings)); + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(settings), &settings_base_, &settings); - // If the EEPROM is empty, store the default settings. - if (!::EEPROMSettings.isSliceValid(settings_base_, sizeof(settings))) { + if (!success) { // If our slice is uninitialized, set sensible defaults. Runtime.storage().put(settings_base_, settings); Runtime.storage().commit(); From f264c7f1cb265ad477672073a57b7e84751167af Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 6 Mar 2024 11:59:12 -0800 Subject: [PATCH 2/2] Try harder to not default Space Cadet to on unless the user has explictly turned it on --- .../src/kaleidoscope/plugin/MouseKeysConfig.cpp | 2 +- .../src/kaleidoscope/plugin/OneShotConfig.cpp | 2 +- .../src/kaleidoscope/plugin/SpaceCadetConfig.cpp | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp b/plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp index 67c7b4ff3a..c5f2b13254 100644 --- a/plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp +++ b/plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp @@ -33,7 +33,7 @@ namespace plugin { // MouseKeys configurator EventHandlerResult MouseKeysConfig::onSetup() { - bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(::MouseKeys::settings_), &settings_base_, &::MouseKeys.settings_); + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(::MouseKeys.settings_), &settings_base_, &::MouseKeys.settings_); if (!success) { Runtime.storage().put(settings_base_, ::MouseKeys.settings_); diff --git a/plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShotConfig.cpp b/plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShotConfig.cpp index e9e87ea5c1..ace1b6cc81 100644 --- a/plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShotConfig.cpp +++ b/plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShotConfig.cpp @@ -30,7 +30,7 @@ namespace kaleidoscope { namespace plugin { EventHandlerResult OneShotConfig::onSetup() { - bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(::OneShot::settings_), &settings_base_, &::OneShot.settings_); + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(::OneShot.settings_), &settings_base_, &::OneShot.settings_); if (!success) { Runtime.storage().put(settings_base_, ::OneShot.settings_); diff --git a/plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp b/plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp index 980f5fdb39..ed03b147d7 100644 --- a/plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp +++ b/plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp @@ -31,7 +31,10 @@ namespace kaleidoscope { namespace plugin { EventHandlerResult SpaceCadetConfig::onSetup() { - bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(::SpaceCadet::settings_), &settings_base_, &::SpaceCadet.settings_); + bool success = ::EEPROMSettings.requestSliceAndLoadData(sizeof(::SpaceCadet.settings_), &settings_base_, &::SpaceCadet.settings_); + if (!success || (::SpaceCadet.settings_.mode != SpaceCadet::Mode::ON && ::SpaceCadet.settings_.mode != SpaceCadet::Mode::NO_DELAY)) { + ::SpaceCadet.disable(); + } return EventHandlerResult::OK; }