Skip to content

fix(color): trims may move by an extra step when trim key released#7473

Open
philmoz wants to merge 1 commit into
mainfrom
philmoz/fix-trim
Open

fix(color): trims may move by an extra step when trim key released#7473
philmoz wants to merge 1 commit into
mainfrom
philmoz/fix-trim

Conversation

@philmoz

@philmoz philmoz commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

When a trim key is released after being held until it starts repeating, the trim may jump an extra step.
Introduced for color radios when the long key press logic was refactored.

Fixes #7329

Applies to 2.11, 2.12 and 3.0.

Summary by CodeRabbit

  • Bug Fixes
    • Improved trim event handling to process only valid key press and repeat events, enhancing accuracy and reliability of trim control during operation.

@philmoz philmoz added this to the 2.11.7 milestone Jun 20, 2026
@philmoz philmoz added color Related generally to color LCD radios UX-UI Related to user experience (UX) or user interface (UI) behaviour bug/regression ↩️ A new version of EdgeTX broke something labels Jun 20, 2026
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e9c60ac3-17ac-49ae-b632-08fc3e1e9ab3

📥 Commits

Reviewing files that changed from the base of the PR and between d67604f and 7905d0a.

📒 Files selected for processing (2)
  • radio/src/edgetx.cpp
  • radio/src/keys.h
💤 Files with no reviewable changes (1)
  • radio/src/keys.h

📝 Walkthrough

Walkthrough

IS_KEY_BREAK is removed from keys.h along with a dead IS_TRIM_EVENT comment. checkTrims() is updated to process trim events only on first-press or repeat (IS_KEY_FIRST || IS_KEY_REPT), replacing the prior !IS_KEY_BREAK guard.

Changes

Trim key-break fix

Layer / File(s) Summary
Remove IS_KEY_BREAK and dead comment
radio/src/keys.h
Deletes the IS_KEY_BREAK inline predicate and the commented-out IS_TRIM_EVENT block; the key-break test is no longer available as a standalone helper.
Tighten trim event condition
radio/src/edgetx.cpp
checkTrims() now enters trim-update logic only on IS_KEY_FIRST or IS_KEY_REPT, preventing key-break events from advancing the trim value on release.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Suggested labels

backport/2.11, backport/2.12

🚥 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 reflects the main change: preventing an extra trim step on color radios when a key is released.
Description check ✅ Passed The description includes the issue reference (#7329) and explains the bug context, but lacks detail on the specific code changes made to fix it.
Linked Issues check ✅ Passed The changes directly address issue #7329 by filtering trim events to only process first-press and repeat events, preventing errant key-release events from causing extra trim steps.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the trim event handling issue: modifying checkTrims() condition and removing IS_KEY_BREAK from keys.h to eliminate the extra event.

✏️ 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 philmoz/fix-trim

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug/regression ↩️ A new version of EdgeTX broke something color Related generally to color LCD radios UX-UI Related to user experience (UX) or user interface (UI) behaviour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Radiomaster tx16s mk3 Trim Centering issue

1 participant