-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Make scrolling wheel on actuator test not be a significant increment #12130
base: master
Are you sure you want to change the base?
Conversation
@@ -257,24 +257,61 @@ SetupPage { | |||
} | |||
} | |||
} | |||
WheelHandler { | |||
onWheel: (event) => { // Capture the event object | |||
let minimum = sliderRepeater.itemAt(0).channel.min; |
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.
I don't like this piece of code since it assumes there is at least 1 actuator, and that all actuators have the same minimum and maximum. I am open to suggestions on how this could be improved
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.
I think that all having the same minimum and maximum is always the same now that I think about it since the Mavlink message sent to do the actuator test only allows a value from 0 to 1.
let stepSize = (maximum - minimum) * 0.01; | ||
|
||
const NAN_THRESHOLD = -0.1; | ||
if (value < NAN_THRESHOLD && event.angleDelta.y > 0) { |
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.
When you scroll down to the point of deactivating the actuator test, the value becomes a NAN (as suggested by onActuatorValueChanged above these changes). Whenever I scrolled back up, instead of having NAN it would be a negative value of -0.16. Since the minimum I have seen has always been 0.0 I interpreted that as NAN, but would be open to suggestions on how to do this better.
} | ||
} | ||
} | ||
} |
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.
These changes are the same to those above, just on different actuator sliders.
I wonder whether a better fix for this is to disable Wheel support in QGCSlider? In general that is what I have done in other controls. The reason being that wheel is used 99% of the time to scroll a page up and down. But then if you happen to scroll the page such that the mouse falls over some control that supports wheel (like a slider) then suddenly you are affecting the values of that control with the wheel when all you really wanted to do was keep scrolling. That would be my suggestion. I don't see huge value in wheel support for sliders. Dragging only is fine. Opinions? |
Probably less code while achieving the same goal so I agree. Personally don't know how to do it but I imagine it would be easy to achieve. Could you take care of that? Otherwise I'll find out how to do it. |
Description
When you use a mouse and scroll up to give just a single input, the power given to the actuators is incredibly big (see screenshot).
To prevent such unexpected large jumps in the amount of power given to the actuators, this PR limits how much the value actually jumps to be 1% per scroll input.
Test Steps
Connect with a drone (preferribly a SITL such as gazebo), go to vehicle setup, then actuators and enable the sliders. Then, simply scroll up and down with your mouse over the sliders.
Checklist:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.