Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I2C fix READ of zero bytes hardware hang #2301

Merged
merged 1 commit into from
Jan 10, 2019

Conversation

stickbreaker
Copy link
Contributor

The i2c peripheral will hang if a READ request is issued with a zero data length. The peripheral
drops into a continuous timeout interrupt response. The STOP command can not be set out to the I2C
bus. The SLAVE device correctly ACK'd the address byte, with READ bit set, it has control of the SDA
pin. The ESP32 send out the next SCL HIGH pulse but, since the SLAVE is in READ Mode, and the First
bit it is sending happened to be a ZERO, the ESP32 cannot send the STOP. When it releases SDA during
the SCL HIGH, the pin does not change state. The pin stays low because the SLAVE is outputing a LOW!
The ESP32 drops into a perminent WAIT state waiting for SDA to go HIGH (the STOP).

esp32-hal-i2c.c

  • add databuff length checks to i2cRead() and i2cWrite()

 The i2c peripheral will hang if a READ request is issued with a zero data length.  The peripheral
drops into a continuous timeout interrupt response.  The STOP command can not be set out to the I2C
bus. The SLAVE device correctly ACK'd the address byte, with READ bit set, it has control of the SDA
 pin.  The ESP32 send out the next SCL HIGH pulse but, since the SLAVE is in READ Mode, and the First
bit it is sending happened to be a ZERO, the ESP32 cannot send the STOP.  When it releases SDA during
the SCL HIGH, the pin does not change state.  The pin stays low because the SLAVE is outputing a LOW!
The ESP32 drops into a perminent WAIT state waiting for SDA to go HIGH (the STOP).

**esp32-hal-i2c.c**
* add databuff length checks to `i2cRead()` and `i2cWrite()`
@@ -1628,6 +1631,9 @@ i2c_err_t i2cWrite(i2c_t * i2c, uint16_t address, uint8_t* buff, uint16_t size,
}

i2c_err_t i2cRead(i2c_t * i2c, uint16_t address, uint8_t* buff, uint16_t size, bool sendStop, uint16_t timeOutMillis, uint32_t *readCount){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if size is 0, why return an error? if size is not zero, but buf is null, then I understand, but size 0 is fine to return OK

Copy link
Contributor Author

@stickbreaker stickbreaker Jan 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning Ok means that the SlaveID is valid and the slave Answered the request. A zero length READ cannot be executed. So, the slave may not exist, or it may be busy.

A zero byte read creates an invalid I2C bus state. Depending on what the first bit the SLAVE responds with, the i2c peripheral may not be able to complete the transmission. A zero byte write is possible, but a zero byte read is not allowed. The zero byte write is used as a presence detect event.:

Wire.beginTransmission(ID);
uin8_t error = Wire.endTransmission();
if(error == 2) { // slave did not answer, is not present
  Serial.printf("NAK, no slave present at 0x%02X\n",ID);
} else if (error == 0) {// Slave ready for communications
  Serial.printf("ACK, slave present at 0x%02X\n",ID);
}

The following code is invalid:

uint8_t requestSize = 0;
uint8_t count = Wire.requestFrom(ID,requestSize);
if( count == requestSize) {// Slave is present and ready for communications
   Serial.printf("ACK, slave present at 0x%02X, received %dbytes\n",ID,count);
   Serial.printf(" requestFrom succeeded =%d(%s)\n",Wire.lastError(),Wire.getErrorText(Wire.lastError()));
   for (uint8_t i = 0 ; i< count; i++){
     Serial.print(Wire.read());
  }
  Serial.println():
}
else { // problem
   Serial.printf(" requestFrom Failed =%d(%s)\n",Wire.lastError(),Wire.getErrorText(Wire.lastError()));
}

change requestSize to a non zero value and the code works a expected,
The act of requesting a zero byte read must error out.

A Zero byte Write will return information, Whether a Slave device is active on the bus.

A Zero byte Read is meaningless. and will only succeed if the correct circumstances exist, else it will hang the i2c bus creating a error cascade that is confusing.

Chuck.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok...

@me-no-dev me-no-dev merged commit 9a7946e into espressif:master Jan 10, 2019
@me-no-dev me-no-dev deleted the patch-3 branch January 10, 2019 20:37
@valki2
Copy link

valki2 commented Jan 11, 2019

I cannot stress out how happy I am that you fixed this. i2c was driving me nuts and I had no clue where it came from :)

@asetyde
Copy link

asetyde commented Jan 11, 2019

I think is my issue .. sht21 not working rc4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants