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

Incorrect use of covariance in distance-sensor uORB topic #9861

Closed
priseborough opened this issue Jul 8, 2018 · 9 comments
Closed

Incorrect use of covariance in distance-sensor uORB topic #9861

priseborough opened this issue Jul 8, 2018 · 9 comments
Assignees
Labels

Comments

@priseborough
Copy link
Contributor

priseborough commented Jul 8, 2018

The uORB distance sensor topic incorrectly contains a covariance field:

https://github.com/PX4/Firmware/blob/master/msg/distance_sensor.msg#L7

The issue is that you cannot have a covariance for a single measurement because a covariance is a measure of the amount of correlation between two random variables. For a single variable, the correct name to use is 'variance' which should have units of m^2

An inspection of the various publishers of this message shows that all but one are publishing a 0 value. The ulanding distance sensor driver is populating it as if it is a variance with units of m^2 using a fixed value.

The distance_sensor message needs to be corrected so that it:

  1. Uses correct variable names and units
  2. Makes the distinction between noise variance and accuracy clear. For example a distance sensor could have only 0.05m RMS noise (a noise variance of 0.0025) but could have a range accuracy of 0.1m. To be used fully by various estimators, the distance sensor message should contain both a 'noise_variance' and a 'accuracy' field.

A 'noise_variance' calculated from the data can be used by the sensor driver to determine if the sensor data is invalid. Sonar sensors are prone to very noisy data under certain circumstances and a noise variance validity test, spike rejection filter and median select filter should be available for each distance sensor.

@xdwgood
Copy link
Contributor

xdwgood commented Jul 9, 2018

Just curious: The sensor driver only publishes the raw data, why isn't the data processing and judgment done by ekf, because ekf is more clear about what it needs.

@priseborough
Copy link
Contributor Author

priseborough commented Jul 9, 2018

The large varying data we get from the sonar sensor under certain conditions which this check will detect is a failure mode specific to that type of sensor which is why I would rather not have it in the ekf2 module.

We may also be able to use the noise variance internal to the EKF to set the observation noise variance, but this is yet to be determined.

@LorenzMeier
Copy link
Member

@priseborough Could you send a PR that renames that field and initializes it with a sane value for all sensors? Happy to assist there, but changing makes total sense, yes.

@TSC21
Copy link
Member

TSC21 commented Jul 9, 2018

@LorenzMeier @priseborough it probably makese sense to sync with mavlink/mavlink#917

@priseborough
Copy link
Contributor Author

I have a branch which can be rebased and submitted as a PR when #9865 is merged

See:

https://github.com/priseborough/Firmware/commits/pr9865

@kyuhyong
Copy link

Is this issue relate to my experience with sonar?
http://discuss.px4.io/t/distance-sensor-covariance-value-not-updating/7267?u=kyuhyong_you
Log data shows no indication of updating covariance in sonar data.
distancesensor
I just wonder what this distance sensor correlate to other sensor signal too.

@TSC21
Copy link
Member

TSC21 commented Jan 21, 2019

Still relevant

@stale stale bot removed the Admin: Wont fix label Jan 21, 2019
@stale stale bot closed this as completed Jul 9, 2019
@julianoes julianoes reopened this Jul 9, 2019
@stale stale bot removed the Admin: Wont fix label Jul 9, 2019
@PX4 PX4 deleted a comment from stale bot Jul 9, 2019
@PX4 PX4 deleted a comment from stale bot Jul 9, 2019
@PX4 PX4 deleted a comment from stale bot Jul 9, 2019
@stale
Copy link

stale bot commented Oct 7, 2019

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Oct 7, 2019
@bresch
Copy link
Member

bresch commented Aug 15, 2022

done in #11520

@bresch bresch closed this as completed Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants