ESP8266 TWI/I2C Master write: No delay between SCL going Low and SDA change.

ind03 commented 1 year ago

Basic Infos

[x ] This issue complies with the issue POLICY doc.
[ x] I have read the documentation at readthedocs and the issue is not addressed there.
[ x] I have tested that the issue is present in current master branch (aka latest git).
[ x] I have searched the issue tracker for a similar issue.
[ -] If there is a stack dump, I have decoded it.
[ x] I have filled out all fields below.

Platform

Hardware: LOLIN(WEMOS) D1 R2 & mini(esp8266_d1_mini), Platform=esp8266, Package=esp8266
Core Version: 3.1.2
Development Env: VisualStudio+VisualMicro
Operating System: Windows10

Settings in IDE

Module: LOLIN(WEMOS) D1 R2 & mini
Flash Mode: qio
Flash Size: 4MB
lwip Variant: v2 Lower Memory
Reset Method: nodemcu
Flash Frequency: 40Mhz
CPU Frequency: 80Mhz
Upload Using: SERIAL
Upload Speed: 921600

Problem Description

There is no delay between SCL going Low and SDA change in core_esp8266_si2c.cpp line 311. The result is a transition in only 125ns which may result in wrong data read or START/STOP conditions. It happens often with slow devices and long lines with high capacitance. To fix it, we only need an additional busywait (twi_dcount); after setting SCL low.

bool Twi::write_bit(bool bit)
{
    SCL_LOW(twi_scl);
    if (bit)
    {
        SDA_HIGH(twi_sda);
    }
    else
    {
        SDA_LOW(twi_sda);
    }
    busywait(twi_dcount + 1);
    SCL_HIGH(twi_scl);
    WAIT_CLOCK_STRETCH();
    busywait(twi_dcount);
    return true;
}

but should be

bool Twi::write_bit(bool bit)
{
    SCL_LOW(twi_scl);
    busywait (twi_dcount);
    if (bit)
    {
        SDA_HIGH(twi_sda);
    }
    else
    {
        SDA_LOW(twi_sda);
    }
    busywait(twi_dcount + 1);
    SCL_HIGH(twi_scl);
    WAIT_CLOCK_STRETCH();
    busywait(twi_dcount);
    return true;
}

d-a-v commented 1 year ago

Can you be more specific ? What is your device and what are the conditions when it does not work ?

I2C specs does not specify any delay between sda change following scl low: << The SDA signal can only change when the SCL signal is low – when the clock is high the data should be stable. >>

edit and capacitance <= 400pF

enjoyneering commented 1 year ago

I think ind03 is talking about tLOW (LOW period of the SCL), minimum 4.7usec. See UM10204 Table.10 for datails. After if-else should be SU;DAT (data set-up time), minimun 250nsec.

d-a-v commented 1 year ago

figure38 does not show minimum timing requirement between scl-low and sda. I understand that higher capacitance makes scl going low slower. I am not against trying to do something to make every setup working, but changes in this driver can break its stability very easily.

We could make Twi a template class, allowing another instantiation with different values. Or we could add more options to the arduino IDE menu / global defines to change defaults (easier / faster to do).

esp8266 / Arduino