adafruit / Adafruit_CircuitPython_MiniMQTT

MQTT Client Library for CircuitPython
Other
72 stars 50 forks source link

loop() uses time.monotonic(), resulting in unwanted behaviors #176

Closed gmparis closed 7 months ago

gmparis commented 10 months ago

Use of time.monotonic() in loop() causes issues with pinging and specified timeout values greater than 0. This is due to the resolution of the float value returned by monotonic. After an amount of uptime, the time subtractions in the code will begin to result in 0.

vladak commented 8 months ago

Can you be more specific about which of the 2 subtractions in loop() is causing issues ? What was the timeout value ? What platform did you reproduce this on ?

gmparis commented 8 months ago

monotonic() is stored as a float. As uptime increases, the last bit of the mantissa begins to represent larger and larger quanta. At some point it is larger than one second and at some other point can be larger than the timeout value. My opinion is that monotonic() is not suitable for use in IoT device code, as such devices may be powered on for exceedingly long periods of time. Instead use one of the time functions that return bigint.

On Wed, Oct 25, 2023 at 5:31 PM Vladimir Kotal @.***> wrote:

Can you be more specific about which of the 2 subtractions in loop() is causing issues ? What was the timeout value ? What platform did you reproduce this on ?

— Reply to this email directly, view it on GitHub https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT/issues/176#issuecomment-1780084441, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF37LIP5W3FBA2Q6OWYMZHDYBGAJ3AVCNFSM6AAAAAA4SQPLLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBQGA4DINBUGE . You are receiving this because you authored the thread.Message ID: @.*** com>

vladak commented 8 months ago

I see. According to my computations based on https://docs.circuitpython.org/en/latest/shared-bindings/time/#time.monotonic after some 24 days the counter will be increased only every second or so.

Wonder what the right solution is, given that:

gmparis commented 8 months ago

Perhaps not as big a dilemma as you imagine. What devices are suitable for using MQTT (i.e., have wireless or Ethernet) and do not support monotonic_ns()? The bottom line is that if the device locks up after a period of a few weeks, it is not good enough for any serious use. Even too frustrating for non-serious use. Better to just say MQTT is not supported on that device.

On Fri, Oct 27, 2023 at 11:38 AM Vladimir Kotal @.***> wrote:

I see. According to my computations based on https://docs.circuitpython.org/en/latest/shared-bindings/time/#time.monotonic after some 24 days the counter will be increased only every second or so.

Wonder what the right solution is, given that:

— Reply to this email directly, view it on GitHub https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT/issues/176#issuecomment-1783125330, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF37LIITLGD3JBUEYKEQEEDYBPIQNAVCNFSM6AAAAAA4SQPLLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBTGEZDKMZTGA . You are receiving this because you authored the thread.Message ID: @.*** com>

vladak commented 8 months ago

Perhaps not as big a dilemma as you imagine. What devices are suitable for using MQTT (i.e., have wireless or Ethernet) and do not support monotonic_ns()? The bottom line is that if the device locks up after a period of a few weeks, it is not good enough for any serious use. Even too frustrating for non-serious use. Better to just say MQTT is not supported on that device.

Thanks for pointing out that only networked boards need to be considered.

If I understand it correctly, time.monotonic_ns() uses Python long ints, so does not wrap around.

https://github.com/adafruit/circuitpython/issues/519 has a comment about non long int builds for tiny devices such as Gemma or Trinket. These do not have any network connection AFAIK and there's probably not an add-on board to add it either. I am not faimilar with CircuitPython and Adafruit ecosystem enough to be able to tell what boards might actually be impacted (maybe Adafruit Feather M0 WiFi or such ?), however monotonic_ns() seems to be the way to go.

If there is anything that could have network but not long int build then there are several ways how to deal with this:

which is pretty lame as the reaction to the exception should be a board reset.

Having the code that would switch based on call availability between a call that provides sufficiently consistent precision but wraps around and a call that has the required precision characterstics and does not wrap around but is not available on all networked boards sounds like a way to make the library too complex and fragile so I'd like to avoid it.

Maybe for a good measure add a __init__ code that would detect the lack of monotonic_ns() and raise some sort of runtime exception to prevent frustration. Or, if it's not available, go with monotonic() as the last resort. Again, the above bullet section applies. Or, raise the exception only if certain init parameter is not set (which would not be by default).