arduino-esp32 icon indicating copy to clipboard operation
arduino-esp32 copied to clipboard

Poor performance of 2.0.2 probably due to I2C

Open FStefanni opened this issue 3 years ago • 22 comments

Board

ESP32 Dev Module

Device Description

  • DevKitC

Hardware Configuration

  • Digital expander I2C PCF8574
  • Display I2C SSD1306
  • ADS1115_WE I2C
  • A digital sensor (proximity -- single gpio) attached to the digital expander
  • An analog inclinometer attached to the ADS1115_WE
  • A I2C GPS

Version

v2.0.2

IDE Name

VS Code + arduino-cli

Operating System

Linux Debian Testing

Flash frequency

default

PSRAM enabled

no

Upload speed

default

Description

Hi,

I have performed some performance tests:

  • All the sensors are read in the loop()
  • If there is a change:
    • Create a JSON message
    • Send the message via MQTT

The results delay in receiving the MQTT message to the PC (which is attached to the same broker):

  • With all the I2C sensors, Arduino ESP32 2.0.0: almost immediate (less then 1 s)
  • With all the I2C sensors, Arduino ESP32 2.0.2: varying delay, between 2 s and 11 s
  • With all the I2C sensors, Arduino ESP32 2.0.3-RC1: varying delay, between 2 s and 11 s
  • Disabling the I2C, and sending forcing the send via an external gpio, Arduino ESP32 2.0.2: almost immediate (less then 1 s)

So:

  • The old version Arduino ESP32 2.0.0 works fine
  • The newer versions have a performance issue, very likely due to I2C

No workaround found. No idea if this is an Arduino ESP32 issue or a ESP-IDF issue.

Regards

Sketch

Not possible to share.

Debug Message

No message.

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

  • [X] I confirm I have checked existing issues, online documentation and Troubleshooting guide.

FStefanni avatar Apr 08 '22 15:04 FStefanni

Without a code sample, no help is possible. In general core 2.0.3-rc1 works well with i2c wifi and mqtt. We use all of this in our Project Tasmota and have instant mqtt message from connected i2c sensors.

Jason2866 avatar Apr 08 '22 18:04 Jason2866

Hi,

as already written it is not possible to share the code: it is a closed source project for the company I work with.

Your comment makes me think that maybe the issue could be about the I2C libraries we use, something like they are not well compatible with the newer Arduino ESP32 versions. Here the list of libraries:

  • PCF8574.h: digital expander
  • Adafruit_SSD1306.h: for the display which show the data locally
  • ADS1115_WE.h: for the analog sensor
  • SparkFun_u-blox_GNSS_Arduino_Library.h: for the GPS

Have you noticed any difference in performance switching between 2.0.0 and 2.0.2 (or 2.0.3-RC1)?

Regards

FStefanni avatar Apr 11 '22 06:04 FStefanni

Hello @FStefanni, did you face same issue on 2.0.3-rc1?

VojtechBartoska avatar Apr 11 '22 09:04 VojtechBartoska

We have seen no noticable performance differences between all core versiins.

Jason2866 avatar Apr 11 '22 09:04 Jason2866

@FStefanni we have switched to using ESP-IDF for I2C and because of that and also preventing threading issues, Wire now has a more strict requirements of how the API is being used. Many libraries do not use proper Wire calls and can end up in problems.

me-no-dev avatar Apr 11 '22 09:04 me-no-dev

Hi,

Hello @FStefanni, did you face same issue on 2.0.3-rc1?

yes, same issues.

@me-no-dev thank you for your reply. To me sounds warring that other people had I2C issues and just switched away from the Arduino layer... And if we switch to ESP-IDF, we have to write by hand the code of the sensors/peripherals libraries... not funny...

Regards

FStefanni avatar Apr 11 '22 12:04 FStefanni

@FStefanni currently Arduino Wire is a thin layer on top of that same ESP-IDF API that you would use. The problem is that people have for a long time written libraries where they use Wire wrong. The most common thing is wrapping Wire.requestFrom in Wire.beginTransaction/Wire.endTransaction. Most chips have a fairly simple I2C peripheral and are not running RTOS on multicore chips, but we are. So if a user has two tasks that want to access I2C devices, he will get into trouble if Wire is not thread-safe. So that is what we did. We added thread-safety and converted to ESP-IDF. Now things work just fine, as long as the Wire is used as it's supposed to. I know this has some negative implications currently for some users, but I hope that this will make the future better for everyone, as micros become more and more powerful.

me-no-dev avatar Apr 11 '22 13:04 me-no-dev

Hi,

if I understand correctly, I can check for beginTransmission()/endTransmission() (I suppose you mean this method, and not beginTransaction()/endTransaction() which do not seem to exist) in the libraries I use, ensuring that all the calls to write() and requestFrom() are enclosed, something as:

Wire.beginTransmission(...);
Wire.requestFrom(...);
Wire.endTransmission();

Is this what you correct? Or is the opposite, i.e. it must not be enclosed? Do I have to wrap also calls to read()?

Thank you, regards

FStefanni avatar Apr 11 '22 13:04 FStefanni

Wire.beginTransmission(...);
Wire.requestFrom(...);
Wire.endTransmission();

This is bad code. Here is correct code:

// Writing to I2C
Wire.beginTransmission(...);
Wire.write(...);
Wire.write(...);
Wire.endTransmission();

//Reading from I2C
Wire.requestFrom(...);
Wire.read(...);
Wire.read(...);

me-no-dev avatar Apr 13 '22 10:04 me-no-dev

@FStefanni Did you already solve your issue?

VojtechBartoska avatar Apr 22 '22 09:04 VojtechBartoska

Hi,

no, not solved. I have done few attempts but with no luck.

I wonder if other people have checked the performances and have found my same issue...

Regards

FStefanni avatar Apr 26 '22 06:04 FStefanni

@FStefanni are you able to retest this on v2.0.3? So far we cannot replicate your issue.

VojtechBartoska avatar May 18 '22 10:05 VojtechBartoska

Hi,

yes, but I need some time due to other projects deadlines (I am working on this at work). I hope to be able to update you within the next week.

Regards.

FStefanni avatar May 18 '22 12:05 FStefanni

Hi,

I have finally done some further investigation.

Actually, it seems an issue of ADC ADS1115_WE: at the moment we have no ADS1115_WE component, and the other parts of the circuit are going fine, with both 2.0.2 and 2.0.3.

The software library I use seems fine, so maybe it is an hw problem.

Maybe does it require a dedicated bus? For example, we have discovered that PN532 requires a dedicated bus on esp32 (https://github.com/adafruit/Adafruit-PN532).

I will test with ADS1115_WE as soon as the component arrives (we have ordered a copy).

Regards.

FStefanni avatar May 31 '22 11:05 FStefanni

thanks @FStefanni for testing, we will wait for your follow up.

VojtechBartoska avatar Jun 14 '22 13:06 VojtechBartoska

I can confirm that I2C starting from 2.0.1 Arduino Release is "slow" compared to older versions.

I use this function in order to read data from SHT21 sensors and i had to increase the TRANSACTION_TIMEOUT from 300ms to 2000ms in order to get the I2C transaction completed:

uint16_t HTU21D::readValue(byte cmd, uint8_t retry = 3)
{
    if (retry > 0) {
        //Request raw value reading
        _i2cPort->beginTransmission(HTU21D_ADDRESS);
        _i2cPort->write(cmd); //Measure value (prefer no hold!)
        uint8_t err = _i2cPort->endTransmission();

        if (err != 0)
            return ERROR_I2C_TRANSMISSION;

        delay(20); // account for conversion time for reading humidity

        uint32_t start = millis(); // start timeout
        while (millis() - start < TRANSACTION_TIMEOUT) {
            if (_i2cPort->requestFrom(HTU21D_ADDRESS, 3) == 3) {
                uint16_t rawValue = _i2cPort->read() << 8 | _i2cPort->read();
                uint8_t checksum = _i2cPort->read();

                if (checkCRC(rawValue, checksum) != 0)
                    return readValue(cmd, --retry);

                return rawValue;
            }
            delay(DELAY_INTERVAL);
        }
        return readValue(cmd, --retry);

    } else {
        return ERROR_I2C_SENSOR;
    }
}

rtu-dataframe avatar Jun 17 '22 09:06 rtu-dataframe

I'm now also running into issues with the latest ESP32 code where I2C is so much slower that it's actually causing issues. For ESPEasy it is the PN532 which is now taking way more than 10x as long to read a tag. screenshots with timing stats in reported issue N.B. this one needed to have clockstretching, but setting a timeout used to be enough to make this sensor work fine.

TD-er avatar Aug 10 '22 23:08 TD-er

Hi,

thanks @FStefanni for testing, we will wait for your follow up.

I am very sorry, but I am using Arduino for work, and just now I am super busy on other stuff. So I hoped to be able to give a quick feedback, but actually I do not know exactly when I will be able. I'll try asap.

Meanwhile, I see also other people had these issues... I hope they can give faster feedback.

Regards

FStefanni avatar Aug 11 '22 06:08 FStefanni

@TD-er Did the PN532 ever work reliable with i2c? In out trys it always got stuck (after a few days) with ESP8266 and ESP32. After we changed to use serial connection it is working.

Jason2866 avatar Aug 11 '22 08:08 Jason2866

@TD-er Did the PN532 ever work reliable with i2c? In out trys it always got stuck (after a few days) with ESP8266 and ESP32. After we changed to use serial connection it is working.

Yep it was working, but there's a lot of checks in the code to reset the unit.

I'm now looking into the traces using a logic analyzer and I'm not really surprised it is working so poorly. It is often doing clock stretching, effectively halting any I2C communication. Also it seems there is some capacitance on it which rounds the data pulses and it does seem to add quite a bit of noise.

image The bottom 2 traces are analog samples of the 2 in the same color above. Even the decoded traces show different data.

TD-er avatar Aug 11 '22 09:08 TD-er

OK, found something here.... Apparently the PN532 is a bit slow and thus it might miss the Wire.requestFrom right after sending a command. This then results in an almost exactly 1 sec wait where the clock is then shortly pulled down, followed by the data being shortly pulled down. Then it all works again.

So what I now do is I added a delay between sending the command and asking for data which seems to work just fine. But this suggests the Wire.requestFrom is blocking if the device does not respond. Where is this timeout defined? (I have set the Wire.timeOut to 20 as an alternative to the missing clock stretch function on ESP32) But this should be in msec and the code running on the ESP seems to be stuck for about a second here.

Just looking at the code: (esp32-hal-i2c.c)

esp_err_t i2cRead(uint8_t i2c_num, uint16_t address, uint8_t* buff, size_t size, uint32_t timeOutMillis, size_t *readCount){
    esp_err_t ret = ESP_FAIL;
    if(i2c_num >= SOC_I2C_NUM){
        return ESP_ERR_INVALID_ARG;
    }
#if !CONFIG_DISABLE_HAL_LOCKS
    //acquire lock
    if(bus[i2c_num].lock == NULL || xSemaphoreTake(bus[i2c_num].lock, portMAX_DELAY) != pdTRUE){
        log_e("could not acquire lock");
        return ret;
    }
#endif
    if(!bus[i2c_num].initialized){
        log_e("bus is not initialized");
    } else {
        ret = i2c_master_read_from_device((i2c_port_t)i2c_num, address, buff, size, timeOutMillis / portTICK_RATE_MS);
        if(ret == ESP_OK){
            *readCount = size;
        } else {
            *readCount = 0;
        }
    }
#if !CONFIG_DISABLE_HAL_LOCKS
    //release lock
    xSemaphoreGive(bus[i2c_num].lock);
#endif
    return ret;
}

Timeout is in millis, in an uint32_t Handed over to the function: timeOutMillis / portTICK_RATE_MS However portTICK_RATE_MS is essentially #define portTICK_PERIOD_MS ( ( TickType_t ) 1000 / configTICK_RATE_HZ ) with configTICK_RATE_HZ being 1000.

Thus: timeOutMillis / (1000/1000)

Not sure how this could lead to something like 0, but could it be that the timeout is essentially being ignored here somewhere?

TD-er avatar Aug 11 '22 10:08 TD-er

Can you please take a look on this issue @me-no-dev in the meantime? Thanks

VojtechBartoska avatar Aug 23 '22 11:08 VojtechBartoska

Status update:

After discussing this internally there is not much we can do. I2C now uses ESP-IDF API and we're not aware of any performance issues. Some external libraries are now not working properly due to implementation on previous version of I2C in ESP32 Arduino core.

VojtechBartoska avatar Nov 22 '22 15:11 VojtechBartoska