-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Joystick: Full support for manual control extensions #13915
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?
Conversation
|
FYI: Not ready for testing yet. Waiting on Copilot review and then my own re-testing after I fix up whatever it finds. |
|
FYI: @Niki-dev12, @ikalnytskyi, @HTRamsey. Stay tuned for when it's ready for testing. |
Pre-commit Checks Failed
Hook ResultsFiles Modified by HooksHow to Fix./tools/pre-commit.sh
git add -u && git commit --amend --no-editRun ID: 21568280778 |
Build ResultsPlatform Status
All builds passed. Test Resultslinux_gcc_64: 635 passed, 0 skipped Total: 635 passed, 0 skipped Code CoverageCoverage: 100.0% No baseline available for comparison Artifact Sizes
No baseline available for comparisonUpdated: 2026-02-01 19:44:33 UTC • Triggered by: Android |
1b226bb to
fd2024b
Compare
fd2024b to
85f5e96
Compare
|
@HTRamsey I ended up with a nasty merge conflict with your |
|
Important things to check:
|
85f5e96 to
87903d5
Compare
Also: * Major cleanup on logging * Much better settings read/save with validation * Changes were to comprehensive to maintain conversion of legacy joystick settings. Joysticks will need re-calibration and enabling
87903d5 to
5c3ca70
Compare
|
Oh yeah that's fine. I was having trouble figuring out if my calibration problem was SDL related or something else. Also FYI for bottom LS and bottom RS, you seem to have to go the other way from neutral a bit and then go to the target extreme to get it to register correctly. |
Hmm, I never noticed that in the like 1000 joystick cals I went through to test this. |
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.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
| Repeater { | ||
| model: [ | ||
| { name: qsTr("Pitch"), extensionEnabled: controller.pitchExtensionEnabled, mapped: controller.pitchExtensionChannelMapped, value: controller.adjustedPitchExtensionChannelValue, deadband: controller.pitchExtensionDeadband }, | ||
| { name: qsTr("Roll"), extensionEnabled: controller.rollExtensionEnabled, mapped: controller.rollExtensionChannelMapped, value: controller.adjustedRollExtensionChannelValue, deadband: controller.rollExtensionDeadband }, | ||
| { name: qsTr("Aux 1"), extensionEnabled: controller.aux1ExtensionEnabled, mapped: controller.aux1ExtensionChannelMapped, value: controller.adjustedAux1ExtensionChannelValue, deadband: controller.aux1ExtensionDeadband }, | ||
| { name: qsTr("Aux 2"), extensionEnabled: controller.aux2ExtensionEnabled, mapped: controller.aux2ExtensionChannelMapped, value: controller.adjustedAux2ExtensionChannelValue, deadband: controller.aux2ExtensionDeadband }, | ||
| { name: qsTr("Aux 3"), extensionEnabled: controller.aux3ExtensionEnabled, mapped: controller.aux3ExtensionChannelMapped, value: controller.adjustedAux3ExtensionChannelValue, deadband: controller.aux3ExtensionDeadband }, | ||
| { name: qsTr("Aux 4"), extensionEnabled: controller.aux4ExtensionEnabled, mapped: controller.aux4ExtensionChannelMapped, value: controller.adjustedAux4ExtensionChannelValue, deadband: controller.aux4ExtensionDeadband }, | ||
| { name: qsTr("Aux 5"), extensionEnabled: controller.aux5ExtensionEnabled, mapped: controller.aux5ExtensionChannelMapped, value: controller.adjustedAux5ExtensionChannelValue, deadband: controller.aux5ExtensionDeadband }, | ||
| { name: qsTr("Aux 6"), extensionEnabled: controller.aux6ExtensionEnabled, mapped: controller.aux6ExtensionChannelMapped, value: controller.adjustedAux6ExtensionChannelValue, deadband: controller.aux6ExtensionDeadband } | ||
| ] |
Copilot
AI
Feb 1, 2026
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.
Same issue as the attitude controls repeater: the extension controls Repeater model depends on rapidly-changing controller properties (values/deadbands/mapping), which can trigger repeated model rebuilds and UI churn. Prefer a static model (names/ids) and compute/bind the live controller values inside the delegate.
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.
Tracked in #13917 and will be fixed in alter pull
| Repeater { | ||
| model: [ | ||
| { name: qsTr("Pitch"), mapped: controller.pitchChannelMapped, value: controller.adjustedPitchChannelValue, deadband: controller.pitchDeadband }, | ||
| { name: qsTr("Roll"), mapped: controller.rollChannelMapped, value: controller.adjustedRollChannelValue, deadband: controller.rollDeadband }, | ||
| { name: qsTr("Yaw"), mapped: controller.yawChannelMapped, value: controller.adjustedYawChannelValue, deadband: controller.yawDeadband }, | ||
| { name: qsTr("Throttle"), mapped: controller.throttleChannelMapped, value: controller.adjustedThrottleChannelValue, deadband: controller.throttleDeadband } | ||
| ] |
Copilot
AI
Feb 1, 2026
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.
Using a JS array literal with live controller properties as the Repeater model will cause the entire model (and delegates) to be rebuilt whenever any channel value/deadband/mapped property changes (which is frequent during calibration). Consider using a static model (names/ids only) and bind delegate properties directly to controller values based on index/id to avoid constant delegate destruction/recreation.
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.
Tracked in #13917 and will be fixed in alter pull
|
This is ready for testing now. @HTRamsey Can you help test RC transmitter. |
Also: