RobTillaart/I2C_EEPROM

writeBlock across page boundaries intermittently failing

gednz opened this issue · 8 comments

gednz commented

Hi

I have a 12 byte struct of meter values that I store in a "circular buffer" on the eeprom which for testing is 60 records.
I am observing that intermittently when I write this struct across a page boundary only the values that fit on the first page are written.
Values being written within a page are 100% correct.
I have put a debug Serial.print in _WriteBlock and I can see it getting called twice for the writes over the boundaries as expected.

could this be some kind of chip not ready for the second write issue?

eeprom: 24LC256

struct
{
  unsigned long ts;
  unsigned long meter;
  unsigned long ticks;
} logRecord;

Regards
Ged

Thanks for opening this issue. I haven't seen it before so I need to investigate.

Which platform / board do you use?
Other devices on the I2C bus?
Do you have pull ups on the I2C bus?

There should be 5 millis between writes and the code tries to be on that edge. So you might change that time to 6 millis or so. (.cpp file)

To be continued.

@gednz

Some more questions:

  • how often will records be written in the EEPROM?
    • records per second / minute / day
  • how much records wil be needed in the final version ?
    • do you intent to use the whole EEPROM for the log?
  • is it an option to extend the record to16 bytes? then there would be no boundary write as pages are 64 bytes (IIRC)
    • add just a 4 byte filler
    • add a CRC32 (or XOR32 ) field for integrity check
  • If (run) time permits one could code a write_with_verify() function as a work around
    • this sounds like a good idea anyway to add to the library.

There should be 5 millis between writes and the code tries to be on that edge. So you might change that time to 6 millis or so. (.cpp file)

Search for the following line in the .cpp file and change 5000 => 6000

#define I2C_WRITEDELAY  5000
gednz commented

Hi @RobTillaart

Thanks very much for getting back so quickly and with so many ideas!

To answer some questions:

  • Which platform / board do you use? custom samd M0
  • Other devices on the I2C bus? no
  • Do you have pull ups on the I2C bus? yes 4k7
  • how often will records be written in the EEPROM? records per second / minute / day 1-15 minutes, new record created every 15 minutes but can be updated in between
  • how much records wil be needed in the final version ? about 3600 - I'm waiting on some 24LC512 to fit this
  • do you intent to use the whole EEPROM for the log? was trying to leave some space for config, only need 1K for that though
  • is it an option to extend the record to16 bytes? then there would be no boundary write as pages are 64 bytes (IIRC) My thoughts too! it might be the best course because I'm updating the latest record from inside an ISR at the moment so time is of the essence.

I tested this in my project code so its got a few other things going on around it.
I increased the I2C_WRITEDELAY 5000 to 6000 and later to 8000.
Interestingly the meter values all appear to be correct from what was uploaded to the MQTT server while there are definite failures on the ticks values. This could just be statistics hitting the ticks (3rd value that sits on the second page more often) more often than the meter (2nd value)

I do like your idea for a write_with_verify() function.

Thanks for the answers,
There is one that triggers - writing to EEPROM from an ISR.

ISR's typically should be made as short (fast) as possible. Writing to EEPROM (especially 2 pages) does not match this criteria. Two 2 pages take up at least 5 or 10 milliseconds (worst case) waiting + time for actual write => 5-12 millis (educated guess).

Can you set a flag in the ISR to indicate it is time to write data to EEPROM, and do the actual write in the main loop?

The write_with_verify() is easy to implement, difficult part is what strategy to use when it fails.
something like

bool write_with_verify(struct_log x)
{
  struct_log y; 
  eeprom.writeBlock(address, x);
  eeprom.readBlock(address, y);
  return memcmp(x,y);
}

Rethinking the case today I would

  • move the EEPROM writing outside the ISR
  • add a CRC to the struct. Check https://github.com/RobTillaart/CRC but a simple XOR would do
  • in case of verify fail I would
    • do one rewrite at most. (later you can see which records failed because of CRC failure)
    • or write the same record to the next address. (CRC would show this one is valid)

@gednz
Created a develop branch with (elementary non optimized) write with verify functions.
If you have time, could you give them a try?

update: merged the develop branch into master

gednz commented

Hi @RobTillaart

Thanks again for the help.
I have been testing the update and it is working great. Very handy to have that in the lib.

Your CRC lib looks great too, I will start testing it in my project.

Regarding the ISR, I was writing to the eeprom both in and outside of the ISR function and I was seeing failures (only) when writing across pages in both cases.
I have now removed the write from the ISR and so far have had no more failures at all...

SOLVED!!