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

Add baro temperature to sensor voter observation #9411

Merged
merged 1 commit into from
May 4, 2018
Merged

Conversation

potaito
Copy link
Contributor

@potaito potaito commented May 4, 2018

For our baro sensor (no driver in px4 master), I keep getting STALE errors every few seconds since a recent merge of master.

ERROR [sensors] Baro #0 fail:  STALE!
ERROR [sensors] Baro #0 fail:  STALE!
ERROR [sensors] Baro #0 fail:  STALE!

The errors appear while the vehicle is steady on a table indoors. Apparently the pressure reading from the baro can stay at a specific value for roughly a second under these conditions. As soon as > 100 (source in ecl) samples have the same value, the error above is triggered and the sensor flagged as stale.

A recent PR (#8466) modified the way baro measurements are published. Instead of the derived altitude, the message now only contains the pressure, which makes sense. Before that change however, the altitude was compared in the sensor voter and not the pressure. The altitude computation might have introduced some numerical magic that caused the measurement to be a bit more noisy - numerically speaking. 😕

My idea is to also add the baro temperature to the voter. But I am not sure if that's smart. What if the baro reading actually does become stale, but the temperature reading does not? I don't have enough experience to know what failure scenario the stale check is trying to detect.

@dagar any input? :)


Altitude example output:

altitude: 385.899048
altitude: 385.928375
altitude: 385.925751
altitude: 385.905731
altitude: 385.928894
altitude: 385.860107
altitude: 385.890503
altitude: 385.889740
altitude: 385.891541
altitude: 385.921204
altitude: 385.923889
altitude: 385.912598
altitude: 385.855103
altitude: 385.878265
altitude: 385.876770
altitude: 385.860931
altitude: 385.864838
altitude: 385.873047
altitude: 385.919250
altitude: 385.925293
altitude: 385.914398
altitude: 385.921326
altitude: 385.915283
altitude: 385.914581
altitude: 385.871460
altitude: 385.921387
altitude: 385.937500
altitude: 385.935303
altitude: 385.879333
altitude: 385.919769
altitude: 385.878693
altitude: 385.882874
altitude: 385.887695

Pressure example output of same sensor, different px4 version:

pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.789978  <- fluctuation
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.809998
pressure: 967.799988
pressure: 967.789978  <- fluctuation
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988
pressure: 967.799988

When determining the confidence of a barometer sensor, we should
consider the temperature as well, alongside the pressure.
Low-noise baros can show the same pressure reading for a second
or two when not moving and in an indoor location.
@potaito potaito self-assigned this May 4, 2018
@potaito potaito requested a review from dagar May 4, 2018 15:18
@dagar
Copy link
Member

dagar commented May 4, 2018

Yes this makes sense, and it's what we've done with the differential pressure sensors.

The other thing you could try is inserting the pressure as pascals. Are these repeated samples byte for byte identical floats or is the difference simply less than the hard coded epsilon in the validator?

@dagar
Copy link
Member

dagar commented May 4, 2018

What board and barometric sensor are you using?

@dagar
Copy link
Member

dagar commented May 4, 2018

This is good to merge, but I'd still consider the pascal conversion in another PR.

@dagar dagar merged commit 3ed093b into master May 4, 2018
@dagar dagar deleted the fix/baro-stale branch May 4, 2018 16:40
@potaito
Copy link
Contributor Author

potaito commented May 7, 2018

I was not around in the last days, sorry for the late response and thanks for your reply.

Are these repeated samples byte for byte identical floats or is the difference simply less than the hard coded epsilon in the validator?

Good point, I just checked that. The 24-bit raw measurement is not as stable as the pressure, meaning it would not trigger a "stale" warning. However, the pressure (milibars) that is sent via sensor.msg can already appear stale. I'm not sure what could be fixed here since the information loss already occurs in the driver. I guess that's where I would have to investigate further.

What board and barometric sensor are you using?

It's an mpc2520. I suppose this is the official website: http://www.maiersensor.com/products/air-sensors/mpc2520.html. I heard from my colleagues, that the driver was in master at one point, or at least there was a PR. But apparently we were the only ones using the sensor.

This is good to merge, but I'd still consider the pascal conversion in another PR.

Thanks. Not sure if I understand correctly: Would you then revert this change at a later point?

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

Successfully merging this pull request may close these issues.

2 participants