fram size incorrect with len

[jerryneedell]

I was experimenting with an SPI FRAM breakout and found that len(fram) only reports the size as 8191 - but I can write to the 8192nd location -- Is this a bug?

Press any key to enter the REPL. Use CTRL-D to reload.
Adafruit CircuitPython 4.0.0-alpha.5-82-g0599fbf7d on 2019-01-09; Adafruit Feather nRF52840 Express with nRF52840
>>>
>>> import board
>>> import busio
>>> import digitalio
>>> import adafruit_fram
>>>
>>> ## Create a FRAM object.
>>> spi = busio.SPI(board.SCK, board.MOSI, board.MISO)
>>> cs = digitalio.DigitalInOut(board.D5)
>>> fram = adafruit_fram.FRAM_SPI(spi, cs)
>>> len(fram)
8191
>>> fram[8191]
bytearray(b'\x00')
>>> len(fram)
8191

>>> fram[8191]=1
>>> fram[8191]
bytearray(b'\x01')
>>> 

[jerryneedell]

same for an I2C fram

Press any key to enter the REPL. Use CTRL-D to reload.
Adafruit CircuitPython 4.0.0-alpha.5-82-g0599fbf7d on 2019-01-09; Adafruit Feather nRF52840 Express with nRF52840
>>> 
>>> import board
>>> import busio
>>> import adafruit_fram
>>> 
>>> ## Create a FRAM object (default address used).
>>> i2c = busio.I2C(board.SCL, board.SDA)
>>> fram = adafruit_fram.FRAM_I2C(i2c)
>>> 
>>> len(fram)
32767
>>> fram[32767]
bytearray(b'\x00')
>>> fram[32767]=1
>>> fram[32767]
bytearray(b'\x01')
>>> 

[jerryneedell]

probably this should be _max_size + 1
https://github.com/adafruit/Adafruit_CircuitPython_FRAM/blob/master/adafruit_fram.py#L122

[jerryneedell]

The description https://github.com/adafruit/Adafruit_CircuitPython_FRAM/blob/master/adafruit_fram.py#L109 is a little confusing -- for the SPI FRAM there are 8192 locations 0 - 8191 so the maximum "register" location is 8191, but the size of the bytearray is 8192. or am I missing something.

[makermelissa]

I have the I2C FRAM and will take a look into this.

[makermelissa]

Confirmed. My FRAM exhibited the same symptoms. Also, just to be sure it was zero-based:

>>> fram[0]
bytearray(b'\x00')
>>> fram[0] = 1
>>> fram[0]
bytearray(b'\x01')

[makermelissa]

Because of the way that python normally handles arrays:

>>> fruit = ["apple", "banana", "orange"]
>>> len(fruit)
3

I think it makes sense to add 1 to the length and to change the description so that it is less confusing.

[dhalbert]

I totally agree. _max_size right now is a misnomer, so either rename it to _max_address or change the constants to be a power of 2, not the current power of 2 minus -1.

[dhalbert]

I think changing the constants makes more sense, not changing the name. People rarely use constants like 8191.

[jerryneedell]

Agreed -- the len(fram) should be the total size -- 8192 or 32768 in these examples. I just wanted someone else to look over ti to make sure "fixing" it would not break something else that depended on the "max_address" value.

[makermelissa]

Changing the constant would require changing all of the checks to make sure it wasn't writing out of range and adding + 1 to the length only required one code change. Either way is fine, just something to keep in mind.

[makermelissa]

After thinking about it, I think logically changing all the checks does make a bit more sense. I just need to doublecheck that this doesn't result in an off by one error.

[dhalbert]

Yes, keep the name as _max_size, increment the constants by 1 to be a power of 2, and change the if key > statements to be if key >=. Otherwise we still have technical debt.

[makermelissa]

Yes, keep the name as _max_size, increment the constants by 1 to be a power of 2, and change the if key > statements to be if key >=. Otherwise we still have technical debt.

Agreed.

[makermelissa]

While testing my code, I found another bug in the code from before I worked on it. With Address Slicing, the addresses are off by one (they don't include the last address and are unable to use the last address by slicing. This is using the original code before my edits:

>>> fram[32767]
bytearray(b'\x01')
>>> fram[32766:32767]
bytearray(b'\x00')
>>> fram[32766:32768]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lib/adafruit_fram.py", line 150, in __getitem__
ValueError: Register + Length greater than maximum FRAM size. (32767)

I can fix this in my changes, but could also open another issue since it's related to the issue, but also is another issue entirely.

[makermelissa]

Size issue fixed as well as the slicing issue I found.