Airspeed: Sanity check MS4525DO data, and utilize a double read #5950
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
looks good to me, thanks michael! |
OXINARF
reviewed
Mar 29, 2017
There was a problem hiding this comment.
@WickedShell Nitpicking your words again, sorry 😄
Other than my small comments, LGTM.
| dT_raw2 = (data2[2] << 8) + data2[3]; | ||
| dT_raw2 = (0xFFE0 & dT_raw2) >> 5; | ||
|
|
||
| // reject any values that are the absolute minum or maximums |
| dT_raw2 = (0xFFE0 & dT_raw2) >> 5; | ||
|
|
||
| // reject any values that are the absolute minum or maximums | ||
| // these can happen due to gnd lifts or communication erros on the bus |
| return; | ||
| } | ||
|
|
||
| // reject any double reads where the value has shifted in the upper more then a byte as corrupted |
There was a problem hiding this comment.
The sentence looks like a mix of two. I would change it to:
reject any double reads where the values have a difference bigger than 255 (one lower byte)
| } | ||
|
|
||
| // reject any double reads where the value has shifted in the upper more then a byte as corrupted | ||
| if (abs(dp_raw - dp_raw2) >= 0xFF || abs(dT_raw - dT_raw2) > 0xFF) { |
There was a problem hiding this comment.
Any reason why one uses >= and the other >? In my opinion should be only >
|
@OXINARF you might convince me to find a spell checking plugin for vim... :) |
WickedShell
force-pushed
the
wickedshell/ms4525do-bad-data
branch
from
69c9ac0 to
b312d50
Compare
|
Flight tested to 20 m/s, worked quite well no periods of data outages, or visible anomalous readings. EDIT: Was also flight tested with the I2C DNF patch. |
magicrub
approved these changes
Mar 31, 2017
|
applied, thanks Michael! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is possible to get a really incorrect data read off the I2C bus, that will come out with either wildly different numbers or a maximum value. Given that this is an incredibly illogical input for airspeed data, we are better off rejecting the data.
To do this we double read the sensor data, the measurements will drift slightly between two readings, but not enough that the upper byte will undergo a change. We also check that no reading was the maximum or minimum value possible.
This needs to be flight tested to ensure that we don't reject to many valid reasons, but given the required rate of change that should be fine.
The image below shows a rejected bad double read. Channel 0 is SDA, Channel 1 is SCL. Notice that the second read gets really bad data off the SCL line, which results in an erroneous read on the Pixhawk. The double pulse on channel 2 indicates that the check on line 143 failed. In this case our second reading was bad, but it can easily be the other way around as well.