Flankenerkennung für UART-Datenlogger rationeller gestalten, aber wie?

[spencer (Gast)]

Hi, ich habe hier nen Code für die flankenberücksichtigende Auswertung 
vom PinA des Mega32@4MHz entworfen. Bei Flankenwechsel wird ein Text 
(später mit Uhrzeit von einer RTC über TWI) ausgegeben. Der 
Übersichtlichkeit halber habe ich nur die steigende Flankenauswertung 
und nur das nötigste gepostet. Jetzt ist die Frage an euch, ob der Code 
rational genug geschrieben ist oder man vielleicht das "checke1(void);" 
eleganter, vielleicht auch mit einer for-Schleife lösen kann.

MfG

spencer

char e1_text[15] = "kein Text1"; //usw..
int e1hi[8]; // Array für Pins, welche auf High gegangen sind
int e1c = 0; // Änderungsvariable = 0 wenn sich E1 (PINA) nicht geändert hat)

SIGNAL (SIG_OVERFLOW0){    // Timer-Interruptroutine

int i;
  for (i=0;i<8;i++) {  // Zustand der Eingänge 1 bis 8 vergleichen und an Array e1 weitergeben

      if (!(PINA&(1<<i) == pina_last&(1<<i))){ // Mindestens ein Pin auf E1 hat sich verändert, prüfen!!!

    e1c =0xFF;  // Benachrichtigungschar "E1 hat sich geändert!"

      if (!(PINA&(1<<i))){
          e1hi[i] = 1;}

      if ((PINA&(1<<i))){
          e1lo[i] = 1;}
                    }
}
}

int main(void){      // Hauptschleife
...
sei(); // IRQ einschalten
reade1();

   while (1) {

 if(!(e1c == 0))     // Änderungschar für E1 prüfen
 {
 checke1();
 }
}
   return 0; // nie erreicht
  
}

int checke1(void){    // E1 hat sich verändert, prüfe, welche Pins und drucke!
cli(); // IRQ abschalten
int i;

for (i=0; i>8; i++){    // steigende Flanke detektieren

if (!(e1hi[i] == 0)){

  if ((i == 0)){
  uart_puts(e1_text);
  get_high();
    }

  if ((i == 1)){
  uart_puts(e2_text);
  get_high();
  }
....
  if ((i == 7)){
  uart_puts(e8_text);
  get_high();
  }
}
varres1();
reade1();
sei();// IRQ einschalten
}

int reade1(void){    // E1 abgleichen
pina_last = PINA;
}

int varres1(void){    // Änderungsindex bzw. Statusarrays auf 0 setzen (E1)
int i;
  
  for (i=0;i<8;i++) {  // Statusarray auf 0 setzen
  e1hi[i] = 0;
     }

  for (i=0;i<8;i++) {  // Statusarray auf 0 setzen
  e1lo[i] = 0;
     }
  e1c = 0;    // Änderungsindex E1 reseten

}

int get_high(void){    // grafische Darstellung für steigende Flanke
cmd_cr(1);
uart_puts( "(kommend)");
cmd_cr(1);
}

[Frank L. (florenzen)]

Hallo spencer,
ich habe deinen Code nicht bis ins Detail angeschaut, dazu waren es dann 
doch schon ein paar Glas Rotwein zuviel, jedoch kurz überflogen.
- Dir ist bewußt wie breit ein int in avr-gcc ist?
- Du weißt das das Makro SIGNAL obsolet ist?
- Schau dir mal Pin-Change Interrupts an.
- Das elhi Array, welches bei dir 128 Bit breit ist lässt sich
  auf 8 Bit eindampfen wenn du ein Bitfeld draus machst:

struct {
 unsigned a1:1;
 unsigned a2:1;
 unsigned a3:1;
 unsigned a4:1;
 unsigned a5:1;
 unsigned a6:1;
 unsigned a7:1;
 unsigned a8:1;
} elhi;

Wenn du möchtest kann man da noch als union ein char drüberlegen, 
meinetwegen "inport" um in einem Rutsch den Port einzulesen:

inport = PINA;

Das wars vom kurz drüberschauen.

[Frank L. (florenzen)]

Zur obigen Struktur:
Wenn du per union ein Portregister da draufschreiben willst solltest du 
die Struktur von Big-Endian auf Little-Endian drehen.

[spencer (Gast)]

Hm stimmt... mein Fehler! Statt int werde ich char beim e1hi verwenden. 
Ich dachte, beim Lesen von Eingängen ist es günstiger, nen Timer-IRQ zu 
verwenden und zu pollen anstatt nen pinchange?

MfG

spencer

[Karl H. (kbuchegg)]

Wenn du nur wissen willst, ob sich irgendein Pin am PINA verändert
hat, dann musst du das nicht Einzelbitweise machen.

Vergleiche einfach den kompletten Portzustand mit dem kompletten
Portzustand unmittelbar davor.

uint8_t PrevPin;

ISR( TIMER0_OVF_vect )
{
  uint8_t i;
  uint8_t Mask;
  uint8_t PinNow = PINA;

  if( PinNow != PrevPin ) {
    // ein Pin am Port A hat seinen Zustand geändert

    e1c =0xFF;  // Benachrichtigungschar "E1 hat sich geändert!"
    PrevPin = PinNow;

    // finde die momentane PinBelegung (Hi Lo raus)
    Mask = 0x01;
    for( i = 0; i < 8; ++i ) {
      ello[0] = !( elhi[0] = ( PinNow & Mask != 0 ) );
      Mask = Mask << 1;
    }
  }
}

Erhebt sich natürlich noch die Frage, wozu du das ello Array
überhaupt brauchst. Wenn ein Pin nicht High ist (also in elhi
an der entsprechenden Stelle eine 1 steht), dann kann er nur
Low sein (elhi muss dann an der Stelle 0 sein, dafür ist
ello an dieser Stelle 1)

Zwischen elhi und ello gibt es also einen Zusammenhang:
elhi[i] != ello[i];

wenn elhi[i] gleich 1 ist, dann muss ello[i] 0 sein und
umgekehrt. Damit kannst du aber ello eleminieren, da du
von elhi immer weist wie der Eintrag lauten muss.

[spencer (Gast)]

Naja ich wollte, dass auf der UART nur der jeweilige pin ausgegeben 
wird, der sich verändert hat. Deshalb E1HI und einmal E1LO. Wenn z.B. 
E1HI 0b00000001 ist, weiß ich, dass PINA.0 (und NUR PINA.0) high wurde 
(und vorher low war) --> jeweiligen Text ausgeben. Wenn also dasselbe 10 
sek. später in E1LO steht, weiß ich, dass selbiger low geworden ist --> 
Text ausgeben. Es geht eben um reine Pinauswertung, nicht nen kompletten 
Port, sonst befürchte ich schaut mein Terminal bei jedem Pinwechsel so 
aus:


PINA.0 hi --> low
PINA.1 low--> hi
PINA.2 hi --> low
PINA.3 low--> hi
PINA.4 hi --> low
PINA.5 hi --> low
PINA.6 low--> hi
PINA.7 hi --> low

(und das, obwohl nur PINA.0 low geworden wäre)

MfG

[Karl H. (kbuchegg)]

spencer wrote:
> Naja ich wollte, dass auf der UART nur der jeweilige pin ausgegeben
> wird, der sich verändert hat.

OK. Das hab ich dann wohl missverstanden.
Mir kommen nur in deinem Code zuviele PIN Abfragen vor, sowie
etwas zu oft eine Schieberei von 1 mit einer Variablen.

Die häufigen PIN Abfragen sin dproblematisch, weil sich ja in
der Zeit von einer Abfrage zur nächsten der Zustand des
Eingangspins schon wieder verändert haben könnte.

zb hier

      if (!(PINA&(1<<i))){
          e1hi[i] = 1;}

      if ((PINA&(1<<i))){
          e1lo[i] = 1;}
                    }

Niemand kann dir garantieren, dass nicht tatsächlichj beide
Vergleiche false ergeben. Der PINA Wert bei der ersten Abfrage
hat ja nichts mit dem PINA der zweiten Abfrage zu tun. Und beide
wiederrum haben nichts mit der übergeordneten if-Bedingung zu tun,
die ebenfalls ihren eigenen PIN Wert benutzt.

Theroretisch könnte es dir also passieren, dass hier

      if (!(PINA&(1<<i) == pina_last&(1<<i))){ // Mindestens ein Pin auf E1 hat sich verändert, prüfen!!!

zwar ein Pin Change detektiert wird, aber die beiden nachfolgenden
Auswertungen weder Low noch High detektieren -> du hast die
Flanke verloren.

-> Das PINA Register nur einmal auslesen, und dann mit dem ausgelesenen
Komplett-Wert weitermachen.

Das andere sind die vielen  1 << i.  Da der AVR das nicht in einer
Assemblerinstruktion machen kann. Wenn du Glück hast, dann hat sich
der Optimizer diese Berechnung vorgezogen und macht sie nur einmal.
Aber darauf würde ich so nicht vertrauen.

[spencer (Gast)]

Also denkst du, es ist besser, wenn ich vorher

char tempPinA = 0;

tempPinA = PINA;
if (!(tempPinA&(1<<i) == pina_last&(1<<i))){ ...

und dann

 if (!(tempPinA&(1<<i))){
          e1hi[i] = 1;}

schreibe? Alles klar, werd ich ausprobieren. Ich konnte den Code leider 
noch nicht testen, da ich unter Linux (KontrollerLab) code und für die 
Zielschaltung das Board noch nicht fertig ist. Desweiteren kenne ich 
unter Linux leider noch keinen Simulator für AVRs. Daher verläuft die 
Entwicklung des Codes unter erschwerten Umständen, aber das ist ein 
anderes Thema.

Danke für die bisherige Hilfe


MfG

spencer