RebelTechnology
This gets called from ISR currently
We store settings from the USB interrupt and current code hangs when disabling interrupts in the old function. AFAICT calling it not from an ISR would be safe too.
Good find. What do you think about moving the critical lock out of the function instead, like this: https://github.com/RebelTechnology/OpenWare/commit/44ea65c3a80c0227e88b45217b8b9f7775e1643e
Well considering that this left saving settings broken for a year or so, I had no choice but debug this.
Regarding linked branch, there are some minor concerns:
- Critical section disables interrupts and we want to make it as short as possible. This update makes CS larger, so probably not a good thing.
- Moving CS to a higher level means that underlying code (
Storage::writeResource()) becomes not interrupt-safe, so it should be documented. It becomes caller's responsibility to add a CS around it and it's easy to forget about this - Is it necessary to place
registry.init();in that CS too? I would guess it doesn't need it as it only reads from flash.
Concerns noted.
The code is called from two places:
When saving a patch, no patch should be running and a longer CS is not a problem. Including registry.init() in the CS means that any attempt to change the patch while storing a new one should not be a problem.
When saving settings, worst case I figure there may be an audio drop, though the saved data is very small so this is probably unlikely. Regardless I don't think saving system settings is something a user needs to do mid set.
I cut a release candidate, please help test: https://github.com/RebelTechnology/OpenWare/releases/tag/v22.5.rc4
When saving settings, worst case I figure there may be an audio drop, though the saved data is very small so this is probably unlikely.
This used to be true with the previous code version, but in that branch the critical section would include the call to Storage::defrag and it's not very fast with external flash.
The PR ended up with a conflict apparent apparently - https://github.com/RebelTechnology/OpenWare/pull/81/conflicts . I'd say it's worth to try making a call for testing on forum as it resolves issues quite a few people have ran into.