ATmega32 AVR C Programmierphänomen Codefehler

[Dirk D. (sureai)]

Hi,
ich habe einen sehr einfachen Versuchsaufbau und einen sehr sehr simplen 
Code und bekomme da einen Fehler.
Ich habe eine Inkrementalscheibe mit Gabellichtschranke und diese an 
einem externen Interrupt angeschlossen. Zwischen zwei externen 
Interrupts zähle ich wie oft der Timer0 überläuft in einer volatile 
UNSINGED int oder long und bekomme hin und wieder wenn ich die 
Inkrementalscheibe drehe negative Werte für die Variable. Wie zur Hölle 
kann das sein?

(ATmega32 UART Code ausgeblendet)

#include <stdint.h>
#include <avr/io.h>
#include <stdio.h>
#include <avr/interrupt.h>

volatile unsigned long counter256 = 0;
ISR( TIMER0_OVF_vect ){       // Overflow Interrupt Vector Timer0
  counter256++;
}

volatile uint8_t flagUmdrehung3 = 0;
volatile unsigned int counterUmdrehung3 = 0;
ISR(INT1_vect){
  //Externer Interrupt 2
    counterUmdrehung3 ++; //Eine Umdrehung wurde registriert
    flagUmdrehung3 = 1;
}

int main(void){
    TIMSK |= (1<<TOIE0);
    TCCR0 = (1<<CS00); //kein Prescaler
    DDRD &= ~(1<<3); //PD3 als Eingang
    PORTD|= (1<<3); //Pullup aktivieren
    GICR|=(1<<INT1); //INT1 enable im General Interupt Control Register
    MCUCR|=(1<<ISC11)|(1<<ISC10); //Steigende Flanke löst aus INT1
    sei();
    while(1){
        if (flagUmdrehung3 == 1) {
            flagUmdrehung3 = 0;
            printf("%d\n",counter256);
            counter256 = 0;
        }
    }
}

Beispielausgabe im Terminal:
11208
2299
1071
736
696
15318
-12307
19177
16627
22843
8265
1836
1114
19570
12010
3456
23045
845
1259

Grüße Sureai

[Ingo L. (corrtexx)]

Dirk D. schrieb:
> printf("%d\n",counter256);
Gibt einen signed int aus, du hast aber einen unsigned long. Ändere 
deine printf-Formatierung zu %lu

Merkwürdig das der Compiler keine Warnung schmeißt...

Was machst du eigentlich mit der Variable "counterUmdrehung3"? Die 
scheint überflüssig!?

[Dirk D. (sureai)]

dankeschön!!!
Ich nehme mal an, unsingned int wäre dann %du.

[Peter II (Gast)]

Dirk D. schrieb:
> Ich nehme mal an, unsingned int wäre dann %du.

nein.

(C ist gut dokumentiert!)

[Dirk D. (sureai)]

Ingo L. schrieb:
> Was machst du eigentlich mit der Variable "counterUmdrehung3"? Die
> scheint überflüssig!?

Ja noch. Habe ein komplexeres Programm und Schritt für Schritt den Code 
vereinfacht um den Fehler zu finden.

[Ingo L. (corrtexx)]

Nicht ganz
http://www.claus-ebert.de/doc/cpp/printf/printf.html

[Dirk D. (sureai)]

danke für das Lesezeichen :)

[Stefan K. (stefan64)]

Erstmal gibst Du mit
  printf("%d\n",counter256);
eine signed in aus. Du hast aber eine unsigned int, also benutze %u.

Und dann ist es sinnvoll, auf Deine counter256 Variable vor Interrupts 
geschützt zuzugreifen. Sonst kann z.B. sowas passieren:

counter256  hat den Wert 1ffh
main löscht High-Byte von counter256 -> Wert jetzt 0ffh
ISR-Einsprung
ISR incrementiert counter256 -> Wert jetzt 0x100h
ISR-Ende
main löscht Low-Byte von counter256 -> Wert jetzt 100h

Dasselbe kann Dir auch bei der printf-Ausgabe passieren: High-und 
Low-Byte müssen an printf als Parameter übergeben werden, tritt 
dazwischen ein ISR mit Überlauf auf, ist das Ergebnis falsch.

Gruß, Stefan

[Dirk D. (sureai)]

Stefan K. schrieb:
> Erstmal gibst Du mit
>   printf("%d\n",counter256);
> eine signed in aus. Du hast aber eine unsigned int, also benutze %u.
>
> Und dann ist es sinnvoll, auf Deine counter256 Variable vor Interrupts
> geschützt zuzugreifen. Sonst kann z.B. sowas passieren:
>
> counter256  hat den Wert 1ffh
> main löscht High-Byte von counter256 -> Wert jetzt 0ffh
> ISR-Einsprung
> ISR incrementiert counter256 -> Wert jetzt 0x100h
> ISR-Ende
> main löscht Low-Byte von counter256 -> Wert jetzt 100h
>
> Dasselbe kann Dir auch bei der printf-Ausgabe passieren: High-und
> Low-Byte müssen an printf als Parameter übergeben werden, tritt
> dazwischen ein ISR mit Überlauf auf, ist das Ergebnis falsch.
>
> Gruß, Stefan

if (flagUmdrehung3 == 1) {
    cli();
    flagUmdrehung3 = 0;
    printf("%d\n",counter256);
    counter256 = 0;
    sei();
}

Meinst du so?

[S. R. (svenska)]

Auch das schützt nicht vor den negativen Ausgaben vom printf(), aber ja.

[Walter S. (avatar)]

Dirk D. schrieb:
> Meinst du so?

fast, üblicherweise holt man sich nur kurz die Variable und gibt dann 
den
Interupt wieder frei. Zeitaufwendigere Sachen wie printf kann man danach 
in Ruhe machen

[Stefan K. (stefan64)]

Ja, schon besser.
Noch genauer wird es so:

volatile unsigned long counter256 = 0;
volatile unsigned long counterAtExtIsr = 0;

ISR( TIMER0_OVF_vect ){       // Overflow Interrupt Vector Timer0
  counter256++;
}

volatile uint8_t flagUmdrehung3 = 0;
volatile unsigned int counterUmdrehung3 = 0;
ISR(INT1_vect){
  //Externer Interrupt 2
    counterUmdrehung3 ++;          //Eine Umdrehung wurde registriert
    flagUmdrehung3 = 1;
    // kopiere den aktuellen Timerwert nach CounterAtExtIsr und 
    // setze counter256 danach wieder zurück.
    // Weil wir in einer ISR sind, sind beide Operationen atomar:
    CounterAtExtIsr = counter256;
    counter256 = 0;                
}

int main(void){
...
    while(1){
        if (flagUmdrehung3 == 1) {
            // kopiere CounterAtExtIsr in lokale Variable und
            // sperre dabei ISRs, um ein atomares Verhalten zu erzwingen.
            // Durch die Verwendung von CounterAtExtIsr wird der 
            // interrupt-genaue Zählerwert ausgegeben, auch wenn main() 
            // ein Delay bis zu einer Umdrehung besitzt:
            cli();
            unsigned long myCounter = CounterAtExtIsr;
            sei();
            flagUmdrehung3 = 0;
            // gib den Zählerwert als unsigned long aus:
            printf("%lu\n",myCounter);
            counter256 = 0;
        }
...

Viele Grüße, Stefan

[Dirk D. (sureai)]

Stefan K. schrieb:
> Ja, schon besser.
> Noch genauer wird es so:

Vielen Dank! Die richtige Formatierung bei printf mit %lu oder %u hatte 
den Fehler schon behoben. Allerdings glaube ich, dass durch die korrekte 
Behandlung der Variablen in und um die ISR die du vorgeschlagen hast 
mein Hauptfehler beseitigt oder verbessert wurde, nach dem ich 
eigentlich auf der Suche war.

 // gib den Zählerwert als unsigned long aus:
           printf("%lu\n",myCounter);
           counter256 = 0;

ist counter256 nicht an der Stelle schon Null, da er ja in der 
ISR(INT1_vect) schon auf Null gesetzt wird?


Wenn ich meine Inkrementalscheibe mit einer konstanten Geschwindigkeit 
angetrieben habe, habe ich ohne deine Korrekturen Messfehler erhalten. 
Dann hatte ich in periodischen Abständen etwa solche Messergebnisse:

Terminal:
80
81
79
80
80
83
78
1
5
1
87
79

Also ein Abfallen der Werte, ohne dass sich die Antriebsgeschwindigkeit 
geändert hätte. Nach deinen Korrekturen ist das Abfallen nicht mehr so 
stark, aber immer noch vorhanden.

Terminal:
81
77
78
79
76
75
79
78
73
47
27
76
48
29
46
28
45
28
28
49
28
76
74
74
76
77

Dass die Werte nicht permanent auf einem konstanten Wert sind führe ich 
auf einen nicht perfekten Rundlauf der Welle mit der Inkrementalscheibe 
zurück. Aber das Abfallen der Werte kann ich mir immer noch nicht 
erklären.

[Walter S. (avatar)]

Dirk D. schrieb:
> ist counter256 nicht an der Stelle schon Null, da er ja in der
> ISR(INT1_vect) schon auf Null gesetzt wird?

das stimmt, aber er ist vermutlich nicht mehr null sondern wurde
zwischenzeitlich schon erhöht, das wird vermutlich auch deine zu kleine 
Werte verursachen

[Dirk D. (sureai)]

Walter S. schrieb:
> Dirk D. schrieb:
>> ist counter256 nicht an der Stelle schon Null, da er ja in der
>> ISR(INT1_vect) schon auf Null gesetzt wird?
>
> das stimmt, aber er ist vermutlich nicht mehr null sondern wurde
> zwischenzeitlich schon erhöht, das wird vermutlich auch deine zu kleine
> Werte verursachen

Ich bilde mir ein, dass es besser ist. Aber es tritt leider immer noch 
auf :/

[Dirk D. (sureai)]

Oder auch nicht, bei hohen Geschwindigkeiten tritt es wieder häufiger 
auf

[lalala (Gast)]

Kann auch die Anbindung der Scheibe an den Controller sein. Wie stellst 
Du sicher, das mix einstreut? Schaltplan?

[Stefan K. (stefan64)]

Sorry, das Löschen des counter256 hinter dem printf() ist natürlich ein 
Fehler. Das kann da ersatzlos verschwinden.  Das war wieder typisch 
copy/paste ...

Viele Grüße, Stefan

[Dirk D. (sureai)]

lalala schrieb:
> Kann auch die Anbindung der Scheibe an den Controller sein. Wie stellst
> Du sicher, das mix einstreut? Schaltplan?

Wie meinst du das? Ich verwende das AVR-NET-IO Board und am INT1 Pin vom 
ATmega32 liegt der Ausgang des Sensors an. Alle Kabel sind vernünftig 
gelötet und mit Schrumpfschläuchen abisoliert. Ein Wackeln an den Kabeln 
beeinflusst auch nicht das Messergebnis.

Ich kann dir auch einen Schaltplan erstellen, aber dafür hab ich kein 
Programm zur Hand.

[M. K. (sylaina)]

Dirk D. schrieb:
> Ich kann dir auch einen Schaltplan erstellen, aber dafür hab ich kein
> Programm zur Hand.

Eagle von Cadsoft kann da helfen. Die haben auch einen formschönen 
Schaltplaneditor.

[lalala (Gast)]

Dirk D. schrieb:
> lalala schrieb:
> Kann auch die Anbindung der Scheibe an den Controller sein. Wie stellst
> Du sicher, das mix einstreut? Schaltplan?
>
> Wie meinst du das? Ich verwende das AVR-NET-IO Board und am INT1 Pin vom
> ATmega32 liegt der Ausgang des Sensors an. Alle Kabel sind vernünftig
> gelötet und mit Schrumpfschläuchen abisoliert. Ein Wackeln an den Kabeln
> beeinflusst auch nicht das Messergebnis.
>
> Ich kann dir auch einen Schaltplan erstellen, aber dafür hab ich kein
> Programm zur Hand.

Wackel nicht, aber was passiert wenn Du mal Dein Handy daneben legst 
(und dann noch  mal anrufst?)

[Dirk D. (sureai)]

Es war die Unwucht der Welle und das leichte Eiern der 
Inkrementalscheibe. Hab das behoben und jetzt funktioniert es, wie es 
soll.

Danke an alle die mir geholfen haben!