adafruit / Adafruit_CircuitPython_DotStar

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

adafruit_dotstar __getitem__ applies incorrect cap to range input #9

Closed kevinjwalters closed 6 years ago

kevinjwalters commented 6 years ago

I'm a little new to Python this so I may be wrong here but I think adafruit_dotstar's getitem method is attempting to cap the range of the slice values using:

range(*index.indices(len(self._buf) // 4)):

But the length cannot be determined correctly with a simple division because of the header and trailer data in the buffer for dotstars. Perhaps this code was cut and paste from other code which has no header data in buffer?

self._n looks like a suitable (calculation-free) substitute here? I see setitem uses len(self) which looks correct but you may wish to avoid the method call given the hardware this typically runs on. Either way, a consistent approach across class would make code easier to read.

kevinjwalters commented 6 years ago

Unrelated, I'm also curious about brightness vs _brightness, the code is a bit mysterious here!

Also in the example is uses an integer value of 1. The value is loosely specified as a floating point value so perhaps that would be better as 1.0?

kevinjwalters commented 6 years ago

indices method is documented here: https://docs.python.org/3.4/reference/datamodel.html?highlight=.indices#slice.indices

tannewt commented 6 years ago

@kevinjwalters I think you are right about indices. Care to fix it?

Please open a second issue about brightness.

kevinjwalters commented 6 years ago

For brightness issue: https://github.com/adafruit/Adafruit_CircuitPython_DotStar/issues/10

margaret commented 6 years ago

I'm going to work on this (pycon sprints).

tannewt commented 6 years ago

DOne by @margaret in #17. Thanks!