RobTillaart/FRAM_I2C

Bug with reading

ddprog opened this issue · 7 comments

Hello!

I found a bug with reading from FRAM from the second half of the addresses (1 Mbit). Please look and correct.
readblock

Corrected for _addr, it seems to read correctly.

Thank you for reporting the bug. (not tested well enough apparently)

Code should indeed be:

void FRAM32::_readBlock(uint32_t memAddr, uint8_t * obj, uint8_t size)
{
  uint8_t _addr = _address;
  if (memAddr & 0x00010000) _addr += 0x01;

  _wire->beginTransmission(_addr);
  _wire->write((uint8_t) (memAddr >> 8));
  _wire->write((uint8_t) (memAddr & 0xFF));
  _wire->endTransmission();
  _wire->requestFrom(_addr, size);
  uint8_t * p = obj;
  for (uint8_t i = size; i > 0; i--)
  {
    *p++ = _wire->read();
  }
}

Will create a PR asap.

I will add a test writing + read back to high address in ** FRAM32_writeObject.ino**

Develop branch + PR created

Note to myself.

problem

The line

  if (memAddr & 0x00010000) _addr += 0x01;

checks if the address is in the range 0x10000 .. 0x1FFFF (64K .. 128K)
As there are afaik no FRAMs bigger than 128K (1Mbit) this code looks OK.
However there is a chance of wrapping if memAddr would be e.g. 0x00030001,
The 3 would set the if expression to true too.
So we need to guard the device address.

idea (to keep the thought)

If bigger FRAM's or in case of "concatenated" 64K FRAM's, the code would need to be something like

addr += (memAddr >> 16);  //  upper part of memory address is used to select the chip.

That could work however the address could become _address + 2 effectively causing to write to another 1 Mb device.

Robust guarding

  if (memAddr & 0xFFFE0000) return;  //  ignore all invalid addresses
  if ((memAddr & 0x00010000) == 0x00010000) _addr += 0x01;

Both for _readBlock() and _writeBlock().

Note

if memAddr would be e.g. 0x00020001, would interfere with lower memory addresses too.
Robust guarding solves this too.

Note in readme.md future section to investigate error handling/ reporting wrt invalid memory addresses

@ddprog
Again thanks for reporting, version 0.8.0 has just been released (build is running).