Programm zur Frequenzmessung

[Peter (Gast)]

Hallo Forum

Ich bin noch Anänger in Mikrocontroller Programmierung. Ich möchte ein 
Programm schreiben das die Freuqenz eines Rechtecksignals Berechnet
Ich möchte dieses mit der Capture Mode des Timer1 (ATMEGA168) 
realisieren.

Ich hab das Programm unten mal angehängt Im Debuggermodus scheint das 
Programm das zu tun was es soll villeciht kann mir jemand einen Tip 
geben ob ich auf dem richtigen weg bin. Bzw bin ich über alle 
verbesserungsvorschläge dankbar


mfg

//*********** Includes***************************************************
#include <avr\io.h>
#include <avr\interrupt.h>
#include  <util\delay.h>



//************************************************************************


//*********** Systemkonstanten******************************************
#define F_CLOCK    2E7     // 20MHz CPU Takt
#define Zahn    6     // 6 Grad Zahn Teilung


//**********************************************************************


//*********** Globale Variablen******************************
double test,b_f;// b_f=1 wenn negative flanke
double ueberlauf=0;
double init=0; //initialisierung bei 1.neg Flanke
double   counter=0;
double  counterold=0; //Zählerstand N-1.fallende Flanke
double counternew=0; //Zählerstand N fallende Flanke
double diff=0;  //Differnez Zählerstand N und Zählerstand N-1
double cmax=65535; // max Zähler des 16 Bit counters
double help=0;//Hilfsgrösse
double flanke=0;//Hilfsgrosse ür dsp
double Frequenz=0;
long zahl=0;
unsigned int ICR1Htemp;
unsigned int ICR1Ltemp;
//**********************************************************************

//*********** Interrupt für fallende Flanke******************************
ISR(TIMER1_CAPT_vect)

{

//PORTB ^= 0xFF;

ICR1Htemp=ICR1H;
ICR1Ltemp=ICR1L;

counter=ICR1Htemp*0x100+ICR1Ltemp;
//counter=ICR1H*0x100+ICR1L;//16 Bit Verrechung H und L Reg Timer 1 capure Register


  
  
  
  
    if(b_f==0) //Erkennung 1. Flanke 

    {
    
    
    
    counterold=counter;//Zählerstand merken
    ueberlauf=0;    
    b_f=1;        
    }

    //if(b_f==1)
    else
    {
    counternew=counter;//Zählerstand merken
    //diff=counternew-counterold;
    
    
    
    help=(ueberlauf-1)*cmax;
    diff=cmax-counterold+(ueberlauf-1 *cmax+counternew;//Berechnungsformel 
    
    ueberlauf=0;
    counterold=counternew;

    



        
  
    }




}


//************************************************************************


//***********Interrupt für Zähler Überlauf********************************
ISR( TIMER1_OVF_vect)
{
ueberlauf++;
}
//***********************************************************************


 
int main(void)
{
 


//---------------------Timer 1 Konfiguration----------------------------

  TCCR1B = 0x01;  // capture neg. Flanke und prescaler 1



    TIMSK1 = 0x21;  // interrupt Enable ovfer flow Enable &Capture Enable
  sei();// interrups global zulassen



//---------------------------------------------------------------
 
   
 
  
while(1==1)
 

}

[Stefan (Gast)]

Was mir nicht gefällt, ist die Formatierung des Quellcodes. Aber das ist 
ein kleines Problem ;-)

//****************
//*** Includes ***
//****************
#include <avr\io.h>
#include <avr\interrupt.h>
#include <util\delay.h>

//************************
//*** Systemkonstanten ***
//************************
#define F_CLOCK 2E7  // 20MHz CPU Takt
#define Zahn    6    // 6 Grad Zahn Teilung

//*************************
//*** Globale Variablen ***
//*************************
double test,b_f;     // b_f=1 wenn negative flanke
double ueberlauf=0;
double init=0;       // initialisierung bei 1.neg Flanke
double counter=0;
double counterold=0; // Zählerstand N-1.fallende Flanke
double counternew=0; // Zählerstand N fallende Flanke
double diff=0;       // Differnez Zählerstand N und Zählerstand N-1
double cmax=65535;   // max Zähler des 16 Bit counters
double help=0;       // Hilfsgrösse
double flanke=0;     // Hilfsgrosse für dsp
double Frequenz=0;
long zahl=0;
unsigned int ICR1Htemp;
unsigned int ICR1Ltemp;

//***********************************
//***Interrupt für fallende Flanke***
//***********************************
ISR(TIMER1_CAPT_vect)
{
    // PORTB ^= 0xFF;

    ICR1Htemp = ICR1H;
    ICR1Ltemp = ICR1L;

    // 16 Bit Verrechung H und L Reg Timer 1 capture Register
    counter = ICR1Htemp * 0x100 + ICR1Ltemp;
    // counter = ICR1H * 0x100 + ICR1L;

    if(b_f == 0) //Erkennung 1. Flanke
    {
        counterold = counter; //Zählerstand merken
        ueberlauf = 0;
        b_f = 1;
    }
    else
    {
        counternew = counter; //Zählerstand merken
        // diff = counternew-counterold;

        help = (ueberlauf-1) * cmax;

        //Berechnungsformel
        diff = cmax - counterold + (ueberlauf-1) * cmax + counternew; 

        ueberlauf = 0;
        counterold = counternew;
    }
}

//**********************************
//***Interrupt für Zählerüberlauf***
//**********************************
ISR(TIMER1_OVF_vect)
{
    ueberlauf++;
}

//*******************
//***Hauptprogramm***
//*******************
int main(void)
{
    //------Timer 1 Konfiguration-------------------------
    TCCR1B = 0x01;  // capture neg. Flanke und prescaler 1
    TIMSK1 = 0x21;  // Interrupt Enable 
                    // & Overflow Enable 
                    // & Capture Enable

    sei();          // interrups global zulassen

    //----------------------------------------------------
    while(1==1);
}

Dann stört mich die Rechnerei mit den vielen double Variablen. Der Timer 
liefert von Natur aus einen Ganzzahlwert und den sollte man nicht in 
einen Gleitkommawert vergewaltigen. Bleibst du bei Ganzzahlen, 
vereinfacht sich die Rechnung auf Assemblerebene. Das spart Platz und 
bringt Speed.

Speed ist ggf. notwendig, wenn du sie Berechnung innerhalb der 
Interruptroutine machst. Der nächste Interrupt ist dir nämlich schon auf 
den Fersen und lauert nur darauf, dass er dir Variablen überschreiben 
kann, mit denen du noch fleissig am rechnen bist.

Abhilfe ist entweder schnell den Interrupt abzuarbeiten oder während der 
Interruptbearbeitung Interrupts zu sperren (weniger sinnvoll bei einem 
Timer/Zähler).

Die schnellere Bearbeitung könnte z.B. so aussehen, dass du zunächst nur 
Variablen füllst, die später im Hauptprogramm ausgewertet werden. Auch 
den Spezialfall der 1. Flanke kannst du bei der Auswertung behandeln und 
du musst nicht bei jedem Interrupt den alten Käse vom Anfang per if 
testen. Vor dem Einschalten der Interrupts den Zählerstand holen und 
pronto hast du den counterold-Wert. Und wie gesagt, keine oder sowenig 
wie möglich Gleitkommabearbeitung. Maximal diff als Gleitkommawert 
berechnen. Besser wäre es sich Gedanken über den Wertebereich zu machen 
und einen passenden Typ auswählen. Mit unsigned long kann man z.B. bis 
über 4 GHz ausgeben ;-)

Für unveränderliche Werte wie dieses cmax solltest du auch eine 
Konstante nehmen, also z.B. ein Makro definieren.

[Peter (Gast)]

Hallo Stefan

Danke erstmal für deine Antwort. Wenn ich im Debugger das Programm 
durchspiele funktioniert alles soweit. Wenn ich es aber auf dem u 
controller spiele und will mir eine Frequenz ausgeben lassen. Kommt nur 
müll raus. Kannst Du mir eventuell bei meinem Problem helfen??


mfg

[Karl H. (kbuchegg)]

Sicher.
Stell mal das komplette Programm rein.

Ansonsten:
* Stell sicher, dass die Ausgafunktion funktioniert.
* Lass dir erst mal die gezählten counter Ereignisse ausgeben
  (eine bekannte Frequenz anlegen und nachschauen ob das
   gewünschte Ergebnis rauskommt)
* Erst wenn counter den richtigen Wert hat, rechne um
  auf die Frequenz
* ISR Funktionen kurz halten.
  In der ISR wirklich nur die Flanken zählen (Dazu reicht
  auch ein int + ein int für die Overflows). Die eigentliche
  Berechnung und Ausgabe in der Hauptschleife machen.
* Für einen Frequenzzähler brauchst du auch noch einen
  zweiten Timer, der dir zb. 0.1 Sekunden Intervalle
  generiert.

* Verzichte auf double. Du brauchst sie in der ISR nicht.
* Achte auf deine Programmformatierung.
  Zumindest ich weigere mich so ein unübersichtliches Chaos
  wie da oben auch nur näher in Augenschein zu nehmen.

[Peter (Gast)]

Hallo Karl heinz

Anbei mal das ganze programm. Ich hab versucht es ein wenig bessert zu 
ormatieren. Auch das sperren der Interrupts hat nix gebracht. Villeicht 
hättest Du mal zeit eine Blick darauf zu werfen weis momentan echt nicht 
wo der fehler liegen könnt. Wenn ich es im Debugger durchspiele kann ich 
einen ehler Finden aber am uc kommt nur Müll raus




merci
Peter

ps: Ich möcht das Programm mal zum laufen bringen, da es mir ausreicht 
frequenzen mit 10khz zu messen kann ich es bzgl doubles auch später noch 
abspecken

[Karl H. (kbuchegg)]

Du hast nichts von den bereit svorgeschlagenen Dingen
umgesetzt.
Ersetz mal die double durch int!

In einer ISR ist Zeit sparen oberstes Gebot! Floating
Point Berechnungen, die man auch in int oder long
machen kann, gehören da auch dazu!

Ansonsten: Zuerst muss der counter Wert stimmen. Hast du
das überprüft? Mach das mal.

[Karl H. (kbuchegg)]

Und sag bitte nicht: Da kommt nur Müll raus.
Darunter kann sich nämlich keiner was vorstellen. Das
hilft niemandem weiter, gibt keinerlei Hinweise in
welche Richtung man suchen soll.

[Karl heinz B. (kbucheg)]

> ps: Ich möcht das Programm mal zum laufen bringen, da es mir
> ausreicht frequenzen mit 10khz zu messen kann ich es bzgl doubles
> auch später noch abspecken

Das nicht vollzogene 'Abspecken' ist einer der Gründe warum
dein Pgm nicht funktioniert. Du kommst mit dem Timing nicht
hin!
Wahrscheinlich. Ohne die Hardware hier zu haben
und Messungen vornehmen zu können ist das aus der Entfernung
schwierig zu sagen.
Ausserdem: Ein Programm das ordentlich formatiert ist und
vernünftig geschrieben ist hat meistens weniger Fehler.
Das sagt die Erfahrung. Ich hab schon ne Menge Programme
gesehen die grauslich formatiert waren, wo Code aus vorhergehenden
Versuchen entweder einfach drinnen gelassen wurde oder auskommentiert
wurde. Das Ergebnis: Ein Programm in dem man absolut nichts mehr
sehen konnte. AUch keine Fehler. Nach einer halben Stunde Arbeit
durch Code-vereinfacheung, Umformatierung und was sonst noch
so nötig ist um das ganze übersichtlich zu kriegen ist man
dann plötzlich in einem Zustand, wo selbst ein Blinder mit
Krückstock den oder die Fehler aus 2 Meter Entfernung ertasten
kann.

[unsichtbarer WM-Rahul (Gast)]

Ich hab nur mal einen Blick auf deine ISR geworfen:

ISR(TIMER1_CAPT_vect)

{
cli(); // völlig unnötig!
counter=ICR1H*0x100+ICR1L; // auch Quatsch: counter = ICR1; sollte reichen
flanke=1;    
sei(); // siehe cli();
}

Im Hauptprogramm:

oldcounter = 0;
// Timer konfigurieren und starten...
while(1)
{
.
.
.

  if (flanke)
  {
   flanke = 0; // Ereignis zurücksetzen.
   Periode = counter - oldcounter;
   oldcounter = counter;
   frequenz = 1/Periode; // das muss noch entsprechend optimiert werden...F_Clock...
  // Frequenz für Ausgabe formatieren...
  // LCD akutalisieren
  }

Es hilft übrigens ungemein, sich vor dem Programmieren Gedanken (auch 
auf Papier) zum eigentlichen Ablauf zu machen. Da kann man sogar schon 
optimieren...

[unsichtbarer WM-Rahul (Gast)]

Was mir gerade noch auffällt:

lcd_home();
          flanke=0;
          lcd_clear();
          }

Mächtig gewaltig sinnvoll, erst etwas auf dem Display auszugeben, um es 
dann sofort wieder zu löschen...

[unsichtbarer WM-Rahul (Gast)]

>F_CLOCK*cmax

ist doch eine Konstante, oder habe ich da was übersehen?
Warum wird cmax dann als double deklariert?

[Karl heinz B. (kbucheg)]

Zum Thema Formatierung:
Stefan hat weiter oben schon mal gezeigt, wie eine
vernünftige Formatierung aussieht. Nichts gegen
Whitespace (also Leerraum). Der kann gut sein! Aber
4 Leerzeilen untereinander bringen nichts. Die erzeugen
nur Unübersicht, weil Dinge unnötiger weise über ner Menge
Platz verstreut werden.

Schau dir mal das hier an:

int main(void)
{
 
unsigned long i=0;   //lokale Zähler Variable
unsigned long rest=0;
b_f=0;  //flanken erkennung init
ueberlauf=0;//
diff=0;
counterold=0; //Zählerstand N-1.fallende Flanke
counternew=0; //Zählerstand N fallende Flanke
Frequenz=0;
long anz;


//---------------------Timer 1 Konfiguration----------------------------

TCCR1B = 0x01;  // capture neg. Flanke und prescaler 1

TIMSK1 = 0x21;  // interrupt Enable ovfer flow Enable &Capture Enable
sei();// interrups global zulassen



//---------------------------------------------------------------
 
wait_ms(200);
lcd_init();
lcd_write(' ');
lcd_write(' ');
lcd_write(' ');
lcd_write(' ');
lcd_write(' ');
lcd_write(' ');
lcd_write(' ');
lcd_write(' ');
lcd_write(' ');
lcd_write(' ');
lcd_write(' ');
lcd_write('H');
lcd_write('z');
lcd_home();
lcd_write ('0');

 
 
 
  while( 1==1 ) 
  {
  

      if(flanke==1)

      {
      
        Frequenz=F_CLOCK*cmax/diff;
        rest=Frequenz;
        for(i=1000000;i>0;i/=10)
          {
  
            anz=rest/i;
            rest %=i;

            if(rest!=zahl)
            {


              lcd_write (anz +'0');
              
            
            
            }

            

            }
          lcd_home();
          flanke=0;
          lcd_clear();
          }







}
}

Da schüttelts mich.

Jetzt das ganze nochmal:

int main(void)
{
  unsigned long i = 0;   //lokale Zähler Variable
  unsigned long rest = 0;
  long anz;

  b_f        = 0;  // Flag für die allerste Flanke auf: noch nicht da gewesen
  ueberlauf  = 0;  // Zählt die Anzahl der Überläufe des Timers von einem Capture zum nächsten
  diff       = 0;  // Die Zaehldifferenz dees Timers von einer Flanke zur nächsten
  counterold = 0;  // Zählerstand N-1.fallende Flanke
  counternew = 0; //Zählerstand N fallende Flanke
  Frequenz   = 0;


//---------------------Timer 1 Konfiguration----------------------------

  TCCR1B = ( 1 << CS10);                       // capture neg. Flanke und prescaler 1
  TIMSK1 = ( 1 << TICIE1 ) | ( 1 << TOIE0 );   // interrupt Enable ovfer flow Enable &Capture Enable

  sei();// interrups global zulassen



//---------------------------------------------------------------
 
  wait_ms(200);
  lcd_init();
  lcd_write(' ');
  lcd_write(' ');
  lcd_write(' ');
  lcd_write(' ');
  lcd_write(' ');
  lcd_write(' ');
  lcd_write(' ');
  lcd_write(' ');
  lcd_write(' ');
  lcd_write(' ');
  lcd_write(' ');
  lcd_write('H');
  lcd_write('z');
  lcd_home();
  lcd_write ('0');
 
  while( 1==1 ) 
  {
    if( flanke == 1 )
    {
      Frequenz = F_CLOCK * cmax / diff;
      rest=Frequenz;

      for( i = 1000000; i > 0; i /= 10 )
      {
        anz  = rest / i;
        rest %= i;

        if( rest != zahl )
        {
          lcd_write( anz +'0' );
        }
      }

      lcd_home();
      flanke = 0;
      lcd_clear();
    }
  }
}

Hier in der Zeile:

  TIMSK1 = ( 1 << TICIE1 ) | ( 1 << TOIE0 );   // interrupt Enable ovfer flow Enable &Capture Enable

fällt dir was auf? Ursprünglich stand da
  TIMSK1 = 0x21;

Obiges ist komplett äquivalent dazu. Trotzdem sehe ich in meiner Version
etwas, was du in deiner Version nicht siehst: Du aktivierst den Capture 
für
Timer 1, aktivierst aber den Overflow für Timer 0 !

Ob das hier:

      for( i = 1000000; i > 0; i /= 10 )
      {
        anz  = rest / i;
        rest %= i;

        if( rest != zahl )
        {
          lcd_write( anz +'0' );
        }

eine Zahl korrekt ausgibt, wage ich mal zu bezweifeln. Teil meines 
Zweifels
liegt hier

        if( rest != zahl )

Da Zahl den Wert 0 hat und, soweit ich auf die Schnelle gesehen habe, 
auch
nie geändert wird, bedeutet das, dass eine 0 nicht ausgegeben wird. Ein
Wert von 500 wird also als 5 ausgegeben.

Wozu der ganze Aufwand? Es ist ok, wenn man sich mal Gedanken drüber 
macht, wie
man einen int in seine Stringrepräsentierung überführt. Aber für realen 
Code
verwendet man fertige Funktionen itoa() bzw. ltoa().
Dazu braucht man natürlich eine Funktion, die einen String ausgeben 
kann.
Die ist aber kein Problem:

void lcd_write_string( char* String )
{
  while( *String )
    lcd_write( *String++ );
}

Damit reduziert sich die auch sofort die Initialisierungssequenz in der 
main()

int main(void)
{
  unsigned long i = 0;   //lokale Zähler Variable
  unsigned long rest = 0;
  long anz;

  b_f        = 0;  // Flag für die allerste Flanke auf: noch nicht da gewesen
  ueberlauf  = 0;  // Zählt die Anzahl der Überläufe des Timers von einem Capture zum nächsten
  diff       = 0;  // Die Zaehldifferenz dees Timers von einer Flanke zur nächsten
  counterold = 0;  // Zählerstand N-1.fallende Flanke
  counternew = 0;  //Zählerstand N fallende Flanke
  Frequenz   = 0;


//---------------------Timer 1 Konfiguration----------------------------

  TCCR1B = ( 1 << CS10);                       // capture neg. Flanke und prescaler 1
  TIMSK1 = ( 1 << TICIE1 ) | ( 1 << TOIE1 );   // interrupt Enable ovfer flow Enable &Capture Enable

  sei();// interrups global zulassen



//---------------------------------------------------------------
 
  wait_ms(200);
  lcd_init();
  lcd_write_string( "           Hz" );

  lcd_home();
  lcd_write ('0');
 
  while( 1==1 ) 
  {
    if( flanke == 1 )
    {
      Frequenz = F_CLOCK * cmax / diff;
      rest=Frequenz;

      for( i = 1000000; i > 0; i /= 10 )
      {
        anz  = rest / i;
        rest %= i;

        if( rest != zahl )
        {
          lcd_write( anz +'0' );
        }
      }

      lcd_home();
      flanke = 0;
      lcd_clear();
    }
  }
}

Noch die Ausgabe durch ltoa() ersetzen, da ein long
dicke reicht um 10000 zu speichern und wir kriegen:

int main(void)
{
  unsigned long i = 0;   //lokale Zähler Variable
  unsigned long rest = 0;
  long anz;
  char Buffer[10];       // Ausgabepuffer für itoa, 10 Zeichen sollte für einen int
                         // locker reichen

  b_f        = 0;  // Flag für die allerste Flanke auf: noch nicht da gewesen
  ueberlauf  = 0;  // Zählt die Anzahl der Überläufe des Timers von einem Capture zum nächsten
  diff       = 0;  // Die Zaehldifferenz dees Timers von einer Flanke zur nächsten
  counterold = 0;  // Zählerstand N-1.fallende Flanke
  counternew = 0; //Zählerstand N fallende Flanke
  Frequenz   = 0;


//---------------------Timer 1 Konfiguration----------------------------

  TCCR1B = ( 1 << CS10);                       // capture neg. Flanke und prescaler 1
  TIMSK1 = ( 1 << TICIE1 ) | ( 1 << TOIE1 );   // interrupt Enable ovfer flow Enable &Capture Enable

  sei();// interrups global zulassen

//---------------------------------------------------------------
 
  wait_ms(200);
  lcd_init();
  lcd_write_string( "           Hz" );

  lcd_home();
  lcd_write ('0');
 
  while( 1==1 ) 
  {
    if( flanke == 1 )
    {
      Frequenz = F_CLOCK * cmax / diff;

      ltoa( Frequenz, Buffer, 10 );
      lcd_write_string( Buffer );

      lcd_home();
      flanke = 0;
      lcd_clear();
    }
  }
}

Das sieht doch schon mal besser aus als das Original.

Bist du dir sicher, dass du zuerst die Zahl am Display ausgeben
willst und danach das Display löschen möchtest? Ich denke nicht,
dass du das willst. Denn in der Zeit die zwischen dem
lcd_write_string und dem lcd_clear liegt, wirst du wahrscheinlich
am Display nichts ablesen können. Das Display ist aber leer
bis zur nächsten Ausgabe und besonders bei kleinen Frequenzen
(< 10Hz) kann das ganz schön dauern. Also, wenn schon löschen,
dann lösch das Display bevor du eine Ausgabe machst!

 while( 1==1 ) 
  {
    if( flanke == 1 )
    {
      Frequenz = F_CLOCK * cmax / diff;

      ltoa( Frequenz, Buffer, 10 );
      lcd_home();
      lcd_clear();
      lcd_write_string( Buffer );

      flanke = 0;
    }
  }

Für itoa() musst du am Anfang noch stdlib.h inkludieren:

  #include <stdlib.h>

Schaun wir uns mal den Interrupt Handler an:

ISR(TIMER1_CAPT_vect)
{
  cli(); //intterrupt spreeren

  counter = ICR1H*0x100+ICR1L;//16 Bit Verrechung H und L Reg Timer 1 capure Register

  if( b_f == 0 )  //Erkennung 1. Flanke 
  {
    counterold = counter;//Zählerstand merken
    ueberlauf=0;    
    b_f = 1;        
  }
  else
  {
    counternew=counter;//Zählerstand merken
    diff = cmax - counterold + (ueberlauf-1) *cmax + counternew;//Berechnungsformel 
    ueberlauf = 0;
    counterold = counternew;
    flanke = 1;
  }

  sei();// interrups global zulassen
}

Das cli() bzw. sei() sind unnötig. Das macht der µC ganz von alleine.
Auch das Auslesen des Capture Registers muss man nicht so kompliziert 
machen.
Das kann der Compiler auch alleine:

   counter = ICR1;

An dieser Stelle:
    diff = cmax - counterold + (ueberlauf-1) *cmax + 
counternew;//Berechnungsformel
musste ich mal scharf überlegen, ob das auch stimmen kann, bzw. was
hier überhaupt passieren soll. Der Kommentar ist dabei nicht
hilfreich. Das das eine Formel ist, die ausgewertet wird, seh ich
auch alleine. Da brauch ich keinen Kommentar dazu. Was mir aber
ein Kommentar erzählen sollte, aber nicht tut, ist: Was passiert
denn hier überhaupt.
Hier wird die Differenz zum vorhergehenden Zählerstand ausgerechnet.
Warum du allerdings von der Anzahl der Überläufe 1 abziehst und dafür
noch mal cmax dazuzählst ist mir ehrlich gesagt ein Rätsel. Ich würde
es so schreiben:

   counternew = ueberlauf * cmax + counter;  // Auf welchem Wert steht
                                             // der Counter jetzt unter Berücksichtung
                                             // der inzwishcen aufgetretenen Überläufe

   diff = counterold - counternew;           // seit dem letzten Interrupt sind daher
                                             // wieviele Ticks vergangen ?

Ohne die LCD Funktionen, sieht dein Pgm also so aus, nachdem etwas
aufgeräumt wurde. Ich hab mal unnötige Variablen rausgeschmissen,
hab einige Kommentare so geändert, dass mir der Kommentar auch
etwas erzählt und nicht nur das wiedergibt, was sowieso schon im
Code steht:

//******************************************************** Includes*********************************************************
#include <stdlib.h>
#include <avr\io.h>
#include <avr\interrupt.h>
#include <util\delay.h>

//*********** Systemkonstanten************************************************************************************************
#define F_CLOCK    2E7     // 20MHz CPU Takt
#define Zahn    6     // 6 Grad Zahn Teilung
#define CMAX        65535L

//****************************************************************** Globale Variablen******************************
unsigned char b_f;               // b_f == 0 wenn erste negative flanke
long ueberlauf;                  // Anzahl Ueberlaeufe zwischen 2 Capture Interrupts
long counterold;                 // Zählerstand N-1.fallende Flanke
long counternew;                 // Zählerstand N fallende Flanke
volatile long diff;              // Differnez Zählerstand N und Zählerstand N-1
volatile unsigned char flanke;   // Zeigt an, dass eine Messung erfolgt ist

//*******************************************************************************************************************************
//*********** Interrupt für fallende Flanke***************************************************************************************
ISR(TIMER1_CAPT_vect)
{
  unsigned int counter = ICR1;

  if( b_f == 0 )  //Erkennung 1. Flanke 
  {
    counterold = counter;//Zählerstand merken
    ueberlauf=0;    
    b_f = 1;        
  }
  else
  {
    counternew = ueberlauf * CMAX + counter;  // Auf welchem Wert steht
                                              // der Counter jetzt unter Berücksichtung
                                              // der inzwischen aufgetretenen Überläufe

    diff = counterold - counternew;           // seit dem letzten Interrupt sind daher
                                              // wieviele Ticks vergangen ?

    /*
    ** alles für den naechsten Interrupt herrichten
    */
    ueberlauf = 0;
    counterold = counternew;
    flanke = 1;
  }
}

//***********Interrupt für Zähler Überlauf**************************************************************************
ISR( TIMER1_OVF_vect)
{
  ueberlauf++;
}

//*********************Display***********************************************************************************************

...

void lcd_write_string( char* String )
{
  while( *String )
    lcd_write( *String++ );
}
...

//*************************************************************************
int main()
{
  long Frequenz;
  char Buffer[10];       // Ausgabepuffer für itoa, 10 Zeichen sollte für einen int
                         // locker reichen

  b_f        = 0;  // Flag für die allerste Flanke auf: noch nicht da gewesen
  ueberlauf  = 0;  // Zählt die Anzahl der Überläufe des Timers von einem Capture zum nächsten
  diff       = 0;  // Die Zaehldifferenz des Timers von einer Flanke zur nächsten
  counterold = 0;  // Zählerstand N-1.fallende Flanke
  counternew = 0;  //Zählerstand N fallende Flanke


//---------------------Timer 1 Konfiguration----------------------------

  TCCR1B = ( 1 << CS10);                       // capture neg. Flanke und prescaler 1
  TIMSK1 = ( 1 << TICIE1 ) | ( 1 << TOIE1 );   // interrupt Enable ovfer flow Enable &Capture Enable

  sei();// interrups global zulassen

//---------------------------------------------------------------
 
  wait_ms(200);
  lcd_init();
  lcd_write_string( "           Hz" );

  lcd_home();
  lcd_write ('0');
 
  while( 1==1 ) 
  {
    if( flanke == 1 )
    {
      Frequenz = F_CLOCK * CMAX / diff;

      ltoa( Frequenz, Buffer, 10 );
      lcd_write_string( Buffer );

      lcd_home();
      flanke = 0;
      lcd_clear();
    }
  }
}

Und dann würde ich mal hergehen und würde am µC mal ausprobieren, ob
in diff vernünftige Werte rauskommen. D.h. zum Testen ersetzte ich
in der Ausgabe

      ltoa( Frequenz, Buffer, 10 );

durch

      ltoa( diff, Buffer, 10 );

und wenn das auch noch kein vernünftiges Bild ergibt, dann seh
ich mir die Werte von counternew und counterold an.


Das war jetzt noch lang nicht alles. Es gibt noch immer viel zu tun
in dem Programm. Aber es soll dir mal einen Einblick geben wie
man an solche Dinge rangeht. Der völlig falsche Weg ist es
Unmengen an Code zu schreiben, der nie oder nicht ausreichend
getestet wird und dann glauben, dass wird schon iregendwie klappen.
Das tut es nämlich nicht. Ich kenne keinen Programmierer (inklusive
mir) der in der Lage ist mehr als 100 Zeilen in einem Rutsch zu
schreiben ohne dass ein Fehler (Syntax oder Logik) drinnen ist.
Und ich würde sagen: Ich bin nicht der schlechtesten einer. Bei
einem Anfänger kannst du getrost aus den 100 Zeilen 5 Zeilen
machen.

[Michael U. (Gast)]

Hallo,

hab nur mal schnell rübergeschaut, weil ich immernoch hoffe, mehr von C 
zu verstehen (nur ASM sonst). ;)

Frage: wenn ich richtig gelesen habe, will er ca. 10kHz per 
Periodendauer messen? Wären dann rund 100µs/Periode.
Jede erkannte Flanke will er in main() dann umgerechnet anzeigen?
Mit einer Displayroutine, die bei jedem Zugriff mindestens 1ms wartet?

@unsichtbarer WM-Rahul:
Da spielt es dann auch keine Rolle, wenn er es gleich wieder löscht. ;)

Falls ich mich irre, bitte korrigieren.

Gruß aus Berlin
Michael

[Karl heinz B. (kbucheg)]

> Frage: wenn ich richtig gelesen habe, will er ca. 10kHz per
> Periodendauer messen? Wären dann rund 100µs/Periode.
> Jede erkannte Flanke will er in main() dann umgerechnet anzeigen?
> Mit einer Displayroutine, die bei jedem Zugriff mindestens 1ms wartet?

Das ist nicht so schlimm. Er muss ja nicht den Zählerstand
für jede gemessene Periode ausgeben. Vermisst er halt
zwischen 2 Ausgaben 100 Perioden.

[Karl heinz B. (kbucheg)]

Ich hab übrigens noch einen logischen Fehler übersehen.
Ist mir erst beim jetzigen durchlesen aufgefallen:

Das hier:

    counternew = ueberlauf * CMAX + counter;  // Auf welchem Wert steht
                                              // der Counter jetzt unter Berücksichtung
                                              // der inzwishcen aufgetretenen Überläufe

    diff =  counterold - counternew;          // seit dem letzten Interrupt sind daher
                                              // wieviele Ticks vergangen ?

kann klarerweise nicht stimmen. Die Differenz ist natürlich
jetzt minus vorher. Muss also heissen

    counternew = ueberlauf * CMAX + counter;  // Auf welchem Wert steht
                                              // der Counter jetzt unter Berücksichtung
                                              // der inzwishcen aufgetretenen Überläufe

    diff =  counternew - counterold;          // seit dem letzten Interrupt sind daher
                                              // wieviele Ticks vergangen ?

Siehst du. Mit einem vernünftigen Stil, kann man solche Dinge
tatsächlich sehen, ohne das Programm jemals laufen gelassen
zu haben.

[Stefan (Gast)]

char Buffer[10];       // Ausgabepuffer für itoa, 10 Zeichen sollte
                       // für einen int locker reichen

Das hatten wir schon mal ;-)

http://www.mkssoftware.com/docs/man3/itoa.3.asp
"The resulting string may be as long as seventeen bytes."

Um ohne Gefahren einen kleineren Puffer als als 17 Bytes zu verwenden, 
muss man die Implementierung für die verwendete Library im Sourcecode 
checken.

[Peter (Gast)]

Hallo Karl heinz

Danke erstmal für die konstruktive Kritik. Ich werd heut abend mal 
versuchen deine Vorschläge umzusetzen und dann nochmal zu testen.
Wenn man das letztde mal vor 5 jahren programiert hat ist halt aller 
anfang wieder schwer. Ich dachte wenn ich zu beginn überall double nehme 
dann kann sicher nix falsch laufen.
Ich hab mir eigentlich schon vor dem programmierung mittels Flowchart 
gedanken gemacht, nur wenn man keien erfahrung hat muss man es halt 
irgendwie lösen und kann nicht abschätzen was gut/schlecht ist. Den Tip 
mit abschalten der Innerrupts (cli, sei) hab ich von einem anderem 
Teilnehmer bekommen.
Danke nochmals für die Mühe. Wenn man erst mal siet wie es besser geht 
kann man auch draus lernen


mfg

[Karl heinz B. (kbucheg)]

Stefan wrote:
> char Buffer[10];       // Ausgabepuffer für itoa, 10 Zeichen sollte
>                        // für einen int locker reichen
>
> Das hatten wir schon mal ;-)
>
> http://www.mkssoftware.com/docs/man3/itoa.3.asp
> "The resulting string may be as long as seventeen bytes."
>
> Um ohne Gefahren einen kleineren Puffer als als 17 Bytes zu verwenden,
> muss man die Implementierung für die verwendete Library im Sourcecode
> checken.

Wir reden hier von einem AVR. Der hat 16 Bit int.
Wie du mit 16 Bit eine Zahl mit 16 Stellen hinkriegen willst,
musst du mir erst mal zeigen, bei einer Basis von 10. Bei
einer anderen Basis ist das allerdings klar. Wenn ich einen
16 Bit in zur Basis 2 ausgeben lasse, krieg ich klarerweise
16 Stellen + abschliessendes '\0' also 17 Stellen.
(Daher die 17, das hat nichts mit der Implementierung zu tun)

Aber du hast Recht. Ich verwende nämlich gar kein itoa, sondern
ltoa. Und ein long geht auch auf einem AVR höher.
Also nicht kleckern, klotzen: Rauf mit der Zahl!

[Stefan (Gast)]

Karl Heinz Buchegger, ich habe da was falsch im Kopf gehabt, sorry.

Das Stringdrehen (strreverse) in ltoa/itoa passiert auf der Stelle am 
Anfang des Puffers und nicht wie ich fälschlich angenommen habe zwischen 
Anfang und Ende des Puffers. Da müsste es auch mit deinem kurzen Puffer 
gehen.

BTW vgl. die verschiedenen Implementationen insbesondere die 
Kernighan/Ritchie Version bei 
http://www.jb.man.ac.uk/~slowe/cpp/itoa.html

Und beim Nachschlagen, wie es in der avr-libc aussieht, bin ich auf die 
interessanten speedoptimierten *toa Funktionen mit den Namen *toa_fast 
gestossen. So hat denn alles sein gutes ;-)

[Stefan (Gast)]

Peter, ich, Stefan, hatte "Interrupts abzuschalten" als eine von zwei 
Möglichkeiten genannt und extra mit dem Hinweis versehen "(weniger 
sinnvoll bei einem Timer/Zähler)" sowie anschliessend ausführliche 
Optimierungstipps zu der anderen Möglichkeit (schnellere Abarbeitung) 
gegeben.

[Karl heinz B. (kbucheg)]

> Und beim Nachschlagen, wie es in der avr-libc aussieht, bin ich auf
> die interessanten speedoptimierten *toa Funktionen mit den Namen
> *toa_fast gestossen. So hat denn alles sein gutes ;-)

Ganz genau.
Die kenne ich nämlich auch nicht.
Werde ich mir aber gleich mal anschauen.

[kein_guter_nic_mehr_frei (Gast)]

ICH BIN KEIN LEICHENSCHÄNDER!

(hallo alle..)

aber ich habe 2 tage damit zugebracht, um zu sehen dass der code von 
Karl-Heinz mir falsche ergebnisse liefert. ich bin dumm. bei allem 
respekt betreffend der formatierung - sollte man doch sein ziel nicht 
aus den augen verlieren. ich bezweifel, dass unser lieber karl den von 
ihm veröffentlichen code jemals an einen atmel geschickt hat. ein 
einfacher taschenrechner hat mir bessere ergebnisse geliefert.

als 'newbie' werde ich in zukunft nichts mehr kopieren wenn ich nicht 
verstehe, was da passiert.

nichts für ungut,
schönes wochenende
bernd

[Karl H. (kbuchegg)]

kein_guter_nic_mehr_frei schrieb:
> ICH BIN KEIN LEICHENSCHÄNDER!
>
> (hallo alle..)
>
> aber ich habe 2 tage damit zugebracht, um zu sehen dass der code von
> Karl-Heinz mir falsche ergebnisse liefert.

Dann hast du sicher auch den Abschnitt hier gelesen:

> Wahrscheinlich. Ohne die Hardware hier zu haben
> und Messungen vornehmen zu können ist das aus der Entfernung
> schwierig zu sagen.

Was schliesst du daraus: Ich konnte dieses Programm nie testen. Warum? 
Weil ich seine Hardware nicht hier hatte.

> dass unser lieber karl den von
> ihm veröffentlichen code jemals an einen atmel geschickt hat.

Da hast du recht. Ich habe den Thread noch einmal durchgelesen. Ich 
denke ich habe genug Hinweise hinterlassen, wie man bei der Fehlersuche 
vorgeht.

> ein
> einfacher taschenrechner hat mir bessere ergebnisse geliefert.

Wow. Du hast einen Taschenrechner, der Frequenzen messen kann :-)

> als 'newbie' werde ich in zukunft nichts mehr kopieren wenn ich nicht
> verstehe, was da passiert.

Gratulation.
Du hast das Wichtigste bei der Übernahme von fremden Code erkannt. Code 
zu übernehmen ist OK. Aber nur dann wenn man nicht einfach stumpfsinnig 
abtippt. Man muss das Abgetippte auch verstehen! In den seltensten 
Fällen kann man ein Programm einfach so übernehmen ohne es anzupassen. 
Und die Gefahr von Fehlern besteht natürlich immer.

> nichts für ungut,

ALso ich habe kein Problem damit, wenn du sagst, dass die letzte Version 
nicht funktioniert. Es gibt im Forum wahrscheinlich 1000-e Stellen an 
denen ich mich geirrt habe. Es wäre allerdings schön gewesen, wenn du 
auch noch gesagt hättest was das Problem war und wie du es behoben hast.

(Ich schätze mal, dass zb. der nicht atomare Zugriff auf diff bei 
schnellen Frequenzen ein Problem darstellt)

[Gast (Gast)]

>als 'newbie' werde ich in zukunft nichts mehr kopieren wenn ich nicht
>verstehe, was da passiert.

Das gilt auch für 'oldbies'. Eigenes Hirn zu haben, ist immer noch 
aktuell.