lvgl / lvgl_esp32_drivers

Drivers for ESP32 to be used with LVGL
MIT License
330 stars 281 forks source link

ST7789: LVGL rotation support #182

Closed C47D closed 2 years ago

C47D commented 2 years ago

Use LVGL (hardware) rotation support instead of using the homegrown one. You might be interested in this one @arktrin, the lv_port_esp32 branch is the same as before, feat/new_driver_test.

We might want to set the rotated field of the lv_disp_drv_t with the symbol from the menuconfig, what do you think @tore-espressif, @arktrin ? I would rather not use it.

TODO:

C47D commented 2 years ago

Just checking on how we can update the display orientation at runtime, we might use the optional drv_update_cb, but that was introduced in 438ba35076bea1bec9d9831a71cbf94ee170af14, so AFAIK in LVGL v8. I think for LVGL v7 we might want to update it in the flush callback. What do you think?

EDIT Just found out this commit from 5 months ago (https://github.com/lvgl/lvgl_esp32_drivers/commit/dfe54e8573c71ab23395df33b959a5d4973fb7d8), we were thinking the same approach, will update that particular function to use it in LVGLv8

arktrin commented 2 years ago

If it makes lvgl_esp32_drivers project closer to lvgl v8 - I'm totally agree (support) with this PR :)

C47D commented 2 years ago

I will check that, some of the symbols still being used in this particular driver are no longer available in v8, such as LV_HOR_RES_MAX and LV_VER_RES_MAX.

This PR only aligns the rotation we did with the ones supported by LVGL.

arktrin commented 2 years ago

This PR only aligns the rotation we did with the ones supported by LVGL.

I do understand that. I'm just trying to force lvgl v8 :)

C47D commented 2 years ago

I know, I've been slow at adopting v8, but with the latest changes we've done I feel very close to achieve that.

I'm now working on changing the orientation at runtime.

I have just a few hours per week to dedicate to this repo, some more to the main repo, which has been taking most of those hours last weeks.

tore-espressif commented 2 years ago

@C47D thanks for all the energy you're giving it.

When it comes to rotation support, I'd implement it only in v8, as the drv_update_cb callback function is intended for this use (AFAIK).

I'll keep you updated

C47D commented 2 years ago

When it comes to rotation support, I'd implement it only in v8, as the drv_update_cb callback function is intended for this use (AFAIK).

That's a good point, I was thinking on 'detecting' the rotation change in the flush callback when using v7. I was about to test it but I'm unable to change the rotation at runtime, see last comments in https://github.com/lvgl/lvgl/issues/2972

The new function st7789_update_cb is meant to be the drv_update_cb, what do you think about it?

C47D commented 2 years ago

The display orientation is being set on the guiTask and it's changing periodically, for v7 I had to update the display orientation on the flush callback, and also update the display offsets.

I also added a helper to handle display dimensions in v8, we need to validate this driver running in v8 tho.

The driver on the video is using v7 with 240x240 dimensions, the other sizes need to be updated.

https://user-images.githubusercontent.com/11037705/154201063-d3152570-b8da-458a-a777-b1a95609b4bc.mp4

Or here if you're not able to run the video:

https://youtu.be/szIrNOcFWdE

C47D commented 2 years ago

@arktrin Your display should now be able to rotate at runtime (see 472f4d4)

Important note: The orientation values on the Kconfig were modified, so the orientation arrays on the displays needs to be updated once this PR gets accepted.

arktrin commented 2 years ago

@C47D Sorry, but I can't test this PR in the near future . Very busy with current full time projects.

meanwhile ... glad not to be forgotten by you :)

C47D commented 2 years ago

No problem, it's OK, I think the tests I did with my display are good for now.

And I don't, I will keep you in mind when working with v8.

EDIT The display and rotation works with LVGLv8! I had to do a patch on a lvgl_helper.c function, but everything else seems to work nicely. 😸

C47D commented 2 years ago

Hi @embeddedt, do you know a nicer way to handle fe2032b?

embeddedt commented 2 years ago

I would say that it's cleaner to refactor disp_driver_init to take an lv_disp_t * instead, but I don't know if that's possible.

C47D commented 2 years ago

I was thinking on hiding those if else blocks into a helper function, that way we don't need to change anything.

embeddedt commented 2 years ago

Hmm, looking at it again, I don't see how this will work correctly. drv is a parameter to the function so &drv will be a pointer to the stack, not the display. Did you test it?

C47D commented 2 years ago

Yes, and it seems to be working as expected.