I2C changes for clock stretch and missing pulse before repeated start

[maarten-pennings]

@devyte asked me in ticket 5528 to submit my suggested fix for I2C; it also obsoletes the fix for 5340.
The rest of this ticket is copied from my github project, which provides a test framework fro ESP8266 I2C. @devyte suggests "The two sketches (see github) should be added as examples in the Wire lib."

---

Clearly, the I2C implementation has some bugs around clock stretching.

I am no longer happy with my first fix in issue 5340.

The code I added was clock stretch detection code in a while loop. I now believe that the while loop
itself is already an exception handler. The loop checks if SDA is low at the end of an I2C segment.
If SDA is low, the SCL is pulsed up to 10 times. This looks like the bus clear in the I2C
spec um10204.

I now believe the problem is actually earlier: namely the behavior between two segements,
i.e. just before the repeated start.

My new suggested fix for both issues is to add a clock pulse before a repeated start.
We needto add a function for that and call it from two locations.
You can do this yourself. Open the directory with core libaries (on my pc they are in
C:\Users\mpen\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\2.4.2\cores\esp8266)
and edit core_esp8266_si2c.c.

First we add a function

// Generate a clock "valey" (at the end of a segment, just before a repeated start)
void twi_scl_valey( void ) {
  SCL_LOW();
  twi_delay(twi_dcount);
  SCL_HIGH();
  unsigned int t=0; while(SCL_READ()==0 && (t++)<twi_clockStretchLimit); // Clock stretching
}

and then fix the two lines tagged Maarten in twi_writeTo()

unsigned char twi_writeTo(unsigned char address, unsigned char * buf, unsigned int len, unsigned char sendStop){
  unsigned int i;
  if(!twi_write_start()) return 4;//line busy
  if(!twi_write_byte(((address << 1) | 0) & 0xFF)) {
    if (sendStop) twi_write_stop();
    return 2; //received NACK on transmit of address
  }
  for(i=0; i<len; i++) {
    if(!twi_write_byte(buf[i])) {
      if (sendStop) twi_write_stop();
      return 3;//received NACK on transmit of data
    }
  }
  if(sendStop) twi_write_stop();
    else twi_scl_valey(); // Maarten
  i = 0;
  while(SDA_READ() == 0 && (i++) < 10){
    SCL_LOW();
    twi_delay(twi_dcount);
    SCL_HIGH();
    // Maarten unsigned int t=0; while(SCL_READ()==0 && (t++)<twi_clockStretchLimit); // twi_clockStretchLimit
    twi_delay(twi_dcount);
  }
  return 0;
}

and smilarly the two lines for twi_readFrom()

unsigned char twi_readFrom(unsigned char address, unsigned char* buf, unsigned int len, unsigned char sendStop){
  unsigned int i;
  if(!twi_write_start()) return 4;//line busy
  if(!twi_write_byte(((address << 1) | 1) & 0xFF)) {
    if (sendStop) twi_write_stop();
    return 2;//received NACK on transmit of address
  }
  for(i=0; i<(len-1); i++) buf[i] = twi_read_byte(false);
  buf[len-1] = twi_read_byte(true);
  if(sendStop) twi_write_stop();
    else twi_scl_valey(); // Maarten
  i = 0;
  while(SDA_READ() == 0 && (i++) < 10){
    SCL_LOW();
    twi_delay(twi_dcount);
    SCL_HIGH();
    // Maarten unsigned int t=0; while(SCL_READ()==0 && (t++)<twi_clockStretchLimit); // twi_clockStretchLimit
    twi_delay(twi_dcount);
  }
  return 0;
}

[devyte]

@maarten-pennings what I meant was to put the proposed changes in a Pull Request, so that they can be discussed/reviewed/tested.
Can you handle it, or do you need help?

[devyte]

Closing in favor of #5528, where the discussion started, and where there is more info, including links to the info provided here.