search

adafruit / Adafruit_CircuitPython_NeoPixel

CircuitPython drivers for neopixels.
MIT License
304 stars 98 forks source link

Initialise self.pin before calling pypixelbuf #87

Closed LukeMoll closed 4 years ago

LukeMoll commented 4 years ago

This fixes #85

Tested on STM32F411CE "blackpill" running CircuitPython 5.3.0, Trinket M0 and ItsyBitsy M4 both running 5.4.0 beta 0. All against pypixelbuf from bundle 5.x-200526.

FoamyGuy commented 4 years ago

As far as I understand it pixelbuf is included in the core code. Not the library bundle. Looking in the bundle download from today I do not see any files in there with "pixelbuf" in the name.

I haven't been able to recreate the issue so far but I can work on testing these changes on a few devices a bit later on tonight.

LukeMoll commented 4 years ago

@FoamyGuy I'd been copying adafruit_pypixelbuf.mpy across - that was included in the bundle I downloaded.

FoamyGuy commented 4 years ago

Ah I see it now the py at the beginning of the name threw me off a bit. I am not familiar with the STM32 black pill device however I think that library would not be needed on the Itsy Bitsy M4.

I think that some devices (IB M4 included) have pixelbuf built into their core system code instead of needing a library. On those devices there is no need to include this external library (the test code posted on the #87 runs on the Neo Trellis M4 without this library present).

Trinket M0 I'm not sure about, I know it's a little more pressed for space since it's a "non-express" device. Perhaps it does require the external library I can check on that tonight as well.

FoamyGuy commented 4 years ago

The support matrix can shed some light on the differences in devices: https://circuitpython.readthedocs.io/en/latest/shared-bindings/support_matrix.html (edited this link. Prior version linked to an out of date support matrix)

Any device that has _pixelbuf listed as an available module uses the internal pixelbuf code in the core and shouldn't need the external library from the bundle I believe.

Any device that does not have _pixelbuf listed would need the external library from the bundle in order to work successfully.

tannewt commented 4 years ago

I think this is fine but also unnecessary. I think the better fix is to make pypixelbuf behave like _pixelbuf. It doesn't enable auto-write until the end of the constructor: https://github.com/adafruit/circuitpython/blob/master/shared-module/_pixelbuf/PixelBuf.c#L74

kattni commented 4 years ago

@tannewt It appears the fix you suggested has been applied. https://github.com/adafruit/Adafruit_CircuitPython_Pypixelbuf/blob/49cadeba1db0f9a0a2ee39d7d8c4949fecd4c337/adafruit_pypixelbuf.py#L108

Does that resolve the issue this PR was attempting to address?

tannewt commented 4 years ago

Yup! I believe so. Thanks @kattni