Kconfig refactoring

[C47D]

Yes, I've seen the request, but it is going to be far more complicated than that.

Currently all KConfig options are global: display orientation, connection(I2C/SPI), pinout, color inversion.... A mere addition of the .c file to compilation list will not be sufficient.

Even LVGL itself is compiled with fixed resolution.
EDIT: LVGL supports multiple screen, so I see the reasoning behind this.

Let me know if there is something obvious that I'm missing

Originally posted by @tore-espressif in #39 (comment)

[C47D]

There's multiple improvement opportunities for this repo, and I think we could start by refactoring the Kconfig files to enable the usage of multiple displays in the same project, as this is one of the LVGL features that we're still missing.

@tore-espressif Do you think making this options

Currently all KConfig options are global: display orientation, connection(I2C/SPI), pinout, color inversion.... A mere addition of the .c file to compilation list will not be sufficient.

configurable in code rather than Kconfig based a better approach?

Any thoughts @kisvegabor @embeddedt @cmumford

[cmumford]

My guess is that the large majority of LVGL users have a single screen. So, whatever is done should't make their lives more complicated. I wouldn't think having a list of screens would be too bad.

It would be nice to take a few settings which are part of "LVGL configuration" and move them into the "LVGL TFT Display controller" section as these values seem specific to a display, namely:

1. DPI
2. Color depth
3. Swap the 2 bytes of RGB565 color
4. Screen transparency (not sure 'bout this)

My preference would be to configure in code. I'm not a big fan of Kconfig (menuconfig) because I always find myself hunting for where the setting is specified, and I have my compile-time variables split (sometimes duplicated) between sdkconfig and other project files.

I must admit, however, that I'm not super familiar with Kconfig. Does it even support the concept of a variable list? Is it robust enough to take all of what is currently in the "LVGL TFT Display controller" section, and create a variable list of those settings?

[embeddedt]

I would suggest not using Kconfig for this. Kconfig is not a very dynamic configuration language. While it does support hiding/showing options based on dependencies, the options all have to be declared in advance. There is no way I can think of for a user to add another display in a flexible enough way.

[tore-espressif]

Point 1

Refactoring the whole system might bring benefits, but regression risk here is huge:

1. I don't have all the LCDs on my desk
2. Manpower?
3. No automated build
4. No automated tests

If we don't want to ditch this idea, then setting up a build GitHub action should be a first step. Issues from users like 'example code not compiling' would outweight potential benefits.

Point 2

Moving KConfig option which @cmumford mentioned make sense.

However, we must take into consideration other platforms (is anyone else apart from Zephyr and ESP32 use KConfig?) so we won't break it. Every other platform is using lv_conf.h, right?

Final suggestion

If we can tackle Point 2 and have all TFT controller related options in one KConfig menu, it would be a good start.

The multiscreen feature request would be postponed.

[C47D]

The issue with moving options from LVGL to DISPLAY is that other projects are already using the LVGL Kconfig, as @tore-espressif pointed out, as far as I know the Nuttx project also uses it.

The final goal of this repo was to make the drivers generic enough so they can be later moved into lv_drivers and users of other platforms would be able to use them, we might need take that into consideration. My suggestion was to use the lv_disp_drv_t users data field to pass a struct with function callbacks into the flush and other LVGL callbacks.
I did a proof of concept in here: psoc_lvgl

I would also like to think a better way to change the orientation configuration instead of having hard coded values.

Point 1
Refactoring the whole system might bring benefits, but regression risk here is huge:
I don't have all the LCDs on my desk
Manpower?
No automated build
No automated tests
If we don't want to ditch this idea, then setting up a build GitHub action should be a first step. Issues from users like 'example code not compiling' would outweight potential benefits.

I was thinking on setting a GitHub action, but I don't know how to start, we might need a docker image with ESP-IDF installed, I'm new into CI and only know some concepts but I had never applied it.

[embeddedt]

If you can give me a sequence of commands to run with a fresh copy of ESP-IDF, I'd be happy to try and configure the GitHub action for you.

[C47D]

@embeddedt Thanks, getting ESP-IDF up and running would be great.

Would we also need to create different configurations with different displays and indev drivers so we compile them in the action?

[embeddedt]

I think so, since menuconfig wouldn't be available at CI time.

[kisvegabor]

In LVGL I used a python script with a lot of configs options to build lvgl and run tests. Something similar can be done here as well.

It would be nice to take a few settings which are part of "LVGL configuration" and move them into the "LVGL TFT Display controller" section as these values seem specific to a display, namely:

1. DPI
2. Color depth
3. Swap the 2 bytes of RGB565 color
4. Screen transparency (not sure 'bout this)

I think we need to keep these options in LVGL as LVGL needs to work on its own (without any other repo) as well and these defines are required to build LVGL.

However, we must take into consideration other platforms (is anyone else apart from Zephyr and ESP32 use KConfig?) so we won't break it. Every other platform is using lv_conf.h, right

Zephyr and NuttX use LVGL only with Kconfig without lv_conf.h

I agree that multi-display support is not critical. But if we set hor res and ver res for each display in menuconfig it seems doable to simply register all the enabled displays for LVGL. However, if they share the same SPI the SPI driver should be written with this in mind.

[VedantParanjape]

@C47D I have a ready setup for esp-idf: https://github.com/SRA-VJTI/sra-board-component/blob/main/.github/workflows/test-build-app.yaml

It compiles the example apps using the component.

[tore-espressif]

Another thing is that ESP-IDF build system is ATM not suited for building static libraries, so we would have to run the build in a repository that contains whole project, e.g. https://github.com/lvgl/lv_port_esp32 (or create a test project here) and build it separately for every LCD driver (see here https://github.com/lvgl/lvgl_esp32_drivers/blob/master/lvgl_tft/disp_driver.c#L8 for example).

Option no. 2 would be running a compile command on every .c file in this repository and call it a success if we can compile it to .o file without errors.

In LVGL I used a python script with a lot of configs options to build lvgl and run tests. Something similar can be done here as well.

@kisvegabor how do you run these tests? Or is it just a mere proof of correct compilation?

[embeddedt]

As far as I know the tests are more like coverage/unit tests where they check that certain behavior happens. LVGL runs in a headless application within the CI runner and it checks whether functions return correct values, etc.

[VedantParanjape]

As far as I know the tests are more like coverage/unit tests where they check that certain behavior happens. LVGL runs in a headless application within the CI runner and it checks whether functions return correct values, etc.

It won't work with esp-idf, it has a unit test framework, but you need a hardware hooked up to run it. For CI/CD only way to check is if it compiles correctly, After all you can't simulate all the minute details in a simulator.

[kisvegabor]

If we are targeting only build tests (to see if to an error in various configurations) we can event stub ESP-IDF. After all, there are only a few API calls we rely on.

ATM, in LVGL we are using the tests only as build tests. However, in the near future, we will add more "real" functional tests (mainly screenshot compares for first).