Denkfehler / Menü-Programmierung - bitte helft mir :)

[pete (Gast)]

Hallo Jungs,

ich habe langsam irgendwie eine Denkblockade und komme nicht weiter.
Ich programmiere auf dem ESP32 in Arduino einen LED-Tisch.
Mein bisheriges Menü funktioniert zwar, besteht aber aus gefühlt 236000 
if then / else - Bedingungen. Das wollte ich jetzt etwas besser machen 
und habe hier viel im Forum gelesen und versucht, mein eigenes Menü zu 
bauen. Das klappt aber nicht so ganz. Ich komme mit dem aktuellen 
Menü-Punkt immer irgendwo raus, nur nicht da, wo ich eigentlich hin 
soll.

Mein Code hat schon einige tausend Zeilen, deshalb hier nur der wichtige 
Teil.
Die "printxxx"-Funktionen sind nur Dummys für die späteren Programme.
Die Funktionen sind deklariert und der Funktionsaufruf funktioniert 
auch.
Es werden halt nur die falschen Funktionen und Untermenüs aufgerufen...

Die Idee war zu schauen, ob ein Untermenü hinterlegt ist 
(FunktionPointer xxx.Menue) oder eine Programm hinterlegt ist 
(FunktionPointer xxx.Funktion).
Ich muss halt immer wieder in der loop() starten, da noch viele andere 
Dinge dort abgearbeitet werden (Fernsteuerung, Tasten auslesen, 
I2C-Kommunikation, Webserver, später OTA, ...).

Habe schon viel rumprobiert, aber ich sehe mitlerweile den Wald 
wahrscheinlich nicht mehr...
Wo liegt denn mein blöder Denkfehler?

Viele Grüße!
pete

//defines.h

#define ARRAY_SIZE(x)  (sizeof(x)/sizeof(*x))

typedef void (*FunktionPointer) (void);         //Zeiger auf Funktionen deklarieren

struct  MenueAufbau { uint8_t           ProgrammEbene;//Ebene des ProgrammEintrages im Menü
                      char              Text[50];     //Bezeichnung
                      FunktionPointer   Funktion;     //Funktionszeiger auf Programm
                      FunktionPointer   Menue;        //Funktionszeiger auf Untermenü
                      int16_t           FPS;          //FPS als Abspielgeschwindigkeit
                    };


const MenueAufbau Menue_Animationen[] =
{ 
  {1, "Farb-Fader",    Print2, NULL, 50},
  {1, "moving Qubes",  Print3, NULL, 50},
  {1, "Snakelines",    Print4, NULL, 50},
  {1, "Matrix",        NULL,   NULL, 50},
  {1, "Knight Rider",  NULL,   NULL, 50},
  {1, "Lemming",       NULL,   NULL, 50},
  {1, "Zurück",        NULL,   NULL,   }
};

const MenueAufbau Menue_Spiele[] =
{ 
  {1, "Slider",   Print5, NULL, 50},
  {1, "Spiel2",   Print6, NULL, 50},
  {1, "Spiel3",   NULL,   NULL, 50},
  {1, "Spiel4",   NULL,   NULL, 50},
  {1, "Spiel5",   NULL,   NULL, 50},
  {1, "Zurück",   NULL,   NULL,  }
};


const MenueAufbau Menue_Ledplayer[] =
{ 
  {1, "Bluetooth",           Print7,  NULL, 50},
  {1, "Spektrum Analyzer",   Print8,  NULL, 30},
  {1, "LED-Player - ext1",   Print9,  NULL, 50},
  {1, "ext2 - frei",         NULL,    NULL, 50},
  {1, "Zurück",              NULL,    NULL,   }
};


const MenueAufbau Menue_Einstellungen[] =   
{ 
  {1, "Helligkeit",               Print10,  NULL, 50},
  {1, "Textfarbe",                Print11,  NULL, 50},
  {1, "Textgeschwindigkeit",      Print12,  NULL, 50},
  {1, "weitere Einstellungen",    weitereE, NULL, 50},
  {1, "Animation bei Start",      NULL,     NULL, 50},
  {1, "Bluetooth Einstellungen",  NULL,     NULL, 50},
  {1, "Einstellungen Speichern",  NULL,     NULL, 50},
  {1, "Werkseinstellungen",       NULL,     NULL, 50},
  {1, "Zurück",                   NULL,     NULL,   }
};

const MenueAufbau Menue_weitereE[] =   
{ 
  {2, "weitere1",               Print13, NULL, 50},
  {2, "weitere2",               Print14, NULL, 50},
  {2, "weitere3",               Print15, NULL, 50}
};

const MenueAufbau Menue_Haupt[] =
{   
  {0, "alle abspielen",  Print1,  NULL,          50},
  {0, "Animationen",     NULL,    Animationen,   50},
  {0, "Spiele",          NULL,    Spiele,        50},
  {0, "LED-Player",      NULL,    Ledplayer,     50},
  {0, "Einstellungen",   NULL,    Einstellungen, 50}
};

//led-table.ino
#include "defines.h"

volatile uint32_t millis_old=0;                        //für speicherung vergangene Zeit seit letztem Frame
volatile uint16_t fps=0, periode_millis=0;
int8_t MenueCounter[4] = {-1,-1,-1,-1};       //Bisher Ebenen 0(Hauptmenü), 1(Untermenü1), 2(Untermenü2), 3(Untermenü3)
int8_t MenueCounter_old[4] = {-1,-1,-1,-1};   // -1=nicht aktiv
uint16_t MenueCounterSumme = 0, MenueCounterSumme_old = 0;
uint8_t Prog_gestartet=0, Prog_gestartet_old=0; 
uint8_t aktuelle_MenueEbene=0;  
uint16_t Text_durchlaeufe_main=0;
int16_t menuedurchlauf = 0;

uint16_t Tasten_unten=0, Tasten_oben=0;
uint8_t Tasten_spezial=0, Tasten_old=0;


MenueAufbau aktuelles_Menue;


void setup() {
//nix wichtiges
}

void loop() {

//viel anderer Kram

if ( (millis()-millis_old) > periode_millis ) {
    
        millis_old = millis();
        menuedurchlauf = 0;

        MenueCounterSumme = 0;
        MenueCounterSumme_old = 0;
        for (uint8_t i=0; i<4; i++) {
          MenueCounterSumme += MenueCounter[i]<<i;                        //entspricht 0 oder 1 *2 hoch i
          MenueCounterSumme_old += MenueCounter_old[i]<<i;
        } 

        if( (Tasten_spezial==12) && (Prog_gestartet) ) {                  // Tasten "Enter" + "abbruch" bei laufendem Programm zusammen gedrückt?
          for (uint8_t i=0; i<202; i++) {
            delay(10);
            Tasten_holen(Tasten_unten, Tasten_oben, Tasten_spezial);
            if (Tasten_spezial != 12) break;
            if (i>=200) Prog_gestartet = 0;
          }
        } else if(Prog_gestartet_old < Prog_gestartet) {                  //Programm gestartet
          Prog_gestartet_old = Prog_gestartet;
          fps = aktuelles_Menue.FPS;
          periode_millis = 1000 / fps;
        } else if(Prog_gestartet_old > Prog_gestartet) {                  //Programm beendet
          Prog_gestartet_old = Prog_gestartet;
          fps = 50;
          periode_millis = 1000 / fps;
        } else if(MenueCounterSumme_old != MenueCounterSumme) {           //Menüauswahl wurde geändert
          for (uint8_t i=0; i<4; i++) {
            MenueCounter_old[i] = MenueCounter[i];
          }
          Text_durchlaeufe_main = 0;
          Display_loeschen(led);
          strcpy(Lauftext, aktuelles_Menue.Text);
          LED_aussen_Zeilen(led, max_helligkeit, MenueCounter[0], MenueCounter[1], MenueCounter[2], MenueCounter[3]);
        } else if( (Prog_gestartet) && (aktuelles_Menue.Funktion) ) {           //Programm soll laufen, also aufrufen
            aktuelles_Menue.Funktion();
        } else {
          HandleMenue( ARRAY_SIZE( Menue_Haupt ), Menue_Haupt );
          Text_durchlaeufe_main=text_slide(led, max_helligkeit, &slide_speed_ueb, Lauftext, Text_Farbe, &offset_Textende);  
          if (Text_durchlaeufe_main>=1000) {
            Text_durchlaeufe_main=0;
          }
        }
}


void HandleMenue(uint8_t MenueSize, const MenueAufbau Menue[]) {        //Funktion für Menü mit mehreren Ebenen

    for (int8_t i=(ARRAY_SIZE(MenueCounter)-1); i>=0; i--) {    //aktuelle Menüebene rausfinden
      if (MenueCounter[i] > -1) {
        aktuelle_MenueEbene=i;
        break;                  //wenn aktuelle Ebene gefunden wurde hier abbrechen
      }
    }

    if( Tasten_old != Tasten_spezial ) {

      Tasten_old = Tasten_spezial;
      
      if( (Tasten_spezial==1) && (!Prog_gestartet) ) {                    // Taste "weiter" gedrückt ?
        if( MenueCounter[aktuelle_MenueEbene] < (MenueSize - 1) ) {       // geht da überhaupt was?
          MenueCounter[aktuelle_MenueEbene]++;
        }
      } else if( (Tasten_spezial==2) && (!Prog_gestartet) ) {             // Taste "zurück" gedrückt?
        if( MenueCounter[aktuelle_MenueEbene] > 0 ) {                     // geht das überhaupt
          MenueCounter[aktuelle_MenueEbene]--;
        }
      } else if( (Tasten_spezial==4) && (!Prog_gestartet) ) {             // Taste "Enter" gedrückt?
        if( Menue[MenueCounter[aktuelle_MenueEbene]].Funktion ) {         //gibt es ein hinterlegtes Programm
          Prog_gestartet = 1;
        } else if ( Menue[MenueCounter[aktuelle_MenueEbene]].Menue ){     //gibt es ein Untermenü?
          MenueCounter[aktuelle_MenueEbene+1] = 0;                        //dann Untermenü starten
        } else {
          MenueCounter[aktuelle_MenueEbene+1] = -1;                       //sonst "Exit"
          //????MenueCounter[aktuelle_MenueEbene] = -1;                       //sonst "Exit"
        }
      } else if( (Tasten_spezial==8) && (!Prog_gestartet) ) {             // Taste "Abbruch" gedrückt?
          if (aktuelle_MenueEbene != 0) {                                 //Untermenü 0 setzen, bei Hauptmenü natürlich nicht
            MenueCounter[aktuelle_MenueEbene] = -1;
          }
      }
    }

    aktuelles_Menue.ProgrammEbene = Menue[MenueCounter[aktuelle_MenueEbene]].ProgrammEbene;           //aktuelles Menü speichern
    strcpy (aktuelles_Menue.Text, Menue[MenueCounter[aktuelle_MenueEbene]].Text);
    aktuelles_Menue.Funktion = Menue[MenueCounter[aktuelle_MenueEbene]].Funktion;
    aktuelles_Menue.Menue = Menue[MenueCounter[aktuelle_MenueEbene]].Menue;
    aktuelles_Menue.FPS = Menue[MenueCounter[aktuelle_MenueEbene]].FPS;
  
} //Ende void HandleMenue(uint8_t MenueSize, const MenueAufbau Menue[])

void Animationen()  {
  HandleMenue( ARRAY_SIZE( Menue_Animationen ), Menue_Animationen );
}

void Spiele()  {
  HandleMenue( ARRAY_SIZE( Menue_Spiele ), Menue_Spiele );
}


void Ledplayer()  {
  HandleMenue( ARRAY_SIZE( Menue_Ledplayer ), Menue_Ledplayer );
}


void Einstellungen()  {
  HandleMenue( ARRAY_SIZE( Menue_Einstellungen ), Menue_Einstellungen );
}

void weitereE()  {
  HandleMenue( ARRAY_SIZE( Menue_weitereE ), Menue_weitereE );
}

[Pete P. (nomispetrus)]

Ach so,
Meine Anzeige ist ein 10x10 RGB-LED-Array aus WS2812b.
Die Anzeige darauf klappt abgesehen vom Menü-Problem sehr gut.
-pete

[Markus F. (mfro)]

... da hast Du so eine hübsche Variable namens aktuelles_Menu, aus der 
Du allerhand Informationen rausholst.

Bloß leider hast Du dem armen Ding nirgends einen Wert zugewiesen.

[Pete P. (nomispetrus)]

Doch, am Ende von void HandleMenue werden doch die Daten des jeweiligen 
Funktionsaufruf geschrieben, Simi bleiben doch die Daten des letzten 
Aufrufes darin.

Oder hab ich hier schon Mist gemacht?

[Alex G. (dragongamer)]

> FunktionPointer   Menue;        //Funktionszeiger auf Untermenü
Weshalb ist das Untermenü nicht vom Typ "MenueAufbau"?
Dann könntest du eine schöne, hierarchische Struktur aus Menüs aufbauen 
und es sollte nicht mehr möglich sein, sich falsch durch zu hangeln.
Es kann auch nicht schaden "eltern_menue" hinzu zu fügen. Dann weisst du 
für jedes MenueAufbau sofort was die nächsthöhere Ebene ist.

Des weiteren, deine Variablen Namen sind etwas eigenartig...
Du hast sowas wie:
aktuelles_Menue
Menue_Ledplayer
Tasten_spezial
Hast du dir irgendwelche Regeln aufgestellt wann du ein Wort groß 
schreibst und wann nicht?
Würde empfehlen wenn du underscore(unterstrich)-variablen verwendest, 
garkeine Groß-Buchstaben mehr zu nehmen, ausser für Abkürzungen.
Der Unterstrich reichts chon um dem gehirn zu sagen dass es zwei 
getrennte Worte sind. Zusätzliche Groß-und Kleinschreibung ist also mehr 
Information als notwendig und die musst du dir merken.

[Markus F. (mfro)]

Pete P. schrieb:
> Doch, am Ende von void HandleMenue werden doch die Daten des jeweiligen
> Funktionsaufruf geschrieben, Simi bleiben doch die Daten des letzten
> Aufrufes darin.
>
> Oder hab ich hier schon Mist gemacht?

Stimmt, tatsächlich. Aber wieso machst Du das so kompliziert?
Warum schreibst Du da nicht einfach

aktuelles_Menue = Menue[MenueCounter[aktuelle_MenueEbene]];

sondern einen Roman??

[Pete P. (nomispetrus)]

Alex G. schrieb:
>> FunktionPointer   Menue;        //Funktionszeiger auf Untermenü
> Weshalb ist das Untermenü nicht vom Typ "MenueAufbau"?

Ich kann doch nicht den noch nicht fertig definierten struct in sich 
selbst schon wieder benutzen, oder?

Danke für die Tips, werde ich auf jeden Fall beherzigen.

Aber habt ihr meinen Logikfehler finden können?

[Pete P. (nomispetrus)]

Markus F. schrieb:
> Warum schreibst Du da nicht einfachaktuelles_Menue =
> Menue[MenueCounter[aktuelle_MenueEbene]];
>
> sondern einen Roman??

Ich wusste nicht, dass das so einfach geht ;) Danke.

[Alex G. (dragongamer)]

Pete P. schrieb:
> Ich kann doch nicht den noch nicht fertig definierten struct in sich
> selbst schon wieder benutzen, oder?
Das Stichwort lautet "Vorwärtsdeklaration"!
Schreibst einfach:
struct MenueAufbau;
davor.

> Aber habt ihr meinen Logikfehler finden können?
Etwas schwierig bei dem Code durchzublicken...
Auf jeden Fall kann man menüs aber einfacher bauen (wobei ich zugeggeben 
selbst sowas noch nie in eienr nicht-objektorientierten Sprache gemacht 
habe)
Auch wenn das äußerst frustrierend ist.. ich würde dir empfehlen den 
Part nochmal von Vorne anzufangen. Immerhin hast du was gelernt!

[Stromverdichter (Gast)]

Hallo Pete,
ich sehe den Fehler auch nicht was wohl an dem vielen Rauschen in deinem 
Code liegt. Das nimmt dir die Sicht. Wie willst du da nach Fehlern 
suchen. solche Codeabschnitte lagere ich daher immer in Funktionen aus 
und benenne sie nach dem, was sie tun. In der Main und den obersten 
Schichten hast du dann nur das Prüfen von States und Funtionsaufrufe und 
keine Berechnungnen die die Übersicht nehmen.

pete schrieb:

 } else if(Prog_gestartet_old < Prog_gestartet) { 
 //Programm gestartet
           Prog_gestartet_old = Prog_gestartet;
           fps = aktuelles_Menue.FPS;
           periode_millis = 1000 / fps;
         } else if(Prog_gestartet_old > Prog_gestartet) { 
 //Programm beendet
           Prog_gestartet_old = Prog_gestartet;
           fps = 50;
           periode_millis = 1000 / fps;

Du blockierst den Programmablauf für die Tastenauswertung, das macht man 
nicht. Du kannst dir auch eine Logik in der main Loop basteln, die das 
Entprellen übernimmt.

  for (uint8_t i=0; i<202; i++) {
            delay(10);
            Tasten_holen(Tasten_unten, Tasten_oben, Tasten_spezial);
            if (Tasten_spezial != 12) break;
            if (i>=200) Prog_gestartet = 0;
          }

Du kannst die Tastenauswertung auch schön abstrahieren.
Die Tasten definierst du dann als enum, dann darfst du schöner prüfen.
Prüfen sieht dann z.B. so aus.

if (Tasten_spezial & (TAST_ENTER | TAST_ABBRUCH))

Du übergibst eine Kopie der Array-Struktur an die Funktion MenuHandler. 
Imho reicht da ein Zeiger. Das spart RAM und macht den Code schneller.

pete schrieb:
> Die Funktionen sind deklariert und der Funktionsaufruf funktioniert
> auch.
> Es werden halt nur die falschen Funktionen und Untermenüs aufgerufen...
ich verstehe nicht richtig, was du damit sagen willst. Funktioniert also 
deine Tastenauswertung nicht? Wieso schickst du dir nicht an den 
kritischen Stellen eine Debug-Ausgabe mit dem aktuellen Zustand deiner 
Tasten und deiner State-Machine und deines jeweiligen Funtions-pointers 
raus?

Du musst deine Code so umstricken, dass er schrittweise durch deinen 
Fehlerfall durchschreitet. Dann schaust du dir zu jedem Schritt deine 
Debug-Ausgabe mit den relevanten Parametern an. Damit grenzt du deinen 
Fehler ein, bis du ihn gefunden hast.

[Pete P. (nomispetrus)]

Die Tasten kommen über I2C Portexpander, und auch das klappt.

Ich habe gerade nochmal genau getestet:
Die oberste Ebene (Ebene 0) funktioniert. Ich kann die einzelnen 
Unterpunkte durchdrücken und print1() startet und beendet wie es soll. 
Der Sprung in die Untermenüs klappt aber nicht, der UntermenüCounter 
wird richtig gesetzt, aber angezeigt wird wieder die oberste Ebene 
(Ebene 0).

Ich habe die ganzen Serialprints aus dem Code oben entfernt, mit denen 
ich versuche zu debuggen. Die verwirren Codefremde eher.

Ich komme halt einfach nicht in die Untermenüs. Obwohl ich schon einige 
Tage versuche, das Ganze zu debuggen. ;(

[Peter D. (peda)]

Das ganze wird erheblich einfacher und übersichtlicher, wenn man die 
Tastenabfrage nicht mit der Mainloop vermanscht.

Z.B. ein Timerinterrupt entprellt und liefert Tastenereignisse an die 
Mainloop. Die Mainloop muß dann nur noch die Ereignisse abholen und 
auswerten.
Drückt man eine Taste, dann liest die Mainloop genau ein Ereignis und 
danach wieder 0, d.h. sie muß sich nichts merken. Es wird je Tastendruck 
nur eine Aktion ausgeführt.

[Jack (Gast)]

Peter D. schrieb:
> Das ganze wird erheblich einfacher und übersichtlicher, wenn man die
> Tastenabfrage nicht mit der Mainloop vermanscht.
>
> Z.B. ein Timerinterrupt entprellt und liefert Tastenereignisse an die
> Mainloop. Die Mainloop muß dann nur noch die Ereignisse abholen und
> auswerten.
> Drückt man eine Taste, dann liest die Mainloop genau ein Ereignis und
> danach wieder 0, d.h. sie muß sich nichts merken. Es wird je Tastendruck
> nur eine Aktion ausgeführt.

Das und eine tabellengetriebene Zustandsmaschine. Die Tabellen sind die 
Menüs, in denen die anzuzeigenden Texte, erlaubte Aktionen und 
zugehörige Zustandsübergänge stehen.

Beispiel: Die Dateien menu.h mit den Tabellen und die Mainloop in main.c 
aus 
http://ww1.microchip.com/downloads/en/DeviceDoc/AVRButterfly_application_rev07.zip

Das muss man nicht genau so machen, aber es zeigt die Idee. Deklarieren, 
statt jeden Fall in eigenem Code abhandeln.

[G. O. (aminox86)]

pete schrieb:
> Habe schon viel rumprobiert, aber ich sehe mitlerweile den Wald
> wahrscheinlich nicht mehr...

Naja, sauber strukturiert sieht anders aus.
Ich habe vor gefühlten ´zig Jahren anlässlich des 1.Wettbewerbs ein 
Grundgerüst für ein beliebig erweiterbares Menüsystem veröffentlicht:
https://www.mikrocontroller.net/articles/Tinykon
Zwar habe ich die Menübeispiele (zu Demonstrationszwecken) damals für 
die Dosbox von Win98 programmiert, aber die grundlegende Menüstruktur 
ist auf beliebige Rechner bzw Hardwaresituationen übertragbar, einfach 
indem die hardwareabhängigen Funktionen geändert werden.

[Zeno (Gast)]

@TO
Warum machst Du das mit if...else?  Für so etwas wurde eigentlich 
switch/case erfunden.

[W.S. (Gast)]

pete schrieb:
> ich habe langsam irgendwie eine Denkblockade und komme nicht weiter.
> Ich programmiere auf dem ESP32 in Arduino einen LED-Tisch.
> Mein bisheriges Menü funktioniert zwar, besteht aber aus gefühlt 236000
> if then / else - Bedingungen. Das wollte ich jetzt etwas besser machen

Na dann mach es besser.. und das fängt damit an, daß du zuerst deinen 
ganzen Spaghetti-Code weglöschst und dann erstmal mit Nachdenken über 
deine Randbedingungen anfängst.

Also:
- wie soll dein Menü denn aussehen? Grafisch und bunt, herausklappbare 
Untermenüs, animiert oder nicht, oder einfach eine oder mehrere 
Textzeilen?

- wie soll dein Menü denn benutzt werden? per Mausklick, per 
Funktionstasten und wenn wie viele? Drei Tasten (rauf runter enter) oder 
fünf Tasten oder sonstwas?

Für die allereinfachste Version (die Dreitasten-Version) machst du das 
am besten mit einem Array aus passenden Struct's. Etwa so:

struct MenuRecord
 { char* Text;
   char  Flags;
   void (*onRauf)(void);      
   void (*onEnter)(void);     
   void (*onRunter)(void);
   unsigned char beiEnter;    
 };

const struct MenuRecord DasMenue[soviel du willst] =
{ und hier dann die diversen Einträge
};
unsigned char MenuePosition;
bool edit;

Du brauchst zum Darstellen nur eine einzige Funktion, die sich an 
MenuePosition orientiert. Diese Variable ist der Index des Eintrages, 
der gerade der fokussierte Eintrag ist.

Mit den Tasten für rauf und runter wandert man im Menü herum. Zum 
Eintreten in ein Untermenü lädt man MenuePosition mit dem Wert in 
beiEnter. Ebenso geht es zum Verlassen des Untermenüs.

Die Flags müssen je ein Bit für "erster" und "letzter" haben, denn in 
jeder Menüseite gibt es einen ersten Eintrag und einen letzten Eintrag. 
Dort kann man die Bewegungen per rauf und runter entweder stoppen oder 
nach Karussell-Art umschlagen.

Wenn dein Menü auch zum Editieren gedacht ist, dann brauchst du edit und 
die Funktionen für onRauf und onRunter. Mit einmal onEnter drücken setzt 
man edit auf true und dann soll die Funktion zum Maneuvrieren nicht im 
Menü herumwandern, sondern die besagten Funktionen onRauf und onRunter 
ausführen, die dann die entsprechenden Editierfunktion ausführen sollen.

Den jeweiligen Text des Menü-Eintrages kannst du gestalten wie du 
willst. z.B. auch mit Sonderzeichen, die du in der Darstell-Funktion 
passend interpretieren kannst.

So.

W.S.