Issue Fix #21 (Addressing 24LC04/08/16) incomplete

[thjean]

The addressing bug fix in #21 is incomplete.

It corrects the addressing in function _beginTransmission(). When using the 24LC04/08/16, the same address calculation must also be applied in function _ReadBlock(), Line 349 when calling uint8_t readBytes = _wire->requestFrom(_deviceAddress, length); With the current implementation, _ReadBlock() always accesses the first 256bytes of the EEPROM.

[RobTillaart]

Hi thjean,

Can you make a PR ? if not I can do it later this week / weekend.

[thjean]

Hi Rob,

Thank you for coming back to the issue report. Unfortunately, I'm a total noob regarding a PR in Git, so it's probably easier for you to take over :).

Here is my local fix, probably not the most elegant one.

• First, I added a var in I2C_eeprom::I2C_eeprom(const uint8_t deviceAddress, const uint32_t deviceSize, TwoWire *wire)

_devAddressBlk = deviceAddress;

• In void I2C_eeprom::_beginTransmission(const uint16_t memoryAddress), I changed the two lines

uint8_t addr = _deviceAddress | ((memoryAddress >> 8) & 0x07);
  _wire->beginTransmission(addr);

in the else-path of if (this->_isAddressSizeTwoWords) to

    _devAddressBlk = _deviceAddress | ((memoryAddress >> 8) & 0x07);
    _wire->beginTransmission(_devAddressBlk);

• and finally in uint8_t I2C_eeprom::_ReadBlock(const uint16_t memoryAddress, uint8_t* buffer, const uint8_t length), I use the newly introduced var _devAddressBlk instead of_deviceAddress

uint8_t readBytes = _wire->requestFrom(_devAddressBlk, length);

I tested my local fix with a 24LC16B (Microchip), the addressing scheme seems to be identical to the one used by STM chips. The write operation was already ok as it just uses the combined address for _beginTransmission(). I did not test the fix with a two words address size EEPROM, as I currently do not have one at hands. As _devAddressBlk is intialized with deviceAddress, _ReadBlock() should remain functional.

[RobTillaart]

Created a develop branch in which I added a minimal fix in _readBlock() only.

Can you verify it works for you?

It might not be the final version yet, but lets first tackle the problem, then try to optimize it.

[RobTillaart]

Good catch by the way, appreciated!

[thjean]

Thank you for creating the development branch.

The fix works perfectly for the Microchip 24LC16. It should work with double-byte addressing EEPROMs as well, however I could not verify that due to lack of chips.

[RobTillaart]

The change is made as local as possible so it should not affect the types that use 2 words.

[RobTillaart]

I will merge it later this week, so if you find other issues they might be included too

[thjean]

Btw thanks for creating this library. I highly appreciate the update() functions, as they significantly help preserving device lifetime.

[RobTillaart]

There might be better ways to safe lifetime. Sometimes you just do not need to write every change back to EEPROM. Imagine that your project could detect a power failure, and has enough energy (backup battery or capacitor) to store the latest values.

[RobTillaart]

Fixed in 1.5.0