lvgl / lv_binding_micropython

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

Integration as external c module with unmodified micropython. #242

Open andrewleech opened 1 year ago

andrewleech commented 1 year ago

This PR provides changes that allow lv_binding_micropython to be used as an External C Module with un-modified / upstream micropython.

This takes advantage of the relativly new macro MP_REGISTER_ROOT_POINTER to remove the need to manually include the LV roots in the mpconfigport.h files.

Other port-specific settings can be added to micropython.mk and micropython.cmake

An example of usage to build a project can be found here: https://github.com/andrewleech/lvgl_mpy_example

Note: I've currently only tested on stm32 and unix - I have not yet built for esp32 or any other cmake based port.

It also includes an update to the binding generator script to support the new types syntax in current upstream micropython, for details see: https://github.com/micropython/micropython/wiki/Build-Troubleshooting#define_const_obj_type

amirgon commented 1 year ago

Thank you @andrewleech for this contribution!
It would be very convenient to use lv_binding_micropython as an external module.

I would like to adapt this change in lv_micropython.
We use lv_micropython for GitHub workflows in lv_binding_micropython and in LVGL. lv_micropython is also used directly in many projects. It contains an important patch that was not accepted yet in Micropython (for very long time) and is critical due to the size of LVGL library API.
For that reason (and future patches like this) I think it's best to keep lv_micropython as a fork of Micropython and not use Micropython directly like you did.

I'm trying to keep lv_micropython aligned to official Micropython releases. Latest release v1.19 does not yet contain these breaking API changes that you fixed (DEFINE_CONST_OBJ_TYPE), so options are:

andrewleech commented 1 year ago

Hi @amirgon , ah yes I'd been watching that PR and it certainly looks like a worthy change, hopefully it can be ready for merge soon! I see there's a few failing unit tests there, that on its own would be enough to block merge :-( perhaps adding a #define MICROPY_PY_* setting to enable/disable the new functionality would be enough to get it across the line, disabling it where it fails (for whatever reason) and/or on smaller ports.

But in the mean time yes there's no harm in working from a fork of upstream (with minimal change) I often do this at work when I'm using new features I'm writing!

I suspect a new realease of Micropython may be imminent, so it's probably fine to hold off with this until that's released - there's probably a few things to clean up with the PR anyway.

Most notably; I'm failing to get esp32 to build. I've fixed the way root pointers are handled and updated the cmake scripts enough to support the rp2 build but am getting tripped up on the esp32 one. It's giving cmake failures like:

$ cd lvgl_mpy_example
$ make esp
...
-- Configuring done
CMake Error at /opt/esp/idf/tools/cmake/project.cmake:428 (add_executable):
  Cannot find source file:

    /code/lib/micropython/ports/esp32/build-GENERIC/lv_mp.c

however adding some status messages shows that the lv_bindings() function is being run correctly with the output file /code/lib/micropython/ports/esp32/build-GENERIC/lv_mp.c but that's apparently not enough to satisft cmake. Perhaps it's got something to do with https://stackoverflow.com/a/57825938 but I'm not that familiar with cmake to follow it all.

M-D-777 commented 1 year ago

Hi, @andrewleech Any updates on this Topic? I have tested https://github.com/andrewleech/lvgl_mpy_example for esp32, but failed. Someone here had the same problem, https://forum.lvgl.io/t/build-lv-bindings-with-micropython-nightly-for-esp32/10967 Looking forward to your update,Thanks.

CMake Error at /project/micropython/py/usermod.cmake:9 (get_target_property):
  get_target_property() called with non-existent target "lvgl_interface".
Call Stack (most recent call first):
  /project/micropython/py/usermod.cmake:24 (usermod_gather_sources)
  /project/micropython/py/usermod.cmake:24 (usermod_gather_sources)
  /project/micropython/py/usermod.cmake:47 (usermod_gather_sources)
  main/CMakeLists.txt:14 (include)

CMake Error at /project/micropython/py/usermod.cmake:15 (get_target_property):
  get_target_property() called with non-existent target "lvgl_interface".
Call Stack (most recent call first):
  /project/micropython/py/usermod.cmake:24 (usermod_gather_sources)
  /project/micropython/py/usermod.cmake:24 (usermod_gather_sources)
  /project/micropython/py/usermod.cmake:47 (usermod_gather_sources)
  main/CMakeLists.txt:14 (include)

CMake Error at /project/micropython/py/usermod.cmake:21 (get_target_property):
  get_target_property() called with non-existent target "lvgl_interface".
Call Stack (most recent call first):
  /project/micropython/py/usermod.cmake:24 (usermod_gather_sources)
  /project/micropython/py/usermod.cmake:24 (usermod_gather_sources)
  /project/micropython/py/usermod.cmake:47 (usermod_gather_sources)
  main/CMakeLists.txt:14 (include)

Found User C Module(s): usermod_lv_bindings, lvgl_interface
CMake Error at /project/esp-idf/tools/cmake/component.cmake:470 (add_library):
  add_library cannot create target "__idf_main" because another target with
  the same name already exists.  The existing target is a static library
  created in source directory
  "/project/micropython/ports/esp32/main".  See
  documentation for policy CMP0002 for more details.
Call Stack (most recent call first):
  main/CMakeLists.txt:158 (idf_component_register)
andrewleech commented 1 year ago

I'm pretty sure I got rp2 working, but esp cmake is weirdly different to rp2 and I never managed to make it build.

I generally really dislike esp. I've never had a good experience with them, always crashing / Guru Meditation Error anytime I try to do anything interesting. I'm also not a huge cmake fan, things there just seem even harder to get working than Makefile, though maybe that's just me.

As such I'm really not motivated to get esp working... I'm always going to choose rp2 or stm for my projects.

But as for the MR, well micropython v1.20 hasn't been released yet (there's some upstream rp2 ble issues getting in the way) but once it does I'll probably try to get this rebased and ready to go... for some platforms at least!

amirgon commented 1 year ago

@andrewleech I'm starting to look at this in the context of #272.

Why did you choose to register root pointers with MP_STATE_VM(lvgl) = m_new0(lvgl_root_pointers_t, 1); instead of calling the more standard MP_REGISTER_ROOT_POINTER?

I imagined calling the LV_ITERATE_ROOTS in some init function, in order to call MP_REGISTER_ROOT_POINTER for each global. What do you think?

Also, instead of tweaking lv_init in gen_mpy.py, a cleaner solution could be to call it explicitly from lv_init in LVGL.

My plan is to first align to Micropython v1.20.0 without an external C module.
Once this is ready we could try rebasing your PR. That means the alignment to the new types syntax and to the new way of registering root pointers need to be done first on the other PR I'm working on. Once it's ready, your PR could focus specifically on the integration as external C module.

andrewleech commented 1 year ago

Sounds great @amirgon !

I'm pretty sure when I first did this, MP_REGISTER_ROOT_POINTER did not yet exist, it came later. It's definitely the right thing to use here.

I'm not sure I follow your other comments about init, but I was aiming to not change the current end user usage of lvgl with my PR, trying to ensure any init things were called automatically at an appropriate time.

Having a seperate / manual init function might make sense, certainly of there's times you might want to reuse it after soft reset etc. As long as things don't segfault if it's accidentally not run beforehand.

Working on that PR was the first time I used lvgl at all though so I was figuring out a lot as I went through it, probably did some things wrong!

amirgon commented 1 year ago

@andrewleech - lv_binding_micropython is now aligned to Micropython v1.20.0, see https://github.com/lvgl/lv_binding_micropython/issues/272, https://github.com/lvgl/lv_micropython/pull/65

Would you like to rebase this PR?
I understand you got this working for unix/stm32/rp2 ports, right?
Do you have any plans for esp32?

andrewleech commented 1 year ago

Nice work! Yep I'll rebase mine when I get a chance.

I did have Unix/stm tested and working... though stm was quite unstable for me.

That's probably something wrong with the driver I wrote for my board. Unix was also not working very well for me due to the known nvidia driver issue iirc. I don't think I actually tested rp2 due to lack of hardware. I tried and tried to get esp32 to work but gave up, the idf cmake trickery got the better of me.

dmazzella commented 8 months ago

@andrewleech @amirgon progress on this? need help to complete some tasks?

andrewleech commented 8 months ago

No I haven't rebased / reworked it for the updated micropython yet; too many projects / too little time.

I never got it working with esp either.

I'm definitely keen to still see this updated and working!

Carglglz commented 2 months ago

I'm definitely keen to still see this updated and working!

@andrewleech Thanks to this PR I got it working with latest MicroPython v1.23.0-preview-324-gd11ca092f7 in unix and esp32 (this one was tricky) too , I will try to open a PR this week in case you want to review it 👍🏼

Here are the changes https://github.com/Carglglz/lv_binding_micropython/tree/userCmod

MathyV commented 2 months ago

@Carglglz I tried your code (on esp32) and have some issues with it:

diff --git a/micropython.cmake b/micropython.cmake
index 5343ef0..2ff55da 100644
--- a/micropython.cmake
+++ b/micropython.cmake
@@ -7,8 +7,9 @@ idf_build_set_property(LV_MICROPYTHON 1)
 idf_build_component(${CMAKE_CURRENT_LIST_DIR}/lvgl)
 idf_build_set_property(COMPILE_DEFINITIONS "-DLV_KCONFIG_IGNORE" APPEND)
 separate_arguments(LV_CFLAGS_ENV UNIX_COMMAND $ENV{LV_CFLAGS})
-list(APPEND LV_CFLAGS ${LV_CFLAGS_ENV} -Wno-unused-function)
+list(APPEND LV_CFLAGS ${LV_CFLAGS_ENV})
 idf_build_set_property(COMPILE_DEFINITIONS "${LV_CFLAGS}" APPEND)
+idf_build_set_property(COMPILE_OPTIONS "-Wno-unused-function" APPEND)
 idf_build_set_property(SRCS "${LV_SRC}" APPEND)
 idf_build_set_property(INCLUDE_DIRS "${LV_INCLUDE}" APPEND)
Carglglz commented 2 months ago

@MathyV thanks for testing this!

In micropython.cmake -Wno-unused-function is added to COMPILE_DEFINITIONS which introduces a spurious -D in front of the gcc command which (on my system at least) fails to build. Since it is not a definition, it should be added to COMPILE_OPTIONS

That makes sense, my knowledge in Cmake / esp-idf is limited, so I'm not 100 % sure that everything in micropython.cmake is right.

I notice you commented a lot of stuff out in the mkrules_usermod.cmake file which prevents the esp32 drivers from building. How did you test on esp32 and with which driver? Everything builds and links fine with your code and I can import the lvgl modules but I haven't immediately found a driver that works for my (st7789) display in combination with LVGL.

Yes I tried to implement those drivers in the build too, but then I realised that they were written with IDF-v4.x and now MicroPython is at IDF-v5.x. So they need to be updated first, which right now it is out of my scope.

In the meantime I've been looking at the drivers at https://github.com/kdschlosser/lvgl_micropython and https://github.com/bdbarnett/mpdisplay, which seem promising.

Let me know if you get it working.

I'm mostly developing/testing on unix port, on a esp32 I've only tested this with a "dummy" display, i.e. just writing the buffers and it seems to work... see https://docs.lvgl.io/master/porting/display.html#

Carglglz commented 1 month ago

I confirm this is working on esp32 with the drivers at https://github.com/bdbarnett/mpdisplay, in any case I think the drivers shouId be independent from the bindings so people can choose/implement its own... I will try to make a PR soon 👍🏼

kdschlosser commented 1 month ago

OK so the only thing you are wanting to do is provide LVGL as a user C module. The user would be responsible for writing and implementing their own drivers for the bus and for the display IC and proper handling of flushing the display buffer.

I was expecting the functionality to be the same as it is right now.

Carglglz commented 1 month ago

OK so the only thing you are wanting to do is provide LVGL as a user C module. The user would be responsible for writing and implementing their own drivers for the bus and for the display IC and proper handling of flushing the display buffer.

Yes exactly, and that's the logic behind the specific test split in api, display and indev at #341.

The functionality doesn't change at all if using lv_micropython.

In the future example board definitions could be added to showcase how to add the .py or USER_C_MODULE set of drivers, but for now I think this is a good step in the right direction. 👍🏼