Diskussion:TWI Slave mit avr-gcc: Unterschied zwischen den Versionen

Aktuelle Version vom 8. August 2012, 10:58 Uhr

Hallo Uwe,

als mein Master mit Deinem Slave reden wollte, hängte sich der wegen nen Programmierfehler meines Masters auf. Die SCL-Leitung hat er dadurch einfach nicht mehr losgelassen. Durch ändern der Zeile

//switched to the non adressed slave mode...

1. define TWCR_RESET TWCR = (1<<TWEN)|(1<<TWIE)|(1<<TWINT)|(1<<TWEA)|(0<<TWSTA)|(0<<TWSTO)|(0<<TWWC);

nach

//switched to the non adressed slave mode...

1. define TWCR_RESET TWCR = (1<<TWEN)|(1<<TWIE)|(1<<TWINT)|(1<<TWEA)|(0<<TWSTA)|(1<<TWSTO)|(0<<TWWC);

war alles in Butter.

Gruß Frank

Ok, habe es jetzt korrigiert, aber noch nicht selbst getestet... Solle laut Datenblatt aber korrekt sein. mfg --uwegw 11:47, 12. Apr 2010 (CEST)

Kelinigkeiten

Hallo,

ich habe den Code für ein Projekt vor 2 Jahren benutzt, jetzt habe ich nach dem Code gesucht um ein Update zu machen.

Die Oben genannten Probleme hatte ich damals auch - jetzt beim ersetzen des Moduls fällt mir folgendes auf:

- I2C_reg_Schreibschutz ist nicht definiert und kommt im Code auch nur einmal vor. Ich denke mal, dass es dafür sorgen soll, dass richtig adressiert worden ist, oder?

I2C_reg_Schreibschutz in twislave.h definieren ich denke das war als array gedacht? I2C_reg_Schreibschutz sollte in der ini routine initialisiert werden... - dabei kann man gleich den Speicherbereich vorbeschreiben.

Das war tatsächlich als Array gedacht, über das man den Schreibzugriff auf bestimmte Register sperren konnte. Wenn ich es mir recht überlege, ist diese Funktion recht überflüssig. Ich habe sie also erst mal entfernt. --uwegw 16:39, 2. Jul 2010 (CEST)

Ich würde auch die Interrupt aktivierung nicht in der Ini routine machen, sondern lieber ausserhalb, denn normalerweise müssen ja noch andere Sachen vorbereitet werden.

Mein Vorschlag für die Ini routine:

void init_twi_slave(uint8_t adr) {

   uint8_t i=0;
    TWAR= adr; //Adresse setzen
    TWCR &= ~(1<<TWSTA)|(1<<TWSTO);
    TWCR|= (1<<TWEA) | (1<<TWEN)|(1<<TWIE);
    buffer_adr=0xFF;
    for(i=0;i<i2c_buffer_size;i++)
	{
		i2cdata[i]=0;
		I2C_reg_Schreibschutz[i]=0;
	}
}


Warum definierst Du i2cdata zweimal? Einmal im header reicht ....

Ansonsten vielen Dank, das ist sehr hilfreich...

Christian

Hallo Uwe,

ich setze mich gerade mit deinem Code in "twislave" auseinander. Ich habe folgende Anmerkungen:

Der Puffer wird mit "volatile uint8_t i2cdata[i2c_buffer_size+1];" deklariert. Warum das "+1" bei der Größe? Du fragst überall die Puffergrenze mit "...<i2c_buffer_size" ab. Damit würde "i2cdata[i2c_buffer_size]" nie angefasst werden.

Der Puffer wird sowohl in der .h- als auch in der .c-Datei deklariert. Die Deklaration in der .h-Datei wäre ausreichend.

init_twi_slave:

Hier steht "TWCR &= ~(1<<TWSTA) | (1<<TWSTO);". Durch ~(1<<TWSTA) sind bereits alle Bits bis auf TWSTA auf 1, also auch TWSTO. "| (1<<TWSTO)" wäre also unnötig. Meinst du etwas anderes?

Danach folgt "TWCR |= (1<<TWEA) | (1<<TWEN) | (1<<TWIE);". Diese Bits wurden bereits in der vorhergehenden Anweisung gesetzt. Diese Zeile müsste überflüssig sein.

sei() in der Initialisierungsroutine ist nicht gut! Wenn mehrere Hardwarekomponenten zu initialisieren sind (z.B. USART), kann es sein, dass diese bereits im unitialisierten Zustand oder bei der Initialisierung Interrupts auslösen. sei() gehört auf jeden Fall ins Hauptprogramm. Ggf. wäre eine Kommentarzeile mit dem Hinweis auf sei() angemessen.

ISR:

 ...
 { buffer_adr = 0; //Adresse auf Null setzen. Ist das sinnvoll? TO DO!
 ...

Ich denke, dass ist nicht sinnvoll. Besser ist es, buffer_adr auf 0xFF stehen zu lassen. Das setzen auf 0 bedeutet, dass bei einer offensichtlich fehlerhaften Ansteuerung ggf. gültiger Speicherbereich überschrieben würde. Das sollte nicht sein.

Die folgende Codefolge taucht in ähnlicher Form mehrfach auf:

{ if(buffer_adr < i2c_buffer_size)
  { i2cdata[buffer_adr] = data; //Daten in Puffer schreiben
  }
  buffer_adr++; //Buffer-Adresse weiter zählen für nächsten Schreibzugriff
  ...

Das bedingungslose Hochzählen des Pufferindex bedeutet, dass er bei genügend langen Übertragungssequenzen zunächst zu 0xFF (also ungültig) wird und dann zu 0x00 überläuft und wieder in gültige Indexbereiche kommt. Die Sequenz muss noch nicht einmal sehr lang sein, da man den Index durch die Addressierung kurz vor das Ende des Puffers positionieren kann. Besser wäre

{ if(buffer_adr < i2c_buffer_size)
  { i2cdata[buffer_adr++] = data; //Daten in Puffer schreiben
  }
  ...

Eigentlich müsste man auch noch berücksichtigen, dass wenn der Index 254 errreicht wurde, weder weiter hochgezählt werden darf (würde zu 0xFF führen) noch ein weiteres Mal Daten gespeichert werden dürfen. Einfacher ist es, man begrenzt die max. Puffergröße auf 253 Byte.

 case TW_ST_DATA_ACK: //0xB8 Slave Transmitter, Daten wurden angefordert
    if (buffer_adr == 0xFF) //zuvor keine Leseadresse angegeben!
    { buffer_adr = 0;
    }

Auch hier: Offensichtlich ein Fehlerzustand, der zu gültigen Daten führt. Wenn man diesen Fehler macht, sucht man ziemlich lange nach der Ursache. Kurz danach wird auf Datenanforderung außerhalb des Pufferbereichs mit dem Senden von 0x00 reagiert. Dies sollte man auch diesem Fehler machen. Anweisung also am besten streichen.

Apropos "lange suchen"... Ich bin schon etwas älter. Da könnte der Ausdruck "totsuchen" zur Realität werden :-)

Viele Grüße RedBaron