Problem mit I2C bei atmega328p (arduino uno)

[Philipp H. (philipp_ho)]

Hallo zusammen,

ich versuche gerade eine I2C Verbindung zwischen 2 atmega328p (arduino 
unos) aufzubauen. Es soll zunächst nur vom Master die Übertragung 
initiiert und ein paar bytes übertragen werden, beim Slave soll nach 
Erkennen der eigenen Adresse die entsprechende Interrupt Routine 
ausgeführt werden. Fehlercodes werden noch nicht wirklich 
berücksichtigt, es geht erstmal nur um die Grundfunktion. Der Slave 
springt tatsächlich auch in die IR, meldet aber für das Statusregister 
TWSR 0x00, also "Bus error due to an illegal
START or STOP condition". Der Master kommt bis zum Aussenden der 
Adresse, aber nicht weiter. Auch der Logic Analyzer zeigt seltsames.

Hier die Funktionen zur Initialisierung, Senden und Interrupt Routine:

void I2cInit(uint8_t sysClock, uint16_t bitRate)
{
  //Set Slave Adress
  TWAR = ((I2C_ADDRESS<<1) & 0xFE);
  //Set bit rate
  TWBR = (((sysClock*1000)/(2*bitRate))-8);
  //Enable ACK (active bus connection)
  TWCR |= (1<<TWEA);
  //Enable TWI
  TWCR |= (1<<TWEN);
  //Enable TWI Interrupt
  TWCR |= (1<<TWIE);
}

bool i2cTxMaster (uint8_t *firstelementofData,uint8_t lengthOfData,uint8_t address)
{
        //Result variable 
  bool result = true;
  //Loop Variable
  uint8_t i=0;
        //Disable TWI Interrupt
  TWCR &= !(1<<TWIE);
  //Transmit START condition
  TWCR |= (1<<TWINT)|(1<<TWSTA)|(1<<TWEN);
  //Wait for TWINT flag
  while(!(TWCR & (1<<TWINT)));
  //Terminate if status != 0x08
  if ((TWSR & 0xF8) != 0x08)
  {
    result = false;
    return result;
  }
  //Clear START bit
  TWCR &= !(1<<TWSTA);
  //Load SLA+W to data register
  TWDR = ((address<<1) & 0xFE);
  //Transmit Address Byte
  TWCR |= (1<<TWINT)|(1<<TWEN);
  //Wait for TWINT flag
  while(!(TWCR & (1<<TWINT)));
  //Terminate if status != 0x18
  if ((TWSR & 0xF8) != 0x18)
  {
    result = false;
    return result;
  }
  //Transmit data bytes
  for (i=0; i<lengthOfData; i++)
  {
    //Load current byte to data register
    TWDR = *(firstelementofData+i);
    //Send data byte
    TWCR |= (1<<TWINT)|(1<<TWEN);
    //Wait for TWINT flag
    while(!(TWCR & (1<<TWINT)));
    //Terminate if status != 0x28
    if ((TWSR & 0xF8) != 0x28)
    {
      result = false;
      return result;
    }
  }
  //Transmit STOP Condition
  TWCR |= (1<<TWINT)|(1<<TWSTO)|(1<<TWEN);
  //Enable TWI Interrupt
  TWCR |= (1<<TWIE);
  return result;
}

ISR(TWI_vect)
{
  //Disable TWI interrupt
  TWCR &= !(1<<TWIE); 
  if (((TWSR & 0xF8) == 0x60) || ((TWSR & 0xF8) == 0x70))
  {
    payloadPendingUpLength=0;
    //Clear TWINT flag
    TWCR |= (1<<TWINT)|(1<<TWEN);
    //Wait for TWINT flag
    while(!(TWCR & (1<<TWINT)));
    while ((TWSR & 0xF8) != 0xA0)
    {
      payloadPendingUpContent[payloadPendingUpLength] = TWDR;
      payloadPendingUpLength++;
      //Clear TWINT flag
      TWCR |= (1<<TWINT)|(1<<TWEN);
      //Wait for TWINT flag
      while(!(TWCR & (1<<TWINT)));
    }
  }
  payloadPendingUp = true;
  //Enable TWI interrupt
  TWCR |= (1<<TWIE);
}

Wie im Datenblatt vorgegeben sind SDA und SCL über Pull-up Widerstände 
mit Vcc verbunden (4k7). Die SCL Frequenz wird mit 50k initialisiert.

Ist da ein dummer Fehler im Code oder könnte es eher an der Hardware 
liegen?

[Spess53 (Gast)]

Hi

>Die SCL Frequenz wird mit 50k initialisiert.

Was soll das heißen?

MfG Spess

[Philipp H. (philipp_ho)]

Im Prinzip die bitrate, also 50kbit, wollte mich nur an die Formulierung 
im Datenblatt halten. Zum testen soll es erstmal eine niedrige bitrate 
sein.

[Karl M. (Gast)]

Hallo, zum Glück wird die Frequenz in Hertz angegeben und was deine 
Kilobit angeht passt das überhaupt nicht zusammen!

[Wolfgang (Gast)]

Philipp H. schrieb:
> Auch der Logic Analyzer zeigt seltsames
Welche Abtastrate verwendest du?

> Logic_2.png

Aus deinem Bild kann man nichts erkennen, weil die Auflösung für 50kHz 
Clock absolut unzureichend ist. Hänge die logicdata-Datei mit an.

[Oliver S. (oliverso)]

Alle grünen Punkte im logic-Diagramm sind Startbedingungen. Da würde ich 
als Slave auch die Arbeit verweigern.

Oliver

[Oliver S. (oliverso)]

Philipp H. schrieb:
> while(!(TWCR & (1<<TWINT)));
> while ((TWSR & 0xF8) != 0xA0)
>...

Das ist in der TWI ISR allerdings völliger Murks.
Da fehlt es doch noch etwas am grundsätzlichen Verständnis, was 
Interrupt ist, und was er macht.

Die grundlegende Ablauf bei TWI Master mit Interrupt ist:

Start senden

ISR: Adresse Write senden, reti
ISR: wenn ACK, Datenbyte senden, wenn Nack, Stop senden, reti
...
ISR: Stop senden, reti

Oliver

[Philipp H. (philipp_ho)]

Oliver S. schrieb:
> Die grundlegende Ablauf bei TWI Master mit Interrupt ist:
>
> Start senden
>
> ISR: Adresse Write senden, reti
> ISR: wenn ACK, Datenbyte senden, wenn Nack, Stop senden, reti
> ...
> ISR: Stop senden, reti
>
> Oliver

Die ISR ist nur für den Slave aktiviert, der Master soll nur mit der 
entsprechenden Funktion senden. Aber klar, ich sehe was du meinst, in 
der ISR sollte nur der Status Code bei einem Interrupt ausgelesen und 
entsprechend ein Schritt ausgeführt werden, das ändere ich. Aber dann 
leuchten mir immer noch nicht die wiederholten STARTs ein. Der Master 
sendet zuletzt Adress write und wartet dann auf TWINT, der Slave gibt 
den Status Code für fehlerhaftes START/STOP aus und springt aus der ISR, 
danach passiert bei beiden nichts mehr.

[Philipp H. (philipp_ho)]

Wolfgang schrieb:
> Hänge die logicdata-Datei mit an.

Habe nochmal eine erstellt.

[Oliver S. (oliverso)]

Philipp H. schrieb:
> Die ISR ist nur für den Slave aktiviert,

Dann zeig halt mal deinen ganzen Code.

Oliver

[Stefan F. (Gast)]

Philipp H. schrieb:
>> Hänge die logicdata-Datei mit an.
> Habe nochmal eine erstellt.

Da kann man nichts erkennen, außer lauter Nadelimpulse.

[Philipp H. (philipp_ho)]

Oliver S. schrieb:
> Philipp H. schrieb:
>> Die ISR ist nur für den Slave aktiviert,
>
> Dann zeig halt mal deinen ganzen Code.
>
> Oliver

Das wäre die while Schleife der main zum testen der Verbindung. Das 
obere für den Master, unten für den Slave. Der Rest des gesamten Codes 
hat noch nichts mit dem I2C Bus zu tun, aber ich hänge mal die 
Projektdatei an.

 
    for (i=0;i<5;i++)
    {
      Atmega328pStopWatchStart(1000);
      while(!Atmega328pStopWatchCheck());
    }
    test[0] = 0x01;
    test[1] = 0x02;
    test[2] = 0x03;
    UartPrintLine("I2C init",0,0);
    UartPrintNewLine;
    
    i2cTxMaster(&test[0], 3, 0x01);
    
    /*
    sei();
    while(1)
    {
      if (payloadPendingUp)
      {
        UartPrintLine("I2C receive: ", &payloadPendingUpContent[0], payloadPendingUpLength);
        UartPrintNewLine();
        payloadPendingUp = false;
      }
    }
    */

[Stefan F. (Gast)]

Philipp, du musst schon aussagekräftige Bilder vom Signal zeigen und den 
ganzen Quelltext, sonst kann dir nach wie vor niemand helfen.

Am besten reduzierst du den Code auf eine minimale Menge von Zeilen, die 
das Problem noch haben, so dass derirrelevante Teil möglichst entfällt. 
Aber es muss compilier- und ausführbar sein, damit wir es nachvollziehen 
können.

Ein kleiner Tipp: "&test[0]" kannst du durch "test" ersetzen. Das ist 
das Selbe und besser lesbar.

[Philipp H. (philipp_ho)]

Stefan ⛄ F. schrieb:
> Philipp, du musst schon aussagekräftige Bilder vom Signal zeigen und den
> ganzen Quelltext, sonst kann dir nach wie vor niemand helfen.

Bei der .logicdata kann man doch beliebig ranzoomen? Das ist das was ich 
bekomme. Und mehr Quelltext als das komplette Projekt habe ich nicht.

Stefan ⛄ F. schrieb:
> Am besten reduzierst du den Code auf eine minimale Menge von Zeilen, die
> das Problem noch haben, so dass derirrelevante Teil möglichst entfällt.

Da ich die Ursache des Problems nicht kenne wüsste ich nicht wie, ich 
habe ja schon die Funktionen die für das Problem relevant sind gepostet.

Stefan ⛄ F. schrieb:
> Ein kleiner Tipp: "&test[0]" kannst du durch "test" ersetzen. Das ist
> das Selbe und besser lesbar.

Sicher? Die Funktion erwartet einen Pointer.

Tut mir leid wenn ich mich etwas doof anstelle, bin ja dankbar dass ihr 
euch die Zeit nehmt mir zu helfen. Aber hier verstehe ich nicht ganz was 
ich noch zusätzlich an Infos liefern kann.

[Einer K. (Gast)]

Philipp H. schrieb:
> Sicher? Die Funktion erwartet einen Pointer.

Sicher!
Der Name eines Arrays zerfällt zu einem Pointer.

Tipp:
Ein Fachbuch erwerben.

[Oliver S. (oliverso)]

Philipp H. schrieb:
> Bei der .logicdata kann man doch beliebig ranzoomen? Das ist das was ich
> bekomme.

Das hat halt mit I2C nix zu tun, was man da sieht.

Ich habe den Sourcecode jetzt mal kurz überflogen.

Mit welcher Frequnez läuft dein Arduino Uno?

Oliver

[Philipp H. (philipp_ho)]

Oliver S. schrieb:
> Das hat halt mit I2C nix zu tun, was man da sieht.

Genau deshalb werde ich ja auch nicht schlau daraus.

Oliver S. schrieb:
> Mit welcher Frequnez läuft dein Arduino Uno?

16MHz, die Fuse bits stehen auf external clock und es sollte kein 
prescaler aktiv sein.

[Oliver S. (oliverso)]

Igendwie fehlt mir da ein define für F_CPU, aber egal.  Die 
Initialisierung der USART solltest du dir dazu nochmal anschauen, und 
alle Warnungen einschalten schadet auch nicht.

Das bzw. ein Hauptproblem am TWI ist dein rumgefummel an den TWCR-Bits.
Schief geht das schonmal in i2cTxMaster in der Zeile hier:

TWCR &= !(1<<TWIE);

Die macht nicht das, was du wolltest, sondern disabled dir den TWI, die 
Leitungen gehen high, und danach macht der nicht mehr das, was du dir 
vorstellst.

Anstelle von

TWCR |= (1<<TWINT)|(1<<TWSTA)|(1<<TWEN);

schreib einfach:

TWCR = (1<<TWINT)|(1<<TWSTA)|(1<<TWEN);

Da ist alles gesetzt, was soll, und alles andere nicht. Das gilt für 
alle Zugriffe auf TWCR.

Weiter hab ich mir das nicht angeschaut, den Rest kriegst du auch selber 
hin.

Oliver

[Philipp H. (Gast)]

Stimmt, vielen Dank für den Input und die Hilfe! Werde das morgen mal 
ändern und weiter probieren.

[Philipp H. (philipp_ho)]

Habe die Anmerkungen umgesetzt und jetzt sieht das ganze schon etwas 
anders aus. Aber dennoch sendet der Master jetzt ununterbrochen STARTS, 
TWINT wird nicht gesetzt, der Slave gibt nach interrupt noch immer 0x00 
als status code.

void I2cInit(uint8_t sysClock, uint16_t bitRate)
{
  //Set Slave Adress
  TWAR = ((I2C_ADDRESS<<1) & 0xFE);
  //Set bit rate
  TWBR = (((sysClock*1000)/(2*bitRate))-8);
}

bool i2cTxMaster (uint8_t *firstelementofData,uint8_t lengthOfData,uint8_t address)
{
  //Result variable 
  bool result = true;
  //Loop Variable
  uint8_t i=0;
  //Transmit START condition
  TWCR = (1<<TWINT)|(1<<TWSTA)|(1<<TWEN);
  //Wait for TWINT flag
  while(!(TWCR & (1<<TWINT)));
  //Terminate if status != 0x08
  if ((TWSR & 0xF8) != 0x08)
  {
    result = false;
    return result;
  }
  //Load SLA+W to data register
  TWDR = ((address<<1) & 0xFE);
  //Transmit Address Byte
  TWCR = (1<<TWINT)|(1<<TWEN);
  //Wait for TWINT flag
  while(!(TWCR & (1<<TWINT)));
  //Terminate if status != 0x18
  if ((TWSR & 0xF8) != 0x18)
  {
    result = false;
    return result;
  }
  //Transmit data bytes
  for (i=0; i<lengthOfData; i++)
  {
    //Load current byte to data register
    TWDR = *(firstelementofData+i);
    //Send data byte
    TWCR = (1<<TWINT)|(1<<TWEN);
    //Wait for TWINT flag
    while(!(TWCR & (1<<TWINT)));
    //Terminate if status != 0x28
    if ((TWSR & 0xF8) != 0x28)
    {
      result = false;
      return result;
    }
  }
  //Transmit STOP Condition
  TWCR = (1<<TWINT)|(1<<TWSTO)|(1<<TWEN);
  return result;
}

ISR(TWI_vect)
{
  //Own address of general call received
  if (((TWSR & 0xF8) == 0x60) || ((TWSR & 0xF8) == 0x70))
  {
    payloadPendingUpLength=0;
    //Clear TWINT flag
    TWCR = (1<<TWINT)|(1<<TWEN);
  }
  //Byte received
  else if ((TWSR & 0xF8) == 0x80 || (TWSR & 0xF8) == 0x90)
  {
    payloadPendingUpContent[payloadPendingUpLength] = TWDR;
    payloadPendingUpLength++;
    //Clear TWINT flag
    TWCR = (1<<TWINT)|(1<<TWEN);
  }
  //STOP received
  else if ((TWSR & 0xF8) == 0xA0)
  {
    payloadPendingUp = true;
    //Clear TWINT flag
    TWCR = (1<<TWINT)|(1<<TWEN);
  }
}

//Main loop Master:
for (i=0;i<5;i++)
    {
      Atmega328pStopWatchStart(1000);
      while(!Atmega328pStopWatchCheck());
    }
    test[0] = 0x01;
    test[1] = 0x02;
    test[2] = 0x03;
    i2cTxMaster(&test[0], 3, 0x01);

//Main loop Slave:
//Enable ACK (active bus connection),TWI and TWI interrupt
    TWCR = (1<<TWEA)|(1<<TWEN)|(1<<TWIE);
    sei();
    while(1)
    {
      if (payloadPendingUp)
      {
        UartPrintLine("I2C receive: ", &payloadPendingUpContent[0], payloadPendingUpLength);
        UartPrintNewLine();
        payloadPendingUp = false;
      }
    }

Warum könnte der Master hier schon bei START stecken bleiben?

[Oliver S. (oliverso)]

I2C init 

IC2 send START  0x24 

TWSR:  0x08 

I2C START ok 

Transmit address+w  0x02 

Address acknowledged 

Data transmission successful

Sagt dein TWI-Master-Code gegen einen dummy-Slave. Der Master-Code 
funktioniert also soweit.

Da wirst du wohl entweder in den nicht-TWI-relevanten Codestellen (die 
ich für den test auskommentiert habe)  oder im Slave oder in der 
Hardware ein Problem haben.

Oliver

[Philipp H. (philipp_ho)]

Vielen Dank, es funktioniert jetzt! GND war nicht richtig verbunden und 
beim Slave musste man zusätzlich noch TWEA TWIE auf 1 halten, das hatte 
ich durch den wechsel von |= zu = zwischendurch nicht mehr drin.

Eine Sache ist allerdings noch seltsam. Nach erfolgreicher Übertragung 
steht payloadPendingUp auf true, in der while(1) schleife der main 
sollte also "I2C receive: ... " ausgegeben und payloadPendingUp wieder 
auf false gesetzt werden, die if(payloadPendingUp) schleife wird aber 
nicht ausgeführt. Wenn ich aber z.b UartPrintLine("Main Loop",0,0) vor 
die if schreibe geht es dann. Eine Idee warum das so ist?

Auf jeden Fall danke für eure Zeit und Hilfe!