From 81f2bd8558b73bc67ef0b8c587888651283bb9f2 Mon Sep 17 00:00:00 2001 From: Peter-J Date: Sat, 6 Feb 2021 20:29:40 +0100 Subject: [PATCH 1/3] Fixing 1 in 256 edge-case that can lead to byte shift followed by checksum 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. --- Adafruit_PM25AQI.cpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/Adafruit_PM25AQI.cpp b/Adafruit_PM25AQI.cpp index a6aee3b..9d68f52 100644 --- a/Adafruit_PM25AQI.cpp +++ b/Adafruit_PM25AQI.cpp @@ -91,15 +91,26 @@ bool Adafruit_PM25AQI::read(PM25_AQI_Data *data) { return false; } } - if (serial_dev->peek() != 0x42) { - serial_dev->read(); + if (serial_dev->read() != 0x42) { //read assuming you got the first byte return false; } - // Now read all 32 bytes - if (serial_dev->available() < 32) { + //now catching 1 in 256 edge-case where last byte of checksum is 0x42 too, being mistaken for first start byte, leading to byte shift and checksum mismatch + if (serial_dev->peek() == 0x42) { //peek if next byte might match first start byte + serial_dev->read(); // if yes last assumption was wrong, read again assuming you now got the start + } + if (serial_dev->read() != 0x4d) { //read & check the second byte + return false; + } + // Now the first two bytes are guaranteed to be 0x42 0x4d but have already been read + buffer[0] = 0x42; + buffer[1] = 0x4d; + // Now read remaining 30 bytes + if (serial_dev->available() < 30) { return false; } - serial_dev->readBytes(buffer, 32); + for (uint8_t i=2; i<32; i++){ + buffer[i] = serial_dev->read(); + } } else { return false; } From 204bbcf139857f6733dc7e06ef2ab81ede43580b Mon Sep 17 00:00:00 2001 From: Peter-J Date: Sat, 6 Feb 2021 21:48:45 +0100 Subject: [PATCH 2/3] Fixed formatting fixed formatting --- Adafruit_PM25AQI.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Adafruit_PM25AQI.cpp b/Adafruit_PM25AQI.cpp index 9d68f52..0771dbd 100644 --- a/Adafruit_PM25AQI.cpp +++ b/Adafruit_PM25AQI.cpp @@ -95,22 +95,22 @@ bool Adafruit_PM25AQI::read(PM25_AQI_Data *data) { return false; } //now catching 1 in 256 edge-case where last byte of checksum is 0x42 too, being mistaken for first start byte, leading to byte shift and checksum mismatch - if (serial_dev->peek() == 0x42) { //peek if next byte might match first start byte + if (serial_dev->peek() == 0x42) { //peek if next byte might match first start byte serial_dev->read(); // if yes last assumption was wrong, read again assuming you now got the start } - if (serial_dev->read() != 0x4d) { //read & check the second byte - return false; + if (serial_dev->read() != 0x4d) { //read & check the second byte + return false; } - // Now the first two bytes are guaranteed to be 0x42 0x4d but have already been read - buffer[0] = 0x42; - buffer[1] = 0x4d; + // Now the first two bytes are guaranteed to be 0x42 0x4d but have already been read + buffer[0] = 0x42; + buffer[1] = 0x4d; // Now read remaining 30 bytes if (serial_dev->available() < 30) { return false; } for (uint8_t i=2; i<32; i++){ - buffer[i] = serial_dev->read(); - } + buffer[i] = serial_dev->read(); + } } else { return false; } From 75622a37ea39703e850fd1f89ca35a05f361a50a Mon Sep 17 00:00:00 2001 From: Peter-J Date: Sat, 6 Feb 2021 22:26:12 +0100 Subject: [PATCH 3/3] fixed formatting this time using clang-format --- Adafruit_PM25AQI.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/Adafruit_PM25AQI.cpp b/Adafruit_PM25AQI.cpp index 0771dbd..21a7430 100644 --- a/Adafruit_PM25AQI.cpp +++ b/Adafruit_PM25AQI.cpp @@ -91,24 +91,29 @@ bool Adafruit_PM25AQI::read(PM25_AQI_Data *data) { return false; } } - if (serial_dev->read() != 0x42) { //read assuming you got the first byte + if (serial_dev->read() != 0x42) { // read assuming you got the first byte return false; } - //now catching 1 in 256 edge-case where last byte of checksum is 0x42 too, being mistaken for first start byte, leading to byte shift and checksum mismatch - if (serial_dev->peek() == 0x42) { //peek if next byte might match first start byte - serial_dev->read(); // if yes last assumption was wrong, read again assuming you now got the start + // now catching 1 in 256 edge-case where last byte of checksum is 0x42 too, + // being mistaken for first start byte, leading to byte shift and checksum + // mismatch + if (serial_dev->peek() == + 0x42) { // peek if next byte might match first start byte + serial_dev->read(); // if yes last assumption was wrong, read again + // assuming you now got the start } - if (serial_dev->read() != 0x4d) { //read & check the second byte + if (serial_dev->read() != 0x4d) { // read & check the second byte return false; } - // Now the first two bytes are guaranteed to be 0x42 0x4d but have already been read + // Now the first two bytes are guaranteed to be 0x42 0x4d but have already + // been read buffer[0] = 0x42; buffer[1] = 0x4d; // Now read remaining 30 bytes if (serial_dev->available() < 30) { return false; } - for (uint8_t i=2; i<32; i++){ + for (uint8_t i = 2; i < 32; i++) { buffer[i] = serial_dev->read(); } } else {