Skip to content

fix(pa01): Optimize key matrix polling with split-cycle scanning#7405

Open
richardclli wants to merge 5 commits into
mainfrom
richardclli/fix-pa01-keyboard-scanning
Open

fix(pa01): Optimize key matrix polling with split-cycle scanning#7405
richardclli wants to merge 5 commits into
mainfrom
richardclli/fix-pa01-keyboard-scanning

Conversation

@richardclli
Copy link
Copy Markdown
Member

@richardclli richardclli commented May 28, 2026

Developed by opencode + deepseek-v4

Split the 4-column matrix scan across 4 poll cycles (1 I2C read + 1 write per cycle instead of 4 reads + 4 writes). The output for each column is set in one cycle and read in the next (10ms later), eliminating the 10us settling delay.

Interrupt-driven caching: when no pin-change interrupt occurs between polls, the I2C read is skipped and per-column cached results are reused. A forced full rescan every 250ms catches key releases that may not trigger an interrupt.

Replaced #7394 and #7315

v2.11 version is at #7409

Summary by CodeRabbit

  • Bug Fixes
    • Improved key scanning to an incremental, cached column-based poll for faster, non-blocking input updates and periodic background scans when idle.
    • Added consecutive-I2C-error tracking and automatic recovery (bus reinitialization) with writes routed through the same tracking to improve communication robustness.

Review Change Stack

…interrupt-driven caching

Split the 4-column matrix scan across 4 poll cycles (1 I2C read + 1 write per cycle
instead of 4 reads + 4 writes). The output for each column is set in one cycle and
read in the next (10ms later), eliminating the 10us settling delay.

Interrupt-driven caching: when no pin-change interrupt occurs between polls, the
I2C read is skipped and per-column cached results are reused. A forced full rescan
every 250ms catches key releases that may not trigger an interrupt.
@richardclli richardclli added this to the 2.11.7 milestone May 28, 2026
@richardclli richardclli self-assigned this May 28, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Refactors key matrix polling from a blocking full-matrix scan to an incremental, cached per-column scanner with idle-driven scheduling and ent_mask handling. Adds consecutive I2C error tracking that deinitializes/reinitializes the I2C bus after a configurable failure threshold; AW9523B device reinitialization/output-state restoration is not included here.

Changes

Key Matrix Incremental Scan

Layer / File(s) Summary
Scan state infrastructure and cached reading
radio/src/targets/pa01/key_driver.cpp
Introduces scan configuration and persistent per-column cache/state plus read_col_to_keys() to decode a column; pollKeys() now computes ent_mask each call and may return cached results when suspendI2CTasks is set.
Progressive scan scheduling and result computation
radio/src/targets/pa01/key_driver.cpp
Replaces blocking full-matrix read with progressive per-column updates gated by syncelem.ui8ReadInProgress: schedules reads with bsp_get_shouldReadKeys()/IDLE_FORCE_SCAN, updates one cached column per call, drives next column output, recomputes full result from col_cache + ent_mask, stores it in syncelem, and refreshes fct_state[].

I2C Error Recovery for AW9523B

Layer / File(s) Summary
Consecutive I2C error tracking and recovery mechanism
radio/src/targets/pa01/bsp_io.cpp
Adds delays_driver.h/debug.h; implements a consecutive-failure counter and _track_i2c_error() that triggers _recover_i2c() after a threshold. _recover_i2c() deinitializes and reinitializes the I2C bus (with delay and TRACE); AW9523B device reinitialization/output-state restoration is not performed in these changes. bsp_input_read() updates inputState only on successful reads, and output writes route results through the same tracker.

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
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 (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: optimizing key matrix polling through split-cycle scanning across multiple poll cycles.
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.
Description check ✅ Passed The pull request description provides a clear summary of changes, technical rationale, and references to related PRs.

✏️ 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-pa01-keyboard-scanning

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.

@richardclli richardclli changed the title fix(pa01): optimize key matrix polling with split-cycle scanning fix(pa01): Optimize key matrix polling with split-cycle scanning May 28, 2026
@richardclli richardclli force-pushed the richardclli/fix-pa01-keyboard-scanning branch from 7ad0780 to 117c743 Compare May 28, 2026 08:19
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/pa01/bsp_io.cpp`:
- Around line 81-84: The three post-init calls aw9523b_write(&i2c_exp,
BSP_OUT_MASK, BSP_KEY_OUT1 | BSP_KEY_OUT2 | BSP_KEY_OUT3 | BSP_KEY_OUT4),
aw9523b_set_direction(&i2c_exp, 0xFFFF, BSP_IN_MASK) and
bsp_output_clear(BSP_U6_SELECT) currently ignore return values; update this
block to check each return like the rest of bsp_io_init() (use BSP_CHECK or
equivalent error handling), and bail/log on failure before logging "I2C recovery
complete" so the device cannot reach an inconsistent state.
🪄 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: 123f361e-6ab9-4892-8b92-9dc800b0bb60

📥 Commits

Reviewing files that changed from the base of the PR and between a9bd6d9 and 7ad0780.

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

Comment thread radio/src/targets/pa01/bsp_io.cpp Outdated
… to bus-only reset

Add _track_i2c_error() helper used by both read and write paths.
_Recover_i2c now only resets the I2C bus (deinit/init) — the
AW9523B retains its register state, so chip re-config is unnecessary.
The bootloader calls pollKeys() only once (bl_keys.cpp:9) to detect held
trims for entry condition. Split-scan approach needs 4 calls to populate
all cached columns, so bootloader never sees the trims.

Add #if defined(BOOT) guard: bootloader path does a synchronous 4-column
scan in one call (same as original code). Firmware path keeps the split-scan
optimization.
@richardclli richardclli modified the milestones: 2.11.7, 2.12.2 May 29, 2026
@pfeerick pfeerick added backport/2.11 To be backported to a 2.11 release also. backport/2.12 To be backported to a 2.12 release also. and removed backport/2.11 To be backported to a 2.11 release also. labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants