Berechnungsterm vereinfachen/optimieren

[Maddin (Gast)]

Hallo,

ich führe eine Frequenzmessung mit Input Capture durch. Ich brauche von 
der Periodendauer exakt die halbe Zeit, um daraus ein Compare Match zu 
bilden, sodass dann eine Aktion ausgeführt werden kann. Es handelt sich 
um einen 16-bit Timer (Atmega8 Timer1)
Nun sieht mein Code folgendermaßen aus:

      Gesamtzeit = (Overflows*65536) + EndTime - StartTime;  // Zeit zwischen 2 Flanken als Timertakt
      //Berechnung der Compare Match Zeit:
      Gesamtzeit = Gesamtzeit/2;
      NrOverflowsCompareMatch = Gesamtzeit/65536;  // berechnet Anzahl der Overflows der Compare Match Zeit
        Gesamtzeit %= 65536;      // berechnet den Rest
        OCR1A = TCNT1+Gesamtzeit;    // Compare Match Register wird mit aktuellem Zähler+Rest geladen

// OVF INT

ISR (TIMER1_OVF_vect){
  if (NrOverflowsCompareMatch > 0){           // wenn TURE und damit ungleich 0
    NrOverflowsCompareMatch--;          // NrOverflowsCompare Match dekrementieren bis = 0
  }

}

// COMPARE MATCH INT
ISR (TIMER1_COMPA_vect){
  if (NROverflowsCompareMatch == 0){          // wenn = 0&&Compare Match Wert erreicht Aktion ausführen
  // do something
  }
}

Ich könnte mir vorstellen, dass man den Berechnungsteil gerade mit der 
Restbildung evtl. optimieren könnte, sodass das Programm schneller wird.
Könnte ich nicht einfach das weglassen:

Gesamtzeit %= 65536

und einfach schreiben

OCR1A = TCNT1+Gesamtzeit;

da sowieso OCR1A nur 16 bit ist?
Geht das? Und gibt es noch weitere Optimierungsmöglichkeiten?

[Flo (Gast)]

eine (hoffentlich mehr als 16bit-) Zahl mit %65536 zu behandeln 
entspricht der Maskierung mit 0x00FF (die unteren 16 bit sind 
wichtig/gewollt).

Allerdings musst du bei deinem Code sehr genau auf die Datentypen 
schaun, da bei so großen Zahlen 16 bit nicht mehr ausreicht.

[Maddin (Gast)]

Hallo,

ja, der Datentyp von Gesamtzeit ist unsigned long (also 32-bit).
Hat sonst noch jemand eine Idee zur Optimierung?

[tuppes (Gast)]

Du könntest noch ausnutzen, dass 65536 dasselbe ist wie 2^16.

    NrOverflowsCompareMatch = Gesamtzeit/65536;  // dies ersetzt hoffentlich der Compiler von sich aus durch >> 16

//    Gesamtzeit %= 65536;      // das hier ersetzen durch:
    Gesamtzeit &= 0x0000FFFF;   // Rest bei Division durch 2^16

[Karl H. (kbuchegg)]

tuppes schrieb:
> Du könntest noch ausnutzen, dass 65536 dasselbe ist wie 2^16.

Wenn ein Compiler das nicht selbst rausfindet, ist es Zeit ihn auf den 
Müll zu schmeissen.

[Jörg W. (dl8dtl) (Moderator)]

Maddin schrieb:
> Könnte ich nicht einfach das weglassen:Gesamtzeit %= 65536
> und einfach schreibenOCR1A = TCNT1+Gesamtzeit;
> da sowieso OCR1A nur 16 bit ist?

Ja, kannst du.  Könnte der Compiler aber auch schon selbst
bemerkt haben.

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

 NrOverflowsCompareMatch = Gesamtzeit/65536;  // dies ersetzt hoffentlich der Compiler von sich aus durch >> 16

Aber hoffentlich nur, wenn er einen Hardware-Barrel-Shifter zur 
Verfügung hat  :-o
Sonst sollte er einfach die unteren 2 Bytes nicht verwenden...

[Falk B. (falk)]

@  Karl heinz Buchegger (kbuchegg) (Moderator)

>> Du könntest noch ausnutzen, dass 65536 dasselbe ist wie 2^16.

>Wenn ein Compiler das nicht selbst rausfindet, ist es Zeit ihn auf den
>Müll zu schmeissen.

Ich arbeite z.Z. in der 4ma mit MPLAB und nem PIC 4221. Dort erkennt der 
Compiler NICHT, dass der eine Division durch 64 als Shift machen kann!

Oder gibt es da noch irgendwo versteckte Optimierungsschalter?

MfG
Falk

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

> Ich arbeite z.Z. in der 4ma mit MPLAB und nem PIC 4221.
> Dort erkennt der Compiler NICHT, dass der eine Division durch 64
> als Shift machen kann!
Das bestärkt eigentlich nur die Aussage:
>>> Wenn ein Compiler das nicht selbst rausfindet, ist es Zeit ihn
>>> auf den Müll zu schmeissen.

> Dort erkennt der Compiler NICHT, dass der eine Division durch 64
> als Shift machen kann!
Evtl. ist in dieser Architektur eine Division schneller als 5 Shifts?
Bei den PICs kann man nie wissen...  ;-)

[Maddin (Gast)]

Hallo,

vielen Dank für die Hinweise.
Ich habe mit dem Code jedoch folgendes Problem:
Das errechnete Compare Match tritt zu früh auf.
Hier mal der Code, mittlerweile etwas abgewandelt, um Fehler zu suchen, 
vereinfacht. Ich messe damit nur Frequenzen, die so groß sind, dass 
16-bit ausreichen und somit keine Überlaufe berücksichtigt werden 
müssen:

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

#ifndef F_CPU
#define F_CPU           8000000UL                   // Prozessor Takt
#endif

#define LED      PB1    //Signal LED

#ifndef TRUE
#define TRUE 1
#define FALSE 0
#endif



volatile uint16_t StartTime = 0;      // Starzeit
volatile uint16_t EndTime = 0;        // Endzeit
volatile unsigned char ErsteFlanke = TRUE;  // Flag für Flanke
volatile uint16_t Gesamtzeit = 0;      // Endzeit-Startzeit
volatile uint16_t Zeitgrenze = 3125;    // ca. 40Hz

//Capture Interrupt

ISR( TIMER1_CAPT_vect ){
  if ( ErsteFlanke ){            //Ist es die erste Flanke oder Wurde schon eine Flanke eingelesen? 
    StartTime = ICR1;          //Liest den aktuellen Start Wert des 16-bit Capture Timers ein bei der 1. steigenden Flanke ein  
    ErsteFlanke = FALSE;             //Die naechste Flanke ist das Ende der Messung
  }
    
  else{                  //es wurde bereits eine 1. Flanke eingelesen, dies ist ist folgich die 2. steigende Flanke
    EndTime = ICR1;            //Liest den aktuellen EndWert des 16-bit Capture Timers ein bei der 2. steigenden Flanke
    Gesamtzeit = EndTime - StartTime;  //berechnet die Zeitdauer als Anzahl der Taktzyklen (inkl. Prescaler), die vergangen ist sind, zwischen 2 fallenden Flanken
    if (Gesamtzeit <= Zeitgrenze){
      OCR1A = TCNT1+(Gesamtzeit/2);    // Compare Match Register wird errechnet ohne overflow, da unsigned gerechnet wird, spielen Überläufe keine Rolle  
      TIMSK |= (1 << OCIE1A);
      PORTB &= ~(1 << LED);        // LED anschalten, zeigt, dass gedrosselt wird
      ErsteFlanke = TRUE;  
    }
  }
}
  
ISR (TIMER1_COMPA_vect){
  PORTB |= (1 << LED);                // LED ausschalten, die zeigt, dass nun nicht mehr gedrosselt wird
}

// Hauptprogramm
int main()
{

  DDRB = 0x00;       
  DDRB |= (1 << DDB1);         //PB1 Ausgang
  PORTB |= (1 << LED);        //LED anfangs aus
      
  TCCR1B = (1 << CS00) | (1 << CS01) | (1 << ICNC1);  // Prescaler 64, Noise Cancellor
  TCCR1B &= ~(1<<ICES1);                  // Interrupt Capture fallende Flanke
  TIMSK = (1<<TICIE1);                   // Aktiviert Input Capture Interrupt und Timer1 Overflow Interrupt 
  TCCR1A &= ~ ((1 << COM1A0) | (1 << COM1A1));      // Timer1 16-bit Compare Match A
  
  sei();                  //setzt globales Interrupt enable
  
  while(1){
  }
}

Wie man dem Code entnehmen kann, soll ab einer gewissen Grenzfrequenz 
eine LED eingeschaltet werden, dies ist auch IMMER der Fall (ich habe 
testweise eine LED eingeschaltet, ein delay eingebaut und danach 
ausgeschaltet). Demnach wird die If-Anweisung immer zuverlässig 
ausgeführt.
Jedoch wird die LED durch das Compare Match zu früh ausgeschaltet, sie 
ist nur sehr kurz angeschaltet (sieht man kaum), ein schätzungsweise 
paar µs.
Das seltsame daran ist, wenn ich das hereinkommende Signal abtrenne und 
wieder zuschalte, so stimmt das errechnete Compare Match manchmal (aber 
nicht immer). Hier bleibt die LED dann (vermutlich richtig) die halbe 
Zeit an, also deutlich länger und besser zu sehen.
Starte ich direkt den µC mit einer Frequenzmessung, so wird das Compare 
Match nicht richtig berechnet.
Hat jemand eine Ahnung woran das liegen könnte?
Wie gesagt, bis zu If-Anweisung dürfte alles laufen, da diese immer 
ausgeführt, nur die LED bleibt dann nicht lange genug an, wie Sie 
eigentlich bei Gesamtzeit/halbe anbleiben müsste.
Die Grenzfrequenz liegt bei ca. 40Hz (hier umgerechnet in Timer Wert).

Eckdaten: LED ist auf active-low, Prozessor Atmega8, Frequenz kommt von 
einem Optokoppler, der Masse an den Pin durchschaltet. Externer Pullup 
von 10k gegen +5V an ICP Pin.
Ich habe auch schon versucht, den Wert für das Register OCR1A vorher zu 
berechnet und dann mit einer Variable zu füllen, also so (jedoch 
erfolglos):

if (Gesamtzeit <= Zeitgrenze){
      Gesamtzeit = TCNT1+(Gesamtzeit/2);
      OCR1A = Gesamtzeit;    // Compare Match Register wird errechnet ohne overflow, da unsigned gerechnet wird, spielen Überläufe keine Rolle  
      TIMSK |= (1 << OCIE1A);
      PORTB &= ~(1 << LED);        // LED anschalten, zeigt, dass gedrosselt wird
      ErsteFlanke = TRUE;

Ist vielleicht beim beschreiben des OCR1A Registers etwas zu beachten? 
Im Datenblatt habe ich nichts gefunden. In der ISR muss es ja auch nicht 
atomic beschrieben werden, da sowieso andere Interrupts deaktiviert 
sind, solange die ISR ausgeführt wird...

[Falk B. (falk)]

@  Lothar Miller (lkmiller) Benutzerseite

>> Dort erkennt der Compiler NICHT, dass der eine Division durch 64
>> als Shift machen kann!
>Evtl. ist in dieser Architektur eine Division schneller als 5 Shifts?
>Bei den PICs kann man nie wissen...  ;-)

;-)
Neee, leider nicht. Die Divison von 3 Longs dauert ~1,5ms, mit Shift 
sind es 150µs.

MFG
Falk

[Karl H. (kbuchegg)]

Falk Brunner schrieb:

> Ich arbeite z.Z. in der 4ma mit MPLAB und nem PIC 4221. Dort erkennt der
> Compiler NICHT, dass der eine Division durch 64 als Shift machen kann!

Du siehst mich verblüfft.
Derartige 'Optimierungen' machen die Compiler seit mindestens 30 Jahren 
routinemässig und mit noch ganz anderen Sachen als nur mit 2-er 
Potenzen. Ist ja auch nicht weiter schwierig zu implementieren, sobald 
der Expression Tree intern erst einmal aufgebaut ist.

[Munchos (Gast)]

Maddin schrieb:
> if (Gesamtzeit <= Zeitgrenze){
>       OCR1A = TCNT1+(Gesamtzeit/2);
>       TIMSK |= (1 << OCIE1A);
>       PORTB &= ~(1 << LED);
>       ErsteFlanke = TRUE;

Ich habe den Fehler mittlerweile gefunden und auch eine Lösung dafür. 
Jedoch weiß ich nicht, wieso dieser auftritt.
Das Compare Match Flag wird hier scheinbar in diesen Codezeilen direkt 
gesetzt, obwohl OCR1A noch nicht erreicht ist. Daher wird die ISR des 
Compare Matches direkt nach der Input Capture ISR ausgeführt.
Abhilfe schafft das Löschen des Flags manuell über das Einfügen der 
Codezeile TIFR = (1 << OCF1A); hier am Ende. Dann wird das anstehende 
Flag gelöscht und anschließend nach dem eigentlich richtig berechneten 
OCR1A Compare Match auch das Programnm so ausgeführt wie es soll.
Das Problem ist zwar gelöst, aber irgendwie nicht sauber.
Hat jemand eine Idee, wieso das Flag direkt gesetzt wird? Das Compare 
Match Interrupt wird nämlich erst nach Berechnen von OCR1A aktiviert und 
in der auszuführenden ISR direkt wieder deaktiviert...

[Jörg W. (dl8dtl) (Moderator)]

Munchos schrieb:
> Hat jemand eine Idee, wieso das Flag direkt gesetzt wird?

Weil der compare match natürlich bei jedem Übereinstimmen des
Zählerwertes mit dem Register auftritt, nicht nur dann, wenn auch
der Interrupt freigeschaltet ist.

Du musst erstmal anhängige alte Interrupts löschen, bevor du sie
freischaltest.

TIFR = (1 << OCF1A);

(Ja, eine Zuweisung, keine VerODERung.)