Bug in Wire timeout loop causes long delays in I2C read/write.

[greiman]

There is a bug these Wire.cpp member functions that causes delay(1) to be called for almost all transactions:

uint8_t TwoWire::read_from(uint8_t address, uint8_t* data, uint8_t length, unsigned int timeout_ms, bool sendStop)
uint8_t TwoWire::write_to(uint8_t address, uint8_t* data, uint8_t length, unsigned int timeout_ms, bool sendStop)

I tested with several I2C devices, a RTC and a OLED display.

Here are results of the RTC test with this program reading the time/date registers for a PCF8523 RTC.

I ran the program on a R4 WiFi.

#include "Wire.h"
// Read PCF8523 RTC registers
const uint8_t PCF8523I2cAdd = 0X68;  ///< I2C address.
const uint8_t PCF8523SecAdd = 0X03;  ///< Seconds register address.
void setup() {
  Serial.begin(9600);
  while (!Serial) {}
  digitalWrite(D13, LOW);
  pinMode(D13, OUTPUT);
  Wire.begin();
  Wire.setClock(400000L);
  Wire.beginTransmission(PCF8523I2cAdd);
  Wire.write(PCF8523SecAdd);
  digitalWrite(D13, HIGH);
  uint8_t rtn1 = Wire.endTransmission();
  digitalWrite(D13, LOW);
  digitalWrite(D13, HIGH);
  uint8_t rtn2 = Wire.requestFrom(PCF8523I2cAdd, 7);
  digitalWrite(D13, LOW);
  Serial.print("ss mm hh DD WD MM YY: ");
  for (uint8_t i = 0; i < 7; i++) {
    Serial.print(Wire.read(), HEX);
    Serial.print(i == 6 ? '\n' : ' ');
  }
  Serial.print("rtn1: ");
  Serial.print(rtn1);
  Serial.print(", rtn2: ");
  Serial.println(rtn2);
}
void loop() {}

Here is timing of Wire with the recent 400kHz fix. I use D13 HIGH on channel 2 to indicate time in Wire.

Here is timing after I did a hack to remove the delay. This is not meant to be a fix for the bug.

Note that the time to to read the RTC was reduced from 2,146 μs to 259 μs.

Here are the two hacked timeout loops:

in read_from at about line 464 of Wire.cpp

    timeout_ms = 10000;   // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
    while(timeout_ms > 0 && bus_status == WIRE_STATUS_UNSET) {
      timeout_ms--;
//      delay(1);  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
    }

In write_to at about line 493 of Wire.cpp

    timeout_ms = 10000;   //  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
    while(err == FSP_SUCCESS && timeout_ms > 0 && bus_status == WIRE_STATUS_UNSET) {
      timeout_ms--;
 //     delay(1);  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
    }

Edit: I tested on a R4 Minima and results are almost identical.

[alranel]

Thank you for this report. It is not that easy to fix it, but it's definitely something that needs to be improved.
Examples of how this can impact a real-world project can help setting a priority for this

[facchinm]

Being tackled in #46 , testing would be very appreciated 🙂