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

Sluggish Touch events proportionate to the amount of screen draw going on. (BSP-627) #492

Closed
1 task done
wreyford opened this issue Jan 24, 2025 · 9 comments · Fixed by #493
Closed
1 task done
Labels
Area: LVGL related to LVGL and LVGL port Type: Bug Something isn't working

Comments

@wreyford
Copy link

Board

ESP32S3 F16 R8

Hardware Description

Custom PCB
LCD ILI9488 over Intel 8080 bus. (480x320)
TOUCH FT5X06

IDE Name

VSCode Insiders

Operating System

Windows 10

Description

Before migrating our code to use the esp_lvgl_port component touch was responsive. I directly linked to interrupt on CPT touch, and managed touch events as they occurred.

In lvgl_port component both the touch-events and lvgl-events are added to the 100 deep lvgl_queue. (lvgl_port_task private function).

This design of both touch and other events in the same queue appears to be the reason for delayed touch or missed touch events when there are many LVLG events on the queue.

My question to the developers - can we split the touch and other events and manage separately?

Sketch

Something like:

if (event.type == LVGL_PORT_EVENT_TOUCH) {
    process_touch_event(event);  // Handle touch events immediately
} else {
    process_generic_event(event);  // Handle other events
}

i.e. before adding to the event queue, and then splitting the queues.

Other Steps to Reproduce

Other tweaks I have made:
I increased the LVLG Task priority from 4 to 6.
I have reduced the task_max_sleep_ms to 1
I have tried to only update digits (areas) that actually change on the screen, thus reducing events, but overcomplicating our code.

This has resulted in a minor improvement, but the device was nice and responsive to touch before I upgraded, vs the way it is now.

The esp_lvgl_port is well written, allows for ease of use in adding devices LCD, touch etc, and integrates perfectly with RTOS, thus we decided to move.
I do not see us going back to our old implementation, as porting opur code took more than a month.
I hope you can help with a solution.

I have checked existing issues, README.md and ESP32 Forum

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@wreyford wreyford added Status: Awaiting triage Type: Bug Something isn't working labels Jan 24, 2025
@github-actions github-actions bot changed the title Sluggish Touch events proportionate to the amount of screen draw going on. Sluggish Touch events proportionate to the amount of screen draw going on. (BSP-627) Jan 24, 2025
@espzav
Copy link
Collaborator

espzav commented Jan 24, 2025

Hello @wreyford, you can try to remove interrupt pin from touch and then it will be handled together with other events - usually after timeout (task_max_sleep_ms). It is not solution, but it can help.

In your proposal, you mean put this function to else (like function in your proposal process_generic_event())?

@wreyford
Copy link
Author

wreyford commented Jan 24, 2025

No sorry for not being clear.
Try to split the screen and touch events before adding to line 238 esp_lvgl_port.c:

xQueueReceive(lvgl_port_ctx.lvgl_queue, &event, wait);

Thus touch and other events managed separately?
I'm not sure where and how the events are added to the queue in the first place.
Thus one queue for touch events and another for other events.

As the lvgl_queue queue is 100 deep, if touch is last event and queue near full, it has to wait for all other events to be processed before it is processed.

@espzav
Copy link
Collaborator

espzav commented Jan 24, 2025

Oh, I understand. It seems that there will be better to use Event Group instead of Queue. It will be faster and processing all in one round.

@wreyford
Copy link
Author

It looks as though all events are passed to:

esp_err_t lvgl_port_task_wake(lvgl_port_event_type_t event, void *param)
{
    if (!lvgl_port_ctx.lvgl_queue) {
        return ESP_ERR_INVALID_STATE;
    }

    lvgl_port_event_t ev = {
        .type = event,
        .param = param,
    };

    if (xPortInIsrContext() == pdTRUE) {
        BaseType_t xHigherPriorityTaskWoken = pdFALSE;
        xQueueSendFromISR(lvgl_port_ctx.lvgl_queue, &ev, &xHigherPriorityTaskWoken);
        if (xHigherPriorityTaskWoken) {
            portYIELD_FROM_ISR( );
        }
    } else {
        xQueueSend(lvgl_port_ctx.lvgl_queue, &ev, 0);
    }

    return ESP_OK;
}

So we can branch here, and then add to touch_queue and events_queue respectively.

Or else you can try optimizing the current lvgl port_task to prioritize touch events over other events.

while (lvgl_port_ctx.running) {
    lvgl_port_event_t event;
    TickType_t wait = pdMS_TO_TICKS(task_delay_ms) >= 1 ? pdMS_TO_TICKS(task_delay_ms) : 1;

    // Check for touch events first
    if (xQueuePeek(lvgl_port_ctx.lvgl_queue, &event, wait) == pdPASS && event.type == LVGL_PORT_EVENT_TOUCH) {
        xQueueReceive(lvgl_port_ctx.lvgl_queue, &event, 0); // Remove the event from the queue
        if (lvgl_port_lock(0)) {
            xSemaphoreTake(lvgl_port_ctx.timer_mux, portMAX_DELAY);
            if (event.param != NULL) {
                lv_indev_read(event.param);
            }
            xSemaphoreGive(lvgl_port_ctx.timer_mux);
            lvgl_port_unlock();
        }
    } else if (xQueueReceive(lvgl_port_ctx.lvgl_queue, &event, wait) == pdPASS) {
        // Handle other events (display or user)
        if (event.type == LVGL_PORT_EVENT_DISPLAY) {
            // Process display events (e.g., lv_timer_handler())
            if (lv_display_get_default() && lvgl_port_lock(0)) {
                task_delay_ms = lv_timer_handler();
                lvgl_port_unlock();
            }
        }
    }

    if (task_delay_ms == LV_NO_TIMER_READY) {
        task_delay_ms = lvgl_port_ctx.task_max_sleep_ms;
    }
    vTaskDelay(1); 
}

@espzav
Copy link
Collaborator

espzav commented Jan 24, 2025

Did you try your solution? Did it fix your issue?
I tried implement the event group here: https://github.com/espressif/esp-bsp/pull/493/files Could you try it with your implementation please?

@wreyford
Copy link
Author

Thx, I tried your implementation, but now I have no touch at all.
I'm initializing my touch as follows:

 ESP_LOGI(TAG, "Init I2C BUS NUM 0.\n");
    /* Initialize I2C bus, used for CTP, LCD_BL, VDC_Mains, REGB_LED (3Pin)*/
    if (loadassist_i2c_init(I2C_NUM_0, 400 * 1000) != ESP_OK)
    {
        ESP_LOGI(TAG, "Error with init of Init I2C BUS NUM 0.\n");
    }

    esp_lcd_panel_io_handle_t tp_io_handle = NULL;
    esp_lcd_panel_io_i2c_config_t tp_io_config = ESP_LCD_TOUCH_IO_I2C_FT5x06_CONFIG();

    ESP_LOGI(TAG, "Initialize touch IO (I2C)");

    /* Touch IO handle */
    ESP_RETURN_ON_ERROR(esp_lcd_new_panel_io_i2c((esp_lcd_i2c_bus_handle_t)LOADASSIST_I2C_NUM, &tp_io_config, &tp_io_handle), TAG, "Failed to init touch IO (I2C)");

    esp_lcd_touch_config_t tp_cfg = {
        .x_max = LCD_HEIGHT,
        .y_max = LCD_WIDTH,
        .rst_gpio_num = -1,
        .int_gpio_num = GPIO_INPUT_IO_CTP_INT, 
        .levels = {                                 //Added levels AJW 2024-11-03 12:16
            .reset = 0,
            .interrupt = 0,
        },
        .flags = {
            .swap_xy = 1,
            .mirror_x = 1,
            .mirror_y = 0,
        },
        //.interrupt_callback = ctp_gpio_isr_handler,
    };

    esp_lcd_touch_handle_t tp = NULL;
    ESP_LOGI(TAG, "Initialize LCD touch controller FT5X06"); //Appears to work in place of FT6x36
    ESP_RETURN_ON_ERROR(esp_lcd_touch_new_i2c_ft5x06(tp_io_handle, &tp_cfg, &tp), TAG, "Failed to init FT5X06");

@wreyford
Copy link
Author

Ok so turns out it was part my fault.
I edited out xSemaphoreTake(lvgl_port_ctx.timer_mux, portMAX_DELAY); when I manually applied your changes.
I ended up modifying esp_vgl_port as follows ( on top of your changes ):

static void lvgl_port_task(void *arg)
{
    //lvgl_port_event_t event;
    EventBits_t events = 0;//
    uint32_t task_delay_ms = 0;
    lv_indev_t *indev = NULL;

    /* Take the task semaphore */
    if (xSemaphoreTake(lvgl_port_ctx.task_init_mux, 0) != pdTRUE) {
        ESP_LOGE(TAG, "Failed to take LVGL task sem");
        lvgl_port_task_deinit();
        vTaskDelete( NULL );
    }

    /* LVGL init */
    lv_init();
    /* Tick init */
    lvgl_port_tick_init();

    ESP_LOGI(TAG, "Starting LVGL task");
    lvgl_port_ctx.running = true;
    while (lvgl_port_ctx.running) {
        /* Wait for queue or timeout (sleep task) */
        TickType_t wait = (pdMS_TO_TICKS(task_delay_ms) >= 1 ? pdMS_TO_TICKS(task_delay_ms) : 1);
       // xQueueReceive(lvgl_port_ctx.lvgl_queue, &event, wait);
       //xEventGroupWaitBits(lvgl_port_ctx.lvgl_events, 0xFF, true, false, wait);//
       events = xEventGroupWaitBits(lvgl_port_ctx.lvgl_events, 0xFF, pdTRUE, pdFALSE, wait);


        if (lv_display_get_default() && lvgl_port_lock(0)) {

            /* Call read input devices */
            //if (event.type == LVGL_PORT_EVENT_TOUCH) {
                if (events & LVGL_PORT_EVENT_TOUCH) { //
                xSemaphoreTake(lvgl_port_ctx.timer_mux, portMAX_DELAY);
                // if (event.param != NULL) {
                //     lv_indev_read(event.param);
                // } else {
                //     indev = lv_indev_get_next(NULL);
                //     while (indev != NULL) {
                //         lv_indev_read(indev);
                //         indev = lv_indev_get_next(indev);
                //     }
                indev = lv_indev_get_next(NULL);
                while (indev != NULL) {
                    lv_indev_read(indev);
                    indev = lv_indev_get_next(indev);
                }
                xSemaphoreGive(lvgl_port_ctx.timer_mux);
            }

            ///* Handle LVGL */
            //task_delay_ms = lv_timer_handler();
             /* Handle LVGL if DISPLAY event triggered or periodically */
            if (events & LVGL_PORT_EVENT_DISPLAY || task_delay_ms == 0) {
                task_delay_ms = lv_timer_handler();
            }
            lvgl_port_unlock();
        } else {
            task_delay_ms = 1; /*Keep trying*/
        }

        if (task_delay_ms == LV_NO_TIMER_READY) {
            task_delay_ms = lvgl_port_ctx.task_max_sleep_ms;
        }

        /* Minimal dealy for the task. When there is too much events, it takes time for other tasks and interrupts. */
        vTaskDelay(1);
    }

    /* Give semaphore back */
    xSemaphoreGive(lvgl_port_ctx.task_init_mux);

    /* Close task */
    vTaskDelete( NULL );
}

Changes:
events = xEventGroupWaitBits(lvgl_port_ctx.lvgl_events, 0xFF, pdTRUE, pdFALSE, wait);

and:

///* Handle LVGL */
            //task_delay_ms = lv_timer_handler();
             /* Handle LVGL if DISPLAY event triggered or periodically */
            if (events & LVGL_PORT_EVENT_DISPLAY || task_delay_ms == 0) {
                task_delay_ms = lv_timer_handler();
            }
            lvgl_port_unlock();

The remainder stays the same.

And Now I can congratulate you.
Now we have a nice and responsive device again.
Fantastic!!!

@espzav
Copy link
Collaborator

espzav commented Jan 24, 2025

@wreyford I am sorry, I made a mistake in my code, now it should work. We will make review on it and release.

@wreyford
Copy link
Author

@espzav Thank you so much. Event Group was a fantastic idea, less overhead, and immediate response. Fantastic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: LVGL related to LVGL and LVGL port Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants