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


von Oliver K. (Firma: TGM) (oliver1990)


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?
1
SIGNAL(SIG_USART0_RECV)
2
{
3
  char itr[10];
4
  int w=0;
5
6
  o=UDR0;
7
  if ((o != 0x0D) && (o != 0x0A)) 
8
  {
9
    buffer[s++]=o;
10
  }
11
  else if((o == 0x0D))
12
  {
13
    buffer[s]=0;
14
    if(strcmp(buffer, "Pause")==0)
15
    {
16
      TIFR = (0<<TOV0);             // Timer Interrupt Flag Register --> disable
17
      w=1;
18
      s=0;
19
      USART_Transmit_String("pausiert");
20
    }
21
    else if(w==0) s=100;
22
    if(w==1)
23
    {
24
      if(strcmp(buffer, "Stop")==0) //Stop
25
      {  
26
        USART_Transmit_String("gestopt");
27
        w=0;
28
        flag=1;
29
        s=0;
30
        TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
31
      }
32
      else if(strcmp(buffer, "Weiter")==0) //Weiter  
33
      {  
34
        USART_Transmit_String("fortf");
35
        w=0;
36
        s=0;
37
        TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
38
      }
39
    }
40
  }
41
}

Vielen Dank schon mal!
Lg

von Karl H. (kbuchegg)


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.

von Stefan E. (sternst)


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.

von Oliver K. (Firma: TGM) (oliver1990)


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

von Oliver (Gast)


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

von Oliver K. (Firma: TGM) (oliver1990)


Lesenswert?

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

von Lothar M. (Firma: Titel) (lkmiller) (Moderator) Benutzerseite


Lesenswert?

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

von Oliver K. (Firma: TGM) (oliver1990)


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.

von Oliver K. (Firma: TGM) (oliver1990)


Lesenswert?

Lothar Miller schrieb:
> Dein Problem liegt aber grundlegend woanders. Denn sowas macht man nicht
> in einem Interrupt:
>
1
>     if(strcmp(buffer, "Pause")==0)
2
>       :
3
>       USART_Transmit_String("pausiert");
4
>
> 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.

von Lothar M. (Firma: Titel) (lkmiller) (Moderator) Benutzerseite


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?

von Oliver K. (Firma: TGM) (oliver1990)


Lesenswert?

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

Ich zeigt dir mal das ganze Programm:
1
/*!
2
    @header Motorsteuerung.c
3
    @autor  Oliver Kratochwil
4
    @discussion Das Programm dient fr die volle Steuerung des Magnetfeldscannertisches.
5
  PortB=X-Ebene+Y-Ebene
6
  pin 2&5=enable
7
  Auch alle anderen Ports der Motoren sind in ihrer Pinbelegung ident
8
*/
9
10
#include <util/delay.h>  
11
#define F_CPU 16000000UL   //Frequenz
12
#define FOSC 16000000// Clock Speed 
13
#define BAUD 9600 
14
#define MYUBRR (FOSC/16/BAUD)-1 
15
16
#include <avr/io.h>     //Deklaration
17
#include <util/delay.h>
18
#include<avr/signal.h>    //fuer Interrupt
19
#include<avr/interrupt.h>
20
#include <stdio.h>
21
#include <string.h>
22
#include "USART.h"
23
24
static char buffer[20];
25
static char o=0;
26
volatile int s=0;
27
static int DURCHLAUFE=200; //sonst 255
28
static int DURCHLAUFEG=200; // = Frequenz nach dem Teiler / gewnschte Frequenz
29
static char SCHRITT=1;
30
static char enablex=0;
31
static char enabley=0;
32
static long SCHRITTG=70;    // entspricht ca. 1 cm bei unserem Tisch
33
static char richtungx;
34
static char richtungy;
35
static char flag;
36
37
SIGNAL(SIG_USART0_RECV)
38
{
39
  char itr[10];
40
  int w;
41
42
  o=UDR0;
43
  if ((o != 0x0D) && (o != 0x0A)) 
44
  {
45
    buffer[s++]=o;
46
    w=0;
47
  }
48
  else if((o == 0x0D))
49
  {
50
    buffer[s]=0;
51
    if(strcmp(buffer, "Pause")==0)
52
    {
53
      TIMSK = (0<<TOIE0);
54
      //TIFR = (0<<TOV0);             // Timer Interrupt Flag Register --> disable
55
      w=1;
56
      s=0;
57
      USART_Transmit_String("pausiert");
58
    }
59
    else if(w==0) s=100;
60
    if(w==1)
61
    {
62
      if(strcmp(buffer, "Stop")==0) //Stop
63
      {  
64
        USART_Transmit_String("gestopt");
65
        w=0;
66
        flag=1;
67
        s=0;
68
        TIMSK = (1<<TOIE0);
69
        //TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
70
      }
71
      else if(strcmp(buffer, "Weiter")==0) //Weiter  
72
      {  
73
        USART_Transmit_String("fortf");
74
        w=0;
75
        s=0;
76
        TIMSK = (1<<TOIE0);
77
        //TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
78
      }
79
    }
80
  }
81
}
82
83
/*SIGNAL(SIG_INTERRUPT0)
84
{  
85
  TIFR = (0<<TOV0);             // Timer Interrupt Flag Register --> disable
86
  int w=0;
87
  //lcd_clrscr();
88
    //lcd_puts("Die Messung  \n");
89
     //lcd_puts("wurde unterbrochen");
90
  delayms(1000);
91
  while(w==0)
92
  {
93
    if((PIND&0x02)==0x02) //Stop
94
    {  
95
      w=1;
96
      flag=1;
97
    }
98
    else if((PINC&0x20)==0x20) //Weiter  
99
    {
100
      w=1;
101
      //lcd_clrscr();
102
          //lcd_puts("Die Messung  \n");
103
      //lcd_puts("wird fortgefuehrt");
104
    }
105
  }
106
  TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
107
}*/
108
109
ISR(SIG_OVERFLOW0)    //Timer fr die Motorsteuerung
110
{    
111
    if(richtungx==1) {PORTB = PORTB |(1 << PB1);}    //Richtungsabfrage x
112
    else  {PORTB = PORTB &~ (1 << PB1);}
113
    if(richtungy==1) {PORTB = PORTB | (1 << PB4);}    //Richtungsabfrage y
114
    else  {PORTB = PORTB &~ (1 << PB4);}
115
    /*if(enablex==1)  {PORTB = PORTB | (1 << PB2);}    //Enableabfrage x
116
    else  {PORTB = PORTB & ~( 1 << PB2 );}
117
    if(enabley==1)  {PORTB = PORTB | (1 << PB5);}    //Enableabfrage y
118
    else  {PORTB = PORTB &~ (1 << PB5);}*/
119
    
120
    if(enablex==1) PORTB ^= (1 << PB0) ;      //Toggeln fuer Takt
121
    if(enabley==1) PORTB ^= (1 << PB3) ;      //Toggeln fuer Takt
122
123
    TCNT0= 256 - DURCHLAUFE;     //Startwert von Timer0; ft=fp/DURCHLAUFE(=(256-Startwert))
124
125
    SCHRITT=SCHRITT + 1;      //Z‰hlt einen Schritt dazu, fuer die Abfrage der get‰tigten Schritte
126
}
127
128
void delayms(int ms) 
129
{
130
  int id;
131
  for(id=0; id<=ms; id++)
132
    _delay_ms(1);
133
}
134
135
void zeilenscan(int laengex, int laengey)    //Steuert die zwei Motoren in einem Zeilenscannverfahren. Zeilen sind durch ez und iz vernderbar
136
{  
137
  int x;  
138
  int y;
139
140
  PORTB = PORTB | (1 << PB2);    //Enable f¸r x aktivieren
141
  PORTB = PORTB | (1 << PB5);    //Enable f¸r y aktivieren
142
143
  for(y=1; y<=(laengey/2); y++)      //Schleife f¸r die Bewegung in der Y-Ebene (positive Richtung)
144
  {
145
  for(x=1; x<=laengex; x++)      //Schleife f¸r die Bewegung in der X-Ebene (positive Richtung)
146
  {
147
    richtungx=1;
148
    enablex=1;
149
    SCHRITT=1;
150
    while(SCHRITT<=SCHRITTG)
151
    {
152
    }
153
    enablex=0;
154
    if(flag>0) return;
155
  }
156
  richtungy=1;
157
  richtungx=0;
158
  enabley=1;
159
  enablex=1;
160
  SCHRITT=1;
161
  while(SCHRITT<=SCHRITTG)
162
  {
163
  }
164
  enabley=0;
165
  enablex=0;
166
  for(x=1; x<=laengex; x++)      //Schleife f¸r die Bewegung in der X-Ebene (negative Richtung)
167
  {
168
    richtungx=0;
169
    enablex=1;
170
    SCHRITT=1;
171
    while(SCHRITT<=SCHRITTG)
172
    {
173
    }
174
    enablex=0;
175
    if(flag>0) return;
176
  }
177
  richtungy=1;
178
  richtungx=0;
179
  enablex=1;
180
  enabley=1;
181
  SCHRITT=1;
182
  while(SCHRITT<=SCHRITTG)
183
  {
184
  }
185
  enablex=0;
186
  enabley=0;
187
  if(flag>0) return;
188
  }
189
  for(x=1; x<=laengex; x++)      //Schleife f¸r die Bewegung in der X-Ebene (negative Richtung)
190
  {
191
    richtungx=1;
192
    enablex=1;
193
    SCHRITT=1;
194
    while(SCHRITT<=SCHRITTG)
195
    {
196
    }
197
    enablex=0;
198
    if(flag>0) return;
199
  }
200
}
201
202
void fourier(int laengex, int laengey)
203
{
204
  int x;  
205
  int y;
206
207
  PORTB = PORTB | (1 << PB2);    //Enable f¸r x aktivieren
208
  PORTB = PORTB | (1 << PB5);    //Enable f¸r y aktivieren
209
  
210
  for(y=1; y<=laengey; y++)      //Schleife f¸r die Bewegung in der Y-Ebene (positive Richtung)
211
  {
212
    richtungy=1;
213
    richtungx=0;
214
    enabley=1;
215
    enablex=1;
216
    SCHRITT=1;
217
    while(SCHRITT<=SCHRITTG)
218
    {
219
    }
220
    enabley=0;
221
    enablex=0;
222
    if(flag>0) return;
223
  }
224
  for(x=1; x<=laengex; x++)      //Schleife f¸r die Bewegung in der X-Ebene (negative Richtung)
225
  {
226
    richtungx=1;
227
    enablex=1;
228
    SCHRITT=1;
229
    while(SCHRITT<=SCHRITTG)
230
    {
231
    }
232
    enablex=0;
233
    if(flag>0) return;
234
    }
235
}
236
237
void einlesen()
238
{  
239
  int laenge[3][2];
240
  int laengex;
241
  int laengey;
242
  int x=0;
243
  int y=0;
244
  char prog[10];
245
  char st[10];
246
247
  flag=0;
248
  PORTB = PORTB & ~( 1 << PB2 );    //Enablex deaktivieren
249
  PORTB = PORTB & ~( 1 << PB5 );    //Enabley deaktivieren
250
  
251
  while(y==0)
252
  {
253
  while(s!=100){}
254
  strcpy(st, buffer);
255
  if(strcmp(st, "gui") ==0) y=1;
256
  s=0;
257
  }
258
259
  USART_Transmit_String("xko");
260
  while(s!=100)
261
  {
262
  }
263
  laengex=atoi(buffer);
264
  s=0;
265
266
  USART_Transmit_String("yko");
267
  while(s!=100)
268
  {
269
  }
270
  laengey=atoi(buffer);
271
  s=0;
272
273
  while(x==0)
274
  {
275
    if(s==100)
276
    {
277
      strcpy(prog, buffer);
278
      if(strcmp(prog,"zeil") ==0 ) 
279
      {
280
        s=0; 
281
        zeilenscan(laengex,laengey); 
282
        x=1;
283
      }
284
      if(strcmp(prog,"four") ==0 ) 
285
      {
286
        s=0; 
287
        fourier(laengex,laengey); 
288
        x=1;
289
        }
290
      else s=0;
291
    }
292
293
  }
294
  
295
}
296
297
int main(void)
298
{
299
  TCCR0 = (0<<CS00)|(1<<CS01)|(1<<CS02) ; // (Teiler) Register richtig setzen fuer den Timer0, dzt. Frequenz = f/128
300
  TIFR = (1<<TOV0);             // Timer Interrupt Flag Register --> enable
301
  TIMSK = (1<<TOIE0);
302
  TCNT0= 256 - DURCHLAUFE;         //Startwert von Timer0; ft=fp/DURCHLAUFE(=(256-Startwert))
303
  USART_Init(MYUBRR); 
304
  sei();  //Interrupts aktivieren
305
306
  DDRB=0xFF;
307
308
  while (1)
309
  {
310
    einlesen();
311
  }  
312
}

von Karl H. (kbuchegg)


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

von Karl H. (kbuchegg)


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.

von Karl H. (kbuchegg)


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)

von Oliver K. (Firma: TGM) (oliver1990)


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.

von Karl H. (kbuchegg)


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 :-)
1
ISR(SIG_OVERFLOW0)    //Timer fr die Motorsteuerung
2
{    
3
    if(richtungx==1) {PORTB = PORTB |(1 << PB1);}    //Richtungsabfrage x
4
    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)

von Oliver K. (Firma: TGM) (oliver1990)


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.

von Karl H. (kbuchegg)


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.

von Oliver K. (Firma: TGM) (oliver1990)


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?

von Oliver K. (Firma: TGM) (oliver1990)


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?

von Oliver K. (Firma: TGM) (oliver1990)


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.

von Karl H. (kbuchegg)


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.

von Oliver K. (Firma: TGM) (oliver1990)


Lesenswert?

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

von Oliver K. (Firma: TGM) (oliver1990)


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

von Karl H. (kbuchegg)


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.


1
SIGNAL(SIG_USART0_RECV)
2
{
3
  char nextChar;
4
5
  nextChar = UDR0;
6
7
  if( nextChar == '\r' )      // Linefeed wird generell ignoriert
8
    return;
9
10
  if( nextChar != '\n' ) {    // alles andere wird dranngehängt
11
    buffer[s++] = nextChar;
12
    return;
13
  }
14
15
  // bleibt nur noch '\n'
16
  // Zeile ist vollständig, ein paar Schlüsselwörter gleich jetzt auswerten
17
  buffer[s] = '\0';
18
19
  if( strcmp(buffer, "Pause") == 0)
20
  {
21
    TIMSK &= ~(1<<TOIE0);   // Overflow für den Timer abschalten
22
    buffer[0] = '\0';       // damit einlesen() nichts auswertet
23
    s = 0;
24
    USART_Transmit_String("pausiert");
25
  }
26
27
  else if( strcmp(buffer, "Stop") == 0 ) 
28
  {  
29
    TIMSK |= (1<<TOIE0);    // Overflow Interrupt wieder erlauben
30
    buffer[0] = '\0';       // damit einlesen() nichts auswertet
31
    s = 0;
32
    USART_Transmit_String("gestoppt");
33
  }
34
35
  else if( strcmp(buffer, "Weiter") == 0 )
36
  {  
37
    TIMSK |= (1<<TOIE0);
38
    buffer[0] = '\0';       // damit einlesen() nichts auswertet
39
    s = 0;
40
    USART_Transmit_String( "fortf" );
41
  }
42
43
  else
44
    s = 100;                // wenn s gleich 100 ist, fängt einlesen() mit
45
                            // der Auswertung an. Warum auch immer.
46
}

von Karl H. (kbuchegg)


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.

von Oliver K. (Firma: TGM) (oliver1990)


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.

>
1
> SIGNAL(SIG_USART0_RECV)
2
> {
3
>   char nextChar;
4
> 
5
>   nextChar = UDR0;
6
> 
7
>   if( nextChar == '\r' )      // Linefeed wird generell ignoriert
8
>     return;
9
> 
10
>   if( nextChar != '\n' ) {    // alles andere wird dranngehängt
11
>     buffer[s++] = nextChar;
12
>     return;
13
>   }
14
>
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.

von Oliver K. (Firma: TGM) (oliver1990)


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.

von Karl H. (kbuchegg)


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.
>
>>
1
>> SIGNAL(SIG_USART0_RECV)
2
>> {
3
>>   char nextChar;
4
>>
5
>>   nextChar = UDR0;
6
>>
7
>>   if( nextChar == '\r' )      // Linefeed wird generell ignoriert
8
>>     return;
9
>>
10
>>   if( nextChar != '\n' ) {    // alles andere wird dranngehängt
11
>>     buffer[s++] = nextChar;
12
>>     return;
13
>>   }
14
>>
> 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.

von Oliver K. (Firma: TGM) (oliver1990)


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?

von Karl H. (kbuchegg)


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' ) {

von Grrrr (Gast)


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!

von Oliver K. (Firma: TGM) (oliver1990)


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!

von Grrrr (Gast)


Lesenswert?

Oliver Kra schrieb:
> ich hoffe das ist ok gg

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

Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.