Proposal to change handling of ESP_ERR_NVS_NEW_VERSION_FOUND error to prevent data loss

[sentinelt]

Currently whenever nvs_flash_init() returns ESP_ERR_NVS_NEW_VERSION_FOUND the code erases the flash and then reruns nvs_flash_init():

        if let Some(err) = EspError::from(unsafe { nvs_flash_init() }) {
            match err.code() {
                ESP_ERR_NVS_NO_FREE_PAGES | ESP_ERR_NVS_NEW_VERSION_FOUND => {
                    esp!(unsafe { nvs_flash_erase() })?;
                    esp!(unsafe { nvs_flash_init() })?;
                }
                _ => (),
            }
        }

This approach, while common in many ESP-IDF examples, is IMHO not suitable for production environments. It leads to a problematic behavior: if older firmware, which is incompatible with the current NVS partition version, attempts to access the NVS, the partition will be erased and reinitialized. This results in the potential loss of critical data stored in the NVS.

Instead of automatically erasing and reinitializing the NVS upon encountering ESP_ERR_NVS_NEW_VERSION_FOUND, I propose to propagate the error back to the caller. This allows the calling function or application to handle the error in a manner that is appropriate for its specific requirements.

[ivmarkov]

What would your app do when it gets ESP_ERR_NVS_NEW_VERSION_FOUND?

[sentinelt]

Show user some error explaining that he uploaded too old firmware and needs to upgrade to continue.

[ivmarkov]

Hmm. How would you even show this error if the app can't even read its own data? Shouldn't you prevent the firmware flashing if it is too old in the first place? Or allow flashing but before that warn the user that all data would be lost?

[ivmarkov]

Maybe you can create an API sketch outlining your idea. The API should contain some way to forcibly erase and initialize the partition in case of one of these errors though.