diy: add support for Waveshare Touch LCD 2

[trevarj]

The Waveshare Touch LCD 2 is an ESP32-S3 board with a capacitive touch screen and camera.

[JamieDriver]

Many thanks. I don't have one of these devices but from a quick look the changes look good. 👍

[trevarj]

@JamieDriver thank you! I am working on an enclosure design and will upload files, photos and instructions when I'm done 🙂

[JamieDriver]

Could you add a small section to diy/README.md similar to the other boards ? 🙏
I'm setting up a CI for it on our internal instance, so if you could push that as an additional commit I'll pull it over and squash it in.
I'll add our required bits in .gitlab-ci.yml and a canned default sdkconfig in the configs dir ... once that passes internal checks/builds etc. and we've updated the README I'll merge it in.
Many thanks, much appreciated!

[JamieDriver]

Also, can you get your working build, do idf.py menuconfig and export the minimal 'sdkconfig.defaults' file (by pressing 'D' and grabbing the build/defconfig file). Feel free to attach here or stick in the commit with the README - either way I'll copy it into my branch to use in ci to verify it builds at least ...
cheers.

[JamieDriver]

I'm using the attached to put together a ci build - not 100% sure if it's correct though as I don't have a device to test.
I started with the working tdisplay-procamera defaults, and changed to this in the menuconfig ...

If you remove build dir and sdkconfig file, and drop this in as sdkconfig.defaults, and build/flash - is it good ?
If it's not right, I'll take any correction you have - cheers.

defconfig.txt

[trevarj]

@JamieDriver going to commit the README additions shortly.

If you remove build dir and sdkconfig file, and drop this in as sdkconfig.defaults, and build/flash - is it good ?

It does work!

Here is the build/defconfig that I generated and the difference between the one you linked above (ex. flash size difference):

defconfig.txt

yours on left, mine on right

2c2
< CONFIG_BOOTLOADER_LOG_LEVEL_NONE=y
---
> CONFIG_APP_NO_BLOBS=y
6c6
< CONFIG_ESPTOOLPY_FLASHSIZE_8MB=y
---
> CONFIG_ESPTOOLPY_FLASHSIZE_16MB=y
15,28d14
< CONFIG_BT_ENABLED=y
< CONFIG_BT_NIMBLE_ENABLED=y
< CONFIG_BT_NIMBLE_MEM_ALLOC_MODE_EXTERNAL=y
< CONFIG_BT_NIMBLE_LOG_LEVEL_NONE=y
< CONFIG_BT_NIMBLE_MAX_CONNECTIONS=1
< CONFIG_BT_NIMBLE_HOST_TASK_STACK_SIZE=5632
< # CONFIG_BT_NIMBLE_ROLE_CENTRAL is not set
< # CONFIG_BT_NIMBLE_ROLE_BROADCASTER is not set
< # CONFIG_BT_NIMBLE_ROLE_OBSERVER is not set
< CONFIG_BT_NIMBLE_NVS_PERSIST=y
< # CONFIG_BT_NIMBLE_SM_LEGACY is not set
< CONFIG_BT_NIMBLE_SVC_GAP_DEVICE_NAME="j"
< CONFIG_BT_NIMBLE_GAP_DEVICE_NAME_MAX_LEN=11
< CONFIG_BT_NIMBLE_ATT_PREFERRED_MTU=517
31a18
> # CONFIG_ETH_USE_SPI_ETHERNET is not set
63d49
< CONFIG_LOG_DEFAULT_LEVEL_NONE=y
64a51,53
> CONFIG_MBEDTLS_ECP_RESTARTABLE=y
> CONFIG_NEWLIB_STDOUT_LINE_ENDING_LF=y
> CONFIG_NEWLIB_STDIN_LINE_ENDING_LF=y
74d62
< # CONFIG_OV5640_SUPPORT is not set
79a68,69
> CONFIG_TINYUSB_DESC_MANUFACTURER_STRING="Waveshare"
> CONFIG_TINYUSB_DESC_PRODUCT_STRING="S3 Touch LCD 2"

[JamieDriver]

Sorry - I'm being an idiot - I missed that you'd already included the config file! 🤯
I'll try your original one in our ci. ^ and thanks for the README.

[greenaddress]

Neat! Just one thing, I noticed from the pictures you are using vertical vs horizontal, and I can see the Jade ID is being cut off - it might work better with the UI maybe if you rotate the display 90 degrees (though I don't have a device to test it)

[trevarj]

it might work better with the UI maybe if you rotate the display 90 degrees

@greenaddress Originally I had that, but then the touch area is in the wrong spot. I saw there is a TODO to fix that in the code. Afterwards the vertical grew on me and I think it looks pretty cool!

Maybe it's possible to wrap the Jade ID?

[greenaddress]

good point, the touch code AFAIK is not ready for rotations

yeah maybe the jade id could scroll or wrap

[JamieDriver]

Hi,
I have pushed a branch here: https://github.com/Blockstream/Jade/tree/waveshare_diy. Please take a look.
It is your changes, with some small reformatting as required by our ci, and a couple of tiny tweaks.
If you are happy with my changes @trevarj I'll look to merge it to master.

I'll also take a look at the label you're talking about.

On that point - just to say none of the the devices we have ported to so far are 'portrait' - so the ui is not well tested in this format. While it tries to be dynamic, there may be screens that do do not look so good in this orientation - the truncated label may just be the first ...
Hopefully everything will still function correctly - this is just a heads up that some things may not look ideal on a screen that is tall and narrow. So have a look around the ui as best you can, in case anything is particularly awful or quick/easy to improve.

Many thanks.

[JamieDriver]

My generated config says:

CONFIG_DISPLAY_WIDTH=240
CONFIG_DISPLAY_HEIGHT=280

Is that right ? From the image the screen doesn't look that close to being square ?
Ah ok the full height is 320 but you've decreased by 40 for the touch buttons ? Ok that makes sense - I think maybe the photo is misleading me ;-)
(These being wrong would mess with the camera/qr-scanning screens, so if you think they are good then all is fine.)

[JamieDriver]

@trevarj - I've added a commit to the branch discussed here: #221 (comment) that should scroll that truncated id on the home screen.
Can you let me know if it looks ok. Thanks.

[trevarj]

@trevarj - I've added a commit to the branch discussed here: #221 (comment) that should scroll that truncated id on the home screen.
Can you let me know if it looks ok. Thanks.

Awesome, I will flash and let you know once I'm back at my PC.

[trevarj]

@JamieDriver

These being wrong would mess with the camera/qr-scanning screens

Camera and QRs work great. I already did a few testnet transactions!

Tried your branch (commit 8e39688) and get a panic:

Guru Meditation Error: Core  0 panic'ed (LoadProhibited). Exception was unhandled.

Core  0 register dump:
PC      : 0x42049b90  PS      : 0x00060530  A0      : 0x8200ad1a  A1      : 0x3fca28c0
--- 0x42049b90: gui_initialized at /COMPONENT_MAIN_DIR/gui.c:273

A2      : 0x00000001  A3      : 0x3c0e4043  A4      : 0x3c0d39a4  A5      : 0x3fca2970
A6      : 0x3fc9cca8  A7      : 0x0000032d  A8      : 0x00000000  A9      : 0x3fca2880
A10     : 0x00000000  A11     : 0x3c0d4785  A12     : 0x00000004  A13     : 0x0000032d
A14     : 0x3fca288c  A15     : 0x00000000  SAR     : 0x00000008  EXCCAUSE: 0x0000001c
EXCVADDR: 0x00000000  LBEG    : 0x40056f5c  LEND    : 0x40056f72  LCOUNT  : 0x00000000
--- 0x40056f5c: memcpy in ROM
0x40056f72: memcpy in ROM



Backtrace: 0x42049b8d:0x3fca28c0 0x4200ad17:0x3fca28e0 0x42048685:0x3fca2990 0x4204acf9:0x3fca29c0 0x4204afd6:0x3f--- 0x42049b8d: gui_initialized at /COMPONENT_MAIN_DIR/gui.c:273
0x4200ad17: jade_abort at /COMPONENT_MAIN_DIR/jade_abort.c:19
0x42048685: push_updatable at /COMPONENT_MAIN_DIR/gui.c:599 (discriminator 17)
0x4204acf9: gui_set_text_scroll at /COMPONENT_MAIN_DIR/gui.c:1523
0x4204afd6: make_status_bar at /COMPONENT_MAIN_DIR/gui.c:191
 (inlined by) gui_init at /COMPONENT_MAIN_DIR/gui.c:264

Tried e0e8fd4 as well (waveshare~2) and it also gives me a corrupt display. I will try to debug more later.
Did a quick sanity check on my original branch and that is working fine.

[JamieDriver]

Ok, ignore that 'scrolling' commit. I'll revisit it later - prob when I've got hold of one of these devices. We can proceed without it for now.

Other than rebasing to latest master, my changes should be:

• reformatting the idf-components file, and updating the 'dependencies.lock' files - should have no impact
• small reformat to the config defaults file - should have no impact on generated config - ie. should have no impact
• small reformat in the Kconfig file - should have no impact
• tweak your change to touchscreen.inc to only #include that file for this device (otherwise breaks other touchscreen builds where it has been included elsewhere) - should have no impact
• add a line in ota_defines.h so the board type is returned in get_versioninfo api call - should have no impact

Difficult to see what may have happened.
How does your branch look when rebased on latest master ? The four commits it will pull in are all ui-related so may have an impact - the last one maybe ? In what way is the display corrupt ?
Cheers.

[trevarj]

@JamieDriver seems that there were changes made to the .defaults that I provided. Here's the diff between my original branch and waveshare_diy:

Yours are the removed ones.

modified   configs/sdkconfig_display_waveshares3_touch_lcd2.defaults
@@ -4,8 +4,6 @@ CONFIG_BF20A6_SUPPORT=n
 CONFIG_BF3005_SUPPORT=n
 CONFIG_BOARD_TYPE_WS_TOUCH_LCD2=y
 CONFIG_BOOTLOADER_APP_ROLLBACK_ENABLE=y
-CONFIG_BOOTLOADER_LOG_LEVEL_INFO=y
-CONFIG_BOOTLOADER_LOG_LEVEL=3
 CONFIG_BOOTLOADER_WDT_ENABLE=n
 CONFIG_BUTTON_LONG_PRESS_HOLD_SERIAL_TIME_MS=100
 CONFIG_BUTTON_LONG_PRESS_TIME_MS=500
@@ -17,9 +15,6 @@ CONFIG_DEBUG_MODE=y
 CONFIG_ESPTOOLPY_FLASHMODE_QIO=y
 CONFIG_ESPTOOLPY_FLASHSIZE_16MB=y
 CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG=y
-CONFIG_ESP_CONSOLE_UART=y
-CONFIG_ESP_CONSOLE_UART_NUM=0
-CONFIG_ESP_CONSOLE_ROM_SERIAL_PORT_NUM=0
 CONFIG_ESP_DEFAULT_CPU_FREQ_MHZ_240=y
 CONFIG_ESP_ERR_TO_NAME_LOOKUP=n
 CONFIG_ESP_MAIN_TASK_STACK_SIZE=12288
@@ -35,7 +30,6 @@ CONFIG_ESP_WIFI_MBEDTLS_TLS_CLIENT=n
 CONFIG_ESP_WIFI_NVS_ENABLED=n
 CONFIG_ESP_WIFI_RX_IRAM_OPT=n
 CONFIG_ESP_WIFI_SOFTAP_SUPPORT=n
-CONFIG_ETH_ENABLED=n
 CONFIG_ETH_USE_SPI_ETHERNET=n
 CONFIG_FATFS_LFN_HEAP=y
 CONFIG_FATFS_VFS_FSTAT_BLKSIZE=4096
@@ -52,7 +46,6 @@ CONFIG_NEWLIB_STDOUT_LINE_ENDING_LF=y
 CONFIG_NT99141_SUPPORT=n
 CONFIG_OV2640_SUPPORT=n
 CONFIG_OV3660_SUPPORT=n
-CONFIG_OV5640_SUPPORT=y
 CONFIG_OV7670_SUPPORT=n
 CONFIG_OV7725_SUPPORT=n
 CONFIG_PARTITION_TABLE_CUSTOM=y
@@ -76,4 +69,4 @@ CONFIG_SPI_SLAVE_ISR_IN_IRAM=n
 CONFIG_TINYUSB_CDC_ENABLED=y
 CONFIG_TINYUSB_CDC_RX_BUFSIZE=64
 CONFIG_TINYUSB_DESC_MANUFACTURER_STRING="Waveshare"
-CONFIG_TINYUSB_DESC_PRODUCT_STRING="S3 Touch LCD 2"
\ No newline at end of file
+CONFIG_TINYUSB_DESC_PRODUCT_STRING="S3 Touch LCD 2"

[JamieDriver]

I think that is done by ./tools/check_default_configs.sh ?
That is supposed to order the config items and remove anything that is already a default.
(Our ci insists on this.)
The changes should make no difference to the generated 'sdkconfig' file, and hence the build.

I made this change to run it locally on just the one file (otherwise it takes forever!)

diff --git a/tools/check_default_configs.sh b/tools/check_default_configs.sh
index 32357d8d..5d3f2c23 100755
--- a/tools/check_default_configs.sh
+++ b/tools/check_default_configs.sh
@@ -12,17 +12,18 @@ export IDF_CCACHE_ENABLE=1
 rm -fr sdkconfig sdkconfig.defaults build managed_components
 
 idf.py set-target esp32s3
-for filename in production/*s3*.defaults configs/*s3*.defaults; do
+#for filename in production/*s3*.defaults configs/*s3*.defaults; do
+for filename in configs/*waveshares3*.defaults; do
     rm -fr sdkconfig sdkconfig.defaults
     idf.py -D SDKCONFIG_DEFAULTS="${filename}" reconfigure save-defconfig
     tail -n +4 sdkconfig.defaults | LC_ALL=C sort -o ${filename}
 done
 
-idf.py set-target esp32
-for filename in production/*.defaults configs/*.defaults; do
-    [[ $filename == *"s3"* ]] && continue
-    rm -fr sdkconfig sdkconfig.defaults
-    idf.py -D SDKCONFIG_DEFAULTS="${filename}" reconfigure save-defconfig
-    tail -n +4 sdkconfig.defaults | LC_ALL=C sort -o ${filename}
-done
+#idf.py set-target esp32
+#for filename in production/*.defaults configs/*.defaults; do
+#    [[ $filename == *"s3"* ]] && continue
+#    rm -fr sdkconfig sdkconfig.defaults
+#    idf.py -D SDKCONFIG_DEFAULTS="${filename}" reconfigure save-defconfig
+#    tail -n +4 sdkconfig.defaults | LC_ALL=C sort -o ${filename}
+#done
 rm -fr sdkconfig sdkconfig.defaults build managed_components

[JamieDriver]

Do you think one of them is responsible for 'corrupt display' ?
As I say, it should only be removing things that are going to take those values anyway, so the generated sdkconfig file (and hence the binary) should be unchanged...

[trevarj]

@JamieDriver you're right, it was not the config file!

If I revert 079df56 (flush ui) and 92ade1a (scroll ID) I am back to a working device.

Here is what I meant by corrupt display:

Thanks for helping me work through this!

[JamieDriver]

So to confirm, this image ^^ is with the 'scroll id' reverted, but with 'flush' still in ? ie. looks like is caused by the 'flush' commit ?
Could you comment out the line that commit adds and check if it's fixed ? sorry/thanks ;-)

[trevarj]

So to confirm, this image ^^ is with the 'scroll id' reverted, but with 'flush' still in ?

That's correct. Scroll was reverted (per your request originally) and 'flush' still there. Reverting 'flush' solves the issue, and commenting too (just checked).

Let me know if I can do anything else to help debug.

[trevarj]

@JamieDriver hi, finally I fixed the scrolling issue for the device name. Please let me know if there is a better way.

PR here: #224

[JamieDriver]

Thanks, pulled these changes into waveshare_diy (moved a bit of it from one file to another) - not 100% sure about it tbh but pulled it in anyway as I agree it works [much better than my initial stab 😆 ] and I can think about it some more ...
( The other alternative ofc is to make the status-bar a little deeper so the text wraps (like the build id on the bottom right). I'll give it a little more thought. )
Also rebased this branch to current latest master - sorry it's a bit of a moving target atm!

[JamieDriver]

Really it's the scrambled display that's the blocker atm.
I have other devices here (some esp32, some s3, - lilygo and m5stack) and they all seem ok with current master - so not sure what's up with the waveshare. I guess in the worse case we #ifdef around the problem 'flush' ... ?

[trevarj]

@JamieDriver

not 100% sure about it tbh but pulled it in anyway as I agree it works

Me either, but it seems that the GUI activity "ex" with has_statusbar == false doesn't update "updatables" (after fixing the crash by setting the title node's activity to that. This fix sort of makes sense if you consider the title displayed in the statusbar "owned" by the dashboard activity (where I set the activity and enable scrolling) 🤷🏻‍♂️

Really it's the scrambled display that's the blocker atm.

Any tips for debugging on the device? I noticed putting a slight task delay before the flush, so it might be a timing issue?

[trevarj]

Removing taking/giving the semaphore works, and also adding an assertion somehow magically fixes the issue too:

    // Flush/clear display as soon as we're able
    JADE_ASSERT(gui_mutex);
    JADE_SEMAPHORE_TAKE(gui_mutex);
    display_flush();
    JADE_SEMAPHORE_GIVE(gui_mutex);

No clue why that would be the case, but commenting it causes the display failure, and then uncommenting fixes it. I get the feeling that I'm doing something wrong, but all I'm doing is changing the code and re-flashing with C-t-f in the idf.py flash monitor REPL.