[XMEGA] Arge Verständnisprobleme mit der Bitmaske

[Holger (Gast)]

Hallo geliebtes Forum!

Ich habe mich dran gesetzt eine 3x4-Tasten-Matrix an meinem XMega zu 
betreiben. So weit so gut. Das Ding läuft, nur macht es nicht exakt was 
ich will :(

Wenn ich eine Taste gedrückt halte werden alle Nummern der Zeile, in der 
sich diese Taste befindet ausgegeben.

Halte ich z.B. die 2 gedrückt sehe ich am LCD 1, 2, 3, 1... durchlaufen.

Das liegt höchstwahrscheinlich am Vergleich in Zeile 11, der in:
if(PORTA.IN & PIN6_bm) {} aufgelöst werden sollte.
PIN6_bm ist per Definition 0x40, was binär 1000000 ergibt.

Ich glaubte also 1000xxx mit 1000000 zu verUNDen, was ich aber scheinbar 
nur zu einem Drittel tue.

Weiß jemand wie's richtig geht?
Ich steh gerade ziemlich auf dem Schlauch :(

// GET KEYPAD BUTTONS //
int8_t key_col[3] = {PIN2_bm, PIN1_bm, PIN0_bm}; // Definiert als OUTPUT
int8_t key_row[4] = {PIN6_bm, PIN5_bm, PIN4_bm, PIN3_bm}; // Definiert als INPUT mit internem PULLDOWN
int8_t key_map[12] = {0x01,0x04,0x07,0x0A,0x02,0x05,0x08,0x00,0x03,0x06,0x09,0x0B};

uint8_t get_key_matrix() {
  int8_t result = 0x0C; // Ungültiger Wert
  for (int8_t x=0; x<=2; x++) {
    PORTA.OUTSET = key_col[x]; // Spalte X unter Spannung setzen
    for (int8_t y=0; y<=3; y++) {
      if(PORTA.IN & key_row[y]) { // Reihe Y abfragen 
        delay_ms(500); // Warteschleife nur zu Anschauungszwecken
        result = key_map[4*x+y]; // Wert der aktuellen x-y-Koordinate aus Array holen 
        ltoa(result,&string,16); // Bytewert in String umwandeln
        lcd_print_string(string,0,0,&twi_lcd_packet); // Wert auf LCD ausgeben
      }
    }
  }
  key_matrix_countdown = 250; // Funktion die nächsten 250ms nicht erneut ausführen
}

Gruß,
Holger

[xfr (Gast)]

Lass Dir vielleicht mal direkt den Wert von PORTA.IN auf dem LCD 
ausgeben.

[Holger (Gast)]

Danke für den Tipp!
Hab ich prompt probiert und dabei gemerkt, dass es keine gute Idee ist 
die Spalten unter Spannung zu setzen, selbige dann aber nicht wieder 
spannungsfrei zu schalten.

Also was den oben stehenden Code angeht:
Der kann so bleiben, braucht aber noch ein...

PORTA.OUTCLR = key_col[x]; // Spalte X abschalten

...am Ende der "x" for-Schleife.

Dann klappt's auch mit dem UND :)

Was mich nun noch interessieren würde ist, ob das guter XMEGA-Stil ist, 
ein Nummernpad so auszulesen, oder ob jemandem ein weiteres Haar in der 
Suppe auffällt, nur raus damit!

Viele Grüße,

Holger

[Karl H. (kbuchegg)]

Mit solchen Formuliereungen
>   for (int8_t x=0; x<=2; x++) {
bin ich immer unglücklich.

Die einfachste for-Schleife in C lautet immer

   for( i = 0; i < Anzahl Durchläufe; i++ )

d.h. da steht die Anzahl der Durchläufe direkt drinnen und anstelle des 
<= steht immer ein <.

Wenn diese beiden Beingungen erfüllt sind, dann ist das eine ganz 
normale for-Schleife, wie man sie benutzt, um ein Array abzuarbeiten

  int values[5];

  for( i = 0; i < 5; ++i )
    mach was mit values[i]

da braucht man nicht lange analysieren und sich überlegen ob man jetzt 
hinten übers Array rausschiesst, oder ob man alle Werte hat. Das ist 
sozusagen ein immer gleiches 'Muster'. Und da sich praktisch alle 
C-Programmierer an dieses Muster halten, erleichtert das die Übernahme 
von fremden Code. Erst wenn das Muster nicht vorhanden ist, fangen die 
Glocken im Kopf zu läuten an.

Selbiges mit der Verwendung von 'magischen Konstanten' mitten im Code 
und deren manchmal seltsame Querverbindungen.
Wenn du nicht musst, tus nicht!

Entweder

#define NR_COLS   3
#define NR_ROWS   4
ädefine NR_KEYS  12    // NR_COLS * NR_ROWS

int8_t key_col[NR_COLS] = {PIN2_bm, PIN1_bm, PIN0_bm}; // Definiert als OUTPUT
int8_t key_row[NR_ROWS] = {PIN6_bm, PIN5_bm, PIN4_bm, PIN3_bm}; // Definiert als INPUT mit internem PULLDOWN
int8_t key_map[NR_KEYS] = {0x01,0x04,0x07,0x0A,0x02,0x05,0x08,0x00,0x03,0x06,0x09,0x0B};

uint8_t get_key_matrix() {
  int8_t result = 0x0C; // Ungültiger Wert
  for (int8_t x=0; x<NR_COLS; x++) {
    PORTA.OUTSET = key_col[x]; // Spalte X unter Spannung setzen
    for (int8_t y=0; y<NR_ROWS; y++) {
...

oder aber: lass den Compiler die Werte abzählen und daraus die 
Arraygrößen bestimmen.

[xfr (Gast)]

Holger schrieb:
> Was mich nun noch interessieren würde ist, ob das guter XMEGA-Stil ist,
> ein Nummernpad so auszulesen, oder ob jemandem ein weiteres Haar in der
> Suppe auffällt, nur raus damit!

Man kann sicherlich noch ein paar Dinge verbessern.

1. Du verschwendest 19 Byte RAM für Konstanten. Arrays mit Konstanten 
sind im Flash-Speicher besser aufgehoben.

2. Als Schleifenvariablen könnte man direkt die Bitmaske nehmen und in 
jedem Schleifendurchung shiften statt inkrementieren.

3. Die Keymap wäre also zweidimensionales Array übersichtlicher.

4. Statt der Keymap mit den Werten 0x00 bis 0x0B könnte man die Tasten 
einfach mit x + 4 * y oder (falls Du Tipp 2 umsetzt mit der Veroderung 
der beiden aktuellen Bitmasken) repräsentieren. Diesen Werten könnte man 
dann per #define oder enum Namen geben. Das hängt aber davon ab, wie Du 
die Tasten in der Anwendung nutzen willst.

Also meine Empfehlung wäre Tipp 2 und 4 umzusetzen, dann kommst Du sogar 
ganz ohne Arrays aus. :)

[Holger (Gast)]

@Karl Heinz
Da ist was dran, ab jetzt werd' ich das <= weg lassen.
Das macht die Sache echt übersichtlicher.
Konstanten könnte ich auch mehr spammen, da hast du recht.

@xfr

xfr schrieb:
> Arrays mit Konstanten
> sind im Flash-Speicher besser aufgehoben.

Flash kann ich noch nicht, ist aber eins meiner nächsten Ziele um mir 
gewisse Dinge von einer Steckdose zur nächsten zu merken.
Das Array muss aber auch schnell verfügbar sein, ist der Flash-Speicher 
dafür nicht zu langsam?

xfr schrieb:
> Also meine Empfehlung wäre Tipp 2 und 4 umzusetzen, dann kommst Du sogar
> ganz ohne Arrays aus. :)

Das hat super geklappt :)
Die Arrays sind weg und nur die Null-Taste muss separat betrachtet 
werden.
Gefällt mir persönlich sehr viel besser als die erste Variante.

Danke euch beiden!

Eine Frage hätte ich noch:
Die Zählvariablen "x" und "y" und das Ergebnis "key_value" müssen mMn 
bei jedem Funktionsaufruf neu angelegt werden.
Wäre es vielleicht schneller die im Hauptprogramm zu deklarieren?
Oder zumindest als static in der Funktion?

#define KEY_FIRST_COL PIN2_bm
#define KEY_FIRST_ROW PIN6_bm
#define KEY_NUM_ROWS 4
#define KEY_NUM_COLS 3

uint8_t get_key_matrix() {
  int key_value = 0x0F; // Ungültiger Wert
  for (int8_t x=0; x<KEY_NUM_COLS; x++) {
    PORTA.OUTSET = KEY_FIRST_COL >> x; // Spalte x unter Spannung setzen
    for (int8_t y=0; y<KEY_NUM_ROWS; y++) {
      if(PORTA.IN & KEY_FIRST_ROW >> y) { // Reihe y abfragen 
        key_value = y*KEY_NUM_COLS+x+1; // Wert der x-y-Koordinate berechnen
        if (key_value == 0x0B) key_value = 0x00; // Taste NULL
      }
    }
    PORTA.OUTCLR = KEY_FIRST_COL >> x; // Spalte x abschalten
  }
  return key_value;
}

[Karl H. (kbuchegg)]

Holger schrieb:

> Eine Frage hätte ich noch:
> Die Zählvariablen "x" und "y" und das Ergebnis "key_value" müssen mMn
> bei jedem Funktionsaufruf neu angelegt werden.

Sofern der Compiler es nicht hinkriegt, die lokalen Variablen überhaupt 
in Registern zu halten, bedeutet 'neu angelegt werden' im Regelfall: Der 
Stackpointer wird mit einer konstanten Zahl subtrahiert oder (beim 
return addiert). D.h. das 'Anlegen' ist im Grunde pipifax und nicht 
wert, dass man sich da gross Gedanken drüber macht. (Soferns nicht 
hunderte sind, die die Stackbelastung in die Höhe treiben)

> Wäre es vielleicht schneller die im Hauptprogramm zu deklarieren?

In dem Fall nicht.
Diese Variablen haben so einen dermassen großen lokalen Bezug (und nur 
lokalen Bezug), dass ich die auf jeden Fall in der Funktion lassen 
würde. Dort sind sie lokal und ich brauch mir keine Gedanken darüber 
machen, wenn eine andere Funktion ebenfalls Variablen x und y hat und 
sich da dubiose Zusammenhänge über mehrere Funktionsaufrufebenen hinweg 
ergeben können.

> Oder zumindest als static in der Funktion?

Könnte man machen. Ich würds aber trotzdem nicht tun. Das sind eindeutig 
lokale Variable, die nur während der Funktionsausführung eine Bedeutung 
haben. Weder davor noch danach noch beim nächsten Funktionsaufruf 
spielen ihre Werte irgendeine Rolle. Ein 'static' würde sie für meinen 
Geschmack zu sehr betonen.


Und im übrigen gilt (wie immer):
Hast du ein Problem mit der Funktion? Ist sie zu langsam? Hast du 
gemessen, dass sie zu langsam ist?
Wenn nein: dann lass sie so wie sie ist und leg dein Hauptaugenmerk auf 
die Korrektheit, Sauberkeit und Wartbarkeit der Funktion.

Getreu dem Motto:
* optimiere nicht, wenn du es nicht musst (ausgenommen natürlich
  Programmiertechniken die unter das Thema 'allgemeine gute
  Programmierrichtlinien' fallen. Also jetzt nicht absichtlich
  blödsinnig ineffizienten Code)
* optimiere nicht auf Verdacht. Erst mal messen, wie groß das
  Einsparungspotential einer Funktion ist. Durch Messen hat
  schon so mancher sein blaues Wunder erlebt.
* eine langsame Funktion, die korrekt arbeitet, ist immer noch
  nützlich
  eine schnelle Funktion, die falsche Ergebnisse liefert, ist
  hingegen niemals nützlich - egal wie schnell sie ist.
  Die Reihenfolge lautet daher: erst korrekt, dann schnell.
  Und nicht umgekehrt.

[xfr (Gast)]

Ich hatte das mit dem Shiften noch etwas anderes gemeint. Du kannst 
nämlich auch direkt die Bitmaske als Schleifenvariable benutzen. Ich 
habe es mal schnell umgeschrieben. Keine Garantie, dass die #defines so 
stimmen, aber das Prinzip sollte klar sein:

#define COL_FIRST PIN0_bm
#define COL_LAST  PIN2_bm
#define ROW_FIRST PIN3_bm
#define ROW_LAST  PIN6_bm

#define KEY_0 (PIN1_bm | PIN3_bm)
#define KEY_1 (PIN2_bm | PIN6_bm)
#define KEY_2 (PIN2_bm | PIN6_bm)
[...]
#define KEY_B (PIN0_bm | PIN3_bm)

uint8_t get_key_matrix()
{
  int key_value = 0x0F; // Ungültiger Wert
  for (uint8_t col = COL_FIRST; col <= COL_LAST; col <<= 1) {
    PORTA.OUTSET = col; // Aktuelle Spalte unter Spannung setzen
    for (uint8_t row = ROW_FIRST; row <= ROW_LAST; row <<= 1) {
      if (PORTA.IN & row) { // Reihe y abfragen 
        key_value = row | col; // Wert zuweisen
      }
    }
    PORTA.OUTCLR = col; // Aktuelle Spalte abschalten
  }
  return key_value;
}

Im Hauptprogramm könntest Du es dann z.B. so auswerten:

uint8_t key = get_key_matrix();
switch (key) {
  case KEY_0:
    [...]
  case KEY_1:
   [...]
}

Das schöne darin ist, dass man die Berechnungen auf ein Minimum 
beschränkt. Man braucht nichts addieren oder multiplizieren, nur einmal 
pro Schleifengang shiften. Wenn es Dir zu ungewohnt aussieht gilt aber 
was Karl Heinz sagt: Im Zweifel lieber die verständlichere Variante als 
die schnellere. Der Mikrocontroller dürfte für beide schnell genug sein. 
Und wenn Dir wichtig ist, dass der Variableninhalt dem Wert der Tasten 
entspricht, musst Du natürlich auch Deine Variante nehmen.

Variablen im Flashspeicher ablegen ist übrigens nicht schwer. Du musst 
nur avr/pgmspace.h inkludieren, PROGMEM vor die Variablendefinition 
schreiben und z.B. per pgm_read_byte() darauf zugreifen. Langsam ist das 
nicht. Der Programmcode liegt ja schließlich auch im Flash. Um sich 
Dinge "von einer Steckdose zur nächsten" zu merken, ist der 
Flashspeicher aber nicht gedacht, dafür gibt es das EEPROM.