Simplify Autorotation Mode Switching and Consolidate Autorotation State in RSC #27786

MattKear · 2024-08-07T15:48:49Z

This PR does the following:

Removes the linking of autorotation flight mode and the interlock.
Consolidates the autorotation state into its own class within the RSC.
Moves the bailout logic out of the autorotation mode and into RSC making it common to both the mode and manual autorotations.

Reasons for this:

The linking of the flight mode switching and interlock was creating lots of edge cases that were proving to difficult to prevent. This is simpler and just like any other flight mode (this is still SITL only).
Removing this link between interlock and flight mode means, in the future, we can add partial power reduction in assisted autorotations via the flight mode which will make initial setup and testing more gradual and less intimidating.
All bailout out handling for the manual autorotation has been tidied up and consolidated into the RSC. The autorotation flight mode will be changed to use this bailout logic in the RSC to remove duplication.

This PR is WIP. I still have the following to address:

Get bailouts working in autorotation flight mode.
Fixup the autotests to accept this new way of entering the autorotation flight mode.
Add autorotation flight mode exit handling e.g. don't allow exiting the flight mode to non-manual throttle modes unless the interlock is engaged and the RSC has flagged the bailout as complete.

IamPete1

just a quick look.

libraries/AC_Autorotation/AC_Autorotation.cpp

libraries/AP_Motors/AP_MotorsHeli_RSC.cpp

MattKear · 2024-09-03T21:11:37Z

Added to DevCallEU for a quick look over/general feedback. Not looking to get this merged today.

ArduCopter/heli.cpp

MattKear · 2024-09-24T00:56:09Z

I am pretty happy with this PR now.

It has been tested IRL now. For complete transparency, this code was actually tested with the code in #28209 on top.

I do like this change to de-couple the flight mode from the interlock. It is far less daunting to test the flight mode for the first time without removing the power with the interlock. This makes for a much nicer user setup and test work flow.

The key bit that we need to ensure is working well, is the autorotation state inside the RSC as that code will be available to users to download on 4.6-dev once this is merged. This plot illustrates the RSC autorotation state transitioning from "active" to "bailing out" to "stopped" During test the vehicle bailed out from the glide smoothly.

The setup that I tested in real life is using a governor in the ESC, and the autorotation signal window was used to invoke a rapid spool up.

I have tested this code with Heli's head speed governor, a lot in Real Flight. It does work, however, I will say that Heli's governor does not play that nice with autorotations. However, this is an issue with way that the governor is currently written. The over-speed/under-speed faults are significantly more likely to be tripped when recovering from an autorotation and the governor just gives up at that point. This is an existing issue, that is not a problem with this code being added. I am simply raising this for awareness now, and I will address the issue with the governor in a future PR.

Also for complete transparency, since testing this IRL I have done some restructuring to move the H_RSC_AROT params to the RSC_Autorotation object. I did not change the logic of the mode, and have re-tested in the sim. It was just something I noticed in testing, that we should have the RSC_AROT params hidden if not enabled.

Param conversions have been added for moving the parameters. This has also been tested.

bnsgeyer · 2024-09-30T02:39:38Z

@MattKear I agree with the premise of this PR. It seems a little odd having to switch over to autorotate mode and then disable interlock to initiate the autorotation. I have tested this in Real Flight sim. I plan to look at the follow on PR this week.

bnsgeyer

@MattKear I looked through this a little more thoroughly. It is a pretty extensive change, for sure. A little surprised to see the new RSC_autorotation library but it looks like it simplified a bunch in RSC and the motors library.

Did you check the cooldown feature to ensure it still works as designed. I know there is a test for the turbine start but I'm not sure if there is one for the cooldown timer.

I am happy with this and approved but want to ensure the cooldown timer still works.

MattKear · 2024-10-01T05:54:03Z

Thanks for taking a look at this.

Yeah, the rework was for a few reasons:

To simplify the autorotation state within the RSC
To handle the bailout for more RSC configs (we previously didn't handle external governors with bailout throttle very well) which is why.
To rename the params so that they appear to users to be more broadly applicable to the autorotation in general not just manual autorotation. This has meant that the duplication of bailout handling in the flight mode and the RSC has been now been removed.

MattKear · 2024-10-01T05:54:50Z

No, I have not tested the turbine cool down functionality. What should I look for to see that it still works?

bnsgeyer · 2024-10-01T13:35:13Z

@MattKear checking it is pretty easy. It is used for ICE or turbine engines. You will need to set a non zero H_RSC_IDLE. It will raise that by 50% for the cooldown time specified in H_RSC_CLDWN in seconds after motor interlock is disabled (RSC State is Idle). You should also verify that it doesn’t use the cooldown timer feature if the aircraft is in autorotation.

MattKear · 2024-10-01T13:51:18Z

@bnsgeyer ok, thanks Bill. I will test and report back.

libraries/AP_Motors/AP_MotorsHeli_RSC.cpp

libraries/AP_Motors/AP_MotorsHeli.cpp

MattKear · 2024-10-08T10:13:26Z

@bnsgeyer I have rebased this now that PR #28310 has gone in.

I have added more cases to the autotest ManAutorotation() to check all of the conditions that you presented on Discord. I have also re-tested in RealFlight to make sure this patch still flys well.

This is what the ManAutorotation test looks like:

Test 1
H_RSC_IDLE = 5, H_RSC_AROT_RAMP = 2, H_RSC_AROT_IDLE = 0, H_RSC_CLDWN_TIME = 0

Test 2
H_RSC_IDLE = 0, H_RSC_AROT_RAMP = 0, H_RSC_AROT_IDLE = 20, H_RSC_CLDWN_TIME = 0

Test 3
H_RSC_IDLE = 5, H_RSC_AROT_RAMP = 2, H_RSC_AROT_IDLE = 0, H_RSC_CLDWN_TIME = 5

Test 4
H_RSC_IDLE = 5, H_RSC_AROT_RAMP = 2, H_RSC_AROT_IDLE = 10, H_RSC_CLDWN_TIME = 5

MattKear · 2024-10-08T10:17:54Z

@peterbarker I have also done the modifications that you recommended in PR #28310 to the turbine cooldown autotest. This is kept as a separate commit.

bnsgeyer

LGTM

…within RSC

…otation state

…ethods

peterbarker · 2024-10-10T22:27:42Z

Tools/autotest/helicopter.py

+        self.progress("testing autorotation with cool down enabled and zero autorotation idle")
+        TestAutorotationConfig(self, rsc_idle=5.0, arot_ramp_time=2.0, arot_idle=0, cool_down=5.0)
+
+        self.progress("testing that H_RSC_AROT_IDLE is used over RSC_IDLE when cool down is enabled")


There's also start_subtest which adds a bit more delineation in the logs

MattKear requested review from bnsgeyer and IamPete1 August 7, 2024 15:48

MattKear added TradHeli WIP labels Aug 7, 2024

IamPete1 reviewed Aug 7, 2024

View reviewed changes

libraries/AC_Autorotation/AC_Autorotation.cpp Show resolved Hide resolved

libraries/AP_Motors/AP_MotorsHeli_RSC.cpp Show resolved Hide resolved

MattKear force-pushed the pr_heli_autorotation_mode_switching branch from 2f74720 to fd536e5 Compare August 10, 2024 18:39

MattKear force-pushed the pr_heli_autorotation_mode_switching branch 3 times, most recently from 4201b9a to 395f524 Compare September 3, 2024 21:05

MattKear added DevCallEU NeedsTesting and removed WIP labels Sep 3, 2024

tridge removed the DevCallEU label Sep 4, 2024

MattKear force-pushed the pr_heli_autorotation_mode_switching branch 5 times, most recently from cf7e8c5 to dda9046 Compare September 9, 2024 16:19

MattKear added the DevCallEU label Sep 11, 2024

peterbarker requested changes Sep 11, 2024

View reviewed changes

ArduCopter/heli.cpp Outdated Show resolved Hide resolved

ArduCopter/heli.cpp Outdated Show resolved Hide resolved

MattKear removed the DevCallEU label Sep 11, 2024

MattKear mentioned this pull request Sep 19, 2024

Blimp: Remove previous flight mode #28078

Merged

MattKear force-pushed the pr_heli_autorotation_mode_switching branch 2 times, most recently from 6e22ddc to e5a8278 Compare September 23, 2024 22:21

MattKear mentioned this pull request Sep 24, 2024

Heli autorotation restructure mode and improve speed controller #28209

Open

MattKear removed the NeedsTesting label Sep 24, 2024

bnsgeyer approved these changes Sep 30, 2024

View reviewed changes

bnsgeyer reviewed Oct 1, 2024

View reviewed changes

libraries/AP_Motors/AP_MotorsHeli_RSC.cpp Outdated Show resolved Hide resolved

bnsgeyer reviewed Oct 3, 2024

View reviewed changes

libraries/AP_Motors/AP_MotorsHeli.cpp Outdated Show resolved Hide resolved

MattKear force-pushed the pr_heli_autorotation_mode_switching branch from a3bd214 to a0be450 Compare October 8, 2024 10:06

MattKear requested a review from peterbarker October 8, 2024 10:15

MattKear force-pushed the pr_heli_autorotation_mode_switching branch 2 times, most recently from 1ab5653 to 1daafdd Compare October 8, 2024 11:44

bnsgeyer approved these changes Oct 9, 2024

View reviewed changes

bnsgeyer added the MergeOnCIPass label Oct 9, 2024

MattKear added 8 commits October 9, 2024 20:07

AP_MotorsHeli: Consolidate all autorotation state into its own class …

67932d9

…within RSC

Copter: Heli: simplify autorotation mode change and support RSC autor…

6fcc6cd

…otation state

AC_Autorotation: Remove bailout case

d07cbaf

Sub: correct comment on prev control mode

91d2a47

Autotest: Add method for check servo channel in range

833c4a5

Autotest: Heli: minor improvements to TurbineCoolDown

e25d78e

Autotest: Update Autorotation tests for new mode change and bailout m…

a18f3ab

…ethods

AC_Autorotation: Add RSC_Autorotation class

039ca25

bnsgeyer force-pushed the pr_heli_autorotation_mode_switching branch from 1daafdd to 039ca25 Compare October 10, 2024 00:07

peterbarker reviewed Oct 10, 2024

View reviewed changes

peterbarker merged commit 20449e3 into ArduPilot:master Oct 10, 2024
98 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify Autorotation Mode Switching and Consolidate Autorotation State in RSC #27786

Simplify Autorotation Mode Switching and Consolidate Autorotation State in RSC #27786

MattKear commented Aug 7, 2024 •

edited

Loading

IamPete1 left a comment

MattKear commented Sep 3, 2024

MattKear commented Sep 24, 2024 •

edited

Loading

bnsgeyer commented Sep 30, 2024

bnsgeyer left a comment

MattKear commented Oct 1, 2024

MattKear commented Oct 1, 2024

bnsgeyer commented Oct 1, 2024

MattKear commented Oct 1, 2024

MattKear commented Oct 8, 2024 •

edited

Loading

MattKear commented Oct 8, 2024

bnsgeyer left a comment

peterbarker Oct 10, 2024

Simplify Autorotation Mode Switching and Consolidate Autorotation State in RSC #27786

Simplify Autorotation Mode Switching and Consolidate Autorotation State in RSC #27786

Conversation

MattKear commented Aug 7, 2024 • edited Loading

IamPete1 left a comment

Choose a reason for hiding this comment

MattKear commented Sep 3, 2024

MattKear commented Sep 24, 2024 • edited Loading

bnsgeyer commented Sep 30, 2024

bnsgeyer left a comment

Choose a reason for hiding this comment

MattKear commented Oct 1, 2024

MattKear commented Oct 1, 2024

bnsgeyer commented Oct 1, 2024

MattKear commented Oct 1, 2024

MattKear commented Oct 8, 2024 • edited Loading

MattKear commented Oct 8, 2024

bnsgeyer left a comment

Choose a reason for hiding this comment

peterbarker Oct 10, 2024

Choose a reason for hiding this comment

MattKear commented Aug 7, 2024 •

edited

Loading

MattKear commented Sep 24, 2024 •

edited

Loading

MattKear commented Oct 8, 2024 •

edited

Loading