Timingprobleme bei Lesen und Schreiben in Array

[Oz z. (ozzy)]

Hi,

über eine ISR empfange ich einzelne Bits, und setze diese dann zu einem 
Byte zusammen. Ist das Byte vollständig, so wird es in ein Array 
geschrieben, und in einem 2ten Array ein Flag gesetzt, dass in dem 
entsprechenden Feld ein neues Byte bereit liegt:

if ( receivedBit == 1 ) receivedByte |= (1<<receivedByteDigit);
else receivedByte &= ~(1<<receivedByteDigit);

lastBit = receivedBit;

if ( receivedByteDigit == 7 ) {
  bytes[bytescounter] = receivedByte;
  bytesready[bytescounter] = 1;
  if ( bytescounter == 255 ) bytescounter = 0; 
  else bytescounter++;
            
  receivedByteDigit = 0;
} else receivedByteDigit++;

In der main-Routine warte ich dann quasi darauf:

cli();
if ( bytesready[bytescounter] == 1 ) {
  receivedChar = bytes[bytescounter];
  bytesready[bytescounter] = 0;
  newByte = 1;
  if ( bytescounter == 255 ) bytescounter = 0;
  else bytescounter++;
}
sei();

Und gebe es über UART aus:

if (newByte == 1) {
  uart_sendchar( receivedChar );
  newByte = 0;
}

Doch sind die Zeichen, die ich bekomme vollkommen für die Grütze! Lasse 
ich aber die Abfrage nach den newByte weg, und die Schleife einfach 
durchlaufen, dann sehe ich, wie sich das Zeichen verändert, und nach 3 
Zeichen dann das richtige Ergebnis kommt.

Aber woran liegt das? Dauert das Schreiben echt so lange? Denn 
eigentlich wird ja erst das Zeichen ins Array geschrieben, und dann das 
Flag gesetzt, dass ein Zeichen da ist, also daran liegt es auch nicht...

Könnt Ihr mir sagen, woran das liegt, oder ich wie ich das verhindern 
kann???

MfG, Ozzy

[Oliver (Gast)]

Die ganze Fehlerbeschreibung für die Grütze. Was lässt du wo wann in 
welcher Schleife weg, und was ist dann nach drei Durchläufen wie 
richtig, und sonst nicht? Welcher Prozessor, welcher Takt, was 
überhaupt?

Egal, das Schreiben in ein Array dauert ein paar Zyklen, nicht mehr, und 
nicht weniger. Wieviele genau, sagt dir ein Blick in das 
Assemblerlisting. Wenn deine ISR mit 100kHz bei 1 Mhz Systemtakt 
aufgerufen wird, kann es schon eng werden :-)

volatile kennst du?

Oliver

[Karl H. (kbuchegg)]

...
Abgesehen von Olivers Anmerkungen.

Du verwendest hoffentlich in deinem realen Code 2 verschiedene
Variablen für bytescounter: Einen zum Schreiben und einen
zum Lesen.

[Oz z. (ozzy)]

Hi,

dann probiere ich es noch einmal ausführlicher zu schreiben:

Zuerst zur Hardware: ein Atmega128 mit 14,7456Mhz. Das Eingangssignal am 
Interrupt hat ungefähr eine Frequenz von 40Khz. Aber das Schreiben in 
den Array findet immer erst nach frühestens! 8x500us = 4ms statt.

Die ISR und die main-Methode stehen in verschiedenen Dateien, weshalb es 
keine Probleme mit dem bytescounter gibt.

Die Arrays sind in der C-Datei als volatile definiert:

volatile uint8_t bytes[265]
volatile uint8_t bytesready[265]
uint8_t bytescounter;

, und in der Header als extern angegeben:

extern volatile uint8_t bytes[265]
extern volatile uint8_t bytesready[265]

Wenn ich nun die main-Methode in das hier ändere:

cli();
receivedChar = bytes[0];
sei();
uart_sendchar( receivedChar );

Dann läuft das Zeichen ja durch. Die ersten beiden Male stehen da 
falsche Zeichen, aber ungefähr ab dem dritten oder vierten Durchlauf 
"erscheint" das richtige Zeichen.

Hoffe, dass konnte es ein wenig erklären, MfG, Ozzy

[Oz z. (ozzy)]

Also noch mal zu den Zeichen:
Sende ich ein "g", und lasse das durchlaufen, bekomme ich 4 Smilies, 3 
Herzen und 4 Hochkomma, bevor das erste g kommt.
Beim "h" sind es mal 3, mal 4 offnende Klammern, bevor das erste h 
kommt. Beim "m" steht ein "---" vorm ersten m...

Ich verstehe das echt nicht...

MfG, Ozzy

[Oliver (Gast)]

>Die ISR und die main-Methode stehen in verschiedenen Dateien, weshalb es
>keine Probleme mit dem bytescounter gibt.

Hast du das im mapfile nachgeprüft? Ohne "static" könnte das für den 
linker die SELBE Variable sein. Guter Programmierstil wären 
unterschiedliche Namen.

Ein Timingproblem mit Arrayzugriffen hast du auf jeden Fall nicht.

Oliver

[Oz z. (ozzy)]

Hi,

danke für Deine Antwort. Egal ob der Compiler denkt, es handelt sich um 
die gleiche Varibale: Warum werden zuerst immer falsche Zeichen 
ausgegeben? Das dürte doch gar nicht sein. Und es passiert ja auch, wenn 
ich das bytescounter in der main gar nicht verwende...

[Oz z. (ozzy)]

Oder könnte das vielleicht ein optimierungsproblem des Compilers sein? 
Ich benutze delays und beim kompilieren sagte mir der Compiler, ich 
müsse die Optimierung einschalten. Habe jetzt -O1 genommen. Kann es auch 
daran liegen?

MfG Ozzy

[Oz z. (ozzy)]

Hat denn echt keiner von Euch eine Idee, woran das noch liegen könnte???

MfG, Ozzy

[Andreas W. (Gast)]

in der main schaltest du den interrupt aus. Wenn es wirklich 
zeitkritisch ist. dann wird da auf alle fälle eine "problem" sein.
Schreib mal noch hin wie schnell die Bits kommen. wie hoch ist die 
Taktfrequenz, welcher interrupt wird ausgelöst.

[Oliver (Gast)]

Solange du hier nur wenig aussagekräftige Codeschnipsel zeigst, ist das 
alles Glaskugelraterei. Also, zeig mal das ganze Programm.

Sicher ist nur: Es ist kein Timingproblem, und auch kein 
Optimierungsproblem. (Die sinnvollste Optimierungsstufe für AVR'S ist 
-Os.)

Lass dir doch mal die Zeichen in der "Durchlauf-Version" als Hex-Wert 
anzeigen. Wenn die ersten Bits eintrudeln, ergibt das ja Zeichen 
ausserhalb des normalen ascii-Zeichensatzes, und da reagiert jedes 
Terminalprogramm anders drauf.

Und DAS tool, um so etwas zu debuggen, ist VMLAB. Wobei, mit AVR-Studio 
müsste das auch noch gehen. Durchsteppen, Eingangswerte vorgeben, und 
schauen, was passiert.

Oliver

[Oz z. (ozzy)]

@Andreas: Also die Bits kommen im Abstand von 500us, also braucht es 4ms 
für das Empfangen von einem Byte. Die Taktfrequenz liegt bei 14,7456Mhz, 
der ausgelöste Interrupt ist der INT0.

@Oliver: das ist eigentlich schon der ganze Code, mehr ist momentan 
nicht implementiert. Es werden eben nur noch Timer un UART 
initialisiert.

Was ich aber eben ausprobiert habe: schreibe ich in der main das hier:

cli();
bitready = bytesready[0];
receivedChar = bytes[0];
sei();
uart_sendstring( "Bit: " );
uart_sendstring( itoa( bitready, s, 10 ) );
uart_sendchar( '\r' );
uart_sendchar( '\n' );
uart_sendstring( "Array: " );
uart_sendstring( itoa( receivedChar, s, 10 ) );
uart_sendchar( '\r' );
uart_sendchar( '\n' );

Dann wird bei der ersten "1" bei Bit auch das richtige Zeichen (wenn 
auch als Binärwert dargestellt) ausgegeben.
Schreibe ich aber das hier:

cli();
bitready = bytesready[0];
receivedChar = bytes[0];
sei();
if ( bitready == 1 ) {
  uart_sendstring( "Bit: " );
  uart_sendstring( itoa( bitready, s, 10 ) );
  uart_sendchar( '\r' );
  uart_sendchar( '\n' );
  uart_sendstring( "Array: " );
  uart_sendstring( itoa( receivedChar, s, 10 ) );
  uart_sendchar( '\r' );
  uart_sendchar( '\n' );
}

Dann funktioniert es nicht mehr (Es werden die falschen Werte 
ausgegeben).
Liegt das daran, dass ich die Abfrage in der main zu häufig mache? 
Lieber einen Timer einbauen, und nur alle paar ms mal die Werte 
überprüfen?

Oder gibt es eine bessere Möglichkeit, dass der Interrupt Bescheid gibt, 
wann ein neues Zeichen da ist? Wir würdet ihr so etwas realisieren???

MfG, Ozzy

[Oz z. (ozzy)]

Es funktioniert!!!

ich habe das "cli();" und "sei();" weggenommen, und nun läuft alles wie 
gewollt! War wohl nicht so prall, die Interrupts kurz auszuschalten... 
Aber warum stört das nur so doll? Ich meine, wenn man das Zeichen lange 
genug hat durchlaufen lassen, war es ja richtig, d.h. der Empfang wurde 
nicht gestört...

mfG, Ozzy

[Oliver (Gast)]

Wie gesagt, ohne kompletten Code ist es schwierig.

Wenn der gezeigte Code hier tatsächlich alles ist, dann sind in deinem 
unterem Programm, wenn

if ( bitready == 1 )

 nicht erfüllt ist, tatsächlich die Interrupts die meiste Zeit gesperrt, 
und da kann dann schonmal einer verloren gehen (in deinem Programm ganz 
oben ist das noch viel ausgeprägter). Ein kleines delay (oder auch 
sinnvoller Code), der da etwas Zeit verbrät, würde helfen. Die Zeit 
zwischen cli und sei ist nicht lang, nur ein paar Zyklen, aber je 
nachdem, wie kurz deine Bitsignale anliegen, kann das natürlich auch was 
ausmachen. Aber das lässt sich von hier aus nicht sagen.

Die schnellste Möglichkeit, aus der ISR an main zu signalisieren, geht 
über ein einzelnes byte. Da ist das schreiben und lesen sowieso atomar, 
und es braucht gar kein cli und sei. Das dein Programm ohne cli und sei 
funktioniert, liegt genau daran, denn

bitready = bytesready[0];
receivedChar = bytes[0];

sind auch nur Byteoperationen. Das gilt auch, wenn du statt der 0 da 
wieder einen variablen Arrayindex einsetzt. Selbst wenn die ISR zwischen 
den beiden Operationen zuschlägt, macht das nichts, solange du in der 
ISR erst receivedChar und danach bitready beschreibst.

Oliver

[Karl H. (kbuchegg)]

Oliver wrote:
> Wie gesagt, ohne kompletten Code ist es schwierig.
>
> Wenn der gezeigte Code hier tatsächlich alles ist, dann sind in deinem
> unterem Programm, wenn

if ( bitready == 1 )

 nicht erfüllt ist,
> tatsächlich die Interrupts die meiste Zeit gesperrt, und da kann dann
> schonmal einer verloren gehen (in deinem Programm ganz oben ist das noch
> viel ausgeprägter).

Das sehe ich ähnlich.

> Ein kleines delay (oder auch sinnvoller Code),

Ich würde da auf sinnvollen Code plädieren. Und als allererstes
würde ich mal dafür plädieren, dass die beiden bytescounter
endlich mal sinnvolle Namen bekommen. Der eine von mir aus
'readBytesCounter' und der andere 'writeBytesCounter'.

Mit diesen beiden Namen, kannst du dann nämlich im Lesecode
folgendes machen:

  if( readBytesCounter != writeBytesCounter ) {
    cli();
    if ( bytesready[readBytesCounter] == 1 ) {
      receivedChar = bytes[readBytesCounter];
      bytesready[readBytesCounter] = 0;
      newByte = 1;
      if ( readBytesCounter== 255 )
        readBytesCounter= 0;
      else
        readBytesCounter++;
    }
    sei();
  }

d.h. du kannst den Vergleich der beiden Counter machen, ohne
die Interrupts sperren zu müssen und die Interrupts erst
danach sperren, wenn feststeht, dass im Buffer etwas
zu tun ist. Dadurch sinkt deine Interrupt-Sperrrate sofort
von geschätzten >90% auf unter ein 1% und die Wahrscheinlichkeit,
dass dir ein Interrupt durch die Lappen geht, sinkt in den Keller.

Ist man soweit, dann erhebt sich allerdings sofort die Frage,
ob ob dieses bytesready Array überhaupt nioch jemand braucht.
Kann ich nicht entscheiden, musst du entscheiden. Nach meinem
oberflächlichen Dafürhalten würdei ich aus dem Bauch heraus
sagen, dass das kein Mensch mehr braucht.

->  vernünftige Variablennamen haben noch nie geschadet. Hatte
man aber schlechte Namen dann macht man sich selbst das Leben
unnötig schwer. In deinem Fall:
  Du musst akzeptieren, dass diese 3 Variablen
    der Buffer
    der Schreib-Index
    der Lese-Index
  untrennbar zusammengehören.

Das ist so wie ein Tag ohne Monat und Jahr einfach keinen Sinn
ergibt. Ein Datum besteht nun mal aus Tag/Monat/Jahr.
Genauso besteht ein Ringbuffer aus
    Buffer  Lese Index  Schreib Index
und nur in dieser Konstellation kann man damit vernünftig arbeiten.


Dieses Prinzip ist so wichtig, dass man dafür in Programmiersprachen
ein Sprachmittel eingeführt hat: Die Möglichkeit sich selbst
Datentypen zu definieren

  struct Datum
  {
     uint8_t Tag;
     uint8_t Monat;
     uint8_t Jahr;
  };

  struct Ringbuffer
  {
    uint8_t  Buffer[ MAX_BUFFER_SIZE ];
    uint8_t  ReadIndex;
    uint8_t  WriteIndex;
  };

und in weiterer Folge arbeitet man mit einer Variablen vom Typ
Ringbuffer und nicht mehr mit 3 einzelnen Variablen, die nur durch
des Programmierers Gnaden als irgendwie logisch zusammengehörig
betrachtet werden.

[Oz z. (ozzy)]

@Karl heinz Buchegger: vielen Dank für Deine ausführliche Antwort! Der 
Buffer hat den Grund, dass es ja aus irgendwelchen Gründen immer einmal 
sein kann, dass nichts über die serielle Schnittstelle gesendet werden 
kann. Dann wäre es natürlich ganz nett, die eintreffenden Zeichen zu 
puffern.

Es den Rest betrifft: gerade mit dem Struct hast Du natürlich Recht, und 
ich werde den Code auch noch ändern!
cli() und sei() habe ich jetzt momentan ganz weggelassen. Ist das 
überhaupt gut? Oder doch lieber sperren; ich denke aber, lieber ein paar 
Werte nicht/oder nur langsam über UART senden, als sie nicht oder falsch 
zu empfangen...

MfG, Ozzy

[Oliver (Gast)]

cli() und sei() brauchst man immer dann, wenn die Möglichkeit besteht, 
daß ein Interrupt dazwischen die Daten verfälschen kann. Das gilt beim 
8-bittigen AVR für Datentypen, die in ISR UND Hauptprogramm gelesen UND 
geschrieben werden, UND die länger als ein byte sind.

Der Zugriff auf ein einzelnes Byte ist nicht unterbrechbar, und muß 
damit auch nicht extra mit cli() und sei() geschützt werden.

Du verwendest nur byte-Werte, daher gehts auch ohne cli und sei.

Oliver