rambo / TinyWire

My modifications to TinyWire Arduino libs
284 stars 121 forks source link

TWI Slave does not work reliable #7

Closed rudi48 closed 10 years ago

rudi48 commented 10 years ago

Hello Rambo, Thank you for the I2C library. For several days I have tried to establish a communication between a Raspberry Pi and an ATtiny85-20 with TWI/I2C bus. The communication works in principle, but not reliable. I have documented that on: http://www.rudiswiki.de/wiki9/RaspberryPiWobbulator#A.2FD_converter.2C_software_Attiny85-20

My solution now is to use an ATmega on an Arduino Pro Mini module. I have tried that with an Arduino Nano3 module.

If you have an idea how to fix the ATtiny program, please let me know. Greetings, Rudolf

rambo commented 10 years ago

On 2013.12.15 15:33 , rudi48 wrote:

Hello Rambo, Thank you for the I2C library. For several days I have tried to establish a communication between a Raspberry Pi and an ATtiny85-20 with TWI/I2C bus. The communication works in principle, but not reliable. I have documented that on: http://www.rudiswiki.de/wiki9/RaspberryPiWobbulator#A.2FD_converter.2C_software_Attiny85-20

Oher people have also had trouble with RPI and TinyWire (I have not tried it), it seems the problem is with the I2C master implementation on RPI, it cannot handle slave streching the clock (usual problem with bit-banged implementations) and the ATTiny85 must stretch the clock since it's too slow (and does not have TWI module for proper hardware I2C support like the ATMegas have).

Also bus-pirate has bit-banged I2C master, before I realized this I spent days trying to figure out what was wrong in TinyWire, ended up writing https://github.com/rambo/I2C/blob/master/examples/i2crepl/i2crepl.ino to confirm that when the master implements the full I2C protocol spec correctly tinywire will also work correctly.

I should probably add this to the README.

/Rambo

rudi48 commented 10 years ago

Hello Rambo, Thank you for pointing it out. But what is the way to archive an improvement? Do you have an idea what to change on the Raspberry Pi? Regards, Rudolf

rudi48 commented 10 years ago

Hello Rambo, I just found, that the problem is well known: http://www.raspberrypi.org/phpBB3/viewtopic.php?f=44&t=13771 Maybe there is a software solution for it: http://www.advamation.com/knowhow/raspberrypi/rpi-i2c-bug.html Citation: the slave stretches the clock only at the end/directly after an I2C-read-ACK-phase (after reading ACK/NACK), but then by more than 0.5 clock periods.

Because I am not so fluent in C, and if you have the time, could you do me a favor and implement that 0.5 clock stretching. I could test your software. Best regards, Rudolf

rambo commented 10 years ago

On 2013.12.15 18:05 , rudi48 wrote:

http://www.advamation.com/knowhow/raspberrypi/rpi-i2c-bug.html Citation: the slave stretches the clock only at the end/directly after an I2C-read-ACK-phase (after reading ACK/NACK), but then by more than 0.5 clock periods.

Because I am not so fluent in C, and if you have the time, could you do me a favor and implement that 0.5 clock stretching. I could test your software.

Unfortunatly I don't fully understand what goes on in the actual implementation (https://github.com/rambo/TinyWire/blob/master/TinyWireS/usiTwiSlave.c), which is made by other people and heavily based on reference code from Atmel. Especially changing something that affects timing is going to be likely to cause subtle bugs.

Also: according to the latter link the stretch is only allowed at the end of a read, on ATTiny85 @8MHz TinyWire must stretch the clock also while it checks if the transmitted slave address belongs to it, and the RPI I2C cannot handle this so we're out of luck.

You could try running the ATTiny from a 16 or 20MHz crystal and see if the added speed helps. Also see if you can lower the I2C bus-speed on the RPI. As long as the ATTiny can do all the needed processing in less than 0.X (where X is a small number to be found experimentally) I2C clock periods the stretching should not cause issues on the master.

/Rambo

rudi48 commented 10 years ago

Hello Rambo, Thanks you for the hints. I will have a look for that. Best regards, Rudolf

rudi48 commented 10 years ago

Hello Rambo, Looking to the scope image, it appears, that we have a slave clock stretching of about 1 ms, at a clock period time of 25 us (40 KHz). To do something with an accuracy of 0.2 clock period time under that circumstances looks hopeless to me. Best regards, Rudolf

rambo commented 10 years ago

1ms sounds way too much (but I have not actually checked the numbers myself), did you try with the RPI or something with a less buggy HW support (like ATMega on Arduino) ?

rudi48 commented 10 years ago

Hello Rambo,

Sorry, I mixed the number. The slave clock stretching of about 50 to 100 us. I made now a measurement with the ATmega328 with 100 KHz I2C clock, which has no clock stretching. Please have a look at http://www.rudiswiki.de/wiki9/RaspberryPiWobbulator to see the actual scope pictures.

Best regards, Rudolf

rambo commented 10 years ago

On 2013.12.16 01:04 , rudi48 wrote:

Hello Rambo,

Sorry, I mixed the number. The slave clock stretching of about 50 to 100 us. I made now a measurement with the ATmega328 with 100 KHz I2C clock, which has no clock stretching. Please have a look at http://www.rudiswiki.de/wiki9/RaspberryPiWobbulator to see the actual scope pictures.

about ADCtiny85r.ino:

requestEvent() is called for each byte the master requests, you can only send one byte per requestevent.

I really need to code an example of the proper way of doing this one of these days... The short version is to have an array for the registers (as you would in a normal I2C device) and on requestevent only send out values from the registers as they're being requested (incrementing the register by one after each send, as is the usual convention).

Also doing multiple analogreads in the mainloop without calling for TinyWireS_stop_check() check in between them is bound to cause issues since analogread is slow. In fact the correct way would be to start the conversion and then letting the mainloop run as usual, checking once per iteration if the conversion is complete and in such case copy the value to the register. I would probably just use a running average, so add the value to previous one and divide by two

/Rambo

rudi48 commented 10 years ago

Hello Rambo, Thank you for taking care of the problem. I think the analog to digital conversion could be a good example for the slave mode. It is always difficult for me to read out the rules, I better learn by example. If you take your time to make an A/D converter example, I will be glad to test it. The average by 8 samples I need to reduce the noise. Best regards, Rudolf

rambo commented 10 years ago

On 2013.12.16 09:43 , rudi48 wrote:

Hello Rambo, Thank you for taking care of the problem. I think the analog to digital conversion could be a good example for the slave mode. It is always difficult for me to read out the rules, I better learn by example. If you take your time to make an A/D converter example, I will be glad to test it. The average by 8 samples I need to reduce the noise.

Now there is (untested, it compiles but that's about the extend of my current testing) an example in the repo: https://github.com/rambo/TinyWire/blob/master/TinyWireS/examples/attiny85_i2c_analog/attiny85_i2c_analog.ino

/Rambo

rudi48 commented 10 years ago

Hello Rambo, I needed a while to do the proper debug setup with TinyDebugSerial(). It looks like for me now, that the receive part works (analog value = 0x01CA), but not the request part.

When I request the analog value with the bus pirate, I get always: I2C>[8 2 [r r] I2C START BIT WRITE: 0x08 ACK WRITE: 0x02 ACK I2C START BIT READ: 0xFF READ: ACK 0xFF NACK I2C STOP BIT

I have set a debug flag inside the void requestEvent() function, but this flag is never seen in the main loop in case I request data with the bus pirate..

Do you have an idea what I am doing wrong?

Best Regards, Rudolf

rambo commented 10 years ago

On 2013.12.27 00:13 , rudi48 wrote:

Hello Rambo, I needed a while to do the proper debug setup with TinyDebugSerial(). It looks like for me now, that the receive part works (analog value = 0x01CA), but not the request part.

When I request the analog value with the bus pirate, I get always:

Bus pirate cannot handle clock stretching by the slave (at least v3 can't [unless a newer firmware has fixed this issue], this had me stumped for days back when I first started working on TinyWireS)

Try Arduino and https://github.com/rambo/I2C/blob/master/examples/i2crepl/i2crepl.ino just to be sure there are no clock issues.

You might want to do a stop and start instead of repeated start in case there is some issues with repeated start, I have vague recollections of something like it but can't test right now.

I have set a debug flag inside the void requestEvent() function, but this flag is never seen in the main loop in case I request data with the bus pirate..

And you're sure your flag is marked volatile ?

/Rambo

rudi48 commented 10 years ago

Hello Rambo, Thank you very much for your hints and patience. The meaning of "volatile" I did not know, I have added it to my flag variable: volatile int db_flag; But no success with catching a sign of the "request" function.

I got your sketch "i2crepl.ino to work. My analog channel is 3. Please have a look at the serial monitor output, I expected 0x01CA as analog value: sendAddress(0x8), stat=0x1 sendByte(0x0), stat=0x1 sendByte(0x83), stat=0x1 STOP sent, stat=0x1 START sent, stat=0x0 sendAddress(0x8), stat=0x0 sendByte(0x0), stat=0x0 START sent, stat=0x0 read 0x0, stat=0x18 STOP sent, stat=0x0 START sent, stat=0x0 sendAddress(0x8), stat=0x0 sendByte(0x2), stat=0x0 START sent, stat=0x0 read 0x0, stat=0x20 read 0x0, stat=0x30 STOP sent, stat=0x0

What I am doing wrong?

Best regards, Rudolf

rambo commented 10 years ago

On 2013.12.27 13:20 , rudi48 wrote:

Hello Rambo, Thank you very much for your hints and patience. The meaning of "volatile" I did not know, I have added it to my flag variable: volatile int db_flag;

volatile is used whenever the variable might be changed from an interrupt. It tells the compiler that the variable must always be read fresh instead of supposing a previously read value is still valid unless something the compiler knows about has changed the value (and the compiler cannot know about those interrupts)

But no success with catching a sign of the "request" function.

I got your sketch "i2crepl.ino to work. My analog channel is 3. Please have a look at the serial monitor output, I expected 0x01CA as analog value: sendAddress(0x8), stat=0x1 sendByte(0x0), stat=0x1 sendByte(0x83), stat=0x1 STOP sent, stat=0x1

nonzero status (usually) means something went wrong. Value <8 means a timeout, from the original library page http://dsscircuits.com/articles/arduino-i2c-master-library.html

return values for read() and write() functions will now return back where, in the communication sequence the timeout, if enabled, occurred. These new return values do not apply to the legacy Wire functions. return values for new functions that use the timeOut feature will now return at what point in the transmission the timeout occurred. Looking at a full communication sequence between a master and slave (transmit data and then readback data) there a total of 7 points in the sequence where a timeout can occur. These are listed below and correspond to the returned value:

1 - Waiting for successful completion of a Start bit 2 - Waiting for ACK/NACK while addressing slave in transmit mode (MT) 3 - Waiting for ACK/NACK while sending data to the slave 4 - Waiting for successful completion of a Repeated Start 5 - Waiting for ACK/NACK while addressing slave in receiver mode (MR) 6 - Waiting for ACK/NACK while receiving data from the slave 7 - Waiting for successful completion of the Stop bit

All possible return values: 0: Function executed with no errors 1 - 7: Timeout occurred, see above list 8 - 0xFF See datasheet for exact meaning

START sent, stat=0x0 sendAddress(0x8), stat=0x0 sendByte(0x0), stat=0x0 START sent, stat=0x0

Here you must send the slave read address (ie 0x9).

read 0x0, stat=0x18 STOP sent, stat=0x0 START sent, stat=0x0 sendAddress(0x8), stat=0x0 sendByte(0x2), stat=0x0 START sent, stat=0x0 read 0x0, stat=0x20 read 0x0, stat=0x30

These status codes don't make much sense (check the ATmega328 datasheet able 22-2 "Status codes for Master Transmitter Mode") in read context but probably the fact that no slave address was sent plays a role.

STOP sent, stat=0x0