T5S3 ePaper build fix #10791
Conversation
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (25)
Build artifacts expire on 2026-07-29. Updated for |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9368009727
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcaea07a3f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
HarukiToreda
added
hardware-support
Firmware Size Report22 targets | vs
Show 17 more target(s)
Updated for a62e18b |
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes build/config issues for the T5S3 ePaper inkhud environment and adjusts runtime behavior around touch handling and USB CDC logging.
Changes:
- Apply the correct 16MB partition layout and re-enable environment validation for
t5s3_epaper_inkhud. - Add a target-specific
custom_component_removelist to avoid removingnetwork_provisioningfor this env. - Reduce redundant touch-controller work and avoid USB CDC logging stalls during reboot/re-enumeration.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| variants/esp32s3/t5s3_epaper/platformio.ini | Fixes partition config key, re-enables board_check, and scopes component removal to the inkhud env. |
| src/platform/extra_variants/t5s3_epaper/variant.cpp | Adds an early-return to avoid redundant work and changes GT911 sleep/wake behavior when timeout forcing toggles. |
| src/SerialConsole.cpp | Attempts to prevent native USB CDC logging from blocking boot by setting TX timeout for a specific target. |
Per Vid, this is not needed once his PR is merged
📝 WalkthroughWalkthroughChanges adjust SerialConsole to disable TX timeout for native USB serial on T5 S3 ePaper Pro, modify GT911 touch controller forced-timeout logic to skip sleep and add an early-return guard, and update platformio.ini partition key naming and board_check/board_level settings for the inkhud environment. ChangesT5 S3 ePaper Fixes
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/extra_variants/t5s3_epaper/variant.cpp`:
- Around line 429-435: The timeout handling in the touch wake logic leaves
touchNeedsWake stale when forced transitions back to false during light sleep,
so the controller never gets a deferred wake. Update the forced branch in
variant.cpp near the touch wake state logic so it mirrors setTouchInputEnabled:
assign touchNeedsWake based on touchControllerReady first, then only clear it
when touchLightSleepActive is false or touch input is not enabled. Use the
existing touchNeedsWake, touchControllerReady, touchLightSleepActive, and
touchInputEnabled checks to ensure readTouch() can trigger the wake after
timeout clears.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e355fc7e-a8ec-4a8c-8aa2-12462f56a884
📒 Files selected for processing (3)
src/SerialConsole.cppsrc/platform/extra_variants/t5s3_epaper/variant.cppvariants/esp32s3/t5s3_epaper/platformio.ini
| if (forced) { | ||
| // While timeout-forced, keep controller asleep to avoid stale IRQ chatter. | ||
| // Timeout only gates touch input in software. Avoid GT911 I2C here because | ||
| // PowerFSM same-state transitions can fire from phone contact and screen timeout. | ||
| touchNeedsWake = false; | ||
| if (touchControllerReady && !touchLightSleepActive) { | ||
| touch.sleep(); | ||
| } | ||
| } else if (touchInputEnabled && touchControllerReady && !touchLightSleepActive) { | ||
| // Defer wake until readTouch() so I2C settles post-state transition. | ||
| touchNeedsWake = true; | ||
| touchNeedsWake = false; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Touch never wakes when un-forcing timeout during light sleep.
When forced becomes false (timeout ends), touchNeedsWake is only cleared when the controller is already awake (!touchLightSleepActive). If touchLightSleepActive is true at that moment, touchNeedsWake is left untouched — and since the sibling forced == true branch always sets it to false, it will almost certainly still be false. readTouch() only wakes the controller when touchNeedsWake && touchControllerReady (line 595), so the deferred wake is never triggered and touch stays asleep/unresponsive after the timeout clears.
Compare with setTouchInputEnabled (lines 476-478), which unconditionally sets touchNeedsWake = touchControllerReady before conditionally clearing it — that pattern correctly defers the wake when light-sleeping.
🐛 Proposed fix mirroring setTouchInputEnabled's pattern
if (forced) {
// Timeout only gates touch input in software. Avoid GT911 I2C here because
// PowerFSM same-state transitions can fire from phone contact and screen timeout.
touchNeedsWake = false;
- } else if (touchInputEnabled && touchControllerReady && !touchLightSleepActive) {
- touchNeedsWake = false;
+ } else if (touchInputEnabled && touchControllerReady) {
+ touchNeedsWake = touchLightSleepActive;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (forced) { | |
| // While timeout-forced, keep controller asleep to avoid stale IRQ chatter. | |
| // Timeout only gates touch input in software. Avoid GT911 I2C here because | |
| // PowerFSM same-state transitions can fire from phone contact and screen timeout. | |
| touchNeedsWake = false; | |
| if (touchControllerReady && !touchLightSleepActive) { | |
| touch.sleep(); | |
| } | |
| } else if (touchInputEnabled && touchControllerReady && !touchLightSleepActive) { | |
| // Defer wake until readTouch() so I2C settles post-state transition. | |
| touchNeedsWake = true; | |
| touchNeedsWake = false; | |
| } | |
| if (forced) { | |
| // Timeout only gates touch input in software. Avoid GT911 I2C here because | |
| // PowerFSM same-state transitions can fire from phone contact and screen timeout. | |
| touchNeedsWake = false; | |
| } else if (touchInputEnabled && touchControllerReady) { | |
| touchNeedsWake = touchLightSleepActive; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/platform/extra_variants/t5s3_epaper/variant.cpp` around lines 429 - 435,
The timeout handling in the touch wake logic leaves touchNeedsWake stale when
forced transitions back to false during light sleep, so the controller never
gets a deferred wake. Update the forced branch in variant.cpp near the touch
wake state logic so it mirrors setTouchInputEnabled: assign touchNeedsWake based
on touchControllerReady first, then only clear it when touchLightSleepActive is
false or touch input is not enabled. Use the existing touchNeedsWake,
touchControllerReady, touchLightSleepActive, and touchInputEnabled checks to
ensure readTouch() can trigger the wake after timeout clears.
Summary
board_build.partition->board_build.partitionsso the 16 MB partition layout is actually applied for the T5S3 e-paper target.board_check = truefort5s3_epaper_inkhudso the env is validated normally.t5TouchSetForcedByTimeout()is called without a state change duringPowerFSMre-entry.Validation
pio run -e t5s3_epaper_inkhud— successfulSummary by CodeRabbit