esp-idf-svc icon indicating copy to clipboard operation
esp-idf-svc copied to clipboard

The state of EspNvsStorage

Open tcbennun opened this issue 3 years ago • 3 comments
trafficstars

Could you please help me understand what the state of affairs is with the NVS storage part, @ivmarkov ? Looking to contribute.

  1. Why does the RawStorage impl utilise both u64 and blob? Is it just more space-efficient to store with u64 if the blob is 8 bytes or less?
  2. For the future Storage impl, are we to allow the user to choose a serialization format, or should we just impl with something efficient and embedded-friendly like postcard?
  3. Should the Storage trait have its own associated error type, separate from that of StorageBase, in order to encompass (de)serialization errors?

tcbennun avatar Aug 15 '22 08:08 tcbennun

Could you please help me understand what the state of affairs is with the NVS storage part, @ivmarkov ? Looking to contribute.

  1. Why does the RawStorage impl utilise both u64 and blob? Is it just more space-efficient to store with u64 if the blob is 8 bytes or less?

Yes. RawStorage impl for ESP-IDF is implemented by utilizing the ESP-IDF NVS library. Using BLOBs for everything is not very efficient.

  1. For the future Storage impl, are we to allow the user to choose a serialization format, or should we just impl with something efficient and embedded-friendly like postcard?

Users will be able to whichever serde they want by implementing the SerDe trait. Or just implement their own Storage impl, if they want to do something more exotic.

  1. Should the Storage trait have its own associated error type, separate from that of StorageBase, in order to encompass (de)serialization errors?

The fact that Storage shares the error type with StorageBase does not mean that its impl (i.e. the SerDe-based impl that we'll provide out of the box) is obliged to share its error type with a potential RawStorage impl to which it might delegate. See here.

I might get rid of EitherError though in favour of a dedicated error type, but in any case the dedicated error type would still have an "either" semantics.

ivmarkov avatar Aug 15 '22 09:08 ivmarkov

Thanks. I didn't know about the next branch in embedded-svc, that makes things a lot clearer! When are you looking to merge that in?

tcbennun avatar Aug 15 '22 09:08 tcbennun

In a week, max two. Need to remove/move elsewhere a bit more util stuff from embedded-svc and then it should be ready.

ivmarkov avatar Aug 15 '22 09:08 ivmarkov

I think this is all addressed with the new release since a couple of months. There is a separate issue request for compatibility between the Rust storage impl and the C one.

ivmarkov avatar Jan 21 '23 15:01 ivmarkov