From 8644a3e75270bb807c4b4209eb2146ad14940165 Mon Sep 17 00:00:00 2001 From: Adam Wonak Date: Sun, 15 Jun 2025 19:20:16 -0700 Subject: [PATCH] Lots of changes and optimizations - add reverse encoder menu option and save state - improve usage of EncoderDir in ISR with pointer to instance and static isr() method. - reduce u8g2 memory usage by using single page buffer - improve save state behavor by using a mutex flag and update check with debounce in main loop - make saving to EEPROM safer by wrapping put calls with noInterrupts() --- encoder_dir.h | 23 +++++--- examples/Gravity/Gravity.ino | 25 +++++++-- examples/Gravity/app_state.h | 1 + examples/Gravity/save_state.cpp | 96 +++++++++++++++++++++------------ examples/Gravity/save_state.h | 20 +++++-- gravity.cpp | 18 +++---- gravity.h | 2 +- 7 files changed, 124 insertions(+), 61 deletions(-) diff --git a/encoder_dir.h b/encoder_dir.h index dbc3c56..5375b57 100644 --- a/encoder_dir.h +++ b/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. - void UpdateEncoder() { - encoder_.tick(); + static void isr() { + // If the instance has been created, call its tick() method. + 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; } diff --git a/examples/Gravity/Gravity.ino b/examples/Gravity/Gravity.ino index 6789ae0..f3502ea 100644 --- a/examples/Gravity/Gravity.ino +++ b/examples/Gravity/Gravity.ino @@ -30,6 +30,7 @@ StateManager stateManager; enum ParamsMainPage { PARAM_MAIN_TEMPO, PARAM_MAIN_SOURCE, + PARAM_MAIN_ENCODER_DIR, PARAM_MAIN_RESET_STATE, PARAM_MAIN_LAST, }; @@ -139,6 +140,9 @@ void loop() { app.channel[i].applyCvMod(cv1, cv2); } + // Check for dirty state eligible to be saved. + stateManager.update(app); + if (app.refresh_screen) { UpdateDisplay(); } @@ -194,19 +198,23 @@ 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); - // stateManager.initialize(app); InitAppState(app); } } } + // Only mark dirty when leaving editing mode. + stateManager.markDirty(); } app.selected_sub_param = 0; app.editing_param = !app.editing_param; - stateManager.save(app); app.refresh_screen = true; } @@ -223,7 +231,6 @@ void HandleRotate(Direction dir, int val) { editChannelParameter(val); } } - stateManager.save(app); app.refresh_screen = true; } @@ -234,7 +241,7 @@ void HandlePressedRotate(Direction dir, int val) { app.selected_channel--; } app.selected_param = 0; - stateManager.save(app); + stateManager.markDirty(); app.refresh_screen = true; } @@ -255,6 +262,9 @@ void editMainParameter(int val) { 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; @@ -303,6 +313,7 @@ void updateSelection(int& param, int change, int maxValue) { void InitAppState(AppState& app) { gravity.clock.SetTempo(app.tempo); gravity.clock.SetSource(app.selected_source); + gravity.encoder.SetReverseDirection(app.encoder_reversed); } Channel& GetSelectedChannel() { @@ -384,6 +395,10 @@ void DisplayMainPage() { 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"; @@ -394,7 +409,7 @@ void DisplayMainPage() { drawCenteredText(subText, SUB_TEXT_Y, TEXT_FONT); // Draw Main Page menu items - const char* menu_items[PARAM_MAIN_LAST] = {"TEMPO", "SOURCE", "RESET"}; + const char* menu_items[PARAM_MAIN_LAST] = {"TEMPO", "SOURCE", "ENCODER DIR", "RESET"}; drawMenuItems(menu_items, PARAM_MAIN_LAST); } diff --git a/examples/Gravity/app_state.h b/examples/Gravity/app_state.h index df7c634..a70661a 100644 --- a/examples/Gravity/app_state.h +++ b/examples/Gravity/app_state.h @@ -8,6 +8,7 @@ // 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; diff --git a/examples/Gravity/save_state.cpp b/examples/Gravity/save_state.cpp index c67cdf5..c214c63 100644 --- a/examples/Gravity/save_state.cpp +++ b/examples/Gravity/save_state.cpp @@ -4,13 +4,16 @@ #include "app_state.h" +StateManager::StateManager() : _isDirty(false), _lastChangeTime(0) {} + bool StateManager::initialize(AppState& app) { if (isDataValid()) { - EepromData load_data; + 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(load_data.selected_source); @@ -30,41 +33,21 @@ bool StateManager::initialize(AppState& app) { return true; } else { - writeMetadata(); - save(app); // Save the initial default state + reset(app); return false; } } void StateManager::save(const AppState& app) { - EepromData save_data; - - // Populate main app state - save_data.tempo = app.tempo; - save_data.selected_param = app.selected_param; - save_data.selected_channel = app.selected_channel; - save_data.selected_source = static_cast(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& saved_ch_state = save_data.channel_data[i]; - - // Use the getters with 'withCvMod = false' to get the base values - saved_ch_state.base_clock_mod_index = ch.getClockModIndex(false); - saved_ch_state.base_probability = ch.getProbability(false); - saved_ch_state.base_duty_cycle = ch.getDutyCycle(false); - saved_ch_state.base_offset = ch.getOffset(false); - saved_ch_state.cv_source = static_cast(ch.getCvSource()); - saved_ch_state.cv_destination = static_cast(ch.getCvDestination()); - } - - // Write the entire state struct to EEPROM - EEPROM.put(sizeof(Metadata), save_data); + // 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; @@ -73,8 +56,25 @@ void StateManager::reset(AppState& app) { app.channel[i].Init(); } - writeMetadata(); - save(app); + 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() { @@ -85,9 +85,35 @@ bool StateManager::isDataValid() { return nameMatch && versionMatch; } -void StateManager::writeMetadata() { - Metadata save_meta; - strcpy(save_meta.sketchName, CURRENT_SKETCH_NAME); - save_meta.version = CURRENT_SKETCH_VERSION; - EEPROM.put(0, save_meta); -} \ No newline at end of file +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(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(ch.getCvSource()); + save_ch.cv_destination = static_cast(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); +} diff --git a/examples/Gravity/save_state.h b/examples/Gravity/save_state.h index 191eb43..d6345ac 100644 --- a/examples/Gravity/save_state.h +++ b/examples/Gravity/save_state.h @@ -16,15 +16,20 @@ const float CURRENT_SKETCH_VERSION = 0.2f; */ class StateManager { public: + StateManager(); + bool initialize(AppState& app); - void save(const AppState& app); 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]; - float version; + byte version; }; struct ChannelState { byte base_clock_mod_index; @@ -37,14 +42,23 @@ class StateManager { // 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 writeMetadata(); + 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 \ No newline at end of file diff --git a/gravity.cpp b/gravity.cpp index 00efe16..4236b37 100644 --- a/gravity.cpp +++ b/gravity.cpp @@ -11,6 +11,9 @@ #include "gravity.h" +// Initialize the static pointer for the EncoderDir class to null. +EncoderDir* EncoderDir::_instance = nullptr; + void Gravity::Init() { initClock(); initInputs(); @@ -68,18 +71,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 diff --git a/gravity.h b/gravity.h index a8137fc..2002616 100644 --- a/gravity.h +++ b/gravity.h @@ -29,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