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: adding new mode QAUTOTUNE #9967

Merged
merged 4 commits into from Dec 14, 2018

Conversation

Projects
None yet
5 participants
@nikhil9
Copy link
Contributor

nikhil9 commented Dec 7, 2018

Copter like autotune support for quadplanes and tailsitter in VTOL mode. This has been ported from copter autotune and procedure is similar copter. Relevant autotune param will appear in quadplane param section with q_ prefix to copter params. As of now this mode will work at flight mode 22. If this is accepted I can send pr to mission planner, qgc and mavproxy for the integration of same.

I have tested this in sitl, quadplane and tailsitter. This works very well and found that copter like autotuning can improve performance in VTOL mode many folds.

@IamPete1

This comment has been minimized.

Copy link
Contributor

IamPete1 commented Dec 7, 2018

Great to see this, I have been thinking how useful it would be for along time.
I think there a a few cases of where tabs have been used instead of 4 spaces, and its quite allot to put in a single commit, maybe you could spread it out abit.

Thanks!

edit: I wonder if it would be beneficial to widen the range of parameters, I know people who flash copter to tune quad-planes often struggle to get it to complete.

@makeflyeasy

This comment has been minimized.

Copy link

makeflyeasy commented Dec 8, 2018

I am very glad to add this function. I hope the authorities will consider it carefully

@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 8, 2018

fantastic, thanks so much for this! I'll look at this as soon as I can

@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 8, 2018

The PR seems incomplete, maybe you missed some commits? For example, qautotune_save_tuning_gains() is never called, and even if it was called it seems to read gains, not save them.

@nikhil9

This comment has been minimized.

Copy link
Contributor Author

nikhil9 commented Dec 8, 2018

I missed a commit. Now its saving gains on disarm. Tabs are also replaced by spaces.

@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 8, 2018

I've updated the PR branch to use a separate class for QAutoTune. This keeps the code a bit more separated, and is also much closer to the code in copter

@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 8, 2018

some issues:

  • we should revert gains (not save), if we change mode before disarming (ie. user cancels the tune)
  • the poshold in QLOITER doesn't seem to work? (it drifts when testing in SITL)
@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 9, 2018

I've pushed some fixes to the logging, and fixed the reversion of gains on mode change

@nikhil9

This comment has been minimized.

Copy link
Contributor Author

nikhil9 commented Dec 9, 2018

  • the poshold in QLOITER doesn't seem to work? (it drifts when testing in SITL)

I have skipped few checks and conversions in qautotune::run() which where copter specific. That might be the reason it is not holding position in qloiter. I have tested the code by switching from qhover mode mostly. Let me check what is missing.

@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 9, 2018

I've done another commit which brings the code much closer to the current copter autotune (from master). I think you may have been working on a slightly older version?

@nikhil9

This comment has been minimized.

Copy link
Contributor Author

nikhil9 commented Dec 9, 2018

I have been working on latest plane release as I wanted to do actual test flights. I tried to make it up-to master but I guess I missed few things as I did not realise changes in copter auto-tune.

@nikhil9

This comment has been minimized.

Copy link
Contributor Author

nikhil9 commented Dec 9, 2018

Fixed posehold during qautotune when switched from Qloiter.

@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 10, 2018

@nikhil9 thanks!

@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 10, 2018

I had a chat to Leonard @lthall about this. We both think that longer term we'd like this to be a library, maybe libraries/AC_Autotune, and both copter and plane use the same code. The library would be done with the vehicle code creating sub-classes that implement the vehicle specific code, much like we use for AP_Arming and RC_Channel.
The other big missing piece is support for autotune of tailsitters. The problem with tailsitters is the need for feed-fwd as a large component of the control. The current autotune code doesn't handle that at all well, and will produce a tune that is likely to crash the plane (I tested it in SITL and it did crash after autotune on a twin-motor tailsitter).
So we'll need to recommend that people only use QAUTOTUNE for multicopter based quadplanes for now. Leonard has plans to support controllers that use feed-fwd in the future, but that may be a while off unless someone else can do it.
I don't think we have to convert it to a library before we merge this PR (although that would be nice!), but I would like to spend a bit more time testing it later this week.
thanks again for your work on this!

@pompecukor

This comment has been minimized.

Copy link

pompecukor commented Dec 11, 2018

This is so awesome guys. I cannot wait to try it.

@tridge tridge force-pushed the nikhil9:pr-qautotune branch from c8812c6 to 3486221 Dec 12, 2018

@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 12, 2018

@pompecukor has volunteered to test fly this on a tilt-vectored quadplane
I've put a CubeBlack build for him here:
http://uav.tridgell.net/QAutoTune/

@pompecukor

This comment has been minimized.

Copy link

pompecukor commented Dec 12, 2018

Tested. Green light all the way. Really impressive thank you. Sent some videos to dr. Andrew :)
I hope you can merge soon.

Only (minor) recommendation that I have is to send mav message "Autotune done". So we can hear or see it on mission planner. The reason is that the beep from the buzzer is not really audible does to the props in action :)

This especially for first time users (like me) who are unsure about how it ends/stops.

2m wind UAV took less than 10mins, for reference. So if you do not have battery for 15mins hover on your plane, then I recommend doing it in bits. Roll, Pitch then Yaw. So those will be less than 3mins each.
And you can swap batteries in-between sessions.

@nikhil9 can I buy you a beer? 👍

nikhil9 and others added some commits Dec 7, 2018

Plane: adding new mode QAUTOTUNE
copter like autotune support for quadplanes and tailsitter in VTOL mode.

cleanup
AC_AutoTune: converted autotune mode to a library
this allows for common code between copter and quadplanes

@tridge tridge force-pushed the nikhil9:pr-qautotune branch from 3486221 to 3f37801 Dec 13, 2018

@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 13, 2018

I've now re-worked it as an AC_AutoTune library, so we are no longer copying code

@tridge tridge requested a review from rmackay9 Dec 13, 2018

tridge added some commits Dec 13, 2018

AC_AutoTune: fixed time subtraction bug
would have failed at time wrap point

@tridge tridge force-pushed the nikhil9:pr-qautotune branch from a686742 to 2e39bb3 Dec 14, 2018

@tridge tridge merged commit 1f70c38 into ArduPilot:master Dec 14, 2018

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
semaphoreci The build is pending on Semaphore.
Details
@tridge

This comment has been minimized.

Copy link
Contributor

tridge commented Dec 14, 2018

I've merged this, but without the copter changes, and will open a separate PR for copter
@nikhil9 many thanks for your work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment