Interrupt-Abfrage >>> Routine vereinfachen

[021aet04]

Wenn du eine 1ms ISR benötigst geht es nicht anders als du es machst.

Es hat jeder seinen eigenen Programmierstil. Wichtig ist nur das es lesbar bleibt. Du musst denken das du z.B. in einem Jahr das Programm anschaust und du bzw jemand anderes ohne "Studium" deinen Code ohne Probleme lesen können musst. Deswegen solltest du Variablen, Defines, Funktionen,... so benennen das es logisch ist und du nicht erst suchen muss was das gerade macht.

Defines kannst du immer nehmen wenn du nur eine "Aktion" hast, wie z.B. einen Ausgang setzen. Wenn du mehrere Dinge machen musst und diese öfters ausführst, solltest du Funktionen verwenden.

Wichtig wird es aber wenn du eigene Bibliotheken erstellst. Dann hast du eine Headerdatei (*.h) und eine dazugehörende Codedatei (*.c). In die Headerdatei kommen die ganzen Defines und Funktionsprototypen und in die Codedatei kommen die eigentlichen Funktionen.

MfG Hannes

[frabe]

Hallo Leute.
Habe an meinem Code weiter gearbeitet, hänge aber gerade an Tasten-/Kontaktentprellung fest.

Bis auf folgende Anweisung mit "i"-Schleife, funktioniert alle einwandfrei - dank Eurer anregungen!
"i" in RegA_Abfrage() sollte die 10ms-Schleifen hoch zählen. Sind 30ms (Entprellzeit) keine Veränderung vorgekommen, werden die veränderten Eingänge ausgewertet und verarbeitet.

Irgendwo scheint sich hier ein Logikfehler zu verstecken, da RegA_Auswertung() nicht mehr angesprochen wird.

ISR(TIM1_COMPA_vect)
{
	OverflowZaehler++;
	EingIntervall++;
	
	if(OverflowZaehler >= 60000)				// 1ms*60 000 = 1Min // uint16_t 0…65.535
	{
		OverflowZaehlerReset();
	}
	if (EingIntervall==10)					// alle 10[ms] werden die Eingangskontakte abgefragt
	{
		RegA_Abfrage();					// Eingänge auf Veränderung analysiert
		EingIntervall=0;
	}
}

void RegA_Abfrage(void)
{	
	uint8_t RegA_neu;
	uint8_t RegA_dif;
	uint8_t i;

	RegA_neu = PINA;						// PINA-Regist. wird ausgelesen
	if(((RegA_dif = RegA_akt ^ RegA_neu) !=0) && i>=3) // Änderungen gegenüber Vor-Interrupt=1 UND >=30ms
	{
		RegA_Auswertung(RegA_dif, RegA_neu);		// Pins einzeld auswerten
		i=0;
	}
	i++;
	RegA_akt = RegA_neu;						// akt. PINA in Vor-Interrupt speichern, für den nächsten ISR
}

void RegA_Auswertung (uint8_t RegA_dif, uint8_t RegA_neu)  // Pins einzeld auswerten
{
	uint8_t RegA_high;
	uint8_t RegA_low;
	uint8_t Zuweisung=0;
	
	RegA_high = RegA_dif & RegA_neu;		// Pins von 0 auf 1 = 1
	RegA_low = RegA_dif & ~RegA_neu;	// Pins von 1 auf 0 = 1

	if(((RegA_high & (1<<PINA4)) != 0) && (PA4frei==1))	Zuweisung=41;		// PA4 wurde high
	if(((RegA_low & (1<<PINA4)) != 0) && (PA4frei==1))	Zuweisung=40;		// PA4 wurde low		
	if(((RegA_high & (1<<PINA5)) != 0) && (PA5frei==1))	Zuweisung=51;		// PA5 wurde high
	if(((RegA_low & (1<<PINA5)) != 0) && (PA5frei==1))	Zuweisung=50;		// PA5 wurde low

	switch(Zuweisung)
	{
		case 41:	LEDrt_ein;	Zuweisung=0;	break;	// PA4 high
		case 40:	LEDrt_aus;	Zuweisung=0;	break;	// PA4 low
		case 51:	SUM_ein;	Zuweisung=0;	break;	// PA5 high
		case 50:	SUM_aus;	Zuweisung=0;	break;	// PA5 low
	}
}

Bemerkung:
- OverflowZaehler ist static und wird mit 1ms, über Timer-Interrupt getaktet/hoch gezählt.
- PA4frei, PA5frei static-Var., kommen aus dem main und schalten die jeweiligen Pin ein/aus.
- RegA_akt, RegA_neu, EingIntervall sind static-Var.

- - - Aktualisiert - - -

Fehler gefunden!
"i" muss eine static-Variable sein!
Klappt jetzt prime - was haltet ihr von der Struktur?

[021aet04]

Ich finde den Code relativ unübersichtlich.

Ich würde es nicht auf so viele Funktionen aufteilen. Ich würde Funktionen nur verwenden, wenn du es öfter als 1x benötigst oder wenn man diese Funktion auch in anderen Projekten genau so verwenden kann (dann würde ich es aber in eine eigenen Bibliothek geben). Das Problem wenn du es auf so viele Funktionen aufteilst ist, das du immer suchen musst.

Ich würde alle Funktionen direkt in der ISR machen. Was mir auch auffällt ist, das du in der Auswertung zuerst eine Zahl einer Variable zuweißt und anschließend diese Variable abfrägst. Ich würde z.B. "Ledxx_ein", Ledxx_aus,... direkt in die "If"-Abfrage schreiben.

Edit:
Wenn ich deinen Code richtig verstanden habe (Pinabfrage und Weiterverarbeitung) sollte dieser Teil das gleiche machen.

Man sollte, wenn es geht, auf >, >=, < oder <= Abfragen und nicht auf == wie du es bei dem 10ms Takt gemacht hast. Der Grund ist das du theoretisch den Wert auf z.B. 11 stellen kannst und dann wird weitergezählt bis der Zähler überläuft und zählt dann wieder bis 10. Wenn du eine Bytevariable hast zählt er 255ms zu lange.

Ich habe bei dem 10ms Takt "5" genommen, weil bei diesem Programm ca. alle 2ms ein INT ausgelöst wird, bei dir muss "10" stehen.
Mit der Variable "Eingang_Freigabe" kannst du bestimmen welche Eingänge abgefragt werden. Du kannst so darauf zugreifen wie ich es im main gemacht habe (nach dem "sei();").
In der "10ms-Abfrage" wird geprüft ob der Eingang freigegeben ist und ob der Eingang vom vorhergehendem Eingang geändert hat (wird ebenfalls mit der Eingangsfreigabe verknüpft).
Wenn das alles zutrifft wird der "Prellzeitzähler" um 1 erhöht, weicht er ab wird sofort der Zähler auf 0 gesetzt und der Durchlauf wird erneut gestartet. Sollte der Zähler den Maximalwert erreichen (Eingang ist konstant) werden die Aktionen ausgeführt. Bei meinem Beispiel steuert PB0 den Ausgang PB4 und PB2 den Ausgang PB5.

Solltest du das Programm nicht ganz verstehen, einfach melden.

#define F_CPU 8000000UL


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


volatile unsigned char Prellzeit=0, Eingang_alt=0, Eingang_Freigabe=0, Takt_10ms=0;

#define Ledgn_ein PORTB |=  (1<<PB4)
#define Ledgn_aus PORTB &= ~(1<<PB4)
#define Ledrt_ein PORTB |=  (1<<PB5)
#define Ledrt_aus PORTB &= ~(1<<PB5)


ISR (TIMER0_OVF_vect)
{
     if (Takt_10ms >= 5)
     {
          if ((PINB & Eingang_Freigabe) & (Eingang_alt & Eingang_Freigabe))
          {
               Prellzeit++;
          
               if (Prellzeit >= 3)
               {
                    if (Eingang_alt & (1<<PB0)) Ledgn_ein;
                    else                        Ledgn_aus;
                    
                    if (Eingang_alt & (1<<PB2)) Ledrt_ein;
                    else                        Ledrt_aus;
                    
                    
               }               
          }    
          else
          {
               Prellzeit = 0;
          }          
     
          Eingang_alt = PINB;    
          Takt_10ms = 0;  
     }         
     
     Takt_10ms++; 
}     


int main(void)
{
     TCCR0B |= (1<<CS00) | (1<<CS01);
     TIMSK |= (1<<TOIE0);
     
     sei();
     
     Eingang_Freigabe = (1<<PB0) | (1<<PB2);

    while (1) 
    {
         
    }
}

MfG Hannes

[frabe]

Erst ein mal vielen Dank für deine Gedanken und Anregungen!

Ich finde den Code relativ unübersichtlich.

Ich weis was du meinst. Ich habe schon für Massenabfragen vorbereitet. Bei mir werden alle PAx überwacht.
Tatsächlich benutze ich aber nur 2 - daher werde ich mal in mich gehen und die "EierLegendeWollmilchSau" hinterfragen.

Ist es nicht wesentlich Recorcenschohnender, das gesamte Register auf Veränderungen abzufragen, als jede 1ms (oder 10ms) jeden Eingang einzelnd abzufragen? Was ist wenn ich 4 Eingänge pro Register nutze, ist das dann anders?

Ich würde alle Funktionen direkt in der ISR machen. Was mir auch auffällt ist, das du in der Auswertung zuerst eine Zahl einer Variable zuweißt und anschließend diese Variable abfrägst. Ich würde z.B. "Ledxx_ein", Ledxx_aus,... direkt in die "If"-Abfrage schreiben.

LedXX_ein, LedXX_aus ist nur ein Test-Platzhalter für aufwendigere Anweisungen.
Aber auch hier sehe ich die "Kanone die auf Spatzen zielt".

Solltest du das Programm nicht ganz verstehen, einfach melden.

Wieso nicht? Ist dein Code doch viel leichter lesbar und einfacher.

Man sollte, wenn es geht, auf >, >=, < oder <= Abfragen und nicht auf == wie du es bei dem 10ms Takt gemacht hast. Der Grund ist das du theoretisch den Wert auf z.B. 11 stellen kannst und dann wird weitergezählt bis der Zähler überläuft und zählt dann wieder bis 10. Wenn du eine Bytevariable hast zählt er 255ms zu lange.

Gute Idee.
Bei >= bringt ein "Verschlucken" nur eine minimale Unsauberkeit, aber kein Durchlaufen mit sich.

Ich habe bei dem 10ms Takt "5" genommen, weil bei diesem Programm ca. alle 2ms ein INT ausgelöst wird, bei dir muss "10" stehen.

Alle 10ms wird bei mir ausserhalb des ISR der EIngangszustand abgefragt. Sollte sich dieser 3 mal nicht ändern (30ms) wird der neue Zustand gesetzt.
Der 1ms-Takt "OverflowZaehler" wird später im main() noch anderweitig als Stopuhr verwendet.

[021aet04]

Deine Variante ist, vermutlich (nicht getestet), so Resourcenschonend wie meine. Ich habe z.B. nur Variablen die ich unbedingt benötige, Das ist die Freigabe und der alte Zustand der Eingänge. Du hast noch viel mehr. Du hast viel mehr Variablen (auch wenn diese nur temporär sind), das ist Speicher (Ram) und zusätzlich gibt es mehr Code, das ist auch Speicher (Flash).

Bei meiner Variante ist es gleich wie bei deiner. Zuerst wird auf Veränderung geachtet. Erst wenn 3x hintereinander der Zustand gleich bleibt wird der Eingang abgefragt (dafür ist "Prellzeit" zuständig). Was man eventuell noch ändern könnte ist, den Zustand des Pinregisters 1x in eine temporäre Variable schreiben und mit dieser dann weiterarbeiten. Ich frage das PIN-Register 2x ab (einmal bei der Abfrage und einmal wenn ich den Zustand in die Variable "Eingang_alt"). In dieser Zeit könnte sich der Wert allerdings ändern.

Du fragst auch jeden Pin einzeln ab, du fragst sogar 2x ab und ich nur einmal. Du schaust ob es High ist und dann ob es low ist. Das kannst du dir eigentlich sparen, denn es gibt keinen 3ten Zustand und somit ist es automatisch low wenn es nicht high ist.

Wenn du auf bestimmte Muster von den Eingängen abfragst, kannst du mit switch arbeiten. Als Beispiel du willst etwas ausführen wenn am PortB nur PB0 und PB2 high sind und alle anderen low kannst du auf 0x03 abfragen.

Also in etwa so:

switch (Eingangsbyte)
{
     case 0x03:
          ........
          ........
          Mach was
          ........
          ........
     break;
}

Bild hier   Zitat von 021aet04 Bild hier  
Man sollte, wenn es geht, auf >, >=, < oder <= Abfragen und nicht auf == wie du es bei dem 10ms Takt gemacht hast. Der Grund ist das du theoretisch den Wert auf z.B. 11 stellen kannst und dann wird weitergezählt bis der Zähler überläuft und zählt dann wieder bis 10. Wenn du eine Bytevariable hast zählt er 255ms zu lange.



Gute Idee.
Bei >= bringt ein "Verschlucken" nur eine minimale Unsauberkeit, aber kein Durchlaufen mit sich.

"Verschlucken" wird er sich nicht, solltest du aber irgendwie auf diese Variable von "Außen" zugreifen können, kann man diesen Wert ändern. Bei deiner Anwendung wird es aber nicht sein (außer dein Speicher sollte kaputtgehen, was aber eher unwahrscheinlich ist). Bei einer SPS, wäre es aber möglich (somit sollte man immer so arbeiten).




Ich habe nur die ISR gemacht (und die Konfiguration des Timers nur über Prescaler), mit einem Attiny2313 mit 8MHz, dadurch stimmt das ganze Timing nicht exakt mit deinem Programm überein.

MfG Hannes

[frabe]

Danke, Hannes.
Deine Ausführungen sind für mich sehr hilfreich!

[frabe]

Frage:
Wie "legetim" ist es, aus einem ISR mit longjmp in das main() zurück zu springen, sollten bestimmte Bedingungen innerhalb des ISR erfüllt sein?
Hintergrund:
Mehrfache Tastenabfrage+Entprellung via Timer-ISR. Je nach Ablaufposition innerhalb des main() sollen unterschiedliche Befehle ausgeführt werden. Einfaches Bsp: ein delay() abbrechen/überspringen.

[Klebwax]

Frage:
Wie "legetim" ist es, aus einem ISR mit longjmp in das main() zurück zu springen, sollten bestimmte Bedingungen innerhalb des ISR erfüllt sein?

Was bedeuted legitim hier? Die Frage ist: darf man es benutzen? Wenn es zulässig ist, dann darf man es auch. Eine zweite Frage wäre, ist es sinnvoll?

Einige aber sicher nicht alle Antworten, die mir so einfallen.

Ich benutze C schon ziemlich lange, hatte aber noch nie das Bedürfnis, longjmp zu benutzen. Ich hab es also noch nie für sinnvoll gehalten.

Setjmp/longjmp ist ein Minenfeld. In MISRA ist es IMHO verboten. Wer sich sehr gut in C auskennt, mag vielleicht in diesem Minenfeld navigieren können, warum Anfänger das tun wollen, erschließt sich mir nicht.

Ein Interrupthandler verhält sich zwar wie eine normale C-Funktion, ist aber keine normale Funktion. Das sieht man an den Atributen, die sie hat. Der Code den der Compiler erzeugt und der vor der ersten Zeile C-Code der Funktion läuft ist ebenso wie der Code, der bei einem return erzeugt wird, ein anderer. Verlässt man eine solche Funktion, wird das, was sonst beim return gemacht wird, ausgelassen. Die Konsequenzen hab ich jetzt nicht untersucht.

Ob das ganze Konzept

Je nach Ablaufposition innerhalb des main() sollen unterschiedliche Befehle ausgeführt werden.

Sinn macht, hab ich jetzt nicht angesehen.

MfG Klebwax

[Sisor]

Frage:
Wie "legetim" ist es, aus einem ISR mit longjmp in das main() zurück zu springen, sollten bestimmte Bedingungen innerhalb des ISR erfüllt sein?
Hintergrund:
Mehrfache Tastenabfrage+Entprellung via Timer-ISR. Je nach Ablaufposition innerhalb des main() sollen unterschiedliche Befehle ausgeführt werden. Einfaches Bsp: ein delay() abbrechen/überspringen.

Wie legitim? Auf einer Skala von 0 bis 100 eine solide 0,001. Mit anderen Worten: Ohne sehr gute Gründe überhaupt nicht.
In C mit normalen Sprachfeatures nur die Möglichkeit über ein return eine Funktion ( und ja: ISR ist streng genommen ein Makro, wird aber vom Kompiler zu einer void ISR_Name (void)-Funktion reduziert) zu verlassen (In void()-Funktionen trägt der Kompiler das return nachträglich ein, falls es nicht ausdrücklich dort steht.) Das hat hauptsächlich den Grund eine korrekte Funktion des Aufrufstapels zu gewährleisten. Mit Sprüngen aus einer Funktion ist das Verhalten des Programms daher undefiniert.

Dein Problem kann man anders lösen, z.B. mit einer abbrechbaren warte-Funktion wie dieser:

volatile bool abbrechen = false;
void warte(uint ms) {
 while(--ms>0 && !abbrechen) delay(1);
 abbrechen = false;
}