Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esp_lvgl_port: change init config to use pdTICKS_TO_MS (BSP-275) #134

Closed
wants to merge 1 commit into from

Conversation

kutu
Copy link

@kutu kutu commented Feb 20, 2023

Disclamer: I'm newbie in C and lvgl

with 100hz tick rate setting
timer_period_ms = currently 5ms is 0 ticks

and task_max_sleep_ms 500ms is too much

I created rtos task where I show/hide colon symbol every 1sec (I'm using xTaskDelayUntil(&last_wakeup, pdMS_TO_TICKS(1000));)
I spend quite some time figuring out why my clock colon symbol was not rendering at rate of 1sec
with timer_period_ms=500ms setting, colon symbol was flashing every 1050ms, and when time debt of 50ms overlap 500ms, it was rendering again

so basically delay was:
1050 - colon appear
1050 - colon disappear
1050 - appear
450 - disappear
1050 - appear
1050 - disappear

I looked at other lvgl ports, and found that
lv_tick_inc() should be invoked as short as possible, usually every 1ms
and
lv_timer_handler() should be every 5ms

with proposed settings, tick will be 10ms and timer handler will be 50ms
my colon is now rendering consistently around every 1sec

The problem lays down in _lv_disp_refr_timer() function
If you configure lvgl settings to show fps counter or memory monitor, then timer is not paused, and you will end up with refresh rate that you set in settings - so no visible problems here
But if you don't use those, timer gets paused, and function lv_timer_handler() in lvgl_port_task() returns 0xffffffff task_delay_ms until next refresh, which is shortened by task_max_sleep_ms to 500ms

maybe better approach would be to put this numbers into KConfig

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions github-actions bot changed the title esp_lvgl_port: change init config to use pdTICKS_TO_MS esp_lvgl_port: change init config to use pdTICKS_TO_MS (BSP-275) Feb 20, 2023
@tore-espressif
Copy link
Collaborator

Hello @kutu ,
the ESP_LVGL_PORT_INIT_CONFIG() macro is there only to provide you with convenient initialization of esp_lvgl_port component.
In case you need to change the configuration for your custom project, you can define your own lvgl_port_cfg_t and pass it to lvgl_port_init.

Would that help your problem?
Cheers, Tomas

@kutu
Copy link
Author

kutu commented Mar 8, 2023

i currently do like this:

lvgl_port_cfg_t lvgl_cfg = ESP_LVGL_PORT_INIT_CONFIG();
lvgl_cfg.task_max_sleep_ms = pdTICKS_TO_MS(5);
lvgl_cfg.timer_period_ms = pdTICKS_TO_MS(1);

i think default config should work without issues out of the box, people who will stumble upon same issue, will have to dig into to understand that they need to change that 500ms task_max_sleep_ms

as i said in my pr, everything works fine if you use FPS meter, but then you remove it and your display needs to update something only every 1s (not often) then problem starts

@tore-espressif
Copy link
Collaborator

@kutu thanks for clarification!
@espzav could you please take a look when you're free?

@tore-espressif tore-espressif requested a review from espzav March 8, 2023 07:09
@espzav
Copy link
Collaborator

espzav commented Mar 8, 2023

Hello @kutu,
thank you for this PR. I think, that this depend on situation. I think, that the task should sleep as much as possible (we plan add more sleep for decreasing power consuption).
I disabled FPS and memory monitor. If I use for example touchscreen, there isn't problem with refreshing. When you want to any animations, you can use animations inside LVGL and then you shouldn't have this problem.

And about using the macro pdTICKS_TO_MS(5) it is ok to use, but when you change the tick rate in menuconfig, then the value 5 wouldn't be 500 ms. It can be 5000 ms or 50 ms. I don't know, if this is ok.

@kutu
Copy link
Author

kutu commented Mar 8, 2023

i choose pdTICKS_TO_MS to make sure that ticks delay is not 0

currently if i choose pdMS_TO_TICKS(1) it will be 0 ticks

i noticed issue in simple clock, when you have colon symbol flashing every 1sec, and nothing else is happening on the screen

if you try to make such example, you might notice that colon is flashing relatively every 1s, but every 10secs or so, you will notice it flashes with 500ms delay, and then all repeats.

while investigating my "colon flashing" issue, i looked into others implementation of lvgl port, and found this
https://github.com/lvgl/lv_port_esp32/blob/master/main/main.c#L155

    while (1) {
        /* Delay 1 tick (assumes FreeRTOS tick is 10ms */
        vTaskDelay(pdMS_TO_TICKS(10));

and https://github.com/lvgl/lv_port_esp32/blob/master/main/main.c#L213

lv_tick_inc(LV_TICK_PERIOD_MS);

which is 1ms

so, after changing default value of task_max_sleep_ms from 500ms to 50ms, issue was resolved

but i also understand that config ESP_LVGL_PORT_INIT_CONFIG is just here for quick start, and i can change it to my needs,

all i wanted is to resolve issue that i have to other people, who might not dig that far to find out why their colon is blinking not consistently

you can close this issue if this 500ms is intended and suit to most users

@tore-espressif
Copy link
Collaborator

For time sensitive applications, you can use esp_timer which has microsecond resolutions and very low latency.

lvgl_cfg.task_max_sleep_ms and lvgl_cfg.timer_period_ms are configurable options, so we won't update defaults for now.

The whole concept of power saving and related LVGL task timing will be revisited after LVGL v9 is released, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants