-
-
Notifications
You must be signed in to change notification settings - Fork 154
Update RX lua #527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Update RX lua #527
Conversation
WalkthroughRX UI gating in src/SCRIPTS/BF/PAGES/rx.lua was reorganized by apiVersion: Interpolation/Interval limited to 1.20–1.42; Cam Angle added for >=1.31; RC Smoothing split into 1.47+ (new block with Setpoint AF id31), 1.44–1.46 (legacy RC Smoothing fields), and older pre-1.44 paths removed/restructured. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant RX as RX Page UI
participant API as apiVersion
U->>RX: Open RX / RC Smoothing section
RX->>API: Query apiVersion
alt apiVersion >= 1.47
RX-->>U: Render new RC Smoothing block (Mode id25, Cutoffs, Setpoint id26 w/Auto, Auto Smoothness, Setpoint AF id31)
else apiVersion >= 1.44 and <= 1.46
RX-->>U: Render legacy RC Smoothing fields (Mode id25, Cutoffs, Setpoint id26 w/Auto, Feedforward id27 w/Auto, Auto Smoothness)
else
RX-->>U: Render older/legacy layout (pre-1.44 paths removed/restructured)
end
alt apiVersion >= 1.31
RX-->>U: Show Cam Angle (id23)
end
alt apiVersion >= 1.20 and <= 1.42
RX-->>U: Show Interpolation (id13) and Interval (id14)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
26189f0
to
6223f90
Compare
6223f90
to
1f77b64
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/SCRIPTS/BF/PAGES/rx.lua (1)
47-50
: LGTM: Interpolation/Interval correctly gated to 1.20–1.42.This matches the previous review note that interpolation was removed in 4.3 and avoids showing obsolete controls on newer API versions.
🧹 Nitpick comments (2)
src/SCRIPTS/BF/PAGES/rx.lua (2)
23-23
: Likely inverted ON/OFF mapping for Mode (0 should be OFF, 1 should be ON).In both branches, Mode uses table = { [0] = "ON", "OFF" }. Conventionally, 0 maps to OFF and 1 to ON. As-is, the UI labels will be misleading.
Apply this diff in both places:
- fields[#fields + 1] = { t = "Mode", x = x + indent, y = inc.y(lineSpacing), sp = x + sp, min = 0, max = 1, vals = { 25 }, table = { [0] = "ON", "OFF" } } + fields[#fields + 1] = { t = "Mode", x = x + indent, y = inc.y(lineSpacing), sp = x + sp, min = 0, max = 1, vals = { 25 }, table = { [0] = "OFF", "ON" } }Also applies to: 30-30
21-27
: Confirm: Is Feedforward setpoint intentionally removed for API ≥ 1.47?The 1.44–1.46 branch includes a Feedforward cutoff (vals {27}); the ≥1.47 branch does not. If upstream removed FF in that API, this is fine; otherwise, it’s a regression.
If FF is still applicable in ≥1.47, mirror the 1.44–1.46 entry under the “Cutoffs” label:
labels[#labels + 1] = { t = "Cutoffs", x = x + indent, y = inc.y(lineSpacing) } fields[#fields + 1] = { t = "Setpoint", x = x + indent*2, y = inc.y(lineSpacing), sp = x + sp, min = 0, max = 255, vals = { 26 }, table = { [0] = "Auto" } } + fields[#fields + 1] = { t = "Feedforward", x = x + indent*2, y = inc.y(lineSpacing), sp = x + sp, min = 0, max = 255, vals = { 27 }, table = { [0] = "Auto" } }
Would you like me to cross-check BF PR 14411 and adjust accordingly?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/SCRIPTS/BF/PAGES/rx.lua
(1 hunks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/SCRIPTS/BF/PAGES/rx.lua (1)
47-50
: Verify scope: Interpolation/Interval currently only shown for 1.40–1.42 due to nestingBecause this check is inside the
apiVersion >= 1.40
branch, versions 1.20–1.39 won’t show Interpolation/Interval despite the condition suggesting 1.20–1.42. If that’s intended, consider simplifying the condition to reduce confusion; if not, move the block outside the 1.40+ branch.Minimal simplification if keeping it within the 1.40–1.42 scope:
- if apiVersion >= 1.20 and apiVersion <= 1.42 then + if apiVersion <= 1.42 then
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/SCRIPTS/BF/PAGES/rx.lua
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: haslinghuis
PR: betaflight/betaflight-tx-lua-scripts#527
File: src/SCRIPTS/BF/PAGES/rx.lua:27-27
Timestamp: 2025-08-17T18:27:20.457Z
Learning: The Auto Smoothness setpoint field (id 31) in rx.lua uses max = 255 for firmware 4.3 and up, which corresponds to API version 1.47+.
Learnt from: haslinghuis
PR: betaflight/betaflight-tx-lua-scripts#527
File: src/SCRIPTS/BF/PAGES/rx.lua:35-35
Timestamp: 2025-08-17T18:27:06.194Z
Learning: In Betaflight firmware 4.3 and up (corresponding to apiVersion >= 1.44), the Auto Smoothness setpoint max value should be 255, not 50 or 250.
📚 Learning: 2025-08-17T18:27:20.457Z
Learnt from: haslinghuis
PR: betaflight/betaflight-tx-lua-scripts#527
File: src/SCRIPTS/BF/PAGES/rx.lua:27-27
Timestamp: 2025-08-17T18:27:20.457Z
Learning: The Auto Smoothness setpoint field (id 31) in rx.lua uses max = 255 for firmware 4.3 and up, which corresponds to API version 1.47+.
Applied to files:
src/SCRIPTS/BF/PAGES/rx.lua
📚 Learning: 2025-08-17T18:27:06.194Z
Learnt from: haslinghuis
PR: betaflight/betaflight-tx-lua-scripts#527
File: src/SCRIPTS/BF/PAGES/rx.lua:35-35
Timestamp: 2025-08-17T18:27:06.194Z
Learning: In Betaflight firmware 4.3 and up (corresponding to apiVersion >= 1.44), the Auto Smoothness setpoint max value should be 255, not 50 or 250.
Applied to files:
src/SCRIPTS/BF/PAGES/rx.lua
🔇 Additional comments (3)
src/SCRIPTS/BF/PAGES/rx.lua (3)
21-27
: RC Smoothing (API ≥1.47) block aligns with 4.3+ behavior — looks good
- Setpoint (id 26) and Auto Smoothness Setpoint (id 31) both use 0–255, which matches the clarified 4.3+ range.
- Feedforward cutoff (id 27) is removed here compared to the 1.44–1.46 branch, which aligns with the upstream direction.
35-35
: Auto Smoothness Setpoint 0–255 for API 1.44–1.46 is correctUsing max = 255 for id 31 in 4.3-era APIs is consistent with the clarified firmware behavior.
36-45
: Legacy RC Smoothing (API ≥1.40) block looks consistentEnums and ranges (Type, Channels, Input/Derivative filters) appear correct and consistent with prior layouts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/SCRIPTS/BF/PAGES/rx.lua (1)
47-50
: Interpolation/Interval UI is effectively gated to 1.40–1.42 (not 1.20–1.42)This block is nested inside the
elseif apiVersion >= 1.40
branch, so even withif apiVersion <= 1.42
it only shows for 1.40–1.42. If the intent is to show Interpolation/Interval for 1.20–1.39 as well, you’ll need a separate branch for 1.20–1.39.If 1.20–1.39 should retain Interpolation, consider appending a legacy block after the 1.40+ branch:
elseif apiVersion >= 1.20 then -- Legacy interpolation-only UI for 1.20–1.39 fields[#fields + 1] = { t = "Interpolation", x = x, y = inc.y(lineSpacing), sp = x + sp, min = 0, max = 3, vals = { 13 }, table={ [0]="Off", "Preset", "Auto", "Manual"} } fields[#fields + 1] = { t = "Interval", x = x + indent, y = inc.y(lineSpacing), sp = x + sp, min = 1, max = 50, vals = { 14 } }Please confirm whether 1.20–1.39 should be covered, or if support for those API versions is intentionally dropped here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/SCRIPTS/BF/PAGES/rx.lua
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: haslinghuis
PR: betaflight/betaflight-tx-lua-scripts#527
File: src/SCRIPTS/BF/PAGES/rx.lua:27-27
Timestamp: 2025-08-17T18:27:20.457Z
Learning: The Auto Smoothness setpoint field (id 31) in rx.lua uses max = 255 for firmware 4.3 and up, which corresponds to API version 1.47+.
Learnt from: haslinghuis
PR: betaflight/betaflight-tx-lua-scripts#527
File: src/SCRIPTS/BF/PAGES/rx.lua:35-35
Timestamp: 2025-08-17T18:27:06.194Z
Learning: In Betaflight firmware 4.3 and up (corresponding to apiVersion >= 1.44), the Auto Smoothness setpoint max value should be 255, not 50 or 250.
📚 Learning: 2025-08-17T18:27:20.457Z
Learnt from: haslinghuis
PR: betaflight/betaflight-tx-lua-scripts#527
File: src/SCRIPTS/BF/PAGES/rx.lua:27-27
Timestamp: 2025-08-17T18:27:20.457Z
Learning: The Auto Smoothness setpoint field (id 31) in rx.lua uses max = 255 for firmware 4.3 and up, which corresponds to API version 1.47+.
Applied to files:
src/SCRIPTS/BF/PAGES/rx.lua
📚 Learning: 2025-08-17T18:27:06.194Z
Learnt from: haslinghuis
PR: betaflight/betaflight-tx-lua-scripts#527
File: src/SCRIPTS/BF/PAGES/rx.lua:35-35
Timestamp: 2025-08-17T18:27:06.194Z
Learning: In Betaflight firmware 4.3 and up (corresponding to apiVersion >= 1.44), the Auto Smoothness setpoint max value should be 255, not 50 or 250.
Applied to files:
src/SCRIPTS/BF/PAGES/rx.lua
🔇 Additional comments (1)
src/SCRIPTS/BF/PAGES/rx.lua (1)
21-26
: RC Smoothing (API ≥ 1.47) block looks correct
- Good separation for 1.47+.
- Setpoint (id 26) range 0–255 with “Auto” at 0 aligns with the firmware change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/SCRIPTS/BF/PAGES/rx.lua (1)
30-37
: Optional: include “(AutoFactor)” for clarity if UI space allowsThe PR description mentions “Setpoint AF (AutoFactor)”. If layout permits, consider the full label for discoverability.
- fields[#fields + 1] = { t = "Setpoint AF", x = x + indent*2, y = inc.y(lineSpacing), sp = x + sp, min = 0, max = 250, vals = { 31 } } + fields[#fields + 1] = { t = "Setpoint AF (AutoFactor)", x = x + indent*2, y = inc.y(lineSpacing), sp = x + sp, min = 0, max = 250, vals = { 31 } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/SCRIPTS/BF/PAGES/rx.lua
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: haslinghuis
PR: betaflight/betaflight-tx-lua-scripts#527
File: src/SCRIPTS/BF/PAGES/rx.lua:27-27
Timestamp: 2025-08-17T19:28:48.890Z
Learning: The Auto Smoothness setpoint field (rc_smoothing_auto_factor_rpy, id 31) in rx.lua uses max = 250 (range 0-250) in Betaflight firmware as defined by RC_SMOOTHING_AUTO_FACTOR_MAX constant in src/main/fc/rc.h, confirmed by firmware source code verification.
📚 Learning: 2025-08-17T19:28:48.890Z
Learnt from: haslinghuis
PR: betaflight/betaflight-tx-lua-scripts#527
File: src/SCRIPTS/BF/PAGES/rx.lua:27-27
Timestamp: 2025-08-17T19:28:48.890Z
Learning: The Auto Smoothness setpoint field (rc_smoothing_auto_factor_rpy, id 31) in rx.lua uses max = 250 (range 0-250) in Betaflight firmware as defined by RC_SMOOTHING_AUTO_FACTOR_MAX constant in src/main/fc/rc.h, confirmed by firmware source code verification.
Applied to files:
src/SCRIPTS/BF/PAGES/rx.lua
🔇 Additional comments (6)
src/SCRIPTS/BF/PAGES/rx.lua (6)
21-24
: Correctly gated Interpolation/Interval to 1.20–1.42Matches the stated removal in newer firmware and avoids surfacing controls for 4.3+.
26-28
: Cam Angle addition for apiVersion ≥ 1.31 looks goodField id 23 with 0–90 range is consistent with expected constraints.
30-37
: RC Smoothing (≥1.47): Setpoint AF id 31 range 0–250 is correctThe rename to “Setpoint AF” and the 0–250 limit align with firmware constants (RC_SMOOTHING_AUTO_FACTOR_MAX = 250).
44-44
: API 1.44–1.46: Setpoint AF id 31 range 0–250 confirmedConsistent with firmware (0–250) and with the ≥1.47 branch.
45-55
: Legacy 1.40–1.43 path is coherent and preserves older semantics
- Keeps Type/Channels/Input/Derivative filters where applicable.
- Adds Auto Smoothness for ≥1.42 with the expected 0–50 range (pre-4.3 behavior).
Looks good.
30-37
: Verify ON/OFF mapping for Mode (id 25)
I’ve confirmed that in both the 1.47+ and 1.44+ branches ofsrc/SCRIPTS/BF/PAGES/rx.lua
, field 25 is rendered as{ [0] = "ON", "OFF" }
and there are no other ON/OFF definitions for id 25 elsewhere in the repo. Since many upstream protocols use 0=OFF, 1=ON, please double-check against the Betaflight/FRSKY spec to ensure these labels aren’t inverted.If 0 should indeed mean OFF and 1 = ON, update both occurrences as follows:
- fields[#fields + 1] = { t = "Mode", x = x + indent, y = inc.y(lineSpacing), - sp = x + sp, min = 0, max = 1, vals = { 25 }, table = { [0] = "ON", "OFF" } } + fields[#fields + 1] = { t = "Mode", x = x + indent, y = inc.y(lineSpacing), + sp = x + sp, min = 0, max = 1, vals = { 25 }, table = { [0] = "OFF", "ON" } }• src/SCRIPTS/BF/PAGES/rx.lua @ lines 32 and 39
Summary by CodeRabbit
• API ≥1.47: new RC Smoothing block with Mode, Cutoffs, Setpoint (with Auto and Auto Smoothness) and a new Setpoint AF.
• API 1.44–1.46: retains older RC Smoothing fields, including Feedforward.