Copter: Autotune performance and safety enhancements #26245

lthall · 2024-02-17T02:21:33Z

This PR attempts to address some of the long term concerns with AutoTune.

Autotune now fails when it becomes clear the aircraft will probably fail to tune well.
The recovery from the rate twitch makes use of the control model to reduce the severity of the ESC command. This should dramatically reduce the chance of desync.
The waiting for level criteria has been adjusted to reduce the chance initial conditions cause problems with the tests.
The practice of sipping a side in an attempt to save time has been removed as it causes aircraft velocity to build. I believe removing this is a net gain in both time and reliability.
Base test angles on ratio's of ANGLE_MAX.

These changes are expected to dramatically reduce crashes due to desync and successful autotune with bad/dangerous tunes.

We will see an increase number of complaints and questions along the lines of "Why did autotune fail to complete" and "Autotune failed with a failed to level error".

Improvements for a future PR

Make the test twitch more consistent on each test by continuing the twitch after the measurement is complete. This will keep twitches symmetrical and reduce the build up of velocity
Update the twitch rate and angle target based on the results of the last four twitches to reduce the number of aborted tests and therefore the total time taken.
Another protection I should add is to stop angle P term increase when negative rates are observed but angle overshoot is small. This will help prevent excessively large Ange P results.

libraries/AC_AutoTune/AC_AutoTune.cpp

libraries/AC_AutoTune/AC_AutoTune_Multi.cpp

libraries/AC_AutoTune/AC_AutoTune.cpp

IamPete1

The only one that makes me a bit nervous is the failing to level level change. I don't think I have ever done a auto-tune without getting the old warning at least once.

lthall · 2024-02-17T13:08:48Z

The only one that makes me a bit nervous is the failing to level level change. I don't think I have ever done a auto-tune without getting the old warning at least once.

Yeh I found this was a problem when I made it fail. But then I found there was a few bugs that could make this happen when it shouldn't. So I hope both will add up to "all good". We will have to see.

I share your concern though.

andyp1per · 2024-02-18T09:13:23Z

The only one that makes me a bit nervous is the failing to level level change. I don't think I have ever done a auto-tune without getting the old warning at least once.

Somehow my reply to this got lost. Yes - I always get at least one "failing to level" error on yaw. Yaw is the problem child. I guess time will tell whether @lthall has managed to get rid of all the gremlins

libraries/AC_AutoTune/AC_AutoTune.h

libraries/AC_AutoTune/AC_AutoTune_Multi.cpp

andyp1per

Just some comments

peterbarker

Not really fussed with the ratio/SCALE thing, just noting we often using SCALE for the 0-1 scaling values.

libraries/AC_AutoTune/AC_AutoTune.cpp

peterbarker · 2024-02-19T01:14:49Z

libraries/AC_AutoTune/AC_AutoTune.cpp

@@ -391,14 +387,20 @@ void AC_AutoTune::control_attitude()
        test_run(axis, direction_sign);

        // Check for failure causing reverse response
-        if (lean_angle <= -AUTOTUNE_TARGET_MIN_ANGLE_RLLPIT_CD) {
+        if (lean_angle <= -attitude_control->lean_angle_max_cd() * AUTOTUNE_TARGET_MIN_ANGLE_RLLPIT) {


I wonder if we should have this:

Suggested change

if (lean_angle <= -attitude_control->lean_angle_max_cd() * AUTOTUNE_TARGET_MIN_ANGLE_RLLPIT) {

if (lean_angle <= -lean_angle_max_cd()) {

No, this is where the aircraft is going in the opposite direction to what it should be going. We don't want to wait for it to get all the way out to lean angle max. This threshold can and should be small.

Sorry, I wasn't clear in my comment.

I was suggesting breaking out a method:

float lean_angle_min() const { return attitude_control->lean_angle_max_cd() * AUTOTUNE_TARGET_MIN_ANGLE_RLLPIT; }

Most of the lines touched in this patch add that multiplication, so make a method for it.

... it might be that these methods would only be used in one place, so perhaps not worth it.

I would offer another suggestion. Could we make this a virtual method so that heli and multi can apply different criteria to reverse response and exceeding max lean angle. I left these alone when i first implemented heli autotune but now I don't know if I can make these changes work for heli, especially the 2/3 max lean angle criteria.

libraries/AC_AutoTune/AC_AutoTune.cpp

peterbarker · 2024-02-19T01:21:54Z

libraries/AC_AutoTune/AC_AutoTune_Multi.cpp

@@ -797,6 +801,8 @@ void AC_AutoTune_Multi::updating_rate_d_up(float &tune_d, float tune_d_min, floa
                tune_d = tune_d_min;
                counter = AUTOTUNE_SUCCESS_COUNT;
                LOGGER_WRITE_EVENT(LogEvent::AUTOTUNE_REACHED_LIMIT);
+                // This may be mean AGGR should be increased or MIN_D decreased


This comment should probably make the Wiki @Hwurzburg

bnsgeyer · 2024-02-20T01:03:16Z

libraries/AC_AutoTune/AC_AutoTune.h

-        SP_UP = 4,                // angle P is being tuned up
-        SP_DOWN = 5,              // angle P is being tuned down
+        SP_DOWN = 4,              // angle P is being tuned down
+        SP_UP = 5,                // angle P is being tuned up


@lthall You shouldn't change the numbering here if you want to change the sequence of the tuning. You change that in the Set_Tune_Sequence method in AC_AutoTune_Multi.h

I changed it here so it matches the ordering. This is really just to make it easier to read and review the logs.

bnsgeyer · 2024-02-20T03:47:12Z

@lthall have you ever thought about using dynamic scaling to scale the target response. Not sure if you do this for larger vehicles. I am considering this for heli autotune as it doesn't work well with large vehicles as the requested rates are too much for larger vehicles. Dynamic scaling, like Froude scaling, would allow the user to set a characteristic length and then the code would scale the requested responses. Just thought this might be a consideration for future PRs for autotune. It is on my todo list.

lthall · 2024-02-20T04:36:27Z

@lthall have you ever thought about using dynamic scaling to scale the target response

Yes. My plan is to calculate the scale from the last X twitches. Each twitch I can measure the acceleration and rate. I hope that by using this to calculate the ideal twitch command I can get a good estimate for the next twitch for those parameters.

I would like to avoid the user having to tell me information to get the scale if I can.

bnsgeyer · 2024-02-23T15:16:02Z

libraries/AC_AutoTune/AC_AutoTune.cpp

@@ -391,14 +389,20 @@ void AC_AutoTune::control_attitude()
        test_run(axis, direction_sign);

        // Check for failure causing reverse response
-        if (lean_angle <= -AUTOTUNE_TARGET_MIN_ANGLE_RLLPIT_CD) {
+        if (lean_angle <= -attitude_control->lean_angle_max_cd() * AUTOTUNE_TARGET_MIN_ANGLE_RLLPIT_SCALE) {


@lthall you should consider moving these protections to a virtual function so that multi and heli can implement their own protections. At a minimum, all of these new defines should be have multi and heli versions.

At the moment these are all very close to the original value for the default lean angle max of 45 degrees. I hoped this would be close enough to no change to not impact Heli.

I am not sure how much value there is in me making a copy of hash defines for heli that are identical to Multi. Would you be willing to add a commit or follow up PR to address any Heli interface issues?

The problem is that 45 deg is no longer the angle max default. So this could have a big impact.

rmackay9 · 2024-05-27T23:27:40Z

libraries/AC_AutoTune/AC_AutoTune_Multi.cpp

@@ -873,6 +896,8 @@ void AC_AutoTune_Multi::updating_rate_d_down(float &tune_d, float tune_d_min, fl
                tune_d = tune_d_min;
                counter = AUTOTUNE_SUCCESS_COUNT;
                LOGGER_WRITE_EVENT(LogEvent::AUTOTUNE_REACHED_LIMIT);
+                // This may be mean AGGR should be increased or MIN_D decreased
+                gcs().send_text(MAV_SEVERITY_INFO, "AutoTune: Min Rate D limit reached");


we should be using GCS_SEND_TEXT() these days.

rmackay9 · 2024-05-27T23:34:54Z

libraries/AC_AutoTune/AC_AutoTune_Heli.cpp

@@ -1724,6 +1732,36 @@ void AC_AutoTune_Heli::updating_max_gains(float *freq, float *gain, float *phase

 }

+float AC_AutoTune_Heli::target_angle_max_rp_cd() const


we could copy the comments from the .h to here in the .cpp. It's not a blocker of course.

tridge · 2024-05-27T23:50:43Z

failed autotune, QuadcopterX, realflight, 2kph wind, loiter:
http://uav.tridgell.net/tmp/00000034.BIN
this is pretty benign conditions. Maybe LOITER autotune no longer works?

tridge · 2024-05-27T23:52:36Z

i think if a later stage fails it shoule be able to save the earlier stage. For example, in above 34.BIN log it failed at yaw stage, it would have been nice to save the roll and pitch that completed

lthall · 2024-05-28T01:30:57Z

failed autotune, QuadcopterX, realflight, 2kph wind, loiter: http://uav.tridgell.net/tmp/00000034.BIN this is pretty benign conditions. Maybe LOITER autotune no longer works?

Yaw failed because the current tune is very close to oscillating.

lthall · 2024-05-28T01:37:53Z

i think if a later stage fails it shoule be able to save the earlier stage. For example, in above 34.BIN log it failed at yaw stage, it would have been nice to save the roll and pitch that completed

While I agree this would be nice, it is also potentially dangerous. Doing this would be a significant change and would need supporting code to ensure that the test on a switch functionality worked, messages were appropriate and wiki was updated.

This PR is the first simple change.

lthall requested review from bnsgeyer, rmackay9 and andyp1per February 17, 2024 02:21

lthall added Copter WIP DevCallTopic labels Feb 17, 2024

lthall force-pushed the 20240217_Autotune branch 2 times, most recently from 218ebb2 to 272f8f8 Compare February 17, 2024 10:45

amilcarlucas suggested changes Feb 17, 2024

View reviewed changes

IamPete1 reviewed Feb 17, 2024

View reviewed changes

libraries/AC_AutoTune/AC_AutoTune.cpp Outdated Show resolved Hide resolved

IamPete1 reviewed Feb 17, 2024

View reviewed changes

andyp1per reviewed Feb 18, 2024

View reviewed changes

libraries/AC_AutoTune/AC_AutoTune.h Outdated Show resolved Hide resolved

andyp1per reviewed Feb 18, 2024

View reviewed changes

libraries/AC_AutoTune/AC_AutoTune_Multi.cpp Outdated Show resolved Hide resolved

andyp1per reviewed Feb 18, 2024

View reviewed changes

libraries/AC_AutoTune/AC_AutoTune_Multi.cpp Outdated Show resolved Hide resolved

andyp1per reviewed Feb 18, 2024

View reviewed changes

libraries/AC_AutoTune/AC_AutoTune_Multi.cpp Outdated Show resolved Hide resolved

andyp1per reviewed Feb 18, 2024

View reviewed changes

peterbarker requested changes Feb 19, 2024

View reviewed changes

lthall force-pushed the 20240217_Autotune branch from 2aac0ea to 0157658 Compare February 19, 2024 03:48

Hwurzburg added the WikiNeeded needs wiki update label Feb 19, 2024

bnsgeyer reviewed Feb 20, 2024

View reviewed changes

CraigElder removed the DevCallTopic label Feb 20, 2024

lthall force-pushed the 20240217_Autotune branch 2 times, most recently from ba0a4ce to 14bef1a Compare February 23, 2024 14:26

bnsgeyer reviewed Feb 23, 2024

View reviewed changes

lthall added the DevCallTopic label Feb 26, 2024

CraigElder removed the DevCallTopic label Feb 26, 2024

lthall added 17 commits May 28, 2024 08:43

Copter: Autotune: Reduce chance of desync

ec5d557

Copter: Autotune: Reduce default AGGR

9f9d0c2

Copter: Autotune: Increase test timeout

49df96d

Copter: Autotune: Set D = 0 for Yaw test

9819867

Copter: Autotune: Look for bounce earlier in test

f0dc9fe

Copter: Autotune: Fail when bad tune is expected

106890f

Copter: Autotune: Base angles limits on lean_angle_max

98028a5

Copter: Autotune: Twitch direction enhancements

73bc732

Copter: Autotune: Notify user of min Rate D for action

ddd86b7

Copter: Autotune: Level requirement improvements

04f612c

Copter: Autotune: Small changes for clarity

a5eae54

Copter: Autotune: Improve angle limit test

4a3c29c

Copter: Autotune: Reduce MIN_D

c17875e

Copter: Autotune: Don't Fail min D for Yaw Filter

1fe5c5f

Copter: Autotune: Virtual methods to separate Heli and Multi

6ea273e

Copter: Autotune: Increase waiting for level timeout.

b304e0d

Copter: Autotune: Remove double call to AC_Attitude_Control

3801f9c

lthall force-pushed the 20240217_Autotune branch from 3bdcc26 to 3801f9c Compare May 27, 2024 23:14

CraigElder removed the DevCallTopic label May 27, 2024

rmackay9 reviewed May 27, 2024

View reviewed changes

rmackay9 approved these changes May 27, 2024

View reviewed changes

rmackay9 reviewed May 27, 2024

View reviewed changes

AC_AutoTune: Move functions out of HAL_LOGGING_ENABLED

e078291

peterbarker added MergeOnCIPass and removed WIP labels May 28, 2024

peterbarker merged commit 0accc61 into ArduPilot:master May 29, 2024
91 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Copter: Autotune performance and safety enhancements #26245

Copter: Autotune performance and safety enhancements #26245

lthall commented Feb 17, 2024 •

edited

Loading

IamPete1 left a comment

lthall commented Feb 17, 2024 •

edited

Loading

andyp1per commented Feb 18, 2024

andyp1per left a comment

peterbarker left a comment

peterbarker Feb 19, 2024

lthall Feb 19, 2024

peterbarker Feb 20, 2024 •

edited

Loading

bnsgeyer Feb 20, 2024

peterbarker Feb 19, 2024

bnsgeyer Feb 20, 2024

lthall Feb 20, 2024

bnsgeyer commented Feb 20, 2024

lthall commented Feb 20, 2024

bnsgeyer Feb 23, 2024

lthall Feb 23, 2024

bnsgeyer Feb 26, 2024

rmackay9 May 27, 2024

rmackay9 May 27, 2024

tridge commented May 27, 2024 •

edited

Loading

tridge commented May 27, 2024

lthall commented May 28, 2024 •

edited

Loading

lthall commented May 28, 2024

	if (lean_angle <= -attitude_control->lean_angle_max_cd() * AUTOTUNE_TARGET_MIN_ANGLE_RLLPIT) {
	if (lean_angle <= -lean_angle_max_cd()) {

		@@ -1724,6 +1732,36 @@ void AC_AutoTune_Heli::updating_max_gains(float freq, float gain, float *phase

		}

		float AC_AutoTune_Heli::target_angle_max_rp_cd() const

Copter: Autotune performance and safety enhancements #26245

Copter: Autotune performance and safety enhancements #26245

Conversation

lthall commented Feb 17, 2024 • edited Loading

IamPete1 left a comment

Choose a reason for hiding this comment

lthall commented Feb 17, 2024 • edited Loading

andyp1per commented Feb 18, 2024

andyp1per left a comment

Choose a reason for hiding this comment

peterbarker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterbarker Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnsgeyer commented Feb 20, 2024

lthall commented Feb 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tridge commented May 27, 2024 • edited Loading

tridge commented May 27, 2024

lthall commented May 28, 2024 • edited Loading

lthall commented May 28, 2024

lthall commented Feb 17, 2024 •

edited

Loading

lthall commented Feb 17, 2024 •

edited

Loading

peterbarker Feb 20, 2024 •

edited

Loading

tridge commented May 27, 2024 •

edited

Loading

lthall commented May 28, 2024 •

edited

Loading