Rethink lvesp32, lvstm32 and lv_async

amirgon commented 3 years ago

Some thoughts:

[x] Remove lvesp32, replaced by lv_utils.py event loop which uses machine.Timer.
machine.Timer is available on both architectures and additional architectures.
[x] Replace lvstm32 by lv_utils event_loop
[x] Remove lv_async and move its functionality to v_utils event_loop. It provides the same service under uasyncio
[x] Should it be ~under /generic or~ under /lib ?
[x] I don't think it should start automatically when importing it, like lvesp32 does today.
Better import and then create an object. In the ctor we could change defaults such as refresh rate, select between async/non-async modes, etc.
This would also require changes in the display drivers usage. ~Should the display driver start it automatically? Or should the user configure it and start it?~ ILI9XXX starts it automatically if not already started. SDL needs to be configured to use it or not.
[x] What about the unix port? Unfortunately machine.Timer is not available there. Today this functionality hides in the display driver (SDL or FB) maybe we should extract it to allow simpler use with uasyncio, make it configurable etc. Can this also be the same script the stm32/esp32 would be using?
[x] Need to prevent oversubscription when calling schedule, otherwise other services can't register (this may be the cause of some BT issues which were related to lvesp32). Use a counter to limit the maximum number of slots it's allowed to occupy on the schedule queue. (2?)

amirgon commented 3 years ago

What about the unix port? Unfortunately machine.Timer is not available there. Today this functionality hides in the display driver (SDL or FB) maybe we should extract it to allow simpler use with uasyncio, make it configurable etc. Can this also be the same script the stm32/esp32 would be using?

As a first step, implemented machine.Timer for the unix port.

Update

The unix FFI-based Timer crashes after some time, possibly because of gc-related problem: ~https://github.com/micropython/micropython/issues/7273 (But the workaround suggested there did not fix the problem)~

Update

The unix FFI-based Timer crashes were root caused and a work-around applied (until Micropython upstream fix is available) Related: https://github.com/micropython/micropython/issues/7372, https://github.com/micropython/micropython/issues/7373

Upadte

Created PR on upstream micropython for FFI callback improvements: https://github.com/micropython/micropython/pull/7401
With this PR lv_timer.py for the unix port works well.

amirgon commented 3 years ago

@embeddedt I've removed lvstm32.py as part of this consolidation effort. (https://github.com/lvgl/lv_micropython/commit/c6db8b7fb3c5bd11149605216fbc1f7accda591a)
Instead, from now on all micropython ports and all drivers are supposed to use a single event loop that calls lv_task_handler and lv_tick_inc.

I've verified this on unix and esp32 ports, but I don't have an stm32 board. Could you help and try running advanced_demo.py (or anything else) that uses lv_utils.py instead of lvstm32.py?
Thanks!

embeddedt commented 3 years ago

I tried freezing example3.py on STM32 and then running it using import example3. This happened:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "example3.py", line 10, in <module>
  File "lv_utils.py", line 76, in __init__
ValueError: Timer doesn't exist

advanced_demo gives a very similar error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "advanced_demo.py", line 412, in <module>
  File "advanced_demo.py", line 401, in init_gui
  File "advanced_demo.py", line 365, in init_gui_stm32
  File "lv_utils.py", line 76, in __init__
ValueError: Timer doesn't exist

machine.Timer does exist on STM32:

>>> from machine import Timer
>>> Timer
<class 'Timer'>

Memory corruption again? :confused:

amirgon commented 3 years ago

Memory corruption again? 😕

I don't think so. The error is on this line:

https://github.com/lvgl/lv_binding_micropython/blob/e0635ed5fd5bcdbeb6d40ffbf593fd866f91c276/lib/lv_utils.py#L76

The default value of timer_id is 4, but it looks like stm32 only supports soft timer(?!)

Could you try again with timer_id=-1 on event_loop constructor?

embeddedt commented 3 years ago

That was all that was needed. With timer_id=-1 added there advanced_demo runs fine.

amirgon commented 3 years ago

That was all that was needed. With timer_id=-1 added there advanced_demo runs fine.

Great! Thanks.

I wonder if it makes sense to automatically set it to -1 only for stm32.
On other ports (esp32, unix) the default value 4 seems to work fine, but -1 is probably illegal for them.

embeddedt commented 3 years ago

The MicroPython docs are confusing regarding which one is supposed to be supported everywhere.

Construct a new timer object of the given id. Id of -1 constructs a virtual timer (if supported by a board).

My interpretation of this would be that -1 is a special feature not available everywhere, and that using zero or positive integers is more standard. However, STM32 only supports the use of a virtual timer, which runs contrary to that interpretation.

ESP32 simply forces the timer ID to be 0 or 1, if that is what index is referring to.

I don't know if there are any other ports which are actively developed which we could reference.

I think the best solution for now is to do what you suggested and have lv_utils automatically choose a sensible default for the target platform. A user who wants to control the exact ID will need to specify it manually based on what their platform supports.

amirgon commented 3 years ago

I think the best solution for now is to do what you suggested and have lv_utils automatically choose a sensible default for the target platform. A user who wants to control the exact ID will need to specify it manually based on what their platform supports.

@embeddedt I've changed lv_utils to set default timer_id to -1 on stm32. (https://github.com/lvgl/lv_binding_micropython/commit/4e871a85e50a3ffaa9ef483538c663f763ad89de)
Could you try this?

embeddedt commented 3 years ago

Looks like this also works.

amirgon commented 3 years ago

Seems complete.
Remaining: wait for https://github.com/micropython/micropython/pull/7401 integration and sync to upstream. Until then, that PR is merged locally on https://github.com/lvgl/lv_micropython/commit/cfbe065ea77eadba3139126e41361f0220bf39f9

Closing.

lvgl / lv_binding_micropython