Updating sequential-storage, adding flash erase support #884
Conversation
src/settings.rs
Outdated
| } else { | ||
| // Erase the whole flash range. | ||
| embassy_futures::block_on( | ||
| self.storage.erase(range.start, range.end), |
There was a problem hiding this comment.
Is this a multi page erase? Wouldn't it be better to have sequential-storage do it "smart"?
I suspect sequential-storage should provide a (much more efficient than multiple remove_item) way to clear (distinct from flash erase) its storage. Maybe they have an idea on what's best here.
There was a problem hiding this comment.
Yes, this erases 1MB of flash and takes quite a while to do (several seconds).
I definitely think this could be done more efficiently on the sequential-storage side of things (i.e. scan the device, look for used pages, erase those pages only). I'll take a look.
There was a problem hiding this comment.
We should also reduce the flash range for sequential storage. We don't need to allocate 1 MB.
There was a problem hiding this comment.
While we don't need 1MB, I think we have to allocate at least 128KB * 3 based on the design of sequential-storage and the layout of flash pages in the device unfortunately.
Sequential storage requires at least 3 pages from what I remember.
There was a problem hiding this comment.
It can do with 2 right now. But your effective storage size page_size * (num_pages - 1) for map, because it needs to keep one page as a buffer page.
There was a problem hiding this comment.
Ack. Nice work on sequential-storage!
Let's just take current sequential storage master, wait for release, and 2 pages (three breaking changes in one ;). It'll be a while in practice until it needs to erase pages.
There was a problem hiding this comment.
Yeah I'm currently testing, but hitting HardFault exceptions after erasing and then re-reading flash sections. I believe it's due to flash ECC code failures, but what we're doing should be supported (since we're only clearing bits). I'll dig into what's going wrong here over the next day or two
There was a breaking change to the on-disk data format in 0.7, so yes this will break things. Also, I'm very close to releasing v2.0 with some API changes that make dealing with the keys and values easier (and more straightforward). |
Fixes #861 by updating
clear <setting>to actually erase the requested setting from flash memory instead of overwriting it with the current default value.This PR also updates to
sequential-storagev1.0 by using embassy-futures to complete the async operations in a blocking manner. This shouldn't matter at all because the underlying impl is blocking anyways. However, the version bump may cause users to lose their flash settings.To test this, I cleared flash settings and rebooted the device. I then reprogrammed it with a new default broker of "mqtt2" and verified that the device used the new default of mqtt2 after reprogramming, which indicated that nothing was loaded from flash.