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

Follow up improvements for the manual control cleanup #18693

Merged
merged 4 commits into from
Nov 23, 2021
Merged

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Nov 22, 2021

Describe problem solved by this pull request
During further testing of the recent changes in #17404 I found two issues:

  1. Moving the mode switch/button to an unassigned slot results in a message "Switching to Unkown is currently not available"
  2. There's no way to disable the stick gesture for arming.
    This was previously implicitly (!) possible by either setting COM_RC_IN_MODE to joystick or mapping an arm switch.
  3. It is possible to enable yaw Airmode even though the stick gesture for arming is enabled

Describe your solution

  1. I'm not sending out an invalid/"Unkown" mode switch request with commander_state.main_state requested to be -1. This is the default value representing unassigned slots but when casted to the main_state type uint8_t being 255 and hence resulting in the expected error.
  2. There needs to be an explicit way MAN_ARM_GESTURE to disable it because I've seen it more than once as a product or hobby vehicle setup requirement independent of joystick or arming switch use.
  3. The new parameter from the last point cannot be enabled at the same time as yaw Airmode for safety reasons..

Test data / coverage
I bench tested everything to work as expected.

They were resulting in a message
"Switching to Unkown is currently not available".
@MaEtUgR MaEtUgR added the bug label Nov 22, 2021
@MaEtUgR MaEtUgR requested a review from julianoes November 22, 2021 15:03
@MaEtUgR MaEtUgR self-assigned this Nov 22, 2021
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 fixes!

… enabled

It's unsafe to arm with the gesture when yaw airmode is enabled
because e.g. in Stabilized mode that results in a high yawrate setpoint
that the drone tries to follow even with zero thrust
because of the airmode.

It was handled before by checking the arm switch parameter because that
used to disable the stick arm gesture.
See 24dc316
@bkueng
Copy link
Member

bkueng commented Nov 23, 2021

It makes sense but I find it a bit unintuitive that you still need to disable stick arming when mapping a switch. Also for people updating with a switch mapped and now suddenly they have the gesture enabled as well.
At the least it needs to be well documented and communicated.

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Nov 23, 2021

I find it a bit unintuitive that you still need to disable stick arming when mapping a switch.

@bkueng You're right. I found this was added here #9622 So as discussed the yaw airmode requires the gesture to be disabled but the easiest is to force disable it when an arm switch is mapped because that's a 1:1 replacement and there was no reported use case that needed both. I'm adding in another pr 👍 EDIT: #18705

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

Successfully merging this pull request may close these issues.

3 participants