Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

EKF: support passing in an external position estimate #23903

Merged
merged 8 commits into from
Jun 6, 2023

Conversation

tridge
Copy link
Contributor

@tridge tridge commented May 27, 2023

This allows for a position update from an external source such as someone spotting a landmark with an FPV camera.
MAVProxy support here:
ArduPilot/MAVProxy#1186
needs this mavlink PR: ArduPilot/mavlink#318

@tridge tridge requested a review from priseborough May 27, 2023 09:50
@priseborough
Copy link
Contributor

See tridge#41

@tridge tridge force-pushed the pr-ext-pos-reset branch from 4658674 to 079b711 Compare May 28, 2023 03:06
@rmackay9
Copy link
Contributor

rmackay9 commented May 28, 2023

Very interesting, I was thinking about doing something like this with an image tracking camera.

Why wouldn't we go through the AP_VisualOdometry library though and use one of the many existing vision-position messages?

@tridge tridge force-pushed the pr-ext-pos-reset branch from b500248 to 856692d Compare May 28, 2023 03:33
@tridge
Copy link
Contributor Author

tridge commented May 28, 2023

Why wouldn't we go through the AP_VisualOdometry library though and use one of the many existing vision-position messages?

a few reasons. First, we want the ACK/NACK as it is nacked when we are not in AID_NONE. Secondly, we need the dual timestamp to cope with much longer than normal lag - it would be beyond fusion horizon if done via vision msgs.
Finally, it is a reset, not a normal fusion, so it leaves velocity alone. A vision position will change velocity.

@tridge tridge force-pushed the pr-ext-pos-reset branch from 856692d to fc35066 Compare May 28, 2023 04:18
@tridge
Copy link
Contributor Author

tridge commented May 28, 2023

log from a CubeOrange quadplane (MakeFlyEasy Hero):
http://uav.tridgell.net/tmp/00000068.BIN

@priseborough
Copy link
Contributor

priseborough commented May 28, 2023

The positon and velocity states and their corresponding variances from the quadplane test flight have the expected behaviour, eg:

Position sets do not directly affect the velocity states

Screen Shot 2023-05-28 at 5 53 57 pm Screen Shot 2023-05-28 at 5 54 18 pm

The position state variance set to the expected value as specified by p3 in the mavlink command
Screen Shot 2023-05-28 at 5 54 24 pm

Velocity state variances do not change at the position set events
Screen Shot 2023-05-28 at 5 55 17 pm

The log can be replayed successfully.

@priseborough priseborough self-requested a review May 28, 2023 08:12
@rmackay9
Copy link
Contributor

I'd like to discuss this a bit before we merge. Obviously the functionality is great, it's just the architecture and possibly message name. Also I've got a similar change that adds a position reset for vision-position sources and I'd like us to make sure they're consistent. PR coming for that later today.

@rmackay9
Copy link
Contributor

Here's my related PR:

  1. mavlink PR Common: add align-vision-position-local-ned mavlink#319 adds a new align-vision-position-local-ned command
  2. flight code PR AP_VisualOdom: support align-position-and-yaw #23911 supports correcting the drift from a vision position system

@tridge
Copy link
Contributor Author

tridge commented May 30, 2023

needs #if for flash cost, 2M boards only

@tridge tridge force-pushed the pr-ext-pos-reset branch from 1ecef35 to ea194ce Compare June 5, 2023 23:40
@tridge tridge merged commit 4e09fe0 into ArduPilot:master Jun 6, 2023
const uint32_t timeStampConstrained_ms = MAX(MIN(timestamp_ms, imuSampleTime_ms), imuSampleTime_ms - 5000);
const int32_t delta_ms = int32_t(imuDataDelayed.time_ms - timeStampConstrained_ms);
const ftype delaySec = 1E-3F * ftype(delta_ms);
const Vector2F newPosNE = EKF_origin.get_distance_NE_ftype(loc) - stateStruct.velocity.xy() * delaySec;

Choose a reason for hiding this comment

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

@priseborough @tridge I might look at this the wrong way but shouldn't this be a positive sign here?

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants