updateBlock() returns bytes read not written

[chrismck]

I've tried using this function in a program which makes repetitive writes to the EEPROM, but wanted to avoid the additional work to see if the write is actually necessary. I'm assuming this function is designed to write blocks that have only changed and return the bytes that were in the end written. However when I use the function it kept writing (or appeared to) even though the data on the EEPROM and buffer were the same.

I realized you are incrementing 'bytes' (return value) from outside memcmp{} block in updateBlock(), therefore always returning full buffer size, not just what was written

    bytes += _ReadBlock(address, buf, count); 
    if (memcmp(buffer, buf, count) != 0) {
         _pageBlock(address, buffer, count, true);
    }

But I believe you meant to do... Hopefully I'm getting this correct. Just an FYI -
haven't check the other update function updateByte()?

    _ReadBlock(address, buf, count); 
    if (memcmp(buffer, buf, count) != 0) {
         _pageBlock(address, buffer, count, true);
         bytes += count;
    }

(updated for syntax highlighting)

[RobTillaart]

Thanks for reporting this issue,
I'm rather busy so it might take a few days to dive into this.

[RobTillaart]

Had a first look to understand the issue.

You found a mismatch between documentation and implementation, that's for sure.
Question that pops up is: What would the user expect as a return value?

1. actual bytes written (as documentation states).
2. the size of the whole block to be in line with writeBlock()

advantage of (1) is that you can measure the gain, where (2) does not have a clear advantage.

Then we have

bool I2C_eeprom::updateBlockVerify(const uint16_t memoryAddress, const uint8_t * buffer, const uint16_t length)
{
  if (updateBlock(memoryAddress, buffer, length) != length) return false;
  return verifyBlock(memoryAddress, buffer, length);
}

which should be rewritten to

bool I2C_eeprom::updateBlockVerify(const uint16_t memoryAddress, const uint8_t * buffer, const uint16_t length)
{
  updateBlock(memoryAddress, buffer, length);
  return verifyBlock(memoryAddress, buffer, length);
}

To be continued.

[RobTillaart]

int I2C_eeprom::updateByte(const uint16_t memoryAddress, const uint8_t data)
{
  if (data == readByte(memoryAddress)) return 0;
  return writeByte(memoryAddress, data);
}

which returns I2C status, 0 = OK - so yes, that is even different from the return values above.

That said, I'm happy with this one as it returns informative state info, success or failure.

So I propose to focus onlu on updateBlock() and updateBlockVerify().

[RobTillaart]

this it becomes

//  returns bytes actually written <= length
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 bytes = 0;
  while (len > 0)
  {
    uint8_t buf[I2C_BUFFERSIZE];
    uint8_t count = I2C_BUFFERSIZE;

    if (count > len) count = len;
    _ReadBlock(address, buf, count);
    if (memcmp(buffer, buf, count) != 0)
    {
      _pageBlock(address, buffer, count, true);
      bytes += count;
    }
    address += count;
    buffer  += count;
    len     -= count;
  }
  return bytes;
}

bool I2C_eeprom::updateBlockVerify(const uint16_t memoryAddress, const uint8_t * buffer, const uint16_t length)
{
  updateBlock(memoryAddress, buffer, length);
  return verifyBlock(memoryAddress, buffer, length);
}

[RobTillaart]

Created develop branch for version 1.9.1, build is running.

If you have time, please give it a try if it now returns the bytes actual written.
(I have no HW setup nearby)

[chrismck]

Will do.

[chrismck]

Sorry for delay, I'm not familiar (so had to look it up) how you get to a branch from the lib_deps in platformio.

Works as expected, it return the bytes written now. Thanks.

As for the updateBlockVerify() don't use it, but I'm guessing if updateBlock() returns 0, no need to call Verify as nothing was written.

[RobTillaart]

As for the updateBlockVerify() don't use it, but I'm guessing if updateBlock() returns 0, no need to call Verify as nothing was written.

That is a good insight, thanks a lot!
Will include it for performance sake :)

update: added in .cpp build runs.

[RobTillaart]

Merged develop in master and released version 1.9.1