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: Consolidate range finder checking #9886

Closed
wants to merge 1 commit into from

Conversation

priseborough
Copy link
Contributor

DESCRIPTION

This brings all the range finder data checks (excluding innovation consistency checks) into one place and eliminates the need to perform range checking external to the library. Having checks in multiple places has previously resulted in unintended loss of functionality over time which had to be fixed by #476. This PR should reduce the likelihood of this type of bug being introduced in the future.

The hard coded optical flow tilt limit has also been changed to use the same value as the range finder for consistency.

Variable names have been changed changed to make a clear distinction between the max/min values calculated by by the stuck range check and the max/min valid values published by the sensor.

DEPENDENCIES

Requires PX4/PX4-ECL#477

Impacts #9865 (could be incorporated)

TESTING REQUIRED

For full test coverage we need to check:

Data below min valid (on ground and in air)
Data above min valid (on ground and in air)
Data returning to valid range after > 10 seconds out of range and sticking at a constant value (on ground and in air)
Vehicle tilted > 45 deg

Initial reversion test using replay log here: https://logs.px4.io/plot_app?log=a56f0800-6944-4f46-8171-4a7fab048923

@priseborough priseborough requested a review from RomanBapst July 11, 2018 23:50
@AlexKlimaj
Copy link
Member

@mcsauder and I can help test this one with our range finder.

@LorenzMeier
Copy link
Member

@priseborough ECL is merged and I think it makes sense to consolidate with #9865.

@philipoe
Copy link
Contributor

Impacts #9865 (could be incorporated)

I'd obviously also welcome if this could be incorprated in some way. @priseborough I modified #9865 according to your feedback, so If you want to directly incorprate it feel free. Otherwise I'd adapt #9865 later, although it would be more efficient to merge both together now ....

@AlexKlimaj
Copy link
Member

We just loaded this along with the new ecl onto our quad and had a successful test flight. We will do more testing here shortly.

https://review.px4.io/plot_app?log=8adafb31-7498-4900-9daa-1ebdd8715832

@dakejahl
Copy link
Contributor

dakejahl commented Jul 12, 2018

@priseborough We have recently been doing a lot of work in our distance sensor driver, mostly the limit the amount of bad data we publish. It appears that the estimator continues to fuse the last range finder data point.
fusedsteady_gpsincreases
rangefinder_data
https://review.px4.io/plot_app?log=89526d09-77bf-4edf-9a02-7c7c34e62681

Notice the last distance value published is at about 0.5m @ t=2:06s and the next point that comes in is at 2m @ t = 2:15s. The behavior we saw was the quad just keeps ascending because it thinks it is still at 0.5m. This is with range aid enabled.

@priseborough
Copy link
Contributor Author

I've created a patch priseborough@3e0d1df which effectively pulls this PR into #9865.

Once that is merged i will submit PR's to fix #9861 and move the quality == 0 check to inside the ecl library.

@AlexKlimaj
Copy link
Member

Just saw the same behavior as @dakejahl on a separate piece of hardware.

https://review.px4.io/plot_app?log=861edb66-663c-46b2-8a60-d1935a7290b6

image

image

@dakejahl
Copy link
Contributor

@priseborough Also I should ask, is simply not publishing the best way to go for a driver that has detected it has bad data? I agree with the conversation in #9865 that data validity checking should be done inside of the driver for the device.

@priseborough
Copy link
Contributor Author

@dakejahl and @AlexKlimaj I will raise this as an issue against the ecl library

@priseborough
Copy link
Contributor Author

@dakejahl In theory not publishing would be more efficient, however then we have no way to log the 'invalid' data which is necessary to develop algorithms to detect invalid operating conditions.

@AlexKlimaj
Copy link
Member

It looks like this function isn't turning off range aiding when the data is old.

https://github.com/PX4/ecl/blob/4d59c834ebb181b78294ec3981c3d32b470e6f15/EKF/control.cpp#L1143

@priseborough
Copy link
Contributor Author

@AlexKlimaj Can you please record your observations and any additional discussion here: PX4/PX4-ECL#478

@priseborough
Copy link
Contributor Author

@AlexKlimaj and @dakejahl Please ensure that your code complies with the updated distance_sensor message interface and that instead of not publishing data when out of range, you set the quality to 0

@priseborough
Copy link
Contributor Author

I am closing this because the required changes have been pulled into #9865

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.

5 participants