Menüstruktur mit Pointern und structs

Das erste was du mal tun solltest:
Schmeiss mal alle Casts raus. Eigentlich solltest du
nirgends einen Cast brauchen. Noch steig ich nicht ganz
durch was du da eigentlich geschrieben hast und wie das
alles zusammenhängen soll. Allerdings:

MENU_t *act_menu[];

macht was anderes als das was du wahrscheinlich vorgehabt hast:
act_menu ist ein Array von Pointern auf MENU_t
Ich denke, du möchtest eigentlich haben, dass act_menu
lediglich ein Pointer auf ein MENU_t (oder ein Array von MENU_t)
darstellt.

MENU_t *act_menu;

Ich werd den Code noch weiter durchschauen und versuchen ihn zu
verstehen.

Das hier

>   if( MENU_COUNT > act_menue[MENU_COUNT].Max_Ct) MENU_COUNT =
>      act_menue[MENU_COUNT].Max_Ct;

geht sowieso schon mal nicht. Wenn MENU_COUNT zu gross geworden
ist, dann kannst du das nicht prüfen indem du auf
   act_menue[MENU_COUNT].Max_Ct
zugreifst. MENU_COUNT ist bereits zu gross und damit geht die
Array Indizierung in die Hose.

Du solltest dir mal die Frage stellen, ob dein Strukturaufbau
sinnvoll ist. Zb. Stelle ich dir die Frage: Warum steht
eigentlich bei jedem MENU_ENTRY dabei, wieviele MENU_ENTRY
dieses Menue aufbauen? Das hat doch nichts in einem MENU_ENTRY
verloren (der ja einen Menuepunkt darstellt), sondern ist
eine Eigenschaft des Menues an sich (fuer welches du keine
Strktur hast).

Ich schlage mal folgende Änderung vor:

typedef void (*MenuFnct)(void);

struct MENU;

struct MENU_ENTRY
{
  const unsigned char *Entry_Name;
  MENU                *SubMenu;
  MenuFnct             Function;
};

struct MENU
{
  const unsigned char *Menu_Name;
  int                  NrEntries;
  MENU_ENTRY          *Entries;
};

Damit baust du die Einträge für die SubMenues:

const struct MENU_ENTRY MENU_SET_ENTRIES[] ={
  { set1_str, NULL, func0 },
  { set2_str, NULL, func1 }
};

const struct MENU_ENTRY MENU_PAR_ENTRIES[] ={
  { par1_str, NULL, func2 },
  { par2_str, NULL, func3 }
};

Soweit, so gut.
Jetzt werden die eigentlichen Menues aufgebaut:

const struct MENU MENU_SET = {
    menu_set_str,
    sizeof( MENU_SET_ENTRIES ) / sizeof( *MENU_SET_ENTRIES ),
    MENU_SET_ENTRIES
};

const struct MENU MENU_PAR[] = {
    menu_par_str,
    sizeof( MENU_PAR_ENTRIES ) / sizeof( *MENU_PAR_ENTRIES ),
    MENU_PAR_ENTRIES
};

Auch noch nichts überaschendes. Schau dir auch noch mal genau
an, was wo definiert wird. Jetzt macht das alles Sinn.
In einem Menü ist der Titel nur einmal gespeichert. Ein Menü
weiss wieviele Einträge es hat und es besitzt einen Pointer
zu den Einträgen.
Jeder Eintrag wiederum hat einen anzuzeigenden Text, einen
Pointer auf ein mögliches SubMenü und einen Funktionspointer auf
die Funktion.
Jede Information ist nur einmal angegeben, was ein Indiz dafür
ist, dass die generelle Struktur schon mal nicht so schlecht ist.

Lass uns noch das Hauptmenü aufbauen.
Wieder, wir brauchen die Einträge selbst:

const struct MENU_ENTRY MENU_MAIN_ENTRIES[] ={
  { menu_set_str, MENU_SET, NULL },
  { menu_par_str, MENU_PAR, NULL }
};

Mit diesen Einträgen bauen wieder das eigentliche Menu auf:

const struct MENU MENU_MAIN {
    menu_main_str,
    sizeof( MENU_MAIN_ENTRIES ) / sizeof( *MENU_MAIN_ENTRIES ),
    MENU_MAIN_ENTRIES
};

und zuguter letzt haben wir noch den Pointer
auf das zunächst oberste Menü:

struct MENU *ActMenu = MENU_MAIN;

(Ich schicke das jetzt mal ab, damit der Text nicht verloren geht
falls was passiert, weiter gehts im nächsten Posting)

ActMenu zeigt auf das aktuelle Menu.
Du wirst mal eine Funktion brauchen, die ein Menü ausgibt.
Da ich nicht weiss wo du die Ausgabe machst, gebe ich die
Texte einfach mit printf aus. Das musst du dann an deine
Gegebenheiten anpassen.

Also ein Code der das aktuelle Menu anzeigt:

void ShowMenu( MENU* TheMenu )
{
  // zuerst den Titel ausgeben
  // das ist leicht, der steht in der Menu Struktur selbst
  // drinnen. TheMenu ist ein Pointer auf das gewünschte Menu

  printf( "%s\n", TheMenu->MenuName );

  // Danach kommen alle Einträge dieses Menüs

  for( i = 0; i < TheMenu->NrEntries; ++i )
    printf( "%s\n", TheMenu->Entries[i].EntryName );

  // Das wars dann auch schon. Schau dir genau an, wann ein ->
  // kommt und wann ein .
  // Wenn du das verstanden hast, ist das schon mal die halbe
  // Miete
}

Als nächstes deine Auswertefunktion

int ActEntry;   // Nummer des aktuell ausgewählten
                // Menüpunktes im Aktuellen Menü

void update_menue(void)
{
  if( get_key_press( 1<<KEY_UP )) // Taste + gedrückt?
    ActEntry++;

  if( get_key_press( 1<<KEY_DWN )) // Taste - gedrückt?
    ActEntry--;

  if( ActEntry < 0 )
    ActEntry = 0;

  if( ActEntry >= ActMenu->NrEntries;
    ActEntry = ActMenu->NrEntries - 1;

  if( get_key_press( 1<<KEY_ENT )) { // Taste Enter gedrückt?
    //
    // Den selektierten Menüpunkt mal besser zugreifbar
    // machen. Das vereinfacht im weiteren die Syntax erheblich
    // (und ist auch schneller)
    //
    MENU_ENTRY *SelectedEntry = ActMenu->Entries[ ActEntry ];

    // Hat dieser Eintrag selber ein SubMenu

    if( SelectedEntry->SubMenu != NULL ) {
      ActMenu = SelectedEntry->SubMenu;
      ActEntry = 0;
      ShowMenu( ActMenu );
    }

    // hat er kein SubMenu, bleibt noch der Fall, dass eine
    // Funktion aufzurufen sein koennte

    else if( SelectedEntry->Function != NULL ) {
      SelectedEntry->Function();
    }
  }
}

Dein Menü hat übrigens einen Schönheitsfehler:
Wenn ich mal in einem Sub-Menü drinnen bin, wie komme
ich wieder zurück?

Warnung: Ich habe den Code direkt im Web-Browser geschrieben.
Es ist also möglich (und wahrscheinlich), dass da Tippfehler drinnen
sind.

Da haben wir schon den ersten

> MENU_ENTRY *SelectedEntry = ActMenu->Entries[ ActEntry ];

das muss heissen:

MENU_ENTRY *SelectedEntry = & ActMenu->Entries[ ActEntry ];

Ich denke Karl Heinz meinte nicht, wie du den Benutzer das Menü
zurückspringen lässt, sondern wie der Vorgang innerhalb deiner
Verketteten Liste gehen soll.

->Man könnte die Liste doppelt verketten. Sprich: Ein Pointer zum
Submenü (wie bisher geschehen) und im Submenü wiederum ein Pointer
zum Parent-Menü.

> //typedef void (*MenuFnct)(void);
> struct MENU;
>
> struct MENU_ENTRY
> {
>   const unsigned char *Entry_Name;
>   struct MENU                *SubMenu;
>   void ( *Function )(void);
>   //struct MenuFnct             *Function;
> };
>>> Den Funktionspointer musste ich in die struct verlegen

Ja, ok. Macht keinen Unterschied. Der typedef da oben haette
Tipparbeit gespart wenn dieser Funktionspointertyp öfter
vorkommt.

> struct MENU
 {
>   const unsigned char *Menu_Name;
>   int                  NrEntries;
>   struct MENU_ENTRY          *Entries;
> };
>
> const struct MENU_ENTRY MENU_SET_ENTRIES[] ={
>   { set1_str, NULL, func0 },
>   { set2_str, NULL, func1 }
> };
>
> const struct MENU_ENTRY MENU_PAR_ENTRIES[] ={
>   { par1_str, NULL, func2 },
>   { par2_str, NULL, func3 }
> };
>
> const struct MENU MENU_SET ={
>     menu_set_str,
>   sizeof( MENU_SET_ENTRIES ) / sizeof( *MENU_SET_ENTRIES ),
>   MENU_SET_ENTRIES
> };
>
>>> warning: initialization discards qualifiers from pointer target

Bei solchen Fehlermeldungen dreht es sich meistens um
const / nicht const - Dinge

Schaun wir mal. menu_set_str wirds nicht sein. Also der
Pointer auf MENU_SET_ENTRIES.

MENU_SET_ENTRIES ist definiert als

const struct MENU_ENTRY MENU_SET_ENTRIES[]

also mit const.
In der Struktur ist der entsprechende Pointer deklariert als:

struct MENU_ENTRY          *Entries

also ohne const.
Jetzt steckt der Compiler in der Klemme: Auf der einen Seite
teilst du ihm mit, dass sich MENU_SET_ENTRIES niemals aendern
wird. Daher das const, und das ist auch gut so, wann immer du
versuchst in diesem Array irgendwas zu ändern klopft dir der
Compiler auf die Finger: Das Array ist const, du kannst dort nichts
aendern.
Auf der anderen Seite versuchst du einen Alternativen Zugriffsweg
über den Pointer Entries aufzubauen, der nicht const ist. D.h
über diesen Pointer wäre eine Änderung in diesem Array möglich.

Tja. Und das teilt der Compiler mit.

> const struct MENU MENU_PAR ={
>     menu_par_str,
>     sizeof( MENU_PAR_ENTRIES ) / sizeof( *MENU_PAR_ENTRIES ),
>     MENU_PAR_ENTRIES
> };
>
>>> warning: initialization discards qualifiers from pointer target
> type

Selbiges hier

> const struct MENU_ENTRY MENU_MAIN_ENTRIES[] ={
>   { menu_set_str, MENU_SET, NULL },
>   { menu_par_str, MENU_PAR, NULL }
> };
>
> error: initializer element is not constant
> error: (near initialization for `MENU_MAIN_ENTRIES[0].SubMenu')
> error: initializer element is not constant
> error: (near initialization for `MENU_MAIN_ENTRIES[0]')
> error: initializer element is not constant
> error: (near initialization for `MENU_MAIN_ENTRIES[1].SubMenu')
> error: initializer element is not constant
> error: (near initialization for `MENU_MAIN_ENTRIES[1]')

Ich denke mal, das ist ähnlich gelagert.
const / non const.
Wenn du obiges korregiert hast (falls du es noch nicht erraten
hast, mach einfach den Pointer Entries zu einem Pointer auf
const
   const struct MENU_ENTRY   *Entries;
), denke ich muesste dieser Fehler alleine verschwinden. Wenn nicht
melde dich noch mal.

> const struct MENU MENU_MAIN = {
>     menu_main_str,
>     sizeof( MENU_MAIN_ENTRIES ) / sizeof( *MENU_MAIN_ENTRIES ),
>     MENU_MAIN_ENTRIES
> };
>
> >> warning: initialization discards qualifiers from pointer target
> type

Ja das kennen wir schon.

> //Pointer auf das zunächst oberste Menü:
>
> struct MENU *ActMenu = MENU_MAIN;
>
> >> error: incompatible types in initialization
>   error: initializer element is not constant
>

Ist dasselbe in Grün: MENU_MAIN ist eine const Struktur, also
muss der Pointer auch auf eine const Struktur zeigen:

const struct MENU *ActMenu = MENU_MAIN;


> Ich glaube ich bin langsam code-blind geworden.

Wenn du noch nicht so fit damit bist: lass die const einfach
mal alle weg. const-korrektes arbeiten ist zwar nicht so schwer,
ist aber wie ein Rattenschwanz: Ein Fehler zieht eine ganze
Lawine nach sich.

> //typedef void (*MenuFnct)(void);
> struct MENU;
>
> struct MENU_ENTRY
> {
>   const unsigned char *Entry_Name;
>   struct MENU                *SubMenu;
>   void ( *Function )(void);
>   //struct MenuFnct             *Function;
> };
>>> Den Funktionspointer musste ich in die struct verlegen

Jetzt seh ich's erst. Du hast das deswegen gemacht, weil du
einen Fehlermeldung bekommen hast.

Schau dir den typedef noch mal genau an:

> //typedef void (*MenuFnct)(void);

                  ^
                  |
                hier

MenuFnct ist bereits ein Pointer! Daher ist das hier:

>   //struct MenuFnct             *Function;

die Dekleration eines Pointers auf einen Pointer auf eine Funktion.
Dem die Adresse einer Funktion zuzuweisen klappt klarerweise nicht.

Du brauchst einen Pointer auf eine Funktion. Nun der Datentyp
'MenuFnct' ist bereits als Pointer Datentyp definiert worden.
Daher:

   struct MenuFnct             Function;

                             ^
                             |
                       Hier kein * mehr!

Die Sache mit

sizeof( MENU_SET_ENTRIES ) / sizeof( *MENU_SET_ENTRIES ),

ist dir klar?

Ja, das stimmt so.
Man haette auch schreiben können

  sizeof( MENU_SET_ENTRIES ) / sizeof( MENU_SET_ENTRIES[0] );

das ist der übliche Weg, wie man die Anzahl an Einträgen
in einem Array vom Compiler ermitteln kann.

int ai[10];
int size = sizeof( ai ) / sizeof( ai[0] );

Wichtig ist nur: Dazu muss das Array auch als Array sichtbar
sein. Im folgenden Fall

void foo( int bi[] )
{
  int i = sizeof( bi ) / sizeof( bi[0] );
}

geht das ganz grauslich schief, weil in der Funktion das Array
bereits zu einem Pointer auf das erste Arrayelement degeneriert
ist. sizeof( bi ) ist hier nicht die Bytegröße des Arrays,
sondern die Bytegröße des Pointers!

Hüte dich auch davor so zu schreiben:

int size = sizeof( ai ) / sizeof( int );

Das sieht harmlos aus, aber wenn du je den Datentyp von ai änderst:

long ai[10];

und vergisst die Berechnung anzupassen

int size = sizeof( ai ) / sizeof( int );

dann geht das furchtbar schief :-) Besser ist es, den eigentlichen
Datentyp aus der Berechnung herauszulassen und den vom Array selbst
abzunehmen.

int size = sizeof( ai ) / sizeof( *ai );
oder
int size = sizeof( ai ) / sizeof( ai[0] );

Hallo Daniel,

bin auf der Suche nach einem geeigneten Ansatz für ein Displaymenü über
die obigen Beiträge gestolpert. Leuchtet mir soweit auch ein, nur habe
ich das Problem wenn ich's so realisiere, dass ums verrecken keine
Anzeige auf dem Display erscheint.

update_display() übergibt HelpStr an lcd_puts().

Wie ist den HelpStr deklariert?

Mein lcd_puts() und lcd_putchar() schaut folgendermaßen aus:

void lcd_puts(unsigned char *s)
{
    while (*s)
        lcd_putchar (*s++);
}

void lcd_putchar(unsigned char value)
{
    LCD_RS = LCD_DATA_REG;
    lcd_data_write((value>>4)& 0x0F);
    lcd_data_write(value & 0x0F);
    delay(4);
    return;
}

Übergeb ich nun z.B: lcd_puts("Test"); wird Test auch wunderbar
dargestellt. In der Debugsimulation funzt auch die Menügeschichte mit
Pointerübergabe usw. nur in der realen Schaltung klappts nicht. Eine
Idee?

@Derek

Mach doch bitte mal ein kleines, komplettes Program fertig.
Irgendetwas hast du bei der Code-Übernahme übersehen.

Nein. Da seh ich eigentlich auch nichts.
Eigenartiges Phänomen

PROGMEM legt die Daten im Flash ab. Das spart also kostbaren
Speicher im SRAM.

Daniel hat das so gemacht und hat auch sein Program so angepasst,
dass die entscheidende Funktion das weiss.
Genau aus diesem Grund verwendet er das Hilfsarray und kopiert
mittels strcpy_P zuerst vom Flash in dieses Hilfsarray und
von dort weiter zum LCD.

Derek hat das zunächst noch nicht so gemacht. Eigentlich könnte
er sich das Hilfsarray einsparen. Muss er aber nicht. Da die
Texte nicht im Flash liegen, verwendet er folgerichtig auch
keinen strcpy_P sondern einen normalen strcpy.

Soweit ist das alles richtig.