Quicktune C++ conversion (WIP)

[MichelleRos]

Add Quicktune to C++ code (conversion from the VTOL-quicktune lua script).

Just doing Copter for starters.

Tested in SITL, and one flight on a 5" copter.

Still a work in progress.

Edit: Tridge has added support for quadplanes and an autotest for it and made it not run for helis. I plan to add an autotest for the copter version shortly.
I have also updated the PR to limit quicktune to only alt_hold and loiter in copter, and Tridge did something similar for quadplanes.

Edit 2: Tridge updated it to make it always allocate, to be more in line with the pattern in copter, and moved it to its own thread. He also added a max attitude error, so it auto-aborts if the vehicle starts oscillating badly during the tune and made it abort the tune if the pilot changes to a mode that doesn't support quicktune. Note that it does not abort if the pilot changes between modes that do support quicktune, allowing people to, for example, do the tuning in loiter mode, then switch to alt_hold to test the tune before saving the gains.

Now tested on a quadplane as well.

As for the flash concerns, yes, it uses around 5k of flash, but autotune uses around 15k of flash, so for a tuner, quicktune is actually rather light.

[andyp1per]

Very nice! Will obviously require some include guards given the flash cost. Not done a review, but I think it would be better to try and combine the autotune parameter limits here. Ultimately I think this should be a function of the autotune module rather than a separate thing so that eventually we can have a "fast" autotune.

[rmackay9]

very nice to see this. Great to see quiktune graduate to C++!

[tridge]

great progress!
need to ask @bnsgeyer if he wants this for heli, if not then #if to disable

[tridge]

need to be in build_options.py and extract_features.py

[bnsgeyer]

@tridge thanks for including me on this PR. I will have to learn more about how the quick tune works. Copters and heli’s have a very different way of tuning. For instance, heli use the feedforward gain where copters do not. Also they tune their yaw axis in a very different way. so at this point, I’m not saying no, but I will have to have somebody show me more about how Quik tune works. I’ve been toying with an idea of how to do a similar quick tune using the frequency domain tuning.

[lthall]

This looks like it is something that can be turned on at any time, in any mode. Is that correct?

[lthall]

I think this needs to be done as a separate flight mode. I am not comfortable with making this something that just runs over any flight mode.

I am also not comfortable with this being stuck in the mode.ccp file like it is as fundamental as the attitude controller.

[MichelleRos]

This looks like it is something that can be turned on at any time, in any mode. Is that correct?

@lthall
Yes, that is the intent. IMO it makes for a much more user-friendly interface than a separate mode where you have to get used to the different behaviour along with watching the tuning process, which is already a risky & stressful time... I have been taken by surprise by autotune's different behavioura few times and had near-crashes because of that. One of the big advantages of quicktune is that while tuning, it just hovers there, with almost no difference to hovering normally in e.g. loiter mode, so it's much less risky.
It also reduces code duplication - to make this a mode, I would have to copy all of e.g. mode Loiter into a new file and then add the quicktune stuff to it and change the name...

We could limit it to only work in certain modes if that'd be better?
Also, the current function in mode.cpp is more complex - I am working on a simpler method, using the aux functions as per Tridge's feedback, although there will still be ~3 lines in mode.cpp. I could add it only to the modes we want it to work in, rather than to mode.cpp if that's better?

[lthall]

We can't have this turn on in any mode. At very least we would need to restrict this to manual modes and not during failsafe events.

There is a wider requirement for a code pocket that is constantly running where we make adjustments to the gains. That would be a good place for the ground resonance handling to reside for example. We also have a future problem if we run the rate controllers at a higher update rate that may also be able to make use of the same pocket.

[robustini]

@MichelleRos in my opinion it should be a feature that can be activated only in Position Hold and Loiter.
Thank you for this, useful here but it would also be useful for VTOLs without using LUA.

[MattKear]

For what it is worth, I am +1 for not having it turn on in any mode. I think that was acceptable when it was a script because no script, no problem. As this is going into cpp I think this needs to have the same constraints on it as the current autotune.

As this is essentially an alternative approach to the current autotune I think it would be best if this inherited from AC_Autotune. Fundamentally, we are doing a test, measuring the response, fiddling the gains, and repeating. There must be enough commonality with the current autotune that we can make this an "Autotune type" or "Autotune option". For example, heli has a different autotune that it inherits from the base class. The main difference here is that we would need some kind of auto tune _MODE or _OPTION param to select which version of autotune you wanted.

From a user perspective I think it is easier if the interface with autotune is kept as similar as possible to the existing method (hence my suggestion to inherit from the existing). It is when we have things that appear, externally, to be very different things that users get confused. I know I certainly do.... 😄

[andyp1per]

100% agree with @MattKear - I think the bar is higher for this to go into C++ and it needs to be done in a sympathetic way to the existing code. It also needs to be done in a way that doesn't confuse users. Quiktune and autotune both have pros and cons and I think the way we have been advising users to get an initial tune with quiktune before progressing to autotune is a not unreasonable one - but I think ideally this would be built into autotune itself so that you could still use the same parameters for selecting (e.g.) axis. I think currently the parameter options for quiktune are too complex and need to be slimmed down a bit in the same way that the parameter set for autotune is very simple. I can imagine a future autotune first doing a quiktune and then an autotune in the same pass and would not want the configuration to create a user expectation of either/or.

[rmackay9]

We should add Guided to the list of acceptable modes I think.

[bnsgeyer]

I agree with @lthall that it should be limited to manual modes. in addition, I'm not sure why we would let the user choose a mode other than alt-hold or stabilize. This is being used to tune an aircraft that may not even be able to successfully fly in loiter or guided modes. So why tune in those modes?

[timtuxworth]

It might be helpful to put the use case slightly differently @bnsgeyer - sure that might be true, but in my experience (mostly quadplanes, a couple of multi-rotors) the the use case for QuickTune is that the vehicle can fly in Q-Loiter, but not well, and specifically not well enough for autotune so you run quick tune until QLoiter is "ok". I have usually used QuickTune like this:

1. manual tune in QStabilize/QHover until it can QLoiter (Loiter in multi rotor)
2. quicktune in QLoiter/Loiter to get it "good enough" that autotune will run
3. autotune

Which is also what how the wiki says to use it. Which to me means - the mode I want quicktune to work in most of all is: QLoiter/Loiter

[lthall]

Quicktune is there to help people get from a very poor tune to a basic tune. It is promoted as safer, easier and more likely to be successful than AutoTune.

Quicktune is supposed to replace a basic manual tune. In my recommended tuning process manual tuning happens before AutoTune and before any position controlled mode is used.

If we let it be run in guided we will have people loading ArduCopter on a brand new aircraft and expecting to press take-off then switch on Quicktune.

Now we have 12 parameters (one an options bitmask) to make it easier for people who struggle to do a manual tune safely and we are talking about letting it run in automated modes. This just seems a little crazy to me.

[tpwrules]

As a big proponent of scripting I concur that I would like to see this remain a script instead of the overhead of flash and maintenance of C++ code.

I have been working on improvements to make scripts more feasible on autopilots with less free RAM, which should enable the quicktune script to be more widely useful. I don't know if it's in the cards for boards with <=1M flash or no SD slots though.

[lthall]

Assuming we elevate this to C++ and don't just leave it as a LUA script the key things I would like to see are:

1. Limited to Alt_Hold and Loiter modes. (Pos_Hold could be included and I need to hear why Randy thinks Guided would be a good idea)
2. Automatically stop running if the pilot is asking for a non zero climb rate.
3. Reduce the footprint in Copter::update_flight_mode() to a single line and put code in a separate function.
4. Change percentage and percentage reduction to simple multiplication factors, consistent with Copter.

[IamPete1]

I wonder if this would be better as a new type of autotune. The existing mode autotune already has all the functionality it needs to hook into a mode and does the pilot override thing and can get at all the params it needs directly.

My suggestion would be a new "AUTOTUNE_TYPE" param you could select the existing one or quicktune. I think you would have to pull out a common parent class, but you only need a run, save, stop and reset methods and no changes to copter except it would need to decide which one to use when you enter the mode for the first time.

[andyp1per]

I wonder if this would be better as a new type of autotune. The existing mode autotune already has all the functionality it needs to hook into a mode and does the pilot override thing and can get at all the params it needs directly.

My suggestion would be a new "AUTOTUNE_TYPE" param you could select the existing one or quicktune. I think you would have to pull out a common parent class, but you only need a run, save, stop and reset methods and no changes to copter except it would need to decide which one to use when you enter the mode for the first time.

100% agree, I think this would be a much better structure and save substantially on flash costs

[timtuxworth]

the two tuning algorithms have two very different goals: Quicktune is intended to just get a reasonably stable rate tune out of a starting tune that is downright dangerous. Autotune requires a pretty good tune to start with, and then improves it to attempt to find the best tune.

I agree, you have described it just how I see it, as steps in a process. Ideally (from my point of view), it would be great it it could work like this:

Manual tune -> Quicktune -> Autotune -> fly with confidence

[IamPete1]

After having a attempt at what Quciktune might look like as part of autotune I am coming round to the idea that it might be better on its own. The key thing is that is does not control the vehicle in any way, autotune puts in inputs, so we need to report that in the mode.

However, this PR has also kicked off some discussion about how the existing autotune might be improved, I suspect we will find that the two converge fairly rapidly.

So although I no longer think that this would be better running in MODE autotune, I do still think we should try and share as much functionality with the existing autotune library as possible. The methods of tunning are not the same, but there is lots of shared functionality. Saving and restoring of gains being the most obvious. They also both get string names for params.

Independent of that I think this PR would benefit from some restructuring, I think switching to storing a current axis and a stage within that axis would allow quite a bit of simplification and flash saving.

[IamPete1]

I agree with Andy, the function is called "Quicktune" we need to have common language throughout the documentation and interface as much as possible. Which GCS's would we need to update?

[tridge]

@MichelleRos I've pushed some updates and rebased

fixed