maintain time.monotonic precision by using adafruit_ticks

kevin-tritz commented 3 months ago

The code originally loses timing precision even using time.monotonic_ns because it stores timestamps after using floating point division. If the timestamps are kept as ns integers, and floating point division is only done after subtracting from the current monotonic_ns time, precision can be maintained.

kevin-tritz commented 3 months ago

An alternative to this is to use the adafruit_ticks library, which counts in msecs, for intervals up to about 6 days.

I originally made the change using the adafruit_ticks library, and it seemed to work just fine. But I believe Justin suggested not including an additional dependency since they would want this library to work with the esp32spi, which is frozen: https://discordapp.com/channels/327254708534116352/633326631460798464/1220549312803504299

dhalbert commented 3 months ago

I would not worry a lot about the new dependency: we can freeze adafruit_ticks if needed.

There are only a few boards that don't have monotonic_ns: basically boards that don't have longints: SAMD21 boards without external flash, and a few STM boards with low flash. I don't think these boards are used for MQTT with such low RAM anyway: I'm not sure even ESP32SPI would fit on them.

kevin-tritz commented 3 months ago

Test build environment needs adafruit_ticks?

kevin-tritz commented 3 months ago

build fails because the test is modifying a private variable using the otherwise unused get_monotonic_time function. Either the build needs to be modified appropriately, or get_monotonic_time needs to be modified with backwards-incompatible changes.

vladak commented 3 months ago

build fails because the test is modifying a private variable using the otherwise unused get_monotonic_time function. Either the build needs to be modified appropriately, or get_monotonic_time needs to be modified with backwards-incompatible changes.

I'd say the function needs to be removed and the test adapted per the changes in the module.

kevin-tritz commented 2 months ago

Ok, I made a couple of modifications to the test_loop file, one which is likely fine, one which I'm not sure if it's ok or not. The .loop function was returning a list of length 3 instead of 2, and I'm not sure if that's just some slight differences in timing, or it's really a failed check.

After those mods, the tests passed, but the build is complaining about adafruit_ticks:

File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/adafruit_ticks.py", line 31, in _TICKS_MAX = const(_TICKS_PERIOD - 1)


TypeError: unsupported operand type(s) for -: 'const' and 'int'

vladak commented 2 months ago

Ok, I made a couple of modifications to the test_loop file, one which is likely fine, one which I'm not sure if it's ok or not. The .loop function was returning a list of length 3 instead of 2, and I'm not sure if that's just some slight differences in timing, or it's really a failed check.

After those mods, the tests passed, but the build is complaining about adafruit_ticks:

File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/adafruit_ticks.py", line 31, in _TICKS_MAX = const(_TICKS_PERIOD - 1) ~~~~~~^~~ TypeError: unsupported operand type(s) for -: 'const' and 'int'

_TICKS_PERIOD is const (imported from micropython) and has not changed in 3 years (https://github.com/adafruit/Adafruit_CircuitPython_Ticks/blame/527d90e91fa70d08399291229bdcb2e9a6605a6a/adafruit_ticks.py#L30-L32) so something else must have changed, possibly in Python/CircuitPython itself or the micropython module. It works for me on Python3 shipped with Ubuntu 22.04.04 LTS:

$ which python3
/usr/bin/python3
$ python3
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> 
>>> from micropython import const
>>> foo = const(42)
>>> bar = const(foo - 1)
>>>

Anyhow, not related to your changes (modulo pulling the adafruit_ticks anew) as far as I can tell.

Also, I am not actually sure where does the micropython module comes from, it is not listed in pip list. Peeking around (with strace), I can see it is located in ~/.local/lib/python3.10/site-packages/micropython.py and comes from Adafruit. Therein the const is defined simply as:

def const(x):
    "Emulate making a constant"
    return x

vladak commented 2 months ago

The micropython.py comes from Adafruit_Blinka: https://github.com/adafruit/Adafruit_Blinka/blob/b3beba7399bc4d5faa203ef705691ef79fc4e87f/src/micropython.py#L12-L14 and is still the same as above so perhaps some more strict enforcement was introduced in CPython itself.

dhalbert commented 2 months ago

@vladak: @jepler discussed this with @kevin-tritz in Discord. The problem I think was that micropython was mocked in the documentation build, so a correct implementation of const() was not available. This was indeed confusing.

vladak commented 2 months ago

Ok, I made a couple of modifications to the test_loop file, one which is likely fine, one which I'm not sure if it's ok or not. The .loop function was returning a list of length 3 instead of 2, and I'm not sure if that's just some slight differences in timing, or it's really a failed check.

I will take a look.