Marzogh / SPIMemory

Arduino library for Flash Memory Chips (SPI based only). Formerly SPIFlash
http://spimemory.readthedocs.io/en/latest/
GNU General Public License v3.0
430 stars 136 forks source link

Bug Report : Arduine freezes when looping on readWriteString.ino example sketch #104

Closed thedub2001 closed 5 years ago

thedub2001 commented 6 years ago

GitHub issue state GitHub issue title GitHub issue author GitHub issue label GitHub issue comments GitHub issue age GitHub issue last update

---------------------------- DO NOT DELETE OR EDIT anything above this line ----------------------------

Hello!

I'm submitting a bug report, I tried to identify what's going wrong but I cannot fix it...

Bug report :

Library version : 3.0.0 Branch : master Arduino IDE : 1.8.4 OS : Windows Microcontroller : Arduino-nano (Atmega328P) Flash memory module : W25Q64 freshly erased Current behaviour : I edited the example sketch readWriteString in order to repeat the Write/Read/Erase functions of the sketch but after 3 iterations the Arduino nano freezes. Here is the serial output :

Written string: This is a test String
To address: 7025326
Read string: This is a test String
From address: 7025326
Written string: This is a test String
To address: 4996536
Read string: This is a test Str
To address: 4996536

Expected behaviour : The Arduino should loop until the for condition is reached

Code snippet that reproduces the bug (edited readWriteString example sketch) :

#include<SPIFlash.h>

uint32_t strAddr;

#if defined(ARDUINO_SAMD_ZERO) && defined(SERIAL_PORT_USBVIRTUAL)
// Required for Serial on Zero based boards
#define Serial SERIAL_PORT_USBVIRTUAL
#endif

#if defined (SIMBLEE)
#define BAUD_RATE 250000
#define RANDPIN 1
#else
#define BAUD_RATE 115200
#define RANDPIN A0
#endif

//SPIFlash flash(SS1, &SPI1);       //Use this constructor if using an SPI bus other than the default SPI. Only works with chips with more than one hardware SPI bus
SPIFlash flash;

bool readSerialStr(String &inputStr);

void setup() {
  Serial.begin(BAUD_RATE);
#if defined (ARDUINO_SAMD_ZERO) || (__AVR_ATmega32U4__)
  while (!Serial) ; // Wait for Serial monitor to open
#endif

  flash.begin();
  randomSeed(analogRead(RANDPIN));

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

      strAddr = random(0, flash.getCapacity());
      String inputString = "This is a test String";
      flash.writeStr(strAddr, inputString);
      Serial.print(F("Written string: "));
      Serial.println(inputString);
      Serial.print(F("To address: "));
      Serial.println(strAddr);
      String outputString = "";
      if (flash.readStr(strAddr, outputString)) {
        Serial.print(F("Read string: "));
        Serial.println(outputString);
        Serial.print(F("From address: "));
        Serial.println(strAddr);
      }
      while (!flash.eraseSector(strAddr));

      delay(1000);
    }  
}

void loop() {

}

//Reads a string from Serial
bool readSerialStr(String &inputStr) {
  if (!Serial)
    Serial.begin(115200);
  while (Serial.available()) {
    inputStr = Serial.readStringUntil('\n');
    Serial.println(inputStr);
    return true;
  }
  return false;
}

Other information :


- I moved the Write/Read/Erase functions into the main "loop" function and the problem is the same
- I uncommented ```#define RUNDIAGNOSTIC``` in SPIFlash.h with no effect
- I have better result by passing ```outputString``` and ```inputString``` as global variables but there is still unexpected behaviour. Here is the serial output with unprintable characters :
![serialoutput](https://user-images.githubusercontent.com/18016853/33804497-9bb3ed8c-dda6-11e7-867f-016f39973d73.png)
![serialoutput_2](https://user-images.githubusercontent.com/18016853/33804509-ce9cbbc0-dda6-11e7-9d1c-fd048bc63a26.png)
_The Arduino reboots at each "strange" line_

I must be sure that the library is reliable before using it in projects so any help is welcome ! 😀 
Marzogh commented 6 years ago

From what I can see, this is a memory issue. The Arduino platform does not play well with String class objects and it is known to chew through memory like there is no tomorrow. I have been able to replicate the same result on my ATMega328 Arduino Pro board - it seems to run out of memory faster than you can say "String loops make me run out of memory" 😛

However, if you run it on a Feather M0 - with the ATSAMD21 ARM Cortex M0+ MCU on it - your code manages to loop ~230 times before freezing. My serial output is here.

If however you modify the code to this:

#include<SPIFlash.h>

#if defined(ARDUINO_SAMD_ZERO) && defined(SERIAL_PORT_USBVIRTUAL)
// Required for Serial on Zero based boards
#define Serial SERIAL_PORT_USBVIRTUAL
#endif

#if defined (SIMBLEE)
#define BAUD_RATE 250000
#define RANDPIN 1
#else
#define BAUD_RATE 115200
#define RANDPIN A0
#endif

#define ARRAYSZ 26

//SPIFlash flash(SS1, &SPI1);       //Use this constructor if using an SPI bus other than the default SPI. Only works with chips with more than one hardware SPI bus
SPIFlash flash;

bool readSerialStr(String &inputStr);

void setup() {
  Serial.begin(BAUD_RATE);
#if defined (ARDUINO_SAMD_ZERO) || (__AVR_ATmega32U4__)
  while (!Serial) ; // Wait for Serial monitor to open
#endif

  flash.begin();
  randomSeed(analogRead(RANDPIN));
  uint32_t _capacity_ = flash.getCapacity();
  uint32_t arrayAddr;
  char inputArray[ARRAYSZ] = "This is a test char array";
  char outputArray[ARRAYSZ];

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

    arrayAddr = random(0, _capacity_);
    if (flash.writeCharArray(arrayAddr, inputArray, ARRAYSZ)) {
      Serial.print("Written: ");
      for (uint8_t i = 0; i < ARRAYSZ; i++) {
        Serial.print(inputArray[i]);
      }
      Serial.println();
    }
    if (flash.readCharArray(arrayAddr, outputArray, ARRAYSZ)) {
      Serial.print("Read: ");
      for (uint8_t i = 0; i < ARRAYSZ; i++) {
        Serial.print(outputArray[i]);
      }
      Serial.println();
      Serial.print("Test ");
      Serial.print(i);
      Serial.println(" SUCCESSFUL");
    }
    else {
      Serial.print("Test ");
      Serial.print(i);
      Serial.println(" FAILED");
    }
    Serial.println();
    while (!flash.eraseSector(arrayAddr));
  }
}

void loop() {

}

then, it runs effortlessly to completion (no delay(1000); needed - the code runs at full speed) 😁

In conclusion, I'm afraid the issue you have run into is a combination of the limitations of the µC and the String class object filling up memory quicker than any other type of data.

Marzogh commented 6 years ago

Also, @hanyazou picked up a serious bug (#102) in the v3.0.0 code - related to writing arrays - and I've just fixed it. I will be rolling out the bugfix release v3.0.1 in a few minutes. If it is not available on the Arduino library manager, I'd recommend pulling the latest code from the stable branch and using that instead of v3.0.0. 🙂

thedub2001 commented 6 years ago

I use String class in programs. Even if the use is limited by bad memory usage, it's functional... When I comment the readStr() function or when I replace it by a readByte() function the code above runs normally. I think the problem here is in the _read() function in SPIFlash.h. I presume the String class has memory issue with the cast at line 3 or the *p++ at line 14 :

template <class T> bool SPIFlash::_read(uint32_t _addr, T& value, uint32_t _sz, bool fastRead) {
  if (_prep(READDATA, _addr, _sz)) {
    uint8_t* p = ((uint8_t*)(void*)&value);

    CHIP_SELECT
    if (fastRead) {
      _nextByte(WRITE, FASTREAD);
    }
    else {
      _nextByte(WRITE, READDATA);
    }
    _transferAddress();
    for (uint16_t i = 0; i < _sz; i++) {
      *p++ =_nextByte(READ);
    }
    _endSPI();
    return true;
  }
  else {
    return false;
  }
}
Marzogh commented 6 years ago

Fair point. I was quite pleased when the new ‘universal’ _read() and _write() functions worked so unexpectedly well. Now it looks like they are not a robust as the old ones.

I’ll take a go at fixing this and #106 later today and hopefully have them sorted ASAP.

thedub2001 commented 6 years ago

Great 😃 As I was intrigued by memory usage of String and flash stored strings, I made some test!

Memory usage test with flash stored strings F(String) and String type

Test maximum string length before a simple program becomes unreliable

Looks like issues appear if Arduino free memory is within 150. In comparison the "edited readWriteString example sketch" had 1530 free memory when freezing.

Code snippet

void setup() {
  Serial.begin(115200);
}

void loop() {
  String testString =  "012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345";
  Serial.print("testString =");
  Serial.println(testString);
  Serial.print("freeMemory()=");
  Serial.println(freeRam());
  delay(10);
}

int freeRam () {
  extern int __heap_start, *__brkval;
  int v;
  return (int) &v - (__brkval == 0 ? (int) &__heap_start : (int) __brkval);
}