Xinyuan-LilyGO / T-Display-S3

MIT License
731 stars 173 forks source link

Constantly getting i2c errors when using touch (nothing else connected to i2c bus) #168

Closed shyney7 closed 1 month ago

shyney7 commented 1 year ago

Im using the Lilygo T-Display-S3 CAP. Touch version and if I implement the touch functionality to my code I'm constantly getting the following i2c errors:

[383725][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1
[401620][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1
[403552][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1

The problem is also that as soon as the errors occur the whole process loop is slowed down. This is unacceptable. Nothing else is connected to the i2c bus and this seems to be a problem with every unit since all 4 units that I bought have this problem.

I'm not doing anything special I'm just changing a variable with the touch function. Another problem is that sometimes even if I don't touch the display the variable is incremented...


#define TOUCH_MODULES_CST_SELF
#include <Arduino.h>
#include "TFT_eSPI.h"
#include "pin_config.h" //default TDisplay-S3 pin occupation
#include "TouchLib.h"

//TFT
TFT_eSPI tft = TFT_eSPI();
TFT_eSprite sprite = TFT_eSprite(&tft);
//                                     wire, 18, 17, 0x15, 21
TouchLib touch(Wire, PIN_IIC_SDA, PIN_IIC_SCL, CTS820_SLAVE_ADDRESS, PIN_TOUCH_RES);

int deb = 0; //debounce touch
uint8_t currentUIwindow = 0;
void switchTouch();

void setup() {
  Serial.begin(115200);
  // init TFT and Touch
  pinMode(PIN_POWER_ON, OUTPUT);
  digitalWrite(PIN_POWER_ON, HIGH);
  pinMode(PIN_TOUCH_RES, OUTPUT);
  digitalWrite(PIN_TOUCH_RES, LOW);
  delay(500);
  digitalWrite(PIN_TOUCH_RES, HIGH);
  tft.begin();
  tft.fillScreen(TFT_BLACK);
  tft.setRotation(1);
  touch.setRotation(1);
  sprite.createSprite(320,170);
  sprite.setTextColor(TFT_WHITE,TFT_BLACK);
  sprite.fillSprite(TFT_BLACK);
  sprite.setSwapBytes(true);
  Wire.begin(PIN_IIC_SDA, PIN_IIC_SCL);
}

void loop() {
  switchTouch();
  delay(10);
}

void switchTouch() {

   if (touch.read()) {
    if (deb==0){
      deb=1;
      TP_Point t = touch.getPoint(0);
      if (t.x >= 160) {
        if (currentUIwindow >= 4 || currentUIwindow < 0) {
          currentUIwindow = 0;
        }
        else {
          ++currentUIwindow;
        }
      }
      if (t.x < 160) {
        if (currentUIwindow <= 0) {
          currentUIwindow = 4;
        }
        else {
          --currentUIwindow;
        }
      }
    }
  }
  else {
    deb=0;
  }

}
mhaberler commented 1 year ago

I'v seen this as well

have a look at lvgl i2c_manager Arduino has no concept of multicore - so it's fly by night or you take care yourself pretty sure one task doing i2c is rescheduled before the i2c transaction is done and another task also does some i2c io

shyney7 commented 1 year ago

I'v seen this as well

have a look at lvgl i2c_manager Arduino has no concept of multicore - so it's fly by night or you take care yourself pretty sure one task doing i2c is rescheduled before the i2c transaction is done and another task also does some i2c io

I dont understand why there are scheduling problems if nothing else is connected via i2c? Also I dont want to be forced to do RTOS or concurrency programming with all its pitfalls.

Edit: I also tried to run the touch functionality on core 0 since the loop function runs on core 1 by default. But still getting the same i2c errors.

mhaberler commented 1 year ago

how exactly can you run touch on core 0 and loop on core 1 if you are saying this is single-threaded?

why not post a repo for inspection

that said: FreeRTOS-based Arduino usually has some background tasks like for Wifi event handling you typically do not see them except if you dig deeper

shyney7 commented 1 year ago

With :

TaskHandle_t Task1;
xTaskCreatePinnedToCore(
      Task1code, /* Function to implement the task */
      "Task1", /* Name of the task */
      10000,  /* Stack size in words */
      NULL,  /* Task input parameter */
      0,  /* Priority of the task */
      &Task1,  /* Task handle. */
      0); /* Core where the task should run */

Void Task1code( void * parameter) {
  while(true) {
    if (touch.read()) {...
    (...)
  }
lewisxhe commented 1 year ago

Are you sure you are using the CST820 touch?

shyney7 commented 1 year ago

Are you sure you are using the CST820 touch?

Yes the touch_test example only works if I use #define TOUCH_MODULES_CST_SELF The touch functionality is also working as intended in my code but I'm constantly getting those i2c errors which also seems to block code execution irregularly for a short time. And also it seems that in irregular time intervals touch is being registered although nothing touched the display. Its really annoying and makes the use of the touch functionality unusable.

The Version No. on the pcb says: T_Display_S3 22-8-2 V1.2

lewisxhe commented 1 year ago

Oh! If using CST820, if the touch chip does not detect being pressed for a certain period of time, and ESP32 accesses the touch device, then CST820 will not return any data. This is mainly due to the access mechanism of CST820 touch, which is not a fault

lewisxhe commented 1 year ago

By the way, the CST820 is not a standard I2C device This is very annoying to me

shyney7 commented 1 year ago

This is the complete code btw: https://github.com/shyney7/TDisplay-S3_PMX_Ground

lewisxhe commented 1 year ago

You can add fully closed internal debugging printing in platformio.ini


build_flags =
    -DCORE_DEBUG_LEVEL=0 
shyney7 commented 1 year ago

Disabling debug messages doesn't fix the problem it just hides it. Also when the random touch registration occurs the values for the x and y coordinates are both 4095 or 3331 which is completely out of range of the real display size. At least I can now ignore touch values higher than 320 for y and 170 for x in my program. But the annoying i2c errors is what bothers me since the loop itself is being blocked for a short time repeatedly due to these errors.

Output with print of y coordinate registering withouth any physical touch operation:

[205323][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1
[209654][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1
[211877][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1
[216427][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1
4095
3331
3331
3331
3331
3331
3331
3331
3331
3331
3331
3331
3331
3331
3331
[218368][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1
4095
3331
3331
3331
3331
3331
3331
3331
3331
3331
3331
3331
3331
3331
3331
[219933][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1
[225431][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1
[227401][E][Wire.cpp:513] requestFrom(): i2cRead returned Error -1
shyney7 commented 1 year ago

Whats also weird is that if I keep touching the display the touch functionality works completely fine. But as soon as I dont use touch the i2c errors keep occuring.

lewisxhe commented 1 year ago

Oh! If using CST820, if the touch chip does not detect being pressed for a certain period of time, and ESP32 accesses the touch device, then CST820 will not return any data. This is mainly due to the access mechanism of CST820 touch, which is not a fault

I think I have already explained.

mhaberler commented 1 year ago

Whats also weird is that if I keep touching the display the touch functionality works completely fine. But as soon as I dont use touch the i2c errors keep occuring.

well touch changes relative timing it's likely a race condition

either use a mutex around I2C transactions, or try lvgl i2c_manager

shyney7 commented 12 months ago

Whats also weird is that if I keep touching the display the touch functionality works completely fine. But as soon as I dont use touch the i2c errors keep occuring.

well touch changes relative timing it's likely a race condition

either use a mutex around I2C transactions, or try lvgl i2c_manager

If nothing else except the touch IC is using the i2c bus why should there be a race condition? Nevertheless I used a pthread mutex and locked it before calling the touch functionality (running on core 0) and unlocked it after that, and I'm still getting the same i2c errors as shown above including touch registrations of 4095 and 3331.

//core 0 task for touch. Default loop is on core 1
void touchTask(void *pvParameters) {
  while (true) {
    if (pthread_mutex_trylock(&uiMutex) == 0) {
      switchTouch();
      delay(1);
      pthread_mutex_unlock(&uiMutex);
    }
  }
}
shyney7 commented 11 months ago

This problem was fixed in the espressif esp-bsp touch driver repo 3 weeks ago with this pull request: https://github.com/espressif/esp-bsp/pull/181

This is the issue that was created: https://github.com/espressif/esp-bsp/issues/178

Is it possible for lilygo to update the touchlib with this information?

I also managed to get rid of the i2c errors by only calling the touch functions when the touch interrupt pin is low (check is done in a if statement). I don't know if this is the best way of doing it. Tried doing it with an interrupt semaphore but was getting core dump issues.

Arielhh commented 9 months ago

Try the CST816S library. It works. In case you don't need all the functionality of that library you can modify it and remove the gestures.

github-actions[bot] commented 2 months ago

This issue is stale because it has been open for 30 days with no activity.

shyney7 commented 2 months ago

Try the CST816S library. It works. In case you don't need all the functionality of that library you can modify it and remove the gestures.

Do you mean this one? https://github.com/fbiego/CST816S

github-actions[bot] commented 1 month ago

This issue is stale because it has been open for 30 days with no activity.

github-actions[bot] commented 1 month ago

This issue was closed because it has been inactive for 14 days since being marked as stale.