Wire Library doesn't handle Repeated START commands in I2C slave mode

[GoogleCodeExporter]

What steps will reproduce the problem?

1.Create a Slave I2C device (Wire.begin(0xCA >> 1)).
2.On the I2C master, send a request that contains a repeated START command
3.Verify that the Arduino does not respond to the second request

Using the BusPirate, to generate a read request the following commands are 
given to write to the I2C slave to specify the register to read, followed by 
the actual read request.  Let's assume the 'Read' register is 0x00.

[OxCA 0x00 [0xCB rr]

English description of the above.

The start command [ is given, and we are writing to the device (LSB addres bit 
is 0, and the data we are writing is '0x00', the register we intend on reading. 
  After writing to the register, in order to do an atomic transaction to 
indicate we want to read from this register, the master creates another START 
request [.  This time, the request is a read (address LSB bit is now 1) and we 
are requesting two bytes.

What is the expected output? What do you see instead?

In the Arduino, the Wire.onReceive() callback is called correctly for the 
write, but due to addition of twi_stop() in the TW_SR_STOP case of the 
interrupt service routine, the master times waiting for the slave.  Note, 
sending a STOP bit is not a valid response in the two-wire protocol per the 
Atmel documentation (see page 237 of the Atmel documentation in the two-wire 
section for case 0xA0{TW_SR_STOP}).  The slave code should never be sending a 
STOP bit.  Inferring the original intent of the author from the provided 
comment, if the desire was to support clock stretching, the way to do that is 
to NOT clear the TWINT flag until the callback completes.  This is what the 
original code did, and the new code would do in twi_busRlease() if the 
two_stop() method and comment is removed.

What version of the Arduino software are you using? On what operating
system?  Which Arduino board are you using?

Arduino 1.0 software on OS/X, using multiple Arduino UNO boards.

Tests were done using a BusPirate 
(http://dangerousprototypes.com/category/bus-pirate/) as well as a National 
Instruments cRIO, both running as an I2C master attempting to do atomic reads 
of a register address on an I2C slave.  The code follows the same patterns as 
used in most I2C drivers, including those in LeJOS (http://www.lejos.org).

Using both, I verified the incorrect behavior with the stock 1.0 Arudino Wire 
library (really, the twi.c ISR), and with the two_stop() method removed, the 
Arduino responded correctly when requested from the two separate I2C masters.

Please provide any additional information below.

See comment from both wallclimber1 and HelenaRobotics (myself).

https://github.com/arduino/Arduino/commit/7392f8514de9d68659c744fb956c089f7b3932
ef#libraries/Wire/utility/twi.c

Original issue reported on code.google.com by natew...@gmail.com on 7 Mar 2012 at 3:05

[GoogleCodeExporter]

It's been over a half-year.  Is there anything I can provide to move this along?

Original comment by natew...@gmail.com on 4 Oct 2012 at 11:43

[GoogleCodeExporter]

have the same problem, verified with scope while comparing arduino-arduino (no 
repeated start) with arduino-p2020(repeated start). 

workaround is to comment out "twi_stop()" in  Wire/utility/twi.c:411

would really like to see this fixed.

Original comment by myownema...@gmail.com on 8 Nov 2012 at 7:47

[GoogleCodeExporter]

I brought up this issue over 3 years ago in this thread:

http://arduino.cc/forum/index.php/topic,46446.msg335953.html#msg335953

You'll have to read the thread in order to understand why nothing will be done 
about it.

Emily Pearson

Original comment by Emily.Ja...@gmail.com on 11 Nov 2012 at 12:18

[GoogleCodeExporter]

Emily, the fact of the matter is that a recent (??) change to the WIRE library 
to allow the MASTER to support repeated starts created this bug.  Prior to the 
change going in place (~14 months ago), a slave would work fine with repeated 
starts.  The bugfix is to remove the change (obviously untested) change to the 
WIRE library, which only affects SLAVE devices.

Original comment by natew...@gmail.com on 11 Nov 2012 at 1:22

[GoogleCodeExporter]

Oh, okay. The documentation is pretty sparse for the Wire library. A few 
examples might make clearer what it is capable of now. The email between Todd 
Krein and David Mellis in December of 2011 doesn't read like what you are 
implying.

Read this email thread:

http://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=3&cad=rja&ved=0CDwQ
FjAC&url=http%3A%2F%2Farduino.cc%2Fpipermail%2Fdevelopers_arduino.cc%2F2012-Janu
ary%2F006184.html&ei=jQOfUOrfIoSNygHf14GAAg&usg=AFQjCNFgr31PuEOWEYszUVoJ1tQ1qETd
kA

Original comment by Emily.Ja...@gmail.com on 11 Nov 2012 at 1:48

[GoogleCodeExporter]

The referenced thread is talking about using the Arduino as a master, and not 
as a slave, which is what the bugfix is about.

Original comment by natew...@gmail.com on 11 Nov 2012 at 4:25

[GoogleCodeExporter]

Sorry, I misunderstood.

Original comment by Emily.Ja...@gmail.com on 11 Nov 2012 at 5:00

[GoogleCodeExporter]

Why is this not fixed since 3 years?

Original comment by christia...@gmail.com on 14 Apr 2015 at 1:31

[GoogleCodeExporter]

3 years, and nothing has been done?  C'mon folks, this is an easy bug to fix, 
and the current code violates the I2C spec...

Original comment by nwilli...@sofi.org on 30 Apr 2015 at 1:44