If-Abfrage in ISR

[David S. (plazeber)]

Hallo Zusammen,

ich möchte in meiner Interruptabfrage, welche bei steigender und 
fallender Flanke ausgelöst wird, prüfen ob die Flanke fallend oder 
steigend war. Das funktionier soweit.
Mein einziges Problem ist es nun, das meine Interruptpins sich je nach 
Programmablauf verändern. Beispielsweise möchte ich manchmal auf PA4 
prüfen und manchmal auf PA5.
Dazu habe ich mir Variable "empfangspin" angelegt, in der der int-Wert 4 
oder 5 steht um diese während des Programmes verändern zu können. Das 
funktioniert  allerdings nicht. Mache ich die Abfrage allerdings mit PA4 
oder PA5 statt der Variable "empfangspin" funktioniert die Abfrage 
hervorragend.
An was liegt, bzw. wie kann ich das ändern?

ISR(PCINT0_vect)    //ISR zum Empfangen
{  
  if ( PINA & (1<<empfangspin))  //Prüfen auf High, steigende Flanke
  {
    TCCR1B |= PRESCALER;    
  }  
  if (!( PINA & (1<<empfangspin)))  //Prüfen auf Low, fallende Flanke
  {    
    TCCR1B =0;      //Timer stoppen
.............

[chris (Gast)]

empfangspin muss volatile sein

also

volatile uint8_t empfangspin;

[Harald N. (haraldn)]

wo und wie ist empfangspin definiert

[Karl H. (kbuchegg)]

David Schuetze schrieb:
> Dazu habe ich mir Variable "empfangspin" angelegt, in der der int-Wert 4
> oder 5 steht um diese während des Programmes verändern zu können. Das
> funktioniert  allerdings nicht.

Das müsste eigentlich schon funktionieren.
Mal ganz davon abgesehen, dass man das eigentlich nicht machen will, 
weil es den Compiler daran hindert, die eigentliche Pinabfrage zu 
optimieren und so schnell zu machen.

> An was liegt, bzw. wie kann ich das ändern?

du könntest zb das ganze so machen

ISR( ... )
{
  uint8_t pinWert;

  if( empfangspin == 4 )
    pinWert = ( PINA & ( 1 << PA4 ) );
  else
    pinWert = ( PINA & ( 1 << PA5 ) );

  if( pinWert )
    TCCR1B |= PRESCALER;
  else
    TCCR1B =0;      //Timer stoppen

...
}

es ist nicht ganz dasselbe, denn die Originallösung würde auf jeden Pin 
am Port A losgelassen werden können und nicht nur auf die Pins 4 bzw. 5.

[Karl H. (kbuchegg)]

Gewöhn dir bitte solche Sachen ab

  if ( PINA & (1<<empfangspin))  //Prüfen auf High, steigende Flanke
  {
...
  }  
  if (!( PINA & (1<<empfangspin)))  //Prüfen auf Low, fallende Flanke

Wenn der Pin als High identifiziert wurde, dann wird der eine Pfad 
genommen. Das Gegenteil davon ist ein else und nicht eine erneute 
Abfrage des Pins. Ein Pin kann nur entweder 0 oder 1 sein. WEnn er nicht 
1 ist, dann MUSS er 0 sein, es gibt keine andere Möglichkeit.

  if ( PINA & (1<<empfangspin))  //Prüfen auf High, steigende Flanke
  {
...
  }
  else
  {
...
  }

Du hast nämlich sonst 2 Abfragen hintereinander, für die du als 
Programmierer verantwortlich bist, dass die zweite immer das Gegenteil 
der ersten aussagt. Menschen machen aber Fehler. WEnn du genau dasselbe 
mit nur einer Abfrage ausdrücken kannst, dann mach das auch. Denn 
dadurch bist du automatisch diesen impliziten Zusammenhang zwischen 2 
Abfragen los, den du 100% korrekt hinkriegen musst. 'else' implementiert 
dir ganz von alleine diese "entweder - oder" Beziehung.

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

An Stelle variabler Shifts würde ich direkt die Maske speichern, 
insbesondere bei AVRs. Also statt
  empfangspin = PA4;
  if (PINA & (1<<empfangspin))
besser
  empfangspin = 1 << PA4;
  if (PINA & empfangspin)

[Peter II (Gast)]

A. K. schrieb:
> An Stelle variabler Shifts würde ich bei AVRs lieber direkt die Maske
> speichern. Also statt
>   empfangspin = 4;
>   if (PINA & (1<<empfangspin))
> besser
>   empfangspin = 1 << 4;
>   if (PINA & empfangspin)

dich denke nicht das das besser ist. Denn er weiß nicht wie viele Bits 
in empfangspin gesetzt sind. Damit kann ich nicht ein Einzels Bit vom 
Port abfragen.

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

Peter II schrieb:
> dich denke nicht das das besser ist. Denn er weiß nicht wie viele Bits
> in empfangspin gesetzt sind. Damit kann ich nicht ein Einzels Bit vom
> Port abfragen.

Ich verstehe nicht, was du damit meinst. Nach
  empfangspin = 1 << 4;
ist nach meinem Verständnis wirklich nur 1 Bit gesetzt.

Mir ging es allerdings eher darum, dass bei AVRs dynamische Shifts 
ziemlich ineffizient sind. Und man sie völlig ohne Verlust durch eine 
Maske ersetzen kann.

[Kaj (Gast)]

Ich denke mal, es handelt sich hier um einen AVR?! Wenn dem so ist, dann 
musst du (laut Datenblatt!) vor dem Einlesen eines Pins ein NOP() 
einfuegen!

z.B. Datenblatt des Mega2560, Kapitel 13.2.4 Reading the Pin Value, 
Seite 72
"Independent of the setting of Data Direction bit DDxn, the port pin can 
be read through the
PINxn Register bit. As shown in Figure 13-2 on page 71, the PINxn 
Register bit and the preced-ing latch constitute a synchronizer. This is 
needed to avoid metastability if the physical pin
changes value near the edge of the internal clock, but it also 
introduces a delay. Figure 13-3 on
page 73shows a timing diagram of the synchronization when reading an 
externally applied pin
value."

und auf Seite 74 der Beispielcode:

unsigned char i;
...
/* Define pull-ups and set outputs high */
/* Define directions for port pins */
PORTB = (1<<PB7)|(1<<PB6)|(1<<PB1)|(1<<PB0);
DDRB = (1<<DDB3)|(1<<DDB2)|(1<<DDB1)|(1<<DDB0);
/* Insert nop for synchronization*/
__no_operation();
/* Read port pins */
i = PINB;

[Peter II (Gast)]

A. K. schrieb:
> Ich verstehe nicht, was du damit meinst. Nach
>   empfangspin = 1 << 4;
> ist nach meinem Verständnis wirklich nur 1 Bit gesetzt.

aber davon weiß die ISR nichts mehr, für sie ist empfangspin nur eine 
variable.

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

Kaj schrieb:
> Ich denke mal, es handelt sich hier um einen AVR?! Wenn dem so ist, dann
> musst du (laut Datenblatt!) vor dem Einlesen eines Pins ein NOP()
> einfuegen!

Nicht in jedem Fall, sondern nur dann, wenn sich der Zustand vom Pin 
durch eine unmittelbar davor liegenden Änderung an irgendeinem 
PORTx/DDRx geändert haben könnte. Das ist hier aber nicht der Fall.

[Peter II (Gast)]

Kaj schrieb:
> Ich denke mal, es handelt sich hier um einen AVR?! Wenn dem so ist, dann
> musst du (laut Datenblatt!) vor dem Einlesen eines Pins ein NOP()
> einfuegen!

nein! Bitte richtig lese!

Nur wenn man die Datenrichtung ändert und dann liest.

[Johann L. (gjlayde)]

Na und? Der variable Shift wird dadurch auf jeden Fall vermieden; es 
wird eben ein AND gegen ein Register gemacht (das, is das empfangspin 
geladen wirde). Das dauert 1 Tick.

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

Peter II schrieb:
> aber davon weiß die ISR nichts mehr, für sie ist empfangspin nur eine
> variable.

Und? Als Pinnummer 8 reinzuschreiben ist auch nicht besser als die Maske 
0x30 reinzuschreiben.

Die Unterscheidung zwischen Nummer und Maske kann man über den Namen
  eingabepinMaske
signalisieren.

[Peter II (Gast)]

Johann L. schrieb:
> Na und? Der variable Shift wird dadurch auf jeden Fall vermieden; es
> wird eben ein AND gegen ein Register gemacht (das, is das empfangspin
> geladen wirde). Das dauert 1 Tick.

warum erst Optimierung und dann wieder ein Takt verschwenden? Dann kann 
man es gleich so stehen lassen, es scheint ja kein Zeitproblem zu geben.

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

Peter II schrieb:
> warum erst Optimierung und dann wieder ein Takt verschwenden? Dann kann
> man es gleich so stehen lassen, es scheint ja kein Zeitproblem zu geben.

Hast du mal nachgesehen, was bei 1<<n rauskommt? Eine Schleife mit n 
Durchläufen (vmtl n*4 Takte). Zusätzlich zu dem einen Takt fürs AND. Das 
ist nichts, was ich völlig unnötig in eine ISR stecken würde.

[Peter II (Gast)]

A. K. schrieb:
> Hast du mal nachgesehen, was bei 1<<n rauskommt? Eine Schleife mit n
> Durchläufen.

wobei n = 4 ist.

und wo ist das Problem?

Er hat keine Probleme mit dem Timing, so Problem liegt woanders. Warum 
wird wieder versucht eine Lösung für ein Problem zu finden was gar nicht 
da ist?

[David S. (plazeber)]

Also die Variable war als volatile deklariert. Ich bin erst ziemlich neu 
in der AVR-Programmierung. Es handelt sich um einen ATtiny20.

Jedenfalls funktioniert es jetzt hervorragend mit dieser Lösung:

ISR( ... )
{
  uint8_t pinWert;

  if( empfangspin == 4 )
    pinWert = ( PINA & ( 1 << PA4 ) );
  else
    pinWert = ( PINA & ( 1 << PA5 ) );

  if( pinWert )
    TCCR1B |= PRESCALER;
  else
    TCCR1B =0;      //Timer stoppen

...
}

Vielen Dank!!!  :)

[Rolf Magnus (Gast)]

Peter II schrieb:
> A. K. schrieb:
>> Ich verstehe nicht, was du damit meinst. Nach
>>   empfangspin = 1 << 4;
>> ist nach meinem Verständnis wirklich nur 1 Bit gesetzt.
>
> aber davon weiß die ISR nichts mehr, für sie ist empfangspin nur eine
> variable.

Ja, und da es eine Variable ist, ist es für die Berechnung auch wurscht, 
ob da nun genau ein Bit gesetzt ist oder mehrere. Die Assembler-Befehle 
sbic/sbir zum Abfragen einzelner Bits in I/O-Registern, auf die du dich 
vermutlich beziehst, gehen sowieso nur mit Immediate-Werten.
Besser ist die Variante von A.K. gegenüber der ursprünglichen auf jeden 
Fall, da man sich wie er schon schreibt die Schleife in der ISR für die 
Bit-Schieberei spart, die bei seiner Lösung komplett entfällt.

[Gefunden (Gast)]

David Schuetze schrieb:
> if( empfangspin == 4 )
>     pinWert = ( PINA & ( 1 << PA4 ) );

noch schöner
 if( empfangspin == PA4 )
     pinWert = ( PINA & ( 1 << PA4 ) );

[Rolf Magnus (Gast)]

Peter II schrieb:
> A. K. schrieb:
>> Hast du mal nachgesehen, was bei 1<<n rauskommt? Eine Schleife mit n
>> Durchläufen.
>
> wobei n = 4 ist.
>
> und wo ist das Problem?

Es werden unnötig Takte verbraten, ohne daß das auch nur im Geringsten 
irgendeinen Vorteil hätte, und es ist sehr einfach zu vermeiden.

> Er hat keine Probleme mit dem Timing, so Problem liegt woanders.

Und? Darf man ihm deshalb keine Tipps geben, wie er sein Programm 
verbessern kann?

[Peter II (Gast)]

Rolf Magnus schrieb:
> Besser ist die Variante von A.K. gegenüber der ursprünglichen auf jeden
> Fall, da man sich wie er schon schreibt die Schleife in der ISR für die
> Bit-Schieberei spart, die bei seiner Lösung komplett entfällt.

schon klar das sie besser ist - nur trägt sie nicht zu Lösung das 
eigentlichen Problems bei.

Er hat zwar jetzt ein Lösung gefunden, aber wo das Problem lagt ist 
immer (mir) nicht bekannt.

[Quack (Gast)]

Noch besser:

pinWert = ( empfangspin == PA4) ? ( PINA & ( 1 << PA4 )) : pinWert;

:-D

[Peter II (Gast)]

Quack schrieb:
> Noch besser:
>
> pinWert = ( empfangspin == PA4) ? ( PINA & ( 1 << PA4 )) : pinWert;

und falscher.

pinWert wurde vorher nicht auf 0 gesetzt.

[Quack (Gast)]

Peter II schrieb:
> pinWert wurde vorher nicht auf 0 gesetzt.

Und jetzt auch nicht. Also?

[Peter II (Gast)]

Quack schrieb:
> Peter II schrieb:
>> pinWert wurde vorher nicht auf 0 gesetzt.
>
> Und jetzt auch nicht. Also?

bei den andere Lösung wird er immer gesetzt, bei deiner leider nicht.

[Quack (Gast)]

Peter II schrieb:
> bei den andere Lösung wird er immer gesetzt, bei deiner leider nicht.

Mein kleiner Spass bezog sich nur auf den zitierten Code, nicht auf den 
Rest.

[Peter D. (peda)]

Das mit der Bit-Maske ist schon o.k.:

volatile uint8_t empfangs_maske; // 0x10 or 0x20

ISR(PCINT0_vect)    //ISR zum Empfangen
{  
  if ( PINA & empfangs_maske ){  //Prüfen auf High, steigende Flanke
    TCCR1B |= PRESCALER;    
  } else {
    TCCR1B &= ~PRESCALER;      //Timer stoppen
  }
// ..

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

Peter Dannegger schrieb:
> Das mit der Bit-Maske ist schon o.k.:

Die muss aber keineswegs volatile sein.  Wenn die ISR aufgerufen wird,
kann der Compiler nie auf einen gecacheten Wert für die Maske
zurückgreifen, folglich muss er sie immer aus dem Hauptspeicher erst
einmal lesen, "volatile" oder nicht.

[Peter II (Gast)]

Jörg Wunsch schrieb:
> Die muss aber keineswegs volatile sein.  Wenn die ISR aufgerufen wird,
> kann der Compiler nie auf einen gecacheten Wert für die Maske
> zurückgreifen, folglich muss er sie immer aus dem Hauptspeicher erst
> einmal lesen, "volatile" oder nicht.

das stimmt, aber im Hauptprogramm muss es noch nicht in den Ram 
geschrieben wurden sein.

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

Jörg Wunsch schrieb:
> Die muss aber keineswegs volatile sein.  Wenn die ISR aufgerufen wird,
> kann der Compiler nie auf einen gecacheten Wert für die Maske
> zurückgreifen,

Ohne volatile kann es aber sein, dass der Inhalt nicht dort in die 
Variable geschrieben wird, wo das Statement steht, sondern erst später. 
Vielleicht zu spät.

[nie (Gast)]

A. K. schrieb:
> sondern erst später.
> Vielleicht zu spät.

oder auch nie