fix sync when there are multiple different pixel objects involved

[rhooper]

This PR needs a bit more testing because it might reintroduce flickering bugs.

[FoamyGuy]

I set out to test this by trying to make 4 Comet animations play across the 4 rows in a NeoTrellis M4, though same principal applies to any 8x4 I think. Maybe I misunderstand out the PixelMap helper is meant to be used, but I've not been able to figure out how to make it work like that. In trying I noticed that some animations behave link Blink instead of themselves when they are applied to a subset of the pixels.

I was trying code like this:

pixels = neopixel.NeoPixel(pixel_pin, pixel_num, brightness=0.01, auto_write=False)
pixel_map = PixelMap(pixels, [
    (0,8)
], individual_pixels=False)
comet = Comet(pixel_map, speed=0.1, color=GREEN, tail_length=2)
while True:
    comet.animate()

It correctly sections off, and uses only the first 8 pixels (first row). But instead of the comet flying back and forth all 8 are blinking at the same time. I also tried Chase and RainbowSparkle and got similar results.

I tested with the current version of the library and it is working the same way so I don't believe it's related to this PR.

I can work on it some more tomorrow. Let me know if I am trying to use the pixel map incorrectly.

[FoamyGuy]

With fresh eyes this morning it occurred to me I probably need to be using either the vertical or horizontal lines pixel maps.

I was able to get a single row comet working like this:

pixel_wing_vertical = helper.PixelMap.vertical_lines(
    pixels, 8, 1, helper.horizontal_strip_gridmap(8, alternating=False)
)
comet = Comet(pixel_wing_vertical, speed=0.1, color=GREEN, tail_length=2)
while True:
    comet.animate()

Later today I will try to figure out how to get the other 3 rows going and then get everything grouped up and running at the same time.

[FoamyGuy]

Poking around a bit more, I've just noticed PixelSubset seems to do exactly what I am after actually. Allowing the strip to be split up in different start and end sections. I was able to get multiple comets going like this:

first_row = helper.PixelSubset(pixels, 0, 8)
second_row = helper.PixelSubset(pixels, 8, 16)
comet = Comet(first_row, speed=0.1, color=GREEN, tail_length=4)
comet2 = Comet(second_row, speed=0.1, color=GREEN, tail_length=4)
group = AnimationGroup(comet,comet2)
while True:
    group.animate()

I do notice that I have trouble if I use reverse=True on the Comets (they aren't showing for me any more if I include that). I don't think this is related to the change in this PR though. I will try to verify and document the issue later.

I think I've got it figured out enough to get this more thoroughly tested today now.

[FoamyGuy]

I have not seen any flickering issues but I do notice the animations in the group will eventually get out of sync. Using the code from my previous comment.

When the animation first begins playing the two comets are in sync:
https://www.dropbox.com/s/lpno5pq0mwwx7f9/VID_20200623_091915.mp4?dl=0

However after leaving it running for a while they start getting further and further out of sync:
https://www.dropbox.com/s/2sreqv6m56vxajd/VID_20200623_091845.mp4?dl=0

[FoamyGuy]

This is the full code from my testing that was in use when the videos in the previous comment were recorded:

import board
import neopixel
from adafruit_led_animation.animation.comet import Comet
from adafruit_led_animation.color import GREEN
from adafruit_led_animation.group import AnimationGroup
from adafruit_led_animation import helper

# Update to match the pin connected to your NeoPixels
pixel_pin = board.NEOPIXEL
# Update to match the number of NeoPixels you have connected
pixel_num = 32

pixels = neopixel.NeoPixel(pixel_pin, pixel_num, brightness=0.1, auto_write=False)

first_row = helper.PixelSubset(pixels, 0, 8)
second_row = helper.PixelSubset(pixels, 8, 16)
comet = Comet(first_row, speed=0.1, color=GREEN, tail_length=4)
comet2 = Comet(second_row, speed=0.1, color=GREEN, tail_length=4)
group = AnimationGroup(comet,comet2)
while True:
    group.animate()

The two rows start out in sync and stay that way for several minutes (maybe close to half hour or so, but I haven't specifically timed it) But the eventually start to separate enough to be visible. In the videos above they are basically one pixel off from each other making one Comet look like it's one pixel "ahead" of the other. I left this running all day and they eventually ended up separated by 4 pixels so one Comet was almost fully "behind" the other one.

I intend to do some more testing after I am finished working for the day.

[rhooper]

This is an interesting quirk... I'll be looking at it soon.

[rhooper]

Congratulations, you have run into the whole reason sync=True exists on groups. Animation draw() is controlled based on the clock within each animation. Grouped animations don't share the clock, so sync=True was added to AnimationGroup to ensure the first animation in a group controls the draw frequency. Sharing the clock amongst animations added to a group could help here, and might be more intuitive than the sync parameter.

[FoamyGuy]

I ended up stepping up to four_comets.py and eventually also created a script for eight_comets.py. And in doing so learned a bit more about how the PixelMap works!

I am not sure that I understand the issue this was meant to fix, but I did successfully test the changes with with these multi-comet scripts.

As noted in my other comment the animations do eventually get out of sync if they play for longer than several minutes. But I don't think it's necessarily related to these changes. I tested the current released version of the library and see the same effect. And it does take quite a while before it really becomes noticeable.

I'm going to go ahead and mark this approved because the changed library does work correctly playing these comet examples as far as I can tell. I never saw any flickering or any other noticeable artifacts.

If there is an example of an animation group that was having problems prior to these change I can test that out on my end but otherwise I'm thinking this looks good for now.

Edit: Ah I see now in the time it took me to write this you have commented some more. I had not seen that when this was written.

[FoamyGuy]

Congratulations, you have run into the whole reason sync=True exists on groups. Animation draw() is controlled based on the clock within each animation. Grouped animations don't share the clock, so sync=True was added to AnimationGroup to ensure the first animation in a group controls the draw frequency. Sharing the clock amongst animations added to a group could help here, and might be more intuitive than the sync parameter.

Ah I see. Thank you! I have a slightly modified eight_comet_sync.py running now. It sets sync=True and does not have any bounces enabled. I will leave this running a while now that sync is enabled.