datenübertragung mit crc5 - seltsame effekte

[Kei N. (mckalle)]

Hallo,

ich habe zwischen meinem PC und einem µC (atmega88 @ 7,3728MHz) eine 
serielle Datenübertragung über Infrarot realisiert.
Dies funktioniert wunderbar, jedoch kommen manchmal korrupte Daten am µC 
an. Da der µC für Lichtsteuerung zuständig ist, macht sich dies durch 
hässliche Blitze o.ä. bemerkbar. Die Daten werden direkt über eine 
rs232-Schnittstelle gesendet (4800 Baud, 5 Datenbits, 1 Startbit, 1 
Stoppbit, 0 Parität). Dabei werden jeweils 5 Datenblöcke hintereinander 
gesendet, wobei der letzte Datenblock eine xor-Prüfsumme enthält.
Um die Fehlererkennung zu verbessern, würde ich diese gerne durch eine 
crc5-Prüfsumme ersetzen. Die Berechnung der Prüfsumme funktioniert im 
sendenden Programm ohne Probleme (Generatorpolynom: 00101, Datenbytes 
und Rest werden jeweils gespiegelt).
Im Debugmodus vom AVR-Studio wird über eine Tabellenbasierte Lösung auch 
die korrekte CRC-Prüfsumme ermittelt.
Trotzdem funktioniert die Datenübertragung nicht mehr.

So, nun ein bisschen Code ;)

//Look-Up-Tabelle
volatile uint8_t crctable[32] =
    {
         0, 14, 28, 18, 17, 31, 13,  3,
        11,  5, 23, 25, 26, 20,  6,  8,
        22, 24, 10,  4,  7,  9, 27, 21,
        29, 19,  1, 15, 12,  2, 16, 30
    };

    //Enthält die Nummer des Empfangenen Bytes
    volatile uint8_t incomingByteCounter = 0;

    //Die Werte der Empfangenen Datenbytes
    volatile uint8_t byte0 = 0;
    volatile uint8_t byte1 = 0;
    volatile uint8_t byte2 = 0;
    volatile uint8_t byte3 = 0;

    //Der aktuelle crc-Wert
    volatile uint8_t crcValue = 0;

    //Wenn nach einer bestimmten Zeit (~6ms) kein weiteres Byte eingetroffen ist, werden die Daten resettet
    uint8_t uartResetCounter = 0;



// ===== Usart Empfangsinterrupt ================================
ISR (USART_RX_vect) //Daten liegen in UDR0
{
    //TimingInterrupt deaktivieren
    cli();
    //ResetCounter zurücksetzen
    uartResetCounter = 0;
    
    //die Bytenummer des eingehenden Bytes bestimmen
    incomingByteCounter++;
    
    //crc-lookup-Index für das aktuelle Byte berechnen
    uint8_t nextCrcIndex = (UDR0 & 0x1f) ^ (crcValue & 0x1f);

    //Eingetroffenes Byte verarbeiten
  //Je nach Bytenummer, entweder zwischenspeichern, oder alle eingetroffenen Daten verarbeiten
    switch(incomingByteCounter)
    {
        case 1:
            byte0 = UDR0;
            crcValue = crctable[nextCrcIndex];
            break;
        case 2:
            byte1 = UDR0;
            crcValue = crctable[nextCrcIndex];
            break;
        case 3:
            byte2 = UDR0;
            crcValue = crctable[nextCrcIndex];
            break;
        case 4:
            byte3 = UDR0;
            crcValue = crctable[nextCrcIndex];
            break;
        case 5:
            //Entscheidender Vergleich: letztes Byte == crc-Wert?
            if(crcValue == UDR0)
            {
        //Bytes aufteilen
          /*
           * Empfangene Bytes:
           *  Byte 0: XXAAA
           *  Byte 1: AAABB
           *  Byte 2: BBBBC
           *  Byte 3: CCCCC
           *  Byte 4: DDDDD
           
           Legende:
            X: zum bestimmen, was für Daten empfangen wurden
            A: 6 Bit Daten 
            B: 6 Bit Daten                                                        
            C: 6 Bit Daten                                                        
            D: Parität/Prüfsumme
           */
        uint8_t tby1 = ((byte0 & 0x07) << 3) | ((byte1 & 0x1C) >> 2);
        uint8_t tby2 = ((byte1 & 0x03) << 4) | ((byte2 & 0x1E) >> 1);
        uint8_t tby3 = ((byte2 & 0x01) << 5) | byte3;
        if((byte0 & 0x18) == 0) //Es wurden bestimmte Daten empfangen
        {
          //ocrR, ocrG, ocrB: PWM-Compare-Values, für die Helligkeit
          //pwmtable: lookup-Table für logarythmische Helligkeitsausgabe (->Linear fürs Auge), enthält 64 Werte
          ocrR = pwmtable[tby1];
          ocrG = pwmtable[tby2];
          ocrB = pwmtable[tby3];
        }
        else if((byte0 & 0x18) == 8) //Es wurden andere Daten empfangen
        {
          ocrK = pwmtable[tby1];
          ocrW = pwmtable[tby2];
          pinB = tby3;
        }
            }

            //Kritische Variablen zurücksetzen
            incomingByteCounter = 0;
            crcValue = 0;
            break;
        default:
            break;
    }

    //Timinginterrupt wieder aktivieren
    sei();
}



/*
 * Timerinterrupt von Timer2
 * Timer2 wurde folgendermaßen initialisiert:
 * Timereinstellungen setzen: TCCR2A = (1<<COM2A0) | (1<<COM2A1) | (1<<COM2B0) | (1<<COM2B1) | (1<<WGM20);
  TCCR2A = 0xF1;

 * Prescaler auf 8 eingestellt (CS21): TCCR2B = (1<<CS21);
  TCCR2B = 0x02;

 * Interrupt für OCR2A aktivieren (für Timings, OCIE2A)
  TIMSK2 = 0x02;

 * Bei einer Baudrate von 4800 kommt alle 208µs ein bit, also kommt mein ganzes Datenpaket
 * (1*Start, 5*Daten, 1*Stop = 7 bit) schnellstens alle 1,45ms.
 *
 * Das AVR-Studio zeigt als Zeit zwischen dem Auslösen von zwei Interrupten eine Zeitspanne von ~4090µs bei 1MHz,
 * was auf 7,3728MHz hochgerechnet etwa 555µs entspricht, daher lasse ich den uartResetCounter bis 10 Zählen (->5,5ms)
 * Auf dasselbe Ergebnis komme ich auch, wenn ich manuell die Zeit berechne: 8*512/7232800 = 555µs
 * Erst wenn 5ms zwischen zwei Datenpaketen vergangen sind, werden die alten Daten resettet.
 */
ISR(TIMER2_COMPA_vect) //TIMER2 COMPA
{
  if(incomingByteCounter != 0) //Wird gerade etwas empfangen? Nur dann muss die Zeit gezählt werden
  {
    uartResetCounter++;  //Den Counter incrementieren
    if(uartResetCounter == 10)  //Wenn der counter bei 10 ist, alles Resetten.
            //Dann ist zu viel Zeit vom Empfang des letzten Bytes vergangen (~6ms),
            //sofern ein Interrupt nicht wärend der byte-Verarbeitung aufgetreten wäre.
            //Dann hat das nächste Byte noch länger Zeit.
    {
      incomingByteCounter = 0;
      uartResetCounter = 0;
      crcValue = 0;
    }
  }
}

Ich bin für jegliche Hilfe dankbar!

[Stefan E. (sternst)]

    //TimingInterrupt deaktivieren
    cli();
...
    //Timinginterrupt wieder aktivieren
    sei();

Das ist Unsinn. Schmeiße das raus.

Dein eigentliches Problem ist aber, dass du versuchst, UDR0 in der ISR 
zweimal zu lesen. Das geht nicht, schließlich steckt hinter UDR0 ein 
FIFO.

Und so nebenbei: welchen Sinn macht es, eine Look-Up-Tabelle als 
volatile zu deklarieren?

[Kei N. (mckalle)]

VIELEN DANK!! So ein blöder Fifo ;)

Das mit dem cli(); sei(); hatte ich irgendwo gelesen, dass man das bei 
Timern machen soll, wenn man Werte ändert, hatte ich zum ausprobieren 
noch drin.

>> Und so nebenbei: welchen Sinn macht es, eine Look-Up-Tabelle als
>> volatile zu deklarieren?
Ich dachte, als volatile soll man alles deklarieren, auf das man 
innerhalb eines Interrupts zugreift?

[Stefan E. (sternst)]

Kei Ner schrieb:
> Das mit dem cli(); sei(); hatte ich irgendwo gelesen, dass man das bei
> Timern machen soll, wenn man Werte ändert, hatte ich zum ausprobieren
> noch drin.

Bei AVRs sind beim Ausführen der ISR die Interrupts sowieso 
abgeschaltet, damit sich Interrupts nicht gegenseitig unterbrechen.

Kei Ner schrieb:
> Ich dachte, als volatile soll man alles deklarieren, auf das man
> innerhalb eines Interrupts zugreift?

Nein. Du deklarierst alles als volatile, auf das sowohl im Interrupt, 
als auch im restlichen Code zugegriffen wird. Und das auch nur dann, 
wenn die Variable auch mal irgendwann irgendwo verändert wird.

[Kei N. (mckalle)]

Stefan Ernst schrieb:
> Bei AVRs sind beim Ausführen der ISR die Interrupts sowieso
> abgeschaltet, damit sich Interrupts nicht gegenseitig unterbrechen.
Gut, zwei Zeilen Code gespart ;)


> Nein. Du deklarierst alles als volatile, auf das sowohl im Interrupt,
> als auch im restlichen Code zugegriffen wird. Und das auch nur dann,
> wenn die Variable auch mal irgendwann irgendwo verändert wird.
Ok. Werd ich mir merken :)