search

adafruit / Adafruit-MCP23017-Arduino-Library

Arduino Library for Adafruit MCP23017
Other
355 stars 204 forks source link

Add enableAddrPins() for MCP23X08 #88

Closed alexmaurer-madis closed 1 year ago

alexmaurer-madis commented 2 years ago

MCP23S08 hardware address pins A1 and A0 are not enabled by default on chip reset. (See Microchip datasheet page 14)

This pull request add the following function to the class Adafruit_MCP23X08 void enableAddrPins();

Otherwise the library does not work with HW address pins on MCP23S08.

README.md is modified accordingly.

ryanrsrs commented 1 year ago

Oh wow, this solves the problem I have spent all day debugging.

Thanks!

alexmaurer-madis commented 1 year ago

Oh wow, this solves the problem I have spent all day debugging.

Thanks!

I'm glad it helped you !

caternuson commented 1 year ago

Thanks for finding this. Looks like SPI HW address support was added for MCP23S17 here: https://github.com/adafruit/Adafruit-MCP23017-Arduino-Library/pull/78 but the MCP23S08 was left out.

Agree this is needed for the MCP23S08 despite what current README says: image

Do you know if the same errata mentioned in that pull request discussion for the MCP23S17 also applies to the MCP23S08? See Adafruit_MCP23X17::enableAddrPins() for ref.

Also, looks like the CI test are failing on clang formatting. Here's how to fix: https://learn.adafruit.com/the-well-automated-arduino-library/formatting-with-clang-format

ryanrsrs commented 1 year ago

I do not believe that errata affects the MCP23S08. The A2 pin is not present on the MCP23S08. Also, Microchip has not published an errata for the MCP23S08, so the optimistic conclusion is that it doesn't have that bug.

Alexandre's pull request (with single write, so no errata workaround) works perfectly on my project with 2x MCP23S08 sharing CS. One is at addr 0, the other at addr 1.

My chips are marked 2216H9K so they should be the latest silicon.

caternuson commented 1 year ago

Cool - thanks for testing and verifying on another setup.

In addition to running clang-format, please also remove the changes to the library.properties file. That'll be dealt with during release.

alexmaurer-madis commented 1 year ago

all done !

caternuson commented 1 year ago

Thanks. Looks like library.properties is still showing a diff. But we can deal with that directly after merging. Might just keep it anyway, since that seems like next logical ver.