Skip to content

Adding rainbow module. #4976

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

Merged
merged 2 commits into from
Jul 9, 2021
Merged

Adding rainbow module. #4976

merged 2 commits into from
Jul 9, 2021

Conversation

kattni
Copy link

@kattni kattni commented Jul 8, 2021

Adds rainbow module with colorwheel() function. Removes colorwheel() from _pixelbuf.

@kattni kattni requested review from tannewt, jepler and dhalbert July 8, 2021 19:57
Copy link
Member

@gamblor21 gamblor21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a quick look, everything seems fine with me. One question but probably something you did on purpose.

@dhalbert
Copy link
Collaborator

dhalbert commented Jul 8, 2021

The one failure is that some escornabot_makech builds were too big. That board is a robot with wheels, etc. Based on that, you could turn off MIDI and I think that would not crimp any uses.

In ports/atmel_samd/boards/escornabot_makech/mpconfigboard.mk, add CIRCUITPY_USB_MIDI = 0. An example of that is in adafruit_proxlight_trinkey_m0/mpconfigboard.mk (along with other things).

Once #4903 is merged, we won't be so close to the edge on many builds.

@kattni
Copy link
Author

kattni commented Jul 8, 2021

@dhalbert Fix submitted. Also updated copyright in _pixelbuf that I meant to include in the first place, but did not want to restart the entire CI process for only that change.

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks and congrats!

@dhalbert dhalbert merged commit 7c2fd27 into adafruit:main Jul 9, 2021
@dhalbert
Copy link
Collaborator

dhalbert commented Jul 9, 2021

Partly addresses #4926.

@tannewt
Copy link
Member

tannewt commented Jul 9, 2021

Sorry I just looked at this. We shouldn't call this rainbow because of the existing pypi package: https://pypi.org/project/rainbow/

We could call it adafruit_rainbow though. Or rainbowio :-)

@dhalbert
Copy link
Collaborator

dhalbert commented Jul 9, 2021

adafruit_rainbow is ok by me, or even rainbowio. Or:

  • rainbows (not taken in PyPi)
  • pixel_colors
  • rgb
  • color_math
  • color_calc

@kattni
Copy link
Author

kattni commented Jul 9, 2021

So I really like rainbowio but on the off chance we have to make a Python library version of colorwheel(), I should perhaps accept adafruit_rainbow so the name would be the same and all the code would work. It comes down to, what is the likelihood that we'll have to do a Python version? The only reason the Python version of Pixelbuf exists is that _pixelbuf was too big for tiny builds. rainbow built fine into all but one build, so I'm inclined to think we're not going to need a Python version. I'd like to avoid needing a Python version, but if that's going to happen, we'll end up in a _pixelbuf/adafruit_pypixelbuf situation again with rainbowio.

@tannewt
Copy link
Member

tannewt commented Jul 9, 2021

We could make a rainbowio pypi package if we really wanted to.

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.

4 participants