Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

av-stefan
Copy link

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).
image

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.

@@ -257,24 +257,61 @@ SetupPage {
}
}
}
WheelHandler {
onWheel: (event) => { // Capture the event object
let minimum = sliderRepeater.itemAt(0).channel.min;
Copy link
Author

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

Copy link
Author

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) {
Copy link
Author

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.

}
}
}
}
Copy link
Author

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.

@DonLakeFlyer
Copy link
Contributor

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?

@av-stefan
Copy link
Author

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.

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

Successfully merging this pull request may close these issues.

2 participants