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

[JDaughenbaugh]

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);
}

[mc-hamster]

I too have run into this bug. Following this issue.

Work around is to write to the full port with writeGPIOAB() rather than using digitalWrite()

[caternuson]

Is this issue happening with MCP23008/17 I2C and/or MCP23S08/17 SPI variants?

[mc-hamster]

I have the MCP23017 on my board.

Can't speak for the SPI variants.

I tried rolling back to 2.0.0, same problem there. I have not tried pre 2.x

[JDaughenbaugh]

Is this issue happening with MCP23008/17 I2C and/or MCP23S08/17 SPI variants?

We are using the I2C MCP23S17 also, but I believe the SPI version would have the same issue (same registers, just different interface).

[mc-hamster]

using 8x MCP23017 connected to an esp32.

If interested in the circuit, see:

https://github.com/mc-hamster/nova-nebula/tree/main/hardware/core

[caternuson]

OK, thanks. "open drain" was mentioned, so was wondering if something like the MCP23018 variant was being used.

Is there a simple sketch that can be used to demonstrate the issue for troubleshooting?

[mc-hamster]

Is there a simple sketch that can be used to demonstrate the issue for troubleshooting?

Catch:

https://github.com/mc-hamster/nova-nebula

[caternuson]

Not seeing an Arduino sketch anywhere in that repo?

[mc-hamster]

It's for platformio using the Arduino framework.

[caternuson]

Is there an Arduino sketch that can be used to recreate the issue?

[mc-hamster]

Is there an Arduino sketch that can be used to recreate the issue?

I'll make one for you after my work day.

[mc-hamster]

Is there an Arduino sketch that can be used to recreate the issue?

I hate giving untested code to reproduce a bug, but my (recently installed) copy of the Arduino IDE doesn't want to identify the serial ports on my Mac and gives the "NO PORTS DISCOVERED" error even though Visual Studio and everything else on my dev system works. I don't use the Arduino IDE and can't find any quick fixes.

Attached is an .ino file with based on the blink example which is functionally similar to what I was doing in my code. This should reproduce the issue. It does compile, so it should work. The relevant code snippets were lifted from the project I linked to you to earlier.

mcp23xxx_blink_bug.ino.zip

[caternuson]

Thanks. That looks nice and simple enough. What's the hardware setup to go with this? Just attach LEDs to each pin?

[mc-hamster]

The bus labelled "NEBULA" comes out of the MCP23017, into a ULN2308 darlington transceiver array, specifically the ULN2803ADWR by TI (branded chip, not off brand, reliable source) and the LED is attached to the transceiver array.

This is a still image of my board. You can see multiple LEDs turned on across the sequence. With the test sequence (what's in the .ino) I would expect that only one LED is on at any one time.

[caternuson]

Wired up LEDs w/ 220ohm resistors directly to a MCP23107 and the scanner sketch is running as expected. Only one LED is on a time.

Does the issue some how involve the darlington transceiver array?

[mc-hamster]

I get the proper behavior if I use writeGPIOAB(), so that tells me it's a software issue.

[caternuson]

How can the issue be reproduced locally for testing without a NOVA board?

[mc-hamster]

Let me dig into my parts bin and see if I can quickly make something and mailed to you. If I don't have the parts, I'll have a board made and populated just for you.

[mc-hamster]

How about this ... I have a Nova dev board that I'm not using. I can mail this out to you and include return post to get it back to me when you're done.

Will that work for you? That'd be easier and cheaper than sending you chips considering the ongoing component shortages.

[caternuson]

Thanks for the offer, but that hopefully shouldn't be required. The MCP's are very simple. If there's an issue setting IO with this library, it should be reproducible with some basic things on breadboard. Like buttons and LEDs.

[JDaughenbaugh]

Carter,

You are right that this is easy to reproduce with just buttons and LEDs.

Simple setup:
Set two of the MCP pins to be open drain, with internal pullup enabled. (Or all of the pins for that matter)
Set all outputs to 1. In this state the pins all float high w/ internal pullups. Reading them shows high levels.

Connect one pin to an LED & Resistor from 3.3V: LED turns on when output is set to 0, floats high when output set to 1.
Connect the other to a button to GND. Pin reads 0 when button is pushed, 1 when not.

To reproduce this bug:
Use digitalWrite to turn the LED on and off.
Use digitalRead to read the button.
If you do a digitalWrite to the LED pin while holding the button, the button pin will change from input w/pullup to driven low. So when releasing the button the button signal will not return high. This breaks the ability to read the button.
This is because the digitalWrite routine reads the input level on the button pin as low, then writes low back to the output register, causing the output level to change from 1 to 0.

Jason

[caternuson]

Thanks. This seems like a much simpler way to set this up. All pins are configured as inputs including the LED? What are the actual pinMode() calls for this?

Set two of the MCP pins to be open drain, with internal pullup enabled. (Or all of the pins for that matter)

[JDaughenbaugh]

Sorry, I was a bit too quick on my description. The MCP doesn't have an open drain mode - I was thinking of another part.

This should be a simple way to recreate this issue:

Use two pins. 3.3V to Resistor & LED on both (Two LEDs).

Setup:
// Set both to drive low- both LEDs will be ON.
mcp.pinMode(LED1, OUTPUT);
mcp.pinMode(LED2, OUTPUT);
mcp.digitalWrite(LED1, 0);
mcp.digitalWrite(LED2, 0);

Test:
// 1. Set LED1 pin to be INPUT_PULLUP. This will turn off LED1.
mcp.pinMode(LED1, INPUT_PULLUP);
// 2. Set LED1 pin to be OUTPUT. This will turn ON LED1.
mcp.pinMode(LED1, OUTPUT);
// 3. Set LED1 pin to be INPUT_PULLUP. This will turn off LED1 (again)
mcp.pinMode(LED1, INPUT_PULLUP);
// 4. digitalWrite LED2 pin to be high, still output. This will turn off LED2
mcp.digitalWrite(LED2, 1);
// 5. Set LED1 pin to be OUTPUT. (same as step 2) This should turn ON LED1.
mcp.pinMode(LED1, OUTPUT);

... but it won't turn on, because the digitalWrite of LED2 modified the output value of LED1.

(Sorry I don't have hardware with me right now so I can't run this myself)

[kisse66]

Perhaps this also has side-effect: Reading pin states will clear interrupt status. IF digitalWrite is reading pin state register it will clear pending interrupt. This is not expected I'd think?

Noticed this with a setup, where I drive LED display and a few buttons with MCP. LED is updated (multiplexed 7-seg) every few ms. Button pins are programmed for CHANGE interrupt to avoid constant polling over I2C. Mcu SW polls the state of INT-pin (no interrupt enabled) and when seen active goes and reads the button states. This was missing occasionally button presses like never saw button down since the INT gets cleared by digitalWrite before SW gets to see the INT?
If I comment out all the periodic digitalWrites this problem disappear, stable pin change detected as expected. WriteGPIO seems to be ok, but not digitalWrite.