search

lvgl / lv_binding_micropython

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

8bit and 16bit parallel display support #254

Closed kdschlosser closed 1 year ago

kdschlosser commented 1 year ago

I have written a rudimentary Micropython driver for the ESP32 using displays that have either an 8bit or a 16bit parallel interface. It does work, cleaned up and refactored to get the best performance. C code is not my strong area and if anyone is willing to help out with it, it would be appreciated.

The driver is for raw 8 and 16 bit displays. what I mean by that is a display that does not need to have column and page addresses set, so no "commands" sent at all only data. This can be used as a stepping off point for displays that do need to have commands sent either when initializing or when flushing the buffer. It has been tested on a MakerFabs 4.3" 16bit parallel display with an ESP32-S3 as the MCU. The display is using the HX8264 interface IC and does not have any commands that get sent when the buffer is flushed. only a single pin needs to be turned on and off to indicate a change from command mode to data mode.

You can read about it here. It is near the end of the topic.

https://forum.lvgl.io/t/errors-building-lvgl-micropython-for-esp32-s3-but-esp32-works/10462/28

I know the parallel displays has come up more than a few times and I feel it would be a worthy addition to the binding.

Just as a note. I also wrote a driver for the ST7796 display IC using SPI and also the FT6x36 touch interface IC. I will be submitting a PR in the next week or so for both of these. The touch IC and the display IC are used on the WT32-SC01 dev kit made by wireless-tag. SeeedStudio sells a rebranded version of the same thing. They are also pretty common ICs that are used in general.

I will also be writing a touch driver for the GT911 touch IC in the next few days.

amirgon commented 1 year ago

I know the parallel displays has come up more than a few times and I feel it would be a worthy addition to the binding.

You are welcome to open a PR. In general, pure Python drivers are preferred when possible.

kdschlosser commented 1 year ago

while I know that pure python drivers would be ideal the esp_lcd component is not exposed to micropython. I tried adding to get exposed to micropython and all kinds of errors come up when the gen_mpy script runs. That is beyond my knowledge on how to fix because of there being no documentation on what the requirements are for the gen_mpy script to run properly.

The gen_mpy script was also written by a C developer and not someone that is strong in python. The code is difficult to read and difficult to understand what is going on. It cannot be looked at to determine what needs to be done in order to get the component added so it is exposed in micropython.

This specific driver would be well suited to add to micropython in native python code because there is no iteration over the frame buffer that needs to get done when flushing the frame buffer. That is all handled internally in the esp_lcd component of the IDF. but unless the functions and structures needed get exposed to the python side of things it is not going to be possible.

kdschlosser commented 1 year ago

I was surprised to see that the esp_lcd component was not available in espidf. so none of the LCD handling functions, structures and constants that are available in the espidf can be used. I would have thought that stuff would have existed due to the nature of lvgl and what it does.

kdschlosser commented 1 year ago

I am also well aware that I can open a PR for it. I rather open a PR for finished code instead of opening a PR and asking for help finishing it. That is what I was doing here was asking for help with polishing it and making it work better, seeing if anyone was interested in lending a hand with actual code writing and or testing.

amirgon commented 1 year ago

I tried adding to get exposed to micropython and all kinds of errors come up when the gen_mpy script runs.

The binding script can handle many things, but not everything.
In case it cannot process the entire esp_lcd API, you can create a dedicated C wrapper to expose only what is needed.
In general, simple structs and functions can be processed by the script usually without special problems.

If you have specific error you are seeing with the binding script, please provide a Minimal, Reproducible Example so we could take a look.

kdschlosser commented 1 year ago

OK I got the binding to add the esp_lcd component. YAY.. Kind of... LOL...

I ran across a problem I am not sure how to tackle.

I need to call the __containerof macro and I know this is not able to be added directly so I created a function that uses the macro and returns the structure from that macro. No matter what I do I keep on getting the same error that points to the line that has that macro being used and it saying before: { which is a pretty nondescript error message. I am sure it has to do with whatever is being set in place by the macro. Unfortunately I have no idea what that is because I do not know where the macro is. It is not defined in the ESPIDF so it has to be defined somewhere in the libraries the compiler uses. I have searched the internet and I do not come up with a macro named __containerof but there are ones called container_of and containerof I just do not know if they are the same as the one I am dealing with.

kdschlosser commented 1 year ago

I would love to provide you with a code example but the problem is there is a whole lot that has been changed to get everything to work. First off the SOC being used has to support the esp_lcd component which are the SOC's that set the SOC_LCD_RGB_SUPPORTED macro.

I believe that to be the ESP32-C3, C6, S2 and S3 series SOC's. the current version of the binding doesn't support those SOC's. I had to make a lot of changes to get support for those SOC's. I am not sure what else __containerof can be used on to test with.

amirgon commented 1 year ago

OK I got the binding to add the esp_lcd component. YAY.. Kind of... LOL...

Great!

I need to call the __containerof macro and I know this is not able to be added directly so I created a function that uses the macro and returns the structure from that macro.

The __containerof macro is just a way to get a pointer to a containing struct. If you know struct B is a field in struct A, and you have a pointer to an instance of struct B, you can get a pointer to struct A by knowing the offset of the struct B field in struct A.

You can find more explanations and examples on the web, for example https://stackoverflow.com/questions/32310148/use-of-macro-container-of-with-struct-member

You cannot use __containerof directly, so creating a function that calls it is a viable solution.
There might be a way to do it in Python, but it would be hacky and involve casting (__cast__ function of the struct type can convert a pointer to an instance of that struct)

No matter what I do I keep on getting the same error that points to the line that has that macro being used and it saying before: { which is a pretty nondescript error message.

I really can't help without more information. a Minimal, Reproducible Example could help.

the current version of the binding doesn't support those SOC's. I had to make a lot of changes to get support for those SOC's.

So maybe the first step is to fix lv_micropython to support those SOCs. There is an open PR to support esp32-s3 https://github.com/lvgl/lv_binding_micropython/pull/243 but unfortunately it looks abandoned. Looks like it needs some cleanup, adding a test to GitHub workflow etc.
There is also a sponsored issue for supporting ESP32-S2, ESP32-C3, ESP32-S3: https://github.com/lvgl/lv_binding_micropython/issues/227

It looks like some people created private versions of lv_micropython with ESP32 SOCs but unfortunately are not making an effort to contribute them back upstream.

kdschlosser commented 1 year ago

Yeah I started to make modifications so a PC could be done but the requirements of having to have different SOC's and providing videos of it working yadda yadda yadda was just too much to deal with. I know it compiles for the ESP32 and the S3 dunno about the S2, C2, C3 or C6 tho. I also do not have a viable display to test with. Last display I ordered I got the shaft on. It shows the pins for the touch and they are exposed but there is no touch IC on the board. I have not gotten around to ordering another.

I would have to spend 75 bucks in hardware to get the 100 prize. hardly seems worth the effort for the 25 bucks.

There are folks using my fork for the S3. in the link to the LVGL forum thread the guy I wrote the driver for is using it.

amirgon commented 1 year ago

I would have to spend 75 bucks in hardware to get the 100 prize. hardly seems worth the effort for the 25 bucks.

Is this all about the money? Maybe it was a mistake to sponsor this issue at all.

My hope is that people building on our work would be willing to contribute back their improvements. That's part of the Free Software philosophy. As of today, 100% of the work on lv_micropython and lv_binding_micropython was done by volunteers without any money involved. This is the first time we thought of offering money and define criteria for receiving it, but maybe it pushes back people from just opening a PR and contributing back their work.

kdschlosser commented 1 year ago

It's not about the money. But there is already a PR for the changes and nothing has been done with it. The person that made the PR did not provide any videos or anything like that and I thought that would be the reason why it has not been reviewed, tested and added. It has been there for a long while too.

Money or no money It sounds like in order to get the PR accepted I would have to open up my wallet and buy all kinds of hardware in order to make videos to get the PR approved. Going through all of that is a large amount of additional work that would need to be done. While I like the idea of having the dollars to get it done the problem is the hardware that would need to be bought in order to get the dollars. It makes it not worth all the effort by time things are said and done. See what I am saying?

Would need to buy an SPI LCD screen, an ESP32-S2 dev kit, ESP32-C2 dev kit, ESP32-C3 dev kit, ESP32-C6 devkit

so 20 for the screen, and 15 for each of the dev kits. That's 80 dollars that would need to get spent to be able to get everything put together to collect the 100 bucks. That's not taking into consideration having to get a devkit that has SPIRAM and one that doesn't. Gotta make sure it will run with a limited mount of memory.

I understand that the idea is to light a fire under some peoples backsides and money is usually a great motivator., 20 bucks really isn't all that much of a fire starter.

I mean look at the amount of work that needs to be done. support for multiple version of the IDF fixing the test cases so they work fix all the other components that currently only work with the ESP32 Fix all of the currently coded display drivers so they will all work with the new SOC's fix the documentation provide videos showing LVGL running on each of the newly supported boards

That is a large amount of work. It's also a massive amount to all have in a single PR as well. What you should offer those payouts on is documentation and documentation alone. I have not met a single developer that has said they like doing the documentation. I know that the developers that have contributed thus far don't. 2/3rds of LVGL is not documented and there isn't a whole lot of the binding that is documented. I don't think there is any documentation at all for the API.

I would be more then happy to work with someone that is familiar with the gen_mpy script to give it an update so that it can be used in conjunction with the doxygen xml output to build stub files for the binding. In those stub files would be the docstrings and sphinx would be able to read them simply by changing the file extension to .py and feeding that into sphinx. Then you would have the API portion of the documentation in alignment with LVGL and it would change when LVGL changes.

I think the issue is the scope of the work is so large. I glazed over when reading it thinking holy cow that is a whole lot to do. You are taking at the very least 50 hours of work to get it done. Probably a whole lot more than that. I am better off then a lot of people because I have a machine that is able to compile the LVGL MicroPython binding in 42 seconds. for someone that has to wait 2, 3 or even 4 minutes only to find out what they changed didn't work.. That kinda sucks. I can get by with C development, I am not all that fantastic at it. I probably compiled 40 times trying different things to figure out how to get integer macros to be exposed and how the naming convention works and then probably another 20 or 30 times messing about with the __containerof which still doesn't want to work.

And I think I just figured out why it is not working. I have the function in the header file and I should only declare it there and have the actual function in the c file. I am going to try that now.

kdschlosser commented 1 year ago

and that was the problem!!! I cannot believe I didn't see it before. I searched the espidf for --containerof and I just noticed it is only used in c files.

amirgon commented 1 year ago

But there is already a PR for the changes and nothing has been done with it. The person that made the PR did not provide any videos or anything like that and I thought that would be the reason why it has not been reviewed, tested and added. It has been there for a long while too.

No. That PR is not merged yet because it needs some cleanup (simplifying mkrules.cmake), tests added etc.
I would do it myself if I had more time and if I had a ESP32-S3 board.

Money or no money It sounds like in order to get the PR accepted I would have to open up my wallet and buy all kinds of hardware in order to make videos to get the PR approved. Going through all of that is a large amount of additional work that would need to be done. While I like the idea of having the dollars to get it done the problem is the hardware that would need to be bought in order to get the dollars. It makes it not worth all the effort by time things are said and done. See what I am saying?

Not really. If you open a PR for adding only the SOC you are working with, you can certainly get it accepted.
The work that is required from you is only to clean up your code, make it generic enough, go through a code review and probably make some changes/fixes usually around making the code clean, generic, easier to maintain etc. And add some test to GitHub workflow to make sure this SOC is built correctly. (Only the unix port really runs anything. All the other tests only make sure the build is not broken).

Would need to buy an SPI LCD screen, an ESP32-S2 dev kit, ESP32-C2 dev kit, ESP32-C3 dev kit, ESP32-C6 devkit

If you are not after the sponsored issue money, then No, I don't think you are expected to buy anything in order to open a PR. Just open a PR for what you got, if you would like to contribute.

I mean look at the amount of work that needs to be done.

The sponsorship is limited to what LVGL LLC can donate. It's not much, and the work involved is big as you mentioned.
So maybe we should really consider closing https://github.com/lvgl/lv_binding_micropython/issues/227 if further funding is not possible. @kisvegabor - what do you think?

I don't think there is any documentation at all for the API.

The Micropython API is auto-generated from the C API, which is well documented in my opinion. The Micropython API maps pretty directly to the C API (with a few exceptions) so it's pretty easy to figure out how it works. And there are lots of examples you can learn from too.
There is information in the READMEs and in LVGL docs about the Micropython bindings, but you are right, we can always improve documentation. If anyone wants to do that - just open a PR!

In those stub files would be the docstrings and sphinx would be able to read them simply by changing the file extension to .py and feeding that into sphinx.

Today the binding script is parsing the preprocessed C headers so C comments are not available for it. The C docs are generated from the comments on the C headers.
Instead of generating the docs in the binding script, perhaps we can consider a new script that would take as input the JSON output of the binding script and the doxygen xml output and generate the docstring from them. That might be cleaner than feeding the doxygen xml as another input to the binding script. What do you think?

and that was the problem!!! I cannot believe I didn't see it before. I searched the espidf for --containerof and I just noticed it is only used in c files.

Great!

kdschlosser commented 1 year ago

Instead of generating the docs in the binding script, perhaps we can consider a new script that would take as input the JSON output of the binding script and the doxygen XML output and generate the docstring from them. That might be cleaner than feeding the doxygen XML as another input to the binding script. What do you think?

That is exactly what I would love to see done.

I know that you understand how things are organized in the created lvgl module. But it is not in alignment with LVGL, not in a manner that is easily seen. I ended up writing a micropython script that loaded the lvgl module and recursivly iterated over the whole thing in order to get an idea of how everything was organized. I then wrote yet another script that ran through the xml files that doxygen created for LVGL to locate some of the things and had it generate a stub file. It was ugly and I had to go and correct a mess of things and it is not even close to complete. But at least it was something,

I ended up with this.

https://github.com/kdschlosser/lv_micropython_stub

As far as completeness of documentation is concerned...

If you go to here.. expand all of the different sections and then tell me that it's well documented.....

https://docs.lvgl.io/master/widgets/arc.html#_CPPv48lv_arc_t

Or go here and tell me where it states that those functions seen here are not all of the functions that are available. and that all of the functions available for lv_obj_t can also be used and that the functions listed here are additional...

https://docs.lvgl.io/master/widgets/arc.html#overview

IDK about you but I use the documentation as a reference, I don't read it from cover to cover like a book, my brain would probably melt if I tried to read it like a book. So information like hierarchy of an object should be display at the very top of the API or all of the inherited bits needs to be listed. Everyone is different on their preference of how to do it. Personally I like everything that works with a specific object to be listed with that object and not have to flip through multiple pages to locate what it is I am looking for. But everyone is different.

Where to find things is not so easy....

If you look at the naming convention for the functions in LVGL the natural thought is going to be that is what was used to determine what goes where in the lv binding for MicroPython.

lv_arc_set_knob_offset lv_arc_get_angle_start ...

lv_img_set_src lv_img_set_offset_x ...

and then you have

lv_draw_rect_dsc_init lv_draw_rect

and where is the draw_rect function located? It's not in the lv_draw_rect_dsc_t structure I can tell you that. It is actually in the draw_ctx_t structure.

This exact same mistake can be seen here https://sim.lvgl.io/v9.0/micropython/ports/javascript/index.html?script_startup=https://raw.githubusercontent.com/lvgl/lvgl/27400cd9add807f545f52ea6c0e43e6b4dab4419/examples/header.py&script=https://raw.githubusercontent.com/lvgl/lvgl/27400cd9add807f545f52ea6c0e43e6b4dab4419/examples/widgets/chart/lv_example_chart_6.py

on line 88

Someone told me that the binding is arranged based on what the first parameter of a function is. If that was the case then the arc functions would be placed in the obj_t structure because the first parameter has a type of lv_obj_t not lv_arc_t

void lv_arc_set_knob_offset(lv_obj_t * arc, int16_t offset);

How things are mapped out in the binding isn't that straight forward. It is more complex then using the file, function name or the first parameter. Because of that no assumptions can be made, explicit documentation showing inheritance needs to exist for the MicroPython binding.

The first parameter is not valid as a way of telling where things are located most other widget specific functions point to lv_obj_t. I still don't know the exact mechanics of how everything is organized and if I didn't have the stub file that I created it would be incredibly difficult to do anything advanced, to the point of pulling my hair out. This is because of the amount of time I would have to spend combing over the lv_mp.c file (36511 lines). Using the draw_rect example above searching the lv_mp.c file returns 413 matches for draw_ctx_t and 105 matches for lv_draw_rect.

Just to mention there is ZERO documentation on lv_draw_rect and draw_ctx_t yet it is used in the examples, incorrectly used but used none the less.

LVGL has capabilities that are endless. I tip my hat to the developers for writing it. Using the code they wrote, big learning curve and probably more difficult for a python developer that isn't familiar with how things are done in C code.

kdschlosser commented 1 year ago

On the off chance you want to have a look see at the changes I have been tinkering about with...

https://github.com/kdschlosser/lv_binding_micropython/tree/expose_esp_lcd_component

kisvegabor commented 1 year ago

The sponsorship is limited to what LVGL LLC can donate. It's not much, and the work involved is big as you mentioned. So maybe we should really consider closing https://github.com/lvgl/lv_binding_micropython/issues/227 if further funding is not possible. @kisvegabor - what do you think?

ESP-MicroPython-LVGL is getting more popular so I can increase the donation, but still not to the level of a real salary.

Could you summarize exactly what the planned tasks are? And which dev modules are targeted (I saw "ESP32-S2 dev kit, ESP32-C2 dev kit, ESP32-C3 dev kit, ESP32-C6 devkit" but I'd like to be sure)

BTW, I think supporting ESP-IDF v5 would be enough.

amirgon commented 1 year ago

Could you summarize exactly what the planned tasks are? And which dev modules are targeted (I saw "ESP32-S2 dev kit, ESP32-C2 dev kit, ESP32-C3 dev kit, ESP32-C6 devkit" but I'd like to be sure)

It's really up to us to define the tasks. Apparently what I defined on #227 is just too much. We can limit it to one SOC for example to make it easier (ESP32-S2 ?).

BTW, I think supporting ESP-IDF v5 would be enough.

I'm not sure about that. A lot of people are still using v4. I suspect supporting ESP-IDF v4 is more important than v5.

amirgon commented 1 year ago

I know that you understand how things are organized in the created lvgl module. But it is not in alignment with LVGL, not in a manner that is easily seen.

and where is the draw_rect function located? It's not in the lv_draw_rect_dsc_t structure I can tell you that. It is actually in the draw_ctx_t structure.

How things are mapped out in the binding isn't that straight forward. It is more complex then using the file, function name or the first parameter. Because of that no assumptions can be made, explicit documentation showing inheritance needs to exist for the MicroPython binding.

I think the JSON generated by the binding script can easily map the C API to Micropython API if we add both the C identifier and the Python function/attribute name and the parent struct/object name for each entry.
If the JSON had that, do you think you could use it with the Doxygen XMLs to generate docstrings?

On the off chance you want to have a look see at the changes I have been tinkering about with...

In general, I think the interesting parts are -

But each of these is a different task. Your repo seems to be a mixture of some (or all?).

If you decide to contribute, please open a PR with a focused goal and we shall review it.

Regarding display drivers, there's another idea to consider.
LVGL has an experimental support for TFT_eSPI which already supports many TFT displays.
I'm still not sure if we should integrate this with lv_micropython and exactly how, but if we did we would instantaneously support many display drivers.
The main difference from our drivers is that our drivers are configurable on runtime, so you can set pins and parameters in your Python script, while with TFT_eSPI every change in configuration, even in pin numbers, would require rebuilding the firmware. What is your opinion about this?

kisvegabor commented 1 year ago

It's really up to us to define the tasks. Apparently what I defined on https://github.com/lvgl/lv_binding_micropython/issues/227 is just too much. We can limit it to one SOC for example to make it easier (ESP32-S2 ?).

We have a quite good relationship with Espressif so I can ask around if they are interested contribution in this topic. I know that they are using MicroPython too. Probably they can also give some input about what features could important for the users, what they are working on, etc.

How things are mapped out in the binding isn't that straight forward. It is more complex then using the file, function name or the first parameter. Because of that no assumptions can be made, explicit documentation showing inheritance needs to exist for the MicroPython binding.

In SquareLine Studio we are using the MicroPython binding but sometimes it gave us hard times to figure out the naming conventions. E.g. 

lv.OBJ.FLAG.CLICKABLE   #or
lv.OBJ_FLAG_CLICKABLE   #or
lv.OBJ_FLAG.CLICKABLE   #or

btn1.OBJ.FLAG.CLICKABLE   #or
btn1.OBJ_FLAG_CLICKABLE   #or
btn1.OBJ_FLAG.CLICKABLE

I'd find a super simple mapping easier to use (e.g. lv.OBJ_FLAG_CLICKABLE) and use classes only where it's really straight forward (mainly widgets, display, indevs, i.e. when there is a create function). @amirgon already said that this way we'd loose the "grouping" in autocomplete, but for me it'd be still easier. Anyway, I'm not deeply into the MicroPython development, so I shouldn't have a too laud voice in this topics :slightly_smiling_face:

Regarding display drivers, there's another idea to consider. LVGL has an experimental support for TFT_eSPI which already supports many TFT displays. I'm still not sure if we should integrate this with lv_micropython and exactly how, but if we did we would instantaneously support many display drivers.

One of the main goals of LVGL v9 is to have run-time configurable driver in the lvgl repo. So they can parsed just like any LVGL functions. It's a question though how to configure the pins. But it seems also manageable as the goal is to have callbacks in the display drivers like send_color, send_command, etc. A lower( vendor specific) layer should also exist in LVGL which provides these callbacks (for ESP, STM, NXP, Arduino, etc), however it should be also possible to add them externally. This way MP can also provide a "vendor layer" or so for the platform independent drivers or the user needs to set only a very few callbacks.

kdschlosser commented 1 year ago

Micropyhton doesn't support version 5 of the IDF. so we are at the mercy of the latest version that MicroPython supports.

It would me it a lot easier to just have support for the latest one that MicroPython supports. There are so many changes that are made between IDF versions and trying to keep up with them is going to end up making for a really large complicated code base that has to be maintained.

Again, I don't care about the money aspect of it. I just don't have the gear on hand to test all of the different ESP32 SOCs. I would be more then happy to submit a PR for the changes I have made if the PR will get merged once it has been tested and confirmed to be working. What lead me to think that it wouldn't is because of the already existing PR that seems to have gone stale.

Ideally it would be best to have a common API that would work across any of the SOCs that MicroPython supports. I came across this for the STM32. This has some pretty usefull code in it for handling the different io types for the STM32

https://github.com/RobertoBenjami/stm32_graphics_display_drivers

The code for the ESP32 is pretty straight forward with the esp_lcd component. Tho I do not believe that the ESP32 supports that component. I think only the S2, S3 and C series SOCs support it.

It would be nice to see an across the board driver support for all brands and models of boards.

I don't think it would be that terribly hard to make a standardized API to handle the IO for the different boards. The user would pick the IO they want to use and pass the created io object to the driver and the driver would be able to read and write using that io. The drivers for the displays can be written in Python where as the io drivers would be written in C code.

It wouldn't need to be as complex and interwoven from a users standpoint that the espidf is. We could simply have functions that can be called from python code passing information needed to initialize a specific type of io the nitty gritty would be handled in C code. There is no reason why a user should have to know things like color format and color space for a display. The current drivers for displays have these things set as parameters and I am not sure as to why. It should be the bare minimum of things that need to be known like pin numbers and display dimensions.

Another thing that is nice about this kind of a design is the ability to pass the same io to a touch driver if it shares the io with the display. That would make it easier when dealing with shared SPI or a combination of analog touch shared with a parallel io display.

My point to all of this is because of the every growing number of SOCs and displays on the market how to move forward needs to be decided. I can write drivers all day that are specific to the ESP32 but I do not feel this is the right thing to do. we need to make the display support identical across the board for all of the supported SOCs.

kdschlosser commented 1 year ago

the problem with TFT_eSPI is it is made to work using the Arduino SDK as the common API for handling the io. I believe the scope of work needed to port the TFT_eSPI so it could be compiled outside of the Arduino IDE would simply be too much work. It would be better to write it from scratch. Another issue with the TFT_eSPI library is everything is set up to be loaded using macros. It's not something that can be chosen at runtime, it has to be picked at compile time. That really negates the purpose of using MicroPython. What I mean by that is the SPI, 8bit parallel and 16 bit parallel code for an SOC all share the same function names. so you would have to define TFT_PARALLEL_8_BIT for picking an 8bit parallel display and if you wanted to use DMA memory you would have to set ESP32_DMA at compile time. Ideally it would be nice to be able to provide binaries to the users so all they have to do is flash it, connect the display and they can pick the io and driver when their script runs.

kisvegabor commented 1 year ago

This has some pretty usefull code in it for handling the different io types for the STM32 https://github.com/RobertoBenjami/stm32_graphics_display_drivers

Very nice! We can make good use of it.

I can write drivers all day that are specific to the ESP32 but I do not feel this is the right thing to do. we need to make the display support identical across the board for all of the supported SOCs.

My view is something similar. For example we should have one (well written and flexible) ILI9341 driver and the IO part (SPI, Parallel, DMA on STM, NXP, ESP, etc) should be handled by callbacks or io_objects or something like this. Although I vote for callbacks because it's hard to imagine a generic io_object which support all the features of all the vendors' all the communication interfaces. Even having a fully generic io_set/clear() function is a challange:

With callbacks we could hide all these and the user could use the API provided by the vendor (STM HAL or LL, ESP-IDF, custom driver). We can have simple vendor wrappers in C and this way they could have a binding in MicroPython too.

So in a Python script you can set up a display runtime and do something like 

esp_spi = lv.esp_spi(spi_id=lv.ESP_HSPI, clk=10, mosi=11, cs=12, dma=True) 
#Assuming there is esp_spi.send_command() and esp_spi.send_image()

disp1.set_driver(esp_spi)

What do you think?

kdschlosser commented 1 year ago

You have to separate the display driver from the io. so you would create an io instance whether it be spi,, spi octal, 8 bit parallel or 16 bit parallel. that instance would get passed to the driver and the driver would call the needed functions in the io instance. That way the io instance would be able to do what it needs to do to get the information sent how it needs to be sent.

Pin number identification should be what the SOC has defined the pins as. not what the board has. It would be up to the user to do the home work and find out what the ppin number to SOC number mapping would be,.. You can only do so much and there are way to many different boards out there to cover them all. Not worth the headache.

I am not sure how other SOCs are set up but on the ESP32 for SPI you have HSPI and VSPI or SPI1 and SPI2 depending on the SOC. they are the same thing just different naming. This is considered to be the "spi host". If a user passes only the host for SPI then it would be assumed that the hardware set pins are being used for the host provided and there would not be a need to pass any pin numbers to the constructor/function.

What I recommend doing is also burying the name of the soc. In your example you have esp_spi as the function name. The compiling of micropython with the binding and how the binding is already set up makes it so that different code gets compiled based on what SOC is being used. The binding has the "drivers" folder and that folder has software that gets compiled for the different SOC's. So what should be done is a single API for all of the io. so function names like spi_io and parallel_io are what should be used. spi octal and parallel 16 bit is easily determined inside those functions based on the number of GPIO pins that have been passed to the function. spi octal uses 8 data pins and a cs pin. and 16 bit parallel uses 16 data pins and a data/command pin. any other pin that is needed fopr the function of the display that is NOT io related would be passed to the display driver along with the io instance, pins like backlight, power, reset etc....

The driver should be completely blind as to the operation of the io. it shouldn't care what it does it should only care about commands that need to be sent and passing those commands to the io when they need to get sent.

kdschlosser commented 1 year ago

OH!!! there was something I did want to ask and it kind of does pertain to this...

With the ESP32-S3 the SPIRAM actually has DMA memory allocation available for it. While the frame buffer can be set to use this memory if needs be the internal memory that is used by LVGL cannot. It seems like a waste to not be able to use that memory if possible for internal purposes. Is there a macro that allows for swapping out calls to malloc, calloc and the like? Also because there is only so much memory that can be used as DMA is there a fallback macro in the event the first one fails??

kdschlosser commented 1 year ago

Because of LVGL being able to be used without the need of MicroPython the display drivers and the io would need to be written in lvgl and not made only for MicroPython. The purpose of the binding is to expose LVGL to MicroPython and it should not house code for things that should also be available if using LVGL without MicroPython. On the flip side LVGL should not be reliant on any external libraries like what is seen when compiling using the Arduino IDE. This is because of the inability for those libraries to be used when compiling in MicroPython is done.

I am not sure of the reasoning behind the split API between LVGL and the binding. From a developer standpoint it would have made life a whole lot easier if the API were the same between the 2. I know it would not be considered Pythonic to have it this way but for the sake of not having to maintain 2 different APIs it would make sense to keep the LVGL API in tact.

I have a dummy question. How come LVGL was not written in C++? I do not think it would be an issue of an SOC running the code.. C++ would allow class specific functions to only be available for that class. In C code this could be done to give the same kind of thing.

Complex.h:

struct Complex {
    double re, im;
    double (*abs)(struct Complex *this);
};
extern const struct ComplexClass {
    struct Complex (*new)(double real, double imag);
} Complex;

Complex.c:

#include "Complex.h"
static double abs(struct Complex *this) {
    return sqrt(this->re*this->re+this->im*this->im);
}
static struct Complex new(double real, double imag) {
    return (struct Complex){.re=real, .im=imag, .abs=&abs};
}
const struct ComplexClass Complex={.new=&new};

Complex_test.c:

struct Complex c=Complex.new(3., -4.);
printf("%g\n", c.abs(&c)); // Prints 5

This way the functions that are specific to say the arc or label would not be exposed to the user. They would be added to the structures and exposed to the users that way..

Again I am no C developer and I do not know what the cost would be to do this. It might not be worth doing.

kisvegabor commented 1 year ago

It seems we are more or less on the same page. I'm waiting for a few things to be tested/fixed in a PR by @amirgon. Once it's merged we can start exploring more on the driver and vendor side.

With the ESP32-S3 the SPIRAM actually has DMA memory allocation available for it. While the frame buffer can be set to use this memory if needs be the internal memory that is used by LVGL cannot.

It's do possible. In lv_conf.h you can define custom malloc, free and realloc.

How come LVGL was not written in C++

First, for historical reasons. LVGL is not that ancient (was published 7 years ago) but back then C++ wasn't that well spread yet. Secondly, C - as being a lower level and simpler language - still has better support. E.g. pycparser which is used by @amirgon to during binding generation supports only C99.
We had long discussions about this topic and decided that it's better to have C++ binding to LVGL. basically it's just an API wrapper to have C++ a styled API.

kdschlosser commented 1 year ago

That makes sense.

I have some ideas that might be useful if coming up with some form of a standard API for the drivers. This conversation rolls right into that topic you mentioned.

Gotta keep the actual driver separated from the io. use callbacks in the io that the driver can call when the data is to be sent. The driver would also have callbacks for things like resetting, drawing and things of that nature. The backlight should also be a separate object, there should be 2 forms, a simple on and off and a pwm.

might be best to start a repo for getting the drivers all organized and the API hammered out and then when it is finished add that code to LVGL. This way it doesn't disrupt anything in LVGL and people will be less likely to try and use it before it's finished.

kisvegabor commented 1 year ago

might be best to start a repo for getting the drivers all organized and the API hammered out and then when it is finished add that code to LVGL. This way it doesn't disrupt anything in LVGL and people will be less likely to try and use it before it's finished.

We have attempted in the past but it didn't work that well. The new repo wasn't that active. I think it's better to have a new branch in the main lvgl repo because more people see the issues and discussions there.

kdschlosser commented 1 year ago

That is an option as well. Start off with a branch of 9.0 and I would then add the drivers exactly how they are into that new branch under the root set up a task list for each of the drivers that can be checked off as they are finished. The finished drivers would be moved into the final location under the "src" directory. This would eliminate the confusion of what has been finished and what has not been.

path structure would look something like the following.

src
    drivers
        common
            includes
            src
        stm32
            includes
            src
        esp
            includes
            src
        linux
            includes
            src
        windows
            includes
            src
        etc......

the common folder would contain the display drivers and the board specific would contain the io drivers The io drivers get included using preprocessor definitions and their paths get added in the build script. You know how that whole system works and I am saying it for the people that do not.

Nailing down an API is not going to be that hard to do if you use callbacks. Those are going to be the ticket to get this to work right. using callbacks also makes it easy for developers to add onto or modify how the whole thing works. If they have some kind of a custom design for a display they will be able to make a driver for it pretty easily.

kisvegabor commented 1 year ago

I agree in general. Looking forward to start working on it :slightly_smiling_face:

jd3096-mpy commented 1 year ago

I try to use https://github.com/kdschlosser/lv_binding_micropython/tree/expose_esp_lcd_component to make firmware,but it failed. In https://github.com/kdschlosser/lv_binding_micropython/blob/expose_esp_lcd_component/driver/esp32/espidf.c line32

include "/home/kgschlosser/esp/components/esp_lcd/src/esp_lcd_rgb_panel.c"

It's a Absolute Path，I tried to modify it to my path ,but stills doesn't work How to solve this problem, esp_lcd components has already in my esp-idf path ,how to include it directly?Thanks

kdschlosser commented 1 year ago

That is a test version not meant to actually be used.

ftab commented 1 year ago

TFT_eSPI was mentioned. What about LovyanGFX? I've used this driver in my C/C++ projects (it's written in C++), and it doesn't have any dependence on the Arduino API. The port for the S3 already includes 8 and 16 bit parallel support, and there's support for many touch panels, displays, and brightness control. Does that help at all?

I have some S2 and S3 based parallel & RGB display hardware so I can help test wherever needed. (Makerfabs ESP32-S2 and ESP32-S3 16-bit parallel; and one custom one-off build that hooks up an S3 devkit to the RGB interface of a 7" display, which I have running off of the esp_lcd rgb panel interface)

amirgon commented 1 year ago

TFT_eSPI was mentioned. What about LovyanGFX?

@ftab This needs to be supported by LVGL first, before discussion this in the context of lv_micropython.
I would suggest you open an issue requesting this on LVGL repo.

ftab commented 1 year ago

It's already possible now. I made this example project a while back as an LVGL+LovyanGFX "hello world" for the Makerfabs screens: https://github.com/radiosound-com/makerfabs-parallel-tft-lvgl-lgfx

amirgon commented 1 year ago

It's already possible now.

Yes but it's not a formal part of LVGL yet, right? If you want, you can open a PR on LVGL to add it as a formal driver with the new driver API.

On the longer term, the goal is to be able to configure the driver (pins, freq, etc.) on runtime instead of compile time, so it would be easier to use with Micropython and configure it from your Python script instead of rebuilding the firmware.

jd3096-mpy commented 1 year ago

That is a test version not meant to actually be used.

So can you provide a repo or firmware?Thanks.