lvgl / lv_binding_micropython

LVGL binding for MicroPython
MIT License
237 stars 156 forks source link

Soft reboot failure #343

Open MathyV opened 1 month ago

MathyV commented 1 month ago

I wasn't sure whether to report this as an issue or not, so feel free to immediately close it, I just wanted to document this for others to find if they were experiencing the same issues as me.

TLDR: using the default lv_conf.h also present in this repository, I failed to have soft-reboots working properly, but I found a solution.

The first hurdle, getting to a clean state, was fairly easily taken by implementing finalizers and using the deinit() function. After that, I could completely tear down and rebuild my setup by manually calling the functions in REPL multiple times in a row and it would keep working.

The second problem wasn't so easy to debug: once I soft rebooted, the finalizers were called but it would always crash with a MemoryError. After a lot of scratching my head and a deep-dive into the memory allocation code of both LVGL and MicroPython I found finally that during deinit() the LVGL code is doing calls to realloc which are failing because the garbage collector doesn't want to hand out memory anymore. Whether this is a problem to be solved in LVGL or in MicroPython I'm not sure at this point.

The solution finally was pretty simple: stop using the LV_STDLIB_MICROPYTHON malloc implementation :shrug:

Here you can see how I solved it in my implementation: https://github.com/MathyV/lvgl_esp32_mpy/commit/e84196d20415162422b7ead386934419384a66d6

I don't know if this is the best (temporary?) solution but so far I haven't experienced any issues with it yet.

PS: after fixing this I also had issues with deinit() of machine.SPI in the ESP32 MicroPython implementation but that is definitely out of scope here, still, here's how I fixed it as an FYI: https://github.com/MathyV/lvgl_esp32_mpy/commit/7330fcc6bdc23e84a8e1bb8db0970650338f5940

Carglglz commented 1 week ago

@MathyV I was able to reproduce this

The second problem wasn't so easy to debug: once I soft rebooted, the finalizers were called but it would always crash with a MemoryError. After a lot of scratching my head and a deep-dive into the memory allocation code of both LVGL and MicroPython I found finally that during deinit() the LVGL code is doing calls to realloc which are failing because the garbage collector doesn't want to hand out memory anymore. Whether this is a problem to be solved in LVGL or in MicroPython I'm not sure at this point.

The solution finally was pretty simple: stop using the LV_STDLIB_MICROPYTHON malloc implementation 🤷

The issue is lvgl.deinit() is not freeing mp_lv_roots pointer, here is the diff that solves it:

+void mp_lv_deinit_gc() +{

- `lvgl`:

```diff 
diff --git a/src/lv_init.c b/src/lv_init.c
index cfc46cf15..c20f5331f 100644
--- a/src/lv_init.c
+++ b/src/lv_init.c
@@ -415,6 +415,10 @@ void lv_deinit(void)

     lv_initialized = false;

+#ifdef LV_GC_DEINIT
+    LV_GC_DEINIT();
+#endif
+
     LV_LOG_INFO("lv_deinit done");

 #if LV_USE_LOG

[EDIT] after some testing this doesn't solve it either. mp_lv_roots is used for lv_global_t struct and with lvgl.deinit only indevand display are being free.

static inline void _lv_cleanup_devices(lv_global_t * global)
{
    LV_ASSERT_NULL(global);

    if(global) {
        /* cleanup indev and display */
        _lv_ll_clear_custom(&(global->indev_ll), (void (*)(void *)) lv_indev_delete);
        _lv_ll_clear_custom(&(global->disp_ll), (void (*)(void *)) lv_display_delete);
    }
}

It seems like a lv_global_deinit function is what is needed? 🤔

MathyV commented 1 week ago

I just tested it and indeed your suggestions do not resolve the issue.

Carglglz commented 1 week ago

Whether this is a problem to be solved in LVGL or in MicroPython I'm not sure at this point.

This may be a bug in MicroPython or LVGL is allocating something that is out of GC reach...😕

from examples/usercmodule/subpackage/modexamplepackage.c

// The "initialised" state is stored on mp_state so that it is cleared on soft
// reset.
MP_REGISTER_ROOT_POINTER(int example_package_initialised);

But lvgl.init() is doing something that is not cleared on soft reset since the bug can be triggered even just calling it before reset, e.g.

>>> import lvgl as lv 
>>> lv.init()
>>>
MPY: sync filesystems
MPY: soft reboot
MicroPython v1.24.0-preview.40.g4434a9a2f8.dirty on 2024-06-18; PYBv1.1 with STM32F405RG
Type "help()" for more information.
>>> import basic
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "basic.py", line 59, in <module>
  File "testrunner.py", line 58, in run
  File "asyncio/core.py", line 1, in run
  File "asyncio/core.py", line 1, in run_until_complete
  File "asyncio/core.py", line 1, in run_until_complete
  File "testrunner.py", line 40, in _run
  File "testdisplay.py", line 247, in get_display
  File "testdisplay.py", line 53, in __init__
MemoryError: memory allocation failed, allocating 1869360762 bytes

Just soft-reset works:

>>>
MPY: sync filesystems
MPY: soft reboot
MicroPython v1.24.0-preview.40.g4434a9a2f8.dirty on 2024-06-18; PYBv1.1 with STM32F405RG
Type "help()" for more information.
>>> import basic

FRAME: 0 (0, 0, 240, 32, 23040)
d5c5d09cff879bb12cb926dc44bf10161cded58d2057806e7cbde536540b1421

FRAME: 1 (0, 32, 240, 32, 23040)
f281e1fce42dc013342ad8a4573d74874238d995e6dff46dc29a1d68b780f920

FRAME: 2 (0, 64, 240, 32, 23040)
46e2096b907947368d310929303a04005b39c4a278e3a7de2225c355b4522694

FRAME: 3 (0, 96, 240, 32, 23040)
46e2096b907947368d310929303a04005b39c4a278e3a7de2225c355b4522694

FRAME: 4 (0, 128, 240, 32, 23040)
424125778438a53da017c2dca09964ec2cec5ad4e2689038dd0e830125112fd8

FRAME: 5 (0, 160, 240, 32, 23040)
9abb7f9219bb7ccc8784119c784b1bf41c451f9957989fd2a9fc12a15606b1d0

FRAME: 6 (0, 192, 240, 32, 23040)
46e2096b907947368d310929303a04005b39c4a278e3a7de2225c355b4522694

FRAME: 7 (0, 224, 240, 32, 23040)
46e2096b907947368d310929303a04005b39c4a278e3a7de2225c355b4522694

FRAME: 8 (0, 256, 240, 32, 23040)
46e2096b907947368d310929303a04005b39c4a278e3a7de2225c355b4522694

FRAME: 9 (0, 288, 240, 32, 23040)
f546d8ae7340f5fb71e30358ef0d6f33a4f2d72946d9b312444b07fa9d659396
EVENT TEST:
RED PRESSED
GREEN PRESSED
BLUE PRESSED
>>>
Carglglz commented 1 week ago

OK this was tricky, it was the root pointers all along... 😓

I will make a PR but here is the diff meanwhile lv_binding_micropython

diff --git a/gen/gen_mpy.py b/gen/gen_mpy.py
index 0f73a60..f3dfaf6 100644
--- a/gen/gen_mpy.py
+++ b/gen/gen_mpy.py
@@ -1321,18 +1321,31 @@ static mp_obj_t mp_lv_obj_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t
 // Register LVGL root pointers
 MP_REGISTER_ROOT_POINTER(void *mp_lv_roots);
 MP_REGISTER_ROOT_POINTER(void *mp_lv_user_data);
+MP_REGISTER_ROOT_POINTER(int mp_lv_roots_initialized);

 void *mp_lv_roots;
+void *mp_lv_user_data;
+int mp_lv_roots_initialized = 0;

 void mp_lv_init_gc()
 {
-    static bool mp_lv_roots_initialized = false;
-    if (!mp_lv_roots_initialized) {
+    if (!MP_STATE_VM(mp_lv_roots_initialized)) {
+        // mp_printf(&mp_plat_print, "[ INIT GC ]");
         mp_lv_roots = MP_STATE_VM(mp_lv_roots) = m_new0(lv_global_t, 1);
-        mp_lv_roots_initialized = true;
+        mp_lv_roots_initialized = MP_STATE_VM(mp_lv_roots_initialized) = 1;
     }
 }

+void mp_lv_deinit_gc()
+{
+
+    // mp_printf(&mp_plat_print, "[ DEINIT GC ]");
+    mp_lv_roots = MP_STATE_VM(mp_lv_roots) = NULL;
+    mp_lv_user_data = MP_STATE_VM(mp_lv_user_data) = NULL;
+    mp_lv_roots_initialized = MP_STATE_VM(mp_lv_roots_initialized) = 0;
+
+}
+
 #else // LV_OBJ_T

 typedef struct mp_lv_obj_type_t {
diff --git a/lv_conf.h b/lv_conf.h
index c2d4e6c..e624057 100644
--- a/lv_conf.h
+++ b/lv_conf.h
@@ -299,7 +299,9 @@
 /*Garbage Collector settings
  *Used if LVGL is bound to higher level language and the memory is managed by that language*/
 extern void mp_lv_init_gc();
+extern void mp_lv_deinit_gc();
 #define LV_GC_INIT() mp_lv_init_gc()
+#define LV_GC_DEINIT() mp_lv_deinit_gc()

 #define LV_ENABLE_GLOBAL_CUSTOM 1
 #if LV_ENABLE_GLOBAL_CUSTOM

lvgl

diff --git a/src/lv_init.c b/src/lv_init.c
index cfc46cf15..1586a8e03 100644
--- a/src/lv_init.c
+++ b/src/lv_init.c
@@ -421,6 +421,13 @@ void lv_deinit(void)
     lv_log_register_print_cb(NULL);
 #endif

+
+#ifdef LV_GC_DEINIT
+    LV_GC_DEINIT();
+#endif
+
+return;
+
 }

Tested on stm32 and esp32

Be aware that lvgl.deinit() needs to be called before the soft reset for this to work.

@MathyV let me know if you test this 👍🏼