Skip to content
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

Cosmetic fixes, updated rainbow_cycle #33

Merged
merged 3 commits into from
Oct 17, 2018

Conversation

kattni
Copy link
Contributor

@kattni kattni commented Oct 17, 2018

No description provided.

@caternuson
Copy link
Contributor

General comment as this applies to the entire file. This example was written before the ability to define pixel order in the constructor was added. See issue #1 and pr #24. Using bpp was the old way of doing it. This could all be re-written to remove use of bpp and change to using pixel_order in the constructor.

@kattni
Copy link
Contributor Author

kattni commented Oct 17, 2018

I totally forgot about that. You're right.

@caternuson
Copy link
Contributor

Yep. Like that.

Now that format_tuple func is bugging me. I'm concerned it's going to be seen as some necessary magic that users will propagate into their own code.

  1. Remove format_tuple
  2. Change wheel to check ORDER, i.e. move the logic from format_tuple to wheel
  3. Change the pixels.fill() lines in the loop to just call with a tuple, and have conditionals to check ORDER to call with a 3 tuple or 4 tuple as needed. That way the pixels.fill() lines will look more like typical usage.

For 2., here's maybe a way to change wheel:

def wheel(pos):
    # Input a value 0 to 255 to get a color value.
    # The colours are a transition r - g - b - back to r.
    if pos < 0 or pos > 255:
        r = g = b = 0
    elif pos < 85:
        r = int(pos * 3)
        g = int(255 - pos*3)
        b = 0
    elif pos < 170:
        pos -= 85
        r = int(255 - pos*3)
        g = 0
        b = int(pos*3)
    else:
        pos -= 170
        r = 0
        g = int(pos*3)
        b = int(255 - pos*3)
    return (r, g, b) if ORDER == neopixel.RGB or ORDER == neopixel.GRB else (r, g, b, 0)

@kattni
Copy link
Contributor Author

kattni commented Oct 17, 2018

That's the thing that caught my eye in the first place. I'll try to work in this suggestion.

Copy link
Contributor

@caternuson caternuson left a comment

Choose a reason for hiding this comment

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

Yeah. Much better!

@caternuson caternuson merged commit 8b1124c into adafruit:master Oct 17, 2018
tannewt pushed a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Oct 18, 2018
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.

2 participants