Cleanup lvgl_helpers

[C47D]

Cleanup of lvgl_helpers and fixes compilation errors.

[C47D]

Project builds successfully in my pc with ESP-IDF v4.4-dev-2740-gf65c8249af and ESP32 as target.
@tore-espressif Any idea on why it might be failing?

Also, all of the functions in lvgl_helpers are espressif specific, do you think it would be proper to move them into the lv_port directory? Same for the esp_lcd_backlight source file.

And rename the helper functions, the names are not very consistent.

[tore-espressif]

yep, there were some changes in esp_rom_*.h headers in IDF4.4. I'll fix it today or tomorrow

(it is weird though, I remember fixing this problem few months ago)

[C47D]

I can try to check your commits and look for the fixes.

[arktrin]

well, I have these errors and warnings:

../components/lvgl_esp32_drivers/lvgl_helpers.c:330:13: warning: 'init_ft81x' defined but not used [-Wunused-function]
 static void init_ft81x(int dma_channel)
             ^~~~~~~~~~

../components/lvgl_esp32_drivers/lvgl_tft/ra8875.c: In function 'ra8875_init':
../components/lvgl_esp32_drivers/lvgl_tft/ra8875.c:186:5: error: implicit declaration of function 'gpio_pad_select_gpio'; did you mean 'esp_rom_gpio_pad_select_gpio'? [-Werror=implicit-function-declaration]
     gpio_pad_select_gpio(RA8875_RST);
     ^~~~~~~~~~~~~~~~~~~~
     esp_rom_gpio_pad_select_gpio

../components/lvgl_esp32_drivers/lvgl_tft/GC9A01.c: In function 'GC9A01_init':
../components/lvgl_esp32_drivers/lvgl_tft/GC9A01.c:114:5: error: implicit declaration of function 'gpio_pad_select_gpio'; did you mean 'esp_rom_gpio_pad_select_gpio'? [-Werror=implicit-function-declaration]
     gpio_pad_select_gpio(GC9A01_DC);
     ^~~~~~~~~~~~~~~~~~~~
     esp_rom_gpio_pad_select_gpio

I guess gpio_pad_select_gpio(GC9A01_RST) from lvgl_tft/GC9A01.c, gpio_pad_select_gpio(ignore) and gpio_pad_select_gpio(measure) from lvgl_touch/adcraw.c will also invoke these problems.

[arktrin]

Single warning left during compiling due to 'init_ft81x' defined but not used. But SPI configuration fails in my esp32c3:

I (327) lvgl_helpers: Initializing SPI master
I (347) lvgl_helpers: Configuring SPI host SPI1_HOST
I (347) lvgl_helpers: MISO pin: -1, MOSI pin: 1, SCLK pin: 2, IO2/WP pin: -1, IO3/HD pin: -1
I (347) lvgl_helpers: Max transfer size: 19200 (bytes)
I (367) lvgl_helpers: Initializing SPI bus...
E (367) spi: spi_bus_initialize(756): SPI bus already initialized.

assert failed: lvgl_spi_driver_init lvgl_helpers.c:301 (ret == ESP_OK)

It tries to use SPI1_HOST but I've set SPI2_HOST in sdkconfig: CONFIG_LV_TFT_DISPLAY_SPI2_HOST=y.
SPI bus already initialized message looks kind of weird to me or I've just forgotten how it should look like?

[C47D]

Thanks for the update, can you please attach the sdkconfig.h file?

[arktrin]

Content of my ./build/config/sdkconfig.h

[tore-espressif]

Hi all,

I couldn't follow all changes you have been doing recently, but I hope you find this useful:

@C47D This commit 5043946 is fixing esp_lcd_backlight compilation issue (it is from august 2021), but apparently @arktrin is having a different problem.

This commit 8c7bc42 breaks compatibility with IDF v4.1 and 4.2. The esp_rom_gpio.h header file is included from v4.3 onwards

@arktrin All your gpio missing declarations (gpio_pad_select_gpio, gpio_matrix_out) are included from driver/gpio.h. This works on all IDF version and all targets. So make sure all compilation units that use these function have driver/gpio.h included

EDIT: looking to commits history in this PR, this is the trouble maker 24e4bf0 , effectively reverting that fix from august

[arktrin]

@tore-espressif Not sure I understand you correctly. As you can see all of my errors associated with gpio_pad_select_gpio() function are the part of lvgl_esp32_drivers itself. Where should I include driver/gpio.h ?

EDIT
Currently I'm using special branch of lv_port_esp32 ( feat/new_driver_test - made by @C47D ) for testing develop branch of lvgl_esp32_drivers.

[tore-espressif]

I see, I thought that you are authoring the branch :)
So the branch is probably broken and misses the driver/gpio.h includes.

[arktrin]

@tore-espressif Would you be so kind to fix feat/new_driver_test branch of lv_port_esp32 project ? :)

[C47D]

Hi @arktrin , sorry for the late reply, I was offline all weekend.

There are no errors with the latest release/v4.4 branch of the esp-idf. I've hardcoded TFT_SPI_HOST to work with SPI2_HOST, everything looks fine but i have nothing on the screen :(

I see the same issue, also tested develop (without this PR) and still have the same issue. I will try to bisect it later today.

with latest 2f70856 and 7722f17 :

../components/lvgl_esp32_drivers/lvgl_tft/FT81x.c: In function 'FT81x_init':
../components/lvgl_esp32_drivers/lvgl_tft/FT81x.c:266:2: error: implicit declaration of function 'gpio_pad_select_gpio'; did you mean 'esp_rom_gpio_pad_select_gpio'? [-Werror=implicit-function-declaration]
  gpio_pad_select_gpio(EVE_PDN);
  ^~~~~~~~~~~~~~~~~~~~
  esp_rom_gpio_pad_select_gpio

EDIT This is true for the latest master branch of the esp-idf.

Thanks for pointing out the compilation error is present on the master branch, I don't think we added it into the CI.

[tore-espressif]

One question, is it possible to have multiple versions of ESP-IDF in the same environment? I'm working with WSL (on Windows 10) and only have ESP-IDF v4 installed. I'm wondering if cloning other versions, and exporting one at the time, would allow me to have "multiple" versions of ESP-IDF.

@C47D

You should have only one IDF version in single environment. But you can have multiple environments.

I'm on Windows 10 and work with Windows Terminal. I have multiple versions of idf cloned so my folder tree look like this:

C:/esp_repos
├───esp-idf-v4.1
├───esp-idf-v4.2
├───esp-idf-v4.3
├───esp-idf-v4.4
├───esp-idf-master

The windows terminal JSON file contains (among other things):

            {
                "altGrAliasing": true,
                "antialiasingMode": "grayscale",
                "closeOnExit": "graceful",
                "colorScheme": "Campbell",
                "commandline": "powershell.exe -NoExit C:\\esp_repos\\esp-idf-v4.2\\export.ps1",
                "cursorShape": "bar",
                "font": 
                {
                    "face": "Cascadia Mono",
                    "size": 12
                },
                "guid": "{d6bd6b46-f392-494d-bf17-e9c670cc8e12}",
                "historySize": 9001,
                "name": "IDF v4.2",
                "padding": "8, 8, 8, 8",
                "snapOnInput": true,
                "startingDirectory": "C:\\esp_repos\\",
                "suppressApplicationTitle": true,
                "tabTitle": "IDF v4.2",
                "useAcrylic": false
            },
            
            ... and so on...

The shell uses Powershell, but it should be easy to use WSL too :)
Result looks like this:

[C47D]

There are no errors with the latest release/v4.4 branch of the esp-idf. I've hardcoded TFT_SPI_HOST to work with SPI2_HOST, everything looks fine but i have nothing on my st7789 display :(

@arktrin I've just pushed a fix to the lv_port_esp32 repo. The disp_drv registration was commented out 😿

[C47D]

@tore-espressif Thanks for the explanation, I'm working from a borrowed pc, will try to setup multiple environments new week, I will also try to get github actions running locally.

[arktrin]

TFT_SPI_HOST issue seems to be fixed ( but I'm still have nothing on my st7789 display ).

Single warning left (this is not related to my issue with display):

../components/lvgl_esp32_drivers/lvgl_helpers.c:330:13: warning: 'init_ft81x' defined but not used [-Wunused-function]
 static void init_ft81x(int dma_channel)
             ^~~~~~~~~~

I've just pushed a fix to the lv_port_esp32 repo. The disp_drv registration was commented out

Can't see any new commits in lv_port_esp32 repo.

[C47D]

@arktrin thanks for letting me know, I wasn't pushing to a public remote. Now you should be able to see the fix in the feat/new_driver_test branch.

[arktrin]

Now you should be able to see the fix in the feat/new_driver_test branch.

No warning at all! And finally now I see my app on the screen! Yay! This is definitely may be merged from my perspective.

[C47D]

I will add a resume so @tore-espressif can review it as well. Thanks a lot for testing it @arktrin , it took a lot longer than I was expecting.

Basically we did:

• GPIO initialization removed from display drivers init functions. The GPIO initialization is handled in display_bsp_init_io
• Detect usage of undefined symbols, such as spi_common_dma_t and handle them in different ESP-IDF versions.
• Cleaning up a bit lvgl_interface_init
• Fix compilation error when logging is enabled. This was an accidental discovery because I enabled LVGL logging to check why we didn't saw anything on the screen.

Some open topics (apart of merging develop into master):

• Update documentation (README)
• Communication interface proposal (for display drivers)
• Move ESP-IDF related code to lv_port
• Cleaning up the display Kconfig
• Cleaning up the touch Kconfig
• Communication interface proposal (for touch drivers)
• Improve naming convention on the lvgl_helpers files.

[tore-espressif]

I will add a resume so @tore-espressif can review it as well.

That's an impressive amount of work, I'll do my best to review next week.

[C47D]

The open topics are not implemented in this PR, they are meant to be done later.

[tore-espressif]

As you might have already noticed, I'm not a big fan of huge #ifdefs 🗡️ :D

But apart from that, if we want to make the drivers platform agnostic we will have to deal with them, because not all platforms use Kconfig.

[C47D]

As you might have already noticed, I'm not a big fan of huge #ifdefs 🗡️ :D

But apart from that, if we want to make the drivers platform agnostic we will have to deal with them, because not all platforms use Kconfig.

Yes, me neither, but I think cleaning up the existing code so later it can be refactored later is a good first step. I will address a couple of comments with leftover code, and wait for your reply on all the others. Thanks for taking the time to review this :)

I will also integrate the latest develop changes 😄

[C47D]

Hi @tore-espressif, I did the requested changes. Is it OK if we merge this?

[C47D]

Version checks removed and the CI is now passing @tore-espressif