Schlechter Programmierstil?

[Thomas (Gast)]

Kann mir jemand sagen wieso das so nicht geht?
Hier mein Programmbeispiel:
volatile char c=0;
ISR(TIMER2_OVF_vect)
{
  time++;
  itoa(time,li,10);
  if(time>=0)
  uart1_puts(li);
}
ISR(USART0_RXC_vect)
{
  while(!(UCSR0A & (1<<RXC0)));
  c=UDR0;
}
int main()
{       uart0_init();
  uart1_init();
  sei();
  timer2_init();
  time=0;

  uart0_puts("at\r");
  while(z!=1)
  {
     while(c!='\r')
    uart1_putc(c);
     while(c!='\r');
     while(c!='\n');
     while(c!='\r')
    uart1_putc(c);
     while(c!='\n');
     z=1;
  }
 while(1);
 return(0);
}
habe jetzt nicht alle Variablen deklariert aber ich glaube da liegt auch 
nicht der Fehler drin!
Also folgendes Problem tritt auf: Das Programm wird Initialisiert. Bevor 
es in die while(1) Schleife rein kommt, wird über die uart0 at\r zum GSM 
geschickt. Normalerweise würde das GSM Modul folgendes zurückgeben:
<CR><N>OK<CR><N>. Folgendes bekomme ich angezeigt über uart1:
at


    OK123usw
Kann mir jemand helfen?

[Sven P. (Gast)]

Zum Stil: In meinen Augen gruselig, total wüst eingerückt, oder auch 
garnicht :-)

Du hast dir da eine wunderbare 'Race Condition' konstruiert: stell dir 
mal vor, zwei Bytes kommen schneller nacheinander an, als du sie in der 
Hauptschleife abfragst. Erweitere deine Empfängerei mal durch einen 
gescheiten Ringpuffer o.ä.

[Thomas (Gast)]

Also meinst du ich sollt die ganze Empfängerrei in der ISR Routine 
abhandeln?
Ich dachte nur es wäre besser so um die ISR Routine so kurz wie möglich 
zu halten

[Marius W. (mw1987)]

Ist es ja auch. Du bastelst dir einfach zwei Ringpuffer mit ein paar 
Byte Kapazität. Einen fürs Senden, einen fürs Empfangen. Dann ließt du 
in den ISRs nur kurz das nächste Byte ausm Ringpuffer oder legst es 
hinein. Den Rest erledigst du in der Hauptschleife.

Aber warum nimmst du nicht einfach ne vorgefertigte UART-Implementierung 
zb. von Peter Fleury oder so?

MfG
Marius

[Justus S. (jussa)]

Thomas wrote:
> Also meinst du ich sollt die ganze Empfängerrei in der ISR Routine
> abhandeln?
> Ich dachte nur es wäre besser so um die ISR Routine so kurz wie möglich
> zu halten

du sollst in der ISR die empfangenen auch nicht komplett auswerten, 
sondern in einen Puffer schreiben...eine kurze Interrupt-Routine bringt 
nix, wenn du deswegen Bytes verlierst...

[Thomas (Gast)]

Wo kann ich denn sowas finden?

[Marius W. (mw1987)]

Ich hab dir doch schon den entscheidenden Tipp gegeben. UART-Lib von 
Peter Fleury. Wenn du mal eine Suchmaschine bedienst, wirst du sie 
finden.

MfG
Marius

[Thomas (Gast)]

Habs gefunden! Vielen Dank

[Peter D. (peda)]

Thomas wrote:

> ISR(TIMER2_OVF_vect)
> {
>   time++;
>   itoa(time,li,10);
>   if(time>=0)
>   uart1_puts(li);
> }

Ja das ist extrem schlechter Stil, weil es die CPU lange blockiert.
Du darfst Dich nicht wundern, wenn dadurch andere Interrupts verloren 
gehen.

Unterfunktionsaufrufe in Interrupts möglichst vermeiden, weil dann 
sämtliche Register gesichert werden müssen.

itoa dürfte 4 Divisionen benötigen, ist aufm AVR ein Langläufer, daher 
besser im Main.

UART-Ausgaben generell nicht in Interrupts, da auch Langläufer.
Bzw. wenn gepuffert, kanns sogar nen Deadlock geben. Der Puffer kann im 
Sendeinterrupt nicht geleert werden, da Du bereits in nem Interrupt 
bist.


Peter

[Johann L. (gjlayde)]

Thomas wrote:
> Ich dachte nur es wäre besser so um die ISR Routine so kurz wie möglich
> zu halten

So kurz wie möglich, aber eben auch so lang wie nötig :-)

Ne ISR, die so kurz ist wie möglich, ist leer....

Johann

[Jörg W. (dl8dtl) (Moderator)]

Johann L. wrote:

> Ne ISR, die so kurz ist wie möglich, ist leer....

EMPTY_ISR(...);

 :-)

(Hat zuweilen Sinn: wenn man einen Interrupt nur benötigt, um aus
dem Schlaf aufzuwachen, es aber nach dem Aufwachen egal ist, was
genau der Auslöser dafür war.)