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

ManualControlSelector: Enable original PX4 default behavior until QGC catches up #18823

Merged
merged 2 commits into from
Dec 10, 2021

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Dec 9, 2021

Describe problem solved by this pull request
Since #17404 by default only RC is accepted and the original PX4 behavior of only accepting the first source present since boot is not supported anymore. I received somewhat obvious feedback from multiple users that were not aware that this has to be configured now.

Describe your solution
This PR enables the original behavior for default. Additionally I added an option to disable stick input completely which doesn't require specific implementation but should explicitly be possible and documented.

Describe possible alternatives
The goal is to make it mandatory for the user to configure what he's planning to use in the UI. Until this UI exists I recover the default behavior to not piss of numerous people that waste time finding the parameter. We should add the UI and then set the default to disabled such that the workflow is obvious.

Test data / coverage
I added unit tests for both cases and made sure they do exactly what we want.

Additional context
Add any other related context or media.

@MaEtUgR MaEtUgR added the UI / UX ✨ User Interface / experience improvements label Dec 9, 2021
@MaEtUgR MaEtUgR self-assigned this Dec 9, 2021
@dagar
Copy link
Member

dagar commented Dec 9, 2021

To add to the anecdata I've seen cases where the default behavior (first come first serve) resulted in confusion and even a crash.
So I'm definitely in favor of making it something that must be configured explicitly, I guess we just need to figure out how to transition.

Copy link
Contributor

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Nice, I think that makes sense!

@dagar dagar merged commit 897775f into master Dec 10, 2021
@dagar dagar deleted the manual-control-default branch December 10, 2021 14:11
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Dec 13, 2021

To add to the anecdata I've seen cases where the default behavior (first come first serve) resulted in confusion and even a crash.

I've seen a lot of confusion around this but luckily no crash (yet) but that's exactly the motivation. I think if it's clear in the UI that you enable RC/Joystick before configuring/calibrating it we can disable it by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI / UX ✨ User Interface / experience improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants