Skip to content
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

I2C Race Condition when using Arduino's TwoWire #105

Open
wurzlphi opened this issue Oct 25, 2024 · 1 comment
Open

I2C Race Condition when using Arduino's TwoWire #105

wurzlphi opened this issue Oct 25, 2024 · 1 comment

Comments

@wurzlphi
Copy link

Info

Platform: ESP32 (M5Stack Core2)
ESP-IDF: 5.1.4
Arduino ESP32: 3.0.6
Connected Hardware: BME280 Environment Sensor on GPIO pins 21 and 22 using https://github.com/adafruit/Adafruit_BME280_Library

Issue

When using M5Unified/M5GFX with another I2C device on the system I2C bus (i.e. Wire1 / I2C_NUM_1 / GPIO 21+22 on M5Stack Core2) I run into a race condition when communicating with this device on a different thread.
As far as I can tell, this is because M5.update() uses lgfx::i2c::beginTransaction(...) internally to update the touchscreen, while an Arduino Wire based library uses Wire or Wire1 but neither have any knowledge of each other's locking mechanisms.

Example

This uses the sensor mentioned above, but I assume any I2C device would encounter these issues.

#include <thread>
#include <M5Unified.hpp>
#include <Adafruit_BME280.h>
#include <Wire.h>
#include <esp_pthread.h> // stack size

Adafruit_BME280 sensor;
std::thread sensorReader;

void setup() {
    // This is only required because I can't set the stack size through ESP-IDF currently
    // and it crashes in printf otherwise
    esp_pthread_t pthreadCfg = esp_pthread_get_default_config();
    pthreadCfg.stack_size *= 4;
    esp_pthread_set_cfg(&pthreadCfg);

    auto cfg = M5.config();
    M5.begin(cfg);

    Wire1.setPins(G21, G22);
    Wire1.begin();

    sensor.begin(0x77, &Wire1);
    sensorReader = std::thread([]() {
         for (;;) {
              M5.Lcd.clear();
              M5.Lcd.setCursor(0, 0);

              // This will print garbage values like a temperature of 180°C or -144°C
              M5.Lcd.printf("Temperature: %f\nHumidity: %f\nPressure: %f",
                  sensor.readTemperature(), sensor.readHumidity(), sensor.readPressure());
         }
    });
}

void loop() {
    M5.update();
}

Workaround

I have worked around this issue for now by enclosing M5.update() like this:

// ...

void loop() {
    Wire1.beginTransmission(0x38);
    M5.update();
    Wire1.endTransmission();
}

However, there are two problems with this approach:

  • This comes with a bit of a performance penalty because it's now locking two mutexes and performing some extra communication.
  • It's not a perfect solution since I'm only protecting against concurrent access from the touchscreen, but I'm assuming the PMIC and other ICs on the same bus also bypass Wire.h.

Is there a way for M5GFX to use Arduino's Wire library through beginTransaction() / endTransaction() that I'm missing?
Alternatively, would it be possible to make

i2c_context_t i2c_context[I2C_NUM_MAX];
public? I could then lock() and unlock() before and after any Arduino llbrary is using the I2C bus and at least address the second issue.

@lovyan03
Copy link
Collaborator

Hello, @wurzlphi
Thank you for your feedback.

Your concerns are valid. We are aware of that, but the Arduino TwoWire mechanism is simply difficult to use, so we have no plans to create a library that depends on it.
We are currently planning to separate and redesign the routines that handle these buses into a new library called M5HAL.
We intend to basically make it independent of Arduino.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants