-
Notifications
You must be signed in to change notification settings - Fork 18
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
Fixing 1 in 256 edge-case that can lead to continuous no-output. #10
base: master
Are you sure you want to change the base?
Conversation
…cksum mismatch and therefore no output. If the last byte of the checksum is 0x42 it can be mistaken for the first start-byte, therefore checking the secondy start-byte is neccessary. The probability of this happening is 1 in 256 and in addition it has 100% probability if testing hepa-filtered air with 0 particels. Reason: All non-zero bytes remaining are: Decimal: 66 + 77 + 28 + 151 checksum: 256 + 66 Hex: 0x42 0x4d 0x00 0x1c ... 0x97 checksum: 0x01 0x42 It is not possible for byte 31 (second checksum-byte) to be 0x42 too, as the sum of the first 30 bytes is at most 30*255 which is less than 66 * 256 which is the smallest checksum that would cause byte 31 to be 0x42 (=66). The sequence 0x42 0x42 0x42 0x4d with the last two being the start bytes is therefore impossible. Though it is not impossible for the sequence 0x42 0x42 0x4d or 0x42 0x4d to appear at another position in the sequence it can only happen with particle count above 16896 = 66*256 and then this event is extremely unlikely itself and will extremely likely lead to a checksum mismatch therefore be discarded.
fixed formatting
this time using clang-format
@dherrada wanna test? |
@caternuson Hi - we are looking at an issue with this driver on WipperSnapper. It's been a few years since you proposed this fix. Do you still think that switching the read to "passive mode" would resolve this issue? |
@brentru I think it's at least worth investigating. Just checked and I do not currently have a PID 3686 to try it out though. I'll order one up to have since it looks like the general issue this PR was fixing still exists. |
@caternuson Thanks! |
If the last byte of the checksum is 0x42 it can be mistaken for the first start-byte, causing a byte-shift which leads to checksum mismatch followed by no ouput (continuously in the HEPA edge-case).
Therefore checking the secondy start-byte is necessary.
The probability of this happening is 1 in 256 and in addition it has 100% probability if testing HEPA-filtered air with 0 particles.
Reason: With HEPA-filtered air all bytes are zero except:
start 1 + start 2 + frame-length + reserved
Decimal: 66 + 77 + 28 + 151 checksum: 256 + 66
Hex: 0x42 0x4d 0x00 0x1c ... 0x97 checksum: 0x01 0x42
Testing with hepa-filtered air, an arduino nano, software serial (2,3) the byte-shift with continuous no-output happened repeatedly after less than 10 measurement cycles. With disabled checksum-check the 15 uint16_t components of data then look like this:
Byte-shift.txt
---- considering & disregarding further edge-cases ----
It is not possible for byte 31 (second checksum-byte) to be 0x42 too, as the sum of the first 30 bytes is at most 30*255 which is less than 66 * 256 which is the smallest checksum that would cause byte 31 to be 0x42 (=66).
The sequence 0x42 0x42 0x42 0x4d with the last two being the start bytes is therefore impossible.
Though it is not impossible for the sequence 0x42 0x42 0x4d or 0x42 0x4d to appear at another position in the sequence it can only happen with:
which is both really unlikely and very likely not a stable condition with a low chance of multiple byte-shift leading to checksum-mismatch and continuous no-output.