Issue with display initialization when trying to add support for a new board

[kylefmohr]

I'm working on adding support for that other USB board I mentioned, the ESP32-S3-GEEK, and when the display gets initialized, the beginning of the text is offset such that the first couple of characters get cut off, and the display is not entirely cleared (there is static instead of a black background for a portion of the top and right sides).

Like the first board I added, this board uses the ST7789 chip for the TFT, however it's only 240×135 pixels.

I took a look through the HardwareTFT.cpp implementation, but since there are parts of it I don't entirely understand, I figured I'd open an issue to discuss this rather than going in and blindly changing things 😊

Specifically, here: https://github.com/i-am-shodan/USBArmyKnife/blob/e9b5c9fe1662d511ac66e088748a6626cdb8c2cd/src/Devices/TFT/HardwareTFT.cpp#L78-L80

Wouldn't it make more sense to change these values based on the DISPLAY_WIDTH and DISPLAY_HEIGHT variables?

Same question regarding this section, although I'm not sure the right way to implement this: https://github.com/i-am-shodan/USBArmyKnife/blob/e9b5c9fe1662d511ac66e088748a6626cdb8c2cd/src/Devices/TFT/HardwareTFT.cpp#L59-L68

Maybe like

cfg.offset_x = int(DISPLAY_WIDTH / 7);
cfg.offset_y = int(DISPLAY_HEIGHT / 10);

I don't understand what dummy_read_pixel and dummy_read_bits do, but since they are currently set to the same value regardless, maybe that doesn't need to change at all.

Here is the addition to the `platformio.ini` file that I'm currently working with for this board

```ini [env:Waveshare-ESP32-S3-GEEK] extends = common board = esp32-s3-devkitc-1 board_build.flash_size = 16MB board_build.partitions = default_8MB.csv monitor_speed = 115200 build_flags = ${common.build_flags} -D ARDUINO_ARCH_ESP32S3 -D HAS_SD ; ESP32 Maurader -D USE_SD_MMC_INTERFACE ; ESP32 Maurader -D GENERIC_ESP32 ; ESP32 Maurader -D CONFIG_ASYNC_TCP_QUEUE_SIZE=128 ;;;;;;;; Pin Config for TFT ;;;;;;;; -D DISPLAY_TYPE_ST7789 -D DISPLAY_RST=9 -D DISPLAY_DC=8 -D DISPLAY_MOSI=11 -D DISPLAY_CS=10 -D DISPLAY_SCLK=12 -D DISPLAY_LEDA=7 -D DISPLAY_MISO=-1 -D DISPLAY_BUSY=-1 -D DISPLAY_WIDTH=240 -D DISPLAY_HEIGHT=135 -D TFT_WIDTH=135 -D TFT_HEIGHT=240 ;;;;;;;; Pin Config for SD ;;;;;;;; -D SD_MMC_D0_PIN=37 -D SD_MMC_D1_PIN=33 -D SD_MMC_D2_PIN=38 -D SD_MMC_D3_PIN=34 -D SD_MMC_CLK_PIN=36 -D SD_MMC_CMD_PIN=35 ;;;;;;;; Pin Config for Status LED and Button ;;;;;;;; -D BTN_PIN=0 -D NO_LED ;;;;;;;; End of Pin Config ;;;;;;;; lib_deps = ${common.lib_deps_core} h2zero/NimBLE-Arduino@^1.4.2 ; ESP32 Maurader mathertel/OneButton bitbank2/PNGdec@^1.0.1 lovyan03/LovyanGFX@^1.1.16 https://github.com/pololu/apa102-arduino ```

[i-am-shodan]

We need to make some changes to the design to better support TFT of different models and implementations. At the moment the model of the panel is being used as both a way to select what driver to load AND what equipment is in place. If we have two models that use the same panel but require different offsets we are scuppered. So it comes as no surprise that adding yet another ones will be a mess of #defines within that class.

I’ve got a comment (line 11) in that class to say in the future, today, we need to have one LGFX_Device per supported device. I think if you tried to implement it like that it would be much easier to get started and debug. Most drivers actually don't need many if any extra values.

To implement this we need to set #defines for:

• DEVICE_LILYGO_DONGLE_S3
• DEVICE_WAVESHARE…
• DEVICE_WAVESHARE_ESP32_GEEK

Then we create a new class for each device and surround it with #ifdefs. This then enables each device to have its own sizes and offsets etc - if required.

The ST7789 shouldn’t need us to set a memory size so we don’t need to worry about setting that for that board. The LGFX_Device implementation class for DEVICE_WAVESHARE_ESP32_GEEK won’t set it. Incidentally those values you mention came from the LilyGo development team so changing them seems like a bad idea - as does keeping them around for other future boards.

We don’t currently read from the display so those values are probably safe as is or could be omitted completely for your boards.

[i-am-shodan]

In the future we may need to actually pull in a different library for a new device. In that case we'll create a new XYZHardwareTFT.cpp file.

[i-am-shodan]

And each device will in all likelihood need a different XY offset value unrelated to the display size.

Some parts of a display are covered up with plastic or aren't actually usable - a case on the other waveshare where the corner is rounded.