Skip to content

feat(ws-s3-touch): Add espp::WsS3Touch Board Support Package (BSP) component for the Waveshare ESP32-S3 TouchLCD hardware. #475

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

Merged
merged 2 commits into from
Jul 14, 2025

Conversation

finger563
Copy link
Contributor

@finger563 finger563 commented Jul 14, 2025

Description

  • Add new component for waveshare esp32-s3 touchlcd
  • Update docs
  • Update ci

Motivation and Context

  • I have one of these and it's nice and I'd like to use it :)

How has this been tested?

Building and running ws-s3-touch/example on a Waveshare ESP32-S3 TouchLCD and playing with it :)

Screenshots (if appropriate, e.g. schematic, board, console logs, lab pictures):

image image
ws-s3-touch-compressed.mp4
CleanShot 2025-07-13 at 22 01 56

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update
  • Hardware (schematic, board, system design) change
  • Software change

Checklist:

  • My change requires a change to the documentation.
  • I have added / updated the documentation related to this change via either README or WIKI

Software

  • I have added tests to cover my changes.
  • I have updated the .github/workflows/build.yml file to add my new test to the automated cloud build github action.
  • All new and existing tests passed.
  • My code follows the code style of this project.

…component for the Waveshare ESP32-S3 TouchLCD hardware.
Copy link

✅Static analysis result - no issues found! ✅

@finger563 finger563 requested a review from Copilot July 14, 2025 02:56
@finger563 finger563 self-assigned this Jul 14, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

A new board support package component for the Waveshare ESP32-S3 TouchLCD is introduced, including source code, documentation, example, and CI updates.

  • Add the espp::WsS3Touch singleton API and its implementation (display, touch, IMU, audio, buttons, board control)
  • Provide documentation (RST, Markdown) and Doxygen entries for the new component and example
  • Update GitHub workflows, idf_component.yml, and CMake files to include the new BSP and its example

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
components/ws-s3-touch/include/ws-s3-touch.hpp Public API definition for the WsS3Touch component
components/ws-s3-touch/src/*.cpp Implementation of display, touchpad, IMU, audio, button, and board methods
doc/en/ws_s3_touch.rst, index.rst, ws_s3_touch_example.md Documentation additions and toctree entries
doc/Doxyfile Added example and header inputs for Doxygen
components/ws-s3-touch/idf_component.yml Component manifest for ESP-IDF Component Manager
.github/workflows/build.yml, upload_components.yml CI updated to build and publish the new component and example
Comments suppressed due to low confidence (2)

components/ws-s3-touch/src/touchpad.cpp:55

  • [nitpick] Remove the trailing newline (\n) in the log format since the logger automatically appends a newline, to avoid a blank line in the output.
    logger_.error("could not update touch driver: {}\n", ec.message());

components/ws-s3-touch/src/video.cpp:1

  • [nitpick] Consider adding unit tests for SPI callbacks (lcd_spi_pre_transfer_callback, lcd_spi_post_transfer_callback) and write_lcd_* methods to ensure correct flag setting and queue behavior.
#include "ws-s3-touch.hpp"

//
// cppcheck-suppress constParameterCallback
static void lcd_spi_post_transfer_callback(spi_transaction_t *t) {
uint16_t user_flags = (uint32_t)(t->user);
Copy link
Preview

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casting a 32-bit user field to a 16-bit variable may truncate flags. Consider using a 32-bit or pointer-sized type (e.g. uintptr_t user_flags = reinterpret_cast<uintptr_t>(t->user);).

Suggested change
uint16_t user_flags = (uint32_t)(t->user);
uint32_t user_flags = (uint32_t)(t->user);

Copilot uses AI. Check for mistakes.

esp_err_t ret;
size_t length = (xe - xs + 1) * (ye - ys + 1) * 2;
if (length == 0) {
logger_.error("lcd_send_lines: Bad length: ({},{}) to ({},{})", xs, ys, xe, ye);
Copy link
Preview

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After logging a bad frame length, the code continues and may queue zero-length SPI transactions. Add an early return; inside this block to prevent invalid operations.

Suggested change
logger_.error("lcd_send_lines: Bad length: ({},{}) to ({},{})", xs, ys, xe, ye);
logger_.error("lcd_send_lines: Bad length: ({},{}) to ({},{})", xs, ys, xe, ye);
return;

Copilot uses AI. Check for mistakes.

bool WsS3Touch::initialize_touch(const WsS3Touch::touch_callback_t &callback) {
if (touchpad_input_) {
logger_.warn("Touchpad already initialized, not initializing again!");
return true;
Copy link
Preview

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] initialize_touch returns true on a redundant call, whereas other initialize_* methods return false if already initialized. Consider unifying the return semantics for consistency across the API.

Suggested change
return true;
return false;

Copilot uses AI. Check for mistakes.

@finger563 finger563 merged commit fa90840 into main Jul 14, 2025
86 of 87 checks passed
@finger563 finger563 deleted the feat/ws-s3-touch branch July 14, 2025 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant