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

ekf2: Move handling of invalid range finder data inside ecl library #12950

Closed
wants to merge 3 commits into from

Conversation

priseborough
Copy link
Contributor

PX4/Firmware side of PX4/PX4-ECL#639 change

The ecl library EKF is able to use vehicle motion and in-air status to better determine when the default on-ground range finder reading can be used.
The description for the EKF2_MIN_RNG parameter has been updated to make its use clearer.
Requires ecl at commit 267195a or later.

TODO testing with this PX4/Firmware version

The ecl library EKF is able to use vehicle motion and in-air status to better determine when the default on-ground range finder reading can be used.
The description for the EKF2_MIN_RNG parameter has been updated to make its use clearer.
Requires ecl at commit 267195a or later.
@dagar
Copy link
Member

dagar commented Sep 15, 2019

@nicovanduijn have you tested this?

@nicovanduijn
Copy link
Contributor

nicovanduijn commented Sep 16, 2019

I've tested a combination of things, and was not very happy with the position hold (https://review.px4.io/plot_app?log=0a0d6b14-f9e7-4550-b083-5c74fae4103d)
I'll try to dissect and figure out what's causing this. I think we can make more improvement on the PWM3901 quality metric, it's publishing invalid way too often. I was thinking to integrate the flow for a bit longer before publishing.
Edit: After talking to @bresch and @DanielePettenuzzo , I think integrating for longer is not the way to go. Instead, we should get rid of the "no movement" check (the quality metric should handle this), as well as setting the frame count properly and averaging the quality over all taken samples. I'm doing some tests and then I'll do a PR for the PMW3901 once again

As to what's relevant to this PR, I think PX4/PX4-ECL#640 will be required for this to have any effect at all

@priseborough
Copy link
Contributor Author

@nicovanduijn

Height above ground before takeoff and after climbout is correct. The issue is the combined inertial and Baro errors during the transition period which results in an initial incorrect height above ground estimate before range finder data is used.

Screen Shot 2019-09-18 at 10 00 43 am

Poor position hold post climbout is a separate issue give that the estimated bottom distance is correct and shouldn't be a reason for blocking this PR.

The ground motion test inside ecl is there to ensure fusion of the 'faked' range finder measurement stops when takeoff starts if vehicle_land_detected.landed transition to false is delayed. This may cause more issues than it solves for vehicle that are subject to on ground vibration induced by wind and other external disturbances . Because vehicle_land_detected.landed message goes false at takeoff without delay, I would support removal of the check.

@priseborough
Copy link
Contributor Author

@nicovanduijn I just noticed in your log that you have set EKF2_RNG_AID = 1. Given a range finder that doesn't work until after takeoff and a baro altitude that is subject to significant propeller disturbance, this is causing issues with setting of the baro offset during takeoff. Other options are to set EKF2_RNG_AID = 0 and either enable terrain hold in the position controller instead by setting MPC_ALT_MODE = 2 or force the EKF to use range finder as the default sensor by setting EKF2_HGT_MODE = 2.

The combination of a range finder that doesn't read on the ground and a noisy baro sensor with a metre of offset when flying out of ground effect makes switching between range finder and baro problematic.

@priseborough
Copy link
Contributor Author

Replay using EKF2_RNG_AID = 0, EKF2_HGT_MODE = 2
Baro used until range finder returns valid data.
Screen Shot 2019-09-18 at 12 20 39 pm

Replay using EKF2_RNG_AID = 0, EKF2_HGT_MODE = 0
Baro is used throughout, but local position height is not stable. Cause TBD.
Screen Shot 2019-09-18 at 12 22 08 pm

Replay using EKF2_RNG_AID = 1, EKF2_HGT_MODE = 0
Switch to using range finder occurs too late after takeoff resulting in a significant baro innovation offset for two seconds prior to switching to range finder.
Screen Shot 2019-09-18 at 12 25 10 pm

jkflying pushed a commit that referenced this pull request Sep 18, 2019
dagar pushed a commit that referenced this pull request Sep 19, 2019
@nicovanduijn
Copy link
Contributor

I think the issues I was seeing with poor performance are unrelated to this PR:

  • Most likely FMUv4: CPU load > 95% #12974 caused degrading performance and the log dropouts
  • The take-off issues are mostly due to tuning and platform setup, as explained by paul. Any potential remedy in software should be addressed elsewhere and also not affect this PR

Bottom line: if @priseborough agrees, LGTM

@nicovanduijn
Copy link
Contributor

Flight test with a rebased version of this:
Log

Two things, both unlikely to be related to this PR:

Not sure yet what caused the second one.

@jkflying
Copy link
Contributor

Merged in #12988

@jkflying jkflying closed this Sep 19, 2019
@priseborough priseborough deleted the pr-ekfTerrainEstFix branch January 25, 2020 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants