ardupilot icon indicating copy to clipboard operation
ardupilot copied to clipboard

APM_Control: add AP_FW_Controller as common base class to roll and pitch controlers

Open IamPete1 opened this issue 1 year ago • 3 comments

This moves to a shared base class between the plane roll and pitch controllers, there is lots of shared code which was duplicated. This helps with future tidyups as they now only need to happen in one place.

IamPete1 avatar Aug 04 '24 09:08 IamPete1

This is a example of something it would simplify. https://github.com/ArduPilot/ardupilot/pull/27694

IamPete1 avatar Aug 04 '24 10:08 IamPete1

I tried this on for size and got bounced: https://github.com/ArduPilot/ardupilot/pull/22023

"Needs a lot of testing" was not the only comment made.

From the meeting notes:

UTC0016 - [APM_Control: factor out an AP_PitchRollController base class by peterbarker · Pull Request #22023 · ArduPilot/ardupilot · GitHub](https://github.com/ArduPilot/ardupilot/pull/22023/files)

    Parameter conversion
    Check scale factor
    Pitch tracking in TECS

... and it was also stated that increasing complexity in this area was not wanted and burning those extra bytes was fine.

peterbarker avatar Aug 04 '24 23:08 peterbarker

This is awesome. It's a very nice structural change that should make some other things much easier in future.

timtuxworth avatar Sep 09 '24 23:09 timtuxworth

Rebased and added enum for autotune type so the autotune_start function can be in the base class. Still need to come-up with a test to prove correctness.

IamPete1 avatar Nov 08 '24 14:11 IamPete1

Thanks Peter, this would be great for a smoother management evolution of automatic controls in the future.

robustini avatar Jan 12 '25 15:01 robustini

I have added a test that runs a 60 second chirp through the roll and pitch controllers. This is then run in a test matrix with a range of roll/pitch/airspeed/flags. Total runs of 882 (441 for roll and 441 for pitch). The sweep is dumped to a csv with time, angle error, output, rate target, rate actual, rate error, P, I D, FF, DFF, DMod, slew rate, limit flag, PD limit flag, reset flag and I term set flag. The floating point values are printed to four decimal places. There is no change in these output files with the rework.

A plot of one run using the included python: image

image

image

image

image

IamPete1 avatar Jan 16 '25 00:01 IamPete1

Nice cut on H743!

Binary Name      Text [B]         Data [B]     BSS (B)      Total Flash Change [B] (%)      Flash Free After PR (B)
---------------  ---------------  -----------  -----------  ----------------------------  -------------------------
arducopter-heli  0 (0.0000%)      0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      120344
antennatracker   0 (0.0000%)      0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      548160
arducopter       0 (0.0000%)      0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      118840
ardurover        0 (0.0000%)      0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      [26](https://github.com/ArduPilot/ardupilot/actions/runs/12799510722/job/35685705920?pr=27747#step:10:27)4632
arduplane        -568 (-0.0363%)  0 (0.0000%)  0 (0.0000%)  -568 (-0.0362%)                                  136864
blimp            0 (0.0000%)      0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      5[27](https://github.com/ArduPilot/ardupilot/actions/runs/12799510722/job/35685705920?pr=27747#step:10:28)156
ardusub          0 (0.0000%)      0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      311404

Ryanf55 avatar Jan 20 '25 23:01 Ryanf55

  • inverted flight

Wider range of roll and pitch added to the test sweep. Roll from -170 to +180 in 10 get steps, pitch from -80 to + 80 in 10 deg steps. This brings the total number of test runs to 11016, still no change in output.

IamPete1 avatar Feb 01 '25 19:02 IamPete1

  • auto-tune

Added results checking to the existing autotest. Roll is very consistent, values within 2% every time. Pitch is not, there seems to be three solutions for P and two for D. There is no change as a result of this PR, but we might want to look into it...

IamPete1 avatar Feb 02 '25 14:02 IamPete1

I have added some more cases to the pitch result checker, I ran it 25 times localy and got this: image

No guarantees that it won't fail occasionally, but it does seem to be a genuine problem that we should try and understand, roll is the same every time.

IamPete1 avatar Feb 02 '25 14:02 IamPete1