Skip to content
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

VTOL: Rework altitude-loss quad-chute #20916

Merged
merged 2 commits into from
Jan 19, 2023

Conversation

sfuhrer
Copy link
Contributor

@sfuhrer sfuhrer commented Jan 12, 2023

Based on #20913 (but basically separate).

Solved Problem

Previously the condition was based on hard coded height rate estimate and setpoint values and an altitude error threshold. That showed to be leading to false positives when the vehicle doesn't tightly follow the altitude ramp given by TECS to achieve a new altitude setpoint, and has become completely infeasible with the latest TECS rework that leads to non-ramped altitude setpoint changes in the tecs_status message.

For example this flight triggered a quadchute due to altitude loss (VT_FW_ALT_ERR was set to 15m, and vehicle was not able to follow TECS altitude ramp and then briefly had an uncommanded descent due to strong crosswind).
image

An additional issue was that during a front transition the VT_FW_ALT_ERR usually is desired to be set smaller than in FW flight, as a 10m altitude loss in transitions are more severe than in FW flight (due to the lower altitude).

Solution

I split up the "loss of altitude" aka "adaptive" QC into two:

  • TransitionAltitudeLoss QC (configurable through new param VT_QC_T_ALT_LOSS, 10 by default).
  • UncommandedDescend QC (only in FW flight, configurable through VT_QC_HR_ERROR_I)

The new check (UncommandedDescend) no longer checks the altitude error but only the height rate instead. It begins to integrate the height rate error once it detects an uncommanded descent condition (height rate negative while setpoint is positive). Integral threshold can be tuned by user (VT_QC_HR_ERROR_I). The integral is reset once out of the uncommanded descent.
I changed parameter name from VT_FW_ALT_ERR to VT_QC_HR_ERROR_I as it needs to be re-tuned should we go for this proposed solution here.

Test coverage

SITL tested by disabling FW throttle (FW_THR_MAX=0) and having VT_QC_HR_ERROR_I = 50m. Uncommanded descent QC is triggered 14s after the failure here, with an altitude loss of 25m (could be configured to trigger much earlier).
image
(btw, no there seems to be an issue with the tecs_status/altitude_filtered here, it always follows the setpoint without error).

Context

Fixes #14947?

@ThomasRigi
Copy link
Member

I clearly agree that the current implementation has its flaws on the height-rate related part, but I did like the explicit altitude error aspect of it. For long-range BVLOS flights where takeoff might be at a significantly different altitude than other parts of the route, we can't use VT_FW_MIN_ALT. So we were using VT_FW_ALT_ERR instead.

I think the integral approach makes sense for the height-rate, but it's harder to get an intuitive feel of it. VT_FW_ALT_ERR is dead clear to anyone on how to set it. You want to trigger when you're more than 15m below ? Set VT_FW_ALT_ERR to 15m. With the new parameter, suppose you're falling at -1m/s with a heightrate setpoint of +1m/s. To trigger at 15m below, you'd set VT_QC_HR_ERROR_I to 30m. But that same parameter value would trigger at 10m altitude loss if instead you were falling at -1m/s, but the setpoint was at +2m/s.

@sfuhrer
Copy link
Contributor Author

sfuhrer commented Jan 13, 2023

I think the integral approach makes sense for the height-rate, but it's harder to get an intuitive feel of it. VT_FW_ALT_ERR is dead clear to anyone on how to set it

It's only clear if you fly level, not when having altitude changes. As an alternative option I though about only enabling the uncommanded descent QC in level flight (would need to find convention what level flight really is, not that straight forward), but personally didn't like that as it only covers parts of a flight. And what I don't want is introduce even more complexity by having different checks in "level" and sinking/descending flight parts.

Speaking of the "tuning" part of it: when do you even want to tune it for specific missions? Isn't that a vehicle parameter that is set once (once you know how well the system tracks the height rate normally and what disturbances are to be expected). Are you exposing it to end-operators?

@ThomasRigi
Copy link
Member

No, we don't expose it to end users. We also don't expose the VT_FW_ALT_ERR param to begin with. I agree, you set it once for your platform and you're good. But it's harder to set it correctly the one time you do.

Completely agree that we don't want multiple modes on level/climb/descent. One (set of) criteria that's always valid is typically the best thing to have.

Another question, how did you get the height rate setpoint to even below 0 at t=239 in your first screenshot ? I mean, you're still like 12m below the altitude setpoint, so the drone should still want to climb ?

@sfuhrer
Copy link
Contributor Author

sfuhrer commented Jan 13, 2023

No, we don't expose it to end users. We also don't expose the VT_FW_ALT_ERR param to begin with. I agree, you set it once for your platform and you're good. But it's harder to set it correctly the one time you do.

Yes it's slightly more abstract than the altitude one. Especially for level flight there's though still a linear correlation of VT_QC_HR_ERROR_I to the actual altitude drop when QC is triggered if I'm not mistaken: the height rate setpoint grows (roughly) linearly to with the altitude error (as long as below max height rate), and thus the height rate error integral grows with a constant factor with the altitude error.
Anyway, at least now with the new check you actually can tune it - before you set it to 10m error and it only sometimes would trigger (e.g. doesn't trigger if the vehicle descends at less than 1m/s during the failure).

@sfuhrer
Copy link
Contributor Author

sfuhrer commented Jan 13, 2023

Another question, how did you get the height rate setpoint to even below 0 at t=239 in your first screenshot ? I mean, you're still like 12m below the altitude setpoint, so the drone should still want to climb ?

Yes the altitude was still below the setpoint, but the gradient of the altitude setpoint is negative and this is passed to the height rate setpoint through a FF, and then depending on your TECS settings you get a positive or negative height rate setpoint (in this case the altitude time constant was set higher and the FF higher, so it was not weighting altitude errors heavily).

Base automatically changed from pr-quadchute-enable-retransition-main to main January 13, 2023 16:02
RomanBapst
RomanBapst previously approved these changes Jan 13, 2023
Copy link
Contributor

@RomanBapst RomanBapst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea, let's do some testing.

@sfuhrer
Copy link
Contributor Author

sfuhrer commented Jan 18, 2023

Testflown on a standard VTOL. tHe VT_QC_HR_ERROR_I param was set to 30, and then a motor failure faked in FW flight by setting FW_THR_MAX to 0. As expected, the vehicle began to drop altitude (and airspeed), and quadchute triggered when the integrated height rate error is above 30m, which resulted in an altitude loss of 20m (for the two times we did it) with this tuning.

image
image

…ransition

By default the threshold is set to 10m.

Signed-off-by: Silvan Fuhrer <silvan@auterion.com>
Previously the condition was based on hard coded height rate estimate and
setpoint values and an altitude error threshold. That showed to be leading
to false positives when the vehicle doesn't tightly follow the altitdue
ramp given by TECS to achieve a new altitude setpoint, and has become
completely infeasibly with the latest TECS rework that leads to non-ramped
altitude setpoint changes in the tecs_status message.
The new check no longer checks the altitude error but only the height rate
instead. It begins to integrate the height rate error once it detects an
uncommanded descend condition (height rate negative while setpoint is
positive). Integral threshold can be tuned by user (VT_QC_HR_ERROR_I).

Signed-off-by: Silvan Fuhrer <silvan@auterion.com>
@sfuhrer
Copy link
Contributor Author

sfuhrer commented Jan 18, 2023

rebased

@sfuhrer sfuhrer merged commit f3cdf70 into main Jan 19, 2023
@sfuhrer sfuhrer deleted the pr-vtol-quadchute-altitude-errors-main branch January 19, 2023 08:36
@hamishwillee
Copy link
Contributor

@sfuhrer Do you think you could update this section: http://docs.px4.io/main/en/config/safety.html#quad-chute-failsafe for the updates here?

@RomanBapst
Copy link
Contributor

@hamishwillee It looks like you have given up on me Hamish. I will help with this, I can start tomorrow.

@RomanBapst
Copy link
Contributor

@hamishwillee Wait, this is actually the outdated one. #21598
This is the newest one.

@hamishwillee
Copy link
Contributor

@RomanBapst I assumed you got eaten by a killer aardvark. If you could do a PR that would be great :-)

@hamishwillee
Copy link
Contributor

@RomanBapst How did starting yesterday go?

@hamishwillee
Copy link
Contributor

@RomanBapst Yesterday, it always feels so far away, .... :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[VTOL] Adaptive Quadchute is broken
4 participants