Nesting AnimateOnce inside AnimationSequence causes random animation order

[dhalbert]

A user commented on this: https://forums.adafruit.com/viewtopic.php?f=60&t=186732

This (simplified) example, after the first round, does the red and blue animations in random order. Each Pulse animation is correct, but, successive Pulses do not strictly alternate between red and blue. Note the AnimateOnce inside AnimationSequence.

import board
import neopixel
from adafruit_led_animation.animation.pulse import Pulse
from adafruit_led_animation.sequence import AnimationSequence
from adafruit_led_animation.sequence import AnimateOnce
from adafruit_led_animation.color import RED, BLUE


num_pixels = 24

pixels = neopixel.NeoPixel(board.A0, num_pixels, auto_write=False)
pixels.brightness = 0.2

pulse0 = Pulse(pixels, speed=.05, color=RED, period=1)
pulse1 = Pulse(pixels, speed=.05, color=BLUE, period=1)

animations = AnimationSequence(
    AnimateOnce(pulse0,pulse1,),
    auto_clear=True,
    auto_reset=True,
    advance_interval=5,
)

while True:
        animations.animate()

Removing the nesting works fine. Red and blue alternate, as expected.

import board
import neopixel
from adafruit_led_animation.animation.pulse import Pulse
from adafruit_led_animation.sequence import AnimateOnce
from adafruit_led_animation.color import RED, BLUE

num_pixels = 24

pixels = neopixel.NeoPixel(board.A0, num_pixels, auto_write=False)
pixels.brightness = 0.2

pulse0 = Pulse(pixels, speed=.05, color=RED, period=1)
pulse1 = Pulse(pixels, speed=.05, color=BLUE, period=1)

animations = AnimateOnce(pulse0,pulse1,
                         auto_clear=True,
                         auto_reset=True,
                         advance_interval=5,
)

while True:
        animations.animate()

[GithubUser99999999]

Is Adafruit ever going to look at these bugs?

[RetiredWizard]

@FoamyGuy got me interested in this during his deep dive stream....

The AnimateOnce method doesn't actually enforce a single animate, the example code indicates that you need to test the return value and stop animating once it returns False, rather than using the usual while True: animate loop. (just a side thought... this changes the meaning of the return value from that of the parent class, which seems like it could be problematic)

I think in order to use AnimateOnce as an AnimationSequence member you should use the advance_on_cycle_complete parameter which tells AnimationSequence to move on to the next animation once a single pass of each member animation has been completed. This seems equivalent to exiting the animate loop when AnimateOnce returns False.

So using this animation block:

animations = AnimationSequence(
    AnimateOnce(pulse0,pulse1,),
    auto_clear=True,
    auto_reset=True,
    advance_on_cycle_complete=True,
)

You get the alternating red, blue result but only a single pulse each rather than 5 seconds of pulsing for each color. Of course you get exactly the same result if you simply list pulse0,pulse1 as animation members of the AnimationSequence with advance_on_cycle_complete turned on.

So the next question is why would you want to use AnimateOnce within a sequence? The only thing I've come up with so far is that you might want to mix single cycle animations with timed animations. You can get close to this by nesting animation sequences like this:

 animat1 = AnimationSequence(
    pulse0,pulse1,
    auto_clear=True,
    auto_reset=True,
    advance_on_cycle_complete=True,
)
animat2 = AnimationSequence(
    pulse3,pulse4,
    auto_clear=True,
    auto_reset=True,
    advance_interval=5,
)

animations = AnimationSequence(
    animat1,animat2,
    auto_clear=True,
    auto_reset=True,
    advance_on_cycle_complete=True,
)

animat1 runs single cycle animations and then animat2 runs advancing each of it's members after 5 seconds. The only problem is that the timer isn't reset when a member moves on as a result of completing a cycle so the first pulse of animat2 runs for 5 seconds minus however long it takes for animat1 to run.

Based on the poking around I've done I don't think there's actually a bug here (except maybe the missing timer reset). Calling the animate method on a AnimateOnce object after the anmiation cycle has completed has an undefined result and probably shouldn't be done.

if an AnimateOnce object is passed into AnimationSequence, the advance_oncycle_complete parameter should be set to True