kd5rxt-mark
Well-known member
casting will result in correct operations & results - not backing up is even better !
I have nailed down one culprit: the calculation of the EEPROM address whenever a "backup" is done is temporarily converted/promoted to a result with more bits than the "uint16_t" requires (to allow for rollover) as follows:
If/when "_adr_EEPROM" happens to be "0", the result of subtracting "1" is an intermediate value of "-1". Without immediately casting that intermediate result to "uint16_t", the results are not as expected. The following change to every "backup" calculation allows for correct results in all cases:
Now, the *REAL* question is thus: why backup at all ?? Why not just start writing after the last 0xFF that was found ?? In spite of the attempted "wear-leveling" actually resulting in "wear-inducing" (as pointed out by KurtE several times), isn't the implementation of "backing up" actually double-writing some locations unnecessarily ??
Getting rid of all "backup calculations" also allows for correct results in all cases, & is actually simpler as well. Go with KISS !!
Mark J Culross
KD5RXT
Here's the current test code:
I have nailed down one culprit: the calculation of the EEPROM address whenever a "backup" is done is temporarily converted/promoted to a result with more bits than the "uint16_t" requires (to allow for rollover) as follows:
Code:
_adr_EEPROM = (_adr_EEPROM - 1) % EEPROMsiz;
If/when "_adr_EEPROM" happens to be "0", the result of subtracting "1" is an intermediate value of "-1". Without immediately casting that intermediate result to "uint16_t", the results are not as expected. The following change to every "backup" calculation allows for correct results in all cases:
Code:
_adr_EEPROM = (uint16_t)(_adr_EEPROM - 1) % EEPROMsiz;
Now, the *REAL* question is thus: why backup at all ?? Why not just start writing after the last 0xFF that was found ?? In spite of the attempted "wear-leveling" actually resulting in "wear-inducing" (as pointed out by KurtE several times), isn't the implementation of "backing up" actually double-writing some locations unnecessarily ??
Getting rid of all "backup calculations" also allows for correct results in all cases, & is actually simpler as well. Go with KISS !!
Mark J Culross
KD5RXT
Here's the current test code:
Code:
#include <EEPROM.h>
#define EEPROMsiz 1024
#define DO_READ false
#define DO_WRITE true
class StringsTointEEPROM
{
public:
//constructor
StringsTointEEPROM();
// Methods
void format_eeprom();
int begin();
void save_string_to_eeprom(char *stringIn);
void save_nstrings_to_eeprom(int nbstring, char *stringsIn[]);
void print_strings_from_eeprom();
int get_string_from_eeprom(char *stringOut); //return 0 if MT
int get_nstrings_from_eeprom(int nbstring, char *stringsOut[]); //return 0 if MT
private:
int find_eeprom_address();
uint16_t _adr_EEPROM;
};
StringsTointEEPROM::StringsTointEEPROM()
{
}
void StringsTointEEPROM::format_eeprom()
{
for (int i = 0; i < EEPROMsiz; i++) EEPROM.write(i, 0xff);
}
int StringsTointEEPROM::begin()
{
return find_eeprom_address();
}
#define PERTINENT_DEBUG
void printDetails(uint16_t adrEEPROM, uint8_t dat, bool readWrite)
{
#ifdef PERTINENT_DEBUG
if (adrEEPROM == 65535)
{
#endif
if (readWrite) // false = DO_READ
{
Serial.print("WRITE: ");
} else {
Serial.print(" READ: ");
}
Serial.print(adrEEPROM / 1000);
Serial.print((adrEEPROM / 100) % 10);
Serial.print((adrEEPROM / 10) % 10);
Serial.print(adrEEPROM % 10);
Serial.print(" : ");
Serial.println(dat);
#ifdef PERTINENT_DEBUG
}
#endif
}
#define REMOVE_BACKUP
void StringsTointEEPROM::save_string_to_eeprom(char *stringIn)
{
// sync (0xaa)
// nchar(0..255)
// char[0]
// char[1]
// ..
// char[nchar-1]
// 0xff (end of latest string)
int i;
uint8_t len, val = 0;
while (val != 0xff)
{
val = EEPROM.read(_adr_EEPROM);
printDetails(_adr_EEPROM, val, DO_READ);
_adr_EEPROM = (_adr_EEPROM + 1) % EEPROMsiz;
}
#ifndef REMOVE_BACKUP
_adr_EEPROM = (uint16_t)(_adr_EEPROM - 1) % EEPROMsiz;
#endif
EEPROM.write(_adr_EEPROM, 0xaa); //write sync
printDetails(_adr_EEPROM, 0xaa, DO_WRITE);
_adr_EEPROM = (_adr_EEPROM + 1) % EEPROMsiz;
len = strlen(stringIn);
EEPROM.write(_adr_EEPROM, len); //write nchar
printDetails(_adr_EEPROM, len, DO_WRITE);
_adr_EEPROM = (_adr_EEPROM + 1) % EEPROMsiz;
for (i = 0; i < len; i++)
{
EEPROM.write(_adr_EEPROM, stringIn[i]);//write string
printDetails(_adr_EEPROM, stringIn[i], DO_WRITE);
_adr_EEPROM = (_adr_EEPROM + 1) % EEPROMsiz;
}
EEPROM.write(_adr_EEPROM, 0xff); //write end of string
printDetails(_adr_EEPROM, 0xf, DO_WRITE);
}
void StringsTointEEPROM::save_nstrings_to_eeprom(int nbstring, char *stringsIn[])
{
// sync (0xaa)
// nchar(0..255)
// char[0]
// char[1]
// ..
// char[nchar-1]
// 0xff (end of latest string)
int i, iter = 0;
uint8_t len, val = 0;
while (val != 0xff)
{
val = EEPROM.read(_adr_EEPROM);
printDetails(_adr_EEPROM, val, DO_READ);
_adr_EEPROM = (_adr_EEPROM + 1) % EEPROMsiz;
}
#ifndef REMOVE_BACKUP
_adr_EEPROM = (uint16_t)(_adr_EEPROM - 1) % EEPROMsiz;
#endif
while (nbstring > 0)
{
EEPROM.write(_adr_EEPROM, 0xaa); //write sync
printDetails(_adr_EEPROM, 0xaa, DO_WRITE);
_adr_EEPROM = (_adr_EEPROM + 1) % EEPROMsiz;
len = strlen(stringsIn[iter]);
EEPROM.write(_adr_EEPROM, len); //write nchar
printDetails(_adr_EEPROM, len, DO_WRITE);
_adr_EEPROM = (_adr_EEPROM + 1) % EEPROMsiz;
for (i = 0; i < len; i++)
{
EEPROM.write(_adr_EEPROM, stringsIn[iter][i]);//write string
printDetails(_adr_EEPROM, stringsIn[iter][i], DO_WRITE);
_adr_EEPROM = (_adr_EEPROM + 1) % EEPROMsiz;
}
nbstring--;
iter++;
}
EEPROM.write(_adr_EEPROM, 0xff); //write end of string
printDetails(_adr_EEPROM, 0xff, DO_WRITE);
}
void StringsTointEEPROM::print_strings_from_eeprom()
{
int i, cntr = 0;
uint8_t len, val = 0;
uint16_t adr_EEPROM = _adr_EEPROM;
while (val != 0xff) //find end of latest string
{
val = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, val, DO_READ);
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
}
while (1) //display all strings that were stored in EEPROM in FIFO order
{
val = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, val, DO_READ);
while (val != 0xaa) //find sync (could be long if EEPROM not full)
{
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
val = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, val, DO_READ);
cntr++;
if (cntr == EEPROMsiz)
{
Serial.println("EEPROM is MT");
break;
}
}//while (val!=0xaa)
if (cntr == EEPROMsiz) break;
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
len = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, len, DO_READ);
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
for (i = 0; i < len; i++)
{
Serial.print((char)EEPROM.read(adr_EEPROM));
printDetails(_adr_EEPROM, (char)EEPROM.read(adr_EEPROM), DO_READ);
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
}//for(i = 0; i < len; i++)
Serial.println();
val = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, val, DO_READ);
if (val == 0xff) break; //break if EEPROM not full or 1 full circle
else val = 0;
}//while(1)
}
int StringsTointEEPROM::get_string_from_eeprom(char *stringOut)
{
//get latest string from eeprom
int i, cntr = 0;
uint8_t len, val = 0;
uint16_t adr_EEPROM = _adr_EEPROM;
while (val != 0xff) //find end of latest string
{
val = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, val, DO_READ);
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
}
#ifndef REMOVE_BACKUP
adr_EEPROM = (uint16_t)(adr_EEPROM - 1) % EEPROMsiz;
#endif
while (val != 0xaa) //find sync
{
val = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, val, DO_READ);
#ifndef REMOVE_BACKUP
adr_EEPROM = (uint16_t)(adr_EEPROM - 1) % EEPROMsiz; //back up
#endif
cntr++;
if (cntr == EEPROMsiz) break; //EEPROM is MT
}//while (val!=0xaa)
if (cntr == EEPROMsiz) return 0;
adr_EEPROM = (adr_EEPROM + 2) % EEPROMsiz; //get to len
len = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, len, DO_READ);
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
for (i = 0; i < len; i++)
{
stringOut[i] = (char)EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, stringOut[i], DO_READ);
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
}//for(i = 1; i < len; i++)
stringOut[len] = '\0';
return 1;
}
int StringsTointEEPROM::get_nstrings_from_eeprom(int nbstring, char *stringsOut[])
{
int i, cntr = 0, stringno;
uint8_t len, val = 0;
uint16_t adr_EEPROM = _adr_EEPROM;
while (val != 0xff) //find end of latest string
{
val = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, val, DO_READ);
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
}
#ifndef REMOVE_BACKUP
adr_EEPROM = (uint16_t)(adr_EEPROM - 1) % EEPROMsiz;
#endif
for (stringno = 0; stringno < nbstring; stringno++)
{
while (val != 0xaa) //find sync
{
val = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, val, DO_READ);
#ifndef REMOVE_BACKUP
adr_EEPROM = (uint16_t)(adr_EEPROM - 1) % EEPROMsiz; //back up
#endif
cntr++;
if (cntr == EEPROMsiz) break; //EEPROM is MT
}//while (val!=0xaa)
val = 0;
if (cntr == EEPROMsiz) return 0;
}//for (stringno=0;stringno<laststring+1;stringno++)
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz; //get to len
for (stringno = 0; stringno < nbstring; stringno++)
{
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz; //get to len
len = EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, val, DO_READ);
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
for (i = 0; i < len; i++)
{
stringsOut[stringno][i] = (char)EEPROM.read(adr_EEPROM);
printDetails(adr_EEPROM, stringsOut[stringno][i], DO_READ);
adr_EEPROM = (adr_EEPROM + 1) % EEPROMsiz;
}//for(i = 0; i < len; i++)
stringsOut[stringno][len] = '\0';
}//for (stringno=0;stringno<nbstring;stringno++)
return 1;
}
int StringsTointEEPROM::find_eeprom_address()
{
uint8_t val = 0;
int cntr = 0;
_adr_EEPROM = 0;
while (val != 0xff)
{
val = EEPROM.read(_adr_EEPROM);
printDetails(_adr_EEPROM, val, DO_READ);
_adr_EEPROM = (_adr_EEPROM + 1) % EEPROMsiz;
cntr++;
if (cntr == EEPROMsiz) return 0;
}
#ifndef REMOVE_BACKUP
_adr_EEPROM = (uint16_t)(_adr_EEPROM - 1) % EEPROMsiz;
#endif
return 1;
}
#if defined(ARDUINO_TEENSY40)
#define FLASH_BASEADDR 0x601F0000
#define FLASH_SECTORS 15
#elif defined(ARDUINO_TEENSY41)
#define FLASH_BASEADDR 0x607C0000
#define FLASH_SECTORS 63
#elif defined(ARDUINO_TEENSY_MICROMOD)
#define FLASH_BASEADDR 0x60FC0000
#define FLASH_SECTORS 63
#endif
extern "C" void eepromemu_flash_erase_sector(void *addr);
bool flash_sector_has_data(uint32_t addr) {
const uint32_t *p = (const uint32_t *)addr;
for (unsigned int i = 0; i < 4096; i += 4) {
if (*p++ != 0xFFFFFFFF) return true;
}
return false;
}
void wipe_eeprom() {
#if defined(FLASH_BASEADDR) && defined(FLASH_SECTORS)
Serial.print("Wiping EEPROM");
uint32_t addr = FLASH_BASEADDR;
for (unsigned int i = 0; i < FLASH_SECTORS; i++) {
Serial.print(".");
if (flash_sector_has_data(addr)) {
eepromemu_flash_erase_sector((void *)addr);
}
addr += 4096;
}
Serial.println();
#else
for ( unsigned int i = 0 ; i < EEPROM.length() ; i++ ) {
EEPROM.write(i, 0xFF);
}
#endif
}
#define TABLESIZ 6
StringsTointEEPROM inteeprom; //internal Arduino EEPROM Library for storing and retreiving c-strings
char eaudgc[10] = "25";
char eaucm[10] = "15";
char eautime[10] = "(05:30)";
char airdgc[10] = "18";
char airhum[10] = "33";
char airtime[10] = "(06:30)";
char *tableadr[] = {eaudgc, eaucm, eautime, airdgc, airhum, airtime};
unsigned long int millistart;
unsigned long milliduration;
// STRUCTURE OF A STORED STRING
//-----------------------------
// sync (0xaa)
// nchar(0..255)
// char[0]
// char[1]
// ..
// char[nchar-1]
// 0xff (end of latest string)
//-----------------------------
void setup() {
int i, iter, nbytes = 0;
Serial.begin(115200);
while (!Serial) ; // wait for Arduino Serial Monitor
delay(10);
wipe_eeprom(); // for consistent test result
//inteeprom.format_eeprom(); //needs to be formatted once
if (inteeprom.begin() == 0) Serial.println("begin() ERROR, Format the EEPROM");
for (i = 0; i < TABLESIZ; i++) nbytes += (strlen(tableadr[i]));
millistart = millis();
inteeprom.save_nstrings_to_eeprom(TABLESIZ, tableadr);
milliduration = millis() - millistart;
Serial.print("execution time for save_nstrings_to_eeprom() is ");
Serial.print((float)milliduration / (float)nbytes);
Serial.println("ms per byte");
Serial.println("latest 6 strings read from EEPROM");
millistart = millis();
if (inteeprom.get_nstrings_from_eeprom(TABLESIZ, tableadr) == 0) Serial.println("EEPROM is MT"); //read string
milliduration = millis() - millistart;
Serial.println(eaudgc);
Serial.println(eaucm);
Serial.println(eautime);
Serial.println(airdgc);
Serial.println(airhum);
Serial.println(airtime);
Serial.print("total execution time for get_nstrings_from_eeprom() is ");
Serial.print(milliduration);
Serial.println("ms");
inteeprom.save_nstrings_to_eeprom(TABLESIZ, tableadr);
for (iter = 0; iter < 500; iter++)
{
inteeprom.get_nstrings_from_eeprom(TABLESIZ, tableadr);
Serial.print("iter: ");
Serial.print(iter);
Serial.print(" eaudgc: ");
Serial.println(eaudgc);
delay(1);
if (strcmp(eaudgc, "25") == 0) strcpy(eaudgc, "1"); else strcpy(eaudgc, "25"); //modify 1st string and test reliability
inteeprom.save_nstrings_to_eeprom(TABLESIZ, tableadr);
}
}
void loop() {
}