adafruit / Adafruit_PM25AQI

Arduino library for PMS* Air Quality Sensors
Other
36 stars 17 forks source link

Library deadlocks when using PM25AQI sensor with uart #14

Open mraleson opened 9 months ago

mraleson commented 9 months ago

When connecting to the particle sensor using the uart, the sensor will after about a minute fail continuously and not give any output. Removing this check allows the sensor to eventually resolve and continue functioning:

https://github.com/adafruit/Adafruit_PM25AQI/blob/ba3f10c1056ba88306cb22e6002396655bfa6e99/Adafruit_PM25AQI.cpp#L117-L121

It looks like the sensor won't send any more data until the bogus bytes are read. After they are consumed the sensor will continue forward, fail the checksum and re-stabilize.

This PR would help prevent the issue: https://github.com/adafruit/Adafruit_PM25AQI/pull/10

Although, there may be other situations that could cause this deadlock to arise, like a noisy serial connection. I think both of these changes might be worth making, the consequence being some reads might fail the checksum, but as it is the reads fail continuously anyway.

Also in the forums it looks like these issues have been what these two people refer to, and sounds like it caused them to return their sensors: https://forums.adafruit.com/viewtopic.php?t=200668&sid=ddf03cb5dfe45415ac3b76f599fcfa4e&start=15

mraleson commented 9 months ago

This is the approach I ended up taking. It's an alternative to #10 but incorporates those fixes and fixes this issue. It also simplifies the code (aside from reusing the buffer). Hopefully I didn't overlook anything / introduce any bugs but it seems to be working and I tested a few cases. Sorry its not a PR I just ran out of time!

uint16_t consumed = 0;
uint8_t buffer[32];
bool read_aqi(PM25_AQI_Data *data) {
  // check that the aqi data structure is allocated
  if (!data) {
    return false;
  }

  // recycle any failed buffer if possible, by finding a new start byte
  if (consumed == 32) {
    uint16_t i, j;
    for (i = 1; i < 32 && buffer[i] != 0x42; i++) {}
    consumed = 32 - i;
    for (j = 0; j < consumed; j++) {
      buffer[j] = buffer[i + j];
    }
  }

  // attempt to read a complete message
  while (Serial1.available() && consumed < 32) {
    buffer[consumed] = Serial1.read();
    consumed++;
  }

  // if unable to read enough bytes, exit and continue next time
  if (consumed < 32) {
    return false;
  }

  // check the start bytes
  if (buffer[0] != 0x42 || buffer[1] != 0x4d) {
    return false;
  }

  // compute check sum
  uint16_t sum = 0;
  for (uint8_t i = 0; i < 30; i++) {
    sum += buffer[i];
  }

  // the data comes in endian'd, this solves it so it works on all platforms
  uint16_t buffer_u16[15];
  for (uint8_t i = 0; i < 15; i++) {
    buffer_u16[i] = buffer[2 + i * 2 + 1];
    buffer_u16[i] += (buffer[2 + i * 2] << 8);
  }

  // put it into a nice struct :)
  memcpy((void *)data, (void *)buffer_u16, 30);

  // validate the checksum
  if (sum != data->checksum) {
    return false;
  }

  // success!
  consumed = 0;
  return true;
}