Merge pull request #1399 from keyboardio/f/slice-loading-cleanup

Clean up EEPROM settings loading and unify duplicated code.

plugins/Kaleidoscope-AutoShift/src/kaleidoscope/plugin/AutoShiftConfig.cpp

@@ -33,15 +33,7 @@ namespace plugin {
 // AutoShift configurator
 
 EventHandlerResult AutoShiftConfig::onSetup() {
-  settings_base_ = ::EEPROMSettings.requestSlice(sizeof(AutoShift::Settings));
-
-  if (!Runtime.storage().isSliceUninitialized(
-        settings_base_,
-        sizeof(AutoShift::Settings))) {
-    // If our slice is initialized, load the settings
-    Runtime.storage().get(settings_base_, ::AutoShift.settings_);
-  }
-
+  settings_base_ = ::EEPROMSettings.requestSliceAndData(&::AutoShift.settings_, sizeof(AutoShift::settings_));
   return EventHandlerResult::OK;
 }
 

plugins/Kaleidoscope-DefaultLEDModeConfig/src/kaleidoscope/plugin/DefaultLEDModeConfig.cpp

@@ -34,17 +34,10 @@ uint16_t DefaultLEDModeConfig::settings_base_;
 struct DefaultLEDModeConfig::settings DefaultLEDModeConfig::settings_;
 
 EventHandlerResult DefaultLEDModeConfig::onSetup() {
-  settings_base_ = ::EEPROMSettings.requestSlice(sizeof(settings_));
-
-  Runtime.storage().get(settings_base_, settings_);
-
-  // If our slice is uninitialized, then return early, without touching the
-  // current mode.
-  if (Runtime.storage().isSliceUninitialized(settings_base_, sizeof(settings_)))
-    return EventHandlerResult::OK;
-
-  ::LEDControl.set_mode(settings_.default_mode_index);
-
+  settings_base_ = ::EEPROMSettings.requestSliceAndData(&settings_, sizeof(settings_));
+  if (::EEPROMSettings.isSliceValid(settings_base_, sizeof(settings_))) {
+    ::LEDControl.set_mode(settings_.default_mode_index);
+  }
   return EventHandlerResult::OK;
 }
 

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

@@ -20,6 +20,8 @@
 #include <Arduino.h>                   // for PSTR, F, __FlashStringHelper
 #include <Kaleidoscope-FocusSerial.h>  // for Focus, FocusSerial
 #include <stdint.h>                    // for uint16_t, uint8_t
+#include <stddef.h>                    // for size_t
+
 
 #include "kaleidoscope/Runtime.h"                     // for Runtime, Runtime_
 #include "kaleidoscope/device/device.h"               // for VirtualProps::Storage, Base<>::Storage
@@ -158,6 +160,32 @@ void EEPROMSettings::update() {
   is_valid_ = true;
 }
 
+// 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) {
+
+  // Request the slice for the struct from storage
+  uint16_t start = requestSlice(size);
+  if (isSliceValid(start, size)) {
+    Runtime.storage().get(start, data);
+  }
+  return start;
+}
+
+bool EEPROMSettings::isSliceValid(uint16_t start, size_t size) {
+
+  // If our slice is uninitialized, then return early.
+  if (Runtime.storage().isSliceUninitialized(start, size)) {
+    return false;
+  }
+
+  // If the slice of storage is initialized, but settings are invalid, then return early.
+  if (!isValid()) {
+    return false;
+  }
+  return true;
+}
+
 /** Focus **/
 EventHandlerResult FocusSettingsCommand::onFocusEvent(const char *input) {
   enum {

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

@@ -17,8 +17,8 @@
 
 #pragma once
 
-#include <stdint.h>  // for uint8_t, uint16_t
-
+#include <stdint.h>                             // for uint8_t, uint16_t
+#include <stddef.h>                             // for size_t
 #include "kaleidoscope/event_handler_result.h"  // for EventHandlerResult
 #include "kaleidoscope/plugin.h"                // for Plugin
 
@@ -74,6 +74,9 @@ class EEPROMSettings : public kaleidoscope::Plugin {
     return settings_.ignore_hardcoded_layers;
   }
 
+  uint16_t requestSliceAndData(void *data, size_t size);
+  bool isSliceValid(uint16_t start, size_t size);
+
  private:
   static constexpr uint8_t IGNORE_HARDCODED_LAYER = 0x7e;
 

plugins/Kaleidoscope-Escape-OneShot/src/kaleidoscope/plugin/Escape-OneShot-Config.cpp

@@ -30,17 +30,13 @@ namespace kaleidoscope {
 namespace plugin {
 
 EventHandlerResult EscapeOneShotConfig::onSetup() {
-  settings_base_ = ::EEPROMSettings.requestSlice(sizeof(EscapeOneShot::settings_));
-
-  if (Runtime.storage().isSliceUninitialized(
-        settings_base_,
-        sizeof(EscapeOneShot::Settings))) {
+  settings_base_ = ::EEPROMSettings.requestSliceAndData(&::EscapeOneShot.settings_, sizeof(::EscapeOneShot.settings_));
+  if (!::EEPROMSettings.isSliceValid(settings_base_, sizeof(::EscapeOneShot.settings_))) {
     // If our slice is uninitialized, set sensible defaults.
     Runtime.storage().put(settings_base_, ::EscapeOneShot.settings_);
     Runtime.storage().commit();
   }
 
-  Runtime.storage().get(settings_base_, ::EscapeOneShot.settings_);
   return EventHandlerResult::OK;
 }
 

plugins/Kaleidoscope-IdleLEDs/src/kaleidoscope/plugin/IdleLEDs.cpp

@@ -75,12 +75,13 @@ EventHandlerResult PersistentIdleLEDs::onNameQuery() {
 }
 
 EventHandlerResult PersistentIdleLEDs::onSetup() {
-  settings_base_ = ::EEPROMSettings.requestSlice(sizeof(uint16_t));
+  uint16_t idle_time;
+
+  settings_base_ = ::EEPROMSettings.requestSliceAndData(&idle_time, sizeof(idle_time));
+
 
   // If idleTime is max, assume that EEPROM is uninitialized, and store the
   // defaults.
-  uint16_t idle_time;
-  Runtime.storage().get(settings_base_, idle_time);
   if (idle_time == 0xffff) {
     idle_time = idle_time_limit / 1000;
   }

plugins/Kaleidoscope-LEDBrightnessConfig/src/kaleidoscope/plugin/LEDBrightnessConfig.cpp

@@ -34,9 +34,7 @@ uint16_t LEDBrightnessConfig::settings_base_;
 struct LEDBrightnessConfig::settings LEDBrightnessConfig::settings_;
 
 EventHandlerResult LEDBrightnessConfig::onSetup() {
-  settings_base_ = ::EEPROMSettings.requestSlice(sizeof(settings_));
-
-  Runtime.storage().get(settings_base_, settings_);
+  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

plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeys.h

@@ -202,7 +202,7 @@ class MouseKeysConfig : public Plugin {
 
  private:
   // The base address in persistent storage for configuration data:
-  uint16_t settings_addr_;
+  uint16_t settings_base_;
 };
 
 }  // namespace plugin

plugins/Kaleidoscope-MouseKeys/src/kaleidoscope/plugin/MouseKeysConfig.cpp

@@ -33,15 +33,15 @@ namespace plugin {
 // MouseKeys configurator
 
 EventHandlerResult MouseKeysConfig::onSetup() {
-  settings_addr_ = ::EEPROMSettings.requestSlice(sizeof(MouseKeys::Settings));
+  settings_base_ = ::EEPROMSettings.requestSliceAndData(&::MouseKeys.settings_, sizeof(MouseKeys::settings_));
+
 
   // If the EEPROM is empty, store the default settings.
-  if (Runtime.storage().isSliceUninitialized(settings_addr_, sizeof(MouseKeys::Settings))) {
-    Runtime.storage().put(settings_addr_, ::MouseKeys.settings_);
+  if (!::EEPROMSettings.isSliceValid(settings_base_, sizeof(::MouseKeys.settings_))) {
+    Runtime.storage().put(settings_base_, ::MouseKeys.settings_);
     Runtime.storage().commit();
   }
 
-  Runtime.storage().get(settings_addr_, ::MouseKeys.settings_);
 
   // We need to set the grid size explicitly, because behind the scenes, that's
   // stored in MouseWrapper.
@@ -141,7 +141,7 @@ EventHandlerResult MouseKeysConfig::onFocusEvent(const char *input) {
 
   // Update settings stored in EEPROM, and indicate that this Focus event has
   // been handled successfully.
-  Runtime.storage().put(settings_addr_, ::MouseKeys.settings_);
+  Runtime.storage().put(settings_base_, ::MouseKeys.settings_);
   Runtime.storage().commit();
   return EventHandlerResult::EVENT_CONSUMED;
 }

plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShot.h

@@ -263,7 +263,7 @@ class OneShotConfig : public Plugin {
 
  private:
   // The base address in persistent storage for configuration data:
-  uint16_t settings_addr_;
+  uint16_t settings_base_;
 };
 
 }  // namespace plugin

plugins/Kaleidoscope-OneShot/src/kaleidoscope/plugin/OneShotConfig.cpp

@@ -30,15 +30,14 @@ namespace kaleidoscope {
 namespace plugin {
 
 EventHandlerResult OneShotConfig::onSetup() {
-  settings_addr_ = ::EEPROMSettings.requestSlice(sizeof(::OneShot.settings_));
+  settings_base_ = ::EEPROMSettings.requestSliceAndData(&::OneShot.settings_, sizeof(::OneShot.settings_));
 
   // If the EEPROM is empty, store the default settings.
-  if (Runtime.storage().isSliceUninitialized(settings_addr_, sizeof(::OneShot.settings_))) {
-    Runtime.storage().put(settings_addr_, ::OneShot.settings_);
+  if (!::EEPROMSettings.isSliceValid(settings_base_, sizeof(::OneShot.settings_))) {
+    Runtime.storage().put(settings_base_, ::OneShot.settings_);
     Runtime.storage().commit();
   }
 
-  Runtime.storage().get(settings_addr_, ::OneShot.settings_);
   return EventHandlerResult::OK;
 }
 
@@ -144,7 +143,7 @@ EventHandlerResult OneShotConfig::onFocusEvent(const char *input) {
     default:
       return EventHandlerResult::ABORT;
     }
-    Runtime.storage().put(settings_addr_, ::OneShot.settings_);
+    Runtime.storage().put(settings_base_, ::OneShot.settings_);
     Runtime.storage().commit();
   }
 

plugins/Kaleidoscope-PersistentLEDMode/src/kaleidoscope/plugin/PersistentLEDMode.cpp

@@ -35,16 +35,12 @@ uint16_t PersistentLEDMode::settings_base_;
 struct PersistentLEDMode::settings PersistentLEDMode::settings_;
 
 EventHandlerResult PersistentLEDMode::onSetup() {
-  settings_base_ = ::EEPROMSettings.requestSlice(sizeof(settings_));
+  settings_base_ = ::EEPROMSettings.requestSliceAndData(&settings_, sizeof(settings_));
 
-  Runtime.storage().get(settings_base_, settings_);
-
-  // If our slice is uninitialized, then return early, without touching the
-  // current mode.
-  if (Runtime.storage().isSliceUninitialized(settings_base_, sizeof(settings_)))
-    return EventHandlerResult::OK;
-
-  ::LEDControl.set_mode(settings_.default_mode_index);
+  // If the EEPROM is empty, store the default settings.
+  if (::EEPROMSettings.isSliceValid(settings_base_, sizeof(settings_))) {
+    ::LEDControl.set_mode(settings_.default_mode_index);
+  }
 
   return EventHandlerResult::OK;
 }

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

@@ -31,13 +31,7 @@ namespace kaleidoscope {
 namespace plugin {
 
 EventHandlerResult SpaceCadetConfig::onSetup() {
-  settings_base_ = ::EEPROMSettings.requestSlice(sizeof(SpaceCadet::settings_));
-
-  // If our slice is uninitialized, then return early.
-  if (Runtime.storage().isSliceUninitialized(settings_base_, sizeof(SpaceCadet::settings_)))
-    return EventHandlerResult::OK;
-
-  Runtime.storage().get(settings_base_, ::SpaceCadet.settings_);
+  settings_base_ = ::EEPROMSettings.requestSliceAndData(&::SpaceCadet.settings_, sizeof(SpaceCadet::settings_));
 
   return EventHandlerResult::OK;
 }

plugins/Kaleidoscope-TypingBreaks/src/kaleidoscope/plugin/TypingBreaks.cpp

@@ -118,20 +118,19 @@ EventHandlerResult TypingBreaks::onNameQuery() {
 }
 
 EventHandlerResult TypingBreaks::onSetup() {
-  settings_base_ = ::EEPROMSettings.requestSlice(sizeof(settings));
+  settings_base_ = ::EEPROMSettings.requestSliceAndData(&settings, sizeof(settings));
 
-  if (Runtime.storage().isSliceUninitialized(
-        settings_base_,
-        sizeof(settings))) {
+  // If the EEPROM is empty, store the default settings.
+  if (!::EEPROMSettings.isSliceValid(settings_base_, sizeof(settings))) {
     // If our slice is uninitialized, set sensible defaults.
     Runtime.storage().put(settings_base_, settings);
     Runtime.storage().commit();
   }
 
-  Runtime.storage().get(settings_base_, settings);
   return EventHandlerResult::OK;
 }
 
+
 EventHandlerResult TypingBreaks::onFocusEvent(const char *input) {
   enum {
     IDLE_TIME_LIMIT,