Volatile sollte doch atomares Lesen garantieren?

[daniel (Gast)]

Hallo,

auf einem ATMEGA32 in der ISR des TIMER1:

volatile uint16_t pwm_cycle_counter;
ISR(TIMER1_COMPA_vect) 
{
    static uint8_t pwm_cnt;
...
    
    if (pwm_cnt == 0) 
  {
        pwm_cnt++;
  pwm_cycle_counter++;
    }
    else 
  {
...
        if (pwm_cnt == pwm_cnt_max) 
    {
            pwm_sync = 1;                       // Update jetzt möglich
            pwm_cnt  = 0;
        }
        else pwm_cnt++;
    }
}

und in der main

main()
{
...

  while(1)
  {
    // Zeit berechnen
    if( (pwm_cycle_counter - last_pwm) >= F_PWM )
    {
      uart_puts("Sekunde:");  
      itoa((pwm_cycle_counter - last_pwm),rs232buf,10);
      uart_puts(rs232buf);
      uart_putc('\n');
      second_counter++;
    }
...
        }
}

Nun passiert etwas, das ich mir nicht ganz erklären kann. Über UART wird 
ein Wert ausgegeben, der kleiner F_PWM ist. Was aber eigentlich gar 
nicht sein darf, da die Abfrage eine Zeile darüber ja genau das abprüft. 
Zusätzlich wird pwm_cycle_counter nur inkrementiert, die Differenz kann 
also gar nicht kleiner werden im Laufe der Zeit.

Weiß jemand was genau da schief läuft?
Eventuell kann etwas schief gehen wenn der Interrupt genau dann kommt 
wenn ich auf die Größe zugreife?

Gruß
Daniel

[Εrnst B. (ernst)]

daniel schrieb:
> Volatile sollte doch atomares Lesen garantieren?

Nein. Dafür brauchst du sei()/cli(), oder die praktischen Atomic-Wrapper 
aus der <util/atomic.h>.

[gerd (Gast)]

Entweder ich bin blind, oder du hast die Variable last_pwn schön aus 
deinen Code-Schnipseln ausgespart. Da die aber ja nicht ganz unbeteiligt 
ist, wäre es vielleicht auch ganz gut zu wissen, wie sie im Kontext zu 
dem Ganzen steht.

Weiterhin vergeht von deiner if-Abfrage zu deiner itoa-Wandlung noch 
einiges an Zeit - u.a. eine (wenn Fleury) interrupt-basierte 
UART-Routine. Du meinst nicht, dass zwischenzeitlich nochmal was 
passiert sein könnte?

Also, so wie der Code dargestellt ist, ist eine Analyse (wieder mal) 
schwierig. ;)

- gerd

[Stefan E. (sternst)]

> Volatile sollte doch atomares Lesen garantieren

Nein, wie kommst du darauf?
Die Notwendigkeiten zu "volatile" und Atomarität kommen zwar gerne als 
Pärchen daher, aber das eine hat mit dem anderen nichts zu tun.

Um die Atomarität musst du dich schon noch gesondert kümmern.

[Karl H. (kbuchegg)]

In deinem Code sind ein paar Probleme

* volatile und Atomarer Zugriff haben nichts miteinander zu tun
  Für atomaren Zugriff musst du kurzfristig die Interrupts ausschalten

* uint16_t pwm_cycle_counter;
  itoa

  Das passt nicht zusammen.
  itoa heißt itoa, weil das i da vorne für _i_nteger steht. Also eine
  vozeichenbehaftete Zahl. Das hast du aber nicht. Du hast einen
  unsigned integer. Und die Funktion dafür heißt utoa. u wie _u_nsigned

[Falk B. (falk)]

Siehe Interrupt

[daniel (Gast)]

Hallo,

danke für die Antworten. Mit dem Abschalten des Interrupts vor dem 
if-Statement und dem Anschalten direkt danach besteht keinerlei Problem 
mehr.

Mit einem 8 Bit Datum wäre das Lesen wahrscheinlich schneller vorbei, 
während pwm_cycle_counter ja ein 16 Bit Wert ist, länger dauert und 
damit der IRQ häufiger dazwischenfunken kann.

Was ich jedoch nicht verstehe - selbst wenn in der Zwischenzeit noch 
etwas mit pwm_cycle_counter passiert, dann nur die Inkrementoperation im 
IRQ. Dass das allerdings dazu führt, dass der später ausgegebene Wert 
kleiner ist als der zuvor im If-Statement verglichene erscheint mir 
komisch. :)

Gruß,
Daniel

[Karl H. (kbuchegg)]

daniel schrieb:

> IRQ. Dass das allerdings dazu führt, dass der später ausgegebene Wert
> kleiner ist als der zuvor im If-Statement verglichene erscheint mir
> komisch. :)

Nochmal:
Du benutzt das falsche Werkzeug!

itoa ist für signed integer.
Du hast unsigned Werte.

Und ja. Wenn ein unsigned Wert groß genug wird, dann stellt ihn eine 
Funktion die dasselbe Bitmuster als signed Wert interpretiert mit einem 
- dar.

[daniel (Gast)]

Hallo Karl,

den Typunterschied habe ich verstanden, danke für den Hinweis.

Aber relevant ist itoa/utoa ja nur für die Ausgabe über RS232. Den Code, 
den ich meine, hat damit nichts zu tun.

Hier nochmal die Stelle:

    // Zeit berechnen
    if( (pwm_cycle_counter - last_pwm) >= F_PWM )
    {...

Funktioniert so:

    // Zeit berechnen
    cli();
    if( (pwm_cycle_counter - last_pwm) >= F_PWM )
    {
       sei();
       uart_puts("Sekunde:");
       ...


Über die RS232 Ausgabe wird klar, dass der Ausdruck
(pwm_cycle_counter - last_pwm) >= F_PWM
nicht wahr ist. D.h. der Wert von pwm_cycle_counter muss kurze Zeit 
vorher größer gewesen sein um in die If-Abfrage zu kommen. Dann kommt 
der IRQ und schwups ist pwm_cycle_counter kleiner. Und das kann ich mir 
nicht erklären.

[(prx) A. K. (prx)]

daniel schrieb:

> vorher größer gewesen sein um in die If-Abfrage zu kommen. Dann kommt
> der IRQ und schwups ist pwm_cycle_counter kleiner. Und das kann ich mir
> nicht erklären.

Wenn er erst das obere Byte läd, dann der Interrupt reinrutscht, dann 
das untere Byte läd, dann ist der geladene Wert ab und zu kleiner als 
der gespeicherte:

anfangs:        00FF
oberes Byte:    00
nach Interrupt: 0100
unteres Byte;     00
geladen also;   0000

[holger (Gast)]

>Was ich jedoch nicht verstehe - selbst wenn in der Zwischenzeit noch
>etwas mit pwm_cycle_counter passiert, dann nur die Inkrementoperation im
>IRQ. Dass das allerdings dazu führt, dass der später ausgegebene Wert
>kleiner ist als der zuvor im If-Statement verglichene erscheint mir
>komisch. :)

Wenn pwm_cycle_counter überläuft ist daran gar nichts komisch.

[daniel (Gast)]

Hallo Holger,

Überlauf ist kein Problem mit unsigned Typen. Bsp: 0x01 - 0xFF = 0x02.

A.K. hat wohl die richtige Erklärung gefunden. Dankeschön!!

[Erdbeere (Gast)]

>    cli();
>    if( (pwm_cycle_counter - last_pwm) >= F_PWM )
>    {
>       sei();

Auch Programmieren kann tödlich sein: sei() findet nur statt, wenn if(1) 
ausgeführt wird.
Am besten nimmt man eine Hilfsvariable, die einmalig unter 
Interruptsperre gelesen wird. Unter Umständen ist auch eine kleine 
Funktion zum Lesen sinnvoll.