Skip to content

Commit 36c369b

Browse files
TD-erearlephilhower
authored andcommitted
Double I2C read in one transaction skips a clock pulse (#5528) (#6654)
* Double I2C read in one transaction skips a clock pulse (#5528) See #5528 and the more elaborate [description](https://github.com/maarten-pennings/I2C-tool/blob/master/I2Ctest8266/README.md#how-to-fix) where @maarten-pennings did all the hard work, but as far as I could see, no PR was made. I also noticed some code duplication, which I moved to separate functions. According to [this documentation on I2C clock stretching](https://www.i2c-bus.org/clock-stretching/) it is not allowed to have some slave keep the clock low during transmission of a byte, only after an ack. So that's why it is not done in the while loop. But I wondered if there should be an extra delay between the sequence of pulses and the extra added clock "valley". See my comment in the code. Fixes #5528 * [I2C] Restore clock stretch functionality during transfer As requested here: https://github.com/esp8266/Arduino/pull/6654/files/8357a8c644244096cce1df30acd660c35d24a0e4#r339310509 * [I2C] Move duplicated clock stretch call to twi_scl_valley function
1 parent 5d0b9aa commit 36c369b

File tree

1 file changed

+24
-8
lines changed

1 file changed

+24
-8
lines changed

cores/esp8266/core_esp8266_si2c.cpp

+24-8
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ class Twi
128128
}
129129
}
130130

131+
// Generate a clock "valley" (at the end of a segment, just before a repeated start)
132+
void twi_scl_valley(void);
131133

132134
public:
133135
void setClock(unsigned int freq);
@@ -386,13 +388,16 @@ unsigned char Twi::writeTo(unsigned char address, unsigned char * buf, unsigned
386388
{
387389
write_stop();
388390
}
391+
else
392+
{
393+
twi_scl_valley();
394+
// TD-er: Also busywait(twi_dcount) here?
395+
// busywait(twi_dcount);
396+
}
389397
i = 0;
390398
while (!SDA_READ() && (i++) < 10)
391399
{
392-
SCL_LOW();
393-
busywait(twi_dcount);
394-
SCL_HIGH();
395-
WAIT_CLOCK_STRETCH();
400+
twi_scl_valley();
396401
busywait(twi_dcount);
397402
}
398403
return 0;
@@ -422,13 +427,16 @@ unsigned char Twi::readFrom(unsigned char address, unsigned char* buf, unsigned
422427
{
423428
write_stop();
424429
}
430+
else
431+
{
432+
twi_scl_valley();
433+
// TD-er: Also busywait(twi_dcount) here?
434+
// busywait(twi_dcount);
435+
}
425436
i = 0;
426437
while (!SDA_READ() && (i++) < 10)
427438
{
428-
SCL_LOW();
429-
busywait(twi_dcount);
430-
SCL_HIGH();
431-
WAIT_CLOCK_STRETCH();
439+
twi_scl_valley();
432440
busywait(twi_dcount);
433441
}
434442
return 0;
@@ -649,6 +657,14 @@ void ICACHE_RAM_ATTR Twi::onTwipEvent(uint8_t status)
649657
}
650658
}
651659

660+
void Twi::twi_scl_valley(void)
661+
{
662+
SCL_LOW();
663+
busywait(twi_dcount);
664+
SCL_HIGH();
665+
WAIT_CLOCK_STRETCH();
666+
}
667+
652668
void ICACHE_RAM_ATTR Twi::onTimer(void *unused)
653669
{
654670
(void)unused;

0 commit comments

Comments
 (0)