Ich brauche Hilfe beim Optimieren eines Codeabschnittes

[Bernhard (Gast)]

Guten Tag

Ich verwende einen ATtiny24 um die Zeitdauer zwischen Signalflanken zu 
stoppen.
Dies mache ich über das PCINT Ereignis.
Ich habe bis jetzt nur sehr wenig mit Mikrocontrollern gearbeitet.
Desswegen weis ich nicht ob und wie man das Programm noch schneller 
machen kann.
Kann mir bitte jemand einen Tipp geben wie ich den Quellcode im Bereich 
der Interruptroutine optimieren kann?

// Pin change 0-7 interrupt service routine
ISR( PCINT0_vect )
{
  // Sichern des Zählerwertes, da der Zähler nicht angehalten werden 
  Timer1Aus;// Der Timer wird über eine Deffinition angehalten
  unsigned short  usTimerTemp = TCNT1; 
  Timer1An;// Der Timer wird über eine Deffinition gestartet

  // Sichern des Port's, da sich dieser während des Abarbeiten des Interupt's ändern kann
     unsigned char   uchPortIn = PINA ^ uchBitMaskeXorA; // uchBitMaskeXorA dient dazu, einzelne Signale zur Laufzeit zu invertieren

//////////////////// Signal 1 ////////////////////    
  // Ermitteln des aktuellen Signalwertes
  unsigned short uchWert  = uchPortIn & DEF_PIN_SIGNAL_1;  
  
  // Pegeländerung beim Signal?
  if(uchWert != (uchPortAalt & DEF_PIN_SIGNAL_1))
  {
    /*High/Low Pegel?*/

    if(uchWert > 0)/* High-Pegel*/
    {
      usStartSignal[1]= usTimerTemp; // Sichern der Startzeit;
    
    }
    else  /* Low-Pegel*/
    {  
      // Berechnen der Signalzeit
      // Ein Auftreten des Überlaufs des 16 Bit Zählers während der Zeitmessung
      // wird bewusst in Kauf genommen.
      usInputSignal[DEF_FELD_SIGNAL_1] = usTimerTemp - usStartSignal[1];

    }
    uchPortAalt = uchPortIn; // Aktueller Portwert = Portwert für nächste für die nächste Zeitermittlung
    return;
  }
//////////////////// Signal 3 ////////////////////    
  // Ermitteln des aktuellen Signalwertes
  uchWert  = uchPortIn & DEF_PIN_SIGNAL_3;  
  
  // Pegeländerung bei Signal3?
  if(uchWert != (uchPortAalt & DEF_PIN_SIGNAL_3))
  {
    /*High/Low Pegel?*/
    if(uchWert > 0)/* High-Pegel*/
    {
      usStartSignal[3]= usTimerTemp; // Sichern der Startzeit;
    }
    else  /* Low-Pegel*/
    {  
      // Berechnen der Signalzeit
      usInputSignal[DEF_FELD_SIGNAL_3 ] = usTimerTemp - usStartSignal[3];
    }
    uchPortAalt = uchPortIn; // Aktueller Portwert = Portwert für nächste für die nächste Zeitermittlung
    return;
  }
// Hier werden die weiteren Signale die am Port liegen ausgewertet
}

[Michael H. (overthere)]

Grundsätzlich gilt, bei Optimierungsfragen im das Listing durchschauen. 
Poste das doch mal.
Willst du auf Codegrö0e oder auf Dauer optimieren?

Was bei dir sein könnte (Bauchgefühl): Arrays ansteuern kann durch die 
Multiplikationen (wenn kein Hardware Multiplizierer da ist) viel Code 
brauchen.
Aber wie gesagt, poste das Listing und dann wird dir geholfen.

[Peter D. (peda)]

Damit kann keiner was anfangen.
Du mußt schon einen compilierbaren Code schicken (als Anhang!).

D.h. sämtliche verwendete Variablen, Macros, Unterfunktionen müssen 
definiert sein.

Unterfunktionen in Interrupts sind generell ne schlechte Idee. Entweder 
als Macro oder Inline-Funktion.


Peter

[Bernhard (Gast)]

Michael H. schrieb:
> Grundsätzlich gilt, bei Optimierungsfragen im das Listing durchschauen.
> Poste das doch mal.
> Willst du auf Codegrö0e oder auf Dauer optimieren?
Ich möchte auf Geschwindigkeit optimieren.
(Der Programmcode sollte aber nicht all zu groß werden.)
Die LSS Datei habe ich in die Zip-Datei gepackt.

Peter Dannegger schrieb:
> Unterfunktionen in Interrupts sind generell ne schlechte Idee. Entweder
> als Macro oder Inline-Funktion.
Wo bitte siehst du im Interrupt einen Aufruf einer Unterfunktion?
Wenn du auf Timer1Aus; und Timer1An; anspielst, dass sind Deffinitionen.
Damit verhindere ich, dass ich beim Ändern des Timer-Taktes eine Stelle 
übersehe.


Ich habe den Quellcode zusammengekürzt und die benötigten Funktionen und 
Deffinitionen in die Main-Datei kopiert.

Könnt ihr mir bitte Tipps geben, wieich die Abarbeitung in dieser 
Interruptroutiene beschleunigen kann?
Denn die Signalzeiten sollten möglichst nicht zu stark verfälscht 
werden.


Bernhard

[Bernhard (Gast)]

Entschuldigung.
Ich habe versehentlich die Datei 2 mal angehängt.

[(prx) A. K. (prx)]

> unsigned short uchWert  = uchPortIn & DEF_PIN_SIGNAL_1;

Wenn man schon die Notation mit Typ als Präfix verwendet, dann sollte 
man wenigstens nicht bescheissen. Code für uch ist kürzer als für us.

[Bernhard (Gast)]

Danke für dan Hinweis.
Den Fehler hatte ich glatt übersehen.

Aber beim Arbeiten mit 16Bit Arrays dürfte man sicherlich auch einiges 
rausholen können wenn man die richtigen Kniffe kennt.
Oder sehe ich das falsch?

mfg Bernhard

[(prx) A. K. (prx)]

Bernhard schrieb:

> Aber beim Arbeiten mit 16Bit Arrays dürfte man sicherlich auch einiges
> rausholen können wenn man die richtigen Kniffe kennt.

Da sämtliche Indizes konstant sind kannst du zwar den Sourcecode kürzen 
indem eine Schleife draus wird, die Laufzeit wird dadurch aber 
schlechter.

[(prx) A. K. (prx)]

Auf mehrere Pin-Changes des gleichen Ports so zu reagieren, dass weder 
Information verloren geht noch doppelt gezählt wird, ist nicht ganz 
einfach. Wenn zwischen dem Eintritt in den ISR und dem Auslesen des 
Portregisters eine weitere Flanke (ggf. an einem anderen Pin) reinkommt, 
dann ist das Interrupt-Flag wieder gesetzt, infolgedessen wird dein 
Handler dafür noch einmal aufgerufen und die gleiche Sache ggf. zweimal 
betrachtet.

Da du den Handler nach dem ersten erkannten Bit beendest, hängt das 
Vehalten dann auch noch von der exakten Reihenfolge der Flanken ab und 
es kann Information dabei sowohl doppelt gezählt werden, als auch 
verloren gehen.

[Bernd (Gast)]

A. K. schrieb:
> Auf mehrere Pin-Changes des gleichen Ports so zu reagieren, dass weder
> Information verloren geht noch doppelt gezählt wird, ist nicht ganz
> einfach. Wenn zwischen dem Eintritt in den ISR und dem Auslesen des
> Portregisters eine weitere Flanke (ggf. an einem anderen Pin) reinkommt,
> dann ist das Interrupt-Flag wieder gesetzt, infolgedessen wird dein
> Handler dafür noch einmal aufgerufen und die gleiche Sache ggf. zweimal
> betrachtet.
>
> Da du den Handler nach dem ersten erkannten Bit beendest, hängt das
> Vehalten dann auch noch von der exakten Reihenfolge der Flanken ab und
> es kann Information dabei sowohl doppelt gezählt werden, als auch
> verloren gehen.
Ich dachte, dass ich dass doppelt Zählen / Auslassen gelöst habe, indem 
ich den Portwert gleich beim Eintritt in den Interrupt sichere und 
diesen Wert für die Flankenerkennung verwende.
Habe ich da etwa einen Denkfehler?

A. K. schrieb:
> Da sämtliche Indizes konstant sind kannst du zwar den Sourcecode kürzen
> indem eine Schleife draus wird, die Laufzeit wird dadurch aber
> schlechter.
Aus diesem Grund habe ich zum ungeliebten Wurst-Code gegriffen.
Gibt es einen Weg die Abarbeitung zu beschleunigen?

[Bernd (Gast)]

Ich bin der Bernhard von oben.

[Karl H. (kbuchegg)]

Bernd schrieb:

> Ich dachte, dass ich dass doppelt Zählen / Auslassen gelöst habe, indem
> ich den Portwert gleich beim Eintritt in den Interrupt sichere und
> diesen Wert für die Flankenerkennung verwende.
> Habe ich da etwa einen Denkfehler?

Ja.
Du sicherst zwar den Wert, das ist schon richtig.
Aber wenn du in der Auswertung auf die erste Änderung triffst, 
bearbeitest du diese und hörst danach auf, nach weiteren Änderungen zu 
suchen.

Deine ISR barbeitet prinzipiell nur 1 Pin Änderung bei einem Aufruf. Nur 
garantiert dir niemand, dass nicht 2 Änderungen gleichzeitig auftreten 
können.

> Gibt es einen Weg die Abarbeitung zu beschleunigen?

Du musst zb hier

  if(uchWert != (uchPortAalt & DEF_PIN_SIGNAL_1))

jedesmal aus uchPortAalt den interessierenden Pin extrahieren. Wenn du 
für jeden Pin eine eigene 'alt' Variable hättest, könntest du dir das 
sparen.

[(prx) A. K. (prx)]

Bernd schrieb:

> Habe ich da etwa einen Denkfehler?

Ja. Das Interrupt-Flag wird mit Aufruf des Handlers gelöscht. Der 
Portzugriff erfolgt um die 20-30 Takte später. Wenn dazwischen was am 
Port passiert, dann wird es interessant.

[Karl H. (kbuchegg)]

Vielleicht sollte man auch einmal darüber sprechen, warum du denkst 
optimieren zu müssen.
Hast du ein Problem, von dem du denkst das du es mit Optimierung lösen 
könntest?

[Karl H. (kbuchegg)]

Eine andere Möglichkeit:

Wenn du den neuen Port Wert mit dem alten kompletten Port-Wert 
ver-xoderst, so bleiben dir genau dort 1 Bits stehen, wo es eine 
Änderung gab.

Dadurch werden dann in weiterer Folge die Abfragen auf die Änderungen 
einfacher.

  unsigned char   uchPortIn = PINA ^ uchBitMaskeXorA; // uchBitMaskeXorA dient dazu, einzelne Signale zur Laufzeit zu invertieren

  unsigned char   changedBits = uchPortIn ^ uchPortAalt;

  if( changedBits & DEF_PIN_SIGNAL_1 )
    behandle Änderung an PIN 1

  if( changedBits & DEF_PIN_SIGNAL_2 )
    behandle Änderung an PIN 2

  if( changedBits & DEF_PIN_SIGNAL_3 )
    behandle Änderung an PIN 3

  ...

  uchPortAalt = uchPortIn;
}

[Bernhard (Gast)]

Karl heinz Buchegger schrieb:
> Vielleicht sollte man auch einmal darüber sprechen, warum du denkst
> optimieren zu müssen.
> Hast du ein Problem, von dem du denkst das du es mit Optimierung lösen
> könntest?
Aktuell ist der Prozessor mit dem Ermitteln, Verarbeiten und 
weiterleiten der Signale voll ausgelastet.
Ich will jetzt noch eine zusätzliche Plausibilitätsprufung mit einbauen.
Dafür ist aber leider in der aktuellen Version nicht mehr genug 
Rechenzeit übrig.

Dein Lösungsansatz hat was.
Gibt es einen Weg zu verhindern dass die Interrupt-Routiene "leer" 
durchläuft wenn mehrere Pegeländerungen gleichzeitig auftreten?

[Karl H. (kbuchegg)]

Bernhard schrieb:

> Aktuell ist der Prozessor mit dem Ermitteln, Verarbeiten und
> weiterleiten der Signale voll ausgelastet.

Wie schnell sind die Signale?

> Ich will jetzt noch eine zusätzliche Plausibilitätsprufung mit einbauen.
> Dafür ist aber leider in der aktuellen Version nicht mehr genug
> Rechenzeit übrig.

Zeig mal den Rest

[Bernhard (Gast)]

Karl heinz Buchegger schrieb:
>> Aktuell ist der Prozessor mit dem Ermitteln, Verarbeiten und
>> weiterleiten der Signale voll ausgelastet.
>
> Wie schnell sind die Signale?

Es sind bis zu 8 unterschiedliche Signale zwischen 50Hz und 100Hz.
Dabei treffen die einzelnen Signale asynchron ein.
Dass bedeutet, es können sich ein oder mehrere Pegel gleichzeitig 
ändern.

Karl heinz Buchegger schrieb:
> Zeig mal den Rest

Da ist eigentlich nicht viel zu zeigen.
Aktuell wird nur dass Array mit den Daten über die USI im SPI-Modus 
übertragen. Ich vermute, dass der Fehler (starkes "flattern" der Werte), 
der bei zusätzlichen Aufgaben aufgetreten ist, vom Fehler bei der 
Auswertung aufgetreten ist.

Ich werde deinen Vorschlag ausprobieren und bescheid geben wie sich der 
uC verhält.

mfg Bernhard

[Karl H. (kbuchegg)]

Bernhard schrieb:
> Karl heinz Buchegger schrieb:
>>> Aktuell ist der Prozessor mit dem Ermitteln, Verarbeiten und
>>> weiterleiten der Signale voll ausgelastet.
>>
>> Wie schnell sind die Signale?
>
> Es sind bis zu 8 unterschiedliche Signale zwischen 50Hz und 100Hz.

Das gibts nicht, dass dein µC da nicht hinterher kommt.
Da hast du irgendwo einen kompletten Bock geschossen

>> Zeig mal den Rest
>
> Da ist eigentlich nicht viel zu zeigen.

Zeigs trotzdem.
Wenn du Rat zu einem Code haben willst:
Zeig den Code her und beschreibe ihn nicht.

[Bernhard (Gast)]

Karl heinz Buchegger schrieb:
> Das gibts nicht, dass dein µC da nicht hinterher kommt.
> Da hast du irgendwo einen kompletten Bock geschossen
Ja.
Nach einiger Suche habe ich den erlegten Bock gefunden.
Der Fehler lag nicht im bestehenden Programm.
Er lag darin, dass ich versucht hatte, Teile der Plausibilitätsprüfung 
im Interrupt zu erledigen.

Wenn alle Signalle gleichzeitig eine steigende Flanke und innerhalb 3 
bis 4 ms eine Fallende haben, tritt der Fehler auf.

Ich habe die gesammte Plausibilitätsprüfung in die Hauptschleife gelegt 
und jetzt läuft alles reibungslos.

Karl heinz Buchegger schrieb:
> Zeigs trotzdem.
> Wenn du Rat zu einem Code haben willst:
> Zeig den Code her und beschreibe ihn nicht.
Lieber nicht. Ich schäme mich dafür zu sehr.