ATmega88 ADC flag ist 0 in Schleife und 1 in if-Anweisung

[MazzMan (Gast)]

Ich habe ein nerviges Problem, an dem ich schon eine weile lang Sitze. 
Ich habe mein ADC auf auto trigger beim Overflow von Timer1 gesetzt und 
den Overflow-Interrupt für Timer 1 aktiviert. Da mein ADC immer dann 
getriggert wird, wenn der Interrupt für Timer1 aufgerufen wird, möchte 
ich dort eine Warteschleife einfügen, die auf mein ADC wartet. Hier ist 
das Problem: Die Warteschleife wird zur Endlosschleife:

//PWM overflow interrupt at bottom
ISR(TIMER1_OVF_vect){
  //wait for the adc to finish
  while ((ADCSRA & (1 << ADIF)) == 0){}
}

Das bedeutet, dass ADIF anscheinend immer 0 ist. Nun habe ich den Code 
geändert und einen Ausgabe auf PINB3 gemacht, sodass ich dort eine 1 
erhalte, wenn ADIF 1 ist. Sonst erhalte ich eine 0.

//PWM overflow interrupt at bottom
ISR(TIMER1_OVF_vect){
  //wait for the adc to finish
  //while ((ADCSRA & (1 << ADIF)) == 0){}
  
  if((ADCSRA & (1 << ADIF)) == (1 << ADIF)){
    PORTB |= (1<<PINB3);
  }else{
    PORTB &= !(1<<PINB3);
  }
}

Komischerweise erhalte ich eine 1, was bedeutet, dass ADIF 1 ist. Ich 
habe es auch schon mit for-Schleifen versucht, nun aber keine Ahnung 
mehr was ich noch alles machen soll. Wäre nett, wenn mir einer Helfen 
könnte. Hier der komplette Code (nach dem ihr immer fragt):

#include <asf.h>
#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdbool.h>

void InitADC(void);
void InitPorts(void);
void InitPWM(void);
void InitTimer(void);
void Commutate(void); 
void StartADCConversation(void);


char phase;
uint16_t time;
uint16_t delay;
bool blind_start;

char steps[] = {
  0b10010000,
  0b10000100,
  0b00100100,
  0b01100000,
  0b01001000,
  0b00011000  
};

ISR(TIMER0_OVF_vect)//every 256µs
{
  time++;
  if(blind_start){
    //blind start
    if(time >= delay){
      time = 0;
      
      Commutate();
      if(delay > 10)
        delay -= 1;
      //else
      //  blind_start = false;
    }
  }else{
  
  }
  return;
}

char test = 1;

//PWM overflow interrupt at bottom
ISR(TIMER1_OVF_vect){
  //wait for the adc to finish
  //while ((ADCSRA & (1 << ADIF)) == 0){}
  
  if((ADCSRA & (1 << ADIF)) == (1 << ADIF)){
    PORTB |= (1<<PINB3);
    test = 0;
  }else{
    PORTB &= !(1<<PINB3);
    test = 1;
  }
  //ADCSRA |= (1<<ADIF);
}

int main(void)
{
  SP = RAMEND;
  
   InitPorts();
   InitTimer();
   InitPWM();
   InitADC();  
  
  phase = -1;
  time = 0;
  delay = 100;
  
  OCR1B = 200;
  
  blind_start = true;
  
  //ADCSRA |= (1<<ADSC);    
  
    while(1)
    {
  }
}

void InitPorts(){
  //PWM output
  DDRB = (1<<DDB3)|(1<<DDB2);
  //BLDC output
  DDRD = 0xff;
  //ADC input
  DDRC = (0<<DDC0) | (0<<DDC1) | (0<<DDC3);
}

void InitTimer(){ 
  //ATMEGA88
  TCCR0B = 0b00000001; //no prescaler
  TIMSK0 = (1<<TOIE0);
  sei(); 
}

void InitPWM(){
  //ATMEGA88
  TCCR1A = (1 << COM1B1)|(0 << COM1B0) | (0 << WGM11)|(1 << WGM10); //clear on match when up, set on match when down; phase correct 8-bit
  TCCR1B = (0 << WGM13)|(0 << WGM12) | (0 << CS12)|(1<<CS11)|(0<<CS10); //prescaler: 8
  TIMSK1 = (1<<TOIE1);//enable timer1 overflow interrupt
  
}

void InitADC(){
  ADMUX  = (0<<REFS1)|(1<<REFS0) | (1<<ADLAR) | (0<<MUX3)|(0<<MUX2)|(0<<MUX1)|(0<<MUX0);//reference: AREF; right adjust; channel 2 selected in the beginning
  ADCSRA = (1<<ADEN) | (0<<ADSC) | (1<<ADATE) | (1 << ADIF) | (0<<ADIE) | (0<<ADPS2)|(0<<ADPS1)|(1<<ADPS0);//enable adc and auto trigger; prescaler: 8; 
  ADCSRB = (1<<ADTS2)|(1<<ADTS1)|(0<<ADTS0);//trigger at timer1 overflow
}

void StartADCConversation(){
  ADCSRA |= (1<<ADSC);
}

void Commutate(){
  phase++;
  if(phase > 5)
    phase = 0;
  
  PORTD = steps[phase];
  
  //switch adc channel
  /*
  char channel = (ADMUX & 3)+1;
  if(channel < 2)
    channel = 0;
  
  ADMUX = (ADMUX & 0b11111000) | channel;*/
}

[spess53 (Gast)]

Hi

>  if((ADCSRA & (1 << ADIF)) == (1 << ADIF)){

Dir ist aber klar, das ohne ADC-Interrupt das ADIF-Flag manuell 
zurückgesetzt werden muss?

MfG Spess

[MazzMan (Gast)]

spess53 schrieb:
> Dir ist aber klar, das ohne ADC-Interrupt das ADIF-Flag manuell
> zurückgesetzt werden muss?

Wenn ich es nicht zurücksetze, dürfe ich aber nicht in einer 
Endlosschleife landen, oder?

[S. Landolt (Gast)]

> StartADCConversation
Klingt ja sehr unterhaltend, aber wo wird es aufgerufen?

[spess53 (Gast)]

Hi

>Wenn ich es nicht zurücksetze, dürfe ich aber nicht in einer
>Endlosschleife landen, oder?

Wenn es einmal gesetzt ist, schon.

MfG Spess

[Karl H. (kbuchegg)]

wenn du schon den ADC vom Timer aus triggern lässt, warum lässt du dir 
dann nicht ganz einfach auch vom ADC aus einen Interrupt triggern, wenn 
er fertig ist.
Der ADC kann das, du musst das nur aktivieren und eine entsprechende ISR 
dafür schreiben. Die wird dann aufgerufen, wenn der ADC ein neues 
Ergebnis fertig hat.

[Karl H. (kbuchegg)]

Tu dir selbst einen Gefallen und ...

void InitTimer(){ 
  //ATMEGA88
  TCCR0B = 0b00000001; //no prescaler
  TIMSK0 = (1<<TOIE0);
  sei(); 
}

verfrachte den sei() hier hin

int main(void)
{
....

  sei();
  
  while(1)
  {
  }
}

also unmittelbar vor die Hauptschleife. Denn das ist die einzige Stelle 
an der du mit Sicherheit sagen kannst, das alles korrekt und fertig 
eingerichtet ist und daher die Interrupts loslegen können.

Dieses Verpacken des sei() in irgendwelche Funktionen ist IMHO keine 
gute Idee. Alles vor dem sei() ist Setup bzw. Initialisierung von 
irgendwelchen Komponenten. D.h während diese Programmteile ablaufen, 
kannst du nicht 100% sicher davon ausgehen, dass zu diesem Zeitpunkt 
jeweils das Gesamtsytem schon lauffähig wäre.

[S. Landolt (Gast)]

Ob das Alles etwas hilft, solange ADSC nicht gesetzt wird?

[Karl H. (kbuchegg)]

int main(void)
{
  SP = RAMEND;
...

davon lässt du die Finger!
Diese Controller-Register gehen dich allesamt nichts an. Das erledigt 
das C-System schon alles für dich. Wenn du dich da einmischt, dann 
gewinnst du nichts und schiesst dir maximal selber ins Knie.

[Karl H. (kbuchegg)]

S. Landolt schrieb:
> Ob das Alles etwas hilft, solange ADSC nicht gesetzt wird?

Sollte nicht notwendig sein, wenn der Timer den ADC triggert.
Diese Einstellung hab ich aber noch nicht kontrolliert.

[MazzMan (Gast)]

S. Landolt schrieb:
>> StartADCConversation
> Klingt ja sehr unterhaltend, aber wo wird es aufgerufen?

Der ADC wird automatisch getriggert, somit muss ich es nicht noch manuel 
manchen. Die Funktion wird nirgendwo aufgerufen.

spess53 schrieb:
> Wenn es einmal gesetzt ist, schon.

Hmmm das versteh ich jetzt aber nicht. In der Schleife wird doch 
überprüft ob ADIF gleich null ist. Wenn es aber einmal auf eins ist und 
ich es nicht zurücksetze, dann bleibt es ja bei eins und die Bedingung 
ist nicht mehr erfüllt, weil ADIF ja ungleich null ist. Somit müsste er 
aus der schleife raus springen.
Ich habe ea aber mal versucht und am ende des interrupts

ADCSRA |= (1<<ADIF);

 eingefügt  um ADIF zurück zu setzen. Hat leider auch nicht geholfen.

[Karl H. (kbuchegg)]

So

  if((ADCSRA & (1 << ADIF)) == (1 << ADIF)){
    PORTB |= (1<<PINB3);
  }else{
    PORTB &= !(1<<PINB3);
  }

wird das nichts.

EIn Bit wird zurückgesetzt mittels

    PORTB &= ~(1<<PINB3);

binäre Negation und nicht logische Negation.
Deine LEd an PINB3 zeigt dir nicht die Wahrheit an.

[Thomas E. (thomase)]

Karl Heinz schrieb:
> wenn du schon den ADC vom Timer aus triggern lässt, warum lässt du dir
> dann nicht ganz einfach auch vom ADC aus einen Interrupt triggern, wenn
> er fertig ist.
> Der ADC kann das, du musst das nur aktivieren und eine entsprechende ISR
> dafür schreiben. Die wird dann aufgerufen, wenn der ADC ein neues
> Ergebnis fertig hat.

Das ist gar nicht nötig. Wenn der ADC vom Timer getriggert wird und die 
Timings vernünftig abgestimmt sind, also die Wandlungszeit kürzer als 
die Intervalzeit des Timers ist, alles andere macht auch keinen Sinn, 
braucht man die ADC-Flags gar nicht abfragen, sondern lediglich in der 
Timer-ISR das ADC-Register auslesen. Denn der ADC ist zu diesem 
Zeitpunkt definitiv fertig.
Der ADC-Wert im ersten Aufruf der Timer-ISR ist logischerweise 
unbrauchbar. Aber das sollte einen grossen Geist nicht stören.

mfg.

[S. Landolt (Gast)]

Bitte um Nachsicht für einen Assemblerprogrammierer; die erste ISR ist 
mit return abgeschlossen, die zweite nicht - macht der Compiler das 
Richtige daraus?

[Karl H. (kbuchegg)]

S. Landolt schrieb:
> Bitte um Nachsicht für einen Assemblerprogrammierer; die erste ISR ist
> mit return abgeschlossen, die zweite nicht - macht der Compiler das
> Richtige daraus?

Ja.

Du denkst zuviel Assembler.
Was soll denn der Compiler am Ende einer Funktion machen, wenn da kein 
return steht? Die Funktion ist zu Ende, wie solls weiter gehen?
Natürlich macht sich der COmpiler da einen return rein, was soll er denn 
auch sonst tun?

Ein explizites return am Funktionsende macht nur dann Sinn, wenn es 
einen Returnwert gibt. Das wiederrum kann aber bei einer ISR gar nicht 
sein.

[Christian K. (the_kirsch)]

Thomas Eckmann schrieb:
> Das ist gar nicht nötig. Wenn der ADC vom Timer getriggert wird und die
> Timings vernünftig abgestimmt sind, also die Wandlungszeit kürzer als
> die Intervalzeit des Timers ist, alles andere macht auch keinen Sinn,
> braucht man die ADC-Flags gar nicht abfragen, sondern lediglich in der
> Timer-ISR das ADC-Register auslesen. Denn der ADC ist zu diesem
> Zeitpunkt definitiv fertig.
> Der ADC-Wert im ersten Aufruf der Timer-ISR ist logischerweise
> unbrauchbar. Aber das sollte einen grossen Geist nicht stören.

Man kann den ADC auch als Trigger Quelle direkt mit dem Timer 
verkupfern.
Dann brauch man keine Timer-ISR aus zu programmieren. Der ADC startet 
dann automatisch bei einem Timer Overflow bzw. Compare Match.

ADC-Wert auslesen würd ich dann über die ADC-ISR.


PS.
wie ich gerade sehe hattest du das in deinem ersten Post auch so 
vorgehabt:
ADCSRB = (1<<ADTS2)|(1<<ADTS1)|(0<<ADTS0);//trigger at timer1 overflow

[Matthias S. (Firma: matzetronics) (mschoeldgen)]

Karl Heinz schrieb:
> Ein explizites return am Funktionsende macht nur dann Sinn, wenn es
> einen Returnwert gibt. Das wiederrum kann aber bei einer ISR gar nicht
> sein.

Man kann allerdings ein return benutzen, um mittendrin aus der ISR 
auszusteigen. So, wie das da steht, kann man es aber ersatzlos 
weglassen.

Auch mir gefällt die Strategie, in der ISR auf den ADC zu warten, 
überhaupt nicht. Was soll denn der Motor machen, wenn kommutiert werden 
soll, der ADC aber noch am Werkeln ist? Unrunder Lauf wäre da noch das 
wenigste. ISR sind nun mal 'Blocking' und das möchte man so wenig wie 
möglich.

[S. Landolt (Gast)]

> Du denkst zuviel Assembler.
Jaja, mein Problem, ich 'denke nur Assembler'. Und da gibt es eben ret 
und reti - weiß der C-Compiler, in welcher Umgebung er sich befindet und 
macht aus dem return automatisch ein reti?

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

S. Landolt schrieb:
> weiß der C-Compiler, in welcher Umgebung er sich befindet und macht aus
> dem return automatisch ein reti?
Ja, du selbst gibst ihm mit dem Schlüsselwort ISR() den nötigen Tipp...