I2C broken with version 2.0.1

[FStefanni]

Hardware:

Board: ESP32 Dev Module, ESP32 Wrover
Core Installation version: 2.0.1
IDE name: Arduino IDE
Flash Frequency: 80Mhz
PSRAM enabled: no
Upload Speed: 921600
Computer OS: Windows 10

Description:

Hi,

I have tested the two boards, with two I2C libraries, namely:

• SparkFun_u-blox_GNSS_Arduino_Library: a GNSS, by using the example sketch Example3_GetPosition
• Adafruit PN532: a RFID, with the sketch readMifareClassic

The tests have been performed with the core installation version 2.0.0 and 2.0.1.
All tests succeeded with version 2.0.0, but failed with version 2.0.1, since they stucked in the setup.
Since the only difference was the core version, I suppose version 2.0.1 introduced a bug in the I2C management.

Please note that I have also tested some basic functionalities of the Adafruit_SSD1306 lib (it is a display), and it worked, so maybe the possible
bug is related to reading from the I2C.

Can you please confirm whether you have the same issues?

Thank you,
regards.

Debug Messages:

Log with ESP32 Dev Module and lib SparkFun_u-blox_GNSS_Arduino_Library.

11:17:57.875 -> rst:0x1 (POWERON_RESET),boot:0x17 (SPI_FAST_FLASH_BOOT)
11:17:57.909 -> configsip: 0, SPIWP:0xee
11:17:57.909 -> clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
11:17:57.909 -> mode:DIO, clock div:1
11:17:57.909 -> load:0x3fff0030,len:1420
11:17:57.909 -> ho 0 tail 12 room 4
11:17:57.909 -> load:0x40078000,len:13540
11:17:57.909 -> load:0x40080400,len:3604
11:17:57.909 -> entry 0x400805f0
11:17:58.012 -> [setCpuFrequencyMhz(): PLL: 480 / 2 = 240 Mhz, APB: 80000000 Hz
11:17:58.084 -> SparkFun u-blox Example
11:17:58.084 -> [    31][E][Wire.cpp:313] beginTransmission(): Unfinished Repeated Start transaction! Expected requestFrom, not beginTransmission! Clearing...
11:17:58.084 -> [    34][E][Wire.cpp:313] beginTransmission(): Unfinished Repeated Start transaction! Expected requestFrom, not beginTransmission! Clearing...
11:17:59.171 -> [  1147][E][Wire.cpp:313] beginTransmission(): Unfinished Repeated Start transaction! Expected requestFrom, not beginTransmission! Clearing...
11:17:59.171 -> [  1149][E][Wire.cpp:313] beginTransmission(): Unfinished Repeated Start transaction! Expected requestFrom, not beginTransmission! Clearing...
11:18:00.300 -> [  2262][E][Wire.cpp:313] beginTransmission(): Unfinished Repeated Start transaction! Expected requestFrom, not beginTransmission! Clearing...
11:18:00.300 -> [  2264][E][Wire.cpp:313] beginTransmission(): Unfinished Repeated Start transaction! Expected requestFrom, not beginTransmission! Clearing...
11:18:01.384 -> u-blox GNSS not detected at default I2C address. Please check wiring. Freezing.

[atanisoft]

This is likely a bug in the libraries that are using Wire outside of the expected / supported patterns. Another Adafruit library was recently found to have a similar bug and likely these are impacted the same. @ladyada can you review?

[tanielkirikal]

I can confirm that Adafruit SCD30 library ( https://github.com/adafruit/Adafruit_SCD30 ) is also broken in 2.0.1.
esp32 2.0.0 version works.

[ladyada]

hiya we now use a wrapper library for i2c for sensors like the SCD30 https://github.com/adafruit/Adafruit_BusIO
which could be why some libraries work and some don't. that said, im v confident that the busio implementation is correct

however, the scd-30 does not use repeated start that we know of
https://github.com/adafruit/Adafruit_SCD30/blob/master/Adafruit_SCD30.cpp#L348
but it may have clock stretching (the PN532 definitely does)

iirc @me-no-dev has been doing the most work on i2c and repeated start.

[atanisoft]

@FStefanni it looks like the u-blox library you reference is doing multiple starts in a single block and may have a bug. There may also be bugs in the I2C code in this repository though.

[caternuson]

Just to add some info that this might be related to repeated start and/or clock stretching, tested behavior real quick with a BME280 (which does none of that) using library example. Also tested SCD30 with library example just to verify behavior above. Used a Metro ESP32-S2 and a HUZZAH ESP32.

BOARD	BME280	SCD30
Metro ESP32-S2	Y	Y
HUZZAH ESP32	Y	N

So it's not broken for everything. Hopefully this info can help narrowing in on issue.

[Jason2866]

I2C is not broken. Some stuff changed. Many many libs out there are not correctly done.
For Tasmota the maintainer had to change some to get it going with core 2.0.1.
The actual implementation is just not so error forgiving as the versions before!

[FStefanni]

Hi,

I reported the issue to the Sparkfun GNSS lib.
Since @ladyada is following this issue, I avoid to repeat the stuff in a official bug report for the Adafruit RFID lib
(unless @ladyada thinks doing this could be of any help).

Nevertheless it seems strange imho that so many libraries broke...

I hope it will be fixed quickly, since we use this stuff at work.

Regards.

[me-no-dev]

I2C is not broken. Some stuff changed. Many many libs out there are not correctly done.
For Tasmota the maintainer had to change some to get it going with core 2.0.1.
The actual implementation is just not so error forgiving as the versions before!

I agree with @Jason2866 here. The library in this repo works fine as long as it is used as per documentation (official Arduino documentation that is). Unfortunately many libs do the calls in some strange order, like wrapping requestFrom in beginTransmission/endTransmission and others. This could not work fine on ESP32 because of how the peripheral itself works and multithreading requirements. If anything is good about this situation is that it might just push the library maintainers to write proper Wire code.

[PaulZC]

Hi All,

I've been investigating the failures with the SparkFun u-blox library on v2.0.1 of the core, and I believe the key issue is actually I2C clock stretching (not I2C restarts - or a lack of proper Wire code).

I see similar badness with the SparkFun Qwiic Button. It uses clock stretching - but does not use restarts when communicating on I2C. More details are here:

sparkfun/SparkFun_u-blox_GNSS_Arduino_Library#77 (comment)

Right now, I believe the name of this issue should be changed to "I2C broken with version 2.0.1 on devices which use clock stretching".

Best wishes,
Paul

[nseidle]

This commit causes the break.

I'm still digging but a .endTransmission(false) causes buffer to be erased.

Code to replicate issue:

  Wire.begin();
  uint8_t deviceAddress = 0x42;
  Wire.beginTransmission(deviceAddress);
  Wire.write(0xB5);
  Wire.write(0x62);
  Wire.endTransmission(false);

  Wire.beginTransmission(deviceAddress);
  Wire.write(0x0E);
  Wire.write(0x30);
  Wire.endTransmission(true);

Below is v201

Below is at this commit (pre thread safe I2C commit):

I should have had my core debug turned on. This appears to be by design:

[E][Wire.cpp:313] beginTransmission(): Unfinished Repeated Start transaction! Expected requestFrom, not beginTransmission! Clearing...

[atanisoft]

Is it possible the condition for triggering that is inverted?

[garageeks]

Here is my little experience on this matter: I have tested 2.0.1 using these libraries and the devices work as expected, probably they don't use clock stretching. I have tested it just for some minutes though.

adafruit/Adafruit VEML6070 Library@^1.0.6
adafruit/Adafruit LIS3DH@^1.2.2
adafruit/Adafruit SHT31 Library@^2.0.0
adafruit/Adafruit MCP23008 library@^2.1.0

[nseidle]

@garageeks - I'm speaking at the edge of my knowledge here but testing or listing individual libraries could be counterproductive. I suspect you will find that devices that don't require repeated-starts (such as the OLED library) work just fine, whereas other devices that use repeated starts fail. If you can find a library that does use repeated starts, and works, that would be really interesting.

[nseidle]

The way I2C is designed in v2.0.1 no bytes are written to the bus with an endTransmission(false) and instead continues queuing bytes until a endTransmission(true). This seems wrong. I believe you need to put a i2c_master_start() onto the I2C que. I tried to implement this by passing sendStop into i2cWrite()

if(sendStop)
{
    ret = i2c_master_stop(cmd);
    if (ret != ESP_OK) {
        goto end;
    }
}
else
{
    ret = i2c_master_start(cmd);
    if (ret != ESP_OK) {
        goto end;
    }
}

This gets close as we have the correct repeated start condition, but we end up with some very weird fits and starts before the 2nd start command is issued:

This is not surprising as it clearly breaks the start/write/write/stop flow the driver expects. I'm trying to pin down where this is occurring in the IDF but I suspect it may need a i2c_master_stop(bool) function added to support repeated starts.

[me-no-dev]

@nseidle that was the same in the previous driver. It has to do with the fact that the ESP32 I2C peripheral sends data ONLY when the transaction ends with STOP, so in the case of repeated start, you endTransmission(false) and then you requestFrom to issue the repeated start and send everything to the device. The peripheral requires that and it's not just some weird decision we made.

[me-no-dev]

Also to say, the devices that I have and use repeated start (also some that others including @ladyada have tested) work fine. I do not have a device to test for stretching (where the device holds SCL low)

[ladyada]

@me-no-dev it might be good to pick up a clock-stretching chip to test against since they are often edge-cases in I2C implemenations. BNO055, PN532, CCS811 are ones you can find easily. i can send you some if there aren't any available where you are at.

[me-no-dev]

@ladyada I ordered BNO055 and CCS811. Will test them when they come and report back

[garageeks]

I have a CCS811 sensor and a PN532 card reader laying around, I can test it if you could provide some directions.

[TD-er]

Hm I don't recall that the CCS811 was particularly tricky to address.
I will try to hook them up on a node, along with the logic analyser attached to it.

Is it possible that the latest code now also has some functionality added to get out of a hung I2C bus?
Maybe clock stretching can also be triggering the procedure to clear the bus?
Does 2.0.1 now have a proper function to set a clock stretch limit? Or should we still interpret the "timeout" as a clock stretch limit? (have not found a similar function in the ESP32 code)
And what would be a good value for the mentioned sensors?

On ESP8266 it seems like the set value is in usec, right? A lot of sources found via Google suggest to set it to "1000" on the ESP8266, so a timeout of 1 msec should be the same.

[me-no-dev]

timeout has a slightly different meaning here. it's the amount of time to wait on the bus to complete the transaction. 8266 has software I2C library, so things there are different :)

[me-no-dev]

Clock stretching fix: #5910

[caternuson]

Just tested those changes with the ESP32 / SCD30 combo that was previously failing. It is now working.

[nseidle]

ESP32 with u-blox works great. Clock stretching looks fixed. And unless I'm mistaken, the writes with repeated start issue is also alleviated. I still understanding how fix #5910 affects this but either way, the communication looks great. Thanks @me-no-dev.

[me-no-dev]

It should not do anything for multiple repeated starts, but stretching and other timing related issues should be fixed :) merging...

[FStefanni]

Hi,

thank you for the fix.
I'll test the fix asap (right now I am very busy on other stuff).
I hope a new official release incorporating this fix will be made to
avoid people using official packages to get this error.

Regards.

[keenanjohnson]

Yes. I just ran into this same error. Any idea when a new official release will be out that incorporates this?

[me-no-dev]

in a couple of weeks