Add Tronbyt S3 Device #24
Conversation
restore pixoticker env working, still no psram though i think. not working, but no watchdog on pixoticker build
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @tavdog, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request adds support for the Tronbyt S3 device, which is an ESP32-S3 version of the Pixoticker with PSRAM. I've added a new board definition file (boards/tronbyt-S3.json), modified extra_scripts/reset.py to include the new board, and updated platformio.ini to define the build environment for the Tronbyt S3. I've also included a default sdkconfig file for the Tronbyt S3.
Highlights
- New board support: Adds support for the Tronbyt S3 device, enabling it to be used with the project.
- Configuration files: Includes board definition, build environment, and default sdkconfig for the Tronbyt S3.
- Display definitions: Adds display definitions for the Tronbyt S3 device
Changelog
Click here to see the changelog
- .gitignore
- Added
.DS_Storeto the ignore list.
- Added
- boards/tronbyt-S3.json
- Created a new board definition file for the Tronbyt S3, including build settings, connectivity, debug configuration, frameworks, upload settings, URL, and vendor information.
- extra_scripts/reset.py
- Added
tronbyt-S3to thePRODUCTION_VERSIONdictionary.
- Added
- platformio.ini
- Commented out the
board = wroom32line and addedboard = tronbyt-S3to the[env:pixoticker]section. - Added a new environment
[env:tronbyt-S3]with board settings, build flags, and library dependencies.
- Commented out the
- sdkconfig.pixoticker
- Created a new sdkconfig file for the pixoticker
- sdkconfig.tronbyt-S3
- Created a new sdkconfig file for the tronbyt-S3
- sdkconfig.tronbyt-S3.defaults
- Created a new sdkconfig defaults file for the tronbyt-S3
- src/display.cpp
- Added display definitions for the TRONBYT_S3 device, including pin assignments for R1, G1, BL1, R2, G2, BL2, CH_A, CH_B, CH_C, CH_D, CH_E, LAT, OE, and CLK.
- Conditionally disabled double-buffering for TRONBYT_S3 devices.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A new board takes its place,
Tronbyt S3, with PSRAM grace,
Pixels dance and gleam,
In a vibrant stream,
Code's canvas, a digital space.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Tronbyt S3 device, an ESP32-S3 version of the Pixoticker with PSRAM. The changes include adding a board definition, updating the reset script, modifying the platformio.ini file, and adding a new sdkconfig file. Overall, the changes seem well-structured and necessary for supporting the new device.
Summary of Findings
- Missing sdkconfig.tronbyt-S3.defaults file: The platformio.ini file references a
sdkconfig.tronbyt-S3.defaultsfile, but this file is not included in the pull request. This file is necessary for configuring the ESP-IDF build for the Tronbyt S3 device. - Double buffering: Double buffering is disabled for the Tronbyt S3 device in
src/display.cpp. Consider if this is the correct approach, and document why this is the case, or re-enable it.
Merge Readiness
The pull request introduces support for a new device and is well-structured. However, the missing sdkconfig.tronbyt-S3.defaults file is a critical issue that must be addressed before merging. I recommend that the pull request not be merged until this file is added and the double buffering issue is resolved. I am unable to approve this pull request, and users should have others review and approve this code before merging.
| board_build.cmake_extra_args = | ||
| -DSDKCONFIG_DEFAULTS="sdkconfig.tronbyt-S3.defaults;" ; copied psram stuff from defaults |
There was a problem hiding this comment.
The sdkconfig.tronbyt-S3.defaults file is referenced here, but it is not included in the pull request. This file is necessary for configuring the ESP-IDF build for the Tronbyt S3 device. Please add this file to the pull request.
| "tidbyt-gen2": "v11/35369" | ||
| "tronbyt-S3": "v10/35833", | ||
| "tidbyt-gen2": "v11/35369", | ||
| "matrixportal-s3": "v11/35369" # just to shut up the errors. |
There was a problem hiding this comment.
The comment "# just to shut up the errors." is not very descriptive. Can you provide a more detailed explanation of why matrixportal-s3 is added here and what errors it prevents? This will help future maintainers understand the purpose of this line.
| board_build.extra_flags = | ||
| -DBOARD_HAS_PSRAM | ||
| board_build.cmake_extra_args = | ||
| -DSDKCONFIG_DEFAULTS="sdkconfig.tronbyt-S3.defaults;" ; copied psram stuff from defaults |
There was a problem hiding this comment.
The comment "; copied psram stuff from defaults" is not very descriptive. Can you provide a more detailed explanation of what "psram stuff" was copied and why? This will help future maintainers understand the purpose of this line.
src/display.cpp
Outdated
| #ifdef TRONBYT_S3 | ||
| false, // double-buffering | ||
| #else | ||
| true, // double-buffering |
There was a problem hiding this comment.
Double buffering is disabled for the Tronbyt S3 device. Is there a specific reason for disabling double buffering on this device? If so, please add a comment explaining why. If not, consider enabling it for better performance.
The tronbyt-s3 device is the esp32-s3 version of the pixoticker with psram