Reset height and position data indoors from last known location#31177
Reset height and position data indoors from last known location#31177rmackay9 merged 13 commits intoArduPilot:masterfrom
Conversation
|
... also, we probably don't need extra parameters for this storage, we persist home already. |
Where is that? |
ff82426 to
726d213
Compare
mission-slot-0. Yeah this is not user friendly and in discussion with others we don't want this associated with a mission. I think what is in the PR is still the best way of doing this. |
|
I have been using this a lot and it is great! |
peterbarker
left a comment
There was a problem hiding this comment.
I think I quite like this as a concept now. Yes, I've changed my mind on the parameters :-)
b4bf319 to
f020dbf
Compare
peterbarker
left a comment
There was a problem hiding this comment.
Spams
LOITER> Received 1366 parameters
Saved 1344 parameters to mav.parm
AP: Field Elevation Set: 0m
AP: Field Elevation Set: 0m
AP: Field Elevation Set: 0m
LOITER> AP: Field Elevation Set: 0m
AP: EKF3 IMU0 is using GPS
LOITER> AP: EKF3 IMU1 is using GPS
LOITER> aAP: Field Elevation Set: 0m
Ah-ha! So that's where the spam is coming from! Okay, need to figure that out because it was annoying me as well. |
|
Spam is pre-existing |
ArduCopter/mode.h
Outdated
| void run() override; | ||
|
|
||
| bool requires_GPS() const override { return true; } | ||
| bool requires_GPS() const override { return false; } |
There was a problem hiding this comment.
Thanks for this, it would definitely be nice if users didn't need to turn off arming checks when using optical flow (or other non-GPS based position estimation).
I think "requires_GPS" and "requires_position()" should always be the same. The "requires_GPS" is just a hang-over from when GPS was the only viable option. If we agree on this then let's just rename, "requires_GPS" to "requires_position".
There was a problem hiding this comment.
Andy and I have discussed this elsewhere.
I was pushing for requires_location and requires_position, using the same terms we use elsewhere - Location as in absolute lat/lon object and "position" as in offsets-from-origin.
requires_absolute_location and requires_position_relative_to_origin could also work :-)
... or requires_latitude_longitude_altitude?
There was a problem hiding this comment.
I am unconvinced about this. requires_GPS() causes GPS checks to be run - they shouldn't be run if the vehicle only requires position and the EKF is ok. Or for instance follow mode requires GPS because position (in the way we mean here) is not good enough. So I don't think there can only be a single function, but I am not fussed about what the two functions are called. FWIW I prefer the current naming - absolute position is pretty much only provided by the GPS.
ArduCopter/AP_Arming_Copter.cpp
Outdated
| fence_requires_gps = (copter.fence.get_enabled_fences() & (AC_FENCE_TYPE_CIRCLE | AC_FENCE_TYPE_POLYGON)) > 0; | ||
| #endif | ||
|
|
||
| // check if we only require position and the EKF thinks we are ok |
There was a problem hiding this comment.
I think we should instead rename "gps_checks" to "position_checks" or maybe just add add a comment to explain that they're synonymous. Then within this function we should put the position checks at the top and GPS checks at the bottom. Just above the GPS checks we should ask the AHRS if it's using GPS as its position, velocity or yaw source and if "no" we can skip the GPS checks. The AHRS doesn't currently have an accessor to ask which sensor it is using but we have similar functions like, "using_extnav_for_yaw" so around the same area we could have add a "using_gps_for_position_or_velocity". I'm not sure immediately how we should implement this function but perhaps DCM would always return "true" and perhaps EKF3 could re-use the logic in AP_NavEKF_Source's pre-arm check
There was a problem hiding this comment.
We should not care how the EKF is getting the data it is using to supply us with stuff. Well, as much as possible, anyway. Asking the EKF how it is getting the data is a layering violation and unnecessary AFAICS (and @andyp1per 's PR here doesn't require it!) Examples of where doing that will stuff you up - beacons which are rooted at a lat/lon, external AHRS of any sort and visual odometry input to the EKF supply lat/lng/alt (e.g. from a google-maps-image-mapping external source).
There was a problem hiding this comment.
Hi @peterbarker, unfortunately we do care about the EKF's sources if we only want to check those sources that the EKF is currently using. So whether that's the GPS or visual odometry or whatever, if we only want to check that the currently used source is healthy, then we need to know what the source is.
There was a problem hiding this comment.
Hi @peterbarker, unfortunately we do care about the EKF's sources if we only want to check those sources that the EKF is currently using. So whether that's the GPS or visual odometry or whatever, if we only want to check that the currently used source is healthy, then we need to know what the source is.
I can see that for some use cases.
But I don't think it's the case for optical flow; we don't (or shouldn't, thus this PR) actually need GPS to be healthy in Copter to arm and fly in loiter if you have good optical flow.
There was a problem hiding this comment.
@rmackay9 this does check the sources via the EKF. If you try this indoors with sources requiring GPS you will get an error "EKF sources require GPS". So I think it behaves in the way you want, its just that the function is a little subtle.
There was a problem hiding this comment.
And if the user is flying indoors and the EKF pos/vel source is not GPS it doesn't raise the GPS pre-arm failures?
There was a problem hiding this comment.
@rmackay9 correct, the code as written only gives a GPS pre-arm check if the sources require the GPS and the mode only requires "position" and not "GPS". This is another reason why we need two functions. Note that I think we have to delegate a bunch of this to the EKF - the EKF check is in some ways stricter and in other ways looser as it not only looks at what has been configured but also whether its good enough as a source for whatever.
7427e7d to
8c4fd7a
Compare
ef73e05 to
bc8b9f9
Compare
|
@rmackay9 this is clean now and I have updated the tests to appropriately check that we can't arm without GPS where necessary |
|
@rmackay9 @peterbarker and I have reached agreement, branch updated to reflect |
Tridge's request to add an option bit has been added
8ad6da0 to
182166c
Compare
There was a problem hiding this comment.
I've tested this fairly well in SITL to confirm it works as expected. Below are some screen shots of the testing
Tested that the origin is recorded and used when EKF sources do not include GPS

Tested that the AHRS_OPTIONS bit for "RecordOrigin" works.

Tested that the AHRS_OPTIONS bit for "UseRecordedOrigin" works

Tested that the AHRS_OPTIONS bit for "UseRecordedOrigin" has no effect if EK3_SRC1_POSXY or EK3_SRC1_VELXY is set to GPS

Confirmed that the Sub parameter conversions from ORIGIN_LAT/LON/ALT to AHRS_ORIGIN_LAT/LON/ALT works

|
We discussed on the dev call and agreed that once @Williangalvani has had a chance to test and approve we will merge (assuming that WillianG is happy of course) |
Co-authored-by: Randy Mackay <rmackay9@yahoo.com> option bit 3 controls whether the EKF origin is recorded to parameters option bit 4 controls whether a recorded origin is used to initialise the EKF when using non-GPS sources (e.g. sources without an absolute position)
Co-authored-by: Andy Piper <github@andypiper.com>
This gives more time for the AHRS update to consume the origin and output the global-position-int mavlink message
182166c to
784a1a1
Compare
|
@Williangalvani ready to go when you approve from the sub side |
Williangalvani
left a comment
There was a problem hiding this comment.
I am happy with the Sub changes.
Also just tested on real hw.
|
Thanks @Williangalvani and @rmackay9 ! |
Summary
This PR enables reliable indoor flight using non-GPS position sources (optical flow, external navigation/motion capture, etc.) by refactoring how flight modes declare their position requirements and adding persistent origin storage.
Problem
Currently, many Copter flight modes declare
requires_GPS()which causes issues when flying indoors with alternative position sources like optical flow or external navigation systems. The EKF can provide valid position estimates from these sources, but the mode requirements and arming checks still demand GPS specifically. This can lead to:Solution
1. Mode Requirement Refactoring:
requires_GPS()→requires_position()All Copter flight modes now declare whether they need position rather than specifically GPS. The AHRS/EKF is then consulted to determine if the position requirement can be satisfied by the current sensor suite (GPS, optical flow, external nav, etc.).
2. GPS Usage Detection
New
using_gps()methods added toAP_AHRS,NavEKF2, andNavEKF3allow the system to distinguish between:3. Persistent Origin Storage
New AHRS parameters for saving/restoring the EKF origin:
AHRS_ORIGIN_LAT- Last known origin latitudeAHRS_ORIGIN_LNG- Last known origin longitudeAHRS_ORIGIN_ALT- Last known origin altitudeNew
AHRS_OPTIONSbit (3):AutoRecordOrigin- Automatically saves the origin to parameters when it becomes valid.4. Origin Restoration on Startup/Arming
New
ARMING_OPTIONSbits allow the origin to be restored from saved parameters:This allows indoor flights to resume with a valid origin after power cycles.
5. Updated Arming Checks
Arming checks now properly handle position-only scenarios:
Affected Vehicles
Testing
Autotest updates included for optical flow scenarios to validate the new behavior.
Usage
For indoor flight with optical flow or external nav:
AHRS_ORIGIN_*parameters manually)AHRS_OPTIONSbit 3 to auto-record the originARMING_OPTIONSbit for origin restorationThis allows position-controlled modes (Loiter, Auto, etc.) to work indoors without GPS when a valid position estimate is available from alternative sources.