adafruit/Adafruit_CircuitPython_LED_Animation

time.time() does not exist on trinket m0

ladyada opened this issue · 23 comments

@caternuson wanna test this library on a trinket m0?

@ladyada Sure, can take a look.

Just to document what's already been mentioned:

Itsy M4 Express:

Adafruit CircuitPython 5.3.1 on 2020-07-13; Adafruit ItsyBitsy M4 Express with samd51g19
>>> import time
>>> dir(time)
['__class__', '__name__', 'localtime', 'mktime', 'monotonic', 'monotonic_ns', 'sleep', 'struct_time', 'time']
>>> 

Trinket M0:

Adafruit CircuitPython 5.3.1 on 2020-07-13; Adafruit Trinket M0 with samd21e18
>>> import time
>>> dir(time)
['__name__', 'monotonic', 'sleep', 'struct_time']
>>> 

indeed!

This might be a limiter. Can hold the float, but not the int representation:

Adafruit CircuitPython 5.3.1 on 2020-07-13; Adafruit Trinket M0 with samd21e18
>>> import time
>>> time.monotonic()
2073.0
>>> time.monotonic() * 1000000
2.08741e+09
>>> int(time.monotonic() * 1000000)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: float too big
>>> 

hm why do we need ns precision anyways?

Not sure. @rhooper Any harm in reducing the timing precision for this lib? The ns requirement is preventing use on smaller boards.

Logic doesn't seem to be highly dependent on int type, so just left it as float to see what would happen. But then just hit int size limit elsewhere.

Adafruit CircuitPython 5.3.1 on 2020-07-13; Adafruit Trinket M0 with samd21e18
>>> import comet_test
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "comet_test.py", line 8, in <module>
  File "adafruit_led_animation/animation/comet.py", line 98, in __init__
  File "adafruit_led_animation/animation/__init__.py", line 70, in __init__
  File "adafruit_led_animation/animation/__init__.py", line 208, in speed
OverflowError: small int overflow
>>>  

So this issue may be more generalized to "support boards with only small int"?

Yep. Was about to link to same line :)

Seems like it's a simple swap out with just a loss in timing precision as a result? But not sure if there are other subtleties as to why things were based on ns to begin with. I'll keep working a refactor FWIW, but also curious what @rhooper says.

There isn't a built in MS equivalent to monotonic_ns(). Not sure there's much to be gained by doing math on monotonic() to convert to MS? Below is from just basing everything on float seconds and using time.monotonic().

import board
import neopixel
from adafruit_led_animation.animation.comet import Comet
from adafruit_led_animation.color import RED

pixels = neopixel.NeoPixel(board.D0, 8, pixel_order=neopixel.GRBW)

comet = Comet(pixels, speed=0.2, color=RED, tail_length=10)

while True:
    comet.animate()

20200811_122155

monotonic_ns was used to avoid issues with decreasing precision of time.monotonic() and time.time() with long uptimes.

@caternuson I would just make this helper work on trinket

Not sure. @rhooper Any harm in reducing the timing precision for this lib? The ns requirement is preventing use on smaller boards.

yeah there are definitely significant issues with using float time. having an int timer avail in all circuitpython builds would be the best fix, but memory/flash is so scarce

okay caught up with the thread. should be trivial to use milliseconds. sorry about multiple posts, I'm on my phone while kattni records.

Sounds like basing on milliseconds is OK in terms of timing precision? But using float instead of int is not a good idea. So could just make the needed changes and then create a monotonic_ms helper to replace monotonic_ns. And then have conditionals to use whatever is best for a given platform.

On Trinket M0, time.monotonic is all that is available. So use on Trinket M0 will just have to live with the decreasing precision issue. Is the issue so bad that animations are unusable?

@caternuson i think its gonna be ok, these LEDs dont react in under 10 ms anyways. we could also use _ns if available (multiply by 1000)

@caternuson The PR in #63 was tested on Feater NRF52840 Express and a Trinket M0.

Testing on a trinket needs a subset of the lib and needs to be mpy-cross compiled.
I used this bash script to compile and install:

for path in __init__.py color.py helper.py animation/__init__.py animation/comet.py ; do ../../circuitpython/mpy-cross/mpy-cross $path -o /Volumes/CIRCUITPY/lib/adafruit_led_animation/`dirname $path`/`basename $path .py`.mpy; done

@caternuson i think its gonna be ok, these LEDs dont react in under 10 ms anyways. we could also use _ns if available (multiply by 1000)

I checked... emulating montonic_ns didn't work on the trinket - it got ValueError: float too big

on trinket, use monotonic() - on platforms that have _ns, you can have the ms function return _ns * 1000 ?

yep yep. i think we're all saying the same thing.
PR LGTM

This is now merged.

@caternuson please bump this to release the updated code and then we can notify the OP!