Add power device (battery + power-off) for lilygo-tdeck-max#553
Open
Crazypedia wants to merge 3 commits into
Open
Add power device (battery + power-off) for lilygo-tdeck-max#553Crazypedia wants to merge 3 commits into
Crazypedia wants to merge 3 commits into
Conversation
Base board bring-up for the LilyGO T-Deck Max (ESP32-S3, 16 MB flash, 8 MB PSRAM), per the plan discussed in TactilityProject#539: display, touch, and keyboard only — power, LoRa radio, GPS, and haptics follow as separate PRs. New reusable drivers: - Drivers/GDEQ031T10: UC8253-based 240x320 1bpp e-paper panel with windowed partial refresh + full-refresh ghost gate, refresh runs on a background task. - Drivers/xl9555-module: XL9555 I2C IO expander as a kernel module with gpio_controller bindings (the board routes touch/keyboard resets and several power rails through it). Board specifics: - CST66xx touch (Hynitron 4-byte register protocol) implemented device-locally; vendor docs mislabel the part as CST328. - TCA8418 keyboard matrix (existing upstream driver) with backlight. - SD card as an espressif,esp32-sdspi devicetree node on the shared SPI2 bus; unused chip-selects (LoRa, SD) are deasserted before the EPD touches the bus. - incubating=true until SD storage is verified on this board. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughChangesThis pull request adds an Sequence Diagram(s)Sequence and flow diagrams are included in the hidden review stack artifact above. Related PRs: None specified. Suggested labels: enhancement, new-device, driver Suggested reviewers: Board and HAL maintainers familiar with ESP32 SPI/I2C drivers and LVGL integration Poem: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 10
🧹 Nitpick comments (1)
Drivers/xl9555-module/include/drivers/xl9555.h (1)
10-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider documenting the valid I2C address range.
addresshas no documented constraint (XL9555 typically uses 7-bit addresses in the0x20-0x27range depending on strap pins). A brief comment would help devicetree authors avoid misconfiguration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ebd69281-b331-45c1-8886-a47cca880b45
📒 Files selected for processing (28)
Devices/lilygo-tdeck-max/CMakeLists.txtDevices/lilygo-tdeck-max/Source/Configuration.cppDevices/lilygo-tdeck-max/Source/devices/Cst66xxTouch.cppDevices/lilygo-tdeck-max/Source/devices/Cst66xxTouch.hDevices/lilygo-tdeck-max/Source/devices/Display.cppDevices/lilygo-tdeck-max/Source/devices/Display.hDevices/lilygo-tdeck-max/Source/devices/TdeckmaxKeyboard.cppDevices/lilygo-tdeck-max/Source/devices/TdeckmaxKeyboard.hDevices/lilygo-tdeck-max/Source/devices/TdeckmaxPower.cppDevices/lilygo-tdeck-max/Source/devices/TdeckmaxPower.hDevices/lilygo-tdeck-max/Source/module.cppDevices/lilygo-tdeck-max/device.propertiesDevices/lilygo-tdeck-max/devicetree.yamlDevices/lilygo-tdeck-max/lilygo,tdeck-max.dtsDrivers/GDEQ031T10/CMakeLists.txtDrivers/GDEQ031T10/README.mdDrivers/GDEQ031T10/Source/Gdeq031t10Display.cppDrivers/GDEQ031T10/Source/Gdeq031t10Display.hDrivers/xl9555-module/CMakeLists.txtDrivers/xl9555-module/LICENSE-Apache-2.0.mdDrivers/xl9555-module/README.mdDrivers/xl9555-module/bindings/xlsemi,xl9555.yamlDrivers/xl9555-module/devicetree.yamlDrivers/xl9555-module/include/bindings/xl9555.hDrivers/xl9555-module/include/drivers/xl9555.hDrivers/xl9555-module/include/xl9555_module.hDrivers/xl9555-module/source/module.cppDrivers/xl9555-module/source/xl9555.cpp
Comment on lines
+507
to
+539
| bool Gdeq031t10Display::startLvgl() { | ||
| if (lvglDisplay != nullptr) { | ||
| return true; | ||
| } | ||
|
|
||
| lvglDisplay = lv_display_create(Gdeq031t10Display::WIDTH, Gdeq031t10Display::HEIGHT); | ||
| if (lvglDisplay == nullptr) { | ||
| return false; | ||
| } | ||
|
|
||
| lv_display_set_user_data(lvglDisplay, this); | ||
| lv_display_set_color_format(lvglDisplay, LV_COLOR_FORMAT_I1); | ||
|
|
||
| // The default colour theme renders accent-coloured text/icons that threshold | ||
| // to near-invisible on a 1bpp panel. Apply LVGL's monochrome theme (light | ||
| // background, dark foreground) so UI content shows as solid black on white. | ||
| lv_theme_t* baseTheme = lv_theme_mono_init(lvglDisplay, false, LV_FONT_DEFAULT); | ||
| // Chain a theme on top of the mono theme that disables the textarea cursor | ||
| // blink. A blinking cursor invalidates its region ~twice a second, and on | ||
| // e-paper every invalidation is a panel refresh, so text-entry screens (e.g. | ||
| // the Wi-Fi password field) flash continuously. The mono theme still applies | ||
| // first (it's the parent); themeApplyCallback only pins the cursor solid. | ||
| static lv_theme_t epaperTheme; | ||
| epaperTheme = *baseTheme; | ||
| lv_theme_set_parent(&epaperTheme, baseTheme); | ||
| lv_theme_set_apply_cb(&epaperTheme, &Gdeq031t10Display::themeApplyCallback); | ||
| lv_display_set_theme(lvglDisplay, &epaperTheme); | ||
| lv_display_set_render_mode(lvglDisplay, LV_DISPLAY_RENDER_MODE_FULL); | ||
| lv_display_set_buffers(lvglDisplay, renderFramebuffer.get(), nullptr, LVGL_I1_PALETTE_SIZE + FRAMEBUFFER_SIZE, LV_DISPLAY_RENDER_MODE_FULL); | ||
| lv_display_set_flush_cb(lvglDisplay, flushCallback); | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant interface and implementation.
git ls-files | rg '(^|/)(DisplayDevice\.h|Gdeq031t10Display\.cpp)$'
echo '--- DisplayDevice.h ---'
cat -n Tactility/Include/Tactility/hal/display/DisplayDevice.h | sed -n '1,260p'
echo '--- Gdeq031t10Display.cpp (relevant sections) ---'
cat -n Drivers/GDEQ031T10/Source/Gdeq031t10Display.cpp | sed -n '1,260p'
echo '---'
cat -n Drivers/GDEQ031T10/Source/Gdeq031t10Display.cpp | sed -n '260,620p'
echo '--- Search for startLvgl/start/queueRefresh/refreshTask/renderFramebuffer/initialized ---'
rg -n "startLvgl|queueRefresh|refreshTask|renderFramebuffer|initialized|lv_display_set_buffers|flushCallback|start\\(" Tactility Drivers -g '!**/build/**' -g '!**/dist/**'Repository: TactilityProject/Tactility
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Outline: Tactility/Source/lvgl/Lvgl.cpp ---'
ast-grep outline Tactility/Source/lvgl/Lvgl.cpp --view expanded || true
echo '--- Lvgl.cpp relevant sections ---'
cat -n Tactility/Source/lvgl/Lvgl.cpp | sed -n '1,220p'
echo '--- Gdeq031t10Display.h relevant sections ---'
cat -n Drivers/GDEQ031T10/Source/Gdeq031t10Display.h | sed -n '1,260p'
echo '--- Search for display start/startLvgl call sites ---'
rg -n "->startLvgl\\(|->start\\(|display->startLvgl\\(|display->start\\(" Tactility Drivers -g '!**/build/**' -g '!**/dist/**' | sed -n '1,200p'Repository: TactilityProject/Tactility
Length of output: 19199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Find Gdeq031t10Display construction / ownership ---'
rg -n "Gdeq031t10Display|DisplayDevice>" Drivers Tactility -g '!**/build/**' -g '!**/dist/**' | sed -n '1,200p'
echo '--- Find direct start() calls on display devices in driver code ---'
rg -n "\.start\(\)|->start\(\)" Drivers/Tactility Tactility/Source Drivers -g '!**/build/**' -g '!**/dist/**' | sed -n '1,250p'
echo '--- Find module/service startup that may call display start ---'
rg -n "findDevices<hal::display::DisplayDevice>|supportsLvgl\(\)|startLvgl\(\)|getLvglDisplay\(" Tactility/Source Drivers -g '!**/build/**' -g '!**/dist/**' | sed -n '1,250p'Repository: TactilityProject/Tactility
Length of output: 8165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Find Gdeq031t10Display construction / ownership ---'
rg -n "Gdeq031t10Display|DisplayDevice>" Drivers Tactility -g '!**/build/**' -g '!**/dist/**' | sed -n '1,200p'
echo '--- Find direct start() calls on display devices in driver code ---'
rg -n "\.start\(\)|->start\(\)" Drivers Tactility -g '!**/build/**' -g '!**/dist/**' | sed -n '1,250p'
echo '--- Find module/service startup that may call display start ---'
rg -n "findDevices<hal::display::DisplayDevice>|supportsLvgl\(\)|startLvgl\(\)|getLvglDisplay\(" Tactility/Source Drivers -g '!**/build/**' -g '!**/dist/**' | sed -n '1,250p'Repository: TactilityProject/Tactility
Length of output: 12701
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Tactility/Source/hal/Hal.cpp ---'
cat -n Tactility/Source/hal/Hal.cpp | sed -n '1,220p'
echo '--- Search for display creation/registration in drivers ---'
rg -n "createDisplay\\(|register.*Display|addDevice\\(|findDevices<display::DisplayDevice>|startLvgl\\(" Drivers Tactility/Source Tactility/Include -g '!**/build/**' -g '!**/dist/**' | sed -n '1,240p'
echo '--- Relevant display start implementations for comparison ---'
cat -n Drivers/ST7796-i8080/Source/St7796i8080Display.cpp | sed -n '200,280p'
echo '---'
cat -n Drivers/EPDiyDisplay/Source/EpdiyDisplay.cpp | sed -n '120,230p'Repository: TactilityProject/Tactility
Length of output: 15645
Drivers/GDEQ031T10/Source/Gdeq031t10Display.cpp:507-536 — Guard startLvgl() against a failed start()
startLvgl() can still run after start() returns false. In that case renderFramebuffer and refreshTask may be null, so lv_display_set_buffers(...) and the flush path can crash. Bail out when the driver isn’t initialized, or retry start() here like the other display backends.
Contributor
|
I'll wait for the other pull request to be merged first, before reviewing this one, as the diff will become smaller that way. |
…ject#552 GDEQ031T10: - waitWhileBusy() now times out (5s) instead of spinning forever on a stuck BUSY pin, which previously could hang the refresh task and make stop() un-recoverable. - writeCommand()/writeData() and all callers now check the SPI transfer result instead of advancing panel/shadow state as if it always succeeds. - start() no longer marks the display powered/initialized before the refresh task is confirmed created; on task-creation failure it now unwinds the SPI device and allocated buffers instead of leaking them. - `powered` and `currentRefreshMode` are now atomic (read on the refresh task, written from arbitrary caller threads). - Corrected a stale doc comment on setRefreshMode() (not one-shot). - README updated to describe the actual windowed partial-refresh pipeline instead of the old full-refresh-every-flush description. TdeckmaxKeyboard: - Modifier state (shift/sym/caps) moved from function-local statics to instance members. - Keyboard queue send is now non-blocking (drops+logs on a full queue) instead of blocking inside the periodic input timer callback. - initBacklight() now returns false when channel config fails, instead of reporting success. xl9555: - set_flags() rejects GPIO_FLAG_ACTIVE_LOW combined with GPIO_FLAG_DIRECTION_OUTPUT: the polarity register only affects input reads, so it silently did nothing for outputs. - set_level()/set_flags() now serialize their read-modify-write register updates with device_lock()/device_unlock(), since the shared i2c_controller_register8_{set,reset}_bits() helpers do a separate read then write. - start() uses check() instead of a soft parent-type check, and drops a log line the parent already emits. Configuration.cpp: i2c0 is unconditionally declared in this board's devicetree, so createDevices() now uses check(i2c) instead of an if, matching the "trust internal guarantees" convention. Cst66xxTouch: unbind the Home/Recents bezel keys. Both called tt::app::start(), which pushes a new app-stack instance on every press; repeated presses would leak Launcher/AppList instances indefinitely. Left unbound until the app stack has a way to resume an existing instance instead of pushing a new one. device.properties / dts: corrected the product name to "T-Deck Max" (was "T-Deck Pro Max") to match the rest of the PR, the vendor repo, and LilyGO's actual naming. Verified with a full idf.py build for lilygo-tdeck-max (IDF 5.5.2). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-max Follow-up to TactilityProject#552 (base board). Battery metrics come from the BQ27220 fuel gauge (I2C 0x55); charging status, charge control, and power-off (ship mode via SY6970 REG09 BATFET_DIS) use the SY6970 charger (0x6A — newer T-Deck Max revisions use this instead of the older BQ25896 at 0x6B, confirmed by I2C scan; the two are register-compatible for everything used here). Enables the launcher's power-off button. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Crazypedia
force-pushed
the
tdeckmax-power
branch
from
e4dc61a to
ad2e7a6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 2 of the incremental plan from #539 (base board → power → radio → GPS/haptics). Stacked on #552 — this branch is
tdeckmax-baseboard+ one commit (e4dc61a7), so the diff below includes #552's content until that merges. Only the last commit is new here; happy to rebase ontomainonce #552 lands if that's easier to review standalone.What this adds
Battery metrics and power-off for the T-Deck Max:
REG09BATFET_DIS). Newer T-Deck Max revisions use the SY6970 instead of the older BQ25896 at 0x6B — confirmed by I2C scan on my unit; the two are register-compatible for everything used here.TdeckmaxPower::supportsPowerOff()returns true, so the launcher's power-off button is enabled.Caveat carried over from the vendor's own driver notes: ship mode only fully powers off on battery with USB unplugged — with USB attached the system rail stays up. This matches the chip's documented behavior, not a bug in this driver.
Verification
SY6970 configured: 4288 mV / 1024 mA, watchdog off, bothBQ27220andT-Deck Max Powerregister as kernel devices without error, and thePowerapp manifest (which drives the launcher's power-off button) registers correctly.🤖 Generated with Claude Code
Summary by CodeRabbit