Fixed size when polling length of FRAM to reflect actual size #7
Conversation
dhalbert
left a comment
dhalbert
left a comment
There was a problem hiding this comment.
Thanks! Some style changes and also check for < 0 on __getitem__ and __setitem__
adafruit_fram.py
Outdated
| " list, or tuple.") | ||
| if key > self._max_size: | ||
| if key >= self._max_size: | ||
| raise ValueError("Requested register '{0}' greater than maximum" |
jerryneedell
left a comment
jerryneedell
left a comment
There was a problem hiding this comment.
Looks good! Tested on an I2C FRAM - len(fram) = 32768 -- yay!
Thank you!
|
of course Dan was far more thorough ... good suggestions. |
|
I went ahead and implemented your changes and fixed a bug that I found with address slicing that caused the last address to not be included. |
dhalbert
left a comment
dhalbert
left a comment
There was a problem hiding this comment.
Good testing, finding the slice problem.
I noted some error messages that still said "register". It is up to you whether to change all the internal uses of "register" to "address" as well, for consistency. I think you could do a global search/replace and it would be fine (_read_register -> _read_adsress, etc.)
|
Thanks. I went through and changed most of register to address except where it didn't make sense and tested again. Also I just did it manually because I don't trust the search/replace on mu. |
|
I just noticed I had used a < instead of <= check for the first address. That's now fixed. |
dhalbert
left a comment
dhalbert
left a comment
There was a problem hiding this comment.
Great! Thank you for fixing the original bugs and for your patience with my nitpicking! I think the code is more readable now.
Updating https://github.com/adafruit/Adafruit_CircuitPython_FRAM to 1.2.2 from 1.2.1: > Merge pull request adafruit/Adafruit_CircuitPython_FRAM#7 from melissagirl/master Updating https://github.com/adafruit/Adafruit_CircuitPython_RA8875 to 1.0.1 from 1.0.0: > Merge pull request adafruit/Adafruit_CircuitPython_RA8875#2 from melissagirl/master Updating https://github.com/adafruit/Adafruit_CircuitPython_BluefruitConnect to 1.0.5 from 1.0.3: > Merge pull request adafruit/Adafruit_CircuitPython_BluefruitConnect#6 from dhalbert/add-simpletest Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Added the following libraries: Adafruit_CircuitPython_RA8875, Adafruit_CircuitPython_MIDI
3 participants
Addressing #6
I also updated examples, comments, and error messages to reflect the change.