Throttle curve now respects throttle scale and clip #3879
Conversation
✅ Deploy Preview for origin-betaflight-configurator ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@haslinghuis I've fixed those comments. |
This comment has been minimized.
This comment has been minimized.
|
@haslinghuis This is now ready to merge I think! I've added an extra line to make the little blob you get when the receiver is connected move along the new line correctly. I've also tested the build. |
This comment has been minimized.
This comment has been minimized.
Suggestions accepted. Squash and merge! |
This comment has been minimized.
This comment has been minimized.
ChrisRosser
force-pushed
the
add-inverse-bezier-curve-function
branch
from
2df2f5f
to
db4127a
Compare
This comment has been minimized.
This comment has been minimized.
Update pid_tuning.js Update pid_tuning.js
ChrisRosser
force-pushed
the
add-inverse-bezier-curve-function
branch
from
db4127a
to
afafa3e
Compare
Co-authored-by: Mark Haslinghuis <[email protected]>
Co-authored-by: Mark Haslinghuis <[email protected]>
Co-authored-by: Mark Haslinghuis <[email protected]>
This comment has been minimized.
This comment has been minimized.
| @@ -1738,29 +1759,52 @@ pid_tuning.initialize = function (callback) { | |||
| throttleCurve.width = throttleCurve.height * | |||
| (throttleCurve.clientWidth / throttleCurve.clientHeight); | |||
|
|
|||
| const throttleScale = throttleLimitType === THROTTLE_LIMIT_TYPES.SCALE ? throttleLimitPercent : 1; | |||
| const canvasHeight = throttleCurve.height; | |||
| const canvasWidth = throttleCurve.width; | |||
|
|
|||
| // math magic by englishman | |||
There was a problem hiding this comment.
do we want to keep this comment or append w/ Rosser?
There was a problem hiding this comment.
I'm an englishman so I say leave it. :P
No idea who originally wrote it!
src/js/tabs/pid_tuning.js
Outdated
|
|
||
| /* --- */ |
There was a problem hiding this comment.
probably remove empty comment unless something to note here.
There was a problem hiding this comment.
I think it's just to separate function definitions from the code body?
There was a problem hiding this comment.
- approving. good stuff!
- @haslinghuis , please see my prior, yet insignificant, code-reviews about code-comments before merging.
|
|
if this isn't a bug fix it shouldn't be added to 10.10 |
|
Do you want to test this code? Here you have an automated build: |
I have opinion of the opposite.
|
|
@nerdCopter it was mislabeled. If it’s a bug fix then there is no issue |
* Update pid_tuning.js Update pid_tuning.js Update pid_tuning.js * Update src/js/tabs/pid_tuning.js Co-authored-by: Mark Haslinghuis <[email protected]> * Update src/js/tabs/pid_tuning.js Co-authored-by: Mark Haslinghuis <[email protected]> * Update src/js/tabs/pid_tuning.js Co-authored-by: Mark Haslinghuis <[email protected]> * Update src/js/tabs/pid_tuning.js * Update src/js/tabs/pid_tuning.js * Update src/js/tabs/pid_tuning.js --------- Co-authored-by: Mark Haslinghuis <[email protected]>
I think the maths for this is now sorted so I've opened a fresh PR to keep things clean. Fixes #3852
@haslinghuis Please feel free to review now. @McGiverGim Please feel free to use this in your own PR and close this PR.