Skip to content

(fix) lvgl deinit on soft-reset #383

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Carglglz
Copy link
Contributor

@Carglglz Carglglz commented May 18, 2025

Include lv_conf.h<include/lv_mp_port_soft_reset.h> in mpconfigboard.h so lvgl deinit is called on soft-reset, so calling lvgl.deinit() is no longer necessary.

This closes #343

EDIT: esp32 port requires Carglglz/micropython@0bd03de

Copy link
Collaborator

@PGNetHun PGNetHun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I think we should define that MICROPY_PORT_DEINIT_FUNC macro in other place, not in lv_conf.h and then explicitly include that config file in mpconfigboard.h.
There must be an other way to include it automatically when LVGL is also compiled into firmware.

@Carglglz Carglglz force-pushed the fix-soft-reset branch 2 times, most recently from 64e0e55 to 24fc870 Compare July 14, 2025 15:25
@Carglglz
Copy link
Contributor Author

Carglglz commented Jul 14, 2025

This seems to be a better solution, I've tested stm32 port and it seems to work, esp32 will be next, so do not merge this yet...

Also I had to update CI make command for esp32, I guess you added USER_C_MODULES in mpconfigboard.cmake for ESP32_GENERIC_S3 board in lv_micropython?

@Carglglz
Copy link
Contributor Author

@PGNetHun I've just tested the esp32 port and it seems to be working as expected, so it is ready for review/merge now 👍🏼

@Carglglz Carglglz requested a review from PGNetHun July 15, 2025 00:35
@Carglglz Carglglz force-pushed the fix-soft-reset branch 2 times, most recently from c80bd34 to 25b8d69 Compare July 15, 2025 01:27
Copy link
Collaborator

@PGNetHun PGNetHun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some comments, suggestion where to define to call deinit function.
What do you think?

@@ -83,12 +83,14 @@ jobs:
make -j $(nproc) -C ports/rp2 BOARD=RPI_PICO USER_C_MODULES=../../user_modules/lv_binding_micropython/micropython.cmake

# ESP32 port
# USER_C_MODULES defined in mpconfigboard.cmake
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this line, it is incorrect.
In ESP32 port USER_C_MODULES is defined in esp32_common.cmake file:
https://github.com/lvgl/lv_micropython/blob/master/ports/esp32/esp32_common.cmake#L14

@@ -434,6 +434,9 @@ extern void mp_lv_deinit_gc();
#define LV_GC_INIT() mp_lv_init_gc()
#define LV_GC_DEINIT() mp_lv_deinit_gc()

// To enable deinit on soft-reset add in mpconfigboard.h
Copy link
Collaborator

@PGNetHun PGNetHun Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you define deinit here with a new LV_ config?

Like:

#define LV_ENABLE_DEINIT_ON_SOFT_RESET 1
#ifdef LV_ENABLE_DEINIT_ON_SOFT_RESET
   extern void mp_deinit_lvgl_mod();
   #define MICROPY_PORT_DEINIT_FUNC mp_deinit_lvgl_mod()
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't matter where you define it, as long as you include MICROPY_PORT_DEINIT_FUNC macro definition in mpconfigboard.h, so this way you would need to include lv_conf.h as I did before ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this file if deinit is defined in lv_conf.h like mentioned in other comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I think we should define that MICROPY_PORT_DEINIT_FUNC macro in other place, not in lv_conf.h and then explicitly include that config file in mpconfigboard.h.

One way or the other, so which one ?

Include `<include/lv_mp_port_soft_reset.h>` in `mpconfigboard.h` so lvgl deinit is called on
soft-reset, so calling `lvgl.deinit()` is no longer necessary.

This closes lvgl#343
@Carglglz Carglglz requested a review from PGNetHun July 17, 2025 00:54
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

Successfully merging this pull request may close these issues.

Soft reboot failure
2 participants