Problem with ESP32 wire

[japhillips87]

First of all, thanks for this wonderful library. I'm using the PCA9635 for a project and I'm controlling it through an ESP32. After putting a bunch of print statements in this library and the Wire library, I found that when using the PCA9635::begin() function for the ESP32 that accepts the SDA and SCL pin numbers, the _wire->begin(sda, scl) function calls the slave version of the begin() function in the wire library. I was able to get the master version of the begin function by changing the offending line to _wire->begin(sda, scl, (uint32_t)100000);. There is something wonky about the overloading and i'm not sure why it chose one over the other, but by passing the uint32_t frequency, it gets the right one. Now my test sketch is properly running on the ESP32 like it did with the same sketch on an arduino uno.

I'm not sure if this is the correct fix, but it allows me to move forward with my project. I'm happy to provide any more info that you might need to address this. I'm also happy to submit a PR with my change. Just let me know what you prefer.

[RobTillaart]

Thanks for supplying this issue.
(and super you found a solution!)

I did not know that difference, the issue never came up in any of my libraries so far.
I will dive into the code of the Wire library of the ESP32 to see if there are any notes mentioning this.

What I know (from the AVR) is that one need to provide an address to become an I2C slave.
See - https://www.arduino.cc/reference/en/language/functions/communication/wire/begin/

to be continued.

[japhillips87]

Here is the issue. The ESP32 board latest tagged version uses some overloading that seems to pick the wrong one if you don't pass that 3rd argument. I think that the issue is within the wire library and not within yours. It seems to be fixed in this PR, but a new version hasn't been tagged yet.

If you decide to change the line that I did, it still looks to be forward compatible though. But you should double check me on that. C++ is definitely not my strong suit. haha

[RobTillaart]

Thanks for this update - I have worked on other issues today (sorry)

Think the issue is partly caused by my code. The begin() function of the PCA9635 uses uint8_t for SDA and SCL.
Therefor the Wire lib thinks the second overload is a better match.

Furthermore your proposal "fiddles" with the frequency of the I2C bus that can be set for another device to e.g 400 KHz?
So there might be interference.

My thoughts.

Could you try to change the pins from uint8_t into int in the begin() function?

.h file

#if defined (ESP8266) || defined(ESP32)
  bool     begin(int sda, int scl, 
                     uint8_t mode1_mask = PCA9635_MODE1_ALLCALL, 
                     uint8_t mode2_mask = PCA9635_MODE2_NONE);
#endif

.cpp file

#if defined (ESP8266) || defined(ESP32)
bool PCA9635::begin(int sda, int scl, uint8_t mode1_mask, uint8_t mode2_mask)
{
  _wire = &Wire;
  if ((sda < 255) && (scl < 255))
  {
    _wire->begin(sda, scl);
  } else {
    _wire->begin();
  }
  if (! isConnected()) return false;
  configure(mode1_mask, mode2_mask);
  return true;
}
#endif

That should force to use the first overload and solve the issue too, without explicitly interfering with the frequency.

[japhillips87]

That did the trick. Would you like for me to fork it and submit a PR?

[RobTillaart]

Good to hear!
I'll do that, gives me a change to look if there are small open ends I can include.

[RobTillaart]

Thanks for testing BTW.

[japhillips87]

You are very welcome. I know how tough it is to test stuff without concrete steps to reproduce. I'm just happy to have this library so that I don't have to program it myself. I need to invert the output pins (I think it is a bit silly that they default to HIGH) so that i can drive some solenoids with it. With this change, it is super simple and works as expected.

[RobTillaart]

I need to invert the output pins (I think it is a bit silly that they default to HIGH) so that i can drive some solenoids with it.

Think this is a fast way to set them all LOW.

setAll(uint8_t v)
{
  uint8_t arr[16] = { v,v,v,v, v,v,v,v, v,v,v,v, v,v,v,v };
  PCA.writeN(0, arr, 16);
}

[RobTillaart]

PR is merged,