www.mikrocontroller.net

Forum: Mikrocontroller und Digitale Elektronik Problem mit if-Anweisung im USART-Receive-Interrupt AtMega128


Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Hi!
Ich bins wiedermal.
Ich habe folgendes Problem:
Im angeführten Quellcode funktioniert die if-Anweisung für Pause und 
pausiert wird ausgegeben, jedoch scheint es so, als würde der Rest nicht 
passieren. Der Timer läuft nach wie vor weiter und auch, wenn ich Weiter 
oder Stop eingebe scheint es so, als würde er, weil w=0 nicht in die 
andere if-Anweisung hupfen.
Habt ihr vl eine Lösung?
SIGNAL(SIG_USART0_RECV)
{
  char itr[10];
  int w=0;

  o=UDR0;
  if ((o != 0x0D) && (o != 0x0A)) 
  {
    buffer[s++]=o;
  }
  else if((o == 0x0D))
  {
    buffer[s]=0;
    if(strcmp(buffer, "Pause")==0)
    {
      TIFR = (0<<TOV0);             // Timer Interrupt Flag Register --> disable
      w=1;
      s=0;
      USART_Transmit_String("pausiert");
    }
    else if(w==0) s=100;
    if(w==1)
    {
      if(strcmp(buffer, "Stop")==0) //Stop
      {  
        USART_Transmit_String("gestopt");
        w=0;
        flag=1;
        s=0;
        TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
      }
      else if(strcmp(buffer, "Weiter")==0) //Weiter  
      {  
        USART_Transmit_String("fortf");
        w=0;
        s=0;
        TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
      }
    }
  }
}

Vielen Dank schon mal!
Lg

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:

> Im angeführten Quellcode funktioniert die if-Anweisung für Pause und
> pausiert wird ausgegeben, jedoch scheint es so, als würde der Rest nicht
> passieren.

> Der Timer läuft nach wie vor weiter

Logisch. Niemand stoppt den Timer.

> und auch, wenn ich Weiter
> oder Stop eingebe scheint es so, als würde er, weil w=0 nicht in die
> andere if-Anweisung hupfen.

Klar ist w dann immer 0.

w ist eine lokale Variable und wird als solche beim Betreten der 
FUnktion immer wieder neu erzeugt und mit 0 initialisiert.

Autor: Stefan Ernst (sternst)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
> Der Timer läuft nach wie vor weiter

Klar, weder kannst du über TIFR den Timer anhalten, noch kannst du dort 
den Interrupt disablen/enablen. Letzteres passiert in TIMSK.

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Das mit der Variable w war eine Unaufmerksamkeit meinerseits.
Das heißt um den Timer abzuschalten müsste ich TIMSK = (0<<TOIE0); 
setzen. Bleibt dann der Timer auch genau in dem Zustand in dem er war?
Wenn ich das richtig verstanden habe, wird der Interrupt dann gar nicht 
mehr aufgerufen, bis ich es halt wieder 1 setze, oder?

lg

Autor: Oliver (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:
> Das heißt um den Timer abzuschalten müsste ich TIMSK = (0<<TOIE0);
> setzen. Bleibt dann der Timer auch genau in dem Zustand in dem er war?
Nein.
Nein.

Hast du schon mal nachgesehen, was im Datenblatt zum Thema Timer so 
alles steht? Das ist zwar auf englisch, aber da musst du durch.

Geheimtipp: AVR-Tutuorial.

Oliver

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Ja ich hab ja eben gerade im Datenblatt nachgesehen. :-(
Werd nochmal schauen und dann meine Erkenntnisse posten.
lg

Autor: Lothar Miller (lkmiller) (Moderator) Benutzerseite
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Dein Problem liegt aber grundlegend woanders. Denn sowas macht man nicht 
in einem Interrupt:
    if(strcmp(buffer, "Pause")==0)
      :
      USART_Transmit_String("pausiert");
Solche Verwaltungsaufgaben macht man in der Main-Loop. Interruptroutinen 
sind prinzipiell kurz und knackig zu halten.

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Also meiner Meinung nach passt das so, zur Sicherheit könnte ich TCNTO 
noch sichern und danach wieder hineinschreiben, aber generell dürfte der 
Interrupt dann nicht aufgerufen werden. Eine andere Möglichkeit hab ich 
nicht gefunden.

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Lothar Miller schrieb:
> Dein Problem liegt aber grundlegend woanders. Denn sowas macht man nicht
> in einem Interrupt:
>
>     if(strcmp(buffer, "Pause")==0)
>       :
>       USART_Transmit_String("pausiert");
> 
> Solche Verwaltungsaufgaben macht man in der Main-Loop. Interruptroutinen
> sind prinzipiell kurz und knackig zu halten.

Das weiß ich, dass ein Interrupt kurz zu halten ist, aber mir fällt 
keine andere Möglichkeit ein, wo ich es hineingebe, sodass es trotzdem 
sofort reagiert. Main ist keine Lösung, da andere Funktionen aufgerufen 
werden.

Autor: Lothar Miller (lkmiller) (Moderator) Benutzerseite
Datum:

Bewertung
0 lesenswert
nicht lesenswert
> Main ist keine Lösung, da andere Funktionen aufgerufen werden.
Und trotzdem gehört es genau dort hinein.
Wenn in deiner Mainloop andere Funktionen dein Programm blockieren 
können, dann ist dein Konzept falsch. Auch die Mainloop sollte in 
absehbarer Zeit (mein Vorgabe ist da max. 5ms) einmal durchlaufen 
werden.

> sodass es trotzdem sofort reagiert.
Definiere "sofort".
Welche Zeit ist das? 1ms, 10ms, 100ms?

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Umso schneller desto Besser, eine genaue Zeitvorgabe hab ich nicht.

Ich zeigt dir mal das ganze Programm:
/*!
    @header Motorsteuerung.c
    @autor  Oliver Kratochwil
    @discussion Das Programm dient fr die volle Steuerung des Magnetfeldscannertisches.
  PortB=X-Ebene+Y-Ebene
  pin 2&5=enable
  Auch alle anderen Ports der Motoren sind in ihrer Pinbelegung ident
*/

#include <util/delay.h>  
#define F_CPU 16000000UL   //Frequenz
#define FOSC 16000000// Clock Speed 
#define BAUD 9600 
#define MYUBRR (FOSC/16/BAUD)-1 

#include <avr/io.h>     //Deklaration
#include <util/delay.h>
#include<avr/signal.h>    //fuer Interrupt
#include<avr/interrupt.h>
#include <stdio.h>
#include <string.h>
#include "USART.h"

static char buffer[20];
static char o=0;
volatile int s=0;
static int DURCHLAUFE=200; //sonst 255
static int DURCHLAUFEG=200; // = Frequenz nach dem Teiler / gewnschte Frequenz
static char SCHRITT=1;
static char enablex=0;
static char enabley=0;
static long SCHRITTG=70;    // entspricht ca. 1 cm bei unserem Tisch
static char richtungx;
static char richtungy;
static char flag;

SIGNAL(SIG_USART0_RECV)
{
  char itr[10];
  int w;

  o=UDR0;
  if ((o != 0x0D) && (o != 0x0A)) 
  {
    buffer[s++]=o;
    w=0;
  }
  else if((o == 0x0D))
  {
    buffer[s]=0;
    if(strcmp(buffer, "Pause")==0)
    {
      TIMSK = (0<<TOIE0);
      //TIFR = (0<<TOV0);             // Timer Interrupt Flag Register --> disable
      w=1;
      s=0;
      USART_Transmit_String("pausiert");
    }
    else if(w==0) s=100;
    if(w==1)
    {
      if(strcmp(buffer, "Stop")==0) //Stop
      {  
        USART_Transmit_String("gestopt");
        w=0;
        flag=1;
        s=0;
        TIMSK = (1<<TOIE0);
        //TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
      }
      else if(strcmp(buffer, "Weiter")==0) //Weiter  
      {  
        USART_Transmit_String("fortf");
        w=0;
        s=0;
        TIMSK = (1<<TOIE0);
        //TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
      }
    }
  }
}

/*SIGNAL(SIG_INTERRUPT0)
{  
  TIFR = (0<<TOV0);             // Timer Interrupt Flag Register --> disable
  int w=0;
  //lcd_clrscr();
    //lcd_puts("Die Messung  \n");
     //lcd_puts("wurde unterbrochen");
  delayms(1000);
  while(w==0)
  {
    if((PIND&0x02)==0x02) //Stop
    {  
      w=1;
      flag=1;
    }
    else if((PINC&0x20)==0x20) //Weiter  
    {
      w=1;
      //lcd_clrscr();
          //lcd_puts("Die Messung  \n");
      //lcd_puts("wird fortgefuehrt");
    }
  }
  TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
}*/

ISR(SIG_OVERFLOW0)    //Timer fr die Motorsteuerung
{    
    if(richtungx==1) {PORTB = PORTB |(1 << PB1);}    //Richtungsabfrage x
    else  {PORTB = PORTB &~ (1 << PB1);}
    if(richtungy==1) {PORTB = PORTB | (1 << PB4);}    //Richtungsabfrage y
    else  {PORTB = PORTB &~ (1 << PB4);}
    /*if(enablex==1)  {PORTB = PORTB | (1 << PB2);}    //Enableabfrage x
    else  {PORTB = PORTB & ~( 1 << PB2 );}
    if(enabley==1)  {PORTB = PORTB | (1 << PB5);}    //Enableabfrage y
    else  {PORTB = PORTB &~ (1 << PB5);}*/
    
    if(enablex==1) PORTB ^= (1 << PB0) ;      //Toggeln fuer Takt
    if(enabley==1) PORTB ^= (1 << PB3) ;      //Toggeln fuer Takt

    TCNT0= 256 - DURCHLAUFE;     //Startwert von Timer0; ft=fp/DURCHLAUFE(=(256-Startwert))

    SCHRITT=SCHRITT + 1;      //Z‰hlt einen Schritt dazu, fuer die Abfrage der get‰tigten Schritte
}

void delayms(int ms) 
{
  int id;
  for(id=0; id<=ms; id++)
    _delay_ms(1);
}

void zeilenscan(int laengex, int laengey)    //Steuert die zwei Motoren in einem Zeilenscannverfahren. Zeilen sind durch ez und iz vernderbar
{  
  int x;  
  int y;

  PORTB = PORTB | (1 << PB2);    //Enable f¸r x aktivieren
  PORTB = PORTB | (1 << PB5);    //Enable f¸r y aktivieren

  for(y=1; y<=(laengey/2); y++)      //Schleife f¸r die Bewegung in der Y-Ebene (positive Richtung)
  {
  for(x=1; x<=laengex; x++)      //Schleife f¸r die Bewegung in der X-Ebene (positive Richtung)
  {
    richtungx=1;
    enablex=1;
    SCHRITT=1;
    while(SCHRITT<=SCHRITTG)
    {
    }
    enablex=0;
    if(flag>0) return;
  }
  richtungy=1;
  richtungx=0;
  enabley=1;
  enablex=1;
  SCHRITT=1;
  while(SCHRITT<=SCHRITTG)
  {
  }
  enabley=0;
  enablex=0;
  for(x=1; x<=laengex; x++)      //Schleife f¸r die Bewegung in der X-Ebene (negative Richtung)
  {
    richtungx=0;
    enablex=1;
    SCHRITT=1;
    while(SCHRITT<=SCHRITTG)
    {
    }
    enablex=0;
    if(flag>0) return;
  }
  richtungy=1;
  richtungx=0;
  enablex=1;
  enabley=1;
  SCHRITT=1;
  while(SCHRITT<=SCHRITTG)
  {
  }
  enablex=0;
  enabley=0;
  if(flag>0) return;
  }
  for(x=1; x<=laengex; x++)      //Schleife f¸r die Bewegung in der X-Ebene (negative Richtung)
  {
    richtungx=1;
    enablex=1;
    SCHRITT=1;
    while(SCHRITT<=SCHRITTG)
    {
    }
    enablex=0;
    if(flag>0) return;
  }
}

void fourier(int laengex, int laengey)
{
  int x;  
  int y;

  PORTB = PORTB | (1 << PB2);    //Enable f¸r x aktivieren
  PORTB = PORTB | (1 << PB5);    //Enable f¸r y aktivieren
  
  for(y=1; y<=laengey; y++)      //Schleife f¸r die Bewegung in der Y-Ebene (positive Richtung)
  {
    richtungy=1;
    richtungx=0;
    enabley=1;
    enablex=1;
    SCHRITT=1;
    while(SCHRITT<=SCHRITTG)
    {
    }
    enabley=0;
    enablex=0;
    if(flag>0) return;
  }
  for(x=1; x<=laengex; x++)      //Schleife f¸r die Bewegung in der X-Ebene (negative Richtung)
  {
    richtungx=1;
    enablex=1;
    SCHRITT=1;
    while(SCHRITT<=SCHRITTG)
    {
    }
    enablex=0;
    if(flag>0) return;
    }
}

void einlesen()
{  
  int laenge[3][2];
  int laengex;
  int laengey;
  int x=0;
  int y=0;
  char prog[10];
  char st[10];

  flag=0;
  PORTB = PORTB & ~( 1 << PB2 );    //Enablex deaktivieren
  PORTB = PORTB & ~( 1 << PB5 );    //Enabley deaktivieren
  
  while(y==0)
  {
  while(s!=100){}
  strcpy(st, buffer);
  if(strcmp(st, "gui") ==0) y=1;
  s=0;
  }

  USART_Transmit_String("xko");
  while(s!=100)
  {
  }
  laengex=atoi(buffer);
  s=0;

  USART_Transmit_String("yko");
  while(s!=100)
  {
  }
  laengey=atoi(buffer);
  s=0;

  while(x==0)
  {
    if(s==100)
    {
      strcpy(prog, buffer);
      if(strcmp(prog,"zeil") ==0 ) 
      {
        s=0; 
        zeilenscan(laengex,laengey); 
        x=1;
      }
      if(strcmp(prog,"four") ==0 ) 
      {
        s=0; 
        fourier(laengex,laengey); 
        x=1;
        }
      else s=0;
    }

  }
  
}

int main(void)
{
  TCCR0 = (0<<CS00)|(1<<CS01)|(1<<CS02) ; // (Teiler) Register richtig setzen fuer den Timer0, dzt. Frequenz = f/128
  TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
  TIMSK = (1<<TOIE0);
  TCNT0= 256 - DURCHLAUFE;         //Startwert von Timer0; ft=fp/DURCHLAUFE(=(256-Startwert))
  USART_Init(MYUBRR); 
  sei();  //Interrupts aktivieren

  DDRB=0xFF;

  while (1)
  {
    einlesen();
  }  
}


Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:
> Also meiner Meinung nach passt das so, zur Sicherheit könnte ich TCNTO
> noch sichern und danach wieder hineinschreiben, aber generell dürfte der
> Interrupt dann nicht aufgerufen werden. Eine andere Möglichkeit hab ich
> nicht gefunden.

Dann schau noch einmal im Dtaenblatt nach, was da zum Thema 'Prescaler' 
steht. OK. Es steht dort nicht unbedingt explizit dort, wie man den 
Timer anhält, aber mit ein bischen Querdenken kommt man drauf.

Und noch ein Nein.
So:
   TIMSK = (0<<TOIE0);
löscht man grundsätzlich kein Bit in einem Register. Genauso wie man so
   TIMSK = (1<<TOIE0);
kein Bit in einem Register setzt.

http://www.mikrocontroller.net/articles/Bitmanipulation

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Dein 'w'-Problem existiert immer noch.

Welche Aufgabe hat denn diesess 'w' eigentlich? Alles wäre viel 
einfacher, wenn du vernünftige Variablennamen benutzen würdest und nicht 
w, o, s und was weiß ich noch alles.

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Du hast 100-tausend Variablen, die nicht global sein müssen , aber 
diejenige, die global sein muss (oder zumindest Funktions-static) ist es 
nicht (nämlich w)

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Karl heinz Buchegger schrieb:
> Dann schau noch einmal im Dtaenblatt nach, was da zum Thema 'Prescaler'
> steht. OK. Es steht dort nicht unbedingt explizit dort, wie man den
> Timer anhält, aber mit ein bischen Querdenken kommt man drauf.

An das hab ich natürlich nicht gedacht. DAnke

> Und noch ein Nein.
> So:
>    TIMSK = (0<<TOIE0);
> löscht man grundsätzlich kein Bit in einem Register. Genauso wie man so
>    TIMSK = (1<<TOIE0);
> kein Bit in einem Register setzt.

Nicht? Dachte, da hab ich endlich mal was richtig?
Ich hab leider noch sehr viel Nachholbedarf.

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:

>> Und noch ein Nein.
>> So:
>>    TIMSK = (0<<TOIE0);
>> löscht man grundsätzlich kein Bit in einem Register. Genauso wie man so
>>    TIMSK = (1<<TOIE0);
>> kein Bit in einem Register setzt.
>
> Nicht? Dachte, da hab ich endlich mal was richtig?

Dann frag ich mich, wer dir diesen Code geschrieben hat :-)
ISR(SIG_OVERFLOW0)    //Timer fr die Motorsteuerung
{    
    if(richtungx==1) {PORTB = PORTB |(1 << PB1);}    //Richtungsabfrage x
    else  {PORTB = PORTB &~ (1 << PB1);}

denn hier ist es richtig. So macht man das (abgesehen von der ausseren 
Form dieses Codestückes. Die ist scheuslich und nicht wirklich so, dass 
das ganze übersichtlich wird)

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Karl heinz Buchegger schrieb:
> Dein 'w'-Problem existiert immer noch.
>
> Welche Aufgabe hat denn diesess 'w' eigentlich? Alles wäre viel
> einfacher, wenn du vernünftige Variablennamen benutzen würdest und nicht
> w, o, s und was weiß ich noch alles.

Ich will nicht all zu lange Variablennamen verwenden, aber du hast 
natürlich recht. Die Namen sollten auch auch Sinn machen.
Ja das mit dem W ist blöd, weiß schon, ich muss sie mal statisch machen. 
Werd das gleich mal tun und dann probieren.

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:

> Ich will nicht all zu lange Variablennamen verwenden, aber du hast
> natürlich recht. Die Namen sollten auch auch Sinn machen.
> Ja das mit dem W ist blöd, weiß schon, ich muss sie mal statisch machen.
> Werd das gleich mal tun und dann probieren.

und wenn du schon dabei bist:
Formatier dir den Code um. Du stellst dir mit deiner Einrückstrategie 
und deinem unübersichtlichen Code und den viel zu kurzen Variablennamen 
nur selbst ein Bein.

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Karl heinz Buchegger schrieb:
> Du hast 100-tausend Variablen, die nicht global sein müssen , aber
> diejenige, die global sein muss (oder zumindest Funktions-static) ist es
> nicht (nämlich w)

Sind wirklich so viele Variablen unnötigerweise als statisch deklariert?

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Karl heinz Buchegger schrieb:
> Dann frag ich mich, wer dir diesen Code geschrieben hat :-)
> ISR(SIG_OVERFLOW0)    //Timer fr die Motorsteuerung
> {
>     if(richtungx==1) {PORTB = PORTB |(1 << PB1);}    //Richtungsabfrage x
>     else  {PORTB = PORTB &~ (1 << PB1);}
>
> denn hier ist es richtig. So macht man das (abgesehen von der ausseren
> Form dieses Codestückes. Die ist scheuslich und nicht wirklich so, dass
> das ganze übersichtlich wird)

Den hab ich geschrieben. Dank eurer Hilfe.
Aber das brauch ich doch nur so, wenn ich den Rest im Register nicht 
ändern will. In dem Fall vom Timer änder ich ja nur ein Bit und der Rest 
ist doch egal, oder? In der Main hab ich es ja auch immer so gehalten 
und es hat gefunkt.
Hab da etwas falsch verstanden?

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Karl heinz Buchegger schrieb:
> und wenn du schon dabei bist:
> Formatier dir den Code um. Du stellst dir mit deiner Einrückstrategie
> und deinem unübersichtlichen Code und den viel zu kurzen Variablennamen
> nur selbst ein Bein.

Wie meinst, aber gerade durch das richtige Einrücken wird es ja 
übersichtlicher. Oder hab ich das auch falsch gemacht? Ich weiß ein paar 
sachen sind noch nicht wirklich formatiert.
Das mit den Variablennamen werd ich auch in Angriff nehmen und wenn es 
geht heute noch alles ändern.

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:

> Aber das brauch ich doch nur so, wenn ich den Rest im Register nicht
> ändern will. In dem Fall vom Timer änder ich ja nur ein Bit und der Rest
> ist doch egal, oder?

Solange bis du bei der nächsten Codeerweiterung auch noch ein anderes 
Bit in diesem Register benötigst. Dann ist das Gejammere groß, dass sich 
dieses Bit irgendwo 'magisch' selbst verstellt.

Man tatscht immer nur das eine Bit an, welches man ändern will und nicht 
alles. Das ist 'defensives Programmieren' - vorausschauend arbeiten, so 
dass man auch in der Zukunft nicht den halben Code wegen einer 
Erweiterung ändern muss.

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Das mit dem Pause, Weiter und Stop funkt jetzt aber einwandfrei. 
Anscheinend passt das doch so.

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
> Man tatscht immer nur das eine Bit an, welches man ändern will und nicht
> alles. Das ist 'defensives Programmieren' - vorausschauend arbeiten, so
> dass man auch in der Zukunft nicht den halben Code wegen einer
> Erweiterung ändern muss.

Aber da mir ja hier die anderen Bits egal funkt es, oder? Ich werde das 
aber auch noch ändern. Ich merke immer öfter, dass Programmieren nicht 
zu meinen Spezialgebieten zählt, gg

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:
> Karl heinz Buchegger schrieb:
>> und wenn du schon dabei bist:
>> Formatier dir den Code um. Du stellst dir mit deiner Einrückstrategie
>> und deinem unübersichtlichen Code und den viel zu kurzen Variablennamen
>> nur selbst ein Bein.
>
> Wie meinst, aber gerade durch das richtige Einrücken wird es ja
> übersichtlicher. Oder hab ich das auch falsch gemacht? Ich weiß ein paar
> sachen sind noch nicht wirklich formatiert.

Ich hab noch mal drübergescannt. Wenigstens hast du es einigermassen 
konsequent gemacht.
Trotzdem: auch nach einem if kommt der abhängige Teil in die nächste 
Zeile eingerückt.


SIGNAL(SIG_USART0_RECV)
{
  char nextChar;

  nextChar = UDR0;

  if( nextChar == '\r' )      // Linefeed wird generell ignoriert
    return;

  if( nextChar != '\n' ) {    // alles andere wird dranngehängt
    buffer[s++] = nextChar;
    return;
  }

  // bleibt nur noch '\n'
  // Zeile ist vollständig, ein paar Schlüsselwörter gleich jetzt auswerten
  buffer[s] = '\0';

  if( strcmp(buffer, "Pause") == 0)
  {
    TIMSK &= ~(1<<TOIE0);   // Overflow für den Timer abschalten
    buffer[0] = '\0';       // damit einlesen() nichts auswertet
    s = 0;
    USART_Transmit_String("pausiert");
  }

  else if( strcmp(buffer, "Stop") == 0 ) 
  {  
    TIMSK |= (1<<TOIE0);    // Overflow Interrupt wieder erlauben
    buffer[0] = '\0';       // damit einlesen() nichts auswertet
    s = 0;
    USART_Transmit_String("gestoppt");
  }

  else if( strcmp(buffer, "Weiter") == 0 )
  {  
    TIMSK |= (1<<TOIE0);
    buffer[0] = '\0';       // damit einlesen() nichts auswertet
    s = 0;
    USART_Transmit_String( "fortf" );
  }

  else
    s = 100;                // wenn s gleich 100 ist, fängt einlesen() mit
                            // der Auswertung an. Warum auch immer.
}

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:

> aber auch noch ändern. Ich merke immer öfter, dass Programmieren nicht
> zu meinen Spezialgebieten zählt, *gg*

Gerade dann solltest du so defensiv wie nur möglich arbeiten.
Es sind die Nebeneffekte die einem bei der Programmierung das Leben 
schwer machen und irgendwann nicht mehr beherrschbar sind. Dazu gehören 
zu viele Flags mit immer kryptischeren Auswertungen, deren Geflecht 
irgendwann nicht mehr durchschaubar ist und dazu gehören auch 
Anweisungen, die ausser dem Gewollten noch andere Dinge machen, wie zb 
Bits setzen/löschen die sie eigentlich in Ruhe lassen sollten.

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Karl heinz Buchegger schrieb:
> Ich hab noch mal drübergescannt. Wenigstens hast du es einigermassen
> konsequent gemacht.
> Trotzdem: auch nach einem if kommt der abhängige Teil in die nächste
> Zeile eingerückt.

>
> SIGNAL(SIG_USART0_RECV)
> {
>   char nextChar;
> 
>   nextChar = UDR0;
> 
>   if( nextChar == '\r' )      // Linefeed wird generell ignoriert
>     return;
> 
>   if( nextChar != '\n' ) {    // alles andere wird dranngehängt
>     buffer[s++] = nextChar;
>     return;
>   }
>
Die bieden Zeilen verstehe ich nicht ganz. Wozu sind die jetzt da? Der 
Linefeed und der /n geben ja nur an, dass das Sende beenden ist bzw. das 
wort zu ende ist. Für mich haben ja die Zeichen keine anderen Bedeutung, 
oder?

Jetzt schließe ich den String ja in jeder if-Anweisung nochmal ab. Das 
heißt es ist immer einmal zu viel, oder?
s setze ich 100, damit ich ansonsten warten kann bis eingelesen wurde. 
Kann mir theoretisch auch einen andere Variable auf 1 oder was weis ich 
setzen.

Danke schon mal an alle für die schnellen und hilfreichen Antworten.

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Karl heinz Buchegger schrieb:
> Gerade dann solltest du so defensiv wie nur möglich arbeiten.
> Es sind die Nebeneffekte die einem bei der Programmierung das Leben
> schwer machen und irgendwann nicht mehr beherrschbar sind. Dazu gehören
> zu viele Flags mit immer kryptischeren Auswertungen, deren Geflecht
> irgendwann nicht mehr durchschaubar ist und dazu gehören auch
> Anweisungen, die ausser dem Gewollten noch andere Dinge machen, wie zb
> Bits setzen/löschen die sie eigentlich in Ruhe lassen sollten.

Ich werde den ganzen Code mal in Ruhe überarbeiten und gegebenenfalls 
verbessern. Kann ich ihn dann diese oder spätestens nächste Woche posten 
und ihr sagt mir eure Meinung?
Will nämlich das Forum nicht unbedingt zumüllen.

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:
> Karl heinz Buchegger schrieb:
>> Ich hab noch mal drübergescannt. Wenigstens hast du es einigermassen
>> konsequent gemacht.
>> Trotzdem: auch nach einem if kommt der abhängige Teil in die nächste
>> Zeile eingerückt.
>
>>
>> SIGNAL(SIG_USART0_RECV)
>> {
>>   char nextChar;
>>
>>   nextChar = UDR0;
>>
>>   if( nextChar == '\r' )      // Linefeed wird generell ignoriert
>>     return;
>>
>>   if( nextChar != '\n' ) {    // alles andere wird dranngehängt
>>     buffer[s++] = nextChar;
>>     return;
>>   }
>>
> Die bieden Zeilen verstehe ich nicht ganz. Wozu sind die jetzt da?

Na ja.
Was macht denn ein 'return'?

return -> Ausstieg aus der Funktion. Funktion ist beendet.

Was passiert daher hier?

Wenn ein \r daher kommt dann passiert gar nichts, es wird schlicht und 
ergreifend einfach ignoriert.

Alles andere: Solange es kein \n ist, wird einfach an den String 
angehängt und das wars dann -> Funktion ist fertig, nichts wie raus

\r und  \n sind die C-Schreibweisen für 0x0D und 0x0A


Ziel des ganzen ist es, da nicht 15 ineinander verschachtelte if zu 
erhalten, die einem die Übersicht rauben.
Auch wenn die generelle Empfehlung ist, mitten in einer Funktion keine 
returns zu machen, so gibt es doch eine Ausnahme:

Funktionen prüfen am Anfang gerne irgendwelche Dinge ab. zb Fehler oder 
sonstige Nebenbedingungen. Und wenn die nicht erfüllt sind, gehts sofort 
wieder raus. Dann ist ein return durchaus ok. Er verhindert, dass alles 
zu einer
   if
      if
         if
             if

Orgie ausartet. Am Anfang der Funktion feststellen ob es was zu tun gibt 
und wenn nicht, wird die Funktion sofort wieder verlassen.

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Karl heinz Buchegger schrieb:
> Na ja.
> Was macht denn ein 'return'?
>
> return -> Ausstieg aus der Funktion. Funktion ist beendet.
>
> Was passiert daher hier?
>
> Wenn ein \r daher kommt dann passiert gar nichts, es wird schlicht und
> ergreifend einfach ignoriert.
>
> Alles andere: Solange es kein \n ist, wird einfach an den String
> angehängt und das wars dann -> Funktion ist fertig, nichts wie raus
>
> \r und  \n sind die C-Schreibweisen für 0x0D und 0x0A

Ich weiß, hab mir das eh gerade nochmal durchgedacht. Aber mit return 
beendet er doch den ganzen Interrupt und würde den String nicht 
abschließen und auch nicht auf Pause etc, prüfen.
Wäre nicht break richtig?

Autor: Karl Heinz (kbuchegg) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:

> Ich weiß, hab mir das eh gerade nochmal durchgedacht. Aber mit return
> beendet er doch den ganzen Interrupt und würde den String nicht
> abschließen und auch nicht auf Pause etc, prüfen.

Solange kein \n daher kommt, kann deiner Vereinbarung nach der String 
gar nicht fertig sein. Erst ein \n beendet den String! Und der Fall ist 
ja ausgenommen :-)

   if( nextChar != '\n' ) {

Autor: Grrrr (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Wenn Du zufällig in der Menge am Strassenrand beim Fastnachtsumzug 
stehst,
wo Erdbeere, Apfel und Haselnuss geschmissen wird...

Wenn Du zufällig gerade eine Erdbeerdiät machst...

Wenn Du etwas auffängst, es als Haselnuss erkennst und der braunäugigen 
Schönheit neben Dir schenkst...

Reisst Du es ihr dann anschliessend noch zweimal aus der Hand um zu 
prüfen ob es nicht eine Erdbeere oder ein Apfel ist?

Dann wird es aber nichts mit der Vermehrung!

Autor: Oliver Kra (Firma: TGM) (oliver1990)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Karl heinz Buchegger schrieb:
> Solange kein \n daher kommt, kann deiner Vereinbarung nach der String
> gar nicht fertig sein. Erst ein \n beendet den String! Und der Fall ist
> ja ausgenommen :-)
>
>    if( nextChar != '\n' ) {

Ah, jetzt versteh ich es!
So wird ja das ganze viel schneller, weil ich die ganzen if anweisungen 
nur Prüfe wenn der String abgeschlossen ist. Vielen Dank!
Ich werd mir erlauben den Code zu kopieren, ich hoffe das ist ok gg
Wie gesagt ich werde mich gleich daran machen alles nochmal zu 
überarbeiten und dann mal die neue Version posten, hoffe das ist ok.

Lg und vielen Dank mal jetzt schon an alle!

Autor: Grrrr (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Oliver Kra schrieb:
> ich hoffe das ist ok gg

K.H. hat hier langsam Lizenzansprüche auch wenn es nur Trivialitäten 
sind.

Antwort schreiben

Die Angabe einer E-Mail-Adresse ist freiwillig. Wenn Sie automatisch per E-Mail über Antworten auf Ihren Beitrag informiert werden möchten, melden Sie sich bitte an.

Wichtige Regeln - erst lesen, dann posten!

  • Groß- und Kleinschreibung verwenden
  • Längeren Sourcecode nicht im Text einfügen, sondern als Dateianhang

Formatierung (mehr Informationen...)

  • [c]C-Code[/c]
  • [avrasm]AVR-Assembler-Code[/avrasm]
  • [code]Code in anderen Sprachen, ASCII-Zeichnungen[/code]
  • [math]Formel in LaTeX-Syntax[/math]
  • [[Titel]] - Link zu Artikel
  • Verweis auf anderen Beitrag einfügen: Rechtsklick auf Beitragstitel,
    "Adresse kopieren", und in den Text einfügen




Bild automatisch verkleinern, falls nötig
Bitte das JPG-Format nur für Fotos und Scans verwenden!
Zeichnungen und Screenshots im PNG- oder
GIF-Format hochladen. Siehe Bildformate.
Hinweis: der ursprüngliche Beitrag ist mehr als 6 Monate alt.
Bitte hier nur auf die ursprüngliche Frage antworten,
für neue Fragen einen neuen Beitrag erstellen.

Mit dem Abschicken bestätigst du, die Nutzungsbedingungen anzuerkennen.