-
Notifications
You must be signed in to change notification settings - Fork 17.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement assisted autorotation for Heli’s #22871
base: master
Are you sure you want to change the base?
Conversation
6d4ddd6
to
d87df0c
Compare
d87df0c
to
a67d3c9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
White space needs fixing throughout.
Guided stuff is a little odd taking roll only, what the use case?
Speeding up the rangefinder read probably has subtle unintended consequences, things like filters now having a different frequency due to the DT change.
@IamPete1 the idea behind the guided stuff is that the autorotational controller will control the vertical and pitch axes to control forward speed and descent rate. So the pilot or autopilot only needs to control the roll and yaw axes |
The guided stuff is adding support for setting roll via the |
Ok. Good point. |
Hi @bnsgeyer and @IamPete1 , the reason behind the guided stuff is BVLOS operation. In my use case most of the flight is performed in guided mode and the operator behind the Ground Station has a joystick for manual take-over ( which happens still in guided flight mode, joystick inputs are translated into attitude targets). Following this logic the operator is able to steer the helicopter also in case of autorotation, choosing the best landing spot through live Fpv feedback from the drone. The guided stuff is optional, so I coded it to allow traditional RC control or guided control. |
I think your probability better to drop the guided stuff from this PR and move it to a future one. As I say its a little odd and removing it make this PR more self contained in heli and auto-rotation code so will need less testing. I think the same thing is probably true of the rangefinder speed. I think you would at least need to do something to rate limit logs. Lots of the back ends do averaging, you may be able to get it to work just as well by providing a new handle to get the min distance or just the latest in the last loop. Removing the averageing will remove 25ms worth of lag. |
it is worth looking at the XKF5.HAGL value as a possible height above ground |
@Ferruccio1984 See @tridge suggestion above for a EKF signal that could provide a higher quality height estimate that is based off of the rangefinder and other sources including providing some reduction in lag that you will see with rangefinders. If it looks suitable then we can add it in and remove the code that changes the the scheduler rate. I will pull out the guided mode stuff and clean up the indentation problems. |
Hi @tridge , I had a look again at last summer test flights, comparing the rangefinder data ( scheduler rate of 100Hz) and the HAGL: |
@tridge based on the lag that we are seeing on the HAGL signal, I don’t think it is usable for this application. Please post a link to your branch that uses a complementary filter. Thanks! |
b9871c9
to
3870277
Compare
…ion, change all measurements to height above ground to be the same variable.
…der measurement for heigh above ground in the autorotation lib. We don't need to check for range finder out of range because inertial nave will interpolate for this case and we check against ground clearance in the autorotation lib.
…ve now needed variables and functions.
…read and to ensure flight phase progression based on heights. Also flip flag bools to be init checks.
…e not used elsewhere in the code.
…remove unused define.
…e and utilise a timer
// driven the collective high to cushion the landing so the landing detector will not trip until we drive the collective back to zero thrust and below. | ||
if ((_time_on_ground > 0) && ((AP_HAL::millis() - _time_on_ground) > MIN_TIME_ON_GROUND)) { | ||
// On ground, smoothly lower collective to just bellow zero thrust, to make sure we trip the landing detector | ||
float desired_col = _motors_heli->get_coll_mid() * 0.95; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs changed to use H_COL_LAND_MIN instead of zero thrust collective, since the landing detector looks for "motors->get_below_land_min_coll()"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are quite a few "lucky dip" units varables. As a rule if its not SI units we append the unit the variable/define. Also knowing we will be making some effort to more to SI units in the future it would be nice to bring this in using SI as much as possible.
if (!pos_control->is_active_xy() && use_psc_case) { | ||
return copter.current_loc.alt; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect we can just remove this whole section. I don't understand why it would just return current loc rather than trying to get the terrain value. As far as I can tell the only user that is not in position control is landing gear.
In anycase, removing this section means you can also remove the new bool.
|
||
//--- Internal functions --- | ||
void warning_message(uint8_t message_n); //Handles output messages to the terminal | ||
uint32_t _last_bad_rpm_ms; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its a little odd having this bad rpm time stamp in a different structure to the bad_rpm
flag. Maybe combine:
struct {
uint32_t last_ms;
bool bad;
} _rpm;
// Update the touchdown controller | ||
void touchdown_controller(void); | ||
|
||
void set_ground_distance(float gnd_dist) { _gnd_hgt = (float)gnd_dist; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void set_ground_distance(float gnd_dist) { _gnd_hgt = (float)gnd_dist; } | |
void set_ground_distance(float gnd_dist) { _gnd_hgt = gnd_dist; } |
g2.arot.init_hs_controller(); | ||
g2.arot.init_fwd_spd_controller(); | ||
// Init autorotation controllers object | ||
g2.arot.init(motors, copter.rangefinder.ground_clearance_cm_orient(ROTATION_PITCH_270)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing in ground clearance here is quite odd. It means that we use the range finder ground clearance for a rangefinder that we may not even be using for the altitude estimate.
I suspect we can just remove for now. Then we can come back and include ground clearance at the copter level before we pass the height above ground estimate down to the arot lib. That way it can only apply the the offset when using the rangefinder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also its a bit mad passing in motors in the mode init. You should be able to pass a const reference in the constructor.
_flags.straight_ahead_initial = true; | ||
_flags.bail_out_initial = true; | ||
_msg_flags.bad_rpm = true; | ||
// Set all intial flags to on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Set all intial flags to on | |
// Set all intial flags off |
@@ -251,54 +321,121 @@ float AC_Autorotation::get_rpm(bool update_counter) | |||
return current_rpm; | |||
} | |||
|
|||
void AC_Autorotation::initial_flare_estimate() | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bit is much better, but still quite scary.
(double)_accel_target, | ||
(double)_pitch_target); | ||
"TimeUS,hsp,hse,co,cff,sf,dsf,vp,vff,az,dvz,h", | ||
"Qfffffffffff", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to add units and multipliers too.
|
||
// Check to see if we think the aircraft is on the ground | ||
if(_current_sink_rate < 10.0 && _gnd_hgt <= _ground_clearance && _time_on_ground == 0){ | ||
_time_on_ground = AP_HAL::millis(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could const uint32_t now = AP_HAL::millis();
Its used again on 673
} | ||
|
||
|
||
void AC_Autorotation::update_avg_acc_z(void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me nervous, it will work differently with different loop rates. I would prefer a low pass. If average is better there is a AverageFilter
class that you should use. https://github.com/ArduPilot/ardupilot/blob/master/libraries/Filter/AverageFilter.h
//--------References to Other Libraries-------- | ||
AP_InertialNav& _inav; | ||
AP_AHRS& _ahrs; | ||
AP_MotorsHeli* _motors_heli; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AP_MotorsHeli* _motors_heli; | |
AP_MotorsHeli*& _motors_heli; |
You can refence the copter pointer and set it in the constructor. You will have to check for nullprt before use.
… to prevent a race on mode init.
…n blade pitch angle
…ermine unhealthy. Col defaults to ang of -2 if no RPM
post touchdown condition requires collective to be lowered below land_col_min_pct in order to get the disarm logic to disarm the aircraft
-collective gets lowered below land_col_min_pct in order to get the aircraft disarmed -adjusted thresholds of lfare update and flare complete check
7c8274a
to
241959e
Compare
This PR incorporates helicopter auto rotation, where it assists the pilot in the vertical axis and maintaining speed from entry all the way to touchdown. The pilot still maintains control of the lateral and directional axes to guide the aircraft to a safe touchdown point.