Bug with TWI/i2c and Atmega168

hi,

I spent a few days trying to get the i2c-bus working with the Atmega168 (Arduino NG rev.c) and finally found the bug in the definition of the interrupt routine. The symptom was that I could not send (nor receive) anything. When trying to send for example, the execution got stuck calling Wire.endTransmission(). My environment is Arduino-0007 on Ubuntu Linux 6.06 (with avr-libc 1.2.3-3). If anyone ends up with the same problem, try the following:

Open the file:
arduino-0007/lib/targets/libraries/Wire/utility/twi.c

Change the following line:
SIGNAL(SIG_2WIRE_SERIAL)

To this one:
SIGNAL(SIG_TWI)

Remove the old object-file:
arduino-0007/lib/targets/libraries/Wire/utility/twi.o

And recompile. That should be it.

The "nice" thing in all this is that the compiler does not complain about anything, it just ends un using the incorrect define SIG_2WIRE_SERIAL of some other (probably Atmega8) processor and binds the interrupt incorrectly.

Regards,

Ouch. I'll have to fix that. Thanks for the report. This is something that's bitten me before with the serial interrupts. Anyone know if there's a way to get a compiler warning for this sort of thing?

Can anyone confirm this? The AVR LIBC documentation lists SIG_2WIRE_SERIAL as a correct (though old) name for the TWI signal handler, with TWI_vect as the new name (SIG_TWI isn't mentioned anywhere). I wonder if the library just needed to be recompiled (because the software accidentally included a pre-compiled object file for the ATmega8).

Hi mellis,

The Wire lib works fine on my 168's without making the above change. I did recompile, because I was having a different problem, so can't confirm if the original .o works as is.

D.

Awesome, thanks. I won't make the change then.

Hello. I have the same problem with my Ubuntu 6.06. I solved it by compiling and uploading in Windows.
I think there is some version difference between the library files provided on the AVR package of ubuntu and the ones shipped with the Windows IDE.

I haven't tested the hack proposed by tek2, but there's definitely something wrong here.
I can test it if it's important.

I seem to have this problem as well. My setup is: Ubuntu 8.04 with avr-libc 1:1.4.7-1, the arduino 0011 environment and a ladyada boarduino atmega168-20p. I've rebuilt my Wire.o and twi.o files with many of the board build targets. Should one in particular work? I've also replaced my ATM168-20p with a new spare with no luck.

What happens is I get a hang when Wire.endTransmission() calls twi_writeTo. Specifically, it locks up at the end of twi_writeTo waiting for the send operation to complete:

while(wait && (TWI_MTX == twi_state)) { continue; }

But, I believe the ISR is working as I have some code at the start of the SIGNAL(SIG_2WIRE_SERIAL) handler that seems to make the led blink when I un/plug my I2C nunchuck.

{ static char led = 0; digitalWrite(13, (led = led ? 0 : 1)); }

I'd like to be able to try 0012, but I don't have easy access to a windows environment. twi.c seems to have had some recent error handling changes - perhaps included in 0012.

Anyone know when 0012 is coming out for linux? Any other suggestions?

I have this issue with ubuntu 6.06. I made the change above and the endtransmission stops blocking. Now I can successfully interface with my i2c device!

Thanks!

After reviewing the wire lib output on my scope it does not seem to be treating read commands properly. According to I2C specs the sequence should be Start-AddrW-SlaveAck-Dataout-SlaveAck-RepeatStart-AddrR-SlaveAck-DataIn-MasterNAck-Stop

The output I am seeing here does not have repeat starts- it's Stop followed by Start. Also the final NAck seems to be wrong. I have spent a week trying to get this to work with a picky slave device and I think I'll just roll my own I2C implementation now.

If you patch the Wire library, it would be great to include those fixes in the Arduino distribution. Let us know what you come up with.

I gave up on that and wrote a simple i2c (master-only) library instead.
If anybody wants to move this to the wiki, be my guest.


/*
This code (and license) is based on Joerg Wunsch's twitest example.
*


  • "THE BEER-WARE LICENSE" (because GNU sucks):
  • Marco Grubert wrote this file. As long as you retain this notice you
  • can do whatever you want with this stuff. If we meet some day, and
  • you think this stuff is worth it, you can buy me a beer in return.
  • ----------------------------------------------------------------------------*/

/* USAGE:
#include "i2c.h"
I2C::Init(); // call this in setup
I2C::SendTo( deviceAddr, deviceRegister, aByte ); // sends a single byte

// For receiving data provide a buffer pointer and the buffer size
I2C::Buffer receiveBuffer(4); // 4 bytes large
I2C::ReceiveFrom( deviceAddr, deviceRegister, receiveBuffer.mBuffer, receiveBuffer.mLength );

All functions return true on success. If you get a false instead, you can inspect the hi and low
byte of I2C::ErrorCode for more details. You could also recompile the code with DEBUG
defined to get callstacks printed to Serial out.
*/

#ifndef I2CMASTER_H
#define I2CMASTER_H

#include <avr/io.h>
#include <compat/twi.h>

#ifndef CPU_FREQ
#define CPU_FREQ 16000000L // Lillypad might have to redefine this to 8MHz instead of 16
#endif

#ifndef TWI_FREQ
#define TWI_FREQ 100000L
#endif

#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif

#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif

// #define DEBUG // enable to print out callstack over serial port

#ifdef DEBUG
#define DPRINT(a) Serial.println(a);
#else
#define DPRINT(a)
#endif

namespace I2C {

// just a simple buffer class. You don't have to use it if you supply your own buffer
class CBuffer {
public:
CBuffer() :
mBuffer(0), mLength(0) {
}
CBuffer(uint8_t bufLength)
{
mLength= bufLength;
mBuffer = (uint8_t*) calloc(bufLength, sizeof(uint8_t));
}
void Init(uint8_t bufLength)
{
mLength= bufLength;
mBuffer = (uint8_t*) calloc(bufLength, sizeof(uint8_t));
}
uint8_t* mBuffer;
uint8_t mLength;
};

// low byte == TW_STATUS
// hi byte == library status
uint16_t ErrorCode=0;

void Init()
{
// activate internal pull-ups for twi
// as per note from atmega8 manual pg167
sbi(PORTC, 4);
sbi(PORTC, 5);

// initialize twi prescaler and bit rate
cbi(TWSR, TWPS0);
cbi(TWSR, TWPS1);
// twi bit rate formula from atmega128 manual pg 204
// SCL Frequency = CPU Clock Frequency / (16 + (2 * TWBR))
// note: TWBR should be 10 or higher for master mode
// It is 72 for a 16mhz Wiring board with 100kHz TWI
TWBR = ((CPU_FREQ / TWI_FREQ) - 16) / 2;

// enable twi module, acks, and twi interrupt
TWCR = _BV(TWEN) ;
}

bool _SendStart(bool bRepeat=false)
{
DPRINT("Start in");
TWCR = (1<<TWINT)|(1<<TWSTA)| (1<<TWEN);
while (!(TWCR & (1<<TWINT)));
DPRINT("Start TWINT");

if (bRepeat)
return (TW_STATUS == TW_REP_START);
else
return (TW_STATUS == TW_START);
}

bool _SendAddr(uint8_t addr, bool bWriteMode)
{
DPRINT("SendAdd in");

if (bWriteMode)
TWDR = (addr << 1) | TW_WRITE;
else
TWDR = (addr << 1) | TW_READ;

TWCR = _BV(TWINT) | _BV(TWEN); // clear interrupt to start transmission
while ((TWCR & _BV(TWINT)) == 0) ; // wait for transmission
DPRINT("SendAdd TWINT");

if (bWriteMode)
return (TW_STATUS== TW_MT_SLA_ACK);
else
return (TW_STATUS== TW_MR_SLA_ACK);
}

bool _SendByte(uint8_t data)
{
DPRINT("SendByte in");

TWDR = data;
TWCR = _BV(TWINT) | _BV(TWEN); // clear interrupt to start transmission
while ((TWCR & _BV(TWINT)) == 0) ; // wait for transmission
DPRINT("SendByte out");

return (TW_STATUS== TW_MT_DATA_ACK);
}

bool _ReceiveBytes(uint8_t* buffer, uint8_t len)
{
DPRINT("RecvByte in");

int16_t length= len; // allow <0
for (uint8_t twcr = _BV(TWINT) | _BV(TWEN) | _BV(TWEA); length > 0; --length)
{
DPRINT("RecvByte read");

if (length == 1)
{
DPRINT("RecvByte req nack");

twcr = _BV(TWINT) | _BV(TWEN); // last byte? don't schedule ACK
}
TWCR = twcr;
while ((TWCR & _BV(TWINT)) == 0) ; // wait for transmission
const uint8_t twst= TW_STATUS;
switch (twst)
{
case TW_MR_DATA_NACK:
if (length!=1) {
DPRINT("RecvByte Nack mismatch");

return false; // we sent NACK too early
}
case TW_MR_DATA_ACK:
DPRINT("RecvByte sent ack/nack");

*buffer++ = TWDR;
break;
default:
DPRINT("RecvByte error out");

return false;
}
}
DPRINT("RecvByte out");

return true;
}

bool _SendStop()
{
DPRINT("Sendstop");

TWCR = _BV(TWINT) | _BV(TWSTO) | _BV(TWEN); // send stop condition
return true;
}

class AutoStopper { // call I2C Stop when this object goes out of scope
public:
~AutoStopper() {
_SendStop();
}
};

#define TRY(a, b) { if (! a ) { ErrorCode=TW_STATUS | (b); return false; } }

bool ReceiveFrom(uint8_t addr, uint8_t reg, uint8_t *buffer, uint8_t buffLength)
{
DPRINT("Receivefrom in");

// first write register pointer
TRY( _SendStart(false), 1 );
{
AutoStopper stopper; // call stop when this object goes out of scope
TRY( _SendAddr(addr, true), 2);
TRY( _SendByte(reg), 3 );
// now request data from device with repeated start
TRY( _SendStart(true), 4 );
TRY( _SendAddr(addr, false), 5 );
TRY( _ReceiveBytes( buffer, buffLength ), 6 );
}
return true;
}

bool SendTo(uint8_t addr, uint8_t reg, uint8_t *buffer, uint8_t buffLength)
{
DPRINT("Sendto in");
TRY( _SendStart(false), 1 );
{
AutoStopper stopper; // call stop when this object goes out of scope
TRY( _SendAddr(addr, true), 2);
TRY( _SendByte(reg), 3 );
while (buffLength>=1) {
TRY( _SendByte(*buffer++), 4 );
--buffLength;
}
}
return true;
}
bool SendTo(uint8_t addr, uint8_t reg, uint8_t data)
{
return SendTo(addr, reg, &data, 1);
}

} // namespace I2C

#endif // I2CMASTER_H

I gave up on that and wrote a simple i2c (master-only) library instead.

Thanks for that. I will try this 'on the bench'.

I have used 'Wire' extensively with my MRMP
http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1232140631

However I have only been able to tweak Wire 0011 library to be a reliable I2C Master<>Slave network. Wire 0012 and 0013 works for about 100 byes then freezes. I have spent several hours trying to determine why, but have yet to find a tweak that will make I2C reliable.

My ultimate goal is a multi-master I2C setup, and your code may be part of the answer. I may need need the slave part to finish that quest.

However I have only been able to tweak Wire 0011 library to be a reliable I2C Master<>Slave network. Wire 0012 and 0013 works for about 100 byes then freezes. I have spent several hours trying to determine why, but have yet to find a tweak that will make I2C reliable.

Are you using the OSX version the IDE? If so then there's a "known" bug with the avr-gcc compiler (4.3) that's included with Arduino 0012 and 0013. The compiler generated bad interrupt handling code which causes crashes and lockups. The Wire library relies on interrupts so your freezes match how this bug manifests.

The Windows version of avr-gcc doesn't have the bug. I don't know about the vaious linux variants.

Are you using the OSX version the IDE? If so then there's a "known" bug with the avr-gcc compiler (4.3) that's included with Arduino 0012 and 0013. The compiler generated bad interrupt handling code which causes crashes and lockups. The Wire library relies on interrupts so your freezes match how this bug manifests.

In a 'working' setup, I can use Arduino 0012 or 0013... except I have to swap out the Wire library and use Wire from 0011.

This one...
Arduino xx/hardware/libraries/Wire

So I don't see how it could be the compiler.

When you "switch out the library" are you copying over the entire Wire folder under hardware/libraries? If so, then you're also bringing along the Wire.o object file that was compiled under an older avr-gcc version that didn't have the bug.

Try this: Use the library from 0011 in 0012 (or 0013) and be sure to delete the Wire.o file. This will force it to be rebuilt with the later avr-gcc compiler. If the interrupt bug is the root of your problem, then the library from 0011 will now also have the lockups.

The Windows version of avr-gcc doesn't have the bug.

Hmmm... I moved my sketch to a arduino 0013 Windows environment.
I uploaded from Windows and it still has the same hang issue after about 100 bytes. :o

I will now try arduino 0011 on OS X.

Well then I'm guessing that OSX avr-gcc interrupt bug is not what's causing your problem.

Since you already said that the 0011 Wire library works for you, I pretty sure that building it under 0011 on OSX will likely work. Have you compared the Wire.cpp source between 0011 and 0012 (or 0013) to see what's changed?

Hi, all, what's the latest on this? I'm using Arduino-0013 with a Duemilanova (ATMega168) and my S/W never completes the first transfer (it sends the three bytes I expect, but never does the Stop condition)?

The Wire library #defines all three vectors:

/* Two-wire Serial Interface */
#define TWI_vect _VECTOR(24)
#define SIG_TWI _VECTOR(24)
#define SIG_2WIRE_SERIAL _VECTOR(24)

...so the initial mod shouldn't be a problem?

Try this: Use the library from 0011 in 0012 (or 0013) and be sure to delete the Wire.o file. This will force it to be rebuilt with the later avr-gcc compiler. If the interrupt bug is the root of your problem, then the library from 0011 will now also have the lockups.

Yes - tried this (downloaded 011 and swapped out entire .../hardware/libraries/Wire directory). The downloaded Zip file for 011 didn't have Wire.o, though, and the compiled version still exhibits the same bug. Perhaps should say I'm running Windows XP SP3, not OSX or Linux. ;D

Man am I glad I found this forum. I spent a late evening trying to figure out why i2c was crashing after a few exchanges ( running 0013 and OSX.. )

0013 under windows is working just fine. (for once, it wasn't me!)

Perhaps related, is that I noticed that delay(xx) in my main loop was cut short any time a receive or request i2c event happened.

You all saved what little hair I have left.

BTW, I'm integrating a LV-MaxSonar and a zlog altimeter in a arduino mini and making the info available via i2c for a UAV project.

Always happy to share code.