Tradheli: fix yaw attitude error corrections while rotors turning on the ground #28690

bnsgeyer · 2024-11-20T17:06:00Z

This PR addresses an issue with the handling of yaw attitude error corrections being fed to the rate controller when the aircraft is on the ground with rotors turning. Currently the yaw attitude error correction is being fed to the rate controller with both integrator management schemes. This PR keeps the heading error correction when using the leaky integrator but removes it when using the integrator management that triggers on the landing detector.

This also cleans up some inconsistencies in the logic with landed and pretakeoff althold states in each of the non-manual throttle flight modes.

It also addresses the definition of behaviors for Spool up/down which was not addressed at all in the manual throttle modes. The only one that affects copter is Drift

Needs testing.

MattKear · 2024-11-22T16:24:39Z

@bnsgeyer why not apply this to the leaky I case as well? I appreciate it is arguably not needed, but there is also no harm in applying it, as far as I can tell. By not applying it would reduce the amount of checks in the if statements and make the code easier to read.

bnsgeyer · 2024-11-23T00:27:33Z

@MattKear Leaky I allows the I term to grow as soon as motor interlock is enabled. This PR assumes no change to the leaky I integrator management case. Thus the heading error corrections are allowed with motor interlock enabled if the user is using leaky I.

Are you referring to the added logic in the Drift mode? I realized as I reviewed the current code that drift mode did not conform to how heli stabilize and heli acro handled the I term. There were some other inconsistencies that I fixed as well.

Ferruccio1984 · 2024-11-24T23:04:10Z

Hi @bnsgeyer code looks good to me, will potentially solve the behavior reported for the tandem heli setup. There is a little window of non-safety though for traditional tail rotor helis when taking off/landing on concrete helipads. In the video attached I poked a little the "light on the skids" condition: the situation can potentially evolve into dynamic roll-over if the pilot keep the collective up but not enough to take-off. Could be problematic also for auto takeoff and landing. I would think if adjusting this implementation into a bitmask option would be a win-win for both trad-helis and tandems?
https://www.youtube.com/watch?v=Y5TvBSDiLH8

…adheli

bnsgeyer · 2024-11-25T23:42:54Z

@Ferruccio1984 I have added a option to H_OPTIONS to turn off/on heading error correction on the ground

bnsgeyer · 2024-11-27T02:56:02Z

After working through the details of the changes, here are the desired behaviors for the yaw axis.

The goal is to allow helicopters sensitive to heading corrections on the ground to operate without these corrections. The heading error correction by the attitude controller is responsible for the heading corrections on the ground.

This change was only intended for the non-leaky I integrator scheme and leaky I only affected manual collective modes. There should be no change when using the leaky i integrator scheme.

The H_OPTIONS Bitmask was used to allow users to choose between attitude controller using heading error correction or not.

When using heading error correction, behaviors should not have changed in manual and non-manual collective modes.
landed, motor interlock disabled (althold state: Landed_Ground_Idle) - targets heading reset and zero I term
this is required to keep stick inputs from moving target heading
zeroing I term was added for non-manual collective modes - this was missed when spool logic was added
landed, motor interlock enabled (althold state: Landed_Pre_Takeoff) - target heading released but still zero I term

When NOT using heading error correction, only yaw rate feedback is used on the ground. This serves two purposes. 1) to provide pilot control of yaw axis and 2) allowing autopilot to help fight gusts.
landed, motor interlock disabled (althold state: Landed_Ground_Idle) - targets heading reset and zero I term
this is required to keep stick inputs from moving target heading
landed, motor interlock enabled (althold state: Landed_Pre_Takeoff) - target heading reset and zero I term

MattKear

Generally speaking, this makes sense, as we do need to improve the heading target handling on the ground.

I do not understand why we need the separation of using leaky I or not in this though. In my mind, regardless of whether leaky I is enabled, we do not want to allow heading or yaw rate errors to build when we are on the ground.

MattKear · 2024-12-02T17:33:53Z

libraries/AP_Motors/AP_Motors_Class.h

+    // use leaking integrator management scheme is always false for multirotors
+    virtual bool using_leaky_integrator() const { return false; }
+    // use heading error correction which is always true for multirotors
+    virtual bool using_hdg_error_correction() const { return true; }


I do not think we should not be making these virtuals in AP_Motors_Class. We are taking something that is a heli-only option and making visible in the multi-copter code. Instead we should keep it in AP_MotorsHeli and use defines for the option, as we do else where in the code.

We have been trying to remove the hash defines in the shared code and instead make it flexible enough to cover all the hovering aircraft.

The aspirational goal is to not have any Heli specific code in the shared code base and instead have optional behaviour that is defined by the vehicle.

MattKear · 2024-12-02T17:37:42Z

libraries/AP_Motors/AP_MotorsHeli.h

@@ -138,11 +138,15 @@ class AP_MotorsHeli : public AP_Motors {

    // enum for heli optional features
    enum class HeliOption {
-        USE_LEAKY_I                     = (1<<0),   // 1
+        USE_LEAKY_I                     = (1<<0),
+        USE_HDG_CORRECTION              = (1<<1),


I do not want to feature creep this PR, so I am just saying this as a recommendation for future work. I think it is weird that we ask AP_Motors whether we should be using leaky I and now this heading correction. I understand that it is a hang over because most of the heli specific code lives in motors, but in the future I would like to move the heli option param out of motors and up to the vehicle(copter) level. This adds to my point below as to why I would rather we didn't make it a virtual function.

MattKear · 2024-12-02T17:45:08Z

ArduCopter/mode_poshold.cpp

        attitude_control->reset_rate_controller_I_terms_smoothly();
+        init_wind_comp_estimate();


This is unrelated to this PR. Did you mean to add this?

@MattKear maybe I was wrong here. I thought that it was needed because I removed the fallthrough. I will remove it unless told otherwise.

fixed. Thanks.

MattKear · 2024-12-02T17:45:14Z

ArduCopter/mode_poshold.cpp

        pos_control->relax_z_controller(0.0f);   // forces throttle output to decay to zero
+        loiter_nav->clear_pilot_desired_acceleration();
+        loiter_nav->init_target();


This is unrelated to this PR. Did you mean to add this?

@MattKear @lthall This does not follow the same flow as Loiter. I changed it because loiter is done this way. Not sure why they would be different. Again I can change it back if you think it should not change. Or I can just do a separate PR.

fixed. Thanks,

MattKear · 2024-12-02T17:58:48Z

libraries/AP_Motors/AP_MotorsHeli.h

+    bool using_leaky_integrator() const override { return heli_option(HeliOption::USE_LEAKY_I); }
+
+    // use heading error correction
+    bool using_hdg_error_correction() const override { return heli_option(HeliOption::USE_HDG_CORRECTION); }


Do we need this as an option? What is the use case that users will be trying to drive yaw whilst on the ground?

MattKear · 2024-12-02T18:02:50Z

ArduCopter/mode_loiter.cpp

@@ -125,17 +125,21 @@ void ModeLoiter::run()

    case AltHoldModeState::MotorStopped:
        attitude_control->reset_rate_controller_I_terms();
-        attitude_control->reset_yaw_target_and_rate();
+        attitude_control->reset_yaw_target_and_rate(true);


this is not a change in behaviour, default value is true.

Ok. I wasn't sure that I needed to do this. I saw that it was done in one place so I thought I needed to do it in all places. I can remove it.

fixed. Thanks.

MattKear · 2024-12-02T18:06:45Z

ArduCopter/mode_loiter.cpp

-        FALLTHROUGH;
+        attitude_control->reset_yaw_target_and_rate(false);
+        attitude_control->reset_rate_controller_I_terms_smoothly();
+        break;


This is a fairly significant change in behaviour by removing the fall through. We would at least want to keep the loiter_nav->init_target()?

Yep, we need to add the loiter_nav->init_target();

I think we can move this to the bottom:

attitude_control->input_thrust_vector_rate_heading(loiter_nav->get_thrust_vector(), target_yaw_rate, false);

@lthall I think I need to move all of the lines in the pretakeoff state up to the landed ground idle since I removed the fall through.

MattKear · 2024-12-02T18:07:28Z

ArduCopter/mode_loiter.cpp

        pos_control->relax_z_controller(0.0f);   // forces throttle output to decay to zero
        loiter_nav->init_target();
        attitude_control->input_thrust_vector_rate_heading(loiter_nav->get_thrust_vector(), target_yaw_rate, false);
        break;

    case AltHoldModeState::Landed_Ground_Idle:
-        attitude_control->reset_yaw_target_and_rate();
-        FALLTHROUGH;
+        attitude_control->reset_yaw_target_and_rate(false);


Why wouldn't we want to reset_rate on landed ground idle?

This provides some damping to the tail if it gets pushed by a gust or something but will stay close to zero as long as we keep the integrator zero.

lthall

Looks like it is close!

lthall · 2024-12-02T21:08:30Z

ArduCopter/mode_acro_heli.cpp

@@ -45,26 +45,29 @@ void ModeAcro_Heli::run()
    switch (motors->get_spool_state()) {
    case AP_Motors::SpoolState::SHUT_DOWN:
        // Motors Stopped
-        attitude_control->reset_target_and_rate(false);


Changing this from false to true here will remove all counter talk on the tail rotor. If you can be still spinning down in this phase of flight this will let the tail move more than it would do now.

Low risk but something to consider.

@lthall What do you mean by counter talk? I think the issue here is that when disarmed, there won't be any target reset that would cause an issue with trying to fly disarmed. I know this sounds crazy but I believe that this was intentional by my predecessor to allow users to fly in acro if the aircraft disarms unexpectedly, like due to code bug. I think that this should true so that the attitude errors are reset. However I may make this change in a separate PR to make this more visible as an intentional change.

@Ferruccio1984 What do you think of having the attitude error correction (pitch and roll) removed for the disarmed state in acro. This means that when you move the pitch and roll stick that the swashplate will not react while disarmed which is what you probably saw in your flight today. I think it is a good idea because it would keep users from moving the swashplate while disarmed and then being surprised on spool up if the swash was not centered. I think the idea of ever having to fly in a disarmed state is very low risk.

lthall · 2024-12-02T21:16:12Z

ArduCopter/mode_loiter.cpp

        pos_control->relax_z_controller(0.0f);   // forces throttle output to decay to zero
        loiter_nav->init_target();
        attitude_control->input_thrust_vector_rate_heading(loiter_nav->get_thrust_vector(), target_yaw_rate, false);
        break;

    case AltHoldModeState::Landed_Ground_Idle:
-        attitude_control->reset_yaw_target_and_rate();
-        FALLTHROUGH;
+        attitude_control->reset_yaw_target_and_rate(false);


This provides some damping to the tail if it gets pushed by a gust or something but will stay close to zero as long as we keep the integrator zero.

lthall · 2024-12-02T21:18:42Z

ArduCopter/mode_loiter.cpp

-        FALLTHROUGH;
+        attitude_control->reset_yaw_target_and_rate(false);
+        attitude_control->reset_rate_controller_I_terms_smoothly();
+        break;


Yep, we need to add the loiter_nav->init_target();

I think we can move this to the bottom:

attitude_control->input_thrust_vector_rate_heading(loiter_nav->get_thrust_vector(), target_yaw_rate, false);

lthall · 2024-12-02T21:23:34Z

libraries/AP_Motors/AP_Motors_Class.h

+    // use leaking integrator management scheme is always false for multirotors
+    virtual bool using_leaky_integrator() const { return false; }
+    // use heading error correction which is always true for multirotors
+    virtual bool using_hdg_error_correction() const { return true; }


We have been trying to remove the hash defines in the shared code and instead make it flexible enough to cover all the hovering aircraft.

The aspirational goal is to not have any Heli specific code in the shared code base and instead have optional behaviour that is defined by the vehicle.

bnsgeyer added Copter TradHeli NeedsTesting labels Nov 20, 2024

bnsgeyer requested review from lthall, rmackay9 and Ferruccio1984 November 20, 2024 17:06

bnsgeyer self-assigned this Nov 20, 2024

bnsgeyer force-pushed the pr-yaw-target-reset-fix branch from b80d244 to c14d498 Compare November 22, 2024 04:09

bnsgeyer added 3 commits November 22, 2024 08:31

Copter: change heading error correction on ground for tradheli

cf918cb

AP_Motors: change heli rotors turning on ground yaw behavior

3013e82

Copter: fix integrator and yaw target handling during spool up/down

edc153d

bnsgeyer force-pushed the pr-yaw-target-reset-fix branch from c14d498 to edc153d Compare November 22, 2024 13:32

bnsgeyer requested a review from MattKear November 22, 2024 16:06

bnsgeyer added 2 commits November 25, 2024 18:32

Copter: Make use of heading correction on ground selectable for tradheli

a4ef66c

AP_Motors: Make use of heading correction on ground selectable for tr…

3c478c9

…adheli

bnsgeyer force-pushed the pr-yaw-target-reset-fix branch from c05659a to 3c478c9 Compare November 25, 2024 23:33

bnsgeyer added 3 commits November 25, 2024 21:30

Copter: Fix logic for all modes

3f8ecf3

AP_Motors: minor edit

e65b5a5

Copter: fix tradheli acro copy-paste error

d42b201

MattKear suggested changes Dec 2, 2024

View reviewed changes

lthall reviewed Dec 2, 2024

View reviewed changes

bnsgeyer added 3 commits December 2, 2024 23:14

Copter: fix modes where fallthrough was removed to keep behavior

40a2c31

Copter: remove unnecessary true statements

13efb77

Copter: correct statements in poshold

fb386e2

bnsgeyer requested a review from MattKear December 3, 2024 19:07

bnsgeyer requested a review from lthall December 3, 2024 19:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tradheli: fix yaw attitude error corrections while rotors turning on the ground #28690

Tradheli: fix yaw attitude error corrections while rotors turning on the ground #28690

bnsgeyer commented Nov 20, 2024

MattKear commented Nov 22, 2024

bnsgeyer commented Nov 23, 2024

Ferruccio1984 commented Nov 24, 2024

bnsgeyer commented Nov 25, 2024

bnsgeyer commented Nov 27, 2024

MattKear left a comment

MattKear Dec 2, 2024

lthall Dec 2, 2024

MattKear Dec 2, 2024

MattKear Dec 2, 2024

bnsgeyer Dec 3, 2024

bnsgeyer Dec 3, 2024

MattKear Dec 2, 2024

bnsgeyer Dec 3, 2024

bnsgeyer Dec 3, 2024

MattKear Dec 2, 2024

MattKear Dec 2, 2024

bnsgeyer Dec 3, 2024

bnsgeyer Dec 3, 2024

MattKear Dec 2, 2024

lthall Dec 2, 2024

bnsgeyer Dec 3, 2024

MattKear Dec 2, 2024

lthall Dec 2, 2024

lthall left a comment

lthall Dec 2, 2024

bnsgeyer Dec 3, 2024

bnsgeyer Dec 3, 2024

lthall Dec 2, 2024

lthall Dec 2, 2024

lthall Dec 2, 2024

		attitude_control->reset_rate_controller_I_terms_smoothly();
		init_wind_comp_estimate();

Tradheli: fix yaw attitude error corrections while rotors turning on the ground #28690

Are you sure you want to change the base?

Tradheli: fix yaw attitude error corrections while rotors turning on the ground #28690

Conversation

bnsgeyer commented Nov 20, 2024

MattKear commented Nov 22, 2024

bnsgeyer commented Nov 23, 2024

Ferruccio1984 commented Nov 24, 2024

bnsgeyer commented Nov 25, 2024

bnsgeyer commented Nov 27, 2024

MattKear left a 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

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

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

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

lthall left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment