knightrider fixed

[armi-ne]

position updated using substep instead of step value below 4 (fixed divisor value) would result in int casting _step to 0, therefore resulting in no position updates animation_step calculations updated _substep and _step now remain the same unless animation speed is changed animation_divisor added give users further fine grained control if needed

[xs5871]

Changing how substeps and positions are calculated will mess with all the other animations. Instead of having meaningless knobs with arbitrary, hardware dependend scales like animation_speed and animation_divisor, we should start refactoring animations to use proper units, like period length in seconds.

[armi-ne]

Changing how substeps and positions are calculated will mess with all the other animations. Instead of having meaningless knobs with arbitrary, hardware dependend scales like animation_speed and animation_divisor, we should start refactoring animations to use proper units, like period length in seconds.

Thank you for your response, I've come to this realisation as I've been working with other animation types currently and have encountered new issues. If I can implement a suitable solution for my own implementation I will be sure to update and submit a proper pull request.

[armi-ne]

I have come across a second solution which does not affect other animation modes and yet fixes knight rider.

Namely, changing how the bounds are checked for the position. Old code:

# Reverse animation when a boundary is hit
if pos >= self.num_pixels or pos - 1 < (self.knight_effect_length * -1):
    self.reverse_animation = not self.reverse_animation

New code looks something like this:

# Reverse animation when a boundary is hit
if pos > self.knight_effect_length and not self.reverse_animation:
    self.reverse_animation = not self.reverse_animation
if pos < (self.knight_effect_length * -1) and self.reverse_animation:
    self.reverse_animation = not self.reverse_animation