search

jenschr / TCA6408

Simple library for the TCA6408 I/O expander
MIT License
4 stars 2 forks source link

No need to wait after Wire.requestFrom(). #1

Open Koepel opened 6 years ago

Koepel commented 6 years ago

In the file "TCA6408/src/TCA6408.cpp", there is waiting after a Wire.requestFrom(). That can be removed.

This:

  uint8_t timeout=0;

  Wire.requestFrom(_address, (uint8_t) 0x01);
  while(Wire.available() < 1) {
    timeout++;
    if(timeout > I2CTIMEOUT) {
      return(true);
    }
    delay(1);
  }

could be this:

  Wire.requestFrom(_address, (uint8_t) 0x01);
  if(Wire.available != 1)
    return(true);

Explanation: Common-mistakes#1

jenschr commented 6 years ago

I'm a little unsure about this one. This bit of the code comes from the reference implementation that has been made for a family of TI MCU's. I've not tested this on that platform, but I've rewritten it so that it SHOULD work on Arduino. The only platform I've tested it on is the Particle platform (STM32) and it works well here. So while the Arduino platform handles this fine, I'm not sure it's worth updating this if it possibly breaks other platforms?

Koepel commented 6 years ago

It does not break on other platforms. The Wire libraries for other platforms are conform the documentation of the Arduino Wire library. That timeout has been made up, and copied many times. In my opinion, code that uses the Wire library in the wrong way is less compatible.

How will you know that the extra code is doing something at all ? You would have to disconnect the device just before the Wire.requestFrom(). That is unlikely to happen.

I think it is wrong to write code that is useless, but the code does no harm. When there was a I2C bus error, the Wire.available() returns zero. That code causes an extra delay (which is not needed), but that's all. Since the I2C bus should work, there should be no I2C bus error in the first place. Therefor it is not a big deal. It is up to you what to do with it.