lvgl-micropython / lvgl_micropython

LVGL module for MicroPython
MIT License
87 stars 29 forks source link

RGBDisplay constructor no longer works #157

Closed MitchBradley closed 3 weeks ago

MitchBradley commented 1 month ago

I have some display init code that was working last month, but I tried updating to the latest commit of lvgl_micropython (4c62391) and now the RGBDisplay constructor fails. The first problem I had was the disp parameter to RBGBus, but I noticed its removal, deleted that line, and got the bus to construct.

Now I get this when constructing RGBDisplay (code attached as file)

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "crowpanel7_init.py", line 67, in <module>
  File "rgb_display.py", line 38, in __init__
  File "rgb_display_framework.py", line 42, in __init__
  File "display_driver_framework.py", line 215, in __init__
  File "display_driver_framework.py", line 280, in _init_bus
SyntaxError: Cannot convert 'bound_method' to pointer!

I tried commenting out various lines in the RBGDisplay parameter list to see which is causing the problem. It seems to be related to the framebuffer1 and framebuffer2 lines. If framebuffer1 is omitted, it fails with a "can't allocate memory" error, presumably because it tries to allocate from scarce regular RAM instead of PSRAM. If framebuffer1=buf1 is present but framebuffer2 is absent, it hangs and execution does not proceed to the following code. Could changes to the bus code be causing problems?

crowpanel7_init.py.txt

kdschlosser commented 1 month ago

I know what is going on here. I messed up and I pushed an update to the submodules that I didn't intend to. You are going to have to give me a bit to figure out how to undo it. It's not a simple thing to undo. What I am probably going to have to do is remove the submodule and then add it again and set it to the correct commit..

kdschlosser commented 1 month ago

this problem should now be fixed

MitchBradley commented 1 month ago

It is still failing. I tried various ways of updating the submodules (after git pull), then took out the big gun and just got a fresh clone of lvgl_micropython and started from scratch. The failure syndrome is the same as above.

Here is the output of git submodule summary:

wmb@DESKTOP-6I1SH7T:/GitHub/lvgl_micropython$ git submodule status
-6ad390fc5093bcbf785b7ad103484501d11d8c2a lib/SDL
 11eaf41b37267ad7709c0899c284e3683d2f0b5e lib/esp-idf (11eaf41b)
 92d424ec7442aa7cf2ba237929c1e8ecb5d0abc3 lib/lvgl (92d424e)
 d7d77d91becd0716ac1672c92652d9dd72ec613f lib/micropython (d7d77d9)
 3cf6bf5eb16f5eadd4a058e41596145c407a79ad lib/pycparser (3cf6bf5)

Which versions should I have?

MitchBradley commented 1 month ago

This is probably related but I am not sure what to make of it - the file api_drivers/lvgl_api_drivers/frozen/display/display_driver_framework.py is somewhat different than the display_driver_framework.py inside the .zip archive in that same directory. The differences appear to be related to subtle differences in lvgl.

wxzed commented 1 month ago

@MitchBradley I encountered the same issue. Did you manage to solve it? On my side, it appears that the issue arises when invoking the callback here:

/py_api_drivers/frozen/display/display_driver_framework.py

self._disp_drv.add_event_cb( self._on_size_change, lv.EVENT.RESOLUTION_CHANGED, # NOQA None )

kdschlosser commented 1 month ago

I know what the issue is and I am trying to solve it. There was a change made upstream in LVGL that is causing the problem with the binding code and I had set the LVGL submodule to before that change was made and I screwed up the submodule somehow and had to redo it and for the life of me I don't remember the commit in LVGL that caused the issue.

In order to fix the problem it is going to require a lot of time because of figuring out where in the code generator script changes need to be made and the extent of the changes that are going to need to be made.

What is causing the problem is this... The code generator literally reads the header files like you or I would read them. it follows includes located in the header file it is handed so recursively it end up reading whatever header files are included in LVGL. In LVGL starting with 9.0 there has been a big push to make portions of LVGL private. This push is being made because the API is getting extremely large and when dealing with a binding where the binding code gets generated there is a very large chunk of the code that gets generated that will go unused because those portions of LVGL are only used internally. In order to correct this problem what is being done is there are forward declarations being made and the definitions mare occurring in private header files. Those private header files are not included in any public ones they are only included in the C course files. The compiler doesn't care because the header files are not what gets compiled it's the source files that are and so long as the definition occurs in a file that is being compiled it's not going to bean issue. But for the code generator it is. C code sees callback functions as pointers to a memory location. Python functions are not pointers so they need to be handled in a very specific way to make it work. In order to make it work a c function is what gets stored and that function is what handles making the call to the python function. Part of the mechanics that triggers that C function to get created is the code generator being able to look at a structure and know what the fields are. But if the definition of a structure doesn't happen inside of any public header file the code generator is not going to be able to see it. The only thing it sees is a structure but it doesn't know anything about it. So what happens is when setting a callback instead of a c function being passed to the actual LVGL function the python function gets passed and that function is not a pointer. Hence the error you are seeing.

The code in the binding generator needs to be changed in a manner so it is able to see what is in those structures but not add any binding code to access anything in the structure. That is where the complications come in because now C objects need to be tied to the header file names that they are defined in so a check can be made to see if the word "private" is in the file name so decisions are able to be made on what to do. Because of the nested mess in the way the code generator was written this is not an easy thing to solve. The code generation script is something I am frankly afraid of, LOL. Making changes to it usually cause cascading problems. It would probably be easier to write a new script then it would be to make this kind of a change to the existing one. Writing a new script is going to take time to do and if I do write a new one I am going to flatten the API so it is identical to the LVGL C API. This will reduce the memory use as well as the compiled size of the binding by a sizeable amount. changing the API is going to be like pissing in a lot of peoples cheerios which is something I don't really want to do either. Stuck between a rock and a hard place with this.

kdschlosser commented 1 month ago

OoOOoO I think I just found the commit it was set to before things broke..

e1c0b21b2723d391b885de4b2ee5cc997eccca91

kdschlosser commented 1 month ago

OK I just updated it, see if it will work now.

wxzed commented 1 month ago

@kdschlosser Thank you, the previous issue has been resolved, but now there's another problem. I'm using the ILI9341 display, and my build command is: python3 make.py esp32 clean BOARD=ESP32_GENERIC_S3 BOARD_VARIANT=SPIRAM_OCT DISPLAY=ili9341 --usb-jtag --flash-size=16 However, I get the following error: Traceback (most recent call last): File "<stdin>", line 43, in <module> File "display_driver_framework.py", line 487, in init ImportError: no module named '_ili9341_init'. My test code is as follows: `import lvgl as lv import lcd_bus import ili9341 from machine import SPI, Pin import machine

import gc gc.collect()

spi_bus = SPI.Bus( host=1, mosi=21, miso=-1, sck=12 )

display_bus = lcd_bus.SPIBus( spi_bus=spi_bus, dc=13, cs=14, freq=40000000, ) print(gc.mem_free()) buf1 = display_bus.allocate_framebuffer(int(240 320 2 ), lcd_bus.MEMORY_SPIRAM) print(gc.mem_free()) buf2 = display_bus.allocate_framebuffer(int(240 320 2 ), lcd_bus.MEMORY_SPIRAM)

print(gc.mem_free())

print(gc.mem_free()) display = ili9341.ILI9341( data_bus=display_bus, frame_buffer1=buf1, frame_buffer2=buf2, display_width=240, display_height=320, reset_state=ili9341.STATE_LOW, color_space=lv.COLOR_FORMAT.RGB565, rgb565_byte_swap=True ) print(gc.mem_free())

display.set_power(True) display.init() display.set_rotation(lv.DISPLAY_ROTATION._90) `

wxzed commented 1 month ago

Got it, I'll use display.init(1).

kdschlosser commented 4 weeks ago

yeah the API has changed so that it deals with initializing displays in a dynamic manner. this keeps the amount of code that needs to be loaded as small as possible since leading a python source file takes up memory and quite a bit of it. so no use having code loaded into ram that never ends up running. some of the initialization code can be quite large for the displays.

kdschlosser commented 4 weeks ago

With the display you are using I suggest not creating any framebuffers, let the driver handle it as it will setup the buffers to give the best possible performance.

MitchBradley commented 4 weeks ago

When I tried that before it failed because you need to allocate the framebuffers in SPIRAM. The on-chip RAM is too small. The automatic allocation did not know to use SPIRAM.

MitchBradley commented 4 weeks ago

But maybe your suggestion was directed at wxzed instead of the OP

kdschlosser commented 4 weeks ago

yes it was directed at @wxzed

but it should also work for RGB displays as well. If it's not I will have a look at the code to see why it's not. How the buffer allocation works is it will try to create 2 buffers in DMA SRAM and if that fails then it will try and make 2 buffers in DMA SPIRAM and if that fails then it will try and create a single buffer in SRAM and if that fails then it will create a single buffer in SPIRAM

When using any driver except for the RGB driver it will do the math for the size of the buffer and divide it by 10. The RGB display it will allocate the buffer at the full size.

Maybe what you did was you forgot to remove the lines where you were allocating the buffers? That would cause an issue. You would need to remove those and leave out the frame buffer parameters or set them to None when constructing the driver.

kdschlosser commented 4 weeks ago

To have it handle the creation of the buffers you would use this code.

# Screen and touch setup for Elecrow "CrowPanel 7"  800x480 display
# https://www.elecrow.com/wiki/esp32-display-702727-intelligent-touch-screen-wi-fi26ble-800480-hmi-display.html
# It is a 7 inch touchscreen panel that uses EK9716BD3 as the display driver
# and GT911 for the touch interface. The module include an ESP32-S3-WROOM-1-N4R8
# with 4 MiB of Quad SPI FLASH and 8 MiB of Octal SPI PSRAM.

from micropython import const
import lcd_bus
import lvgl as lv
import rgb_display
from i2c import I2C
import gt911

_WIDTH = const(800)
_HEIGHT = const(480)

bus = lcd_bus.RGBBus(
    hsync=39,
    vsync=40,
    de=41,
    pclk=0,
    data0=15,
    data1=7,
    data2=6,
    data3=5,
    data4=4,
    data5=9,
    data6=46,
    data7=3,
    data8=8,
    data9=16,
    data10=1,
    data11=14,
    data12=21,
    data13=47,
    data14=48,
    data15=45,
    freq=12000000, # The display creeps if this is too fast
    hsync_idle_low=False,
    hsync_front_porch = 40,
    hsync_pulse_width = 48,
    hsync_back_porch  = 40,
    vsync_idle_low=False,
    vsync_front_porch = 1,
    vsync_pulse_width = 31,
    vsync_back_porch  = 13,
    de_idle_high=False,
    pclk_idle_high=False,
    pclk_active_low=1,
)

display = rgb_display.RGBDisplay(
    data_bus=bus,
    display_width=_WIDTH,
    display_height=_HEIGHT,
    backlight_pin=2,
    backlight_on_state=rgb_display.STATE_PWM,
    color_space=lv.COLOR_FORMAT.RGB565,
    rgb565_byte_swap=False
)

display.set_power(True)
display.init()
display.set_backlight(100)
# display.set_rotation(lv.DISPLAY_ROTATION._90)

# For reference, the I2C IRQ is on pin 38
I2C_BUS = I2C.Bus(
    host=1,
    scl=20,
    sda=19,
    freq=400000,
    use_locks=False
)

TOUCH_DEVICE = I2C.Device(
    I2C_BUS,
    dev_id=gt911.I2C_ADDR,
    reg_bits=gt911.BITS
)

indev = gt911.GT911(TOUCH_DEVICE)
indev.enable_input_priority()
kdschlosser commented 4 weeks ago

There is also one other thing you are missing in your driver code...

# Screen and touch setup for Elecrow "CrowPanel 7"  800x480 display
# https://www.elecrow.com/wiki/esp32-display-702727-intelligent-touch-screen-wi-fi26ble-800480-hmi-display.html
# It is a 7 inch touchscreen panel that uses EK9716BD3 as the display driver
# and GT911 for the touch interface. The module include an ESP32-S3-WROOM-1-N4R8
# with 4 MiB of Quad SPI FLASH and 8 MiB of Octal SPI PSRAM.

from micropython import const
import lcd_bus
import lvgl as lv
import rgb_display
from i2c import I2C
import gt911

_WIDTH = const(800)
_HEIGHT = const(480)

bus = lcd_bus.RGBBus(
    hsync=39,
    vsync=40,
    de=41,
    pclk=0,
    data0=15,
    data1=7,
    data2=6,
    data3=5,
    data4=4,
    data5=9,
    data6=46,
    data7=3,
    data8=8,
    data9=16,
    data10=1,
    data11=14,
    data12=21,
    data13=47,
    data14=48,
    data15=45,
    freq=12000000, # The display creeps if this is too fast
    hsync_idle_low=False,
    hsync_front_porch = 40,
    hsync_pulse_width = 48,
    hsync_back_porch  = 40,
    vsync_idle_low=False,
    vsync_front_porch = 1,
    vsync_pulse_width = 31,
    vsync_back_porch  = 13,
    de_idle_high=False,
    pclk_idle_high=False,
    pclk_active_low=1,
)

display = rgb_display.RGBDisplay(
    data_bus=bus,
    display_width=_WIDTH,
    display_height=_HEIGHT,
    backlight_pin=2,
    backlight_on_state=rgb_display.STATE_PWM,
    color_space=lv.COLOR_FORMAT.RGB565,
    rgb565_byte_swap=False
)

display.set_power(True)
display.init()
display.set_backlight(100)
# display.set_rotation(lv.DISPLAY_ROTATION._90)

# For reference, the I2C IRQ is on pin 38
I2C_BUS = I2C.Bus(
    host=1,
    scl=20,
    sda=19,
    freq=400000,
    use_locks=False
)

TOUCH_DEVICE = I2C.Device(
    I2C_BUS,
    dev_id=gt911.I2C_ADDR,
    reg_bits=gt911.BITS
)

indev = gt911.GT911(TOUCH_DEVICE)

if indev.hw_size != (_WIDTH, _HEIGHT):
    print('setting display size to the touch controller')
    print('old size:', indev.hw_size)
    fc = indev.firmware_config
    fc.width = _WIDTH
    fc.height = _HEIGHT
    fc.save()
    print('new size:', indev.hw_size)
    del fc

indev.enable_input_priority()
kdschlosser commented 4 weeks ago

the gt911 driver has all kinds of extra goodies you can adjust.

you have these properties you can adjust using the firmware_config just remember to call save after making any changes. width height noise_reduction touch_press_level touch_leave_level pad_left pad_right pad_top pad_bottom

There are more adjustments available but I have not added the code to alter them. I will have to look over them again and see if any of them are useful.

MitchBradley commented 4 weeks ago

The new driver is loading without error and the backlight turns on, but nothing displays on the screen.

If there are API changes, where can I find out what they are?

MitchBradley commented 4 weeks ago

I am using exactly the driver code that you posted above.

MitchBradley commented 4 weeks ago

Here is the simplified code I used to test, after getting no display from my app:

import crowpanel7_init

import lvgl as lv
scrn = lv.screen_active()
scrn.set_style_bg_color(lv.color_hex(0xffb6c1), 0)

slider = lv.slider(scrn)
slider.set_size(300, 50)
slider.center()

label = lv.label(scrn)
label.set_text('HELLO WORLD!')
label.align(lv.ALIGN.CENTER, 0, -50)

import task_handler
th = task_handler.TaskHandler()
MitchBradley commented 4 weeks ago

As a test, I reverted to the old .bin file, added the disp=-1 line back to the driver, and things worked correctly. My app displayed widgets as expected and the test code above created a visible label and slider.

kdschlosser commented 4 weeks ago

does your display have a "disp" pin? If it does pass that pin as the power pin to the display driver. the disp pin IS the power pin. The disp pin might be connected to an IO expander and that becomes a little bigger animal to get working but ultimately that pin need to be set to high in order for the display to turn on. The backlight is a separate circuit and will turn on and off regardless of what the power to the display is set at.

MitchBradley commented 4 weeks ago

Okay I will check the schematic tomorrow and see what I can find.

MitchBradley commented 4 weeks ago

There is no DISP pin that I can find. All of the controllable display pins are reflected in the rgbdisplay parameters.

image

image

The schematic shows some circuitry for turning off the display power via an I2C expander, but those components are not populated and the voltmeter shows 3V3 on the TFT-3V3 rail, downstream of where the unpopulated switching mosfet would be.

MitchBradley commented 4 weeks ago

Solved it

The old rgb_bus.c has this line that the new one omits:

        self->panel_io_config.disp_gpio_num = (int)args[ARG_disp].u_int;

It sets that field in panel_io_config to -1 in conjunction with disp=-1 in the python display init file.

ESP_IDF:esp_lcd_panel_rgb.c:lcd_rgb_panel_configure_gpio() has this at the very end:

    // disp enable GPIO is optional
    if (panel_config->disp_gpio_num >= 0) {
        gpio_hal_iomux_func_sel(GPIO_PIN_MUX_REG[panel_config->disp_gpio_num], PIN_FUNC_GPIO);
        gpio_set_direction(panel_config->disp_gpio_num, GPIO_MODE_OUTPUT);
        esp_rom_gpio_connect_out_signal(panel_config->disp_gpio_num, SIG_GPIO_OUT_IDX, false, false);
    }

The new rgb_bus.c leaves the disp_gio_num field uninitialized, so its value is indeterminate, probably 0. If it is >= 0 , the configuration code will mux that GPIO as the disp pin. If that GPIO happens to have been used for something else, that other function will be overridden. For this hardware, GPIO0 is PCLK, so the likely value of 0 will end up disabling PCLK.

I added this line to rgb_bus.c and it started working:

        self->panel_io_config.disp_gpio_num = -1;;

I think that is not the correct fix, but it proves that the cause of the problem is not setting that struct field. I get the impression that you intended to pass in the information via rgb_panel_t, but it seems like some plumbing is missing, as there is no way to set that field with the new code.

kdschlosser commented 4 weeks ago

it actually is the correct fix. can you submit a PR for it?

MitchBradley commented 4 weeks ago

Sure, I'll PR it tomorrow