Skip to content

Add lock to protect concurrent i2c transactions performed by differen… #8214

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
53 changes: 29 additions & 24 deletions libraries/Wire/src/Wire.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ TwoWire::TwoWire(uint8_t bus_num)
,_timeOutMillis(50)
,nonStop(false)
#if !CONFIG_DISABLE_HAL_LOCKS
,nonStopTask(NULL)
,currentTaskHandle(NULL)
,lock(NULL)
#endif
,is_slave(false)
Expand Down Expand Up @@ -412,15 +412,15 @@ void TwoWire::beginTransmission(uint16_t address)
return;
}
#if !CONFIG_DISABLE_HAL_LOCKS
if(nonStop && nonStopTask == xTaskGetCurrentTaskHandle()){
Copy link
Member

Choose a reason for hiding this comment

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

why discard this case? It has actually helped quite a bit, since we require strict use of the Wire API

log_e("Unfinished Repeated Start transaction! Expected requestFrom, not beginTransmission! Clearing...");
//release lock
xSemaphoreGive(lock);
}
//acquire lock
if(lock == NULL || xSemaphoreTake(lock, portMAX_DELAY) != pdTRUE){
log_e("could not acquire lock");
return;
TaskHandle_t task = xTaskGetCurrentTaskHandle();
if (currentTaskHandle != task)
{
//acquire lock
if(lock == NULL || xSemaphoreTake(lock, portMAX_DELAY) != pdTRUE){
log_e("could not acquire lock");
return;
}
currentTaskHandle = task;
}
#endif
nonStop = false;
Expand Down Expand Up @@ -452,15 +452,13 @@ uint8_t TwoWire::endTransmission(bool sendStop)
if(sendStop){
err = i2cWrite(num, txAddress, txBuffer, txLength, _timeOutMillis);
#if !CONFIG_DISABLE_HAL_LOCKS
currentTaskHandle = NULL;
//release lock
xSemaphoreGive(lock);
#endif
} else {
//mark as non-stop
nonStop = true;
#if !CONFIG_DISABLE_HAL_LOCKS
nonStopTask = xTaskGetCurrentTaskHandle();
#endif
}
switch(err){
case ESP_OK: return 0;
Expand All @@ -482,13 +480,26 @@ size_t TwoWire::requestFrom(uint16_t address, size_t size, bool sendStop)
return 0;
}
esp_err_t err = ESP_OK;
if(nonStop
#if !CONFIG_DISABLE_HAL_LOCKS
&& nonStopTask == xTaskGetCurrentTaskHandle()
#endif
){
TaskHandle_t task = xTaskGetCurrentTaskHandle();
if (currentTaskHandle != task)
{
//acquire lock
if(lock == NULL || xSemaphoreTake(lock, portMAX_DELAY) != pdTRUE){
log_e("could not acquire lock");
return 0;
}
currentTaskHandle = task;
}
#endif
if(nonStop){
if(address != txAddress){
log_e("Unfinished Repeated Start transaction! Expected address do not match! %u != %u", address, txAddress);
#if !CONFIG_DISABLE_HAL_LOCKS
currentTaskHandle = NULL;
//release lock
xSemaphoreGive(lock);
#endif
return 0;
}
nonStop = false;
Expand All @@ -499,13 +510,6 @@ size_t TwoWire::requestFrom(uint16_t address, size_t size, bool sendStop)
log_e("i2cWriteReadNonStop returned Error %d", err);
}
} else {
#if !CONFIG_DISABLE_HAL_LOCKS
//acquire lock
if(lock == NULL || xSemaphoreTake(lock, portMAX_DELAY) != pdTRUE){
log_e("could not acquire lock");
return 0;
}
#endif
rxIndex = 0;
rxLength = 0;
err = i2cRead(num, address, rxBuffer, size, _timeOutMillis, &rxLength);
Expand All @@ -514,6 +518,7 @@ size_t TwoWire::requestFrom(uint16_t address, size_t size, bool sendStop)
}
}
#if !CONFIG_DISABLE_HAL_LOCKS
currentTaskHandle = NULL;
//release lock
xSemaphoreGive(lock);
#endif
Expand Down
2 changes: 1 addition & 1 deletion libraries/Wire/src/Wire.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class TwoWire: public Stream
uint32_t _timeOutMillis;
bool nonStop;
#if !CONFIG_DISABLE_HAL_LOCKS
TaskHandle_t nonStopTask;
TaskHandle_t currentTaskHandle;
SemaphoreHandle_t lock;
#endif
private:
Expand Down