Struct; Was ist eleganter?

[Max (Gast)]

Hallo ich brauche mal eurer Hilfe, weil ich den Eindruck habe das was 
ich da mache nicht so ganz elegant ist! ;-)

meine Struktur

typedef struct
{
  uint8_t color;
  uint8_t won;
}pProfil;

später im Code

void PlayTheGame()
{
//...
static pProfil spieler1 = {0,0};
static pProfil spieler2 = {0,0};

static pProfil *sp1 = &spieler1;
static pProfil *sp2 = &spieler2;
//...
}

Wie kann man das kürzer machen? Möglichst in zwei Zeilen... Das 
Einzigste was mir einfallen würde, wäre mit malloc zu arbeiten. Wobei 
man ja hier eigentlich eine dynamische Speicherverwaltung eigentlich 
nicht braucht?! Zu mal man ja dann trotzdem noch Zuweisung der Werte 
machen muss.
Ich muss alle vier als static deklarieren ne? Sprich das so richtig?!

Und so kommen wir zu meinem zweiten Knackpunkt. Später wenn das Spiel 
dann vorbei ist möchte ich die Werte von sp1 und sp2 zurücksetzten.

So habe ich es jetzt gemacht. Was wie ich finde auch nicht so prall 
ist...

void resetplayer(pProfil *sp1, pProfil *sp2)
{
 
 sp1->color = 0;
 sp1->won = 0;
 sp2->color = 0;
 sp2->won = 0;
}

Mit folgendem würde ich mich ja schon zu frieden geben. Allerdings 
reserviere ich ja wieder für die "profil" Speicherplatz?!

void resetplayer(pProfil *sp1, pProfil *sp2)
{
 pProfil profil = {0,0};
 *sp1 = profil;
 *sp2 = profil;
}

LG Max

[Stefan B. (stefan)]

Deine Lösung ist im Grund doch OK.

Man kann die beiden static Structs noch aus der Funktion rausziehen und 
zu globalen Structs machen. Bei µC ist das angenehmer, weil der RAM 
Bedarf von Variablen DATA (=> globale Structs) und BSS zur Compilezeit 
ausgerechnet werden kann. Zur Laufzeit erlebt man dann selterner 
unliebsame Überraschungen mit überlaufendem Stack/Heap (lokale Variablen 
in Funktionen, malloc/free).

Um das Optimieren beim Initialisieren beim neuen Spiel würde ich mich 
nicht kümmern. Das ist eine einmalige Peanuts-Aktion. Die macht man 
irgendwie (z.b. wie dein 1. Beispiel, bei globalen Struchts ohne 
Funktionsargumente) und gut ist's. Da lohnen sich andere Optimierungen 
bestimmt mehr ;-)

[Rolf Magnus (Gast)]

> Wie kann man das kürzer machen? Möglichst in zwei Zeilen...

Hmm...

static pProfil spieler1 = {0,0}, spieler2 = {0,0};
static pProfil *sp1 = &spieler1, *sp2 = &spieler2;

Ist halt nicht so lesbar. Was erhoffst du dir eigentlich durch eine 
Reduktion auf zwei Zeilen?

> Ich muss alle vier als static deklarieren ne? Sprich das so richtig?!

Das kommt darauf an, wozu du sie brauchst.

> So habe ich es jetzt gemacht. Was wie ich finde auch nicht so prall
> ist...

Wieso? Was gefällt dir daran nicht?

> Mit folgendem würde ich mich ja schon zu frieden geben. Allerdings
> reserviere ich ja wieder für die "profil" Speicherplatz?!

Temporär, möglicherweise, ja.

[Sven P. (Gast)]

Mir erschließt sich ehrlich gesagt der Sinn der globalen Zeiger nicht so 
ganz.

[Max (Gast)]

Ah dann bin ich ja beruhigt! Hätte ja sein können, dass man es kürzer 
machen kann... :-)

Jetzt habe ich allerdings ein Speed Problem. Und zwar will ich mit 
folgender Funktion Registieren, welche Taste gedrückt worden ist. 
Allerdings mit dieser Funktion geht das nicht wirklich gut. Muss 
meinstens mehr mals drücken, bis ein Druck registriert worden ist.

Die Funktion

int8_t getAnyKeyPush(void)
{
  int8_t ret;
  //Zeile 1
  if(delay_buttonpush(&TASTEN_Z12, TASTE_Z1_1))
  {
    ret = FELD1;
  }
  else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z1_2))
  {
    ret = FELD2;
  }
  else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z1_3))
  {
    ret = FELD3;
  }
  //Zeile 2
  else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z2_1))
  {
    ret = FELD4;
  }
  else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z2_2))
  {
    ret = FELD5;
  }
  else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z2_3))
  {
    ret = FELD6;
  }
  //Zeile 3
  else if(delay_buttonpush(&TASTEN_Z3, TASTE_Z3_1))
  {
    ret = FELD7;
  }
  else if(delay_buttonpush(&TASTEN_Z3, TASTE_Z3_2))
  {
    ret = FELD8;
  }
  else if(delay_buttonpush(&TASTEN_Z3, TASTE_Z3_3))
  {
    ret = FELD9;
  }
  // Wenn keine Taste gedrückt worden ist
  else
  {
    ret = -1;
  }
  
  return ret;
}

Die Defines

//Die Feldtasten
#define TASTEN_Z12_DDR  DDRB
#define TASTEN_Z12_PORT PORTB
#define TASTEN_Z12 PINB
//Zeile 1
#define TASTE_Z1_1 PB0
#define TASTE_Z1_2 PB1
#define TASTE_Z1_3 PB2
//Zeile 2
#define TASTE_Z2_1 PB3
#define TASTE_Z2_2 PB4
#define TASTE_Z2_3 PB5
//Zeile 3
#define TASTEN_Z3_DDR  DDRC
#define TASTEN_Z3_PORT PORTC
#define TASTEN_Z3 PINC

#define TASTE_Z3_1 PC0
#define TASTE_Z3_2 PC1
#define TASTE_Z3_3 PC2

Und noch die delay Funktion

uint8_t delay_buttonpush(volatile uint8_t *port, uint8_t pin)
{
  if ( ! (*port & (1 << pin)) )
    { 
        _delay_ms(50); 
        if ( *port & (1 << pin) )
        {
            _delay_ms(50); 
            return 1;
        }
    }
    return 0;
}

LG Max

[Stefan B. (stefan)]

Ich würde unbedingt die Entprellung per Software mittels /debounce 
time/ Methode und dem 2*50ms Warten ersetzen. Z.b. durch die bewährten 
Routinen aus der Artikelsammlung mit dem Einlesen der Tasten im 
Timer-Interrupt.

Ich würde bei 8 Tasten alle auf einen Port legen und den komplett nur 
einmal einlesen. Das Rauspfriemeln der gedrückten Taste zum Auswerten 
dann mit Bitmasken. Damit spart man die vielen Aufrufe von 
delay_buttonpush() in getAnyKeyPush()

Du hast allerdings 9 Tasten - scheint eine 3x3 Tastaturmatrix zu sein. 
Da gibt es in der Artikelsammlung auch Routinen dafür. Alternativ die am 
wenigsten benutzte Taste auf einen extra Pin legen und 8+1 abfragen.

[Max (Gast)]

So ich hab es jetzt wie folgt geändert. Ist jetzt so einigermaßen wie 
ich es haben will. Stören sich eigentlich ein Timer Interrupt und die 
_delay_ms()? Weil ich noch eine Multiplexer LED Schaltung hinzufügen 
will wie über Timer Interrupt laufen wird. Und ich will halt nicht das 
die sich gegenseitig erheblich in Quäre kommen...

uint8_t delay_buttonpush(volatile uint8_t *port, uint8_t pin)
{
  if ( ! (*port & (1 << pin)) )
    { 
        _delay_ms(50); 
    _delay_ms(50); 
        if ( *port & (1 << pin) )
        {
            _delay_ms(50); 
      _delay_ms(50); 
            return 1;
        }
    }
    return 0;
}

int8_t getAnyKeyPush(void)
{
  int8_t ret = -1;
  /*Zeile 1 u. Zeile 2 registrieren*/
  if((TASTEN_Z12&0x3F)<0x3F)
  {
    if(delay_buttonpush(&TASTEN_Z12, TASTE_Z1_1))
    {
      ret = FELD1;
    }
    else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z1_2))
    {
      ret = FELD2;
    }
    else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z1_3))
    {
      ret = FELD3;
    }
    else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z2_1))
    {
      ret = FELD4;
    }
    else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z2_2))
    {
      ret = FELD5;
    }
    else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z2_3))
    {
      ret = FELD6;
    }
  }
  else if((TASTEN_Z3&0x7)<0x7)
  {
    if(delay_buttonpush(&TASTEN_Z3, TASTE_Z3_1))
    {
      ret = FELD7;
    }
    else if(delay_buttonpush(&TASTEN_Z3, TASTE_Z3_2))
    {
      ret = FELD8;
    }
    else if(delay_buttonpush(&TASTEN_Z3, TASTE_Z3_3))
    {
      ret = FELD9;
    }
  }
  else
  {
    ret = -1;
  }
  
  return ret;
}

Warum geht die modifizierte getAnyKeyPush() gefühlstechnisch besser? 
Denn das Ausmaskieren macht doch die delay_buttonpush auch schon?! 
Sprich die geht doch erst gar nicht zum _delay_ms(), wenn keine Taste 
gedrückt ist?!

[Max (Gast)]

So ich hab es jetzt doch noch was konsequenter gemacht! ;-)

int8_t getAnyKeyPush(void)
{
  int8_t ret = -1;
  if((TASTEN_Z12&0x7)<0x7) //Zeile 1
  {
    if(delay_buttonpush(&TASTEN_Z12, TASTE_Z1_1))
    {
      ret = FELD1;
    }
    else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z1_2))
    {
      ret = FELD2;
    }
    else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z1_3))
    {
      ret = FELD3;
    }
  }
  else if((TASTEN_Z12&0x38)<0x38) //Zeile 2
  {
    if(delay_buttonpush(&TASTEN_Z12, TASTE_Z2_1))
    {
      ret = FELD4;
    }
    else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z2_2))
    {
      ret = FELD5;
    }
    else if(delay_buttonpush(&TASTEN_Z12, TASTE_Z2_3))
    {
      ret = FELD6;
    }
  }
  else if((TASTEN_Z3&0x7)<0x7)
  {
    if(delay_buttonpush(&TASTEN_Z3, TASTE_Z3_1))
    {
      ret = FELD7;
    }
    else if(delay_buttonpush(&TASTEN_Z3, TASTE_Z3_2))
    {
      ret = FELD8;
    }
    else if(delay_buttonpush(&TASTEN_Z3, TASTE_Z3_3))
    {
      ret = FELD9;
    }
  }
  else
  {
    ret = -1;
  }
  
  return ret;
}

[Stefan B. (stefan)]

Max wrote:

> Stören sich eigentlich ein Timer Interrupt und die
> _delay_ms()?

Sicher doch. Die _delay_ms() wird von den Timer-Interupts unterbrochen 
und braucht länger als von dir angegeben, d.h. sie wird unpräziser.

> Weil ich noch eine Multiplexer LED Schaltung hinzufügen
> will wie über Timer Interrupt laufen wird. Und ich will halt nicht das
> die sich gegenseitig erheblich in Quäre kommen...

Dann lass die das Softwaredebounce weg und polle die Tasten im sowieso 
vorhandenen Timer-Interrupt. Eleganter kannst du das nicht lösen.

> Warum geht die modifizierte getAnyKeyPush() gefühlstechnisch besser?
> Denn das Ausmaskieren macht doch die delay_buttonpush auch schon?!
> Sprich die geht doch erst gar nicht zum _delay_ms(), wenn keine Taste
> gedrückt ist?!

Durch die beiden neuen Zusatzabfragen gehst du nur in die zeitmäßig 
teure delay_buttonpush(), wenn eine Taste gedrückt ist. Vorher gingst du 
immer rein. Der Zeitverbrauch ist die Aufrufzeit der delay_buttonpush().

(Edit: Teil gelöscht. Analyse deiner Tastenabfrage ist mir im Moment zu 
zeitaufwändig)

50 ms ist schon rel. viel Entprellzeit. Zur Info: In BASCOM AVR sind 
beim Befehl DEBOUNCE 25 ms default (einstellbar mit CONFIG DEBOUNCE).

[Max (Gast)]

Ach so also 2 mal 25ms statt 2 mal 50 ?