peterhinch / micropython_eeprom

MicroPython device drivers for memory chips (EEPROM, FRAM, Flash, SPIRAM)
MIT License
73 stars 33 forks source link

Write issue with eeprom i2c and writeblocks() #21

Closed warthog9 closed 10 months ago

warthog9 commented 10 months ago

I suspect this is more that I'm using it wrong but ran into an issue today where I was attempting to write a bytearray down to a T24C64, all 8192 bytes, and resulted in not getting what I was expecting written.

ESP32, latest micropython downloaded and flashed today.

import os
from machine import Pin, SoftI2C
from eeprom_i2c import EEPROM, T24C64
i2c = SoftI2C( scl=Pin(22), sda=Pin(21), freq=100000 )
eep = EEPROM(i2c, T24C64)

buf = bytearray( len(eep) )
buf_cp = bytearray( len(eep) )

eep.readblocks( 0, buf )
eep.readblocks( 0, buf_cp )
print( buf )

So far so good, chip is detected, read, and the printout shows what I'm expecting with data that's not \xff

for x in range( len(eep) )
   eep[ x, x+1 ] = b'\xff' 
ee.readblocks( 0, buf )
print( buf )

Clearing the eeprom so it's all reset, and printing it back out does in fact show it's been re-written (and that writes work).

eep.writeblocks( 0, buf_cp )
eep.readblocks( 0, buf )
print( buf )

printout shows only \xff, even after a reboot.

Going back to doing this:

for x in range( len(eep) )
   eep[ x, x+1 ] = buf_cp[ x, x+1 ]
ee.readblocks( 0, buf )
print( buf )

Writes the data out as expected, so something seems off on writeblocks() (and by extensions readwrite() )here and I'm not sure why. I tried a few variations around the block writes but could not get any of them to work, it was only via slices and doing it one byte at a time was I able to get things written as I'd expected.

For good measure, and to make sure I wasn't doing something else wrong I did run eep_i2c.py's test() function and the first two stages passed, last was skipped, and in readblocks() afterwards there was clearly data on the eeprom.

peterhinch commented 10 months ago

I am puzzled why you're using .readblocks and .writeblocks directly. These methods exist only to support filesystems. Given that you're using byte access I suggest you use slice notation as documented here.

If you can demonstrate a problem using slice notation I will investigate.

peterhinch commented 10 months ago

@warthog9 I perhaps didn't explain myself well. The .readblocks, .writeblocks and ioctl methods are undocumented. They exist purely to support filesystems via the block protocol and their explicit use is not recommended.

If you're not mounting a filesystem, the driver makes an EEPROM array behave exactly like a bytearray. You can copy to and from it exactly as if it were a bytearray, with the only difference being that it is nonvolatile. Assuming you have an EEPROM instance eep the following provides a good test of read and write:

import os
arr = bytearray(8192)
arr[:] = os.urandom(8192)  # Fill with random data
eep[:8192] = arr  # Copy to EEPROM
eep[:8192] == arr  # Readback: should return True

I use 8192 rather than len(eep) because the array I checked this on was much bigger.

If this fails please report. Some EEPROM chips have a page size other than 128 bytes,as per the above reference. This can be fixed.

warthog9 commented 10 months ago

So if those methods are not intended to be used, outside a specific use case, the documentation makes absolutely no mention of this, and in a quick reading made me think they would be a more useful set of actions for my purposes than the slice notation.

https://github.com/peterhinch/micropython_eeprom/blob/98ebb3c7db823aa84fa060a0b84f7241d06b27f6/eeprom/i2c/I2C.md?plain=1#L213

My use case was explicitly to just read the entire contents of an eeprom out, and write it to another eeprom, that's quite arguably a block level action since I'm not even remotely trying to interpret anything that's coming out or going in, I just wanted the bits in as raw a form as possible.

So I'll accept I was using the functions not as intended, but I would strongly argue I was not entirely off in what I was trying to do given what's documented in I2C.md either.

Obviously my own fix was to use slice notation, I have already fundamentally solved my own problem, I'm reporting back that what I ran into seemed like a very obvious bug, which I'd contend it still is either in the documentation which should be more explicit about the read/write/readwrite/ioctl functions around L213, or just forgo mentioning those functions entirely and hope that someone poking around in the repl itself won't play with them.

peterhinch commented 10 months ago

I'll amend the doc to provide an explicit warning. [EDIT] Now done.