A couple enhancements for your review...

[microfoundry]

Greetings Rob - thank you for the great library. I've added a couple features you might be interested in:

1. I utilize Write Control and wanted it enabled by default, but made it a compile time define for those who don't. Implemented in the begin function and if there's no WC pin supplied, the #define is irrelevant.

2. Modified the updateBlock function have the choice to perform a byte level compare and multiple updates as needed, or stick with the I2C_BUFFERSIZE compare. Also a minor update to the original block of code that would return the full length of the buffer as number of bytes changed, even if it were just a buffer chunk (assuming the supplied buffer was larger than a I2C_BUFFERSIZE)

Best regards,

Terry Phillips

BTW - I'm also working with the ST M24256-D series of EEPROM which includes an additional Lockable Identification Page. It was super easy to extend the library for the functionality. I have a separate codebase for that if you're interested...

[RobTillaart]

Thanks for thi PR. Will look into the details later, might take a few days due to other (prio) tasks.

Interested in the extended version, preferably as a derived class I think, as that gives people choice which class to use. Do you have a repository link?

[RobTillaart]

Relates to #68

[RobTillaart]

Got the review started.

1. I utilize Write Control and wanted it enabled by default, but made it a compile time define for those who don't. Implemented in the begin function and if there's no WC pin supplied, the #define is irrelevant.

OK I see the application and the implementation is quite straightforward, however the implementation is missing command line support. Adding an ifndef construct it would also allow to set this flag.

Furthermore the default value of the flag EN_AUTO_WRITE_PROTECT in the library must be 0 (false) to have backwards compatible behavior. E.g. if someone updates the library the _autoWriteProtect flag is "suddenly" true instead of false (current default).

in code:

I2C_EEPROM.h

//  set the flag EN_AUTO_WRITE_PROTECT to 1 to enable the Write Control at compile time 
//  used if the write_protect pin is explicitly set in the begin() function.
//  the flag can be set as command line option.
#ifndef EN_AUTO_WRITE_PROTECT
#define EN_AUTO_WRITE_PROTECT 0
#endif

---

Same command line argument would be true for the PER_BYTE_COMPARE flag. Haven't reviewed that code yet (only a quick glance). It are a lot of lines added and the added value is not 100% clear to me.

Remarks:

• does the default value true change the default behavior?
• what is the effect on the library size.
• bool getPerByteCompare() { return _perByteCompare }; could be added for convenience.

---

Point of attention: Need to keep this functionality in sync with - https://github.com/RobTillaart/I2C_24LC1025 after this PR.

To be continued.

[RobTillaart]

@microfoundry

BTW - I'm also working with the ST M24256-D series of EEPROM which includes an additional Lockable Identification Page. It was super easy to extend the library for the functionality. I have a separate codebase for that if you're interested...

Yes interested, do you have a link?

Read a bit about the LIP, it works like an (separate) EEPROM of 128 bytes (memAddress 0..127).

It would need these 5 functions in the derived class.

• void writeLIP(uint8_t memAddress, uint8_t value);
• uint8_t readLIP(uint8_t memAddress);
• void lock();
• void unlock();
• bool isLocked();

[microfoundry]

Hmm - a derived class might be more elegant than my current brute force modification of adding an additional bool to each function: bool IDPage (defaulted to =false). The separate IDPage i2c address is always the chosen EEPROM base address + 8 and set during the begin(...). And in the end:

_wire->beginTransmission(IDPage ? _idPageDeviceAddress : _deviceAddress);

But on the flip-side, I can manage both from one instantiation. Guess it's a coin-toss.

BTW, it's an irreversible lock!

You'll find it here: I2C_eeprom_wIDPage

Something else to note: to avoid address wrapping, I added a couple validation tests in the function _pageBlock which would apply to setBlock, writeBlock and updateBlock functions

  // Check if IDPage is true and length from the specified address is larger than the single ID page boundary
  if (IDPage && (addr + len > this->_pageSize)) {
    return 11; // Trying to crank it up to 11, and it will wrap when crossing page boundary
  }

  // Check if the entire write operation stays within the EEPROM's memory limits
  if (addr + len > this->_deviceSize) {
    return 12; // Error code 12 for attempting to write beyond the EEPROM's memory limits
  }

[RobTillaart]

@microfoundry

perByteCompare

Found some time to dive into the perByteCompare modus.

In the code you allocate two buffers that are potentially large, larger than I2C buffer.

• uint8_t origBuf[length];
• uint8_t writeBuf[length];

Note that you do not need the writeBuf as the data you want to write is already in buffer.

• reduces memory usage
• possibly slightly faster (no intermediate copy).

Furthermore the final remaining different bytes can be caught in the for loop too. Does save a few lines.

uint16_t I2C_eeprom::updateBlock(const uint16_t memoryAddress, const uint8_t * buffer, const uint16_t length)
{
  uint16_t addr = memoryAddress;
  uint16_t len = length;
  uint16_t rv = 0;

  if (_perByteCompare) {
    //  Serial.println("Performing BYTE SIZE updates");
    uint16_t writeCnt = 0;

    //  Read the original data block from the EEPROM.
    uint8_t origBuf[length];
    _ReadBlock(addr, origBuf, length);

    uint16_t diffCount = 0;
    uint16_t startDiffAddr = 0;

    //  Iterate over each byte to find differences.
    for (uint16_t i = 0; i < len; ++i) {
      if (buffer[i] != origBuf[i]) {
        //  Start buffering when the first difference is found.
        //  Save the starting address of the difference.
        if (diffCount == 0) {
          startDiffAddr = addr + i; 
        }
        diffCount++;  //  count the differing bytes.
        if (i < len - 1) continue;  //  <<<<<<<<<<<<<<  falls through when i == len -1.
      } 
      //  If there was a difference and now it stops, 
      //  write the buffered changes.
      if (diffCount > 0) {
        rv += diffCount;
        _pageBlock(startDiffAddr, &buffer[startDiffAddr], diffCount, true);
        diffCount = 0; //  Reset difference count after writing.
        writeCnt++;
      }
    }
    // Serial.print("EEPROM Write cycles: ");
    // Serial.println(writeCnt);

    return rv;
  } 

  // Serial.println("Performing BUFFERSIZE updates");
  while (len > 0)
  {
    uint8_t buf[I2C_BUFFERSIZE];
    uint8_t cnt = I2C_BUFFERSIZE;

    if (cnt > len) cnt = len;
    _ReadBlock(addr, buf, cnt);
    if (memcmp(buffer, buf, cnt) != 0)
    {
      rv   += cnt; // update rv to actual number of bytes written due to failed compare
      _pageBlock(addr, buffer, cnt, true);
    }
    addr   += cnt;
    buffer += cnt;
    len    -= cnt;
  }
  return rv;    
}

Did you run tests like:

• performance comparison of the BYTE and BUFFER modi?
• with length larger than the I2C_BUFFER? (will that work in byte mode?)
• an update of 256 bytes and all bytes changed?
• an update of 256 bytes and every other byte changed? (worst case).
• an update of 256 bytes and every 16th byte changed?

Can you post some figures?

[RobTillaart]

BTW, it's an irreversible lock!

You'll find it here: I2C_eeprom_wIDPage

Thanks,

If time permits I will dive into the code.

[microfoundry]

Hi Rob - thank you for the code recommendations, I've updated my stuff to follow.

As to your questions:

• with length larger than the I2C_BUFFER? (will that work in byte mode?) Yes, it functions as expected.
• performance comparison of the BYTE and BUFFER modi? I ran a series of single pass tests as per your outline. MCU is the ESP32-S3 WROOM2 w/32MB Octal Flash, 400KHz i2c, and the default 128byte buffer (more on this after the test...). Nothing else running on the MCU except the test code.

The byte size updates were randomly placed through the 256 byte test data, so in essence if there were 16 changes, there would be 16 individual write calls when in Byte Compare mode (conversely 2 updates in BUFFER mode). Certainly not surprising that the 128 modified bytes took an astounding 429934µs to complete. But hey, that's 128 individually addressed write cycles with an avg of 3358µs each... Note: the following output originates with the 256 byte test data being loaded, and then each test. The same base 256 bytes was reloaded between each test, but I eliminated the Serial.print() for those reloads to minimize the noise.

I think the best way to quantify the usefulness of the Byte Size Compare would be: The need to work with large buffers && (low quantity of random non-contiguous updates || the desire to minimize unnecessary writes / maximize individual memory cell life expectancy).

NOTE: published times in micros...

Writing base 256 bytes...
Write time: 15898

Setting perByteCompare = false
Performing BUFFERSIZE updates
*****************************
Updating every 16th byte...
Bytes Written with return size of: 256
Update time: 23874

Updating every 2nd byte...
Bytes Written with return size of: 256
Update time: 25004

Updating every byte...
Bytes Written with return size of: 256
Update time: 25127

Setting perByteCompare = true
Performing BYTESIZE updates
*****************************
Updating every 16th byte...
Bytes Modified: 16
Update time: 58126

Updating every 2nd byte...
Bytes Modified: 128
Update time: 429934

Updating every byte...
Bytes Modified: 256
Update time: 25275   // NOTE: This is essentially the same as 2 BUFFER writes

Updating 1 byte...
Bytes Modified: 1
Update time: 9551

Updating 2 bytes...
Bytes Modified: 2
Update time: 12861

Updating 3 bytes...
Bytes Modified: 3
Update time: 16172

Updating 4 bytes...
Bytes Modified: 4
Update time: 19483

Updating 5 bytes...
Bytes Modified: 5
Update time: 22606

Updating 6 bytes...
Bytes Modified: 6
Update time: 25836

Updating 7 bytes...
Bytes Modified: 7
Update time: 29344

Updating 8 bytes...
Bytes Modified: 8
Update time: 32718

Updating 9 bytes...
Bytes Modified: 9
Update time: 36054

Updating 10 bytes...
Bytes Modified: 10
Update time: 39350

With regards to the I2C_BUFFERSIZE : While working with the various test configurations, I cranked up the Wire.setBufferSize() and I2C_BUFFERSIZE define to 256 out of curiosity and discovered a bug in the base code. The initial test produced an error from the ESP32 i2cWriteReadNonstop() function in the Wire wrapper, which lead me to: All the public function accept a uint16_t length parameter value. Followed by: the private _WriteBlock, _ReadBlock and _VerifyBlock functions which only accept a uint8_t length parameter, producing some undesirable results. I updated the 3 functions to accept uint16_t params and eliminated the error.

Again, I appreciate your feedback. And I hope the additional information helpful.

-Terry

[RobTillaart]

Thanks for these Insightful figures. They confirm my gut feeling that depending on the number of bytes changed the byte mode is faster.

The tests should be ran on an UNO (MEGA) too, to see what behavior it shows. My expectation is that the AVR will show a larger spread of times.

---

The issue with buffers beyond 8 bit is known and so far it was not reported as a problem. However imho it is important (in fact a bug) for systems with more resources. So I will make a separate issue for it.

[RobTillaart]

@microfoundry Created a develop branch to fix #70, making the length parameter uint16_t . Also included the compile time option EN_AUTO_WRITE_PROTECT Further some edits for readability in .cpp file

---

I still need to test the compareByte code as I am not convinced of the added value. The idea is useful in some cases, no doubt, but is it useful enough. Also not clear how much extra memory it uses, and for smaller boards that is important. And how can the idea be optimized using the page size of the EEPROM used.

Question: can the library decide which mode is fastest? automatically?

[RobTillaart]

@microfoundry

Had a though about a variation in strategy (not tested) which might write non-changed bytes. Definitely not a "write only changed bytes" strategy however possibly interesting to consider.

If you have time, please share your opinion.

Reading is done in a number of blocks (Think length >> I2C_BUFFERSIZE) Within every block that is read, the segment with the changed bytes is detected (start and end position). Write only that part to EEPROM ==> might include non=changed bytes

uint16_t I2C_eeprom::updateBlock(const uint16_t memoryAddress, const uint8_t * buffer, const uint16_t length)
{
  uint16_t address = memoryAddress;
  uint16_t len = length;
  uint16_t bufSize = I2C_BUFFERSIZE;
  uint16_t writeCount = 0;

  if (_perByteCompare) {
    //  Serial.println("Performing BYTE SIZE updates");
    while (len > 0) {
      if (bufSize > len) bufSize = len;  //  final iteration.

      //  Read data block from the EEPROM.
      uint8_t buf[I2C_BUFFERSIZE];
      uint16_t start = 65535;
      uint16_t end   = 0;
      _ReadBlock(address, buf, bufSize);
      for (uint16_t i = 0; i < bufSize; ++i) {
        //  which part of the buffer should be written to EEPROM
        if (buffer[i] != buf[i]) {
          if (start == 65535) start = i;
          end = i;
        }
      }
      if (start != 65535) writeCount += _pageBlock(address + start, buffer + start, end - start + 1);
    }
    return writeCount ;
  }

  // Serial.println("Performing BUFFERSIZE updates");
  while (len > 0)
  {
    uint8_t buf[I2C_BUFFERSIZE];
    uint8_t bufSize = I2C_BUFFERSIZE;

    if (bufSize > len) bufSize = len;
    _ReadBlock(address, buf, bufSize);
    if (memcmp(buffer, buf, bufSize) != 0) {
      writeCount += bufSize; // update rv to actual number of bytes written due to failed compare
      _pageBlock(address, buffer, bufSize, true);
    }
    address += bufSize;
    buffer  += bufSize;
    len     -= bufSize;
  }
  return writeCount;    
}

Expect that on boards with a smaller I2C buffer (UNO, MEGA) , it will work quite well.

[RobTillaart]

Another function to give your opinion on. Almost the above without the write, in fact I wrote this function first as knowing the number of different bytes (or ratio diffCount vs length) might help to determine the strategy.

uint16_t I2C_eeprom::diffCount(const uint16_t memoryAddress, const uint8_t * buffer, const uint16_t length)
{
  uint16_t address = memoryAddress;
  uint16_t len = length;
  uint16_t bufSize = I2C_BUFFERSIZE;
  uint16_t count = 0;

  while (len > 0) {
    if (bufSize > len) bufSize = len;  //  final iteration.
    uint8_t buf[I2C_BUFFERSIZE];
    _ReadBlock(address, buf, bufSize);
    for (uint16_t i = 0; i < bufSize; ++i) {
      if (buffer[i] != buf[i]) count++;
    }
    address += bufSize;
    buffer  += bufSize;
    len     -= bufSize;
  }
  return count;
}

[RobTillaart]

@microfoundry

Some theoretical thoughts

Gaps

If I want to store one byte in EE, I need to send a header (byte deviceAddress, 2 byte memoryAddress) plus the value. The transport efficiency is 25%. If I send more bytes the efficiency will increase. However bytes that have not changed do not need to be sent, however...

If the gap == 1 If I have 2 bytes changed with one unchanged (gap) in between = CUC, then i can send them separately or as a block. Separate: HHHC. HHHC = 8 bytes 25% efficiency Block: HHHCUC = 6 bytes. 33% efficiency (2 changed out if 6 send)

If the gap == 2 Separate: HHHC. HHHC = 8 bytes 25% efficiency Block: HHHCUUC = 7 bytes 28% efficiency (2 changed out of 7 send)

If the gap == 3 Block: HHHCUUUC = 8 bytes, so 25% efficiency (break even)

page factor

After a write to a page the EE is off line for up to 5 milliseconds. So if 2 separate single byte writes are on the same page the EE will block for up to 10 ms.

If 1 block of 8 bytes is written on the same page it will block for up to 5 ms.

If the writes are done over 2 pages, they are roughly equally performant.

Optimal block size.

Within one page one can calculate the break even point of how long a gap may be before blocking becomes slower.

Assume 100 KHz == 1 byte takes ~ 100 us

2 separate takes 2 x 4 x 100 us + 10 ms = 10800 us 1 block takes 8 x 100 us + 5 ms = 5800 us. So we have 5000 us before break even, That is about 50 bytes!

Assuming 400 KHz == 1 byte takes ~ 25 us So the 5000 us to break even are almost 200 bytes.

If the write delay is lower e.g 3.5 ms the numbers will be lower of course.

The conclusion seems to be that combining writes per page seems to be an important efficiency factor. However single byte writes have the advantage that the processor can do other things.

Question remains how to translate these insights into code.

Code

1) the range updated must be split at page boundaries. 2) per page one minimizes the number of writes. 3) if the I2CBUFFER < page size one has to iterate within one page. 4) if the I2CBUFFER>= page size one can do it in one write.

So far my theoretical thoughts,

Does this makes sense?

[RobTillaart]

@microfoundry You might need to merge the new 1.8.4