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

Replaced TFT_eSPI with LovyanGFX #94

Merged
merged 3 commits into from
Dec 24, 2024
Merged

Replaced TFT_eSPI with LovyanGFX #94

merged 3 commits into from
Dec 24, 2024

Conversation

CoretechR
Copy link
Owner

I replaced TFT_eSPI in preparation for rev 5. There should be no change in performance with SPI, but a huge improvement when using 8-bit in the upcoming rev 5.

Touch is now also handled by LovyanGFX with more direct harwdare access than the Adafruit library.

Replaced GFX library in preparation for rev 5. There should be no change in performance with SPI, but a huge improvement when using 8-bit in rev 5.
Touch is now also handled by Lovyan with more direct harwdare access than the Adafruit library.
@CoretechR CoretechR requested review from KlausMu and removed request for KlausMu December 23, 2024 22:42
@KlausMu
Copy link
Collaborator

KlausMu commented Dec 24, 2024

To me, the PR looks good. I tested both env:esp32 and env:esp32_testboard.
When using SPI, LovyanGFX seems to provide 1 fps more (during swiping) than TFT_eSPI.

In platformio.ini, these lines also can be removed. They only have been used by TFT_eSPI.

	-D DISABLE_ALL_LIBRARY_WARNINGS=1
	-D TFT_WIDTH=${env.custom_screen_width}
	-D TFT_HEIGHT=${env.custom_screen_height}

And I think, both in tft_hal_esp32.h and main_boardtest.cpp these lines are not needed:

  const char* driver_name = "ILI9341";
  const char* mcu_name    = "ESP32";

@CoretechR
Copy link
Owner Author

Thanks for checking, Klaus! I thought I had seen driver_name in an example but yes, that's not used anywhere.

@KlausMu
Copy link
Collaborator

KlausMu commented Dec 24, 2024

You're welcome! You could also remove the driver_name and mcu_name from main_boardtest.cpp

@CoretechR CoretechR merged commit fb0d414 into main Dec 24, 2024
5 checks passed
@CoretechR CoretechR deleted the LovyanGFX branch December 24, 2024 10:58
@KlausMu
Copy link
Collaborator

KlausMu commented Dec 24, 2024

@CoretechR
I have to add some parameters to _touch_instance, otherwise I am not able to touch anything on the left side. All x values are above 63.
Can you confirm this behaviour and the change?

auto cfg = _touch_instance.config();
...
cfg.x_min = 0;
cfg.x_max = 239;
cfg.y_min = 0;
cfg.y_max = 319;

You could test all red areas. The area in the bottom left corner does not work without the change.
image

@CoretechR
Copy link
Owner Author

You are right, without the parameters there is an offset. Strange that this was not necessary with the Adafruit library.
I'll add the parameters like this:

    cfg.x_min = 0;
    cfg.x_max = SCR_WIDTH-1;
    cfg.y_min = 0;
    cfg.y_max = SCR_HEIGHT-1;

@KlausMu
Copy link
Collaborator

KlausMu commented Dec 24, 2024

Two more things:

1. Touch activity should reset timer
The setLastActivityTimestamp_HAL(); for touch events got lost.
It has to go before this line. Do you agree?

    if (tft.getTouch(&x, &y)) {
        data->state = LV_INDEV_STATE_PR;
        data->point.x = x;
        data->point.y = y;
        setLastActivityTimestamp_HAL();
    } else {

2. Problems going to sleep
I realized that when the timeout is set to a somewhat higher value, 3 minutes in my case, the device never went to sleep. It turned out that motion was detected, but definitely there was no motion. Sometimes it happened excactly 1 or 2 seconds before the device should go to sleep. I think this is an interference of the two I2C devices, touch and the LIS3DH.
I added this line to the touch_instance.config() and problems seem to be gone. Did you see something similar?

    cfg.bus_shared = true;

@CoretechR
Copy link
Owner Author

Oh, I forgot about setLastActivityTimestamp_HAL. That should definitely remain in my_touchpad_read() like you suggested.

I usually have the timout set very low so I did not notice that the device can't go to standby. Enabling bus_shared absolutely makes sense. Seems like an error that's hard to find later. Thank you!

@KlausMu
Copy link
Collaborator

KlausMu commented Dec 25, 2024

I still have problems with the device not going to sleep.
cfg.bus_shared = true; seems only to be of effect if the panel and touch are using the same bus. So it does not help us.

I added some code to see what the IMU detects.

Currently we have MOTION_THRESHOLD = 50;
With TFT_eSPI it looks like this. Three minutes inactivity, max value of 35.

...
motion: 17, max: 35
motion: 6, max: 35
motion: 10, max: 35
motion: 8, max: 35
Inactivity for 179900 ms
motion: 5, max: 35
Entering Sleep Mode. Goodbye.

With LovyanGFX and MOTION_THRESHOLD = 100;, it looks like this:

...
motion: 7, max: 69
motion: 6, max: 69
motion: 9, max: 69
motion: 11, max: 69
Inactivity for 179946 ms
motion: 4, max: 69
Entering Sleep Mode. Goodbye.

I would suggest to set MOTION_THRESHOLD = 100; (which is still very sensitive) and to remove cfg.bus_shared = true;

If you want to test by yourself:
Going back to TFT_eSPI:

git reset --hard e6a993b30012a8921bf327d0eb9ff9356d0bbab8

Going to LovyanGFX

git pull

@CoretechR
Copy link
Owner Author

I see, cfg.bus_shared is meant for SPI not i2c. But what does this have to do with the IMU? Do you mean that we are too close to the threshold? If so, I don't mind increasing the value. I usually have a very low standby timeout set so outliers in the IMU values don't matter too much. But over a few minutes, that might be a problem. Usually that would mean filtering the IMU output, but changing the threshold is much easier.

@KlausMu
Copy link
Collaborator

KlausMu commented Dec 25, 2024

Exactly. First I thought there were very high outliers, but this is not the case. The values are only slightly higher. Don't know how this is related to the graphics library / touch library. Maybe the problem dissapears when switching to parallel transmisson.

In my tests, MOTION_THRESHOLD = 100; solved the problem. Seems to be the easiest solution.

@CoretechR
Copy link
Owner Author

I put a few print statements into activityDetection() and tested it:

if(motion > MOTION_THRESHOLD) {
    setLastActivityTimestamp_HAL();
    Serial.print("Motion detected: x:");
    Serial.print(abs(accXold - accX));
    Serial.print(" y:");
    Serial.print(abs(accYold - accY));
    Serial.print(" z:");
    Serial.println(abs(accZold - accZ));
  }

A threshold of 50 is good enough for my remote if it is absolutely still. But just typing on my keyboard triggers the detection. Then again, holding it in one hand causes acceleration barely above the threshold. I think that's why I settled for the 50 value. Because at low timeout durations, normal hand motion keeps the device awake. I'll set it to 80 as a first step.

@KlausMu
Copy link
Collaborator

KlausMu commented Dec 27, 2024

BTW, LovyanGFX needs approx. 2,7k more memory compared to TFT_eSPI ... With the default configuration, now only 25.8k is left ...

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.

2 participants