A clock phase correction in write_start(void) (I2C)

[xpalaa00]

Some devices require the data wire SDA to be held down at the moment while the clock wire is pulled down too to execute the start condition (e.g. devices using "TinyWires.h" library). This change follows a behaviour of Arduino Wire.h library, where the SCL signal is pulled down in half of period of start condition.

[TD-er]

Is this variation also allowed according to the standard?
Are there devices known that may not like this behavior? Or is it in general a good idea to do it like this?

[xpalaa00]

What I have looked around the I2C protocol, the declaration of "start and stop condition" is defined as pulling down or releasing SDA wire while SCL wire is released, nothing more, but a lot of diagrams in a protocol specification shows the SCL line is pulled down earlier than the data line is set to send a bit (when sending start flag) and is released earlier than the communication stops (when sending stop flag ... this one is correctly implemented in current protocol). There is one point in specification which can help, the "start condition" can be achieved by sending 0x01 byte (https://www.pololu.com/file/0J435/UM10204.pdf page 19). While transmission is running, the clock signal and data signal changes are delayed between themself by half of period (data bit is changed on falling edge of clock and transmitted on rising edge). So, current implementation of "start condition" is out of phase when sending the 1st bit (other ones are in phase) if it was done by sending 0x01 byte.

How I have described above, I have tried to communicate via I2C between ESP8266 and device what is using the TinyWires library (specificaly it was Attiny85 controller and library from https://playground.arduino.cc/Code/USIi2c/ -> https://github.com/rambo/TinyWire/blob/master/TinyWireS/ ). I have tried to resolve a reason, why Attiny85 answers only sometimes (rarely) by sending ACK, when calling it's address, in other tries just received NACK, and just only if frequency of the bus was slow. I have made a lot of changes in my own code, also tried example code to communicate with ESP8266, I have tried to look for any bugs in libraries I used, but nothing helped with rising of reliability of communication, solution came after moving the phase in start condition of the clock in ESP8266 I2C Master protocol. I don't know true cause, but I think there wasn't a lot of time between clock falling edge and the first sended bit, if data bit is logic one, it perhaps causes detection of stop condition on slower devices, or just it doesn't execute a start condition.

I have tried this modification also to communicate with some sensors and without any complications, although I'm not able to verify this change among every combination of devices, I think the communication standard isn't harmed. This modification is in accordance with Arduino I2C Master's implementation (picture above again) and delay calculation takes into consideration a possible odd number of entered value of delay, that could cause a dephasing.

[TD-er]

Found Understanding the I2C Bus - Texas Instruments

2.1.1 START and STOP Conditions
I2C communication with this device is initiated by the master sending a START condition and terminated by the master sending a STOP condition. A high-to-low transition on the SDA line while the SCL is high defines a START condition. A low-to-high transition on the SDA line while the SCL is high defines a STOP condition.

Just based on this explanation, it seems like some delay of the SCL is indeed needed.
What happens according to your logic analyzer screenshot is that SDA goes up while SCL may still be high and thus that would mean it is technically a STOP condition.
Depending on the capacitance in the wires and the used pull-up resistors, the SDA may rise in a shorter amount of time than the SCL may fall, which may cause the connected I2C devices to actually experience a STOP condition.

[xpalaa00]

So, my hypothesis was partially right, thank you for your working around.

Also I have been looking into TinyWires library a little deeper and I have founded another possible reason, why Attiny devices aren't compatible with current implementation related on unwanted call of stop condition. After start condition there is a check if the communication hasn't been terminated after 'while loop' checking if SCL wire is pulled down and SDA wire is still released (need to specify that Attiny devices has stop condition detection, but it isn't an interrupt vector but only a flag, so stop detection has to be realised by software). Upcomming condition tests if SCL is pulled down also until SDA is still pulled down, if not, the communication is terminated. This check is technically right, because it filters wire cut off situation for exaple, but the software realization of this check brings a delay that helps to cause this error, am I right? (rows 493-546 in https://github.com/rambo/TinyWire/blob/master/TinyWireS/usiTwiSlave.c )

But, if would be possible to use my modification as fix of this problem, I noticed on Thuesday, I thing, has failed a workflow test at Style and formatting point, because near dividion operators there were missing some spaces. It should be fixed now, I hope.

[d-a-v]

Given above TI documentation and write_stop() in current master, and despite I'm not an i2c specialist, I'd approve this PR after clarification about why dcount/2 is used instead of dcount (stop vs start, write_stop() vs write_start())

[xpalaa00]

In fact, I didn't want to modify a length of current start bit, because I wasn't sure, if it is possible, but if stop bit can be twice wider, why the start bit couldn't be too.

My variant of modification use dividion, but it is a good point, the dividion can cause error when delay is equal to 1. That cause a fall down of both signals at the same time and not executing a start condition.

Not using the dividion and making start condition wider by using plain delay without any mathematical operators may be a better way. Should I modify it by this way instead?

I noticed the stop bit contains clock stretching detection, I wanted to ask, if in the start bit it should be inserted too, but if I am right, clock stretching is not possible unless addressign proccess is completed (it is possible only if real data, not address, are received or transmitted what I read here: https://onlinedocs.microchip.com/pr/GUID-CA6B1F8F-E09D-4780-A52A-CBBF61902464-en-US-2/index.html?GUID-5CCAB0DB-28BD-4095-B2E2-2F3CF0FC6966 ).

[TD-er]

Clock stretching is done by the connected I2C devices, not the I2C master.
It is a way for the connected device to tell the master to slow down a bit.

So it does not make sense to have clock stretching in the start sequence.

[TD-er]

Don't forget to add the comments:

bool Twi::write_start(void)
{
    SCL_HIGH(twi_scl);
    SDA_HIGH(twi_sda);
    if (!SDA_READ(twi_sda))
    {
        return false;
    }
    busywait(twi_dcount);
    // A high-to-low transition on the SDA line while the SCL is high defines a START condition.
    SDA_LOW(twi_sda);
    busywait(twi_dcount);
    SCL_LOW(twi_scl);
    busywait(twi_dcount);
    return true;
}

bool Twi::write_stop(void)
{
    SCL_LOW(twi_scl);
    SDA_LOW(twi_sda);
    busywait(twi_dcount);
    SCL_HIGH(twi_scl);
    WAIT_CLOCK_STRETCH();
    busywait(twi_dcount);
    // A low-to-high transition on the SDA line while the SCL is high defines a STOP condition.
    SDA_HIGH(twi_sda);
    busywait(twi_dcount);
    return true;
}

[xpalaa00]

I've added a reason comment of additional delay too, if never minds?

[earlephilhower]

Looking through the code this effectively just adds twi_dcount setup delay for the SDA which seems safe to do. Existing code has the START sequence correct, but only a few ESP clock cycles between SCL-> and SDX->x transitions which may not be enough for some systems. I can't seem top find a good reference for tSETUP here, but half a clock seems like a good #.