adafruit / Adafruit_ADS1X15

Driver for TI's ADS1015: 12-bit Differential or Single-Ended ADC with PGA and Comparator
Other
289 stars 301 forks source link

Infinite loop (and/or watchdog reset) if ADS1X15 device not present #64

Closed dstulken closed 3 years ago

dstulken commented 3 years ago

I'm testing some code after the 2.x library changes, and hit an interesting snag, which seems to be a regression caused by commit #59.

If an ADS1X15 device becomes disconnected, or simply isn't present on the I2C bus, instead of detecting this and failing gracefully (error return code, etc), the library will get stuck in an infinite while loop. This can be verified with the singleended example - the begin() function doesn't detect if the device is present, so the code proceeds to the first readADC_SingleEnded() call, which then stalls forever at the newly added "while (!conversionComplete())" section.

At that point, the behavior depends on the platform. It will either hang forever (such as the default singleended example), or in my case, where I was running this on the second core of an ESP32, the infinite loop triggered an infinite sequence of watchdog resets of the processor.

I see that the while loop was added as a more efficient replacement for the fixed conversion time delay, so aside from rolling that back, I'm not completely sure what the best solution(s) would be...

Probing for the I2C device at the specified address during the begin() function, then setting an "initOK" type of flag would allow the readADC function calls to bail (with an error return code) if the chip wasn't there. It's a logical spot for that type of check, and would probably help users who might have made wiring errors, etc. But it wouldn't handle an intermittent connection or a disconnection later on.

Perhaps a counter (or a for-loop) could be used to change that while() loop into something that would exit with an error code after 'x' number of repeated calls without a valid conversion result?

Both might be appropriate...

caternuson commented 3 years ago

Probably a good idea to add something to begin() to make sure the ADS is at least initially there. For intermittent connection and disconnect, that should be fixed in hardware by fixing wiring and not disconnecting while powered and running.

caternuson commented 3 years ago

Can you clarify the hardware state of the I2C bus you are trying to protect against. If there is nothing on the bus, then the bus is not even in a valid hardware state, since there are no pull ups.

caternuson commented 3 years ago

Closing due to lack of response. There's currently no way to protect against an I2C bus without anything attached. The underlying Wire library itself locks up: https://github.com/adafruit/Adafruit_BusIO/issues/44#issuecomment-812181697

Assuming I2C bus hardware is OK, a check could be done by sending a "general call" to address 0x00 with reset byte 0x06 and then checking the config register for the expected default 0x8583.

caternuson commented 3 years ago

Adding code snippet here for posterity. This seemed to work as long as I2C bus was valid (i.e. hardware connected).

bool Adafruit_ADS1X15::begin(uint8_t i2c_addr, TwoWire *wire) {
  m_i2c_dev = new Adafruit_I2CDevice(i2c_addr, wire);
  m_i2c_dev->begin();

  // reset using general call address
  Adafruit_I2CDevice *reset_dev = new Adafruit_I2CDevice(0x0, wire);
  buffer[0] = 0x06;
  reset_dev->write(buffer,1);

  // check config reg is expected default
  return (readRegister(ADS1X15_REG_POINTER_CONFIG) == 0x8583);
}
ladyada commented 3 years ago

sure, you can/should also if ! m_i2c_dev->begin(); return false to verify the i2c address is deteced

caternuson commented 3 years ago

Gotcha. Like a two step process - (1) something is at address and (2) that something is what is expected. Code snippet above is missing anything for (1).

ladyada commented 3 years ago

yep!