WT32 SC01 Plus #416
WT32 SC01 Plus #416
Conversation
📝 WalkthroughWalkthroughThis pull request introduces support for the Wireless Tag WT32-SC01 Plus ESP32 device. It includes a new ST7796 i8080 parallel display driver implementation with hardware initialization, LVGL integration, and lifecycle management. The device configuration aggregates I2C and SPI settings for display, touch input via FT6x36, SD card support, and PWM backlight control. A new device configuration option is added to the build system, and the CI/CD workflow is updated to include the new board in automated builds and releases. Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🪛 actionlint (1.7.9).github/workflows/build.yml27-27: description is required in metadata of "Build" action at "/home/jailuser/git/.github/actions/build-sdk/action.yml" (action) 77-77: description is required in metadata of "Build" action at "/home/jailuser/git/.github/actions/build-firmware/action.yml" (action) 90-90: description is required in metadata of "Bundle All" action at "/home/jailuser/git/.github/actions/bundle-firmware/action.yml" (action) 102-102: description is required in metadata of "Publish Firmware" action at "/home/jailuser/git/.github/actions/publish-firmware/action.yml" (action) 🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
Drivers/ST7796-i8080/README.md (1)
1-3: Consider expanding the documentation.The README provides only a basic description. Consider adding sections for:
- Hardware compatibility and wiring requirements
- Configuration examples
- Known limitations or gotchas
- References to datasheets or related documentation
This would help future users integrate the driver more easily.
Drivers/ST7796-i8080/Source/St7796i8080Display.h (1)
135-136: Reconsider the globalcreateDisplay()factory declarationThis header declares a global
createDisplay()returningDisplayDevice, but no implementation is shown here, and the WT32-SC01-Plus board already provides its owncreateDisplay()factory inDevices/wt32-sc01-plus/Source/devices/Display.cpp.To avoid confusion and potential undefined references or symbol clashes if this is ever used, either:
- Provide a concrete implementation for this factory (e.g., a generic ST7796-only configuration), or
- Remove the declaration and keep the factory board-specific.
Drivers/ST7796-i8080/Source/St7796i8080Display.cpp (1)
149-155: Remove unusedbuf1buffer allocation — it is never passed to LVGLThe
buf1buffer allocated at line 154 is never used. It is allocated withMALLOC_CAP_DMAbut is never passed tolvgl_port_add_disp(); instead,esp_lvgl_portallocates and manages its own DMA buffers internally using thebuffer_sizefield (which is in pixel count, not bytes). The buffer is simply deallocated instop()without serving any purpose.Remove the
buf1allocation fromstart()(lines 154–159) and its deallocation fromstop()(lines 200–202) inDrivers/ST7796-i8080/Source/St7796i8080Display.cpp. If you need to control transfer size or buffer placement, use the existingdisplay_cfgflags (buff_dma,buff_spiram,trans_size) instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/build-firmware.yml(1 hunks)Devices/wt32-sc01-plus/CMakeLists.txt(1 hunks)Devices/wt32-sc01-plus/Source/Configuration.cpp(1 hunks)Devices/wt32-sc01-plus/Source/devices/Display.cpp(1 hunks)Devices/wt32-sc01-plus/Source/devices/Display.h(1 hunks)Devices/wt32-sc01-plus/Source/devices/SdCard.cpp(1 hunks)Devices/wt32-sc01-plus/Source/devices/SdCard.h(1 hunks)Devices/wt32-sc01-plus/device.properties(1 hunks)Drivers/ST7796-i8080/CMakeLists.txt(1 hunks)Drivers/ST7796-i8080/README.md(1 hunks)Drivers/ST7796-i8080/Source/St7796i8080Display.cpp(1 hunks)Drivers/ST7796-i8080/Source/St7796i8080Display.h(1 hunks)Firmware/Kconfig(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-11-11T17:22:20.192Z
Learnt from: KenVanHoeylandt
Repo: ByteWelder/Tactility PR: 411
File: sdkconfig.board.waveshare-s3-touch-lcd-43:39-39
Timestamp: 2025-11-11T17:22:20.192Z
Learning: In Tactility sdkconfig.board.* files, board names (CONFIG_TT_BOARD_NAME) that include screen size measurements intentionally use escaped quotes (e.g., "WaveShare S3 Touch LCD 4.3\"") where the \" represents the inch symbol (") in the final board name string. This is not a syntax error but deliberate formatting.
Applied to files:
Firmware/Kconfig
📚 Learning: 2025-11-11T17:22:17.720Z
Learnt from: KenVanHoeylandt
Repo: ByteWelder/Tactility PR: 411
File: sdkconfig.board.waveshare-s3-lcd-13:39-39
Timestamp: 2025-11-11T17:22:17.720Z
Learning: In sdkconfig.board.* files for Tactility boards, CONFIG_TT_BOARD_NAME values that include display sizes in inches intentionally use a trailing escaped quote (e.g., "WaveShare S3 LCD 1.3\"") to display the literal inch symbol (") in the board name. This pattern is correct and should not be flagged as a string escaping error.
Applied to files:
Firmware/Kconfig
📚 Learning: 2025-10-29T14:45:02.914Z
Learnt from: fonix232
Repo: ByteWelder/Tactility PR: 380
File: Boards/m5stack-papers3/Source/devices/Display.cpp:9-18
Timestamp: 2025-10-29T14:45:02.914Z
Learning: For M5Stack PaperS3 in Boards/m5stack-papers3/Source/devices/Display.cpp, the GT911 touch configuration must use PAPERS3_EPD_HORIZONTAL_RESOLUTION (960) as yMax, PAPERS3_EPD_VERTICAL_RESOLUTION (540) as xMax, with swapXY=true and mirrorX=true. Alternative configurations (swapping the resolution parameters or setting swapXY=false) result in badly resolved touch positions. This specific configuration is empirically verified to work correctly.
Applied to files:
Devices/wt32-sc01-plus/Source/devices/Display.cpp
📚 Learning: 2025-10-26T12:44:21.858Z
Learnt from: KenVanHoeylandt
Repo: ByteWelder/Tactility PR: 391
File: Boards/CYD-2432S028RV3/Source/devices/Display.h:11-19
Timestamp: 2025-10-26T12:44:21.858Z
Learning: In the Tactility codebase, prefer `constexpr auto` for constant declarations (such as LCD pin configurations, resolutions, and buffer sizes) rather than explicit type annotations like `gpio_num_t`, `spi_host_device_t`, `uint16_t`, or `size_t`.
Applied to files:
Devices/wt32-sc01-plus/Source/devices/Display.cpp
📚 Learning: 2025-10-28T19:44:35.263Z
Learnt from: NellowTCS
Repo: ByteWelder/Tactility PR: 398
File: Drivers/ST7789-i8080/Source/St7789i8080Display.cpp:326-329
Timestamp: 2025-10-28T19:44:35.263Z
Learning: In the LilyGo T-Display S3 ST7789 i8080 display driver (Drivers/ST7789-i8080/Source/St7789i8080Display.cpp), both callback registrations are required: setting on_color_trans_done during panel IO creation in configurePanelIO() and calling esp_lcd_panel_io_register_event_callbacks() in initializeLvgl() after the LVGL display is created. This dual registration is intentional and necessary for proper operation on this hardware.
Applied to files:
Devices/wt32-sc01-plus/Source/devices/Display.cppDrivers/ST7796-i8080/README.mdDevices/wt32-sc01-plus/Source/Configuration.cppDrivers/ST7796-i8080/Source/St7796i8080Display.cppDrivers/ST7796-i8080/Source/St7796i8080Display.h
📚 Learning: 2025-10-27T22:33:23.840Z
Learnt from: KenVanHoeylandt
Repo: ByteWelder/Tactility PR: 394
File: Boards/WaveshareEsp32C6Lcd147/Source/WaveshareEsp32C6Lcd147.cpp:49-50
Timestamp: 2025-10-27T22:33:23.840Z
Learning: In the Tactility project, when configuring SPI for displays that use esp_lvgl_port, the `.lock` field must be set to `tt::lvgl::getSyncLock()` rather than `tt::hal::spi::getLock()`, because esp_lvgl_port creates and manages its own synchronization lock for display operations.
Applied to files:
Devices/wt32-sc01-plus/Source/devices/Display.cppDevices/wt32-sc01-plus/Source/devices/SdCard.cppDevices/wt32-sc01-plus/Source/Configuration.cppDrivers/ST7796-i8080/Source/St7796i8080Display.cppDrivers/ST7796-i8080/Source/St7796i8080Display.h
📚 Learning: 2025-10-28T05:42:03.394Z
Learnt from: NellowTCS
Repo: ByteWelder/Tactility PR: 398
File: Boards/LilygoTdisplayS3/Source/devices/Display.cpp:183-199
Timestamp: 2025-10-28T05:42:03.394Z
Learning: The LilyGo T-Display S3 board (Boards/LilygoTdisplayS3) requires a non-standard LVGL flush pattern in its ST7789 i8080 display driver. In st7789_send_color_cb, lv_display_flush_ready() must be called immediately after esp_lcd_panel_io_tx_color() with a vTaskDelay(1), rather than using standard DMA completion callbacks via esp_lcd_panel_io_register_event_callbacks. Standard DMA callback approaches cause display corruption on this specific board. This pattern was tested against multiple alternatives (callback registration, buffer tuning, clock reduction) and is the only stable solution for T-Display S3 hardware.
Applied to files:
Drivers/ST7796-i8080/README.mdDevices/wt32-sc01-plus/device.propertiesDevices/wt32-sc01-plus/Source/Configuration.cppDrivers/ST7796-i8080/Source/St7796i8080Display.cppDrivers/ST7796-i8080/Source/St7796i8080Display.h
📚 Learning: 2025-11-01T19:57:58.233Z
Learnt from: NellowTCS
Repo: ByteWelder/Tactility PR: 405
File: Tactility/Source/hal/usb/UsbTusb.cpp:12-12
Timestamp: 2025-11-01T19:57:58.233Z
Learning: In ESP-IDF projects like Tactility, the wear levelling header can be included directly as `#include <wear_levelling.h>` without requiring a directory prefix like `wear_levelling/wear_levelling.h`.
Applied to files:
Drivers/ST7796-i8080/CMakeLists.txt
📚 Learning: 2025-10-27T22:52:15.539Z
Learnt from: KenVanHoeylandt
Repo: ByteWelder/Tactility PR: 395
File: Boards/M5stackCardputerAdv/Source/devices/CardputerKeyboard.cpp:145-153
Timestamp: 2025-10-27T22:52:15.539Z
Learning: In the Tactility codebase, for LVGL device lifecycle methods (e.g., stopLvgl(), startLvgl()), the maintainer prefers using assert() to catch double-calls and programming errors rather than adding defensive null-checks. The intent is to crash on misuse during development rather than silently handle it, avoiding the overhead of defensive checks in every driver.
Applied to files:
Drivers/ST7796-i8080/Source/St7796i8080Display.cpp
🧬 Code graph analysis (6)
Devices/wt32-sc01-plus/Source/devices/Display.h (2)
Tactility/Include/Tactility/hal/display/DisplayDevice.h (1)
DisplayDevice(15-47)Devices/wt32-sc01-plus/Source/devices/Display.cpp (2)
createDisplay(22-41)createDisplay(22-22)
Devices/wt32-sc01-plus/Source/devices/Display.cpp (2)
Drivers/RgbDisplay/Source/RgbDisplay.h (1)
Configuration(22-58)Drivers/ST7796-i8080/Source/St7796i8080Display.h (1)
setBacklightDuty(120-124)
Devices/wt32-sc01-plus/Source/devices/SdCard.h (3)
Tactility/Source/service/sdcard/Sdcard.cpp (1)
sdcard(29-52)Tactility/Include/Tactility/hal/sdcard/SdCardDevice.h (1)
SdCardDevice(14-67)Devices/wt32-sc01-plus/Source/devices/SdCard.cpp (2)
createSdCard(8-23)createSdCard(8-8)
Devices/wt32-sc01-plus/Source/Configuration.cpp (3)
Tactility/Include/Tactility/Tactility.h (1)
hal(38-43)Devices/wt32-sc01-plus/Source/devices/Display.cpp (2)
createDisplay(22-41)createDisplay(22-22)Devices/wt32-sc01-plus/Source/devices/SdCard.cpp (2)
createSdCard(8-23)createSdCard(8-8)
Drivers/ST7796-i8080/Source/St7796i8080Display.cpp (1)
Drivers/ST7796-i8080/Source/St7796i8080Display.h (2)
St7796i8080Display(13-81)start(106-112)
Drivers/ST7796-i8080/Source/St7796i8080Display.h (4)
Drivers/ST7796-i8080/Source/St7796i8080Display.cpp (15)
St7796i8080Display(14-22)createI80Bus(24-50)createI80Bus(24-24)createPanelIO(52-85)createPanelIO(52-52)createPanel(87-144)createPanel(87-87)start(146-177)start(146-146)stop(179-211)stop(179-179)startLvgl(213-264)startLvgl(213-213)stopLvgl(266-272)stopLvgl(266-266)Tactility/Include/Tactility/hal/display/DisplayDevice.h (1)
DisplayDevice(15-47)Drivers/RgbDisplay/Source/RgbDisplay.h (1)
Configuration(22-58)Devices/wt32-sc01-plus/Source/devices/Display.cpp (2)
createDisplay(22-41)createDisplay(22-22)
🪛 Clang (14.0.6)
Devices/wt32-sc01-plus/Source/devices/Display.h
[error] 3-3: 'Tactility/hal/display/DisplayDevice.h' file not found
(clang-diagnostic-error)
Devices/wt32-sc01-plus/Source/devices/SdCard.h
[error] 3-3: 'Tactility/hal/sdcard/SdCardDevice.h' file not found
(clang-diagnostic-error)
Drivers/ST7796-i8080/Source/St7796i8080Display.h
[error] 3-3: 'Tactility/hal/display/DisplayDevice.h' file not found
(clang-diagnostic-error)
🔇 Additional comments (10)
.github/workflows/build-firmware.yml (1)
52-53: LGTM!The build matrix addition follows the existing pattern and correctly specifies the ESP32-S3 architecture for the WT32-SC01-Plus board.
Devices/wt32-sc01-plus/Source/devices/Display.h (1)
1-5: LGTM!The factory function declaration follows the established pattern for device-specific display creation. The include path and function signature are correct.
Firmware/Kconfig (1)
82-83: LGTM!The Kconfig option addition follows the established pattern and correctly registers the WT32 SC01 Plus as a selectable device.
Devices/wt32-sc01-plus/device.properties (1)
1-20: LGTM!The device properties are well-configured for the WT32 SC01 Plus hardware. The ESP32-S3 with 16MB flash and QUAD SPI RAM at 80MHz provides a solid foundation for LVGL-based applications with the 3.5" display.
Drivers/ST7796-i8080/CMakeLists.txt (1)
1-5: LGTM!The component registration correctly declares dependencies and follows the standard ESP-IDF CMake pattern. The inclusion of
esp_lcd_st7796indicates proper use of ESP-IDF's LCD component infrastructure.Devices/wt32-sc01-plus/Source/devices/SdCard.h (1)
1-7: LGTM!The header follows the established factory pattern for SD card device creation. The using declaration and function signature are consistent with the codebase conventions.
Devices/wt32-sc01-plus/Source/devices/SdCard.cpp (1)
8-23: I need to verify the initialization context further to ensure the SPI bus is properly initialized before the SD card device is created.I need to get more context around the SPI initialization in Configuration.cpp to verify the complete setup.
SPI bus for SD card is properly initialized in Configuration.cpp.
The verification confirms the SPI bus must be initialized by
spi_bus_initialize()before calling the SD SPI device initialization function. The codebase correctly follows this pattern: Configuration.cpp initializes SPI2_HOST with explicit pin mappings (MOSI=GPIO_NUM_40, MISO=GPIO_NUM_38, SCK=GPIO_NUM_39), while SdCard.cpp uses only the CS pin (GPIO_NUM_41) with GPIO_NUM_NC for MOSI, MISO, and SCK. This is the proper configuration approach and the SD card device will use the pins defined during the SPI bus initialization.No action required.
Devices/wt32-sc01-plus/CMakeLists.txt (1)
1-7: Component registration and dependencies look goodThe GLOB_RECURSE and
idf_component_registerwiring (including the ST7796-i8080, backlight, touch, VFS, and FATFS dependencies) is consistent and should integrate cleanly.Devices/wt32-sc01-plus/Source/devices/Display.cpp (1)
22-40: Display configuration wiring is coherentPin mapping, resolution constants, mirrorX, touch attachment, and the backlight duty callback all line up logically with the rest of the WT32-SC01-Plus configuration.
Devices/wt32-sc01-plus/Source/Configuration.cpp (1)
10-19: Device creation and boot init look fineCreating the display and SD card devices and initializing PWM backlight on GPIO 45 matches the display configuration and seems straightforward.
|
Regarding the driver... may want to check the other i8080 driver. |
that would be me 😅 |
Think that's all?
|
Updated with all the requested changes. |
|
Thanks, looks good to me. |
I guess that's just renaming the build-firmware.yml and changing the contents to match current plus new board? |
3 participants
Wireless Tag WT32 SC01 Plus and ST7796 i8080 driver.
Basic functionality atm, screen, touch, sd card, usb mass storage.
It has 4 external headers, debug, extended io, speaker, rs485. Good detailing of them here that i haven't done anything with yet.
Docs to come... soon ™️