search

rust-embedded-community / embedded-storage

An Embedded Storage Abstraction Layer
Apache License 2.0
63 stars 21 forks source link

Scope of embedded-storage #10

Closed Sympatron closed 9 months ago

Sympatron commented 3 years ago

I wanted to start a discussion about the scope of embedded-storage.

It could be one of the following:

In my experience with Rust most crates contain either traits to act as a common interface or implementations of other crates' traits. That would mean that the proposed NorFlashStorage of #9 would be better off in a separate crate. I have no strong feelings about this, this is just my observation and I wanted to start a discussion about this so we are all on the same page what the goals of embedded-storage are. It may even be a good idea to start with one crate to easily experiment and separate them later.

At the moment I am not fully convinced of the Storage trait's usefulness. I think traits are only useful if you plan to use it in a library that should be independent of the implementation. I don't really see that for Storage, because it seems to be geared towards the application which can just as easily use the concrete implementation rather than the trait.

Of course it may be that one of you had a specific use case in mind that I am missing and that is why I wanted to have a conversation about it.

MathiasKoch commented 3 years ago

I think this is a super valid point.

Personally my aim would be Aiming for traits for specific flash types with one super-trait that "JustWorks" as non-volatile storage where the performance at best is equal, and at worst is magnitudes worse that using the type specific trait correctly

Personally i don't see why it could be one of the items you listed, rather than all of them?

I think traits are only useful if you plan to use it in a library that should be independent of the implementation.

This part i fully agree on, but i do see lots of usecases for a generic Storage trait? I am not sure i understand why you would think the performance is necessarily as bad as you make it out to be? Pretty much every application with uneven write cycles i can think of would have exactly the same performance as using the NorFlash trait, but with much less hassle on the user?

Sympatron commented 3 years ago

This was not about the performance, but about the Storage trait. I see the use case for your proposed NorFlashStorage implementation and I think it can be in this crate for the time being. What I don't see is the use case for the trait. This does not have to mean that there is none, but I just don't see any.

I will stop pestering you about the performance issues. :P I am happy as long as it's behavior properly documented.

MathiasKoch commented 3 years ago

Ahh, okay, that's my bad!

In that case i might be more inclined towards agreeing with you. I have a few actual libraries where i need the user to provide a storage interface, and i while i guess i could just ask them for an actual NorFlashStorage, but i would much rather ask them for something implementing Storage, making it possible for them to wrap stuff up as they see fit?

I will stop pestering you about the performance issues. :P I am happy as long as it's behavior properly documented.

Hah, it's no problem. Im just trying to figure out if there is something i am missing here? But all my actual runs and implementations end up with exactly the same register reads/writes in both cases.

MathiasKoch commented 3 years ago

Actually, looking through my libraries it might be that i can live without it as a trait for most it.

Sympatron commented 3 years ago

But all my actual runs and implementations end up with exactly the same register reads/writes in both cases.

That's because you are using it correctly.

The way I would use it is to save records that are sent one by one to my device. These would have to be written and acknowledged one by one and this wouldn't go very well with this approach. My other use case would be data logging which would be equally as bad. I think have very different use cases in mind and there lies the confusion.

I looked into #12 and your MultiwriteNorFlash version would make this a lot better for most use cases. Maybe we should look into how to make this possible without specialization. Maybe a NewType wrapper?

As I said I am not against a Storage trait in general, so if you need it that's fine with me.

MathiasKoch commented 3 years ago

I looked into #12 and your MultiwriteNorFlash version would make this a lot better for most use cases

I fully agree! Unfortunately my internal flash has ECC, and does not allow writing multiple time to the same word as such.

Maybe we should look into how to make this possible without specialization. Maybe a NewType wrapper?

Sure, we could even make it as simple as MultiwriteNorFlashStorage. Naming might be off :p

chrysn commented 3 years ago

Discussing scope, is atomicity and more controlled page erasure something this crate should concern itself with?

Granted, an erase-free interface would (even with conservative assumptions) be much more generic-heavy than the current one, but is this still the right place? (Typical limitations would be that erasing happens for full pages, writes are only possible in particular alignments, and even if overwrites were to have zeroing-only semantics, some implementations would not allow overwrites[1]).

[1]: Particular example I looked at was nrf52, which does allow zeroing overwrites, but only 181 total writes between two erasures of a 512 byte block, which combined with the requirement to write in 32bit words looks like a margin of error in case 0xffffffff are written twice than a realistic method to do zeroing overwrites (which might be used by applications to write bits one-by-one, which a generic API can't allow for these).

chrysn commented 3 years ago

Scratch that, sorry for the noise -- the traits are already there and I just didn't find them / only looked at the top-level ones.