Repository navigation
fix(gen): support MicroPython's renamed mp_obj_int_to_bytes() API - #411
andrewleech wants to merge 2 commits into
Conversation
9a4d2d1 to
5309970
Compare
|
Confirming independent testing on the unix port (you noted you hadn't tried it): rebuilt Side note: this is very likely also why the |
|
Confirming this also fixes the unix port, since the description notes it wasn't tested there. Applied the same guard to Built and ran it repeatedly (dozens of rebuilds over the last day), including live LVGL UI code -- buttons, labels, style colors, indev state -- that routes through So: confirmed working on unix in addition to the mimxrt testing already in the description. |
|
Confirming this also fixes the unix port (the description notes it wasn't tested there): built Worth calling out how live this bug currently is: CI on We've got a follow-up PR (#413) building on top of this one - adds LVGL's native Wayland driver, and separately fixes a regression that's kept every desktop window driver (SDL included) out of the Python bindings since May. Rebasing that once this lands. |
MBM-PR: 411 MBM-URL: lvgl#411 MBM-Author: andrewleech
MicroPython commit e00daa3a ("py/binary,objint: Add overflow checks and
int.to_bytes(signed=True)") renamed mp_obj_int_to_bytes_impl() to
mp_obj_int_to_bytes() with a new signature (adding is_signed and
overflow_check parameters), for the 1.29 release. The generated binding
called the old name unconditionally in mp_obj_get_ull(), so building against
1.29 or later failed to link with an undefined reference to
mp_obj_int_to_bytes_impl.
Guard the call on MICROPY_VERSION so the binding builds against both APIs.
5309970 to
2fe3619
Compare
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="micropython.mk">
<violation number="1" location="micropython.mk:93">
P2: This switches the generator input from `lvgl_private.h` to `lvgl.h`, so private LVGL symbols are now only generated when `LV_USE_PRIVATE_API` is enabled. The default `lv_conf.h` enables it, so the default build is fine, but a user config that disables it will silently drop those bindings. Please note this change in the PR description, since it is not covered by the stated purpose of the PR.</violation>
</file>
<file name=".github/workflows/micropython_lvgl_ci.yml">
<violation number="1" location=".github/workflows/micropython_lvgl_ci.yml:91">
P2: This removes the LVGL user module from the ESP32 build, so the job no longer compiles the binding. Restore the `USER_C_MODULES=../../user_modules/lv_binding_micropython/micropython.cmake` argument or include the module another way.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| -I $(LVGL_BINDING_DIR)/stubs/include \ | ||
| $(CFLAGS_USERMOD) \ | ||
| $(LVGL_DIR)/lvgl_private.h | $(LV_PP_FILTER) > $(LVGL_PP) | ||
| $(LVGL_DIR)/lvgl.h | $(LV_PP_FILTER) > $(LVGL_PP) |
There was a problem hiding this comment.
P2: This switches the generator input from lvgl_private.h to lvgl.h, so private LVGL symbols are now only generated when LV_USE_PRIVATE_API is enabled. The default lv_conf.h enables it, so the default build is fine, but a user config that disables it will silently drop those bindings. Please note this change in the PR description, since it is not covered by the stated purpose of the PR.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At micropython.mk, line 93:
<comment>This switches the generator input from `lvgl_private.h` to `lvgl.h`, so private LVGL symbols are now only generated when `LV_USE_PRIVATE_API` is enabled. The default `lv_conf.h` enables it, so the default build is fine, but a user config that disables it will silently drop those bindings. Please note this change in the PR description, since it is not covered by the stated purpose of the PR.</comment>
<file context>
@@ -90,7 +90,7 @@ $(LVGL_MPY): $(ALL_LVGL_SRC) $(LVGL_BINDING_DIR)/gen/gen_mpy.py
-I $(LVGL_BINDING_DIR)/stubs/include \
$(CFLAGS_USERMOD) \
- $(LVGL_DIR)/lvgl_private.h | $(LV_PP_FILTER) > $(LVGL_PP)
+ $(LVGL_DIR)/lvgl.h | $(LV_PP_FILTER) > $(LVGL_PP)
$(Q)$(PYTHON) $(LVGL_BINDING_DIR)/gen/gen_mpy.py -M lvgl -MP lv -MD $(LVGL_MPY_METADATA) -E $(LVGL_PP) $(LVGL_DIR)/lvgl.h > $@
</file context>
| source tools/ci.sh && ci_esp32_idf_setup | ||
| source tools/ci.sh && ci_esp32_build_common | ||
| make -C ports/esp32 BOARD=ESP32_GENERIC_S3 USER_C_MODULES=../../user_modules/lv_binding_micropython/micropython.cmake | ||
| make -C ports/esp32 BOARD=ESP32_GENERIC_S3 |
There was a problem hiding this comment.
P2: This removes the LVGL user module from the ESP32 build, so the job no longer compiles the binding. Restore the USER_C_MODULES=../../user_modules/lv_binding_micropython/micropython.cmake argument or include the module another way.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At .github/workflows/micropython_lvgl_ci.yml, line 91:
<comment>This removes the LVGL user module from the ESP32 build, so the job no longer compiles the binding. Restore the `USER_C_MODULES=../../user_modules/lv_binding_micropython/micropython.cmake` argument or include the module another way.</comment>
<file context>
@@ -88,7 +88,7 @@ jobs:
source tools/ci.sh && ci_esp32_idf_setup
source tools/ci.sh && ci_esp32_build_common
- make -C ports/esp32 BOARD=ESP32_GENERIC_S3 USER_C_MODULES=../../user_modules/lv_binding_micropython/micropython.cmake
+ make -C ports/esp32 BOARD=ESP32_GENERIC_S3
# Unix port
</file context>
| make -C ports/esp32 BOARD=ESP32_GENERIC_S3 | |
| make -C ports/esp32 BOARD=ESP32_GENERIC_S3 USER_C_MODULES=../../user_modules/lv_binding_micropython/micropython.cmake |
Summary
MicroPython 1.29 renamed
mp_obj_int_to_bytes_impl()tomp_obj_int_to_bytes()and changed its signature. The generated LVGL binding still called the old helper, so it no longer builds against current MicroPython. This updates the generated call while retaining the older API path for MicroPython versions before 1.29.lv_micropython#106upgrades the pinned MicroPython from v1.24.1 to v1.29.0 and needs this fix for its LVGL Unix build. The guard keeps the existing helper for the current v1.24.1 pin and selects the new API at v1.29.0, so #411 can merge before the pin is upgraded.Testing
Built the binding into an i.MX RT1176 firmware against MicroPython v1.29.0; it links. The pre-v1.29 branch was checked against a v1.24 header and selects the old call. Reviewers also built the Unix usermodule against a current MicroPython checkout reporting
sys.implementation.version == (1, 29, 0, 'preview')and exercised imports, object creation and event callbacks. The current API test workflow still has three failures becauselv.is_initializedis missing, which this PR does not add.Generative AI
I used generative AI tools when creating this PR, but a human has checked the code and is responsible for the code and the description above.