peterhinch / micropython-async

Application of uasyncio to hardware interfaces. Tutorial and code.
MIT License
742 stars 168 forks source link

GPS crashes on invalid input #99

Closed Zuikkis closed 1 year ago

Zuikkis commented 1 year ago

Hi!

I'm using GPS for RTC sync. The GPS module is generic china module that connects to UART, rx only.

Anyway it works fine 99.99% of time. However sometimes it receives something "bad" which actually crashes the receiver:


future: <Task> coro= <generator object '_run' at 2000e6b0>
Traceback (most recent call last):
  File "uasyncio/core.py", line 1, in run_until_complete
  File "as_drivers/as_GPS/as_GPS.py", line 185, in _run
  File "uasyncio/stream.py", line 1, in readline
TypeError: unsupported types for __iadd__: 'bytes', 'NoneType'

This is bad because it crashes the receiver and I think it doesn't recover from it automatically?

Now, in the code I see this:

    async def _run(self):
        while True:
            res = await self._sreader.readline()
            try:
                res = res.decode('utf8')
            except UnicodeError:  # Garbage: can happen e.g. on baudrate change
                continue
            asyncio.create_task(self._update(res))
            await asyncio.sleep(0)  # Ensure task runs and res is copied

It probably would work if I just remove "UnicodeError" so except would catch "TypeError" as well? Any particular reason why it is only catching UnicodeError?

Also I'm slightly worried are there other places where similar crash could occur? I need this code running 24/7 for months or even years. I think I'll add daily or weekly machine.reset() which should take care of any these kind of lockups. I have a battery backed RTC on board as well, so it really is enough to get GPS sync once a day or even weekly.

Zuikkis commented 1 year ago

Sorry, I just realized line 185 is the readline(), not decode().

So perhaps need to move readline inside of "try"?

peterhinch commented 1 year ago

I think this is what is happening. The UART is timing out, returning None. This is crashing uasyncio here.

You haven't said what platform you're using but one option is to increase or disable the timeouts. These are specified as UART constructor args timeout and timeout_char. Values are in ms. Some platforms accept -1 to disable timeouts. Otherwise make them longer than the interval between GPS messages.

I have raised this issue. I'm not sure if it qualifies as a uasyncio bug.

Zuikkis commented 1 year ago

Thanks for your prompt reply!

I already "fixed" this by adding try/except to the readline call, but your suggestion to add timeouts seems to work too so I now reverted back to unmodified uasyncio. :)

I'm using Raspberry Pi Pico, sorry for not mentioning it in the first place.

I'll close this issue. I'm sure you are right that if there's anything to fix it's in micropython's side.