3x4 Tastenmatrix in der ISR

[Erwin (Gast)]

Hallo Community,

ich dachte, ich versuche mich auch mal mit einer 3x4 Tastenmatrix am 
ATMega8. Ich setze abwechselnd die Spalten auf LOW und prüfe dabei die 
Zeilen ab. Die Zeilen sind als Eingänge definiert mit internen Pullups 
und die Spalten sind als Ausgänge definiert. Zeilen 2-4 werden 
fehlerfrei übertragen, jedoch Zeile 1 nicht. Ich habe die erste Zeile 
mit dem Multimeter auf Durchgang überprüf-> i.O.
Ich bitte um Durchsicht meines Codes.

in diesem Sinne:

ISR (TIMER0_OVF_vect)
{
  
  PORTB|=(1<<Spalte1)|(1<<Spalte2)|(1<<Spalte3);
  
  switch(position)
  {    
    case 0:
    PORTB&=~(1<<Spalte1);
    if (!(PINB&(1<<Zeile1)))
    {
      uart_puts("1");
      _delay_ms(300);
    }
    else if (!(PINB&(1<<Zeile2)))
    {
      uart_puts("4");
      _delay_ms(300);
    }    
    else if (!(PINB&(1<<Zeile3)))
    {
      uart_puts("7");
      _delay_ms(300);
    }    
    else if (!(PINB&(1<<Zeile4)))
    {
      uart_puts("*");
      _delay_ms(300);
    }    
    break;
        
    case 1:
    PORTB&=~(1<<Spalte2);
    
    if (!(PINB&(1<<Zeile1)))
    {
      uart_puts("2");
      _delay_ms(300);
    }
    else if (!(PINB&(1<<Zeile2)))
    {
      uart_puts("5");
      _delay_ms(300);
    }    
    else if (!(PINB&(1<<Zeile3)))
    {
      uart_puts("8");
      _delay_ms(300);
    }    
    else if (!(PINB&(1<<Zeile4)))
    {
      uart_puts("0");
      _delay_ms(300);
    }
    break;
        
    case 2: 
    PORTB&=~(1<<Spalte3);
    if (!(PINB&(1<<Zeile1)))
    {
      uart_puts("3");
      _delay_ms(300);
    }
    else if (!(PINB&(1<<Zeile2)))
    {
      uart_puts("6");
      _delay_ms(300);
    }    
    else if (!(PINB&(1<<Zeile3)))
    {
      uart_puts("9");
      _delay_ms(300);
    }    
    else if (!(PINB&(1<<Zeile4)))
    {
      uart_puts("#");
      _delay_ms(300);
    }
    break;  
  }
  position++;
  if (position==3)
  {
    position=0;
  }
    
}

[Keymatrix (Gast)]

Ich würde an deiner Stelle das Rad nicht neu erfinden.

Insbesondere wenn die Neuerfindung so schlecht ist.

siehe z.B.

Beitrag "Taster-Matrix Code von Peda auf AVR"

[Erwin (Gast)]

Keymatrix schrieb:
> Ich würde an deiner Stelle das Rad nicht neu erfinden.
>
> Insbesondere wenn die Neuerfindung so schlecht ist.
>
> siehe z.B.
>
> Beitrag "Taster-Matrix Code von Peda auf AVR"

Was genau findest du daran schlecht? Ich bin für Verbesserungen offen, 
zähle mich aber -vlt. im Gegensatz zu dir- zu der Fraktion, die nicht 
gleich Code kopiert und nach Libs schreit.

[Achim (Gast)]

Vermutlich brauchst du ein paar us oder Takte, bis du Zeile 1 lesen 
darfst.

Der Code ist wirklich schlecht.
Kopierter Code für Spalten und reihen
Bitshiftmanipulation direkt
Warten und senden in ISR
Nur eine Taste von x gewinnt
Position weder statisch noch Abfrage > 2

[A. S. (Gast)]

jetzt bin ich am PC und möchte meine Kritik begründen.

> Kopierter Code für Spalten und Reihen
Die cases unterscheiden sich nur dadurch, dass ein anderes Bit in Port B 
gesetzt wird und ein anderes Zeichen gesendet. Dafür werden 2 Dutzend 
Zeilen kopiert und angepasst. obfuscated Alternative: 
uart_put("1234567890#*"[Zeile+position*3]); //;-)

> Bitshiftmanipulation direkt
(!(PINB&(1<<Zeile2)))
Bei den meisten (flüchtigen) Zeichenfehlern kann niemand warnen. Bei 
"PIN_LOW(PINB, Zeile2) dagegen führen 99% aller Zeichenfehler zu einem 
Build-Fehler. Hier wäre praktisch nur der Suffix von PIN oder Zeile zu 
fragil.

> Warten und senden in ISR
eine ISR sollte nur wenige Takte lang laufen, zumindest nicht viel mehr 
als der "Herzschlag", also der SysTicker. Sonst besser auf Taskebene.

> Nur eine Taste von x gewinnt
wenn das gewollt ist, ok. Aber sonst spräche ja nichts gegen parallele 
Auswertung und ggf. "e" bei mehr als 1 Taste.

> position weder statisch noch Abfrage > 2

wenn Position auch außerhalb verändert werden kann, so ist

  if (Position>2)
  {
    position=0;
  }

einfach robuster.

Ich habe die Kritik auch nur geäußert, weil

Erwin schrieb:
> Was genau findest du daran schlecht? Ich bin für Verbesserungen offen,