Converted EEPROM library to use nvs instead of partition. #2678

lbernstone · 2019-04-16T14:17:54Z

Converted EEPROM library to use nvs instead of partition.
Removed eeprom partition from all partition table CSV files.

…rom partition from all partition table CSV files.

libraries/EEPROM/src/EEPROM.cpp

me-no-dev · 2019-04-22T19:13:19Z

libraries/EEPROM/src/EEPROM.cpp

+      nvs_commit(_handle);
+      nvs_set_blob(_handle, _name, hold_data, size);
+      free(hold_data);
+      nvs_commit(_handle);


this whole block of code needs some comments to explain exactly what is going on :) I got lost with all those reads, writes and erases. What data is where... you need 2 blobs to hold the default 4K EEPROM, but reading through your code I got lost.

There is no way to expand a key, so in order to do so, it tests for free space, then holds the data while it deletes the key. If it is a new key, it just fills with zeros. I think changing the variable name to key_data will make it clearer.

me-no-dev · 2019-04-22T19:14:49Z

left a few notes :)

…dev's requests

me-no-dev · 2019-04-23T07:51:26Z

libraries/EEPROM/src/EEPROM.h

@@ -117,11 +105,10 @@ class EEPROMClass {
    template <class T> T writeAll (int address, const T &);

  protected:
-    uint32_t _sector;
+    nvs_handle _handle;


could you maybe forward declare the nvs_handle so it is not required to include nvs in the header?

…the library and persist data

stickbreaker · 2019-04-23T15:03:43Z

libraries/EEPROM/examples/eeprom_extra/eeprom_extra.ino

+  Serial.println(EEPROM.readByte(address));
+  address += sizeof(byte);
+
+  Serial.println(char(EEPROM.readChar(address)));


Does this actually compile? Shouldn't it be println((char)EEPROM.readChar(address));

travis would have complained :) not sure how Make will like it though

stickbreaker · 2019-04-23T15:05:35Z

libraries/EEPROM/examples/eeprom_extra/eeprom_extra.ino

  Serial.println(EEPROM.readString(address));
+  address += sentence.length();


pgrisu · 2019-04-27T15:35:11Z

This is a good feature that save a bit of flash space but it breaks all deployed systems where factory settings are stored into EEPROM flash partition.
Already deployed devices based on previous arduino-esp32 will lose all EEPROM factory/config settings if they move to latest release by FOTA
@me-no-dev Please remember to mark this change as 'breaking changes' in next release notes

me-no-dev · 2019-04-28T07:13:32Z

Will do! @lbernstone any ideas on how that can be detected and moved to NVS?

fedy0 · 2019-09-11T21:42:12Z

@me-no-dev
The new move of the EEPROM by this pull request from flexible flash data partition to NVS, in my opinion, is superfluous

My reasons:
[1] Already Existing Interface: The Preference library already provides a complete and an excellent friendly access to ESP32's Non-Volatile Storage. As a novice in software, I've always been warned not to repeat myself
[2] Not Backward Compatible: The pull request breaks changes not only in deployed systems (which is fatal) as mentioned by @pgrisu but also in the classical EEPROM examples.
[3] NVS Usage: Non-Volatile Storage (NVS) was primarily designed to store key-value pairs in flash which the preference library mastered.
[4] Flexibility: The partition table already encouraged user-defined partitions (with data subtypes starting @0x99) which EEPROMClass can enjoy that this pull request deprecated.
[5] Same storage: All data partitions are on one flash memory with predefined subtypes (as used in the partition table) acting as constraints to their method of usage. If an NVS partition is truncated (for example, when the partition table layout is changed), its contents should be erased which is same for the previous EEPROM implementation. Why move the EEPROM?

arduino-esp32 is too matured to start having breaking changes caused by memory partitioning.
Thanks once again for your great contribution in the Preference library @me-no-dev

tomtobback · 2019-10-21T02:56:36Z

Hi, in addition to breaking EEPROM it also breaks any files stored earlier in SPIFFS; the SPIFFS offset moved from 0x291000 to 0x290000. On on my system, any SPIFFS files saved with core 1.0.2 cannot be read by core 1.0.3. I can only read SPIFFS with 1.0.3 when i edit my partitions/default.csv back to the 1.0.2 values (line 6: spiffs). Those 1000 bytes gained are not worth breaking backward compatibility in my humble opinion. I was surprised not to find any documentation on this because i assume lots of people have/will run into this problem.

Converted EEPROM library to use nvs instead of partition. Removed eep…

20f9f83

…rom partition from all partition table CSV files.