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

Plane: add AirMode RC option for quadplanes #14671

Merged
merged 6 commits into from Aug 12, 2020

Conversation

kd0aij
Copy link
Contributor

@kd0aij kd0aij commented Jun 23, 2020

add Q_OPTION for AirMode (auto-enabled if RCx_OPTION ARMDISARM assigned). There is no behavior change unless an RC option is assigned for either ARMDISARM or AIRMODE.

This improves control in VTOL modes with the throttle near zero (SPIN_MIN) by preventing throttle suppression and by setting throttle mix to manual when in manual throttle modes.

includes a bugfix for manual throttle mix for qacro

ArduPlane/Plane.h Outdated Show resolved Hide resolved
@IamPete1
Copy link
Member

Do we need a Qoption and a switch?

@Hwurzburg
Copy link
Collaborator

I think Mark was concerned about altering existing operation....if its mirroring Copter it would only need to make airmode active if armed by switch function 41 and then add the air mode disable switch like copter just added...but that is different than now with switch 41

@kd0aij
Copy link
Contributor Author

kd0aij commented Jun 23, 2020

@IamPete1 Without the new q_option (defaulting to 0) this would change behavior for quadplane users who used a switch for arming.
@Hwurzburg I just realized that this implementation for Q_OPTION=AIRMODE completely overrides the RC_OPTION for airmode; if you set that option bit airmode is always ON when armed, regardless of the RC switch position. But if you use rudder arming, you can turn airmode on and off (while armed) using the assigned switch.

@Hwurzburg
Copy link
Collaborator

yep,I would think that the option would turn on AIRMODE with no AIRMODE switch (better use an arm/disarm switch then), but that the AIRMODE switch, if configured, would determine when it was on or off....and disarm/arm switch is independent of all of this....assuming you want to keep the same profile as now...wont be like copter, but ....not a problem I guess...

@IamPete1
Copy link
Member

IamPete1 commented Jun 23, 2020

I would remove the ability for Airmode via arming/disarming switch, that was really accident on copter. If you going to have to have a switch you might as well have it dedicate to Airmode.

@kd0aij
Copy link
Contributor Author

kd0aij commented Jun 23, 2020

@Hwurzburg Tested in SITL:
(edited)
If you set the new Q_OPTIONS bit 7, and have arming on a switch, you'll have airmode at all times when armed unless
rudder arming is enabled, then you can't get airmode unless you set an RC option to AIRMODE. Then the RC_OPTION switch controls airmode, regardless of rudder_arming_type and Q_OPTIONS.

I believe this now behaves the same as copter only if the Q_OPTIONS bit is set, and does not change existing behavior unless the new RC_OPTION is assigned.

@khancyr khancyr added the Plane label Jun 24, 2020
@Hwurzburg
Copy link
Collaborator

Mark, with option bit set...is it when rudder arming is enabled? or used to arm? that the switch is needed to get into AIRMODE....in copter if you arm by rudder, you need the switch....so the option bit is only for airmode on arm switch, right? with no arm/disarm you need the airmode switch, right?

also, I think you need to update RC_Channel.cpp with the AIRMODE switch metadata now that Plane uses it,right?

@kd0aij
Copy link
Contributor Author

kd0aij commented Jun 24, 2020

I forgot there was separate metadata for each vehicle... will update the plane entry.

Is my description of the options bit and rc_option above ambiguous or unclear? That is essentially the same text I'd propose to add to the wiki.

@Hwurzburg
Copy link
Collaborator

"If you set the new Q_OPTIONS bit 7, and have arming on a switch, you'll have airmode at all times when armed." okay and what if you armed with the rudder?immediate air mode and jumping around until you release rudder? is AIRMODE active then? this is not what Copter does, so I do think it might be unclear a bit...thats why I asked?

@kd0aij
Copy link
Contributor Author

kd0aij commented Jun 25, 2020

The next paragraph is "If rudder arming is enabled, then you can't get airmode unless you set an RC option to AIRMODE. Then the switch controls airmode, regardless of rudder_arming_type and Q_OPTIONS."

I can't see how that doesn't answer your question, but maybe I'm missing something?

@Hwurzburg
Copy link
Collaborator

Its just a matter of having every statement true on its own and not qualified by later statements. I would word it thus:

If the Q_OPTIONS bit 7 is set to "1", and only have arming possible via an ARM/DISARM RCx_OPTIONS switch, or via GCS arm command, then AIRMODE will be active at all times when armed.
If rudder arming is enabled (ARMING_RUDDER = 1 or 2), then AIRMODE can only be activated by using an AIRMODE RCx_OPTIONS switch.

@kd0aij
Copy link
Contributor Author

kd0aij commented Jun 25, 2020

OK, I thought you were saying the behavior was not what you wanted.
If this gets merged, I'll let you write the wiki entry :)

@IamPete1 In reply to your last comment the Q_OPTIONS bit for airmode on arming was specifically requested by Henry.

@Hwurzburg
Copy link
Collaborator

so that people migrating from Copter to QuadPlane have the same experience in VTOL modes

ArduPlane/quadplane.cpp Outdated Show resolved Hide resolved
ArduPlane/quadplane.cpp Outdated Show resolved Hide resolved
@kd0aij kd0aij force-pushed the pr-airmode-plane branch 2 times, most recently from 3166b2f to f55f13a Compare July 8, 2020 00:24
@kd0aij
Copy link
Contributor Author

kd0aij commented Jul 8, 2020

@Hwurzburg I changed this to limit auto-airmode to switch arming only:
If you set the new Q_OPTIONS bit 7, and arm using a switch, arming/disarming also turns airmode on/off.
If you assign an aux switch for AirMode, that switch controls airmode, with no exceptions.

@Hwurzburg
Copy link
Collaborator

super! sorry my last addition causes you to fix and rebase!

@kd0aij kd0aij force-pushed the pr-airmode-plane branch 2 times, most recently from 1a157ad to 1256dd6 Compare July 13, 2020 03:05
ArduPlane/quadplane.cpp Outdated Show resolved Hide resolved
@kd0aij kd0aij force-pushed the pr-airmode-plane branch 3 times, most recently from c67fc68 to f2b1bdf Compare July 14, 2020 12:51
@@ -1920,6 +1920,11 @@ void QuadPlane::update_throttle_suppression(void)
return;
}

// if in a VTOL manual throttle mode and air_mode is on then allow motors to run
if (plane.control_mode->is_vtol_man_throttle() && air_mode == AirMode::ON) {
Copy link
Contributor Author

@kd0aij kd0aij Jul 14, 2020

Choose a reason for hiding this comment

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

throttle suppression (and throttle mix) behavior changes restricted to qstab and qhover using new accessor in Mode class

@kd0aij
Copy link
Contributor Author

kd0aij commented Jul 28, 2020

Demos in RF8/SITL

Convergence tilt-tri, QSTABILIZE mode: takeoff then cut throttle to zero, first with airmode off, then with airmode on
https://youtu.be/DFXEZXBejnE

BlackCat tailsitter, QSTABILIZE mode: flying around at zero throttle in airmode, airmode switched off at 40 seconds; complete loss of control.
https://youtu.be/1DeOen5ySoE

    add Q_OPTION for AirMode (auto-enabled if RCx_OPTION ARMDISARM assigned)
    bugfix: manual throttle mix for qacro
    qualify auto airmode on/off
    add Air Mode to Plane RC_OPTION metadata
    restrict airmode to manual throttle modes
    add qhover to manual throttle mix
    move air_mode from Plane to QuadPlane
    add Mode::is_vtol_man_throttle()
@tridge
Copy link
Contributor

tridge commented Aug 4, 2020

Henry will do testing

@kd0aij kd0aij force-pushed the pr-airmode-plane branch 2 times, most recently from c1b0fa7 to e8f8113 Compare August 4, 2020 13:45
@kd0aij
Copy link
Contributor Author

kd0aij commented Aug 4, 2020

@peterbarker I added a commit to extend the autotest with the additional testing that Tridge suggested. Does that change look OK to you?

Tools/autotest/quadplane.py Outdated Show resolved Hide resolved
@peterbarker
Copy link
Contributor

@kd0aij thanks for fixing that test up. Just waiting for test results now, I think.

@tridge
Copy link
Contributor

tridge commented Aug 12, 2020

thanks to @Hwurzburg for testing!

@tridge tridge merged commit d0ff26e into ArduPilot:master Aug 12, 2020
@Hwurzburg Hwurzburg removed the WikiNeeded needs wiki update label Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants