add x sampling to throttle curve preview to match firmware #4439
Conversation
✅ Deploy Preview for origin-betaflight-app ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Yes, i saw this issue before. |
|
Checked the 0.4 0.3 0.7 similar settings point. |
|
About my other notice. 0.5 0.5 0 curves settings. CfgIn BFin BFout The throttle curve works good. |
WalkthroughThe changes update the throttle curve rendering logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PID_Tuning_Module
participant Canvas
User->>PID_Tuning_Module: Adjust throttle input
PID_Tuning_Module->>PID_Tuning_Module: Validate throttle parameters
PID_Tuning_Module->>PID_Tuning_Module: Compute Bézier parameter t via inversion (getTfromXBezier/getTfromYBezier)
PID_Tuning_Module->>PID_Tuning_Module: Determine throttle curve points and limits
PID_Tuning_Module->>Canvas: Draw throttle curve with clipping or scaling
PID_Tuning_Module->>Canvas: Draw throttle position indicator and label
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/js/tabs/pid_tuning.js(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/js/tabs/pid_tuning.js (1)
src/js/fc.js (1)
FC(130-992)
🔇 Additional comments (2)
src/js/tabs/pid_tuning.js (2)
1650-1654: mid-point Y calculation now properly scales with throttle limitThe new formula
const midY = canvasHeight * (1 - throttleScale * hover);removes the earlier
(canvasHeight - throttleScale)confusion and keeps the curve correctly anchored when a throttle-limit scale is active. Nice catch.
1722-1725: 👍 Expo throttle output now safely clampedWrapping the computation with
Math.max(0, Math.min(100, …))prevents accidental negatives/overshoot in the label. Looks good.
… get the Throttle Limit type Clip to draw correctly. This works perfectly.
|
|
Preview URL: https://74ed8989.betaflight-configurator.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/js/tabs/pid_tuning.js (1)
1920-1921: Consider handling error condition more gracefullyThere's a console.error that would only be reached if thrpos calculation failed, but there's no fallback behavior specified. While this error condition should be extremely rare with the improved calculations, consider adding a minimal fallback to maintain UI consistency even in case of error.
} else { console.error("thrpos calculation failed"); + // Provide a fallback position using mid-point or previous position if available + thrpos = { x: canvasWidth / 2, y: canvasHeight / 2 }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/js/tabs/pid_tuning.js(2 hunks)
🔇 Additional comments (7)
src/js/tabs/pid_tuning.js (7)
1597-1658: Excellent implementation of Bézier curve parameter inversion for Y coordinatesThe
getTfromYBezierfunction is a solid implementation that solves the quadratic equation to find the parameter 't' for a given y-coordinate on a quadratic Bézier curve. It handles all edge cases appropriately:
- Near-linear curves when the quadratic coefficient is close to zero
- Cases where the discriminant is negative (point outside curve)
- Cases where multiple solutions exist within the valid range
This implementation is crucial for correctly determining the curve intersection points when dealing with throttle limits.
1660-1695: Well-implemented Bézier parameter inversion for X coordinates with good validationThe
getTfromXBezierfunction correctly solves for the 't' parameter given an x-coordinate. The implementation:
- Handles linear cases when coefficient 'a' is near zero
- Checks for NaN results that might occur with floating-point errors
- Properly handles the case when both solutions are valid
- Appropriately defaults to 0 or 1 when no valid solution is found
This function is critical for the accurate mapping of throttle stick inputs to curve positions, fixing the core issue described in the PR objectives.
1708-1734: Improved input validation with detailed error checksThe validation logic has been enhanced to:
- Check all input parameters (mid, expo, hover, etc.) against their min/max values
- Explicitly check for NaN values
- Exit early if any validation fails
This prevents potential rendering issues or unexpected behavior caused by invalid inputs.
1741-1750: Clear separation of original curve parameters calculationThis section improves code organization by separating the calculation of the original curve parameters from the drawing logic. The parameters are clearly named and commented, making the code much more readable and maintainable.
1756-1757: Added bounds checking for throttle percentageThe code now clamps the throttle percentage to the range [0,1], preventing issues when RC.channels[3] reports values outside the normal range (e.g., during calibration or failsafe scenarios). This ensures that
thrXstays within valid bounds for Bézier inversion.
1760-1885: Completely redesigned throttle curve rendering with distinct limit type handlingThe implementation has been fundamentally improved to:
- Handle each throttle limit type (CLIP, SCALE, OFF) with dedicated, specialized code
- Properly calculate curve intersections for the CLIP mode using the new Bézier parameter inversion functions
- Use canvas clipping regions for cleaner visual rendering of clipped curves
- Correctly position the throttle indicator on the curve based on X position
This is the core fix for the issue mentioned in the PR objectives - now the throttle curve preview correctly matches firmware behavior by sampling at X positions instead of regular t intervals.
1887-1919: Enhanced throttle position indicator with clearer visual feedbackThe throttle position indicator now:
- Uses the exact X position on the curve via Bézier parameter inversion
- Displays both input and output throttle percentages in a clear text label
- Has proper bounds checking to prevent rendering outside the canvas
- Uses consistent styling with the curve
These improvements make the throttle curve preview much more informative and accurate.
when throttle curve expo was set to 0, the preview showed a straight line, but the numeric preview showed mismatched values leading users to the conclusion there was a default hidden expo applied.
the issue was that the curve was sampled at regular intervals of t (length fraction along the curve). since at expo 0 there are two control points on top of each other, regular t sampling leads to bunched up spacing in the center.
the fix is sampling the curve at x positions (throttle stick values), just like the firmware builds the lookup table.
in both of these videos, the throttle stick is moved at a constant speed.
before fix shows the dot slowing down in the center because of the bunched up points.
after fix shows the dot moving at constant speed and numeric preview matches input to output 1:1 again.
before fix:
Recording.2025-04-25.152644.mp4
after fix:
Recording.2025-04-25.153126.mp4
Summary by CodeRabbit