Tradheli-improvement to internal rotor speed governor #16534
Conversation
There was a problem hiding this comment.
@bnsgeyer I noted the param macro index conflict with the SITL autorotation param. Otherwise the code looks intact after the rebase. Since the autothrottle logic is quite complicated I think it would be a good idea for me to build this and run it in a real helicopter before the PR is merged.
Thanks!
|
@bnsgeyer I ran this early this morning in one of my piston engine helicopters. As you are aware, the governor portion itself is not new, we have flown that for over a year. The main concern was the autothrottle sequencing, fault lockout, and integration with the spool logic in 4.1-dev. That functions perfectly after the rebase. So from a safety and functionality standpoint, this is good to go on my end for 4.1-dev. Works as I designed it to. |
|
The changes are all tradheli specific so I have no real concerns about this going in. Thanks to @Gone4Dirt for all the review comments as well. |
|
@bnsgeyer I made some changes that addresses most of the comments here.
This set of changes is in my branch. If it looks good to you I'll push it to the PR branch Is it really necessary to have the defaults for the throttle curve as a #define in the header? Those could be moved to the AP_GROUPINFO in the cpp, as well, since they are never changed anyway. Also, since this also intended as a general cleanup and overhaul of the RSC, function declarations are usually self-explanatory and re-commented in the C file where they are used. Could get rid of more unnecessary comment lines in the header and group other things, like the autorotation functions() and maybe the functions for rotor speeds under one comment line. Comments are fine, but when it starts getting too many of them, one for each declaration, it gets messy. I prefer how it was done at the bottom of the header in the public access specifier for the parameter assessors, it's a lot cleaner. |
MidwestAire
force-pushed
the
pr-new-governor
branch
from
9cba692
to
b073dcb
Compare
bnsgeyer
force-pushed
the
pr-new-governor
branch
from
b073dcb
to
253da94
Compare
bnsgeyer
force-pushed
the
pr-new-governor
branch
from
4bd502d
to
8bd9276
Compare
bnsgeyer
force-pushed
the
pr-new-governor
branch
from
fb94127
to
a41fc47
Compare
|
@Gone4Dirt would like to get final approval from you for the changes I have implemented since your initial review. Most of it was addressing your comments. I did add a fix for a bug I found in the spool up logic of the governor. I also moved the cool down timer out of the governor to make it generic to all RSC Modes. Lastly I restored the governor range parameter. Chris had fixed this to -3/+2 %. I have had some feedback that in some cases this is too restrictive and causes the governor to fault. |
| // @Range: 0 40 | ||
| // @Param: GOV_TORQUE | ||
| // @DisplayName: Governor Torque Limiter | ||
| // @Description: Adjusts the engine's percentage of torque rise on autothrottle during ramp-up to governor speed. The torque rise will determine how fast the rotor speed will ramp up when rotor speed reaches 50% of the rotor RPM setting. The sequence of events engaging the governor is as follows: Throttle ramp time will engage the clutch and start the main rotor turning. The collective should be at feather pitch and the throttle curve set to provide at least 50% of normal RPM at feather pitch. The autothrottle torque limiter will automatically activate and start accelerating the main rotor. If the autothrottle consistently fails to accelerate the main rotor during ramp-in due to engine tune or other factors, then increase the torque limiter setting. NOTE: throttle ramp time and throttle curve should be tuned using MODE Throttle Curve before using MODE AutoThrottle |
There was a problem hiding this comment.
I am not sure what 'feather pitch' is. Is this common parlance in our community?
There was a problem hiding this comment.
I believe he is referring to flat pitch or zero blade pitch angle. Not entirely sure though
There was a problem hiding this comment.
@MidwestAire could you provide some clarification on what you mean by feather pitch please?
There was a problem hiding this comment.
He means flat pitch or the collective pitch that is used for ground run. I will use flat pitch in the description. Are happy with that phrase?
bnsgeyer
force-pushed
the
pr-new-governor
branch
from
4d483f0
to
1417894
Compare
move rpm out of case switch so it updates outside of autothrottle_run()
… with autothrottle
…onstrain_float - change lower limit of governor enaged output to 150% of idle speed - add governor_reset() function - remove governor defaults from header - rearrange governor metadata in alphabetical order, assign new eeprom slot for rpm
bnsgeyer
force-pushed
the
pr-new-governor
branch
from
1417894
to
3fefd82
Compare
This PR provides a significant enhancement to the internal rotor speed governor. It enables automatic spool up from ground idle to throttle unlimited. it also provides cool down period for internal combustion engines. Many thanks to Chris Olson (@MidwestAire) for his work on this and others who helped test it.