search

UncleRus / esp-idf-lib

Component library for ESP32-xx and ESP8266
https://esp-idf-lib.readthedocs.io/en/latest/
1.38k stars 428 forks source link

i2c_dev mutex and multiple i2c_dev_t devices #436

Closed claud9999 closed 2 years ago

claud9999 commented 2 years ago

The issue

Looking at the mcp23x17 API, it appears to create its own i2c_dev_t struct. I have an I2C bus with multiple slaves but if I create additional i2c_dev_t structs, they won't share mutex's and will end up in contention on the master endpoint in a multithreaded environment.

I'm thinking the easiest is to have the mcp23x17 component take an i2c_dev_t pointer in and store it, plus the mcp's I2C address (more akin to the SPI interface) so that other components can use the same i2c_dev_t struct and use the I2C_DEV mutex api's on it.

Which SDK are you using?

esp-idf

Which version of SDK are you using?

5.x

Which build target have you used?

Component causing the issue

mcp23x17, i2c_dev

Anything in the logs that might be useful for us?

No response

Additional information or context

No response

Confirmation

claud9999 commented 2 years ago

FYI, as of May, the master branch of esp-idf says that i2c is thread-safe. Clearly you'll need to maintain i2c_dev_t for projects using older esp-idf; but might want to start marking as deprecated.

UncleRus commented 2 years ago

Each physical I2C device needs its own instance of i2c_dev_t struct. This struct contains device mutex. It is necessary for devices where one atomic transaction requires several operations on the bus. For example, read-change-write. In addition to these mutexes, there are special internal mutexes for physical ports. They do not allow using the same port at the same time from different tasks. Thus, the continuity of a single operation on within the physical port is ensured. Please read the README.

Feel free to open the issue again, with the example code and the description of the errors it causes.

FYI, as of May, the master branch of esp-idf says that i2c is thread-safe. Clearly you'll need to maintain i2c_dev_t for projects using older esp-idf; but might want to start marking as deprecated.

As for the thread-safety of the ESP-IDF master: this will allow you to get rid of port mutexes, while in no way affecting the device mutexes.