adafruit / Adafruit_CircuitPython_DotStar

Dotstarrin' away in CircuitPython land
MIT License
46 stars 38 forks source link

(#9) change __getitem__ to use self._n instead of self._buf // 4 #17

Closed margaret closed 6 years ago

margaret commented 6 years ago
margaret commented 6 years ago

More context setup for self._buf is like this

:param int n: The number of dotstars in the chain
...
self._n = n
self.start_header_size = 4
# Supply one extra clock cycle for each two pixels in the strip.
self.end_header_size = n // 16
if n % 16 != 0:
    self.end_header_size += 1
self._buf = bytearray(n * 4 + self.start_header_size + self.end_header_size)

so if you had one dotstar, length of self._buf would be 1*4 + 4 + 0 and self._buf // 4 would be 2 instead of the expected 1 in

    def __getitem__(self, index):
        if isinstance(index, slice):
            out = []
            for in_i in range(*index.indices(self._n)): # used to be self._buf // 4
                out.append(
                    tuple(self._buf[in_i * 4 + (3 - i) + self.start_header_size] for i in range(3)))
            return out
margaret commented 6 years ago

Tested this manually with the pycon Gemma M0 ✅

(Test with current master of this library) code.py (modified from the default pycon open session file) is ``` # Gemma IO demo # Welcome to CircuitPython 2.2.4 :) from adafruit_hid.keyboard import Keyboard from adafruit_hid.keycode import Keycode from digitalio import DigitalInOut, Direction, Pull from analogio import AnalogIn, AnalogOut from touchio import TouchIn import adafruit_dotstar as dotstar import microcontroller import board import time # One pixel connected internally! dot = dotstar.DotStar(board.APA102_SCK, board.APA102_MOSI, 1, brightness=0.2) tests = [ '[0]', '[1]', '[-1]', '[:1]', '[1:]', '[0:]' ] for test_idx in tests: teststr = 'dot'+test_idx print(teststr + ' ' + '-'*80) try: print(eval(teststr)) except IndexError as e: print('Error:') print(e) print() ``` This prints ``` Auto-reload is on. Simply save files over USB to run them or enter REPL to disable. code.py output: dot[0] -------------------------------------------------------------------------------- (0, 0, 0) dot[1] -------------------------------------------------------------------------------- Error: dot[-1] -------------------------------------------------------------------------------- (0, 0, 0) dot[:1] -------------------------------------------------------------------------------- [(0, 0, 0)] dot[1:] -------------------------------------------------------------------------------- Error: bytearray index out of range dot[0:] -------------------------------------------------------------------------------- Error: bytearray index out of range ``` Compared to this from CPython Python 3.6.1 (v3.6.1:69c0db5050, Mar 21 2017, 01:21:04) [GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin ``` >>> dot [(0, 0, 0)] >>> tests = [ ... '[0]', ... '[1]', ... '[-1]', ... '[:1]', ... '[1:]', ... '[0:]' ... ] >>> for test_idx in tests: ... teststr = 'dot'+test_idx ... print(teststr + ' ' + '-'*80) ... try: ... print(eval(teststr)) ... except IndexError as e: ... print('Error:') ... print(e) ... print() ... dot[0] -------------------------------------------------------------------------------- (0, 0, 0) dot[1] -------------------------------------------------------------------------------- Error: list index out of range dot[-1] -------------------------------------------------------------------------------- (0, 0, 0) dot[:1] -------------------------------------------------------------------------------- [(0, 0, 0)] dot[1:] -------------------------------------------------------------------------------- [] dot[0:] -------------------------------------------------------------------------------- [(0, 0, 0)] ``` With this PR's changes ``` Auto-reload is on. Simply save files over USB to run them or enter REPL to disable. code.py output: dot[0] -------------------------------------------------------------------------------- (0, 0, 0) dot[1] -------------------------------------------------------------------------------- Error: dot[-1] -------------------------------------------------------------------------------- (0, 0, 0) dot[:1] -------------------------------------------------------------------------------- [(0, 0, 0)] dot[1:] -------------------------------------------------------------------------------- [] dot[0:] -------------------------------------------------------------------------------- [(0, 0, 0)] ``` This matches the behavior of standard slicing, so I think this change is ✅ Note that the IndexError description doesn't seem to get bubbled up in circuitpython, but I think that should be a separate issue.
margaret commented 6 years ago

@tannewt Ready for review