Merge pull request #5227 from jepler/esp32-nvm

esp32s2: rework nvm/nvs storage

Author: tannewt

locale/circuitpython.pot

@@ -1225,7 +1225,7 @@ msgstr ""
 msgid "Insufficient encryption"
 msgstr ""
 
-#: ports/raspberrypi/audio_dma.c
+#: ports/atmel-samd/audio_dma.c ports/raspberrypi/audio_dma.c
 msgid "Internal audio buffer too small"
 msgstr ""
 
@@ -1392,7 +1392,8 @@ msgstr ""
 #: ports/mimxrt10xx/common-hal/busio/SPI.c ports/nrf/common-hal/busio/I2C.c
 #: ports/raspberrypi/common-hal/busio/I2C.c
 #: ports/raspberrypi/common-hal/busio/SPI.c
-#: ports/raspberrypi/common-hal/busio/UART.c
+#: ports/raspberrypi/common-hal/busio/UART.c shared-bindings/busio/SPI.c
+#: shared-bindings/busio/UART.c
 msgid "Invalid pins"
 msgstr ""
 
@@ -3915,8 +3916,10 @@ msgstr ""
 #: ports/esp32s2/boards/adafruit_funhouse/mpconfigboard.h
 #: ports/esp32s2/boards/adafruit_magtag_2.9_grayscale/mpconfigboard.h
 #: ports/esp32s2/boards/adafruit_metro_esp32s2/mpconfigboard.h
+#: ports/esp32s2/boards/ai_thinker_esp_12k_nodemcu/mpconfigboard.h
 #: ports/esp32s2/boards/artisense_rd00/mpconfigboard.h
 #: ports/esp32s2/boards/atmegazero_esp32s2/mpconfigboard.h
+#: ports/esp32s2/boards/crumpspace_crumps2/mpconfigboard.h
 #: ports/esp32s2/boards/electroniccats_bastwifi/mpconfigboard.h
 #: ports/esp32s2/boards/espressif_kaluga_1.3/mpconfigboard.h
 #: ports/esp32s2/boards/espressif_kaluga_1/mpconfigboard.h
@@ -3933,6 +3936,7 @@ msgstr ""
 #: ports/esp32s2/boards/morpheans_morphesp-240/mpconfigboard.h
 #: ports/esp32s2/boards/muselab_nanoesp32_s2_wroom/mpconfigboard.h
 #: ports/esp32s2/boards/muselab_nanoesp32_s2_wrover/mpconfigboard.h
+#: ports/esp32s2/boards/odt_pixelwing_esp32_s2/mpconfigboard.h
 #: ports/esp32s2/boards/targett_module_clip_wroom/mpconfigboard.h
 #: ports/esp32s2/boards/targett_module_clip_wrover/mpconfigboard.h
 #: ports/esp32s2/boards/unexpectedmaker_feathers2/mpconfigboard.h

ports/esp32s2/bindings/espidf/__init__.c

@@ -26,9 +26,12 @@
 
 #include "py/obj.h"
 #include "py/runtime.h"
+#include "py/mphal.h"
+
 
 #include "bindings/espidf/__init__.h"
 
+#include "nvs_flash.h"
 #include "components/heap/include/esp_heap_caps.h"
 
 //| """Direct access to a few ESP-IDF details. This module *should not* include any functionality
@@ -65,6 +68,20 @@ STATIC mp_obj_t espidf_heap_caps_get_largest_free_block(void) {
 }
 MP_DEFINE_CONST_FUN_OBJ_0(espidf_heap_caps_get_largest_free_block_obj, espidf_heap_caps_get_largest_free_block);
 
+//| def erase_nvs() -> None:
+//|     """Erase all data in the non-volatile storage (nvs), including data stored by with `microcontroller.nvm`
+//|
+//|     This is necessary when upgrading from CircuitPython 6.3.0 or earlier to CircuitPython 7.0.0, because the
+//|     layout of data in nvs has changed. The old data will be lost when you perform this operation."""
+STATIC mp_obj_t espidf_erase_nvs(void) {
+    ESP_ERROR_CHECK(nvs_flash_deinit());
+    ESP_ERROR_CHECK(nvs_flash_erase());
+    ESP_ERROR_CHECK(nvs_flash_init());
+    return mp_const_none;
+}
+MP_DEFINE_CONST_FUN_OBJ_0(espidf_erase_nvs_obj, espidf_erase_nvs);
+
+
 //| class IDFError(OSError):
 //|     """Raised for certain generic ESP IDF errors."""
 //|     ...
@@ -117,6 +134,8 @@ STATIC const mp_rom_map_elem_t espidf_module_globals_table[] = {
     { MP_ROM_QSTR(MP_QSTR_heap_caps_get_free_size), MP_ROM_PTR(&espidf_heap_caps_get_free_size_obj)},
     { MP_ROM_QSTR(MP_QSTR_heap_caps_get_largest_free_block), MP_ROM_PTR(&espidf_heap_caps_get_largest_free_block_obj)},
 
+    { MP_ROM_QSTR(MP_QSTR_erase_nvs), MP_ROM_PTR(&espidf_erase_nvs_obj)},
+
     { MP_ROM_QSTR(MP_QSTR_IDFError), MP_ROM_PTR(&mp_type_espidf_IDFError) },
     { MP_ROM_QSTR(MP_QSTR_MemoryError),      MP_ROM_PTR(&mp_type_espidf_MemoryError) },
 };

ports/esp32s2/common-hal/nvm/ByteArray.c

@@ -24,9 +24,13 @@
  * THE SOFTWARE.
  */
 
+#include <string.h>
+
 #include "common-hal/nvm/ByteArray.h"
+#include "bindings/espidf/__init__.h"
 
 #include "py/runtime.h"
+#include "py/gc.h"
 #include "nvs_flash.h"
 
 uint32_t common_hal_nvm_bytearray_get_length(nvm_bytearray_obj_t *self) {
@@ -50,48 +54,94 @@ static void get_nvs_handle(nvs_handle_t *nvs_handle) {
     }
 }
 
+// Get a copy of the nvm data, or an array initialized to all 0 bytes if it is not present
+static esp_err_t get_bytes(nvs_handle_t handle, uint8_t **buf_out) {
+    size_t size;
+    void *buf;
+    esp_err_t result = nvs_get_blob(handle, "data", NULL, &size);
+    if (result == ESP_ERR_NVS_NOT_FOUND) {
+        size = CIRCUITPY_INTERNAL_NVM_SIZE;
+    } else if (result != ESP_OK) {
+        *buf_out = NULL;
+        return result;
+    }
+    buf = gc_alloc(size, 0, false); // this SHOULD be the same as
+    if (result == ESP_OK) {
+        result = nvs_get_blob(handle, "data", buf, &size);
+    } else {
+        result = ESP_OK;
+    }
+    *buf_out = buf;
+    return result;
+}
+
 bool common_hal_nvm_bytearray_set_bytes(nvm_bytearray_obj_t *self,
     uint32_t start_index, uint8_t *values, uint32_t len) {
-    char index[9];
 
     // start nvs
     nvs_handle_t handle;
     get_nvs_handle(&handle);
 
-    // stage flash changes
-    for (uint32_t i = 0; i < len; i++) {
-        sprintf(index, "%i", start_index + i);
-        if (nvs_set_u8(handle, (const char *)index, values[i]) != ESP_OK) {
-            return false;
-        }
+    // get from flash
+    uint8_t *buf = NULL;
+    esp_err_t result = get_bytes(handle, &buf);
+    if (result != ESP_OK) {
+        gc_free(buf);
+        nvs_close(handle);
+        raise_esp_error(result);
+    }
+
+    // erase old data, including 6.3.x incompatible data
+    result = nvs_erase_all(handle);
+    if (result != ESP_OK) {
+        gc_free(buf);
+        nvs_close(handle);
+        raise_esp_error(result);
+    }
+
+    // make our modification
+    memcpy(buf + start_index, values, len);
+
+    result = nvs_set_blob(handle, "data", buf, CIRCUITPY_INTERNAL_NVM_SIZE);
+    if (result != ESP_OK) {
+        gc_free(buf);
+        nvs_close(handle);
+        raise_esp_error(result);
     }
 
-    // commit flash changes
-    if (nvs_commit(handle) != ESP_OK) {
-        return false;
+    result = nvs_commit(handle);
+    if (result != ESP_OK) {
+        gc_free(buf);
+        nvs_close(handle);
+        raise_esp_error(result);
     }
 
     // close nvs
+    gc_free(buf);
     nvs_close(handle);
     return true;
 }
 
 void common_hal_nvm_bytearray_get_bytes(nvm_bytearray_obj_t *self,
     uint32_t start_index, uint32_t len, uint8_t *values) {
-    char index[9];
 
     // start nvs
     nvs_handle_t handle;
     get_nvs_handle(&handle);
 
     // get from flash
-    for (uint32_t i = 0; i < len; i++) {
-        sprintf(index, "%i", start_index + i);
-        if (nvs_get_u8(handle, (const char *)index, &values[i]) != ESP_OK) {
-            mp_raise_RuntimeError(translate("NVS Error"));
-        }
+    uint8_t *buf;
+    esp_err_t result = get_bytes(handle, &buf);
+    if (result != ESP_OK) {
+        gc_free(buf);
+        nvs_close(handle);
+        raise_esp_error(result);
     }
 
+    // copy the subset of data requested
+    memcpy(values, buf + start_index, len);
+
     // close nvs
+    gc_free(buf);
     nvs_close(handle);
 }

ports/esp32s2/mpconfigport.h

@@ -42,8 +42,14 @@
 
 #define CIRCUITPY_INTERNAL_NVM_START_ADDR (0x9000)
 
+// 20kB is statically allocated to nvs, but when overwriting an existing
+// item, it's temporarily necessary to store both the old and new copies.
+// Additionally, there is some overhad for the names and values of items
+// in nvs, and alignment to 4kB flash erase boundaries may give better
+// performance characteristics (h/t @tannewt). This implies we should select an
+// 8kB size for CircuitPython'ns NVM.
 #ifndef CIRCUITPY_INTERNAL_NVM_SIZE
-#define CIRCUITPY_INTERNAL_NVM_SIZE (20 * 1024)
+#define CIRCUITPY_INTERNAL_NVM_SIZE (8 * 1024)
 #endif
 
 #endif  // __INCLUDED_ESP32S2_MPCONFIGPORT_H