Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mcspr merged 4 commits intoesp8266:masterfromxpalaa00:master
Nov 28, 2021

Conversation

@xpalaa00
Copy link
Contributor

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.

obrazek

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.
@xpalaa00xpalaa00 changed the titleA clock phase correction in write_start(void)A clock phase correction in write_start(void) (I2C)Nov 22, 2021
@TD-er
Copy link
Contributor

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
Copy link
ContributorAuthor

xpalaa00 commentedNov 23, 2021
edited
Loading

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 fromhttps://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
Copy link
Contributor

TD-er commentedNov 25, 2021
edited
Loading

FoundUnderstanding 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.

image

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.

d-a-v reacted with thumbs up emoji

@xpalaa00
Copy link
ContributorAuthor

xpalaa00 commentedNov 25, 2021
edited
Loading

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 inhttps://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
Copy link
Collaborator

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

@xpalaa00
Copy link
ContributorAuthor

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
Copy link
Contributor

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.

xpalaa00 reacted with thumbs up emoji

@TD-er
Copy link
Contributor

Don't forget to add the comments:

boolTwi::write_start(void){SCL_HIGH(twi_scl);SDA_HIGH(twi_sda);if (!SDA_READ(twi_sda))    {returnfalse;    }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);returntrue;}boolTwi::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);returntrue;}
d-a-v reacted with thumbs up emoji

@xpalaa00
Copy link
ContributorAuthor

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

Copy link
Collaborator

@earlephilhowerearlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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 #.

@mcsprmcspr merged commitd5444c4 intoesp8266:masterNov 28, 2021
hasenradball pushed a commit to hasenradball/Arduino that referenced this pull requestNov 18, 2024
* A clock phase correction in write_start(void)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.* Formatting of modification restyled* Removing mathematical operations from delay* Comments added
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@earlephilhowerearlephilhowerearlephilhower approved these changes

@d-a-vd-a-vd-a-v approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@xpalaa00@TD-er@d-a-v@earlephilhower@mcspr

[8]ページ先頭

©2009-2025 Movatter.jp