EEPROM_library_ported_Partition_table_updated #535

pbecchi · 2017-07-23T10:49:03Z

This is the outcome of issue 525.

everslick · 2017-07-24T16:30:06Z

I'm about to test your implementation today. Thanks for providing a clean implementation for the EEPROM emulation. At first glance I noticed, that your PR contains build files like dependency and object files. Those should probably get removed. But maybe the maintainers can cherry pick files? I dunno.

Also 'libraries/EEPROM/library.properties' seems to contain outdated information.

pbecchi · 2017-07-24T17:26:38Z

I have deleted all unnecessary files and folders from example directory and I have somehow corrected the properties file!

everslick · 2017-07-24T20:54:31Z

Tested. Works as advertised. :-D Thanks a lot!

Readability change, Concern about the SubType being 0x99.

igrr · 2017-07-27T12:50:39Z

libraries/EEPROM/EEPROM.cpp

+  if (!_data)
+    return false;
+
+  noInterrupts();


Not needed on the ESP32

You mean interrupts doesn't need to be stopped? I will take out the relevant lines.

There's no need to stop the interrupts. Besides, noInterrupts and interrupts functions are no-op in the current version.

This comment is still valid

igrr · 2017-07-27T12:50:47Z

libraries/EEPROM/EEPROM.cpp

+		  ret = true;
+	  }
+  }
+  interrupts();


igrr · 2017-07-27T12:51:02Z

libraries/EEPROM/EEPROM.cpp

+  }
+  else
+  {
+	  if (esp_partition_write(_mypart, 0, (void *)_data, _size) == ESP_ERR_INVALID_SIZE)


Indentation does not match the rest of the file

igrr · 2017-07-27T12:52:07Z

libraries/EEPROM/examples/eeprom_write/eeprom_write.ino

+int addr = 0;
+#define N512 64
+void setup()
+{


Please use "Format Sketch" feature in Arduino IDE when preparing example sketches.

I am not using Arduino Ide I will check it anyhow.

igrr · 2017-07-27T12:53:06Z

libraries/EEPROM/examples/eeprom_write/eeprom_write.ino

+// the current address in the EEPROM (i.e. which byte
+// we're going to write to next)
+int addr = 0;
+#define N512 64


Giving this define some descriptive name would be nice (e.g. EEPROM_SIZE?)

igrr · 2017-07-27T12:54:06Z

libraries/EEPROM/EEPROM.cpp

+
+  noInterrupts();
+
+  if (esp_partition_erase_range(_mypart, 0, 4096) != ESP_OK)


suggest using SPI_FLASH_SEC_SIZE for the constant instead

igrr · 2017-07-27T12:55:06Z

tools/partitions/default.csv

@@ -3,4 +3,5 @@ nvs,      data, nvs,     0x9000,  0x5000,
 otadata,  data, ota,     0xe000,  0x2000,
 app0,     app,  ota_0,   0x10000, 0x140000,
 app1,     app,  ota_1,   0x150000,0x140000,
-spiffs,   data, spiffs,  0x290000,0x170000,
+eeprom,   data, 0x99,    0x290000,0x1000,
+spiffs,   data, spiffs,  0x291000,0x169000


It may be a better idea to shrink app partition instead. Otherwise SPIFFS partition will need to be re-uploaded after this change.

Initially I had the EEprom partition after the Nvs one shrinking Nvs partition 4kbytes.
Then I change it for me-no-dev comment on #525

Since spiffs is not running yet and the app partitions are purposly enlarged to fit BT and WIFI it is better to do it like that ;)

Otherwise you are correct

also giving the 0x99 SubType value a name would be helpful

What shall I do?

As @igrr say, the address change will cause some problems in the Unofficial SPIFFS library

Else we should cause problems with bigger sketches?

…sp32 into EEprom_library Correction to eeprom_write.ino

pbecchi · 2017-07-27T14:35:30Z

I have corrected the example eeprom_write.ino

pbecchi · 2017-07-27T15:11:49Z

Why not on upper 4k of nvs partition? So that everybody is happy.

me-no-dev · 2017-07-27T16:11:19Z

Less NVS?

pbecchi · 2017-07-27T16:52:58Z

4k less!

pbecchi · 2017-07-27T16:54:59Z

Is this acceptable?

copercini · 2017-07-27T16:59:54Z

As the @me-no-dev is irreducible, SPIFFS library is quite new and still have few downloads, so this will not be a bigggg problem too =)

igrr · 2017-07-28T08:24:25Z

It is not advised to change the size of NVS partition, as part of its data will be lost, leaving WiFi configuration in a potentially semi-defined state. I would say that shrinking sketch space by 4k would be acceptable, but in the end it is @me-no-dev's call. If SPIFFS library is not widely deployed yet, shrinking SPIFFS is also an option.

pbecchi · 2017-07-28T09:45:05Z

All modification requested has been done!
Regarding EEprom partition I didn't make any change , you can change it as you like.
It is now overlapping and shirking the SPIFFS partition.

lonerzzz · 2017-07-29T09:50:17Z

@copercini, @igrr, what prevents this branch from being merged at this point and do we have a core group responsible for doing such merges?

copercini · 2017-07-29T11:33:37Z

@lonerzzz I want to know too, because I also have some waiting PRs #504

igrr · 2017-07-31T04:14:41Z

@lonerzzz There is one change which still needs to be done (remove noInterrupts/interrupts) and then it is up to @me-no-dev to approve and merge this.

pbecchi · 2017-07-31T05:27:53Z

Last correction done.

me-no-dev · 2017-07-31T19:56:22Z

libraries/EEPROM/EEPROM.cpp

+
+  if (esp_partition_erase_range(_mypart, 0, SPI_FLASH_SEC_SIZE) != ESP_OK)
+  {	  
+     ESP_LOGE(TAG, "partition erase err.");


please use Arduino's log facility :) log_e("partition erase err.");

pbecchi · 2017-08-01T09:47:40Z

corrected!

me-no-dev · 2017-08-01T09:51:49Z

Done! :)

pbecchi · 2017-08-01T10:37:31Z

Thanks!

* EEPROM_library_ported_Partition_table_updated * Cleanup & formal corrections * Make formatting of file consistent. Readability change, Concern about the SubType being 0x99. * Code formatting cleanup * Code formatting cleanup * Code formatting cleanup * Remove commented out code * Restore dropped bracket * Update EEPROM.cpp * Format Corrections * deletedExample * Corrected example * Deleted interrupts/nointerrupts * Update EEPROM.cpp

EEPROM_library_ported_Partition_table_updated

2edcd25

Cleanup & formal corrections

deb4e13

everslick mentioned this pull request Jul 24, 2017

Porting of Esp8266 EEPROM library #525

Closed

lonerzzz added 6 commits July 27, 2017 00:28

Make formatting of file consistent.

c8d009c

Readability change, Concern about the SubType being 0x99.

Code formatting cleanup

6c5c00e

Code formatting cleanup

a27f139

Code formatting cleanup

6b2bbcb

Remove commented out code

09b91da

Restore dropped bracket

355e094

igrr reviewed Jul 27, 2017

View reviewed changes

pbecchi added 4 commits July 27, 2017 15:38

Update EEPROM.cpp

4f1f9e3

Format Corrections

fd316c9

deletedExample

92345da

Merge branch 'EEprom_library' of https://github.com/pbecchi/arduino-e…

2d51387

…sp32 into EEprom_library Correction to eeprom_write.ino

Corrected example

a503574

Deleted interrupts/nointerrupts

c4e629e

me-no-dev reviewed Jul 31, 2017

View reviewed changes

Update EEPROM.cpp

6a8e0e0

me-no-dev merged commit 706bf48 into espressif:master Aug 1, 2017


		noInterrupts();

		if (esp_partition_erase_range(_mypart, 0, 4096) != ESP_OK)

EEPROM_library_ported_Partition_table_updated #535

EEPROM_library_ported_Partition_table_updated #535

Conversation

pbecchi commented Jul 23, 2017

everslick commented Jul 24, 2017

pbecchi commented Jul 24, 2017

everslick commented Jul 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

copercini Jul 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbecchi commented Jul 27, 2017

pbecchi commented Jul 27, 2017

me-no-dev commented Jul 27, 2017

pbecchi commented Jul 27, 2017

pbecchi commented Jul 27, 2017

copercini commented Jul 27, 2017 • edited Loading

igrr commented Jul 28, 2017

pbecchi commented Jul 28, 2017 • edited Loading

lonerzzz commented Jul 29, 2017

copercini commented Jul 29, 2017

igrr commented Jul 31, 2017

pbecchi commented Jul 31, 2017

me-no-dev Jul 31, 2017 • edited Loading

Choose a reason for hiding this comment

pbecchi commented Aug 1, 2017

me-no-dev commented Aug 1, 2017

pbecchi commented Aug 1, 2017

copercini Jul 27, 2017 •

edited

Loading

copercini commented Jul 27, 2017 •

edited

Loading

pbecchi commented Jul 28, 2017 •

edited

Loading

me-no-dev Jul 31, 2017 •

edited

Loading