added sparkle mask parameter

[lila]

the sparkle animations look great on an 8x8 led matrix. But at each iteration, the code picks a random led to sparkle. I wanted to mask the 8x8 matrix so that the sparkles would make an image. for example, here it is with a heart shaped mask:

this change adds a optional mask parameter to the Sparkle class. the mask parameter is an array that specifies the narrowed set of values to select from. So if mask=[0, 2, 4, 6, 8, 10], then sparkle would pick randomly among those values.

I also updated the example file for sparkle where it uses the heart shaped mask.

There might be a better way to do this, but this seemed to work. I did compile it locally, and used it in this project:

https://github.com/lila/circuitpythonexpress-matrix

• k

[FoamyGuy]

@lila thanks for working on this! It looks very neat. The CI actions are showing failed because it wants you to run black code formatting and pylint checker. This page covers the process for Black formatting: https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/black the same guide has a section for pylint as well which it may complain about next.

[FoamyGuy]

Thanks for working through those! I know it's a bit of a pain sometimes. There are few more pylint errors. I've provided some suggestions for how to fix the remaining ones.

[FoamyGuy]

instead of [] as the default can we use None instead?

[FoamyGuy]

if we go with None as the default parameter, then we can move this length check down below the next thing.

[FoamyGuy]

here we can check for None and set it up accordingly like this:

if mask:
    self._mask = mask
else:
    self._mask = []

if len(self._mask) >= len(pixel_object):
    raise ValueError("Sparkle mask should be smaller than number pixel array")

[lila]

makes sense, will fix. thanks...

[FoamyGuy]

the one other pylint issue it has right now is that it wants this if statement to not use the else: like this:

if len(self._mask) == 0:
    return random.randint(0, (len(self.pixel_object) - 1))
return self._mask[random.randint(0, (len(self._mask) - 1))]

Honestly I kind if prefer it written more explicitly like how you did it with the else: personally. But I'll leave it up to you, or we can see if anyone else has thoughts. You can either change it like above to make pylint happy, or add a disable comment in this in this function to have it ignore the check.

[lila]

no problem, i'll fix...

[FoamyGuy]

@lila I think it's very close now, sorry for all the little hangups with PyLint and Black.

I think the last error it has now is it's wanting these extra spaces at the end of the heart mask declaration in the example script to be removed:

I can see many of the spaces have been used in order to put the code into a shape that matches the physical reality also though. So if it is still complaining about extra spaces after removing the ones I show highlighted above personally I'd be in favor of just adding the pylint disable comment for it so that we can keep the code shape that matches the physical shape.

[FoamyGuy]

These changes to the sparkle animation look good to me. Thanks for working on this @lila this is a really neat new feature!

For the example I think we want to keep the original example intact as well since it's got a comment that specifies what device it's made for (NeoPixel Featherwing) which only has 32 LEDs (8x4 instead of 8x8)

Can you put the original example back into led_animation_sparkle_animations.py and then make a new file for your new one that uses the masks to make a heart on the 8x8 grid. You can name it something like led_animation_sparkle_animations_masked.py

After that I think this is good to go. I tested these changes with a NeoPixel featherwing using this slightly modified version of the example to work with 8x4 and make a "checkerboard" pattern instead of heart:

import board
import neopixel

from adafruit_led_animation.animation.sparkle import Sparkle
from adafruit_led_animation.sequence import AnimationSequence
from adafruit_led_animation.color import JADE, AQUA, PINK, BLUE

# Update to match the pin connected to your NeoPixels
pixel_pin = board.IO38
# Update to match the number of NeoPixels you have connected
pixel_num = 32
# fmt: off
first_mask = [1,   3,  5,  7,
              8,  10, 12, 14,
              17, 19, 21, 23,
              24, 26, 28, 30]


second_mask = [0,   2,  4,  6,
               9,  11, 13, 15,
               16, 18, 20, 22,
               25, 27, 29, 31]


# fmt: on
pixels = neopixel.NeoPixel(pixel_pin, pixel_num, brightness=0.5, auto_write=False)

animations = AnimationSequence(
    Sparkle(pixels, speed=0.1, color=JADE, num_sparkles=2, mask=first_mask),
    #Sparkle(pixels, speed=0.05, color=AQUA, num_sparkles=1),
    Sparkle(pixels, speed=0.1, color=BLUE, num_sparkles=2, mask=second_mask),
    advance_interval=5,
    auto_clear=False,
)

while True:
    animations.animate()

@kattni can you take a look when you have a moment once you're back in the office

[lila]

that makes sense, i'm happy to make the change, and thank you for your patience.

would you recommend cancelling this pull request and doing a new one where the existing example file has not been edited, that way the file doesn't get unnecessary commits in its history?

or should i just add a new commit to bring it back to what it was?

• k

[FoamyGuy]

I think it's fine to stay with this PR, the extra commits to that file won't be an issue I don't think.

[jposada202020]

@FoamyGuy @lila , what do you think about this, is the conflicting merges the only thing left to do here, I think that would be an easy fix and if Foamyguy agrees we could merge. let me know. thanks :)

[lila]

i apologize, i got busy and this dropped out of my cache :-( let me look at it this evening. i would love to get it merged...

• k

[kattni]

The conflicting merge issue will not be present if the changes to the sparkle_animations example are removed and put into a new example as FoamyGuy suggested. I agree that it should be a new example.