AP_NavEKF3: added in a check for high velocity innovation

[tridge]

if using GPS and our horizontal velocity innovation is high then mark the filter as unhealthy, which causes an arming failure

this helps with #29384

[peterbarker]

Looks right. How was it tested?

[tridge]

Looks right. How was it tested?

replay on the log, with printf() for when it triggers

[peterbarker]

@rmackay9 we have users moving from various non-GPS-navigation modes (eg. inside hangars) to using-GPS. Will this adversely affect them?

I'm thinking of things like the ahrs-source-extnav-optflow.lua workflows. I don't think we have any autotest for that one.

[rmackay9]

Hi @peterbarker, I'm not sure.

TBH, I'm not sure if we should be changing the meaning of "healthy". If we want the pre-arm to protect against arming with high innovations then I think we should instead add a specific check for that.

[rishabsingh3003]

I think this would block arming in some valid use cases; for example, a user trying to take off manually in alt-hold mode from a location that is adversely affecting the GPS (hence causing high innovations) but will be fine once high up in the air. It should be a specific check, as Randy suggested.

[peterbarker]

I think this would block arming in some valid use cases; for example, a user trying to take off manually in alt-hold mode from a location that is adversely affecting the GPS (hence causing high innovations) but will be fine once high up in the air. It should be a specific check, as Randy suggested.

We have an arming bit for this specific situation - "always require position for arming"....

[tridge]

@rmackay9 @rishabsingh3003 I've changed it so that the check is only done if position is required

[rmackay9]

hmm, this essentially adds another type of healthy. I still don't understand why we don't just add an arming check if we're trying to prevent arming with a high velocity innovation. That would surely be a more direct approach?

[tridge]

hmm, this essentially adds another type of healthy. I still don't understand why we don't just add an arming check if we're trying to prevent arming with a high velocity innovation. That would surely be a more direct approach?

we want it to apply to all lanes, and we want it on the raw innovations in those lanes. We don't have that information outside the EKF code. So we could add a new set of APIs to do it externally, or we can extend the existing NavEKF3::pre_arm_check() which is what I've done.
I could add a NavEKF3_core::pre_arm_check() instead of extending healthy() if you like. I did it by extending healthy() as it is so similar to the existing checks inside healthy()
When this triggers the EKF really is unhealthy

[rishabsingh3003]

I like this change. However, checking just velocity seems a bit arbitrary. We should probably be checking at least some of the other states as well for high innovations?

For example, I probably wouldn't want to takeoff in guided if the vertical position innovations are super high. Might be scope creep considering this in response to a particular crash.

[peterbarker]

So this will give us a pre-arm check, and I think it's mergeable just for that. Would have prevented a bad mishap, and is easily back-portable.

The current formulation doesn't give us any post-arming protection. We could possibly look at telling the EKF we require it to have position, instead of passing that information in as part of the arming checks. That would allow us to do this velocity check continuously rather than just at prearm.

[tridge]

For example, I probably wouldn't want to takeoff in guided if the vertical position innovations are super high. Might be scope creep considering this in response to a particular crash.

The position states will reset on high innovation, but the velocity states won't. We could add a check for how recently a reset has happened if we want to cover that. I was trying to get a minimal change in for this particular issue that we can put into 4.6.0.

[rmackay9]

I think functionally this is fine but the way it's hidden in the ahrs::healthy() seems unintuitive for future developers to figure out.

I very much like Tridge's suggestion to, "I could add a NavEKF3_core::pre_arm_check()". I think then it becomes very obvious what the check's purpose is.

[tridge]

@rmackay9 I added the per-core pre-arm check function as discussed

[rmackay9]

Great, thanks!

[rmackay9]

this is included in 4.6.0-beta5