Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Soft reboot failure #343

Open
MathyV opened this issue May 25, 2024 · 4 comments
Open

Soft reboot failure #343

MathyV opened this issue May 25, 2024 · 4 comments

Comments

@MathyV
Copy link

MathyV commented May 25, 2024

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 🤷

Here you can see how I solved it in my implementation:
MathyV/lvgl_esp32_mpy@e84196d

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:
MathyV/lvgl_esp32_mpy@7330fcc

@Carglglz
Copy link

Carglglz commented Jun 18, 2024

@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:

  • lv_binding_micropython:
diff --git a/gen/gen_mpy.py b/gen/gen_mpy.py
index 0f73a60..0ac30f2 100644
--- a/gen/gen_mpy.py
+++ b/gen/gen_mpy.py
@@ -1333,6 +1333,12 @@ void mp_lv_init_gc()
     }
 }

+void mp_lv_deinit_gc()
+{
+    mp_lv_roots = MP_STATE_VM(mp_lv_roots);
+    lv_free_core(mp_lv_roots);
+}
+
 #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..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
Copy link
Author

MathyV commented Jun 18, 2024

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

MathyV added a commit to MathyV/lvgl_esp32_mpy that referenced this issue Jun 18, 2024
LV_STDLIB_BUILTIN, while making soft-reboots work, introduces issues with indev
(and probably others) so we give up the convenience of soft-reboots in favor of
working firmwares.

Related: lvgl/lv_binding_micropython#343
@Carglglz
Copy link

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
Copy link

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 👍🏼

Carglglz added a commit to Carglglz/lv_binding_micropython that referenced this issue Jul 28, 2024
Properly handle root pointers on lvgl init/deinit which fixes
init error after a soft reset (see lvgl#343).
Carglglz added a commit to Carglglz/lv_binding_micropython that referenced this issue Jul 28, 2024
Properly handle root pointers on lvgl init/deinit which fixes
init error after a soft reset (see lvgl#343).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants