esp-rs / esp-idf-svc

Type-Safe Rust Wrappers for various ESP-IDF services (WiFi, Network, Httpd, Logging, etc.)
https://docs.esp-rs.org/esp-idf-svc/
Apache License 2.0
333 stars 183 forks source link

Add missing mutability qualifiers for nvs #334

Closed torkleyy closed 9 months ago

torkleyy commented 11 months ago

I think this is an oversight, set_str and set_blob require &mut self. OTOH, it seems the nvs functions perform internal locking, indicating that they might be thread safe and none of the functions require mutability.

Vollbrecht commented 11 months ago

you say that they don't require it but now you want to enforce it? Can you elaborate a bit more?

torkleyy commented 11 months ago

@Vollbrecht haha yeah I'm not sure which one is correct, I think one could argue to make them mutable even though it would be safe without (to avoid data races, because there are multiple individually locked calls).

torkleyy commented 11 months ago

If that's the case I will revert my change and just add some docs explaining the rationale. It just seemed weird at first sight.