Introduce StateManager to persist state between power cycles (#6)

- add reset state menu option to return all settings back to default values.
- add reverse encoder menu option and save state
- make saving to EEPROM safer by wrapping put calls with noInterrupts()
- improve save state behavior by using a mutex flag and update check with debounce in main loop
- refactor gravity.h global const definitions to be static and more readable.
- improve usage of EncoderDir in ISR with pointer to instance and static isr() method.
- reduce u8g2 memory usage by using single page buffer

Reviewed-on: https://git.pinkduck.xyz/adam/libGravity/pulls/6
Co-authored-by: Adam Wonak <adam.wonak@gmail.com>
Co-committed-by: Adam Wonak <adam.wonak@gmail.com>

clock.h

@@ -23,22 +23,22 @@
 #define MIDI_STOP 0xFC
 #define MIDI_CONTINUE 0xFB
 
-const int DEFAULT_TEMPO = 120;
-
 typedef void (*ExtCallback)(void);
 static ExtCallback extUserCallback = nullptr;
 static void serialEventNoop(uint8_t msg, uint8_t status) {}
 
-enum Source {
+class Clock {
+   public:
+    static constexpr int DEFAULT_TEMPO = 120;
+
+    enum Source {
         SOURCE_INTERNAL,
         SOURCE_EXTERNAL_PPQN_24,
         SOURCE_EXTERNAL_PPQN_4,
         SOURCE_EXTERNAL_MIDI,
         SOURCE_LAST,
-};
+    };
 
-class Clock {
-   public:
     void Init() {
         NeoSerial.begin(31250);
 

encoder_dir.h

@@ -34,7 +34,9 @@ class EncoderDir {
 
    public:
     EncoderDir() : encoder_(ENCODER_PIN1, ENCODER_PIN2, RotaryEncoder::LatchMode::FOUR3),
-                   button_(ENCODER_SW_PIN) {}
+                   button_(ENCODER_SW_PIN) {
+        _instance = this;
+    }
     ~EncoderDir() {}
 
     // Set to true if the encoder read direction should be reversed.
@@ -81,15 +83,19 @@ class EncoderDir {
         }
     }
 
-    // Read the encoder state and update the read position.
+    static void isr() {
-    void UpdateEncoder() {
+        // If the instance has been created, call its tick() method.
-        encoder_.tick();
+        if (_instance) {
+            _instance->encoder_.tick();
+        }
     }
 
    private:
+    static EncoderDir* _instance;
+
     int previous_pos_;
     bool rotated_while_held_;
-    bool reversed_ = true;
+    bool reversed_ = false;
     RotaryEncoder encoder_;
     Button button_;
 
@@ -115,15 +121,18 @@ class EncoderDir {
             change *= 2;
         }
 
+        if (reversed_) {
+            change = -(change);
+        }
         return change;
     }
 
     inline Direction rotate_(int dir, bool reversed) {
         switch (dir) {
             case 1:
-                return (reversed) ? DIRECTION_INCREMENT : DIRECTION_DECREMENT;
-            case -1:
                 return (reversed) ? DIRECTION_DECREMENT : DIRECTION_INCREMENT;
+            case -1:
+                return (reversed) ? DIRECTION_INCREMENT : DIRECTION_DECREMENT;
             default:
                 return DIRECTION_UNCHANGED;
         }

examples/Gravity/Gravity.ino

@@ -1,5 +1,5 @@
 /**
- * @file clock_mod.ino
+ * @file Gravity.ino
  * @author Adam Wonak (https://github.com/awonak/)
  * @brief Demo firmware for Sitka Instruments Gravity.
  * @version 0.1
@@ -19,22 +19,19 @@
 
 #include <gravity.h>
 
+#include "app_state.h"
 #include "channel.h"
+#include "save_state.h"
 
-// Firmware state variables.
-struct AppState {
-    bool refresh_screen = true;
-    bool editing_param = false;
-    int selected_param = 0;
-    byte selected_channel = 0;  // 0=tempo, 1-6=output channel
-    Source selected_source = SOURCE_INTERNAL;
-    Channel channel[OUTPUT_COUNT];
-};
 AppState app;
 
+StateManager stateManager;
+
 enum ParamsMainPage {
     PARAM_MAIN_TEMPO,
     PARAM_MAIN_SOURCE,
+    PARAM_MAIN_ENCODER_DIR,
+    PARAM_MAIN_RESET_STATE,
     PARAM_MAIN_LAST,
 };
 
@@ -114,6 +111,10 @@ void setup() {
     // Start Gravity.
     gravity.Init();
 
+    // Initialize the state manager. This will load settings from EEPROM
+    stateManager.initialize(app);
+    InitAppState(app);
+
     // Clock handlers.
     gravity.clock.AttachIntHandler(HandleIntClockTick);
     gravity.clock.AttachExtHandler(HandleExtClockTick);
@@ -135,10 +136,13 @@ void loop() {
     // Read CVs and call the update function for each channel.
     int cv1 = gravity.cv1.Read();
     int cv2 = gravity.cv2.Read();
-    for (int i = 0; i < OUTPUT_COUNT; i++) {
+    for (int i = 0; i < Gravity::OUTPUT_COUNT; i++) {
         app.channel[i].applyCvMod(cv1, cv2);
     }
 
+    // Check for dirty state eligible to be saved.
+    stateManager.update(app);
+
     if (app.refresh_screen) {
         UpdateDisplay();
     }
@@ -150,7 +154,7 @@ void loop() {
 
 void HandleIntClockTick(uint32_t tick) {
     bool refresh = false;
-    for (int i = 0; i < OUTPUT_COUNT; i++) {
+    for (int i = 0; i < Gravity::OUTPUT_COUNT; i++) {
         app.channel[i].processClockTick(tick, gravity.outputs[i]);
 
         if (app.channel[i].isCvModActive()) {
@@ -191,6 +195,25 @@ void HandleShiftPressed() {
 }
 
 void HandleEncoderPressed() {
+    // Check if leaving editing mode should apply a selection.
+    if (app.editing_param) {
+        if (app.selected_channel == 0) {  // main page
+            if (app.selected_param == PARAM_MAIN_ENCODER_DIR) {
+                bool reversed = app.selected_sub_param == 1;
+                gravity.encoder.SetReverseDirection(reversed);
+            }
+            // Reset state
+            if (app.selected_param == PARAM_MAIN_RESET_STATE) {
+                if (app.selected_sub_param == 0) {  // Reset
+                    stateManager.reset(app);
+                    InitAppState(app);
+                }
+            }
+        }
+        // Only mark dirty when leaving editing mode.
+        stateManager.markDirty();
+    }
+    app.selected_sub_param = 0;
     app.editing_param = !app.editing_param;
     app.refresh_screen = true;
 }
@@ -212,12 +235,13 @@ void HandleRotate(Direction dir, int val) {
 }
 
 void HandlePressedRotate(Direction dir, int val) {
-    if (dir == DIRECTION_INCREMENT && app.selected_channel < OUTPUT_COUNT) {
+    if (dir == DIRECTION_INCREMENT && app.selected_channel < Gravity::OUTPUT_COUNT) {
         app.selected_channel++;
     } else if (dir == DIRECTION_DECREMENT && app.selected_channel > 0) {
         app.selected_channel--;
     }
     app.selected_param = 0;
+    stateManager.markDirty();
     app.refresh_screen = true;
 }
 
@@ -228,15 +252,22 @@ void editMainParameter(int val) {
                 break;
             }
             gravity.clock.SetTempo(gravity.clock.Tempo() + val);
+            app.tempo = gravity.clock.Tempo();
             break;
 
         case PARAM_MAIN_SOURCE: {
             int source = static_cast<int>(app.selected_source);
-            updateSelection(source, val, SOURCE_LAST);
+            updateSelection(source, val, Clock::SOURCE_LAST);
-            app.selected_source = static_cast<Source>(source);
+            app.selected_source = static_cast<Clock::Source>(source);
             gravity.clock.SetSource(app.selected_source);
             break;
         }
+        case PARAM_MAIN_ENCODER_DIR:
+            updateSelection(app.selected_sub_param, val, 2);
+            break;
+        case PARAM_MAIN_RESET_STATE:
+            updateSelection(app.selected_sub_param, val, 2);
+            break;
     }
 }
 
@@ -279,12 +310,18 @@ void updateSelection(int& param, int change, int maxValue) {
 // Helper functions.
 //
 
+void InitAppState(AppState& app) {
+    gravity.clock.SetTempo(app.tempo);
+    gravity.clock.SetSource(app.selected_source);
+    gravity.encoder.SetReverseDirection(app.encoder_reversed);
+}
+
 Channel& GetSelectedChannel() {
     return app.channel[app.selected_channel - 1];
 }
 
 void ResetOutputs() {
-    for (int i = 0; i < OUTPUT_COUNT; i++) {
+    for (int i = 0; i < Gravity::OUTPUT_COUNT; i++) {
         gravity.outputs[i].Low();
     }
 }
@@ -331,7 +368,7 @@ void DisplayMainPage() {
     switch (app.selected_param) {
         case PARAM_MAIN_TEMPO:
             // Serial MIDI is too unstable to display bpm in real time.
-            if (app.selected_source == SOURCE_EXTERNAL_MIDI) {
+            if (app.selected_source == Clock::SOURCE_EXTERNAL_MIDI) {
                 sprintf(mainText, "%s", "EXT");
             } else {
                 sprintf(mainText, "%d", gravity.clock.Tempo());
@@ -340,30 +377,39 @@ void DisplayMainPage() {
             break;
         case PARAM_MAIN_SOURCE:
             switch (app.selected_source) {
-                case SOURCE_INTERNAL:
+                case Clock::SOURCE_INTERNAL:
                     sprintf(mainText, "%s", "INT");
                     subText = "CLOCK";
                     break;
-                case SOURCE_EXTERNAL_PPQN_24:
+                case Clock::SOURCE_EXTERNAL_PPQN_24:
                     sprintf(mainText, "%s", "EXT");
                     subText = "24 PPQN";
                     break;
-                case SOURCE_EXTERNAL_PPQN_4:
+                case Clock::SOURCE_EXTERNAL_PPQN_4:
                     sprintf(mainText, "%s", "EXT");
                     subText = "4 PPQN";
                     break;
-                case SOURCE_EXTERNAL_MIDI:
+                case Clock::SOURCE_EXTERNAL_MIDI:
                     sprintf(mainText, "%s", "EXT");
                     subText = "MIDI";
                     break;
             }
+            break;
+        case PARAM_MAIN_ENCODER_DIR:
+            sprintf(mainText, "%s", "DIR");
+            subText = app.selected_sub_param == 0 ? "DEFAULT" : "REVERSED";
+            break;
+        case PARAM_MAIN_RESET_STATE:
+            sprintf(mainText, "%s", "RST");
+            subText = app.selected_sub_param == 0 ? "RESET ALL" : "BACK";
+            break;
     }
 
     drawCenteredText(mainText, MAIN_TEXT_Y, LARGE_FONT);
     drawCenteredText(subText, SUB_TEXT_Y, TEXT_FONT);
 
     // Draw Main Page menu items
-    const char* menu_items[PARAM_MAIN_LAST] = {"TEMPO", "SOURCE"};
+    const char* menu_items[PARAM_MAIN_LAST] = {"TEMPO", "SOURCE", "ENCODER DIR", "RESET"};
     drawMenuItems(menu_items, PARAM_MAIN_LAST);
 }
 
@@ -469,7 +515,7 @@ void DisplaySelectedChannel() {
     gravity.display.drawHLine(1, boxY, SCREEN_WIDTH - 2);
     gravity.display.drawVLine(SCREEN_WIDTH - 2, boxY, boxHeight);
 
-    for (int i = 0; i < OUTPUT_COUNT + 1; i++) {
+    for (int i = 0; i < Gravity::OUTPUT_COUNT + 1; i++) {
         // Draw box frame or filled selected box.
         gravity.display.setDrawColor(1);
         (app.selected_channel == i)

examples/Gravity/app_state.h

@@ -0,0 +1,21 @@
+#ifndef APP_STATE_H
+#define APP_STATE_H
+
+#include <gravity.h>
+
+#include "channel.h"
+
+// Global state for settings and app behavior.
+struct AppState {
+    int tempo = Clock::DEFAULT_TEMPO;
+    bool encoder_reversed = false;
+    bool refresh_screen = true;
+    bool editing_param = false;
+    int selected_param = 0;
+    int selected_sub_param = 0;
+    byte selected_channel = 0;  // 0=tempo, 1-6=output channel
+    Clock::Source selected_source = Clock::SOURCE_INTERNAL;
+    Channel channel[Gravity::OUTPUT_COUNT];
+};
+
+#endif  // APP_STATE_H

examples/Gravity/channel.h

@@ -30,10 +30,24 @@ static const int clock_mod_pulses[MOD_CHOICE_SIZE] = {4, 8, 12, 16, 24, 32, 48,
 class Channel {
    public:
     Channel() {
+        Init();
+    }
+
+    void Init() {
+        // Reset base values to their defaults
+        base_clock_mod_index = 7;
+        base_probability = 100;
+        base_duty_cycle = 50;
+        base_offset = 0;
+        cv_source = CV_NONE;
+        cv_destination = CV_DEST_NONE;
+
         cvmod_clock_mod_index = base_clock_mod_index;
         cvmod_probability = base_probability;
         cvmod_duty_cycle = base_duty_cycle;
         cvmod_offset = base_offset;
+        duty_cycle_pulses = 0;
+        offset_pulses = 0;
     }
 
     // Setters (Set the BASE value)
@@ -121,11 +135,11 @@ class Channel {
     }
 
    private:
-    // User-settable "base" values.
+    // User-settable base values.
-    byte base_clock_mod_index = 7;
+    byte base_clock_mod_index;
-    byte base_probability = 100;
+    byte base_probability;
-    byte base_duty_cycle = 50;
+    byte base_duty_cycle;
-    byte base_offset = 0;
+    byte base_offset;
 
     // Base value with cv mod applied.
     byte cvmod_clock_mod_index;
@@ -133,8 +147,8 @@ class Channel {
     byte cvmod_duty_cycle;
     byte cvmod_offset;
 
-    int duty_cycle_pulses;
+    uint32_t duty_cycle_pulses;
-    int offset_pulses;
+    uint32_t offset_pulses;
 
     // CV configuration
     CvSource cv_source = CV_NONE;

examples/Gravity/save_state.cpp

@@ -0,0 +1,119 @@
+#include "save_state.h"
+
+#include <EEPROM.h>
+
+#include "app_state.h"
+
+StateManager::StateManager() : _isDirty(false), _lastChangeTime(0) {}
+
+bool StateManager::initialize(AppState& app) {
+    if (isDataValid()) {
+        static EepromData load_data;
+        EEPROM.get(sizeof(Metadata), load_data);
+
+        // Restore main app state
+        app.tempo = load_data.tempo;
+        app.encoder_reversed = load_data.encoder_reversed;
+        app.selected_param = load_data.selected_param;
+        app.selected_channel = load_data.selected_channel;
+        app.selected_source = static_cast<Clock::Source>(load_data.selected_source);
+
+        // Loop through and restore each channel's state.
+        for (int i = 0; i < Gravity::OUTPUT_COUNT; i++) {
+            auto& ch = app.channel[i];
+            const auto& saved_ch_state = load_data.channel_data[i];
+
+            ch.setClockMod(saved_ch_state.base_clock_mod_index);
+            ch.setProbability(saved_ch_state.base_probability);
+            ch.setDutyCycle(saved_ch_state.base_duty_cycle);
+            ch.setOffset(saved_ch_state.base_offset);
+            ch.setCvSource(static_cast<CvSource>(saved_ch_state.cv_source));
+            ch.setCvDestination(static_cast<CvDestination>(saved_ch_state.cv_destination));
+        }
+
+        return true;
+    } else {
+        reset(app);
+        return false;
+    }
+}
+
+void StateManager::save(const AppState& app) {
+    // Ensure interrupts do not cause corrupt data writes.
+    noInterrupts();
+    _save_worker(app);
+    interrupts();
+}
+
+void StateManager::reset(AppState& app) {
+    app.tempo = Clock::DEFAULT_TEMPO;
+    app.encoder_reversed = false;
+    app.selected_param = 0;
+    app.selected_channel = 0;
+    app.selected_source = Clock::SOURCE_INTERNAL;
+
+    for (int i = 0; i < Gravity::OUTPUT_COUNT; i++) {
+        app.channel[i].Init();
+    }
+
+    noInterrupts();
+    _metadata_worker();  // Write the new metadata
+    _save_worker(app);   // Write the new (default) app state
+    interrupts();
+
+    _isDirty = false;
+}
+
+void StateManager::update(const AppState& app) {
+    // Check if a save is pending and if enough time has passed.
+    if (_isDirty && (millis() - _lastChangeTime > SAVE_DELAY_MS)) {
+        save(app);
+        _isDirty = false;  // Clear the flag, we are now "clean".
+    }
+}
+
+void StateManager::markDirty() {
+    _isDirty = true;
+    _lastChangeTime = millis();
+}
+
+bool StateManager::isDataValid() {
+    Metadata load_meta;
+    EEPROM.get(0, load_meta);
+    bool nameMatch = (strcmp(load_meta.sketchName, CURRENT_SKETCH_NAME) == 0);
+    bool versionMatch = (load_meta.version == CURRENT_SKETCH_VERSION);
+    return nameMatch && versionMatch;
+}
+
+void StateManager::_save_worker(const AppState& app) {
+    static EepromData save_data;
+
+    // Populate main app state
+    save_data.tempo = app.tempo;
+    save_data.encoder_reversed = app.encoder_reversed;
+    save_data.selected_param = app.selected_param;
+    save_data.selected_channel = app.selected_channel;
+    save_data.selected_source = static_cast<byte>(app.selected_source);
+
+    // Loop through and populate each channel's state
+    for (int i = 0; i < Gravity::OUTPUT_COUNT; i++) {
+        const auto& ch = app.channel[i];
+        auto& save_ch = save_data.channel_data[i];
+
+        // Use the getters with 'withCvMod = false' to get the base values
+        save_ch.base_clock_mod_index = ch.getClockModIndex(false);
+        save_ch.base_probability = ch.getProbability(false);
+        save_ch.base_duty_cycle = ch.getDutyCycle(false);
+        save_ch.base_offset = ch.getOffset(false);
+        save_ch.cv_source = static_cast<byte>(ch.getCvSource());
+        save_ch.cv_destination = static_cast<byte>(ch.getCvDestination());
+    }
+    EEPROM.put(sizeof(Metadata), save_data);
+}
+
+void StateManager::_metadata_worker() {
+    Metadata currentMeta;
+    strcpy(currentMeta.sketchName, CURRENT_SKETCH_NAME);
+    currentMeta.version = CURRENT_SKETCH_VERSION;
+    EEPROM.put(0, currentMeta);
+}

examples/Gravity/save_state.h

@@ -0,0 +1,65 @@
+#ifndef SAVE_STATE_H
+#define SAVE_STATE_H
+
+#include <Arduino.h>
+#include <gravity.h>
+
+// Forward-declare AppState to avoid circular dependencies.
+struct AppState;
+
+// Define the constants for the current firmware.
+const char CURRENT_SKETCH_NAME[] = "Gravity";
+const float CURRENT_SKETCH_VERSION = 0.2f;
+
+/**
+ * @brief Manages saving and loading of the application state to and from EEPROM.
+ */
+class StateManager {
+   public:
+    StateManager();
+
+    // Populate the AppState instance with values from EEPROM if they exist.
+    bool initialize(AppState& app);
+    // Reset AppState instance back to default values.
+    void reset(AppState& app);
+    // Call from main loop, check if state has changed and needs to be saved.
+    void update(const AppState& app);
+    // Indicate that state has changed and we should save.
+    void markDirty();
+
+   private:
+    // This struct holds the data that identifies the firmware version.
+    struct Metadata {
+        char sketchName[16];
+        byte version;
+    };
+    struct ChannelState {
+        byte base_clock_mod_index;
+        byte base_probability;
+        byte base_duty_cycle;
+        byte base_offset;
+        byte cv_source;       // Cast the CvSource enum to a byte for storage
+        byte cv_destination;  // Cast the CvDestination enum as a byte for storage
+    };
+    // This struct holds all the parameters we want to save.
+    struct EepromData {
+        int tempo;
+        bool encoder_reversed;
+        byte selected_param;
+        byte selected_channel;
+        byte selected_source;
+        ChannelState channel_data[Gravity::OUTPUT_COUNT];
+    };
+
+    void save(const AppState& app);
+
+    bool isDataValid();
+    void _save_worker(const AppState& app);
+    void _metadata_worker();
+
+    bool _isDirty;
+    unsigned long _lastChangeTime;
+    static const unsigned long SAVE_DELAY_MS = 2000;
+};
+
+#endif  // SAVE_STATE_H

gravity.cpp

@@ -11,6 +11,10 @@
 
 #include "gravity.h"
 
+// Initialize the static pointer for the EncoderDir class to null. We want to
+// have a static pointer to decouple the ISR from the global gravity object. 
+EncoderDir* EncoderDir::_instance = nullptr;
+
 void Gravity::Init() {
     initClock();
     initInputs();
@@ -68,18 +72,13 @@ void Gravity::Process() {
     }
 }
 
-void ReadEncoder() {
-    gravity.encoder.UpdateEncoder();
-}
-
-// Define Encoder pin ISR.
-// Pin Change Interrupt on Port C (D17/A3).
-ISR(PCINT2_vect) {
-    ReadEncoder();
-};
 // Pin Change Interrupt on Port D (D4).
+ISR(PCINT2_vect) {
+    EncoderDir::isr();
+};
+// Pin Change Interrupt on Port C (D17/A3).
 ISR(PCINT1_vect) {
-    ReadEncoder();
+    EncoderDir::isr();
 };
 
 // Singleton

gravity.h

@@ -14,6 +14,8 @@
 // Hardware abstraction wrapper for the Gravity module.
 class Gravity {
    public:
+    static const uint8_t OUTPUT_COUNT = 6;
+
     // Constructor
     Gravity()
         : display(U8G2_R2, SCL, SDA, U8X8_PIN_NONE) {}
@@ -27,7 +29,7 @@ class Gravity {
     // Polling check for state change of inputs and outputs.
     void Process();
 
-    U8G2_SSD1306_128X64_NONAME_2_HW_I2C display;  // OLED display object.
+    U8G2_SSD1306_128X64_NONAME_1_HW_I2C display;  // OLED display object.
     Clock clock;                                  // Clock source wrapper.
     DigitalOutput outputs[OUTPUT_COUNT];          // An array containing each Output object.
     EncoderDir encoder;                           // Rotary encoder with button instance

peripherials.h

@@ -39,6 +39,4 @@
 #define OUT_CH5 9
 #define OUT_CH6 11
 
-const uint8_t OUTPUT_COUNT = 6;
-
 #endif