Adafruit-MCP23017-Arduino-Library icon indicating copy to clipboard operation
Adafruit-MCP23017-Arduino-Library copied to clipboard

Adafruit_MCP23XXX::digitalWrite should read/modify/write the output register, not the input register

Open JDaughenbaugh opened this issue 1 year ago • 24 comments

MCP23XXX_GPIO should be MCP23XXX_OLAT

This bug got us when using this library. The read/modify/write should be performing this operation on the output latch register, not the raw input register. This way tristate or open drain pins don't interfere with level modifications.

Example: A pin (say GPIO0) is open drain, with other external circuitry (AND function common for OK signals, etc). The GPIO is high, so "open" on the GPIO expander, but another part is holding the net low. This is all fine and expected.

Now if another pin is modified, say GPIO4 is set low, digitalWrite(4,0) this should have no effect on GPIO0. But in this case it does: This routine reads the input levels, which GPIO0 is low, then writes the register, thus driving GPIO0 low. Then the external circuitry stops holding GPIO0 low, and now the GPIO expander holds it low.

This is a common mistake when writing routines with I/O that can be tristated.

// /*! @brief Write a HIGH or a LOW value to a digital pin. @param pin the Arduino pin number @param value HIGH or LOW */ // void Adafruit_MCP23XXX::digitalWrite(uint8_t pin, uint8_t value) { Adafruit_BusIO_Register GPIO(i2c_dev, spi_dev, MCP23XXX_SPIREG, getRegister(MCP23XXX_GPIO, MCP_PORT(pin))); Adafruit_BusIO_RegisterBits pin_bit(&GPIO, 1, pin % 8);

pin_bit.write((value == LOW) ? 0 : 1); }

JDaughenbaugh avatar Nov 09 '22 19:11 JDaughenbaugh