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

AP_NavEKF3: accept set origin even when using GPS #27081

Merged
merged 3 commits into from
May 20, 2024

Conversation

rmackay9
Copy link
Contributor

@rmackay9 rmackay9 commented May 17, 2024

This removes two unnecessary constraints on when the EKF origin can be set and adds an autotest (courtesy of PeterB).

The existing constraints cause problems when the vehicle is simultaneously using GPS and OpticalFlow (e.g. EK3_SRC1_POSXY = 3/GPS, EK3_SRC1_VELXY=5/OptFlow, EK3_SRC_OPTIONS = 1/FuseAllVelocities) and the vehicle's start location is indoors where there is no GPS. In this situation the user is currently stuck because the constraint means the origin must come from the GPS but the GPS cannot provide a Location. I've received two reports of this inconvenience (the most recent report is here).

Note that I've also removed a constraint for, "PV_AidingMode == AID_ABSOLUTE". As far as I know there is no way the EKF can be in AID_ABSOLUTE without an origin which we check for moments later in setOrigin(). It does no harm but it also does no good and is probably just a left-over from the earlier changes related to a common origin or the moving internal origin.

I have tested this in SITL to confirm that it resolves the issue. Using master/latest with both GPS and optical flow enabled and SIM_GPS_DISABLE=1 the EKF origin could not be set using MAVProxy's right-mouse-button menu.
image

With this PR the origin could be set.
image

In addition the vehicle could be force armed (to bypass the "GPS 1: Bad fix" message) and flown. Below is a screen shot of a test in which the GPS was disabled and the EKF origin was purposely set 100m off from the correct Location. The vehicle was armed and flew correctly albeit with a persistent position offset.
image

When GPS was restored (e.g. param SIM_GPS_DISABLE 0) the vehicle correctly (and smoothly) jumped back to its actual position).
image

Note that I found a bug in master during testing. This PR makes it no better but also no worse.

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

This sanity check was in place in case the user managed to se origin a very long way away from where the vehicle actually was.

We believe that that isn't a problem post-Extreme ArduPilot moving-origin stuff.

What testing have you done for the case which Extreme ArduPilot thinks it fixed?

I've created this PR which was my attempt at testing this scenario: #27084 in my branch equivalent to this one.

libraries/AP_NavEKF3/AP_NavEKF3.cpp Outdated Show resolved Hide resolved
@peterbarker
Copy link
Contributor

... in fact, here's my pre-existing PR: #25110

Note tridge's objection on my PR which would also seem to be the same objection on your PR here (you're also not modifying the function tridge pointed to)

rmackay9

This comment was marked as outdated.

@rmackay9 rmackay9 force-pushed the ekf3-allow-set-origin-even-with-gps branch from fe55c1d to 279d3ef Compare May 20, 2024 10:35
@tridge tridge merged commit 1a04ead into ArduPilot:master May 20, 2024
91 checks passed
@rmackay9 rmackay9 deleted the ekf3-allow-set-origin-even-with-gps branch May 21, 2024 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants