Skip to content

fix(pl18u): Reinit I2C when touch screen not working#7402

Open
richardclli wants to merge 5 commits into
mainfrom
richardclli/fix-pl18-touch-dead
Open

fix(pl18u): Reinit I2C when touch screen not working#7402
richardclli wants to merge 5 commits into
mainfrom
richardclli/fix-pl18-touch-dead

Conversation

@richardclli
Copy link
Copy Markdown
Member

@richardclli richardclli commented May 28, 2026

PL18U can have touch screen unresponsive, and need I2C reinit to make it work again.

This PR will fix all similar symptom in all Flysky radios supported in pl18 target.

A bug is discovered in stm32_i2c_driver that will make reinit i2c bus not working in other color touch screen (opencode + deepseek did this, yeah)

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened I2C peripheral driver cleanup with enhanced memory clearing during deinitialization to prevent resource leaks and stale state.
    • Improved touch panel driver with an active I2C bus reinitialization sequence, including timing control and diagnostic logging for more reliable recovery.
  • Tests

    • Added a disabled debugging block for future I2C reinitialization testing and diagnostics.

Review Change Stack

@richardclli richardclli added this to the 2.11.7 milestone May 28, 2026
@richardclli richardclli self-assigned this May 28, 2026
@richardclli richardclli marked this pull request as draft May 28, 2026 02:30
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

📝 Walkthrough

Walkthrough

Adds and zeroes the I2C handle Init after deinit; enables touch driver _i2c_reInit() to deinit, delay 1 ms, and reinit with trace logs; adds a compile-disabled polling counter in touchPanelRead() for optional reinit triggering.

Changes

STM32 I2C Driver Cleanup and Touch Reinitialization

Layer / File(s) Summary
STM32 I2C driver handle cleanup
radio/src/targets/common/arm/stm32/stm32_i2c_driver.cpp
Adds <cstring> header and introduces memset to zero the I2C handle's Init structure after successful HAL_I2C_DeInit() call.
Touch driver I2C reinitialization
radio/src/targets/pl18/touch_driver.cpp
_i2c_reInit() actively deinitializes the I2C bus, logs start/end, delays 1 ms, reinitializes it, and returns on failure. touchPanelRead() gains a compile-disabled (#if 0) counter/polling block that can trigger _i2c_reInit() for testing.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description provides context about the touch screen issue and the bug fix, but does not follow the template structure with required sections like 'Fixes #' issue reference and 'Summary of changes'. Add the required template sections: include a 'Fixes #' reference to link the related issue, and structure the description with a clear 'Summary of changes' section following the repository template.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title 'fix(pl18u): Reinit I2C when touch screen not working' directly describes the main change: implementing I2C reinitialization to recover unresponsive touch screens on PL18U devices.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch richardclli/fix-pl18-touch-dead

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Fix stm32_i2c_deinit() to reset HAL Init struct so deinit→init cycle
actually reinitializes the I2C peripheral. On STM32F4, HAL_I2C_Init
toggles SWRST which hardware-resets the stuck I2C state machine.

Add periodic _i2c_reInit() trigger every 50 polls in touchPanelRead()
with TRACE output for debugging.
@richardclli richardclli force-pushed the richardclli/fix-pl18-touch-dead branch from dee91fc to d66b80d Compare May 28, 2026 02:41
@richardclli richardclli marked this pull request as ready for review May 28, 2026 02:41
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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 `@radio/src/targets/pl18/touch_driver.cpp`:
- Around line 173-180: The TRACE call always reports "I2C reinit: bus reset
complete" even if deinit or re-init failed; update _i2c_reInit() to check the
return value of i2c_deinit(TOUCH_I2C_BUS) and the result of _i2c_init() and only
emit the success TRACE when both succeed, otherwise log an error/TRACE with the
failure reason and propagate/return an error code; specifically modify the block
around i2c_deinit, delay_ms(1), and _i2c_init() (references: i2c_deinit,
_i2c_init, and the surrounding TRACE calls) to conditionally log success or
failure and avoid masking recovery failures.
🪄 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: 4d0c0c70-f4c9-4e23-85a2-a837cb459f13

📥 Commits

Reviewing files that changed from the base of the PR and between 00d7544 and d66b80d.

📒 Files selected for processing (2)
  • radio/src/targets/common/arm/stm32/stm32_i2c_driver.cpp
  • radio/src/targets/pl18/touch_driver.cpp

Comment thread radio/src/targets/pl18/touch_driver.cpp
@richardclli
Copy link
Copy Markdown
Member Author

@pfeerick Need to wait for Flysky confirm if this can fix the problem, I can hardly reproduce touch screen dead.

@richardclli richardclli changed the title fix(PL18U): Reinit I2C when touch screen not working fix(pl18u): Reinit I2C when touch screen not working May 28, 2026
@pfeerick pfeerick added the backport/2.11 To be backported to a 2.11 release also. label Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/2.11 To be backported to a 2.11 release also.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants