Adding auto_write and show() #15
Conversation
|
Hmm.... hold on this. It's faster then with auto_write, but still not as fast as my hacky implementation... looking. |
|
Ah.... darn. So the |
adafruit_trellism4.py
Outdated
| @@ -82,8 +83,53 @@ def __setitem__(self, index, value): | |||
| raise IndexError("Pixel assignment outside available coordinates.") | |||
|
|
|||
| self._neopixel[offset] = value | |||
| if self._auto_write: | |||
| self.show() | |||
There was a problem hiding this comment.
This check is done by neopixel here:
https://github.com/adafruit/Adafruit_CircuitPython_NeoPixel/blob/master/neopixel.py#L171
There was a problem hiding this comment.
Not in this case because the Trellis M4 module turns off auto_write in the Neopixel module when it's initialized. I was trying to keep that the case and therefore implemented the logic at this level.
adafruit_trellism4.py
Outdated
| @@ -125,7 +171,8 @@ def fill(self, color): | |||
|
|
|||
| """ | |||
| self._neopixel.fill(color) | |||
| self._neopixel.show() | |||
| if self._auto_write: | |||
| self._neopixel.show() | |||
There was a problem hiding this comment.
This check is done by neopixel here:
https://github.com/adafruit/Adafruit_CircuitPython_NeoPixel/blob/master/neopixel.py#L210
There was a problem hiding this comment.
Same as comment above.
adafruit_trellism4.py
Outdated
|
|
||
| trellis.pixels.auto_write = True | ||
| """ | ||
| return self._auto_write |
There was a problem hiding this comment.
Why not just have this be a pass thrus to the underlying neopixel object?
return self._neopixel.auto_writeThere was a problem hiding this comment.
Because it would break _NeoPixelArray.fill(), as noted above I was attempting to make the change with minimal modification to how _NeoPixelArray already worked.
adafruit_trellism4.py
Outdated
|
|
||
| @auto_write.setter | ||
| def auto_write(self, val): | ||
| self._auto_write = val |
There was a problem hiding this comment.
Same here.
self._neopixel.auto_write = val
adafruit_trellism4.py
Outdated
| @@ -62,6 +62,7 @@ def __init__(self, pin, *, width, height, rotation=0): | |||
| width, height = height, width | |||
| self._width = width | |||
| self._height = height | |||
| self._auto_write = True | |||
|
|
|||
There was a problem hiding this comment.
If this is the desired state why not just set auto_write=True in the _init above?
There was a problem hiding this comment.
Because of the reasons mentioned previously. I can change it to use the Neopixel autowrite instead, I just thought this was cleaner at the time.
|
@caternuson @jerryneedell Updated to do it the way you suggested... late last night I swear I had a better reason for not doing it that way, but upon reassessment I can't think of what it was. |
|
There may be Reasons™ why it was originally set to |
|
The |
|
Thanks, I've gone ahead and signed off for my part. Looks like what I was thinking. Huge caveat though - I don't actually have this HW so can't test. Hopefully I'm not overlooking Reasons™. |
|
I will test it later today. Please wait to merge until I have tested it. |
There was a problem hiding this comment.
Tested successfully! Thank you! I have a couple of suggestions.
Please see my change request below. I updated both docstrings but it was easier to do in one comment. It's a small change to show. And there's no need to explain why your auto_write example could be different.
Updating https://github.com/adafruit/Adafruit_CircuitPython_TrellisM4 to 1.3.0 from 1.2.1: > Merge pull request adafruit/Adafruit_CircuitPython_TrellisM4#15 from adammhaile/master
@kattni As discussed. Works great on my Trellis M4