Frequenzzähler PCINT2 TIMER1 Verzweifelung

[trm_1904 (Gast)]

Hallo AVR-Experten,
ich habe mich gestern und heute mit einen Problem rumgeärgert und immer 
noch keine Lösung dazu gefunden.
Ich habe mir einen Frequenzmesser geschrieben bei dem ich die Frequenz 
von über 100 Herz über den Port PD5/PCINT21 einlese.
Das Programm arbeit größtenteils korrekt, doch es gibt Situation indem 
nur ein Viertel der Frequenz ausgegeben wird.
Die ISR werden wie beim Oszi zu sehen aber immer abgearbeitet.
Die CPU läuft mit 20.000.000 Hz. Es ist ein Atmega328.


Ich habe echt viel probiert Timereinstellungen andere Berechnung aber 
bisher ohne Erfolg. Vllt hab ich auch etwas übersehen und finde es nicht 
mehr weil ich blind geworden bin.

Danke für eure Hilfe!

#include "main.h"
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>

#include "freqcounter.h"





int main(void){
  
  // Als Eingang konfigurieren
  DDRD|=((1<<DDD2));
  DDRD|=((1<<DDD3));
  // PD2+3 0 Setzen
  PORTD&=~((1<<PORTD2));
  PORTD&=~((1<<PORTD3));

  initFreqCounter();  // Frequenzzähler initialisieren

  while(1){
    // Frequenzen aktualisieren
    volatile uint16_t dummy=(uint16_t)getFreqValue(0);
    dummy++;

    }


}

 
#include "freqcounter.h"
#include "register.h"
#include <util/delay.h>

#define UDIFF(a,b) ((a)>(b) ? (a-b) : (b-a))
#define MAX(a,b) ((a)>(b) ? (a) : (b))


void initFreqCounter(void){
  // Durchflussmesser initilaisieren
  // Interruptpins aktivieren PD4..7
  // Als Eingang konfigurieren
  DDRD&=~((1<<DDD4)|(1<<DDD5)|(1<<DDD6)|(1<<DDD7));
  // Pullups deaktvieren
  PORTD&=~((1<<PORTD4)|(1<<PORTD5)|(1<<PORTD6)|(1<<PORTD7));
  // Pins PD4..7 bzw PCINT20..23 als Interrupteingang maskieren
  PCMSK2=((1<<PCINT20)|(1<<PCINT21)|(1<<PCINT22)|(1<<PCINT23));
  // Interrupt bei Port D aktivieren
  PCICR=(1<<PCIE2);
  //
  prev_reg=PIND&0xF0;

  
  // 16-Bit Timer initialisieren
  TCCR1B=0;
  TCCR1A=0;
  TCCR1C=0;
  GTCCR=0;
  TIMSK1=0;
  // Zeiteinheit festlegen dt=0,4 mikros CS12..10=010 Vorteiler=clk/8
  TCCR1B|=((1<<CS11));
  //TCCR1B&=~((1<<CS12)|(1<<CS10));
  //// Interrupts aktivieren
  TIMSK1|=(1<<TOIE1);
  timer1_high_val=0;

  sei();
}



uint16_t getFreqValue(uint8_t id){
  cli();
  // t0 erzeugen
  volatile uint32_t t0=freq0_t0_high_val;
  t0=t0<<16;
  t0|=freq0_t0_low_val;
  // t1 erzeugen
  volatile uint32_t t1=freq0_t1_high_val;
    t1=t1<<16;
    t1|=freq0_t1_low_val;
  volatile uint32_t udiff0;

  // Differenz zwischen t0 und t1 berechnen
  if(t0>t1)
    udiff0=t0-t1;
  else
    udiff0=t1-t0;

  // Frequenz berechnen
  udiff0=250000000UL/udiff0;

        // Frequenz kleiner 60,00 Hz
  if(udiff0<6000){
    // PD3 1 Setzen
    PORTD|=((1<<PORTD2));
  }
  else{
    // PD3 0 Setzen
    PORTD&=~((1<<PORTD2));
  }

  sei();
  return udiff0;

}


// Interruptroutinen
ISR(PCINT2_vect){
  // PD3 1 Setzen
  PORTD|=((1<<PORTD3));

  // Unteren Bits der Uhr einlesen

  unsigned char sreg;
  /* Save global interrupt flag */
  sreg = SREG;
  /* Disable interrupts */
  cli();
  volatile uint16_t timer1_low_val=TCNT1;
  SREG = sreg;

  // aktuelles Register einlesen
  volatile uint8_t act_reg=PIND&0xF0;
  // reg gibt an ob ein Sprung von 1 auf 0 erfolgt ist für jeden Pin
  volatile uint8_t reg=((prev_reg)&(~act_reg));


  // Frequenzeingang 0
  if(reg&(1<<(1+4))){
    freq0_t1_high_val=freq0_t0_high_val;
    freq0_t1_low_val=freq0_t0_low_val;
    freq0_t0_high_val=timer1_high_val;
    freq0_t0_low_val=timer1_low_val;


  }
  prev_reg=act_reg;
  // PD3 0 Setzen
  _delay_us(20);
  PORTD&=~((1<<PORTD3));




}

ISR(TIMER1_OVF_vect){
  timer1_high_val++;

}

#ifndef FREQCOUNTER_H_
#define FREQCOUNTER_H_

#include <avr/io.h>
#include <inttypes.h>
#include <avr/interrupt.h>
#include <float.h>

volatile uint16_t      freq0_t0_high_val;
volatile uint16_t      freq0_t0_low_val;
volatile uint16_t      freq0_t1_high_val;
volatile uint16_t      freq0_t1_low_val;
volatile uint16_t      freq0_deadtime;
volatile uint16_t      freq0_pulses;
volatile uint16_t      freq0_values[3];

volatile uint16_t      timer1_high_val;  // Überlaufregister für Timer 1
volatile uint8_t      prev_reg;      // Vorheriges Register von Port D Eingängen


void initFreqCounter(void);
uint16_t getFreqValue(uint8_t id);





#endif /* FREQCOUNTER_H_ */

[Uwe (de0508)]

Hallo,

ich denke Du solltest auch erklären nach welcher Methode dein 
Frequenzzähler funktionieren soll.
Welches Problem besteht genau ? Das wird nicht beschrieben !

So etwas ist unsinnig, da ja gerade jetzt keine globale 
Interruptfreigabe vorhanden ist.

/* Save global interrupt flag */
  sreg = SREG;
  /* Disable interrupts */
  cli();
  volatile uint16_t timer1_low_val=TCNT1;
  SREG = sreg;

Über den Einsatz von

volatile

 musst du nochmal nachdenken. Da sind bestimmt einige zu viel.

[trm_1904 (Gast)]

Danke für die Anmerkung ich habe alles versucht wie gesagt. Da ich mit 
meinen Latein am Ende war. Sicherlich ist es nicht nötig, aber ich habe 
es vorsichthalber mit reingetan.

Der Frequenzzähler arbeitet wie folgt:

Es gibt einen 32-Bit Timer der aus zwei 16 Bit Timern besteht:
timer1_low_val entspricht dem TCNT1 Register
timer1_high_val wird bei OVF des Timer1 hochgezählt

bei negativer Flanke wird der neue Timer-Wert gespeichert der davor als 
t0 und t1 gespeichert

bei Aufruf von getFreqValue(uint8_t id) werden t0 und t1 verglichen und 
dabei die Frequenz berechnet.

Das Problem ist, dass die Frequenzzähler Aussetzer hat, und machmal nur 
ein Viertel der tatsächlichen Frequenz ausgibt und das sind ziehmlich 
genau ein Viertel. Ich verstehe aber nicht warum...

Danke für deine Mühe

[Karl H. (kbuchegg)]

  if(t0>t1)
    udiff0=t0-t1;
  else
    udiff0=t1-t0;

das ist falsch.

Wenn du unsigned rechnest (was grundsätzlich richtig ist), dann 
errechnet sich die diff in allen Fällen zu

    udiff0=t0-t1;

das Ergebnis ist auch dann richtig, wenn es einen Overflow gibt.

Die Sache mit dem sei und cli ist schon angesprochen worden. Das hat in 
einer ISR nichts verloren. Um das Interrupt Flag brauchst du dich in 
einer ISR nicht kümmern. Während eine ISR läuft, sind Interrupts sowieso 
abgeschaltet und werden beim Beenden der ISR Funktion automatisch wieder 
aktiviert.

[Karl H. (kbuchegg)]

Karl Heinz schrieb:
>

>     udiff0=t0-t1;
>

>
> das Ergebnis ist auch dann richtig, wenn es einen Overflow gibt.

(Und tu dir einen Gefallen und nenn die Dinge nicht irgendwas0 und 
irgendwas1. Da muss man ständig aus dem Code entnehmen welches jetzt der 
'alte' Wert ist und welches der 'Neue'. Das ist nur eine sprachliche 
Falle, auch für dich selbst.

[Karl H. (kbuchegg)]

trm_1904 schrieb:

> Das Problem ist, dass die Frequenzzähler Aussetzer hat, und machmal nur
> ein Viertel der tatsächlichen Frequenz ausgibt und das sind ziehmlich
> genau ein Viertel. Ich verstehe aber nicht warum...

Beobachte mal, ob die Aussetzer einigermassen regelmässig kommen. Ist 
ein Muster erkennbar?

(lass dir auch mal an die Anzeige t1 ausgeben)

[Karl H. (kbuchegg)]

Karl Heinz schrieb:
> trm_1904 schrieb:
>
>> Das Problem ist, dass die Frequenzzähler Aussetzer hat, und machmal nur
>> ein Viertel der tatsächlichen Frequenz ausgibt und das sind ziehmlich
>> genau ein Viertel. Ich verstehe aber nicht warum...
>
> Beobachte mal, ob die Aussetzer einigermassen regelmässig kommen. Ist
> ein Muster erkennbar?
>
> (lass dir auch mal an die Anzeige t1 ausgeben)

Du kennst ja offenbar den Wert der sich ergeben müsste. Zumindest so 
ungefähr. Wenn sich also eine Frequenz errechnet, die kleiner als sagen 
wir mal die Hälfte dieses erwarteten Wertes ist, dann lässt du dir t1 
ausgeben. Sonst wirst du vermutlich mit Werten geflutet und kannst nicht 
sagen, welcher Zählerstand bei deinen falschen Werten vorgelegen hat.

[Karl H. (kbuchegg)]

Du hast hier natürlich

  volatile uint16_t timer1_low_val=TCNT1;

eine Race Condition.

Wenn der Timer kurz vor dem nächsten Puls knapp vor dem Überlauf stand 
dann kann dir die Situation passieren, dass
* die ISR aufgerufen wird
* während sich der µC bis zur Stelle vorarbeitet, an der du TCNT1 holst 
läuft der Timer natürlich weiter, erreicht seinen Overflow und beginnt 
wieder bei 0. Der springende Punkt ist nun, dass du hier ja in einer ISR 
bist. D.h. die Overflow-ISR muss noch warten, bis die hier abgearbeitet 
ist. Als Konsequenz davon hast du mit TCNT1 einen Wert von 0, 1, 2, ... 
(etwas sehr kleines), aber  timer1_high_val ist noch nicht erhöht, weil 
der Overflow Interrupt noch keine Chance hatte.

(genau deshalb wären diese Zähler Werte alle interessant. Und zwar dann, 
wenn es zu einer Fehlmessung kommt. Das Problem ist real, das hab ich 
jetzt nicht erfunden. Aber ich kann nicht sagen, ob es genau dieses 
Problem ist, das dir zu schaffen macht)

[Karl H. (kbuchegg)]

Musst du denn die Frequenz wirklich so genau messen?
Die Probleme werden nicht kleiner, wenn man die Genauigkeit bzw. 
Auflösung in die Höhe treibt. Schon alleine das rumwerfen mit uint32 ist 
für den AVR nicht so toll. Würde es nicht reichen, wenn du mit 16 Bit 
Werten hantieren könntest? Dann bräuchtest du die ganze Overflow 
Problematik gar nicht weiter behandeln.

[asdfasd (Gast)]

Ich denke, die zwei gröbsten Fehler sind die schon erwähnte falsche 
udiff-Berechnung und das falsche Auslesen des Timers in der ISR.  Du 
kannst nicht einfach das low-Word aus dem Register lesen und dann 
irgendwann später mal das high-Word aus der Variablen - 
zwischenzeitliche Overflows liefern dann falsche Werte.  Google mal nach 
AVR 32bit Timer.

Und schmeiss bitte die volatiles für lokalen Variablen raus - das tut 
weh ;-)

[asdfasd (Gast)]

Da war mal wieder einer schneller ;-)

[trm_1904 (Gast)]

Vielen Dank für eure Anmerkungen. Ich habe nun endlich eine 
funktionierende Version. Ich habe statt dem Timer1 Timer0 verwendet.
Statt die Differenz zweier uint32 Zahlen zu bilden, zähle ich jetzt mit 
f0_n_ref_actual hoch und setze ihn bei der negativen Flanke 0, dabei 
speicher ich auch f0_n_ref_actual in f0_n_ref_stored. Ich denke der 
Fehler lag beim auslesen von TCNT1. Aber da es jetzt so funktioniert bin 
ich glücklich und zufrieden. Am Oszi habe ich noch bebachten können, 
dass der Fehler dann auftrat wenn PCINT ISR knapp vorher aufgerufen 
worden war.

void initFreqCounter(void){
  // Durchflussmesser initilaisieren
  // Interruptpins aktivieren PD4..7
  // Als Eingang konfigurieren
  DDRD&=~((1<<DDD4)|(1<<DDD5)|(1<<DDD6)|(1<<DDD7));
  // Pullups deaktvieren
  PORTD&=~((1<<PORTD4)|(1<<PORTD5)|(1<<PORTD6)|(1<<PORTD7));
  // Pins PD4..7 bzw PCINT20..23 als Interrupteingang maskieren
  PCMSK2=((1<<PCINT20)|(1<<PCINT21)|(1<<PCINT22)|(1<<PCINT23));
  // Interrupt bei Port D aktivieren
  PCICR=(1<<PCIE2);
  // Initialregister speichern
  prev_reg=PIND&0xF0;

  
  // Timer0 initialisieren
  // Timer aktivieren mit Prescaler=8
  TCCR0A=0x00;
  TCCR0B=(1<<CS01);
  // Output Compare Interrupt erlauben
  TIMSK0=(1<<TOIE0);
  // ((20*10^6)/8)/10000 = 250
  OCR0A=250-1;

  // Timmer 0 setzen
  TCNT0=0x00;

  sei();
}



uint16_t getFreqValue(uint8_t id){

  return (uint16_t)(250000000UL/f0_n_ref_stored);

}


// Interruptroutinen

// Änderung an den Frequenzpins
ISR(PCINT2_vect){
  // aktuelle Zeit sichern
  uint8_t time_actual=TCNT0;

  // aktuelles Register einlesen
  uint8_t act_reg=PIND&0xF0;
  // reg gibt an ob ein Sprung von 1 auf 0 erfolgt ist für jeden Pin
  uint8_t reg=((prev_reg)&(~act_reg));

  // Frequenzeingang 0
  if(reg&(1<<(1+4))){
    f0_n_ref_stored=f0_n_ref_actual|time_actual;
    f0_n_ref_actual=0;
  }
  // Aktuelles Register wird zum letztem Register
  prev_reg=act_reg;


}
// Überlauf Timer0
ISR(TIMER0_OVF_vect){
  f0_n_ref_actual+=256;
}

#ifndef FREQCOUNTER_H_
#define FREQCOUNTER_H_

#include <avr/io.h>
#include <inttypes.h>
#include <avr/interrupt.h>
#include <float.h>


volatile uint32_t    f0_n_ref_actual;
volatile uint32_t    f0_n_ref_stored;
volatile uint8_t    prev_reg;      // Vorheriges Register von Port D Eingängen



void initFreqCounter(void);
uint16_t getFreqValue(uint8_t id);
uint32_t get_ticks( void );





#endif /* FREQCOUNTER_H_ */

[asdfasd (Gast)]

Ich denke, du hast immer noch Raceconditions, nur jetzt etwas kleinere 
Zeitfenster.  Und das resetten des Overflowcounters klappt so nicht: du 
müsstest auch TCNT0 resetten und evtl ausstehende Overflowinterrupts. 
Klappt dann natürlich nicht mehr bei mehreren Pins.  Ausserdem muss das 
Auslesen in getFreqValue atomar sein - bei 32-bit-Werten kann das der 
AVR nicht.  Also cli/sei drum herum.

Ich tipp hier mal ein, wie ich das angehen würde (completely untested!):

volatile static uint32_t cycles[1];
static uint32_t time_prev[1];
static uint32_t timer0_ovf;
static uint8_t pins_prev;

ISR(TIMER0_OVF_vect)
{
    timer0_ovf += 256;
}

ISR(PCINT2_vect)
{
    uint8_t cnt, ifr, pins, edges;
    uint32_t time;

    cnt = TCNT0;
    ifr = TIFR;
    pins = PIND;

    time = timer0_ovf + cnt;
    if (ifr & (1 << TOV0) && ~cnt & 0x80) // overflow before reading TCNT0
        time += 256;

    edges = pins_prev & ~pins; // falling edge
    pins_prev = pins;

    if (edges & (1 << PD5))
    {
        cycles[0] = time - time_prev[0];
        time_prev[0] = time;
    }
}

uint16_t getFreqValue(uint8_t id)
{
    uint32_t n;

    cli();
    n = cycles[id];
    sei();

    return 25000000ul / n;
}

Die 1er-Arrays um anzuzeigen, welche Variablen bei mehreren Pins 
anzupassen wären.  Btw, wenn der 16-Bit-Timer frei ist, würd ich eher 
den nehmen - generiert weniger Overflowinterrupts.  Noch besser 
natürlich Karl-Heinz' Hinweis: gar keine Overflows ;-)

[Die Overflow-Handling-Idee kommt übrigens aus: 
Beitrag "AVR Timer mit 32 Bit"]

[asdfasd (Gast)]

Noch vergessen: nimm doch den sei() in initFreqCounter() raus - der 
gehört in main().