search

tweedegolf / sequential-storage

A crate for storing data in flash memory with minimal need for erasing pages
Apache License 2.0
87 stars 8 forks source link

Queue Iterator which can decide to pop or peek after looking at the item #37

Closed t-moe closed 3 months ago

t-moe commented 4 months ago

Hello again @diondokter ,

I'm using your crate not only to/store restore device configs (map), but also to store logs (queue).

I've created a defmt sink ("global logger") which will store the logs using sequential_storage::queue::push. ( It does not do so directly, instead the logs first go to memory and a concurrent task will then store the logs in the flash, whenever the cpu is idle). Later, the cloud can request logs from the device, if there has been a problem. The device will then iterate the flash using the sequential_storage::queue::PopIterator and send the logs to the cloud. This works pretty well so far.

I currently store logs of all log-levels in the flash, but the cloud might only be interested in logs above a certain log-level. For that it would be helpful if I can iterate the queue, look at the item, and only then decide if I want to pop the item (and send it to the cloud) or continue with the next item. The idea is to be able to first request only the logs of level error. And later the cloud can request also debug/info/warn logs for a specific time-window, if more investigation is nedded.

This goes a bit against the Queue/FIFO principle, but would be very useful nonetheless.

I dont need this feature very urgently, but I wanted to get your opinion on it and discuss options...

Thank you

diondokter commented 4 months ago

Ah yes, I can see this work! And it's pretty reasonable too.

Not sure when I have time to get to it, but I think it's a good thing to add.

t-moe commented 3 months ago

Same here: Cool, thanks a lot for implementing this 🥳 . I will take a look in the coming weeks and report back.

t-moe commented 3 months ago

I've migrated to the master branch today, in order to test this feature. This seems to work as expected. Thank you!

Sidenote:

The migration to the master branch went reasonably smooth, except that adapting to the following change was a bit cumbersome:

Breaking: Made the cache API a bit more strict. Caches now always have to be passed as a mutable reference. The API before would lead to a lot of extra unncesessary binary size.

With this change I now have to store the NoCache instance for the entire life-time of the iterator, which is a bit cumbersome, because I'm trying to create a self-contained LogIterator which uses your QueueIterator internally. But yeah, nothing unsolvable and its the same thing for the flash instance. I just found it a bit funny, that I have to store a no-op type now and I liked the version which consumed the cache a bit better. I can totally live with it though and I'm glad it helps with reducing the binary size :)

diondokter commented 3 months ago

With this change I now have to store the NoCache instance for the entire life-time of the iterator, which is a bit cumbersome, because I'm trying to create a self-contained LogIterator which uses your QueueIterator internally. But yeah, nothing unsolvable and its the same thing for the flash instance. I just found it a bit funny, that I have to store a no-op type now and I liked the version which consumed the cache a bit better. I can totally live with it though and I'm glad it helps with reducing the binary size :)

Yep, I agree! The old way was nice, but it would generate 3(!!!) versions of many functions. Once for T, once for &mut T and once for &mut &mut T. So yeah, sadly this is the cost. If you feel there's a good alternative, then let me know! This breaking change hasn't been released yet.