A note concerning 'REV D'

[Octet-nl]

As I understand it, the problem with GPA7 and GPB7 as input is not a recent one. All chips since 2005 seem to have the same die.

Microchip confirms this in their document https://www.microchip.com/product-change-notifications/#/18149/SYST-30BCIH037:

"NOTE: Please be advised that this is a change to the document only the product has not been changed."

So there is no revision D as far as the hardware is concerned, the bug was already present all these years. Only now they have adapted the documentation.

So if you plan a change in the workings of the Arduino library you can safely assume there is no revision D.

Regards,
Hans.

[RobTillaart]

Thanks,
I will add a link to this issue in the "revision D section" of the readme.md.

Furthermore I remove the dedicated REV D class from the Future->Could section, never got a request for that.

There is a PR pending so it will be in the description shortly,

Thanks again for reporting, appreciated.

[RobTillaart]

So if you plan a change in the workings of the Arduino library you can safely assume there is no revision D.

I interpret this as that there is no need to change the library.
Please correct me if I'm wrong.

[Octet-nl]

Well, no direct need. For my current application it is simply a if (pin==7) continue; , but in general I think it would be advisable to skip pins GPA7 and GPB7 in pin numbering for input. In the future I have probably forgotten about this issue and use them for input, possibly getting strange behavior. Especially as this seems to affect all revisions.

[RobTillaart]

Well, no direct need.

I will have a look again this evening, it might be simple to add in the library.

[RobTillaart]

It would mean that e.g. pinMode1 needs a guard

bool MCP23017::pinMode1(uint8_t pin, uint8_t mode)
{
  if (pin > 15)
  {
    _error = MCP23017_PIN_ERROR;
    return false;
  }
  if ((mode != INPUT) && (mode != INPUT_PULLUP) && (mode != OUTPUT))
  {
    _error = MCP23017_VALUE_ERROR;
    return false;
  }

  uint8_t dataDirectionRegister = MCP23x17_DDR_A;
  if (pin > 7)
  {
    dataDirectionRegister = MCP23x17_DDR_B;
    pin -= 8;
  }
  uint8_t val = readReg(dataDirectionRegister);
  if (_error != MCP23017_OK)
  {
    return false;
  }
  uint8_t mask = 1 << pin;
  //  only work with valid
  if ((mode == INPUT) || (mode == INPUT_PULLUP))
  {
    //  REV D
    //  if (pin == 7) return false;  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 
    val |= mask;
  }
  else if (mode == OUTPUT)
  {
    val &= ~mask;
  }
  //  other values won't change val ....
  writeReg(dataDirectionRegister, val);
  if (_error != MCP23017_OK)
  {
    return false;
  }
  return true;
}

Functions to investigate

• pinMode1(uint8_t pin, uint8_t mode)
• read1(uint8_t pin) no problem, could prevent an unneeded readReg(IOR);.
• setPullup(uint8_t pin, bool pullup)
• pinMode8(uint8_t port, uint8_t mask) ?? might need to mask the "7-bits" ??
• setPullup8(uint8_t port, uint8_t mask)
• pinMode16(uint16_t mask)
• setPullup16(uint16_t mask)
• enableInterrupt(uint8_t pin, uint8_t mode)
• disableInterrupt(uint8_t pin, uint8_t mode)
• enableInterrupt16(uint16_t mask, uint8_t mode)
• disableInterrupt16(uint16_t mask, uint8_t mode)

(so no small change)

mmmm,
lib does not have a getPinMode(), a getPinMode8() or a getPinMode16() to read the pinMode setting.
(and yes, no one ever asked for it)

[Octet-nl]

No, certainly no small change. And almost impossible to make it intuitive. I suggest you just print a warning that these functions are not guaranteed to work if GPA7 and GPB7 are used for input.
Before I read about this 'problem' I happily used both ports for input (in 1pin mode) and that worked fine so YMMV.
Some people suggest that this issue emerged because Microchip wanted to certify the expanders for automotive use. And that means far more rigorous testing.

[RobTillaart]

REV D is explicitly mentioned in the readme.md, it now includes your remarks (this issue) and I will add some comments in the .h file too. I will link this issue to the upcoming PR (0.8.0 version) so it will close when all is merged into master.

Thanks for your insights, appreciated.