Skip to content

I2C fix READ of zero bytes hardware hang#2301

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.

Sign up for GitHub

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

Merged
me-no-dev merged 1 commit into from
Jan 10, 2019
Merged

I2C fix READ of zero bytes hardware hang #2301

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cores/esp32/esp32-hal-i2c.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1609,6 +1609,9 @@ i2c_err_t i2cFlush(i2c_t * i2c)
}

i2c_err_t i2cWrite(i2c_t * i2c, uint16_t address, uint8_t* buff, uint16_t size, bool sendStop, uint16_t timeOutMillis){
if((i2c==NULL)||((size>0)&&(buff==NULL))){// need to have location to store requested data
return I2C_ERROR_DEV;
}
i2c_err_t last_error = i2cAddQueueWrite(i2c, address, buff, size, sendStop, NULL);

if(last_error == I2C_ERROR_OK){//queued
Expand All@@ -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

@me-no-devme-no-devJan 10, 2019

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
ContributorAuthor

@stickbreakerstickbreakerJan 10, 2019•
edited
Loading

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)} elseif (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

@me-no-devme-no-devJan 10, 2019

Choose a reason for hiding this comment

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

ok...

if((size == 0)||(i2c == NULL)||(buff==NULL)){// hardware will hang if no data requested on READ
return I2C_ERROR_DEV;
}
i2c_err_t last_error=i2cAddQueueRead(i2c, address, buff, size, sendStop, NULL);

if(last_error == I2C_ERROR_OK){//queued
Expand Down