Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scope of embedded-storage #10

Closed
Sympatron opened this issue Mar 3, 2021 · 8 comments
Closed

Scope of embedded-storage #10

Sympatron opened this issue Mar 3, 2021 · 8 comments

Comments

@Sympatron
Copy link
Contributor

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

It could be one of the following:

  • Define a common interface between device drivers and generic storage implementations (e.g. filesystems) like embedded-hal.
    That way things like LittleFS on any flash chip with an embedded-storage compatible driver would be possible. That's basically what NorFlash currently does.
  • Define an traits for generic, simple, easy to use, byte addressed storage that could be implemented based on any technology. That is currently the Storage trait.
  • Provide most commonly used storage implementations based on the traits above.

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
Copy link
Collaborator

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
Copy link
Contributor Author

Sympatron commented Mar 3, 2021

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
Copy link
Collaborator

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
Copy link
Collaborator

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

@Sympatron
Copy link
Contributor Author

Sympatron commented Mar 3, 2021

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
Copy link
Collaborator

MathiasKoch commented Mar 3, 2021

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
Copy link

chrysn commented Sep 13, 2021

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
Copy link

chrysn commented Sep 13, 2021

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.

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

No branches or pull requests

3 participants