Conversation
ESP32-S3 e-paper board (V2.3/V2.4, ED047TC1 960x540) with GT911 touch, PCF8563 RTC, SD card and battery sense. The display driver is derived from the PaperS3 one and uses epdiy's LILYGO T5 4.7 S3 board with the row-by-row render engine on the S3. That engine is not in upstream epdiy, so the epdiy dependency points at AdaSzi's fork of 2.1.3 (tag 2.1.3-t5s3.1). The new option is disabled by default and other boards build the same as before.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughAdds device configuration and module support for the LILYGO T5 4.7-inch E-Paper S3. Adds an ED047TC1 driver with a 4-bit framebuffer, fast and quality update modes, and panel power and clear operations. The display adapter converts luminance for the panel and selects refresh behavior based on update area and refresh state. Priority: ➖ Normal Merge Risk: 🔵 Low · up to A failed initial clear can leave the display in an incorrect state while startup reports success. Propagate that failure before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 8e9664de-f825-42ac-98bc-10a657c7f6d0
📒 Files selected for processing (11)
Devices/lilygo-t5-epd47-s3/CMakeLists.txtDevices/lilygo-t5-epd47-s3/LICENSE-Apache-2.0.mdDevices/lilygo-t5-epd47-s3/bindings/lilygo,t5s3-display.yamlDevices/lilygo-t5-epd47-s3/device.propertiesDevices/lilygo-t5-epd47-s3/lilygo,t5-epd47-s3.dtsDevices/lilygo-t5-epd47-s3/module.yamlDevices/lilygo-t5-epd47-s3/source/bindings/t5s3_display.hDevices/lilygo-t5-epd47-s3/source/drivers/t5s3_display.cppDevices/lilygo-t5-epd47-s3/source/drivers/t5s3_display.hDevices/lilygo-t5-epd47-s3/source/module.cppTactility/idf_component.yml
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
The hold deadline was compared with a plain less-than, so a deadline left from just before the 32 bit tick counter wraps kept every update in quality mode. Compare the signed difference, treat 0 as no hold and clear the deadline when the session cap is reached.
Use a built-in e-paper engine in the T5 4.7 Inch E-Paper S3 device Replaces the epdiy dependency with a minimal engine in the device folder. It only has what the display driver uses: the shift register control, the CKV pulses (RMT), the row transfer (LCD peripheral in i80 mode), the row by row update loop and the clear sequence. There are three update modes. Fast draws black and white only. Quality draws 16 levels and leaves white pixels that stay white alone, which is used for partial updates. Full also flashes those white pixels, which is used for the clear and the refresh. Without that split, partial quality updates left faint vertical lines on the screen. A fast update of a tile takes about 30 to 60 ms and a full quality refresh about 1.2 s. The two driver files are built with -O2, rows are converted 8 pixels at a time and rows without changes are skipped with one train of pulses. The row conversion has no hardware access. The pixel clock is a devicetree property (default 10 MHz). The engine and the quality waveform are derived from epdiy and are marked LGPL v3.0 or later, with a notice in THIRD-PARTY-NOTICES.md.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 63d4c784-91c7-496b-8d34-18ac93877087
📒 Files selected for processing (11)
Devices/lilygo-t5-epd47-s3/CMakeLists.txtDevices/lilygo-t5-epd47-s3/bindings/lilygo,t5s3-display.yamlDevices/lilygo-t5-epd47-s3/device.propertiesDevices/lilygo-t5-epd47-s3/lilygo,t5-epd47-s3.dtsDevices/lilygo-t5-epd47-s3/source/drivers/t5s3_display.cppDevices/lilygo-t5-epd47-s3/source/drivers/t5s3_display.hDevices/lilygo-t5-epd47-s3/source/drivers/t5s3_epd.cppDevices/lilygo-t5-epd47-s3/source/drivers/t5s3_epd.hDevices/lilygo-t5-epd47-s3/source/drivers/t5s3_epd_rows.hDevices/lilygo-t5-epd47-s3/source/drivers/t5s3_epd_waveform.hTHIRD-PARTY-NOTICES.md
💤 Files with no reviewable changes (1)
- Devices/lilygo-t5-epd47-s3/device.properties
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
A timeout while waiting for a transfer or a pulse only set a flag, so the rest of the update still waited for the full limit on every row. The row, phase and clear loops and the wait itself now stop once an update has failed, so a lost interrupt costs one timeout. A failed update no longer copies its rows to the back buffer, so the next update draws them again.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Propagate a failed panel clear from start(). · t5s3_display.cpp:271-278
Devices/lilygo-t5-epd47-s3/source/drivers/t5s3_display.cpp:271-278
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPropagate a failed panel clear from
start().
t5s3_epd_clear()can returnfalsewhen the engine fails during the full clear update.start()ignores that result, logs successful initialization, and returnsERROR_NONE. The driver can therefore report successful startup while the panel clear did not complete.Suggested fix
power_on(internal); - t5s3_epd_clear(); + if (!t5s3_epd_clear()) { + t5s3_epd_deinit(); + device_set_driver_data(device, nullptr); + free(internal); + return ERROR_RESOURCE; + } LOG_I(TAG, "Initialized (%dx%d)", T5S3_EPD_WIDTH, T5S3_EPD_HEIGHT);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: ed350322-91b1-4074-9029-6771b42ae599
📒 Files selected for processing (1)
Devices/lilygo-t5-epd47-s3/source/drivers/t5s3_epd.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- Devices/lilygo-t5-epd47-s3/source/drivers/t5s3_epd.cpp
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
1 participant
Adds the LILYGO T5 4.7 Inch E-Paper S3 as an incubating device: ESP32-S3-WROOM-1-N16R8, ED047TC1 960x540 panel, GT911 touch, PCF8563 RTC, SD card, battery sense. Product page: https://www.lilygo.cc/products/t5-4-7-inch-e-paper-v2-3 (PCB marking "Screen-4.7-S3 V2.4").
This board drives latch, start, output enable, mode and the panel power rails through a 74HCT4094 shift register. epdiy only has a render engine for that on the original ESP32 (I2S), not on the ESP32-S3, and its maintainers decided not to support this board (martinberlin/lv_port_esp32-epaper#11 (comment)).
So the device has its own minimal engine for this one panel in the device folder, as discussed in the review. It only contains what the display driver uses: the shift register control and power sequence, the CKV gate clock (RMT), the row transfer (LCD peripheral in i80 mode), the row by row update loop and the clear sequence. There is no dependency change:
Tactility/idf_component.ymlis the same as upstream and PaperS3 keeps using epdiy as before.Display
t5s3_epd.cpp/.h(about 600 lines),t5s3_epd_rows.h(row conversion, no hardware access) andt5s3_epd_waveform.h(waveform table). The static RAM of the engine is about 1.3 KB.t5s3_display.cppis the Tactility driver (derived from the PaperS3 one) with the same quality/fast policy.pixel-clock-hz, default 10 MHz).Licensing
The four engine files are derived from epdiy (LGPL v3.0) and are marked
LGPL-3.0-or-later, with an entry inTHIRD-PARTY-NOTICES.md. The display driver, devicetree and binding are Apache-2.0.Testing
Known gaps
Summary by CodeRabbit