Pages: [1] 2   Go Down
Author Topic: Bug with TWI/i2c and Atmega168  (Read 4036 times)
0 Members and 1 Guest are viewing this topic.
0
Offline Offline
Newbie
*
Karma: 0
Posts: 1
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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,
Logged

Forum Administrator
Cambridge, MA
Offline Offline
Faraday Member
*****
Karma: 12
Posts: 3538
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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?
Logged

Forum Administrator
Cambridge, MA
Offline Offline
Faraday Member
*****
Karma: 12
Posts: 3538
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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).
Logged

0
Offline Offline
Jr. Member
**
Karma: 0
Posts: 61
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Forum Administrator
Cambridge, MA
Offline Offline
Faraday Member
*****
Karma: 12
Posts: 3538
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

0
Offline Offline
Full Member
***
Karma: 0
Posts: 113
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged


0
Offline Offline
Newbie
*
Karma: 0
Posts: 1
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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?
Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 37
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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!
Logged

0
Offline Offline
Jr. Member
**
Karma: 0
Posts: 58
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Forum Administrator
Cambridge, MA
Offline Offline
Faraday Member
*****
Karma: 12
Posts: 3538
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

0
Offline Offline
Jr. Member
**
Karma: 0
Posts: 58
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

0
Offline Offline
Jr. Member
**
Karma: 1
Posts: 83
Retired Electronic Technologist focusing on Wireless network development.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
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.


Logged

Indiana
Offline Offline
Full Member
***
Karma: 1
Posts: 234
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
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.
Logged

0
Offline Offline
Jr. Member
**
Karma: 1
Posts: 83
Retired Electronic Technologist focusing on Wireless network development.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
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.

Logged

Indiana
Offline Offline
Full Member
***
Karma: 1
Posts: 234
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.



Logged

Pages: [1] 2   Go Up
Jump to: