Fix infinite climb bug when using EK3_OGN_HGT_MASK

[peterbarker]

This bug-finding and fix are not mine. The autotest is mine, however :-)

@pavloblindnology reported them here: https://github.com/ArduPilot/ardupilot/issues/24039

@pavloblindnology was this the fix you were recommending?

This PR is based on https://github.com/ArduPilot/ardupilot/pull/26916 which adds some infrastructure changes to make autotesting with baro drift easier.

[rmackay9]

@peterbarker,

I guess this is a bug that can happen on a real vehicle as well? It's not just a SITL issue is it?

[peterbarker]

Some pictures from a log where we never install the check_altitude message hook (pre-fix):

[peterbarker]

I guess this is a bug that can happen on a real vehicle as well? It's not just a SITL issue is it?

I have no idea.

I'm not even sure I'm replicating what @pavloblindnology is seeing.

[pavloblindnology]

@peterbarker Sorry for late response. Yes, the fix is right.

I guess this is a bug that can happen on a real vehicle as well? It's not just a SITL issue is it?

@rmackay9 I tested this in SITL only, but I don't see any reason why it can't happen on a real vehicle. I don't remember all the details of my tests as it was quite a while ago. But the main idea is that with EK3_OGN_HGT_MASK=5, Ardupilot doesn't use previously computed value of ekfGpsRefHgt to convert new GPS measurements. And as barometer altitude usually rises faster than GPS altitude, this results in constant ekfGpsRefHgt monotonical correction.

[davidbuzz]

@priseborough

[peterbarker]

Note that my autotest only reproduces the issue - I didn't point any fail condition in for the specific bug

[peterbarker]

I've reset this to Paul's branch and rebased this PR on master and removed the commit/revert pair.

So some less-than-fortunate news. Somehow tridge's PR to use a better atmospheric model (https://github.com/ArduPilot/ardupilot/pull/26915) also fixed the autotest.

I bisected the code to work that out:

IOW the autotest passes before the fixes in this branch are applied, which is Less Than Good. OTOH, since I've no idea how tridge's patch influences things, I'm quite happy to just carry on.

@priseborough are you OK with merging this even 'though the problem doesn't reproduce on master post-tridge-patches?

[priseborough]

Will check log and get back to you

[priseborough]

The better atmospheric model changes how the offset between baro height and GPs height varies so it makes sense the autotest would be sensitive to this. I'm happy to merge on the basis the old model shows it failing before. It should be possible to make it fail on the new model by fudging the baro scale factor or similar.

[peterbarker]

The better atmospheric model changes how the offset between baro height and GPs height varies so it makes sense the autotest would be sensitive to this. I'm happy to merge on the basis the old model shows it failing before. It should be possible to make it fail on the new model by fudging the baro scale factor or similar.

Thanks Paul. Marking as MergeOnCIPass.

[peterbarker]

Thanks @pavloblindnology - we've merged a fix for the bug you found.

[ddomit]

Any idea to which version of arducopter is going to include this?

Does ":Apply corrections to local position" mean that it will update the home altitude accordingly? i have tried using it set to 1, but when it comes back to home its sometimes 10 meters off...

Log: https://drive.google.com/file/d/1oEntDXzGjo2kmv54ihtVaRBwAyEa0qKx/view?usp=drive_link

[rmackay9]

Hi @ddomit,

This PR's fix will most likely be included in 4.5.5. We would normally have included it in the next release (4.5.4) but that's likely to include only a single small fix

[ddomit]

Hi @ddomit,

This PR's fix will most likely be included in 4.5.5. We would normally have included it in the next release (4.5.4) but that's likely to include only a single small fix

Thanks!! Do you think this fix and update will solve the issue im having? Will it update the home location with the altitude corrections it does?

[rmackay9]

@ddomit,

I think it's best to move this to the support forums. Support within a PR is difficult and the issue may very well be unrelated. Could you create a new topic in the support forum?

[ddomit]

@ddomit,

I think it's best to move this to the support forums. Support within a PR is difficult and the issue may very well be unrelated. Could you create a new topic in the support forum?

Thanks @rmackay9 it is okay i'll wait for the release, i dont need to create an issue.

I just have one doubt, i havent found anywhere, does "Apply corrections to local position" mean that it will update home altitude as well?

[rmackay9]

Hi @ddomit,

If bit2 ("Apply corrections to local position") is set then the vehicle's current altitude estimate is adjusted (instead of adjusting the EKF origin's altitude). The EKF doesn't have access to the AHRS home location (including its altitude) so it is not directly adjusting it. To put it another way, if bit2 is set and the EKF adjusts the vehicle's current altitude, before and after a flight, if you asked the AHRS what the home lat, lon and alt are it should show that they have not changed.

[postelzhuk]

Hi everyone. I made some SITL-tests from master, Plane-4.4, Plane-4.5. Almost sure this changes also fixes problem with baro drift: https://discuss.ardupilot.org/t/ekf3-use-barometer-and-gps-for-altitude-posz/119540/2 EK3_OGN_HGT_MASK, 5 does help with SLOW baro drifts after this changes.