ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
10.81k stars 17.26k forks source link

Support QuickTune like algorithm and improve Autotune #27624

Open lthall opened 2 months ago

lthall commented 2 months ago

I have created this Feature request to try to build a set of requirements to include QuickTune in Copter without the limitations apparent in the existing LUA script and C++ equivalent.

I will attempt to extend this based on feedback from people below.

A big picture brief would include:

Things we would like to improve on in the existing Quicktune algorithm:

Things we would like to improve on in the existing Loiter algorithm:

One of the key questions I would like input on below is what can we learn from QuickTune on the human interface with Autotune. The original human interface was a two position switch:

This let the person run Autotune then switch back and forth between the two sets of gains to test. It then saved what ever you landed with. People moved to just running it by switching modes and at some point this functionality was removed.

Open to the floor.

Creative Ideas:

timtuxworth commented 2 months ago

I'm pretty sure I heard yesterday that a key "requirement" was that it would run on 1M boards with small RAM such as the F405.

Personally I disagree with this, but some people seemed quite adamant about it.

timtuxworth commented 2 months ago

One of the key questions I would like input on below is what can we learn from QuickTune on the human interface with Autotune. The original human interface was a two position switch:

  • Low: Initial gains and pause tuning
  • High: Final tuned gains and continue tuning

This let the person run Autotune then switch back and forth between the two sets of gains to test. It then saved what ever you landed with. People moved to just running it by switching modes and at some point this functionality was removed.

I like the switches in QuickTune but IMO it's more important for the UI/Workflow to be consistent - can we not agree on one standard way for all flavors of Autotune (including Plane/fixed wing) to work?

andyp1per commented 2 months ago

I agree this is a reasonable requirement set. My two main issues with quiktune are:

The thing I like about quiktune is it is fast, but for copter at least I would never use it as autotune is also fast enough (for me) and autotune produces signifcantly superior tunes. I think integrating it into autotune would allow you to achieve both goals - a fast and accurate tune.

lthall commented 2 months ago

I'm pretty sure I heard yesterday that a key "requirement" was that it would run on 1M boards with small RAM such as the F405.

I believe that autotune is on those boards. The goal would be able to support both or even only the quicktune component to save space. That would support Tridges desire to save space by removing autotune.

I like the switches in QuickTune but IMO it's more important for the UI/Workflow to be consistent - can we not agree on one standard way for all flavors of Autotune (including Plane/fixed wing) to work?

Yes, that is the plan. The question is what should the switch configuration be. Quicktune seems to have a couple switch options. Autotune used to have switch options but they were removed because people were just using the mode and calling it good. It was then removed and nobody noticed.

Ideas and suggestions on switch options is one of the key feedback I would like to see.

MichelleRos commented 2 months ago

As I've already said, I don't think it's a good idea to combine autotune and quicktune. The point and purpose of making it a separate implementation is so that people can choose to only have one or the other on eg low-flash boards etc.

Also, not everyone will need both: For example, for a lot of mid-sized copters, the default tune is probably good enough to just go to autotune directly.

As for angle_p, I've already pointed out on the PR's discussion why quicktune doesn't need to do angle_p gains.

And as we've already tried to explain on the PR, all of the parameters that are there have been added because of specific needs for certain vehicles.

lthall commented 2 months ago

making it a separate implementation is so that people can choose to only have one or the other on eg low-flash boards etc.

Yes if we combine both we should endeavour to make them both a compile time option. I mentioned doing that with just the autotune part but yes, we can probably do both. I suspect people won't want to do that though.

And as we've already tried to explain on the PR, all of the parameters that are there have been added because of specific needs for certain vehicles.

Yes but you have not explained what those specific needs were and given I have a lot of tuning experience and can't immediately see why they are there it would be helpful if you went into more detail. That would then let us decide if we wanted to add those parameters to the existing autotune too.

MichelleRos commented 2 months ago

Yes if we combine both we should endeavour to make them both a compile time option.

Sounds like you want to entangle them both into the current autotune mode, and then detangle them as compile time options with ifdefs... I don't see how that's not gonna end up as very messy code.

lthall commented 2 months ago

I have been thinking about this and am feeling more and more enthusiastic about the potential of this approach. I also think that we can make it significantly more reliable with some additional factors from inside the C++ code if done as an Autotune Mode.

I see some parallels with @bnsgeyer approach. I have an idea that may let us directly measure the 180 degree gain margin. Very similar to QuickTune but more robust. I will have to test this idea and chat to Bill but it may be an idea to address one of the current weaknesses of QuickTune.

timtuxworth commented 2 months ago

Yes if we combine both we should endeavour to make them both a compile time option.

Sounds like you want to entangle them both into the current autotune mode, and then detangle them as compile time options with ifdefs... I don't see how that's not gonna end up as very messy code.

In his review @andyp1per gave some pretty good examples of reasons why it would probably be a good design choice to recognize that there is a common base of code that would be shared between the two autotunes. Sure "entangling" can be bad if it's just a mess of spaghetti code, but a properly factored set of classes with suitable inheritance should lead to much cleaner code and likely good, easy ways to include no autotune, base autotune + classic autotune or base autotune + quicktune or base autotune + classic + quicktune.

Once you start down this path it would likely be easier over time to reduce flash usage by moving to a common standard workflow/UI and shared parameters.

andyp1per commented 2 months ago

In his review @andyp1per gave some pretty good examples of reasons why it would probably be a good design choice to recognize that there is a common base of code that would be shared between the two autotunes. Sure "entangling"

It actually won't be bad at all - the refactoring has already done for Heli - quiktune would easily and simply fit as a subclass of the main autotune class.

andyp1per commented 2 months ago

There seems to be some misconception that this is about addressing flash cost - while there might be some flash savings the main reason for incorporating quiktune features would be to provide a better experience for users. Currently neither provides a truly simple, quick and reliable experience for users, but I am reasonably confident that some combination of the two could provide a truly superior experience. I would really like us to set aside our own personal biases and focus on doing something better for users as tuning is still the greatest impediment to using ArduPilot IMO.

lthall commented 2 months ago

I would be very interested in hearing from people that have successfully used both QuickTune and Autotune on what they would like to see from a human interface perspective. In particular switch operation.

lthall commented 2 months ago

I just had a very interesting conversation with @timtuxworth about the user interface for Autotune. Out of that conversation came a couple ideas I want to note down.

3 position switch:

  1. Original Gains / Do Nothing
  2. Tune until completion then do nothing
  3. Post Tune gains and save on disarm (it would be great to do this in any mode)

Mode switch: may or may not be required to initiate Autotune.

This is very close to how we did this originally.

It would also be nice to be able to record a partial tune. This is difficult to do with AutoTune but there is an opportunity to autotune the rate PID's and Angle P separately. So we could stop and store only just the Rate tune. It may also be possible to separate Rate P from Rate D but what we often find is that Rate D needs to be reduced as we increase Rate P. At the end of the Rate D tune the Rate P is low because that is required to tune Rate D.

We could add a bitmask to select a subset of Rate P, Rate D/FLT_E(yaw), and Angle P. This would support using Autotune to tune just part of the system. I have often wanted to do this but thought it would add too much complexity for the community.

The other thing we talked about was improving the Loiter / pos hold behaviour. I have ideas on how to do this. My hope is I can replicate how I manually control the aircraft in the Alt_Hold version of Autotune to prevent speed build-up and control position (and perhaps better).

Thanks Tim for taking the time to go through this in detail!!

More feedback and ideas would be great!!!!

timtuxworth commented 2 months ago

Thanks for listening Leonard.

As I pointed out, when tuning a quadplane you have to tune both the multirotor and the fixed wing behavior, and right now Fixed Wing "Autotune" uses a mode. It's unfortunate that this is different to how Quicktune works (no mode). Could we not have all tuning happen in a mode for consistency and ease of understanding for the user? Combined with Leonard's suggestion this would be something like this:-

Fixed Wing tune = Autotune mode + switches to save/abandon tune (rather than current FW autotune saving) Multirotor Basetune (current Quicktune) = TuneBase mode + switches Mulirotor Autotune (current Autotune) = TuneAuto mode + switches

Of course it doesn't all have to be done at once, but could set the goal that we want to have a similar UI/workflow for all 3 different "autotune"s?

lthall commented 2 months ago

One option with Autotune is to change the axis choice to start with a quicktune like option. So you could actually automatically start with quicktune to get close then follow up with the full autotune.

lthall commented 2 months ago

@peterbarker had a good point that multi mode entry. We may be able to remove the Loiter/Alt_Hold starting decision and simply use a Options Bit to define it.

Currently we have a switch that only switches to the Autotune Mode. His suggestion was to add a middle position that would test the autotune gains. There is a challenge with systems that only have a two position switch. They would only get the current behaviour.