Skip to content

add x sampling to throttle curve preview to match firmware #4439

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

Conversation

marc-frank
Copy link
Contributor

@marc-frank marc-frank commented Apr 25, 2025

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

  • Improvements
    • Enhanced throttle curve rendering for more accurate mapping of throttle input to the curve.
    • Improved calculation of throttle position for smoother and more precise visual feedback.
    • Throttle percentage values are now constrained to valid ranges for increased reliability.

Copy link

netlify bot commented Apr 25, 2025

Deploy Preview for origin-betaflight-app ready!

Name Link
🔨 Latest commit 55a646f
🔍 Latest deploy log https://app.netlify.com/sites/origin-betaflight-app/deploys/680b90aaeddfce0008671a1c
😎 Deploy Preview https://deploy-preview-4439.app.betaflight.com
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@nerdCopter nerdCopter left a comment

Choose a reason for hiding this comment

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

so much math 😸

@demvlad
Copy link
Contributor

demvlad commented Apr 26, 2025

Yes, i saw this issue before.
The full test both throttle curves PRs are on BF page
My linear settings looks good now.
But i've found Configurator issue at next settings:
0.4 0.3 0.7 - Configurators chart shows 465% output for 100% throttle input
This configurators issue start from 0.63 expo for this point:
0.4 0.3 0.63 - Cfg chart out 160% for 100 input
The BF firmwware has correct 2000 output for these settings. This is configurators issue probably.

@haslinghuis haslinghuis marked this pull request as draft April 26, 2025 18:21
@demvlad
Copy link
Contributor

demvlad commented Apr 27, 2025

Checked the 0.4 0.3 0.7 similar settings point.
The all work good.

@demvlad
Copy link
Contributor

demvlad commented Apr 27, 2025

About my other notice.
This is not your curves issue probably, and this is not big problem.

0.5 0.5 0 curves settings.
CfgIn - input at configurators chart
BFIn - raw Throttle RC data in BF
BFOut - Throttle RC after curves

CfgIn BFin BFout
4% 0 1000
5% 4 1003
10% 53 1052
15% 104 1103
20% 153 1152
40% 364 1363
80% 788 1788

The throttle curve works good.
But why configurator shows 4% when real RC in BF firmware data is zero? Or it shows 10% for 5% RC data value?
This is the other history probably....

Copy link
Contributor

coderabbitai bot commented May 3, 2025

Walkthrough

The changes update the throttle curve rendering logic in the redrawThrottleCurve function within the PID tuning JavaScript module. The original direct computation of throttle position on a quadratic Bézier curve was replaced by a more robust approach that inverts the Bézier equations using new helper functions getTfromXBezier and getTfromYBezier. The throttle curve drawing now varies by throttle limit type (OFF, SCALE, CLIP), with clipping and scaling handled explicitly. Input validation was enhanced, and the throttle position indicator is drawn precisely on the curve with corresponding labels. All modifications are internal and do not alter public interfaces.

Changes

File(s) Change Summary
src/js/tabs/pid_tuning.js Rewrote throttle curve rendering in redrawThrottleCurve; added Bézier inversion helpers getTfromXBezier and getTfromYBezier; handled throttle limit types with clipping and scaling; improved input validation; refined throttle position indicator drawing and labeling.

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
Loading

Poem

🐇 Upon the curve where numbers dance,
A rabbit hopped to take a chance.
It found the t where Béziers hide,
And clipped the throttle far and wide.
With math and art in perfect blend,
The throttle’s path now finds its end.
Hop on, code, and never bend! 🎨✨

Tip

⚡️ Faster reviews with caching
  • CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.

Enjoy the performance boost—your workflow just got faster.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d45e568 and 6463c9b.

📒 Files selected for processing (1)
  • src/js/tabs/pid_tuning.js (2 hunks)
🔇 Additional comments (8)
src/js/tabs/pid_tuning.js (8)

1597-1642: Well-implemented quadratic Bezier curve parameter inversion function

The new getTfromYBezier function properly handles the mathematical challenge of finding the 't' parameter for a given y-coordinate on a quadratic Bezier curve. The implementation correctly:

  • Converts the equation to standard quadratic form
  • Handles the near-linear case when coefficient A is very small
  • Manages cases with no real solutions
  • Selects the appropriate solution when multiple valid values exist
  • Ensures values remain within the valid [0,1] range

1644-1675: Robust x-to-t parameter inversion implementation

The getTfromXBezier function complements the y-inversion function by providing the inverse mapping from x coordinates to bezier t parameter. The function correctly:

  • Handles linear cases where coefficient a is near zero
  • Checks for solutions outside the valid parameter range
  • Provides appropriate fallbacks when no valid solutions exist
  • Prioritizes the first root when multiple valid solutions exist

1688-1713: Improved input validation for throttle curve parameters

The enhanced validation checks properly verify all parameters before proceeding with drawing, guarding against:

  • NaN values
  • Values outside their valid ranges
  • Handling the case when values aren't properly initialized

This makes the function more robust against potential edge cases and invalid inputs.


1720-1731: Well-structured base curve parameter calculation

This section clearly separates the base curve parameters from scaling and clipping operations, which improves code readability and maintenance. The parameter calculations properly translate the throttle mid, expo, and hover settings into appropriate Bezier control points.


1742-1812: Properly implemented throttle CLIP limit type

The clipping implementation correctly:

  1. Calculates the clip Y position based on the throttle limit
  2. Uses the correct Bezier inversion to find the intersection point
  3. Leverages canvas clipping regions to draw only the valid portion of the curve
  4. Adds a horizontal line at the limit position
  5. Handles the throttle position indicator at the clipped level

This approach gives a clear visual representation of how the throttle is limited in the CLIP mode.


1812-1861: Well-implemented SCALE and OFF throttle limit types

The code for the SCALE and OFF limit types correctly adjusts the curve parameters based on the scale factor:

  1. Sets scaleFactor to throttleLimitPercent for SCALE mode or 1.0 for OFF mode
  2. Properly scales the curve points vertically according to the scale factor
  3. Correctly calculates the throttle position on the scaled curve using the Bezier inversion functions

This provides an accurate visual representation of how the throttle curve behaves with different limit types.


1863-1896: Improved throttle position indicator and labeling

The throttle position indicator implementation:

  1. Properly clamps values to canvas bounds for robustness
  2. Draws a clearly visible circle at the exact throttle position
  3. Displays a text label showing both input and output throttle percentages
  4. Uses consistent styling that matches the rest of the curve

This provides a better user experience with clear visual feedback about the current throttle position.


1736-1741: Clean separation of curve drawing logic by limit type

The code now properly branches based on throttle limit type, which makes the implementation clearer and easier to maintain. Each limit type (CLIP, SCALE, OFF) has its own well-defined behavior that accurately represents how the throttle curve works in the firmware.

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@haslinghuis haslinghuis marked this pull request as ready for review May 3, 2025 18:53
Copy link
Contributor

@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: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a062a9c and ac9e3e4.

📒 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 limit

The 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 clamped

Wrapping 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.
Copy link
Contributor

@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: 0

🧹 Nitpick comments (1)
src/js/tabs/pid_tuning.js (1)

1920-1921: Consider handling error condition more gracefully

There'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

📥 Commits

Reviewing files that changed from the base of the PR and between ac9e3e4 and 285799f.

📒 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 coordinates

The getTfromYBezier function 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 validation

The getTfromXBezier function 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 checks

The 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 calculation

This 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 percentage

The 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 thrX stays within valid bounds for Bézier inversion.


1760-1885: Completely redesigned throttle curve rendering with distinct limit type handling

The implementation has been fundamentally improved to:

  1. Handle each throttle limit type (CLIP, SCALE, OFF) with dedicated, specialized code
  2. Properly calculate curve intersections for the CLIP mode using the new Bézier parameter inversion functions
  3. Use canvas clipping regions for cleaner visual rendering of clipped curves
  4. 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 feedback

The throttle position indicator now:

  1. Uses the exact X position on the curve via Bézier parameter inversion
  2. Displays both input and output throttle percentages in a clear text label
  3. Has proper bounds checking to prevent rendering outside the canvas
  4. Uses consistent styling with the curve

These improvements make the throttle curve preview much more informative and accurate.

@nerdCopter nerdCopter self-requested a review May 5, 2025 13:42
Copy link

sonarqubecloud bot commented May 9, 2025

Copy link
Contributor

github-actions bot commented May 9, 2025

Preview URL: https://3b8a9a71.betaflight-configurator.pages.dev

@haslinghuis haslinghuis merged commit 6696b93 into betaflight:master May 9, 2025
7 checks passed
@demvlad
Copy link
Contributor

demvlad commented May 10, 2025

Yes, i commented about small configurators preview issues, while BF FW works normal.
.....
But why configurator shows 4% when real RC in BF firmware data is zero? Or it shows 10% for 5% RC data value?
This is the other history probably....

For interesting, I've found reason of small difference between internal BF firmware state and configurators preview what i've noticed early.
There are "Stick Low threshold" and "Stick High threshold" settings influence. Its change small and high source raw RC throttle value in BF firmware, but its do not work in configurators preview.
It has not matter for throttles curves work

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

Successfully merging this pull request may close these issues.

4 participants