Implementing Thread-Safe I2C

[kkloesener]

Centralising I2C-Functions
Implementing Thread-Safe I2C Communication when use with multiple I2C-devices

[tueddy]

Perhaps this old issue will resolve itself if we ever switch to Arduino 3:
espressif/arduino-esp32#8127

[r-schmidt]

Perhaps this old issue will resolve itself if we ever switch to Arduino 3: espressif/arduino-esp32#8127

I looked at the change for the esp and no, it won't fix this issue. The problem is that there is nothing making sure that only the one who triggered the transfer will be the one to get the requested data. The whole implementation of the TwoWire class for the esp32 (and probably Arduino in general) is seriously flawed and only works fine if you have a single core without multithreading or all i2c accesses are handled by a single thread.

Short example with 2 threads accessing 2 devices (addresses 0x20 and 0x40) on the same bus:
Thread 1:

uint8_t reg_val;
// read from register 0x00
beginTransmission(0x20); // takes lock
write(0x00);
endTransmission(false); // keeps lock because the parameter is false
requestFrom(0x20, sizeof(reg_val), true); // releases lock; and the last parameter is not even used...

// fetch value from buffer
reg_val = read();

Thread 2:

uint8_t reg_val;
// read from register 0x07
beginTransmission(0x40); // takes lock
write(0x07);
endTransmission(false); // keeps lock because the parameter is false
requestFrom(0x40, sizeof(reg_val), true); // releases lock; and the last parameter is not even used...

// fetch value from buffer
reg_val = read();

For simplicity we take a single core system i.e. only one thread is ever awake while the others are suspended.
Both threads run in parallel, let's assume Thread 1 gets the access first and does it's i2c transfer. After it is done (i.e. requestFrom() returned), Thread 1 gets suspended and Thread 2 can do its i2c transfer.
Now it depends which thread calls read() first. That is the one that gets the value Thread 2 requested while the other will read -1 (or nothing if it checks first with available()).

This is because there are 2 issues here:

1. requestFrom() doesn't take a pointer to a buffer to return the value(s) in but instead writes to a bus global buffer
2. the lock is released before the bus global buffer is emptied, so whoever calls read() first, gets it

If you have multiple values in the buffer, not just 1 like in my example, every thread could get parts of the last read buffer.

That being said, I don't like the approach here. You lose the return value from any function that gets wrapped.
The clean solution would be to implement i2c handling from scratch solving the fundamentally flawed design of the TwoWire class. But that would break compatibility with anything that isn't an esp32... Alternatively a worker thread handling all i2c transfers would be possible.