cmaglie / FlashStorage

A convenient way to store data into Flash memory on the ATSAMD21 and ATSAMD51 processor family
202 stars 69 forks source link

FlashAsEEROM weird issues #44

Open tuxedo0801 opened 4 years ago

tuxedo0801 commented 4 years ago

We have some issues with the EEPROM lib ...

The effects we see in a more complex program:

We write >1,5k of data and finally do a commit(). This gives no error, but it seems that after next device startup not all the data is there.

I tried to narrow down the issue to a small testable example:

#define EEPROM_EMULATION_SIZE 2048
#include <FlashAsEEPROM.h>

void setup() {
  SerialUSB.begin(115200);

  while (!SerialUSB) {
  }

  byte firstByte = EEPROM.read(0x0000);

  // if EEPROM is "empty" ...
  if (firstByte == 0xFF) {

    // write repeatedly 0x00...0xFF to "EEPROM"
    SerialUSB.println("write to EEPROM ...");

    byte v = 0x00;

    for (int i = 0; i < EEPROM_EMULATION_SIZE; i++) {

      SerialUSB.print("Writing: #");
      SerialUSB.print(i);
      SerialUSB.print(" -> ");
      SerialUSB.println(v);
      EEPROM.write(i, v);

      v++;

    }

    SerialUSB.println("  Doing commit ...");
    EEPROM.commit();
    SerialUSB.println("  Doing commit ...*DONE*");

    SerialUSB.println("Writing *DONE*");
    SerialUSB.println("Do manual reboot now ...");

  }
  else
  {

    // Verify that EEPROM contains repeatedly 0x00...0xFF 
    SerialUSB.println("Read from EEPROM ...");
    byte v = 0x00;
    for (int i = 0; i < EEPROM_EMULATION_SIZE; i++) {

      byte b = EEPROM.read(i);
      SerialUSB.print("Read: #");
      SerialUSB.print(i);
      SerialUSB.print(" -> ");
      SerialUSB.print(b);
      SerialUSB.print(" vs. ");
      SerialUSB.println(v);

      if (b!=v) {
        SerialUSB.println("WRONG!!!!");  
        while(true);
      }

      v++;

    }
    SerialUSB.println("All fine!!!");  

  }
}

void loop() {
  // nothing to do here
}

The idea: On the first startup, fill the entire EEPROM with sequences of 0x00...0xFF Then you need to restart the device. Now it will compare the EEPROM content with the required 0x00..0xFF sequences.

Interestignly, I get a different, but also weird behavior:

If I set the EEROM emulation size to 1024, it works quite well.Writing and reading works as expected. All the data can be written und reading out return the same data.

On 2048 the "commit" after writing all the data is hanging forever.

And on 4096 or 8192 it struggles with writing to EEPROM (the for-loop is not finished): Somewhere around EEPROM index ~2200 it just stops writing.

Any ideas on what's wrong? I'm using a custom SAMD21 board...

best regards, Alex

tuxedo0801 commented 4 years ago

Same behavior with a commercial SAMD21 board: Elecrow Crowduino M0-SD v1.0

So it's not just my custom board :-(

Adminius commented 4 years ago

tested the same sketch... same problems... :/

Ing-Dom commented 4 years ago

I tested with a ItsyBitsy M0 Express Board. Arduino IDE 1.8.8 / FlashStorage 0.7.1 1024: All finde 2048: "Writing: #1623 -> 87" is last output, hangs 4096: "Writing: #1623 -> 87" is last output, hangs 8192: "Writing: #1623 -> 87" is last output, hangs

Arduino IDE 1.8.8 / FlashStorage 1.0.0 1024: All finde 8192: "Writing: #1623 -> 87" is last output, hangs

tuxedo0801 commented 4 years ago

Looks like the EEPROM_EMULATION_SIZE define from sketch is not seen in header/cpp file of library. Question is: Why?

Proof: Add #warning !!! Using default size 1024 !!! to FlashAsEEPROM.h when setting default size of no size is set.

Compiler will complain with this warning, even if you set the define in your sketch.

tuxedo0801 commented 4 years ago

As I'm not an export regarding preprocessing stuff, I did some research on this... Final result: One cannot influnce a #define that is set or used in a library/header file.

If thas is really the case, it makes no sense to specify the EEPROM size with #define ... I would rather take the approach ESP8266 did with their EEPROM library: https://arduino-esp8266.readthedocs.io/en/latest/libraries.html#eeprom --> create "begin(eepromSize)" method specify the size with an argument.

Are there any other ideas? @cmaglie? any?

If not, I would create an PR for this.

cmaglie commented 4 years ago

--> create "begin(eepromSize)" method specify the size with an argument.

I'm ok with your proposal, it should also solve #31 if you perform the allocation when begin is called.

tuxedo0801 commented 4 years ago

Now as I think I understood how the FlashStorage class is storing the eeprom data without getting in conflict with the sketch which also resides in flash:

if we now define the EEPROM size during runtime with "begin()" method, we cannot store the "eeprom" as a constant within the sketch (which would require compile time knowledge about eeprom/constant size. So we require to specify a flash memory address which does not get in conflict with bootloader or sketch.

Or am I thinking the wrong way? @cmaglie

If I'm right with my understanding: Where do we store the eeprom in flash? I would recommend the "end of the flash". This in general can cause conflicts if sketch is such big in size, that it conflicts with the eeprom at the end of the flash...

Is there a way we can detect during runtime the lenght of the sketch so that we can at least react on the possible conflict?

Adminius commented 4 years ago

Idea: we can reserve 8192 or so at the end of flash memory with a linker script. so this area will be defined as "eeprom" space and IDE will not override it #34 . Also no space conflicts with a sketch. But this requires approve from @cmaglie

Or may be dynamic size depends on flash size, similar to region lock sizes? image

cmaglie commented 4 years ago

Ok now that I've re-read the source code (...sorry I made it almost 6 years ago now!) I think you're right @tuxedo0801, for this to work we need to create the eeprom storage as const so it gets allocated in flash and aligned to 256 (or 8192 for SAMD51):

#define FlashStorage(name, T) \
  __attribute__((__aligned__(256))) \
  static const uint8_t PPCAT(_data,name)[(sizeof(T)+255)/256*256] = { }; \
  FlashStorageClass<T> name(PPCAT(_data,name));

FlashStorage(eeprom_storage, EEPROM_EMULATION); <-- this goes in flash

Where do we store the eeprom in flash? I would recommend the "end of the flash". This in general can cause conflicts if sketch is such big in size, that it conflicts with the eeprom at the end of the flash...

the "EEPROM" is allocated in the middle of the FLASH, there is no pre-defined location, it's the compiler that finds the first available spot, so it's impossible to have conflicts. The downside is that the "EEPROM" content is lost if a new sketch is uploaded to the board (because the flash is wiped out and the location will likely change). As the library name suggests: this is a Flash storage with an emulated EEPROM.

BTW, due to the way this library works, it also uses an equal amount of SRAM to back the EEPROM storage:

typedef struct {
  byte data[EEPROM_EMULATION_SIZE];
  boolean valid;
} EEPROM_EMULATION;

class EEPROMClass {
[...]
  private:
[...]
    EEPROM_EMULATION _eeprom;   <-- this is in SRAM!
 ;

In other words, even if we bump up the EEPROM_EMULATION_SIZE to 8192 we will consume:

The only way out that comes to my mind is to use the methods from the FlashClass:

class FlashClass {
public:
  FlashClass(const void *flash_addr = NULL, uint32_t size = 0);

  void write(const void *data) { write(flash_address, data, flash_size); }
  void erase()                 { erase(flash_address, flash_size);       }
  void read(void *data)        { read(flash_address, data, flash_size);  }

  void write(const volatile void *flash_ptr, const void *data, uint32_t size);
  void erase(const volatile void *flash_ptr, uint32_t size);
  void read(const volatile void *flash_ptr, void *data, uint32_t size);

private:
  void erase(const volatile void *flash_ptr);

  const uint32_t PAGE_SIZE, PAGES, MAX_FLASH, ROW_SIZE;
  const volatile void *flash_address;
  const uint32_t flash_size;
};

this class allows to write directly in the flash memory, but this requires a heavy rework of the EEPROM emulation class due to the way the FLASH memory works, the main problems are:

  1. you cannot write on already written parts of the flash: you must erase it first
  2. you cannot write or erase single bytes: the minimum amount is a "page" (or 256 contiguous bytes)
  3. because of 1. and 2. if you want to change a byte, you must first save the page containing the byte in RAM, erase the page, apply your change in RAM, write back the page to flash.
  4. every write in the Flash wears out the memory, after an amount of writes, the Flash will start to lose it's capability to retain memory. To reduce this problem we added the commit() method to the emulated EEPROM, so the RAM-backed content is dumped into flash only after doing a commit but, as already said, this has the downside that we need as much SRAM as the emulated EEPROM to keep track of the changes.
Adminius commented 4 years ago

@cmaglie, @tuxedo0801 wrote FlashAsEEPROM part of this library ;) You are right about RAM usage. But today's implementations doesn't give an easy way (directly from sketch) to change a EEPROM size. In my actual case I need about 4096 EEPROM and I have enough RAM free for it (24003 RAM free with 4096 EEPROM Array)

Another problem is "dynamic" address and full wipe with new FW upload:/ That's why we had an idea to "reserve" a "EEPROM" space via linker script at the and of flash memory. Since we have als lot of space (256k/512k) 8k doesn't play any role if it will permanently "stolen" ;)

cmaglie commented 4 years ago

@cmaglie, @tuxedo0801 wrote FlashAsEEPROM part of this library ;)

lol, I guess my flash memory is getting worn out then :-)

In my actual case I need about 4096 EEPROM and I have enough RAM free for it (24003 RAM free with 4096 EEPROM Array)

In that case another solution could be to allocate the EEPROM area in the sketch and pass the pointer to the EEPROM class, something like:

FlashStorage(my_eeprom_storage, 8192);

void setup() {
  EEPROM.setStorage(my_eeprom_storage, 8192);
  ...
}

(totally untested just sketching the idea...)

Another problem is "dynamic" address and full wipe with new FW upload:/ That's why we had an idea to "reserve" a "EEPROM" space via linker script at the and of flash memory. Since we have als lot of space (256k/512k) 8k doesn't play any role if it will permanently "stolen" ;)

Well, it's not said that changing the linker script may save the "EEPROM" part from being erased, we should see if bossac or openocd are smart enough to understand that they should not fully erase the memory but just the part thas is being programmed.

There may be also some lock bits that may help to protect some areas... but here we are navigating out from the "library" border and going into the "platform" area... these changes will require support from the cores, so every change we perform to the linker script should be made with the backward compatibility in mind.

thiagothimotti commented 3 years ago

I'm having similar problems.

After 11k: Do not run if power on with USB plugged(SAMD21 Native USB) After 12k: Do not run anymore in any case.

I'll give a try on your commit.

thiagothimotti commented 3 years ago

Solved my problems. SAMD21 running normal even with 12k Flash EEPROM.

But I got: Error 'class FlashClass' has no member named 'length' I need to edit FlashStorage.h to make this work.