[esp32-p4] esp_lcd_touch_gt911_read_data false positive release reports (BSP-633)

[chegewara]

Hi team, there is small issue with false positive release report with lvgl on esp32-p4 (and probably all boards with GT911). It is because driver does not follow programming guide

[0x814E]:
Bit7: Buffer status, 1 = coordinate (or key) is ready for host to read; 0 = coordinate (or key) is not ready and
data is not valid. After reading coordinates, host should configure this flag (or the entire byte) to 0 via I2C.

This small change seems to work, but you may find a better solution (maybe return something else than ESP_OK instead) https://github.com/espressif/esp-bsp/blob/7969b01008bf4097f3386e7d175ab5dbddbc9a71/components/lcd_touch/esp_lcd_touch_gt911/esp_lcd_touch_gt911.c#L209-L225

    uint8_t repeat = 0;

    assert(tp != NULL);
repeat:
    err = touch_gt911_i2c_read(tp, ESP_LCD_TOUCH_GT911_READ_XY_REG, buf, 1);
    ESP_RETURN_ON_ERROR(err, TAG, "I2C read error!");

    /* Any touch data? */
    if ((buf[0] & 0x80) == 0x00) {
        // touch_gt911_i2c_write(tp, ESP_LCD_TOUCH_GT911_READ_XY_REG, clear);       <---- dont clear buffer if data has not been read and processed

    esp_rom_delay_us(3000);    <----- with shorter delay it may take up to 10 loops, so 3ms seems to be optimal
    if(repeat++ < 3)
        goto repeat;

I am using v1.1.0

Thanks

Page 15 https://www.orientdisplay.com/pdf/GT911.pdf

[espzav]

Hi, I checked this issue. There shouldn't be delay, because the upper layer (caller) is responsible for calling this read in loop with delay and sleep the task. And we don't read data if data are not ready. I am not sure, if the clear is ok or not now.

[chegewara]

Ok, i will maintain own version then. Like I said, library is reporting false positive release events (read 0 touch) when it can't read from g911. Maybe the code should report -1 or something, but for me it was the quickest solution.

Thanks

PS maybe you don't read data when it's not ready, but you return 0 from code, which is interpreted as "no touch" in upper layer, which is wrong and clearing ESP_LCD_TOUCH_GT911_READ_XY_REG when its not ready to read is against G911 specification, as mentioned in OP

[federico-carbone]

I think I might found the issue here. I think I'm facing the same problem as OP, but it looks like that at line 313 in function esp_lcd_touch_gt911_get_xy the following code is executed:

/* Invalidate */
tp->data.points = 0;

This makes any following call to esp_lcd_touch_gt911_get_xy useless.

By moving the invalidation after line 259 in function esp_lcd_touch_gt911_read_data no more false release reports are returned:

/* Count of touched points */
touch_cnt = buf[0] & 0x0f;
if (touch_cnt > 5 || touch_cnt == 0) {
    touch_gt911_i2c_write(tp, ESP_LCD_TOUCH_GT911_READ_XY_REG, clear);
    /* Invalidate */
    tp->data.points = 0;
    return ESP_OK;
}

[PetrESP]

Hello @federico-carbone Could you please explain to me what exact behavior are you experiencing and which board you are using? I would like to recreate it and confirm your solution.

[federico-carbone]

Hello @PetrESP ,

I'm using a custom board with a UI built with slint. When I press the touch without releasing it, the UI keeps triggering press and release events.

Basically slint runs this on every render cycle (which can, but not only, be triggered by a touch interrupt):

esp_lcd_touch_read_data();

bool touchpad_pressed = esp_lcd_touch_get_coordinates(...);

Slint reference

In case of consecutive cycles, like for animations, the second call to esp_lcd_touch_read_data reads no data from gt911, and second call to esp_lcd_touch_get_coordinates returns false since the previous call to this function invalidates data.

Now, in my opinion, there are 2 solutions: 

• the one I proposed, which consists of not invalidating data

• the one OP proposed, which consists in returning a different return value from esp_lcd_touch_read_data in case data is not ready to be read (for me not ideal since I would anyway need to fix slint too)

(I don't know why this comment is not rendering correctly, sorry for the inconvenience)

[PetrESP]

I took a look into your original post @chegewara and I think that you are mistaking esp_lcd_touch_gt911_read_data with esp_lcd_touch_gt911_get_xy this is because the read_data function is not supposed to report the touch state but just the error code of sending and receiving data from GT911 over I2C. To get the pressed/released status you need to use the get_xy function. Yes, clearing the ESP_LCD_TOUCH_GT911_READ_XY_REG in this case is questionable but we only do so when it contains invalid data so it should not be an issue.

@federico-carbone Thank you for your explanation.

returns false since the previous call to this function invalidates data.

I don't think that this is correct because the invalidation tp->data.points = 0; only happends if this condition is met if ((buf[0] & 0x80) == 0x80). This means that it is only invalidated when coordinate (or key) is ready for host to read and if there is data ready to be read then the data structure will be updated in the same function. (since you are still touching the display then the value which you read should stay the same) Even in consecutive cycles when the data is not ready to be read then the value inside the data struct is not invalidated and should stay the same -> there should not be any false release. I've tried to recreate your issue with ESP-BOX-3 and even when confirming that I got it to the same state (subsequent calls to read_data) I was getting the correct behavior. Could you please take a look at our esp_lvgl_port and confirm that you are using this driver correctly?

Please tell me if this answers your issue or help further with the recreation.

[federico-carbone]

Hello @PetrESP ,

sorry for my mistake. Looks like the version I was relying were not updated. My original suggestion has been implemented here

Sorry for wasting your time.

[PetrESP]

No problem and glad it's fixed. 🙂 Waiting for @chegewara to answer but otherwise we will close this issue.