OpenServo.com Forum Index OpenServo.com
Discussion of the OpenServo project
 
 FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

TWI/I2C Problems
Goto page 1, 2  Next
 
Post new topic   Reply to topic    OpenServo.com Forum Index -> Software
View previous topic :: View next topic  
Author Message
mpthompson



Joined: 02 Jan 2006
Posts: 650
Location: San Carlos, CA

PostPosted: Wed Jan 25, 2006 9:53 pm    Post subject: TWI/I2C Problems Reply with quote

Andy sent me the following over IM:

Quote:
Andrew Lippitt: I've been looking very closely at the read write errors that I'm getting on the I2C bus. The reads from the servo often yield data where at some random bit, all remaining bits are 1. So when I'm expecting 0x0200 I receive 0x02FF. While this particular one is the most frequent, it seems like it might be able to start on any bit, not just byte boundries. I've seen 0x0201, 0x05FF. As for the writes, there is a different problem. The value that is written is often bit shifted right by 1. I seen other values I can't make sense of, but may be things like addresses or other data I've not recognized yet that has been shifted by some other amount. This happens with both the old TWI code and the new. I've varied the clock speed quite a bit and introduced inter-byte delays to no effect.


Any one else have thoughts on this.

We tracked down an issue with the SDA pull-up resistor being enabled on the servo that we thought could cause problems, but further testing from Andy indicated the problem lies elsewhere.

Andy is going to continue to poke at this to try and find the culprit. I'll start working on this on Thursday if Andy hasn't found anything by then.

My leading theory is line noise caused by the h-bridge/motor as I've noticed similar issues when the motor is active. However, Andy's testing indicates the problem exists even when PWM is turned off. Maybe we are chasing multiple issues.

-Mike
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
andylippitt
Site Admin


Joined: 02 Jan 2006
Posts: 155
Location: Denver, CO

PostPosted: Wed Jan 25, 2006 10:27 pm    Post subject: Reply with quote

At Mike's suggestion I poked around with the ADC interrupts. By poking around I mean completely and totally breaking. As he speculated, this cleared up the I2C issues. Seems there's some interrupt priority issue or a peripheral conflict. I'll keep poking (breaking). Smile
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
andylippitt
Site Admin


Joined: 02 Jan 2006
Posts: 155
Location: Denver, CO

PostPosted: Wed Jan 25, 2006 11:37 pm    Post subject: Reply with quote

Ah-HA! We beat it. There's a new twi.c checked in that clears it up.
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
mpthompson



Joined: 02 Jan 2006
Posts: 650
Location: San Carlos, CA

PostPosted: Thu Jan 26, 2006 8:53 am    Post subject: Reply with quote

Andy, I checked in some further enhancements to pwm.c to correctly disable and restore interrupts when setting the PWM signals coming from PortB output pins PB1 and PB4. I'm 99% certain these changes shouldn't cause your I2C problem to reappear, but let me know if you see any issues again.

Thank you for all your help in tracking down this issue.

-Mike
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
andylippitt
Site Admin


Joined: 02 Jan 2006
Posts: 155
Location: Denver, CO

PostPosted: Fri Jan 27, 2006 6:15 pm    Post subject: Reply with quote

After going back and forth a few times, we settled on what seems to be a pretty solid build. With the motors turned off, there are almost zero errors in communication. With them on, the noise is manageable. To help cope with this noise that I doubt we'll find a way to completely eliminate, I stole back all those image size gains Mike made, by adding read and write transactions with checksums. These live along side the regular addressed reads and writes.

Since I'm making up my I2C protocol graph format here, I'll show standard read/writes for comparison.

A Standard read goes as follows:
Code:

Start, I2CAddress, RegisterAddress, Stop
Start, I2CAddress+READ, Read[0], Read[1] ... Read[Count], Stop

A ""checked read"":
Code:

Start, I2CAddress, TWI_CMD_CHECKED_TXN, ByteCount, RegisterAddress, Stop
Start, I2CAddress+READ, Read[0], Read[1] ... Read[Count], Read[Checksum], Stop

A Standard write:
Code:

Start, I2CAddress, RegisterAddress, Write[0], Write[1] ... Write[Count], Stop

A ""checked write"":
Code:

Start, I2CAddress, TWI_CMD_CHECKED_TXN, ByteCount, RegisterAddress, Write[0], Write[1] ... Write[Count], Write[Checksum], Stop

During the checked write, if the checksum does not match, the servo wil NAK when receiving the checksum byte.

These are simple checksums, start at zero/add/wrap at 256. They are prone to added or dropped zero bytes, byte reordering or multiple errors that cancel each other out. Both checked transactions are limited to 16 data bytes. If the write checksum comparison fails, the registers will not be committed to memory and the buffer discarded.
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
mpthompson



Joined: 02 Jan 2006
Posts: 650
Location: San Carlos, CA

PostPosted: Fri Jan 27, 2006 9:31 pm    Post subject: Reply with quote

I asked Andy to just use 16 bytes for the checked read/write commands for right now as we are getting tight on SRAM -- the ATtiny45 only has 256 bytes of SRAM. We can increase the buffer size once we are able to move to the ATtiny85 with 512 bytes of SRAM.

-Mike
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
mpthompson



Joined: 02 Jan 2006
Posts: 650
Location: San Carlos, CA

PostPosted: Sat Jan 28, 2006 10:10 am    Post subject: Reply with quote

I took Andy's checksum code and rearranged it so that it is now enabled or disabled at compilation time using a TWI_CHECKED_ENABLED define near the top of the file. With the checksum code we are very close to the end of the 3K of application Flash space, but with it disabled we get back nearly 300 bytes for experiments in other parts of the code.

I may have broke something in the code while making these changes. I'll verify them over the weekend and hopefully get fixes in before it causes a problem for anyone.

-Mike
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
mpthompson



Joined: 02 Jan 2006
Posts: 650
Location: San Carlos, CA

PostPosted: Sat Jan 28, 2006 6:16 pm    Post subject: Reply with quote

I backed Andy's original changes back into CVS as I ran into problems with my changes to them. I'll be looking at this today.

-Mike
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
mpthompson



Joined: 02 Jan 2006
Posts: 650
Location: San Carlos, CA

PostPosted: Sat Jan 28, 2006 8:06 pm    Post subject: Reply with quote

OK, I think I was able to fix everything. As best I can determine the stack growing down from the top of SRAM collided with global variables causing all sorts of nastiness. I reduced the size of the global variables by reducing the register array from 64 to 48 bytes and reducing the size of some other buffers. I also made a few other changes to gain back some Flash space as well.

I believe the code checked into CVS at this time should be functioning correctly. I'll be monitoring the forum in case anyone finds anything that still looks broken.

-Mike
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
andylippitt
Site Admin


Joined: 02 Jan 2006
Posts: 155
Location: Denver, CO

PostPosted: Sat Jan 28, 2006 8:38 pm    Post subject: Reply with quote

I think the problem is in your reworking of my state vars. I was using TWI_DATA_STATE_CHECKED_COUNTING as your revised TWI_DATA_STATE_CHECKED_DATA, but on line 181 in twi_read_data() your using my definition instead of your updated one.

My only other concern is the default register read in twi_read_data(). It will be called with twi_address advanced one beyond the intended read when it comes time to send the checksum. The value is discarded, but it seems like it's waiting to become a problem if more safetys are built into the register reading.
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
andylippitt
Site Admin


Joined: 02 Jan 2006
Posts: 155
Location: Denver, CO

PostPosted: Sat Jan 28, 2006 8:41 pm    Post subject: Reply with quote

looks like you beat me to it Smile
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
mpthompson



Joined: 02 Jan 2006
Posts: 650
Location: San Carlos, CA

PostPosted: Sun Jan 29, 2006 4:52 am    Post subject: Reply with quote

Something I noticed about using checksums for read and write to the registers is that the address field is unprotected. Perhaps we should modify the protocol as follows:

Checked Read

Code:
Start, I2CAddress, TWI_CMD_CHECKED_TXN, ByteCount, RegisterAddress, Checksum, Restart, I2CAddress+READ, Read[0], Read[1] ... Read[Count], Read[Checksum], Stop


Checked Write

Code:
Start, I2CAddress, TWI_CMD_CHECKED_TXN, ByteCount, RegisterAddress, Write[0], Write[1] ... Write[Count], Write[Checksum], Stop


As indicated above, in a checked read the master would checksum the byte count and register address (which is just retransmitting it I guess). For a checked write the checksum would just extend over the byte count and register address as well as the written bytes.

Since we are providing a minimal level of protecting the data we should consider protecting the byte count and address as well.

-Mike
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
andylippitt
Site Admin


Joined: 02 Jan 2006
Posts: 155
Location: Denver, CO

PostPosted: Sun Jan 29, 2006 5:02 am    Post subject: Reply with quote

Agreed. Are you going to make the changes or would you like me to?
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
mpthompson



Joined: 02 Jan 2006
Posts: 650
Location: San Carlos, CA

PostPosted: Sun Jan 29, 2006 5:26 am    Post subject: Reply with quote

Why don't you go ahead.

-Mike
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
andylippitt
Site Admin


Joined: 02 Jan 2006
Posts: 155
Location: Denver, CO

PostPosted: Sun Jan 29, 2006 8:20 pm    Post subject: Reply with quote

Tested and checked in with some differences from Mike's suggestion above. The protocol remains as I originally had it, but the checksums include the bytecount and address values.
Back to top
View user's profile Send private message Send e-mail Visit poster's website Yahoo Messenger
Display posts from previous:   
Post new topic   Reply to topic    OpenServo.com Forum Index -> Software All times are GMT
Goto page 1, 2  Next
Page 1 of 2

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group