Forum: Compiler & IDEs main loop - while (1) - nur mit delay


von Christian R. (crothe)


Lesenswert?

Hallo,

mitten in einem umfangreichen Programm funktioniert plötzlich nichts 
mehr, obwohl ich es nun auf ein Minimum geschrupft habe.
Auswirkung ist, dass meine Hauptschleife nur mit delay 'richtig' 
funktioniert.
Programmierumgebung emacs mit AVRDude und GCC

Mein Programm leitet Zeichen von USART2 per IRQ in einen FIFO. Wass dann 
passiert ist bereits egal, da selbst die FIFO Routine nicht mehr 
aufgerufen wird, ohne das delay von 50ms n der main loop.
Hat jemand eine Idee woran das liegen könnte? Es muss doch ein 
grundlegender Fehler sein, den ich hier mache.

Der ATMEGA2560 arbeitet mit internem Takt. In der Funktion 
"UpdateCParserString" lasse ich eine LED anschalten um zu sehen ob die 
Funktion aufgerufen wird.
Hier die main loop:
1
while(1) 
2
    { 
3
    _delay_ms(50);
4
5
//    SetLED(2,1,1);
6
  
7
    if ((FFLG2 & DATAV)==1)
8
    UpdateCParserString();   //    Update Command Parser String;
9
10
    }

Die ISR des USART:
1
ISR (USART2_RX_vect) 
2
{ 
3
    char c;
4
    uint8_t tmp_sreg;
5
    tmp_sreg = SREG;
6
    cli();
7
    c=UDR2; 
8
    WriteFifo2(c);
9
    SREG = tmp_sreg;
10
}


Nebenbei läuft noch Timer 1, in dessen ISR ein Counter für die Kontrolle 
der LEDs runtergezählt wird.
1
ISR (TIMER1_COMPA_vect)
2
{
3
    uint8_t tmp_sreg;
4
    tmp_sreg = SREG;   // save statusregister 
5
    cli();
6
    ToggleLED(0);
7
    ToggleLED(1);
8
    ToggleLED(2);
9
    ToggleLED(3);
10
    SREG = tmp_sreg; // restore status register
11
}

Wenn ich z.B. das Delay auf 5ms setze wird "Update..." nicht mehr 
aufgerufen. Wenn ich dann die auskommentierte "// SetLed(2,1,1)" wieder 
einhänge geht es auch mit 5ms.
Irgend etwas scheint hier Zeit zu brauchen, aber was??

Gruß
Christian

von Patrick D. (oldbug) Benutzerseite


Lesenswert?

Ist nur ne Vermutung, aber:
1
    if ((FFLG2 & DATAV)==1)

Wo wird denn FFLG2, speziell DATAV gesetzt, bzw, zurückgesetzt?

von Christian R. (crothe)


Lesenswert?

das wird gesetzt wenn in den FIFO geschrieben wird um dem Hauptprogramm 
zu sagen, dass Daten zur Verarbeitung stehen:
1
int WriteFifo2(char c)
2
{
3
    SetLED(0,1,1);
4
5
    if (FFLG2 & FF)             //return 0 if fifo is full
6
    return 0;  
7
    
8
    *wp2 = c;                   //copy variable to fifo
9
    wp2++;
10
    
11
    if (wp2==(fifo2+FSIZE2))    //reset wp at fifo end
12
    {
13
      wp2=fifo2;
14
      FFLG2 |= CA;              //set CARRY0 flag
15
    }
16
17
    FFLG2 |= DATAV;             //set Data available flag
18
19
    if((wp2 == rp2)&(FFLG2 & CA)){
20
    FFLG2 |= FF;                //set fifo full flag
21
    }
22
    
23
    SetLED(1,1,1);
24
25
    return 1;
26
}

von Karl H. (kbuchegg)


Lesenswert?

Ich denke das Problem entsteht hier
1
ISR (USART2_RX_vect) 
2
{ 
3
    char c;
4
    uint8_t tmp_sreg;
5
    tmp_sreg = SREG;
6
    cli();
7
    c=UDR2;

die RXC_vect ISR wird aufgerufen, wenn das entsprechende Interrupt 
Request Flag gesetzt wird. In diesem Fall wird dieses Flag aber erst mit 
dem Zugriff auf UDR2 zurückgesetzt. D.h. durch die vorhergehende 
Freigabe des Interrupts kann es dir passieren, dass innerhalb der 
Abarbeitung der ISR die ISR gleich nochmal aufgerufen wird. Und nochmal 
und nochmal und nochmal.

Wozu machst du das eigentlich?
Lass das SREG in Ruhe und lass die Interrupts abgeschaltet, während du 
eine ISR abarbeitest. Mit solchen Dingen kann man sich ganz wunderbar 
selber ins Knie schiessen.

von Karl H. (kbuchegg)


Lesenswert?

Christian Rothe schrieb:
> das wird gesetzt wenn in den FIFO geschrieben wird um dem Hauptprogramm
> zu sagen, dass Daten zur Verarbeitung stehen:

Gefragt war eigentlich:
Wie ist es definiert? Hast du es volatile gemacht?

von Christian R. (crothe)


Lesenswert?

Karl Heinz Buchegger schrieb:
> Christian Rothe schrieb:
>> das wird gesetzt wenn in den FIFO geschrieben wird um dem Hauptprogramm
>> zu sagen, dass Daten zur Verarbeitung stehen:
>
> Gefragt war eigentlich:
> Wie ist es definiert? Hast du es volatile gemacht?

Es steht in der fifo.h als
1
#define DATAV   1

von Karl H. (kbuchegg)


Lesenswert?

Christian Rothe schrieb:
> Karl Heinz Buchegger schrieb:
>> Christian Rothe schrieb:
>>> das wird gesetzt wenn in den FIFO geschrieben wird um dem Hauptprogramm
>>> zu sagen, dass Daten zur Verarbeitung stehen:
>>
>> Gefragt war eigentlich:
>> Wie ist es definiert? Hast du es volatile gemacht?
>
> Es steht in der fifo.h als
>
1
> #define DATAV   1
2
>

Zum mitmeisseln:

WIe ist FFLG2 definiert?

Das ist ja schliesslich die Variable auf die zugegriffen wird! Dass 
DATAV irgendeine Konstante ist, die ein einzelnes Bit davon bezeichnet, 
war wohl allen aus dem Zusammenhang klar.

von Christian R. (crothe)


Lesenswert?

Karl Heinz Buchegger schrieb:
> Ich denke das Problem entsteht hier
>
>
>
> die RXC_vect ISR wird aufgerufen, wenn das entsprechende Interrupt
> Request Flag gesetzt wird. In diesem Fall wird dieses Flag aber erst mit
> dem Zugriff auf UDR2 zurückgesetzt. D.h. durch die vorhergehende
> Freigabe des Interrupts kann es dir passieren, dass innerhalb der
> Abarbeitung der ISR die ISR gleich nochmal aufgerufen wird. Und nochmal
> und nochmal und nochmal.
>
> Wozu machst du das eigentlich?

Das verstehe ich jetzt nicht richtig.
Die Register sichern und mit cli(); die Interrupts ausschalten muesste 
doch richtig sein. Erst mit dem Zurückschreiben der Register werden die 
Interrupts doch wieder freigegeben, oder?

von Karl H. (kbuchegg)


Lesenswert?

Christian Rothe schrieb:
> Das verstehe ich jetzt nicht richtig.
> Die Register sichern und mit cli(); die Interrupts ausschalten muesste
> doch richtig sein.

Was denkst du, wozu du einen Compiler hast, der sich um den Kleinkram 
kümmert?
Überlass das ihm. Damit hast du nichts zu tun.

1
ISR (USART2_RX_vect) 
2
{ 
3
    char c;
4
    c=UDR2; 
5
    WriteFifo2(c);
6
}

oder überhaupt gleich
1
ISR (USART2_RX_vect) 
2
{ 
3
    WriteFifo2( UDR2 );
4
}

und für alle anderen ISR genauso.

von Christian R. (crothe)


Lesenswert?

sorry, zu schnell überlesen.

FFLG ist auch in der fifo.h definiert als int:
1
int FFLG2;  //FIFO 2 flag register

von Karl H. (kbuchegg)


Lesenswert?

Christian Rothe schrieb:
> sorry, zu schnell überlesen.
>
> FFLG ist auch in der fifo.h definiert als int:
>
1
> int FFLG2;  //FIFO 2 flag register
2
>

da muss ein volatile hin

FAQ: Was hat es mit volatile auf sich

von Christian R. (crothe)


Lesenswert?

> FAQ: Was hat es mit volatile auf sich


OK, das werde ich mir wohl nochmal ansehen.

Das mit den Interrupts ist generell so? Sorry für die dumme Frage, aber 
auch die Register retterei habe ich irgendwann aus glaube ich dem Forum 
hier. Da gibt es glaube ich irgendwo so eine tolle Anleitung zu allen 
möglich Themen rund um die AVRs. Nagel mich nicht gleich fest, aber ich 
glaube da stand es. Muss ich denn die gesamten CLI() etc weglassen und 
alles dem Compiler überlassen?

von Karl H. (kbuchegg)


Lesenswert?

Christian Rothe schrieb:
>> FAQ: Was hat es mit volatile auf sich
>
>
> OK, das werde ich mir wohl nochmal ansehen.
>
> Das mit den Interrupts ist generell so?

Ja.

> Sorry für die dumme Frage, aber
> auch die Register retterei habe ich irgendwann aus glaube ich dem Forum
> hier. Da gibt es glaube ich irgendwo so eine tolle Anleitung zu allen
> möglich Themen rund um die AVRs.

Wenn du in Assembler programmieren würdest, müsstest du dich darum 
kümmern. Aber in C erledigt das der Compiler.

von Christian R. (crothe)


Lesenswert?

Na dann erstmal vielen Dank.
Ich werde mich mal ans Testen machen mit "Volatile"

Gruß

von Benedikt (Gast)


Lesenswert?

Hmm, also erstmal behandelt C das SREG schon selbst, das brauchst selbst 
nicht zu machen. Das
1
tmp = SREG
2
SREG = tmp
ist also unnötig. Außerdem deaktiviert
1
cli()
 die interrupts, was aber bei allen AVRs nach Sprung in eine ISR eh ohne 
weiteres Zutun der Fall ist, kann also auch wegfallen.

Ansonsten würde ich
1
if(FFLG2 & DATAV)
 schreiben. Einmal gibt das dem Compiler mehr Freiheiten das umzusetzen 
(und ich würde vermuten das es auch mindestens ein
1
cpi
 spart), andererseits ist es so weniger fehleranfällig falls du die 
DATAV aus irgendeinem Grund in ein anderes bit schieben solltest.

Ich schließe mich aber Karl Heinz an, in den isolierten Code-Stücken die 
du gepostet hast finde ich auf Anhieb keinen Fehler, weshalb du 
vielleicht am Besten die ganzen beteiligten Dateien hier reinstellst.

von Christian R. (crothe)


Lesenswert?

Hallo Benedikt,

danke für die ausführlichen Worte. Schön, dass ich mich um manches 
garnicht kümmern muss, da es der Compiler macht,... wusste ich nicht an 
einigen Stellen.

Das mit dem volatile hat bis jetzt nicht viel verändert, aber ich muss 
erst weiter prüfen ob Änderungen in der Hauptschleife jetzt zu 
unerklärlichem Verhalten führen (im Moment läuft es).

Die isolierten Codeschnipsel, ... nun ja, der Rest ist durch meine 
Fehlersuche derzeit nicht gerade vorzeigbar :-)
Die Prüfung des FFLG war ursprünglich so wie von Dir oben geschrieben. 
Das war mein erster Versuch bei der Fehlersuche FFLG gezielt auf "1" zu 
prüfen.

von Patrick D. (oldbug) Benutzerseite


Lesenswert?

Ich wollte auf was anderes hinaus:

Die 50ms(?) delay lassen aus meiner Sicht vermuten, dass das DATAV Bit 
zwar gesetzt, aber nie wieder zurückgesetzt wird. In dem if(..) ist es 
deshalb in jedem Durchlauf der while(1) gesetzt, oder habe ich irgendwas 
übersehen?

von Christian R. (crothe)


Lesenswert?

...aber dann müsste ja die UpdateCParserString() permanent aufgerufen 
werden. Tatsächlich wurde (im Moment geht alles wieder glaube ich) sie 
aber nicht angesprungen wenn das delay kleiner war.


werde gleich mal das volatile entfernen um zu sehen, ob das Prog dann 
wieder spinnt.

von Benedikt (Gast)


Lesenswert?

Aber selbst wenn der test jedes mal true ergibt, müsste die LED doch an 
sein (aber wie gesagt, ohne mehr Code zu sehen...vllt acuh nicht).

Was gesagt wurde zu volatile stimmt denke ich. Die delay-Funktion kennt 
der Compiler nciht (außer dem Prototypen), wenn diese Datei compiliert 
wird, sie könnte also theoretisch FFLG2 ändern, also muss die Variable 
danach jedes Mal neu gelesen werden, was volatile auch ohne delay 
bewirkt.

"Wass dann passiert ist bereits egal, da selbst die FIFO Routine nicht 
mehr aufgerufen wird"

Welche FIFO-Routine? Falls du Writefifo2 meinst ist vllt auch sonstwo 
ein Fehler, denn das hieße, dass die ISR nie aufgerufen wird.

von Christian R. (crothe)


Lesenswert?

in der UpdateCParserString() wird das Zeichen verarbeitet und aus dem 
Fifo gelesen durch die ReadFifo(). Dabei wird das Flag zurückgesetzt.
1
int ReadFifo2(char* c)
2
{
3
    if ((rp2==wp2) & (!(FFLG2 & (1 << CA))))
4
  return 0;    //fifo empty
5
 
6
    *c = *rp2;
7
8
    rp2++;
9
    if(rp2==(fifo2+FSIZE2))    //reset rp at fifo end
10
    {
11
  rp2=fifo2;
12
  FFLG2 &= ~CA;
13
    }
14
    
15
    if(rp2==wp2)
16
  FFLG2 &= ~DATAV; //reset data available Flag
17
    
18
    return 1;
19
}


Sorry, den Code darzustellen ist nicht so ganz einfach, da alleine das 
was ich jetzt zusammengekratzt habe schon aus mehreren c-files stammt. 
Es ist sonst zu viel Zeug drinnen, was garnicht angesprochen wird in 
aktuellen Fall.


>Was gesagt wurde zu volatile stimmt denke ich. Die delay-Funktion kennt
>der Compiler nciht (außer dem Prototypen), wenn diese Datei compiliert
>wird, sie könnte also theoretisch FFLG2 ändern, also muss die Variable
>danach jedes Mal neu gelesen werden, was volatile auch ohne delay
>bewirkt.

hmm, das muss ich noch paarmal lesen um es zu dieser Zeit wirklich zu 
verstehen :-)

von Karl H. (kbuchegg)


Lesenswert?

Benedikt schrieb:

> Was gesagt wurde zu volatile stimmt denke ich. Die delay-Funktion kennt
> der Compiler nciht (außer dem Prototypen),

Das stimmt nicht.
_delay_ms ist eine 'Funktion', die darauf angewiesen ist, dass sie 
ge-inlined wird. Das ist wichtig und ist auch der Grund dafür, warum die 
Delayzeiten nur dann einigermassen stimmen, wenn der Compiler optimieren 
darf.

Es könnte jetzt sein, dass ab einer gewissen 'Kürze' der Delayzeit, der 
Compiler dadurch ein paar Register frei kriegt (weil er nicht so viele 
Schleifen ineinander schachteln muss), in denen dann FFLG2 platziert 
werden kann, so dass innerhalb der while Schleife FFLG2 tatsächlich 
nicht nachgeladen werden muss.
Das ist aber nur eine Vermutung dessen, was passiert sein könnte. Ob das 
tatsächlich so ist oder nicht, darüber kann nur ein Studium des 
Assembler Outputs Klarheit bringen.

von Christian R. (crothe)


Lesenswert?

Also zum Tagesabschluss nochmal danke für den kompetenten Input.

Nicht ganz zufriedenstellend muss ich für heute feststellen, dass der 
Fehler nicht mehr reproduzierbar ist, auch nachdem die "notwendigen" 
volatile deklarationen zur Gegenprobe wieder rückgängig gemacht wurden.

Wenn der "Geist" wieder auftaucht versuche ich das nochmal.

==================

von Benedikt (Gast)


Lesenswert?

Karl Heinz Buchegger schrieb:
> Benedikt schrieb:
>
>> Was gesagt wurde zu volatile stimmt denke ich. Die delay-Funktion kennt
>> der Compiler nciht (außer dem Prototypen),
>
> Das stimmt nicht.
> _delay_ms ist eine 'Funktion', die darauf angewiesen ist, dass sie
> ge-inlined wird. Das ist wichtig und ist auch der Grund dafür, warum die
> Delayzeiten nur dann einigermassen stimmen, wenn der Compiler optimieren
> darf.

Du hast Recht, es heißt
1
static inline void _delay_ms(double __ms) __attribute__((always_inline));
 in avr/util.h, das war mir nicht bewusst. Das inlining findet also auch 
ohne Optimierung statt.
Wieder was dazugelernt :)

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.