lvgl / lv_binding_micropython

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

Adding driver for Zephyr OS #132

Closed svetelna closed 3 years ago

svetelna commented 3 years ago

This PR introduces a driver for Zephyr OS so that Micropython+LVGL could be used as a Zephyr application. More information is in the enclosed README.

The lvgl.c file is taken from the Zephyr codebase and adjusted for this use case.

The PR was tested on i.MX RT1050, with ELCDIF display driver and FT5336 input driver from Zephyr OS.

Edit: Zephyr already supports LVGL (in C language) - driver abstraction layer is available in Zephyr codebase and LVGL as a Zephyr's module can be enabled with Kconfig.

For Zephyr + Micropython + LVGL use case are LVGL source files taken from the LV bindings project; thus LVGL has to be disabled in Zephyr's Kconfig configuration in order to prevent redefinitions. The reason is, that LVGL should in this use case use Micorpython's memory functions and not Zephyr's, so it seemed more appropriate to preserve LVGL + Micropython and not LVGL + Zephyr connection.

amirgon commented 3 years ago

Thank you for this contribution @svetelna !

With Micropython we try to move configuration to runtime as much as possible.
The idea is to have a single firmware image that could be used with multiple types of display. The selection of the display type and display parameters (such as SPI pins for example, landscape/portrait orientation etc.) is usually determined on runtime by Python code.

As an example, see ili9xxx driver for ESP32.
The user constructs a Python display driver on runtime (either ili9488 or ili9341) and passes it many configuration parameters.

How does this align with Micropython+Zephyr?
I noticed that you rely on many compile time macros (for example the device name "FT5336") instead of receiving them on runtime, and your Micropython module ZDD really contains only an "init" function that calls lvgl_init with no arguments. So it seems that the user has to configure everything on compile time and rebuild the firmware on every configuration change.

svetelna commented 3 years ago

@amirgon Thank you for your feedback. In Zephyr, needed drivers have to be chosen with Kconfig, thus have to be known before compilation. So, the user needs to make sure, the compiled image already contains the drivers, that he needs. In the context of this PR, these have to be stated in Micropython's Zephyr port's prj.conf file or similar. I think it would be possible to enable multiple drivers in Zephyr's Kconfig (but still chosen by the user before compilation) and then, as you pointed out, the ZDD.init could take as parameters names of the specific drivers. If it seems OK with you, I could try to change the implementation. Also, display resolution could be configurable, with some default values, possibly?

amirgon commented 3 years ago

I think it would be possible to enable multiple drivers in Zephyr's Kconfig (but still chosen by the user before compilation) and then, as you pointed out, the ZDD.init could take as parameters names of the specific drivers. If it seems OK with you, I could try to change the implementation.

Yes that sounds good.

Also, display resolution could be configurable, with some default values, possibly?

Yes. And other parameters. For example, where does the user specify through which pins and interfaces the display is connected? Other parameters include orientation and color mode, interface parameters such as speed, etc.


On Micropython v1.15 (released today) Zephyr port switched to build as pure CMake project.
Are you building Zephyr+Micropython with CMake?
Do you plan to integrate lv_micropython_bindings into this CMake build?

svetelna commented 3 years ago

Yes. And other parameters. For example, where does the user specify through which pins and interfaces the display is connected? Other parameters include >orientation and color mode, interface parameters such as speed, etc.

Zephyr uses devicetrees, so the pins and display connections need to be defined before compilation. With this in mind, I am rethinking, if driver names as parameters in lvgl_init are a good idea. Other display driver configurations are being done via Kconfig, also before the compilation.

On Micropython v1.15 (released today) Zephyr port switched to build as pure CMake project. Are you building Zephyr+Micropython with CMake? Do you plan to integrate lv_micropython_bindings into this CMake build?

I tested this PR on v1.14, thus with Makefiles. With the upgrade to CMake, only the README would have to be updated. I will update it.

amirgon commented 3 years ago

Zephyr uses devicetrees, so the pins and display connections need to be defined before compilation.

In my opinion this is unfortunate and doesn't align well with the Micropython way of configuring most things on runtime.

I tested this PR on v1.14, thus with Makefiles. With the upgrade to CMake, only the README would have to be updated. I will update it.

Instead of guiding the user how to edit the Makefile or CMake scripts, why don't you integrate these changes into the Zephyr port on lv_micropython?

In many cases lv_micropython is used directly to run Micropython+LVGL, and on other cases it could at least be a good reference.

svetelna commented 3 years ago

Sorry for delayed answer.

Instead of guiding the user how to edit the Makefile or CMake scripts, why don't you integrate these changes into the Zephyr port on lv_micropython?

I can do that, but it seems, that lv_micropython project uses Micropython version 1.14. I can either switch to cmake and integrate PR with Micropython 1.15 or stay with Makefile build and integrate the Makefile rules info Makefile in lv_micropython project. Which version is better fo you? Is lv_micropython going to upgrade to the v1.15 in the future?

In my opinion this is unfortunate and doesn't align well with the Micropython way of configuring most things on runtime.

That is true, in my opinion philosophies of Zephyr OS and Micropython differ a little.

embeddedt commented 3 years ago

MicroPython 1.15 will be used starting with LVGL 8; a beta version of the bindings is in dev-8.0.

svetelna commented 3 years ago

Thanks for pointing me out @embeddedt.

svetelna commented 3 years ago

Hello, I have just created a PR for the lv_micropython project: https://github.com/lvgl/lv_micropython/pull/25 Here the lv_bindings are integrated into Micropython v1.14.

I have also tried to integrate changes into Micropython 1.15 (with CMake build); however, this attempt has failed on the fact that the current Zephyr stable version (2.5) does not support LVGL 8. Thus the bindings between Zephyr display drivers and LVGL flushing and reading functions are invalid. Otherwise, I think that the solution works, and I can create a new pull request for the lv_micropython dev-8.0 branch, which can be merged later (when the LVGL 8 is supported by the Zephyr). Just let me know what would be OK for you.

amirgon commented 3 years ago

Otherwise, I think that the solution works, and I can create a new pull request for the lv_micropython dev-8.0 branch, which can be merged later (when the LVGL 8 is supported by the Zephyr). Just let me know what would be OK for you.

@svetelna LVGL v8 wasn't released yet, so it might be too early for LVGL v8 integration in Zephyr.
When Zephyr supports LVGL 8 feel free to open a new PR. By then we would probably already merge dev-8 branch into main on lv_micropython and lv_micropython_binding.

On the README you specify changes than need to be done in the Zephyr Micropython port. It's worth mentioning there that this is already implemented in lv_micropython (once we merge the lv_micropython PR), so the user is not required to do all that when using lv_micropython.
Is there anything the user still needs to do if he uses lv_micropython?

@embeddedt - Ok to merge this? or do you want more time to review this?

embeddedt commented 3 years ago

I think merging this is fine. It sounds like MIT and Apache are compatible enough, and also no one is likely to use that file outside of Zephyr anyway.

svetelna commented 3 years ago

@amirgon The user needs set properly the display and input driver entries in lvgl_driver.h (in lv_bindings/drivers/zephyr) and in specific-desk.prj (in micropython/ports/zephyr). Moreover, the lv_conf.h needs to be edited, to user Zephyrs LV_TICK_CUSTOM_INCLUDE.

I have already updated the readme file with information about lv_micropython project.

amirgon commented 3 years ago

Merged! Thank you for your contribution @svetelna!

Please consider adding a CI job that checks that Zephyr+Micropython+LVGL builds correctly.
Today we have such test for the unix port, and we are planning to add ESP32 / STM32 tests.

Personally I don't use Zephyr so without an automated test I would not be able to tell when Zephyr port breaks.

svetelna commented 3 years ago

Thanks for merging. I will try to look into the CI checks for the Zephyr port.