C: If-Abfrage spielt verrückt

[Karel M. (marsalek)]

Liebe Kollegen,

habe einen Programmier-Fehler entdeckt, mit dem ich im Moment nicht 
weiter komme. Im Timer-Interrupt inkrementiere ich (jede 100 ms) eine 
Variable, die dann im Hauptprogramm abgefragt wird (counter >= 600), um 
einmal pro Minute etwas durchführen zu können.

Wenn ich mir diesen Counter ausgeben lasse, enthält er fast immer genau 
den Wert 600 aber ab und zu auch noch den Wert 512 und nur diesen. 
Warum? Mit meinem Realtime-Clock (DS1339) habe ich herausgefunden, dass 
diese If-Abfrage zu FRÜH kommt, also nach 51,2 Sekunden nach dem letzten 
Durchlauf.

Diese Werte von 512 kommen unregelmässig, einmal sogar zwei 
hintereinander, einmal ist der Abstand 7 Minuten, einmal 19 Minuten. Ich 
sehe darin keine Regelmässigkeit.

Zu meinem Setup: ATMEGA1284P, IDE: Eclipse mit WinAVR, AVR-Studio4 mit 
JTAGICEII zum Flashen.

Hier ein Auszug aus meinem Programm:

volatile uint16_t  one_hundred_ms_ticks_counter = 0;  
//gets incremented in timer1 compare match interrupt

ISR(INT0_vect)
{ "ADC auslesen" }

// Timer 1 interrupt
ISR(TIMER1_COMPA_vect)    
{
  one_hundred_ms_ticks_counter++;    
}

int main(){
  cli();  //global disable interrupts
  ....
  //start timer
  TCCR1B |= (1<<CS11) | (1<<CS10); // Pre-scaler for Timer1 set to 64 (p.136 data sheet)
  OCR1A=31249; TCCR1B |= (1<<WGM12); TIMSK1|= (1<<OCIE1A);}
  // CTC clear on compare match
  //  with prescaler 64 and 31249
  //  every 100 ms

  EIFR |= (1<<INTF0);    // Delete INT0 flag in case it was set
  INT0_Enable();        // Interrupt 0 on rising edge at PD2

  sei();  //global enable interrupts

  while(1){
    ... tue etwas leichtes :-)

    if(one_hundred_ms_ticks_counter >= 600)   //1 minute
    {
      USART_puts(getTimeStamp());
      USART_puts(itoa(one_hundred_ms_ticks_counter, buff, 10));USART_puts("\r\n");  //GIBT MANCHMAL 512 :-(
      one_hundred_ms_ticks_counter = 0;
    }

Und hier die Ausgabe am Terminal:

2012-04-10 11:50:47
one_hundred_ms_ticks_counter = 600.

2012-04-10 11:51:47
one_hundred_ms_ticks_counter = 600.

2012-04-10 11:52:47
one_hundred_ms_ticks_counter = 600.

2012-04-10 11:53:38
one_hundred_ms_ticks_counter = 512.

2012-04-10 11:54:38
one_hundred_ms_ticks_counter = 600.

Warum funktioniert die IF-Abfrage so komisch?

Vielen Dank für eure Tipps und Hilfe!
Karel

[adfg (Gast)]

Die Abfrage muss atomar erfolgen da sizeof>1 Byte.

[MaWin (Gast)]

Du darfst dir in Abfragen nicht per Interrupt reinpfuschen lassen.

cli();
   if(one_hundred_ms_ticks_counter >= 600)   //1 minute
    {
      USART_puts(getTimeStamp());
      USART_puts(itoa(one_hundred_ms_ticks_counter, buff, 
10));USART_puts("\r\n");  //GIBT MANCHMAL 512 :-(
      one_hundred_ms_ticks_counter = 0;
    }
sei();

[Udo S. (urschmitt)]

MaWin schrieb:
> Du darfst dir in Abfragen nicht per Interrupt reinpfuschen lassen.
>
> cli();
>    if(one_hundred_ms_ticks_counter >= 600)   //1 minute
>     {
>       USART_puts(getTimeStamp());
>       USART_puts(itoa(one_hundred_ms_ticks_counter, buff,
> 10));USART_puts("\r\n");  //GIBT MANCHMAL 512 :-(
>       one_hundred_ms_ticks_counter = 0;
>     }
> sei();

Wobei das zu weit gefasst ist da die USART... Funktionen und der 
getTimestamp ziemlich lange dauern könnten. Ausserdem könnte es sein daß 
darin auch cli() oder sei() aufgerufen wird.

besser ist hier vor dem if

cli();
int tempCounter = one_hundred_ms_ticks_counter;
sei()

und dann mit dem tempCounter weiterarbeiten

am Anfang der Schleife (in der Schleife dann):

cli();
one_hundred_ms_ticks_counter = 0;
sei();

[Εrnst B. (ernst)]

Und wenn dein counter genauer laufen soll und gleichzeitig die 
IRQ-Sperre nicht die ganzen UART-ausgaben mit abdecken soll, ist es 
vielleicht auch sinnvoll, es umzustellen:

  cli();
  if(one_hundred_ms_ticks_counter >= 600) { // 1 Minute
    one_hundred_ms_ticks_counter = 0; // Sofort zurückstellen
    sei();
    USART_puts(getTimeStamp()); // UART ausgabe kann lange dauern...
    ...
  }
  sei();

[Erich (Gast)]

>Du darfst dir in Abfragen nicht per Interrupt reinpfuschen lassen.

Anstelle von cli/sei würde ich jedoch die Variable 
one_hundred_ms_ticks_counter am Beginn der while(1) Loop kopieren und 
dann diese Kopie abfragen.

[Peter II (Gast)]

Erich schrieb:
> Anstelle von cli/sei würde ich jedoch die Variable
> one_hundred_ms_ticks_counter am Beginn der while(1) Loop kopieren und
> dann diese Kopie abfragen.

und dann? dann wird halt unsinn kopiert.

[Peter S. (psavr)]

>Anstelle von cli/sei würde ich jedoch die Variable
>one_hundred_ms_ticks_counter am Beginn der while(1) Loop kopieren und
>dann diese Kopie abfragen.

Da brauchst Du aber trotzdem ein cli/sei beim kopieren.

[oldmax (Gast)]

Hi
Ich kann mich zwar in C nicht ausdrücken, aber warum nicht in der ISR 
einfach nur hochzählen, solange kleiner 600. Dann in einem "Else" -Zweig 
die Variable auf 0 und einen Merker setzen, der dann im ganz normalen 
Programm abgefragt wird. Ist er gesetzt, erfolgt die beabsichtigte 
Aktion  und der Merker wird wieder gelöscht. Es ist doch sicherlich 
egal, wenn jede Minute eine Aktion ausgeführt werden soll, ob diese in 
einem Zeitfenster von +- 2.3 mSekunden stattfindet. Auf diese Weise wird 
die ISR überhaupt nicht belastet. Und nach einer Stunde sind auch 60 
Aktionen durchgeführt, nach einer Woche 10080 ...
Gruß oldmax

[oldmax (Gast)]

Hi
Vergesst mein vorheriges Schreiben... sehe grad, das es im Prinzip so 
gemacht, wie vorgeschlagen, also die Bearbeitung außerhalb der ISR.
Grußß oldmax

[da1l6 (Gast)]

16-Bit Werte zwischen interrupt und Hauptprogramm zu teilen ist, wie 
hier schon hinreichend beschreiben, problematisch.
Anstatt jedoch interrupts global zu deaktivieren und so zu riskieren 
welche zu verpassen, habe ich mir angewöhnt Zähler nur innerhalb der 
Interrupts zu halten und (atomar änderbare) 8-Bit Variablen für die 
Ereignisse zu verwenden:

volatile uint8_t timer_event = 0;

ISR(TIMER1_COMPA_vect)    
{
  static uint16_t one_hundred_ms_ticks_counter = 0;
  one_hundred_ms_ticks_counter++;    
  if (one_hundred_ms_ticks_counter >= 600){
    one_hundred_ms_ticks_counter = 0;
    timer_event = 1;
  }
}
...
  if(timer_event)   //1 minute
    {
      timer_event = 0;
      USART_puts(getTimeStamp());
    }
...

da1l6

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Εrnst B✶ schrieb:

 cli();
   if(one_hundred_ms_ticks_counter >= 600) { // 1 Minute
     one_hundred_ms_ticks_counter = 0;       // Sofort zurückstellen
     sei();
     USART_puts(getTimeStamp()); // UART ausgabe kann lange dauern...
     ...
   }
   sei();

Und wenn schon eine Abfrage auf größer-gleich drin ist, dann würde ich 
noch das hier vorschlagen:

   if(one_hundred_ms_ticks_counter >= 600) { // 1 Minute
     one_hundred_ms_ticks_counter -= 600;    // um 1 Minute zurückstellen

Denn was wäre, wenn der Zähler schon auf 601 stehen würde?


Aber mir ist auch klar: keiner schreibt Routinen, die länger als 100ms 
dauern. Sicher... ;-)

[Krapao (Gast)]

// http://www.nongnu.org/avr-libc/user-manual/group__util__atomic.html
#include <util/atomic.h>
uint16_t tmp_one_hundred_ms_ticks_counter;

  // ...
  while(1){
    // ... tue etwas leichtes :-)
    ATOMIC_BLOCK(ATOMIC_FORCEON)
    {
      tmp_one_hundred_ms_ticks_counter = one_hundred_ms_ticks_counter;
    }
    if(tmp_one_hundred_ms_ticks_counter >= 600)   //1 minute
    {
      ATOMIC_BLOCK(ATOMIC_FORCEON)
      {
        one_hundred_ms_ticks_counter -= 600;
      }
      USART_puts(getTimeStamp());
      USART_puts(itoa(tmp_one_hundred_ms_ticks_counter, buff, 10));
      USART_puts("\r\n");
    }

[Gregor B. (Gast)]

Die Frage ist, wie sieht das Ganze im Assembler aus?

Folgendes Szenario:

16Bit Timer steht auf 0x01FF. Mit dem Clock-Cycle, in dem vom 8Bit 
Register gelesen wird, springt der Zähler um auf 200.
Dann liest du in der IF-Abfrage zunächst das Low-Byte (0xFF) danach das 
High-Byte (0x02), => Integer-Wert 0x02FF = 767.
767>600, also If-Abfrage True, nächste Abfrage vom Timer (in der 
If-Abfrage) liefert danach 0x0200 = 512.

[Karel M. (marsalek)]

Hallo Kollegen,

ihr seid alle goldig!!!

Vielen Dank für so viele konstruktiven Beiträge. Seit 8 Stunden zählt 
der Prozessor schon richitig jede 1 ganze Minute :-)

Gute Nacht
Karel

PS: Diesen Thread kann man schliessen.