-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Updates to pixelbuf API - Addresses #884 #2034
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good here! Just a couple comments. I'm a bit torn about having the constants for every order still but it does give us the ability to change it later.
@rhooper hi - what are ya currently blocking on for this PR - we can get some help to wrap up this PR. |
From the top of my head, these are the remaining tasks:
|
@rhooper would it be ok for other folks to implement/test some of these TODOs? |
@ladyada absolutely fine with me. the more people that know the changes the better! |
sure, they'll post here in this thread |
This reverts commit ca5b277.
…bclass show method from the native superclass
I worked around the native |
Dotstar and Neopixel changes appear to work, but I need some assistance gathering up examples to test compatibility. adafruit/Adafruit_CircuitPython_NeoPixel#59 |
Pypixelbuf has had some testing done - seems to work - but needs the same kind of testing as the Dotstar and NeoPixel PR. It also needs putting in pypi so that the Dotstar and Neopixel builds can pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really really close. The show stuff looks fine except the name. Another questions about unused param but good otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! Thank you for your incredible persistence. @dhalbert you will need to approve as well or at least dismiss your review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thank you for all this work!!
💯 💯 🔥 🏅 💯 💯 to everyone who helped get this over the finish line - you are heros! many glowy projects will thank u :) |
:🎉🎉🎉Whoooo🎉🎉🎉🥳 Awesome contribution, thank you! Much glow will be had |
This work-in-progress still requires some thorough testing. It switches over _pixelbuf to the same string based API as pypixelbuf.
Early feedback would be highly welcome, @dhalbert @tannewt
Also thoughts on what a good test suite looks like would help me.