x16() functions don't work

[BertvanRossem]

Can't be bothered to make a pr

bool MCP23S17::setPullup16(uint16_t mask)
{
  writeReg(MCP23S17_PUR_A, mask >> 8);
  //writeReg(MCP23S17_PUR_B, mask & 8); //this is the original line with an incorrect bit mask
  writeReg(MCP23S17_PUR_B, mask & 0xFF); //this fixes it

Otherwise only the 4th bit is actually written of the B register, so it does not set the correct bits

Problems are with the following functions: setPullup16 setPolarity16 write16 pinMode16

[BertvanRossem]

otherwise nice lib

[RobTillaart]

Thanks for pointing out this bug, I will fix it asap

[BertvanRossem]

thanks for the fix!

[RobTillaart]

Bug is also in MCP23017_RT lib,

Thanks for pointing out.

[RobTillaart]

Did a small investigation and in my projects I never used the X16() functions. No excuse but it explains why they were not detected before.

If there are other issues, or missing functionality let me know.

[BertvanRossem]

That explains it, as it is not a small bug i guess. A feature request would be a user-changable SPI instance. I modified the lib to accept a SPIClass pointer, as my board uses SPI1 on teensy 4.1. I could make a pr if you like, or send you the files. Like so:

MCP23S17(uint8_t select, SPIClass* spi, uint8_t address = 0x00);

MCP23S17::MCP23S17(uint8_t select, SPIClass* spi, uint8_t address)
{
  _address = address;
  _select  = select;
  _error   = MCP23S17_OK;
  mySPI = spi;
  _hwSPI   = true;
}

(added syntax highlighting for readability)

[RobTillaart]

That explains it, as it is not a small bug i guess.

It is a big one,

Wrote a test sketch that should detect it next time I do HW tests with this chip. (not released yet) MCP23S17_test.zip If you have time and a test setup, you might give it a try

---

looks like a useful extension

Please send (attach) a zip with the .h and .cpp so I can investigate the impact. Might need also a few lines in the readme.md too

[BertvanRossem]

If I remember it, I will take a look at your hardware test.

I have dumped the header and source in this pastebin. https://pastebin.com/HWq74hLV

I made a couple of changes as your library didn't support the spi bus change, but if that is integrated I could use library directly so disregard the other changes.

In my constructor the spi bus comes before the address, but I think that it is more userfriendly to move the spi to be the last parameter, since that will not often be used, like:

MCP23S17(uint8_t select, uint8_t address = 0x00, SPIClass* spi = &SPI);

[RobTillaart]

Thanks, there are not so much changes, only in the constructor and begin()

Need to see how to break the API as little as possible.

MCP23S17(uint8_t select, uint8_t address = 0x00, SPIClass* spi = &SPI);

sounds like a good step.

I will try to make a branch today based upon the 0.2.0 version.

[RobTillaart]

@BertvanRossem

Refactored the hardware SPI constructors to this: (SW SPI constructor did not change).

//  .h
  MCP23S17(uint8_t select, SPIClass* spi);
  MCP23S17(uint8_t select, uint8_t address = 0x00, SPIClass* spi = &SPI);   // default address 0x00 and default SPI

//  .cpp
MCP23S17::MCP23S17(uint8_t select, SPIClass* spi)  
{
  MCP23S17(select, 0x00, spi);
}

MCP23S17::MCP23S17(uint8_t select, uint8_t address, SPIClass* spi)
{
  _address = address;
  _select  = select;
  _error   = MCP23S17_OK;
  _mySPI   = spi;
  _hwSPI   = true;
}

This would allow to call 4 different constructors.

• MCP23S17(10); // select pin only
• MCP23S17(10, 7); // select pin + address pins
• MCP23S17(10, 7, &SPI1); // select pin + address pins + SPI port
• MCP23S17(10, &SPI); // select pin + SPI port

Technically two chips could use the same select pin, but another address. I assume that this is less used. The constructors would allow to setup such a configuration, (not tested or recommended)

[BertvanRossem]

Yeah, I think that will work for most application while keeping the original interface. Thanks!

[RobTillaart]

Develop branch created so you might test with the teensy if time permits. The existing examples compile - I have still no HW setup to test. (have to attend other projects now, will be back later today)

---

PR started

[RobTillaart]

took some time to get CI test stable. Now it is time for an evening walk ..

[BertvanRossem]

Hi, I tested the code and it compiles without warnings and works fine. For reference, I used the following constructor: MCP23S17 MCP(37, 0x4, &SPI1); // select pin + address pins + SPI port passing all the tests. So thanks for the changes, now I can use the library directly.

[BertvanRossem]

The only negative thing is that you cannot check if the device is actually attached, but that is inherent to SPI and this chip has no ID or WHOAMI register as far as I know, so I suppose I'm out of luck there

[RobTillaart]

The only negative thing is that you cannot check if the device is actually attached, but that is inherent to SPI and this chip has no ID or WHOAMI register as far as I know, so I suppose I'm out of luck there

thinking out loud How about writing some magic number to the pullup or polarity registers and be able to read it back? should give some confidence a device is attached. you could even iterate over all addresses with a different pattern?

Depends on what is attached if this method is "dangerous or damaging".

[RobTillaart]

Thanks for testing, it is late now, I'll have tomorrow another look at the code / documentation and will merge it.

[BertvanRossem]

Ah, that is a really smart strategy. I think I will implement that. Indeed depending on the application you cannot output anything, but changing the polarity register should not change anything as the chip powers up as all input. But I would not integrate this in begin() just to be on the safe side.

[BertvanRossem]

I made issue #13 for this problem