Skip to content

Persist bank + sample state and calibration data to flash across restarts (ectocore only)#784

Merged
schollz merged 9 commits intomainfrom
copilot/save-bank-and-sample-on-load
Oct 25, 2025
Merged

Persist bank + sample state and calibration data to flash across restarts (ectocore only)#784
schollz merged 9 commits intomainfrom
copilot/save-bank-and-sample-on-load

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 25, 2025

  • Explore repository structure and understand the codebase
  • Identify key files: main.c, globals.h, flashmem.h, sdcard_startup.h
  • Understand that sel_bank_cur and sel_sample_cur are the current bank/sample
  • Understand that playback_stopped flag tracks playback state
  • Understand that flashmem.h already provides flash read/write functions
  • Create a persistent storage structure for bank+sample state
  • Add code to save bank+sample when playback stops
  • Add code to restore bank+sample on startup
  • Add validation checks for corrupted data
  • Mark flash access functions with __not_in_flash_func() to ensure they run from RAM
  • Fix FLASH_SECTOR_SIZE constant references
  • Mark PersistentState_load with __not_in_flash_func() for safety
  • Complete code review and address issues
  • Refactor to combine calibration and bank/sample data structures
  • Fix file loading issue: Trigger file change after savefile_do_load
  • Synchronize sel_bank_next and sel_sample_next with restored values
  • Restrict feature to ectocore only
  • Fix ectocore loop to preserve restored bank state
    • Changed initialization of sel_bank_next_new from 0 to sel_bank_cur
    • Changed initialization of sel_sample_next_new from 0 to sel_sample_cur
    • These local variables in the ectocore loop now start with current state instead of always 0
    • Prevents the loop from immediately resetting bank to 0 when checking for changes

Root Cause:

The ectocore input handling loop has local variables sel_bank_next_new and sel_sample_next_new that were always initialized to 0. When the loop checked if the bank changed (sel_bank_cur != sel_bank_next_new), it would always detect a "change" from the restored bank (e.g., 1) to 0, then set sel_bank_next = 0, effectively resetting the bank.

Solution:

Initialize these local tracking variables with the current bank/sample values so they start in sync with the global state, allowing proper change detection.

Original prompt

This section details on the original issue you should resolve

<issue_title>Save bank + sample and restore on load</issue_title>
<issue_description> I'd like to add a new feature to the ectocore. Whenever playback is STOPPED (zeptocore or ectocore), save
the current bank and sample so that when it starts up again it starts from that bank+sample. There are two
choices for saving - internal flash or sd card. I'd try going with the internal flash first, which is fine.
It just means that when a new firmware is uploaded the dat amight be corrupted so there should be some basic
checks to make sure the data is not corrupted and that the bank/sample do exist before using them on startup.

</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits October 25, 2025 15:23
Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>
Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>
@schollz
Copy link
Copy Markdown
Owner

schollz commented Oct 25, 2025

@copilot A problem: The persistent save is corrupting the save data from the calibration. Probably better to combine the data structures for calibration + the bank/sample to load from because both need to be loaded on startup. You may need to refactor the calibration/saving loading so it works with your PersistentStorage

…as RAM-resident

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>
Copilot AI changed the title [WIP] Add feature to save bank and sample on playback stop Persist bank + sample state to flash across restarts Oct 25, 2025
Copilot AI requested a review from schollz October 25, 2025 15:32
…cture

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 25, 2025

@copilot A problem: The persistent save is corrupting the save data from the calibration. Probably better to combine the data structures for calibration + the bank/sample to load from because both nee...

Fixed in commit 750b27e.

I've refactored the code to use a unified PersistentState structure that combines both calibration data and bank/sample data in a single flash storage location. Key changes:

  1. Expanded PersistentState: Now includes center_calibration[8] array alongside bank/sample fields
  2. Mutual preservation:
    • PersistentState_save() (bank/sample) reads existing data first to preserve calibration
    • PersistentState_save_calibration() reads existing data first to preserve bank/sample
  3. Replaced EctocoreFlash: Removed the separate structure and updated ectocore.h to use the new unified functions
  4. Updated checksum: Now includes calibration data for complete validation

Both features now share the same flash sector without conflicts.

Copilot AI changed the title Persist bank + sample state to flash across restarts Persist bank + sample state and calibration data to flash across restarts Oct 25, 2025
@schollz
Copy link
Copy Markdown
Owner

schollz commented Oct 25, 2025

@copilot Perfect! It is actually getting the stored bank/sample information. However it is NOT loading the new bank/file on startup. Here's the startup debug info:

sdcard_startup] checking settings/grimoire/rune7
[sdcard_startup] bank 0 has 2 samples
[sdcard_startup] bank 1 has 2 samples
[PersistentState] Successfully loaded bank=1 sample=1 fh
[sdcard_startup] Restoring to bank 1, sample 1 from flah
memory usage: 64.4% (136580/212116)
[FV_Reverb_malloc] heap_size: 5484 (0 combs, 3 allpasse)
[FV_Reverb_malloc] heap_size: 9976 (1 combs, 3 allpasse)
[FV_Reverb_malloc] heap_size: 16232 (2 combs, 3 allpass)
[FV_Reverb_malloc] heap_size: 21012 (3 combs, 3 allpass)
[FV_Reverb_malloc] heap_size: 27004 (4 combs, 3 allpass)
[FV_Reverb_malloc] heap_size: 32456 (5 combs, 3 allpass)
[FV_Reverb_malloc] heap_size: 37592 (6 combs, 3 allpass)
[FV_Reverb_malloc] num_combs: 4, num_allpasses: 3
memory usage: 77.2% (163668/212116)
updating bpm timer: 185-> 170 (172)
[ectocore] startup
[PersistentState] Successfully loaded calibration from h
[ectocore] calibration data loaded from flash
[ectocore] calibrate 7=511,[ectocore] calibrate 6=570,[,
CTRL-A Z for help | 115200 8N1 | NOR | Minicom 2.9 | V

It says its loading, but the actual file is not being loaded for playback.

…xists

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 25, 2025

@copilot Perfect! It is actually getting the stored bank/sample information. However it is NOT loading the new bank/file on startup. Here's the startup debug info:

sdcard_startup] checking settings...

Fixed in commit 4027bda.

The issue was that the restored bank/sample values weren't triggering the file change mechanism. The code was setting sel_bank_cur and sel_sample_cur correctly, but the file loading system uses sel_bank_next/sel_sample_next and fil_current_change to trigger changes.

Now after savefile_do_load() completes, if no savefile was loaded from SD card, the code triggers a file change with the flash-restored values. This gives savefiles precedence over flash restoration, which makes sense since savefiles are explicit user configurations.

@schollz
Copy link
Copy Markdown
Owner

schollz commented Oct 25, 2025

@copilot Perfect! It is now loading the sample correclty! But one more thing - it is not setting the current bank such that when the ectocore "sample" knob is turned, it switches to a sample from bank 0, not from the saved bank which was set (which in this case was bank 1)

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 25, 2025

@copilot Perfect! It is now loading the sample correclty! But one more thing - it is not setting the current bank such that when the ectocore "sample" knob is turned, it switches to a sample from bank...

Fixed in commit d3cc4d4.

The issue was that only sel_bank_cur and sel_sample_cur were being set during restoration, while sel_bank_next and sel_sample_next remained at their initialized value of 0. This caused an inconsistency where the sample knob code (which uses banks[sel_bank_cur]->num_samples) would see the correct bank, but other parts of the system checking the _next variables would see bank 0.

Now all four variables are synchronized when restoring from flash, ensuring consistent bank selection throughout the system.

@schollz
Copy link
Copy Markdown
Owner

schollz commented Oct 25, 2025

@copilot One more thing, make this feature available to the ectocore only for now

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 25, 2025

@copilot One more thing, make this feature available to the ectocore only for now

Done in commit 6104976.

Added #ifdef INCLUDE_ECTOCORE guards around all bank+sample persistence code in main.c and sdcard_startup.h. The feature now only compiles and runs for ectocore builds, not zeptocore.

Copilot AI changed the title Persist bank + sample state and calibration data to flash across restarts Persist bank + sample state and calibration data to flash across restarts (ectocore only) Oct 25, 2025
@schollz
Copy link
Copy Markdown
Owner

schollz commented Oct 25, 2025

@copilot Hm. There is still an issue, where it loads the bank/sample correctly and plays it from the right file and All that is perfect. However, when changing the sample knob it reverts to bank 0 instead of staying on the bank loaded. Somehow the bank loading persists to load the sample, but not persisting into the main ectocore loop when sample knob is changed.

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 25, 2025

@copilot Hm. There is still an issue, where it loads the bank/sample correctly and plays it from the right file and All that is perfect. However, when changing the sample knob it reverts to bank 0 ins...

Fixed in commit 1c41960.

The issue was in the ectocore input handling loop. The local variables sel_bank_next_new and sel_sample_next_new were always initialized to 0 at the start of the loop. When the loop checked for bank changes (sel_bank_cur != sel_bank_next_new), it detected a false "change" from the restored bank (e.g., 1) to 0, then set sel_bank_next = 0, resetting the bank.

Now these variables are initialized with the current bank/sample values (sel_bank_cur and sel_sample_cur), so they start in sync with the global state and only detect actual changes.

@schollz schollz marked this pull request as ready for review October 25, 2025 16:56
@schollz schollz merged commit 4e3ae1c into main Oct 25, 2025
@schollz schollz deleted the copilot/save-bank-and-sample-on-load branch October 25, 2025 16:57
schollz added a commit that referenced this pull request Oct 25, 2025
…arts (ectocore only) (#784)

* Initial plan

* Add persistent bank+sample state saving to flash

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>

* Fix flash access by marking functions as RAM-resident

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>

* Fix code review issues: use correct constants and mark load function as RAM-resident

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>

* Combine calibration and bank/sample into unified PersistentState structure

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>

* Fix bank/sample restoration to trigger file change when no savefile exists

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>

* Initialize sel_bank_next and sel_sample_next with restored values

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>

* Restrict bank+sample persistence to ectocore only

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>

* Initialize ectocore loop bank/sample tracking with current values

Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: schollz <6550035+schollz@users.noreply.github.com>
Former-commit-id: 4e3ae1c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Save bank + sample and restore on load

2 participants