Skip to content

adafruit_dotstar __getitem__ applies incorrect cap to range input #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kevinjwalters opened this issue Jan 18, 2018 · 6 comments
Closed

Comments

@kevinjwalters
Copy link

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
Copy link
Author

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
Copy link
Author

@tannewt
Copy link
Member

tannewt commented Jan 18, 2018

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

Please open a second issue about brightness.

@kevinjwalters
Copy link
Author

For brightness issue: #10

@margaret
Copy link
Contributor

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

margaret added a commit to margaret/Adafruit_CircuitPython_DotStar that referenced this issue May 14, 2018
- self._buf would be incorrect if there are headers
- adafruit#9
margaret added a commit to margaret/Adafruit_CircuitPython_DotStar that referenced this issue May 14, 2018
- self._buf would be incorrect if there are headers
- adafruit#9
tannewt added a commit that referenced this issue May 14, 2018
(#9) change __getitem__ to use self._n instead of self._buf // 4
@tannewt
Copy link
Member

tannewt commented May 16, 2018

DOne by @margaret in #17. Thanks!

@tannewt tannewt closed this as completed May 16, 2018
mattytrentini pushed a commit to mattytrentini/micropython-dotstar that referenced this issue Jan 4, 2019
- self._buf would be incorrect if there are headers
- adafruit#9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants