esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
15.96k stars 13.34k forks source link

Missing EEPROM.length() method #2066

Closed ricardojlrufino closed 6 years ago

ricardojlrufino commented 8 years ago

Hardware

Hardware: ESP-12f Core Version: 2.2.0

Description

The EEPROM.length () method is not available for the ESP Ref: https://www.arduino.cc/en/Tutorial/EEPROMClear

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

MacroYau commented 8 years ago

The current EEPROM Library implementation for ESP8266 requires you to initialize the size in the EEPROM.begin() function as you may notice in the example sketch.

@igrr I am afraid that the change is not as straightforward as adding the EEPROM.length() function. It seems that the function was introduced as a part of the EEPROM Library V2.0, which is largely different from our current design.

ricardojlrufino commented 8 years ago

Why the function begin (4096), creates a buffer of the same size?

In my view the EEPROM lib would be used to save RAM too, does not make sense I create a buffer of the same size.

I think what could be done is to have a smaller buffer when full, write the data in the EEPROM.

In my implementation I need to store a list of rfid tags , then I plan to allocate the maximum size: 4096 in this case I would finish with two buffers = 2 * 4096

It would be viable to improve this design?

pgScorpio commented 8 years ago

I think the EEPROMClass::begin should also have a version like EEPROMClass::begin(startaddress,size) which would create a buffer of 'size' bytes and would apply 'startaddress' as an offset between addresses and the buffer index.

After that you can only read/write addresses between startaddress and (startaddress+size-1) write should return a bool, false if the write is not within the buffer range. read should return 0 if the read is not within the buffer range.

This way you can have a very large EEPROM, but never allocate more memory than needed for the current read/write actions.

PS: There should be functions to retrieve the size of the EEPROM and the current min- and max- read/write addresses (-1 if not between begin and end() ?)

PS 2: And EEPROMClass::end() should return the result of commit();

DouglasBair commented 8 years ago

Hello, I was about to enter the missing function/method of EEPROM.length when I noticed theis issue. It was in the Arduino.cc's EEPROM library up to and including 1.6.9, then in Arduino.org's IDE, it is missing. No regression testing, no coordination. I've been testing to see what all works( or doesn't) in the different versions of the IDE. I hope someone at Arduino.ORG will fix and TEST their software for backward compatibility. July 17,2016

igrr commented 8 years ago

Excuse me, what does this repository have to do with arduino.org?

DouglasBair commented 8 years ago

Maybe I am wrong, but I thought you had a problem with missing a function of 'EEPROM.length'. I use it in the Arduino.cc's IDE and it went missing in the version 1.7.0. Are you using the IDE provided by Arduino? Ignore my comments if they don't help you. Doug


From: Ivan Grokhotkov [mailto:notifications@github.com] Sent: Sunday, July 17, 2016 11:07 AM To: esp8266/Arduino Cc: DouglasBair; Comment Subject: Re: [esp8266/Arduino] Missing EEPROM.length() method (#2066)

Excuse me, what does this repository have to do with arduino.org?

You are receiving this because you commented. Reply to this email directly, view https://github.com/esp8266/Arduino/issues/2066#issuecomment-233192189 it on GitHub, or mute https://github.com/notifications/unsubscribe-auth/ARmIyPBWqSqOD__QilxTMykB8 KdS5OZEks5qWmElgaJpZM4IpSkm the thread. https://github.com/notifications/beacon/ARmIyEhSjJj9hzrbVKdQP2OOBrK_Kg73ks5 qWmElgaJpZM4IpSkm.gif


No virus found in this message. Checked by AVG - www.avg.com Version: 2016.0.7640 / Virus Database: 4627/12632 - Release Date: 07/17/16

igrr commented 8 years ago

In Arduino, EEPROM library is normally part of the core and not a standalone library. So there is one version for AVR core, one for ARM, one for ESP8266, etc. All these versions of EEPROM library are derived from the original AVR version as seen in Arduino.cc project. However as you may have noticed that original library (Arduino.cc one) has recently been updated with some new methods. The libraries which were derived from it may not have been updated by their maintainers yet to match the new API of Arduino.cc's library. This is the case here (we haven't updated our library to use the new API) and also the case with Arduino.org's AVR library (they haven't updated their library either). So this doesn't really have anything to do with regression testing and backwards compatibility. Arduino.cc's libraries are evolving and everyone has to catch up :)

middelink commented 8 years ago

Some fly-by commenting and 2 observations.

1) can we steer away from using begin()/end() for things different than C++'s iterators? EEPROM looks like a prime offender here, it would have been awesome to iterate over the content of the EEPROM for some operations, like this alomost one-liner to clear the entire eeprom.

for (auto &data : EEPROM) { data = 0x00 }

2) Caching concerns should be either made explicitly if they introduce large chunks of RAM usage or implicit if it can be done "invisible". For example, SPI_FLASH_SEC_SIZE(4k) should not really a concern of the user of the class since they only want to r/w some dumb data. However for advanced users, who need to squeeze every byte out of the chip, it can be good to just alloc enough cache for their little struct. Or ... in fact, don't want any cache, as they know their data is only gotten once, directly into their data structs.

devyte commented 6 years ago

@ricardojlrufino I can add the length method without problems. As for the size of the buffer, it is meant for reducing wear on the underlying flash, which is also why commit() exists. This is due to the inherent difference in hardware between AVR and ESP: the ESP doesn't have EEPROM, it is just emulated on flash. I understand your concern about the buffer size, but at this time there is no plan to change the underlying implementation. If you think memory reduction is a must-have, I invite you to make a PR with a proposal.

devyte commented 6 years ago

@pgScorpio about EEPROMClass::begin(startaddress,size), that is unnecessary. If you need different addresses, just instantiate a diffrent EEPROM object, with a different address passed to the constructor:

uint32_t sector = flashsectoryouwanttouse;
EEPROMClass myEEPROM(sector);
myEEPROM.begin(4096);
devyte commented 6 years ago

@middelink about begin() / end(), that's a BigDiscussion topic related to Arduino bad design. In fact, even their current EEPROM v2 is not well designed, they implemented additional classes to try and sort-of mimic C++ iterators. Right now, what I can do is get closer to the v2 interface by adding length() and operator[](), but there isn't anything I can do about begin() / end(), because it would break compatibility, and who knows how many sketches out there would stop building. I suppose an entirely new v2 class could be implemented, and it could even incorporate what you explain about cache enable/disable. You could then choose which version of EEPROM to use. However, there is no plan for that at this time. As I explained elsewhere, if you think this is a must-have, I invite you to make a PR with a proposal.

ricardojlrufino commented 6 years ago

@devyte The main point, it's make API compatible

devyte commented 6 years ago

Closed via #3855 .