Skip to content

I2C fix READ of zero bytes hardware hang #2294

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

Closed
wants to merge 11 commits into from
Closed

I2C fix READ of zero bytes hardware hang #2294

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
daea578
Add APB change callbacks and move cpu code to own file
me-no-dev Dec 28, 2018
f7c56e3
Properly set esp_timer and FreeRTOS tick dividers
me-no-dev Dec 30, 2018
67d5ebc
Improve updated devisors
me-no-dev Dec 31, 2018
55c725f
No need to update REF_TICK yet
me-no-dev Dec 31, 2018
344c76d
Add initial handling for UART baud change
me-no-dev Dec 31, 2018
0532c88
fix uartWriteBuf and uartDetectBaudrate
me-no-dev Dec 31, 2018
7443fe2
trigger callbacks even when APB did not change
me-no-dev Dec 31, 2018
c31ea22
toggle UART ISR on CPU change
me-no-dev Jan 3, 2019
8b6483d
add XTAL freq getter and add cpu freq validation
me-no-dev Jan 7, 2019
276faf5
Support CPU frequency changes in I2C
Jan 8, 2019
946e4dc
I2C fix READ of zero bytes hardware hang
Jan 10, 2019
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 number Diff line number Diff 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

this can possibly be simplified to:

if(!i2c||(size && !buff))

NULL is defined as zero..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@atanisoft yes it could be obfuscated down to your concise example. I like to write code such that assumptions are minimized. I have found that I make more mistakes when I try to rely on assumptions.
The !i2c starts off wrong in my mind. !(not) starts right off describing a boolean operation, then up comes a pointer? So, my mind says "if not a pointer" What? What's NOT a pointer? Oh!, this construct means "if the value of i2c is not zero" well why don't you just write it that way, a few extra characters in a source file don't mean squat when the hard drive is measured in terabytes.

Chuck.

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){
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