Merge pull request #1403 from keyboardio/bugfix/eeprom-data-loading

My earlier optimizations to how we load data from EEPROM turned out to have unintended consequences

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;
 }
 

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;

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) {

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:

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();

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);

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;

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();
   }

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();
   }

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);
   }
 

plugins/Kaleidoscope-SpaceCadet/src/kaleidoscope/plugin/SpaceCadetConfig.cpp

@@ -31,7 +31,10 @@ 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_);
+  if (!success || (::SpaceCadet.settings_.mode != SpaceCadet::Mode::ON && ::SpaceCadet.settings_.mode != SpaceCadet::Mode::NO_DELAY)) {
+    ::SpaceCadet.disable();
+  }
 
   return EventHandlerResult::OK;
 }

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();