Skip to content

Initialise self.pin before calling pypixelbuf #87

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
wants to merge 2 commits into from

Conversation

LukeMoll
Copy link

@LukeMoll LukeMoll commented Jun 3, 2020

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

FoamyGuy commented Jun 3, 2020

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

LukeMoll commented Jun 3, 2020

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

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Jun 3, 2020

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

FoamyGuy commented Jun 3, 2020

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

tannewt commented Jun 3, 2020

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

kattni commented Aug 19, 2020

@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
Copy link
Member

tannewt commented Aug 20, 2020

Yup! I believe so. Thanks @kattni

@tannewt tannewt closed this Aug 20, 2020
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

Successfully merging this pull request may close these issues.

Can't set brightness in NeoPixel()
4 participants