Ich habe einen Atmega8 in Arbeit.
Leider hängt er sich (ausschließlich) innerhalb folgender Schleife auf:

while( !btn_enter && !btn_menue ){

    if( btn_up && *constant < i ) *constant = *constant + 1;
    if( btn_down && *constant > 0 ) *constant = *constant - 1;

    btn_down = btn_up = 0;

    set_cursor(x, 1);

    // Zeit oder Datum anzeigen?
    if( time ) show_time();
    else {

      show_wkday();
      show_date();

    }



    if( btn_menue ) return;

    _delay_ms(100);

  }

Das passiert rein zufällig.

btn_... wird über einen Interrupt von einem Portexpander gesetzt.
Ansonsten wird nur ein Wert erhöht, bzw. erniedrigt und dann wird er
angezeigt.

Aktiviert sind zudem beide Externe-Interrupts, 3 Timer für SoftPWM etc.
Fällt jemanden ein Grund ein, warum der µC sich an dieser Stelle
aufhängen kann oderliegt es an einem anderen Codeteil?

Leider kann man bei diesem Codefetzen nur raten. Es fehlen die
Definitionen aller Variablen und die Definitionen der aufgerufenen
Funktionen.

> btn_... wird über einen Interrupt von einem Portexpander gesetzt.

Und sind deshalb volatile gekennzeichnet?

Hängt sich die nackische Variante, d,h. keine Funktionsaufrufe auch auf?
Um das zu erkennen, kannst du z.B. vor dem while eine Debug-LED
anschalten und dannach ausschalten. Wenn du ein Oszi oder ein LA hast,
reicht das Schalten eines freien Portpins natürlich auch.

  while( !btn_enter && !btn_menue )
  {
    if( btn_up && *constant < i ) 
      *constant = *constant + 1;

    if( btn_down && *constant > 0 ) 
      *constant = *constant - 1;

    btn_down = 0;
    btn_up = 0;

#if 0
    set_cursor(x, 1);
    // Zeit oder Datum anzeigen?
    if( time ) 
    {
      show_time();
    } 
    else 
    {
      show_wkday();
      show_date();
    }
#endif

    if( !btn_menue ) 
      _delay_ms(100);
  }

> Und sind deshalb volatile gekennzeichnet?
Jup,ich brauche Sie später noch in der main();
Die Variablen sind im übrigen alle int.
*constant zeigt auf ein Element einer Struktur.

Durch das ständige auftreten von Interrupts (habe alle 3 Timer an)
könnte der Programmablauf bei entsprechend langen Interruptroutinen ja
blockiert werden.
Könnte das ein Grund sein. Dann müsste das aber jedes mal passieren und
nicht rein zufällig.
Oft lässt sich der Wert auch ein paar ma ändern und dann hängt der AVR
sich auf.

Könnte es sein, dass constant urplötzlich in den Stack zeigt?
Falls ja, ist Aufhängen vorprogrammiert.

Es wäre ansonsten viel besser, wenn man den kompletten Code sieht.
Du glaubst gar nicht, wie viele Leute schon behaupteten, dass die
anderen Funktionen ihres Programms garantiert fehlerfrei seien. Meist
steckte der Fehler dann in diesen Funktionen.

Btw:
Lange Interrupts sollte es nicht geben. In einem INT wird nur das
nötigste gemacht (kein delay!); idealerweise nur Flags gesetzt, die dann
in der Hauptschleife ausgewertet werden.

> Wie hast du das denn herausgefunden?
Die Funktion ändert einen Wert (z.B. eine Stelle der Uhrzeit). Die
Gesamte Uhrzeit ist in einer Struktur gespeichert, welche dann mittels
show_time() ausgegeben wird. Drück man jetzt also eine Taste, erhöht
sich der Wert und der neue Wert wird angezeigt. Manchmalpassiert aber
gar nix, obwohl der Interrupt ausgelöst wird.
Der rest funktioniert eigendlich so wie er soll, ich poste aber mal
alles nötige:

// ----------------------------------------------------------
// Interrupt für Tastatur
ISR(INT_KEYBRD){

  uint8_t keybrd;
  keybrd = twi_read( IO_ADDR, 0x00 );

  if( keybrd == 0x10 ) btn_menue = 1;
  if( keybrd == 0x08 ) btn_enter = 1;
  if( keybrd == 0x04 ) btn_up = 1;
  if( keybrd == 0x02 ) btn_down = 1;
  if( keybrd == 0x01 ) btn_snooze = 1;

  if( keybrd ) btn_menue_key = 1;

}
// ----------------------------------------------------------

// ----------------------------------------------------------
// AUSSCHNITT AUS main()

// ggf.Menü aufrufen
    if( btn_menue_key == 1 ){

      menue();

      btn_menue_key = 0;
      edit_time = 0;

      lcd_clear();

    }
// -----------------------------------------------------------

// -----------------------------------------------------------
// Funktionen für die Menünavigation
void menue(){

  struct menueP *tmp;
  void (*function)(uint8_t);

  tmp = start;

  while( tmp ){

    // Innerhalb des Menüs
    function = tmp -> func;
    btn_menue = 0;
    btn_enter = 0;
    btn_up = 0;
    btn_down = 0;
    btn_menue_key = 0;

    
    // Anzeigen des Menüpunktes
    lcd_bg(1);            // Displaybeleuchtung an
    lcd_clear();          // Display löschen
    lcd_home();

    lcd_string(tmp -> name);


    while( !btn_menue_key ){

      set_cursor(7,2);
      lcd_string(">");

      // ggf.Funktion aufrufen
      if( btn_enter && *function  ){
        
        lcd_clear();

        (*function)(tmp -> fdata);

        btn_menue_key = 1;
        btn_menue = 0;
        btn_enter = 0;
        btn_up = 0;
        btn_down = 0;        

      }

      // Warten auf Benutzereingabe
      if( btn_menue ) tmp = tmp -> parent;
      if( btn_enter && !function ) tmp = tmp -> child;
      if( btn_up ) tmp = tmp -> prev;
      if( btn_down ) tmp = tmp -> next;

    }

  }

  btn_menue = 0;
  btn_enter = 0;
  btn_up = 0;
  btn_down = 0;

  return;

}
// -----------------------------------------------------------

// -----------------------------------------------------------
// Aufruf der funktion zum Setzen einer Stelle der Uhrzeit (Beispiel)
  m_set_time_constant(&new_timestamp.thours, 2, 4, 1);
// -----------------------------------------------------------

// -----------------------------------------------------------
void m_set_time_constant(int *constant, int i,int x, int time){
  
  //Einstellen einer Zeitkonstante
  while( !btn_enter && !btn_menue ){

    if( btn_up && *constant < i ) *constant = *constant + 1;
    if( btn_down && *constant > 0 ) *constant = *constant - 1;

    btn_down = btn_up = 0;

    set_cursor(x, 1);

    // Zeit oder Datum anzeigen?
    if( time ) show_time();
    else {

      show_wkday();
      show_date();

    }

    if( btn_menue ) return;

    _delay_ms(100);

  }

  btn_enter = 0;

  return;

}
// -----------------------------------------------------------

// -----------------------------------------------------------
// -- Ausgabe des Wochentags
void show_wkday(){

  get_time();        // aktuelle Zeit
  edit_timestamp = timestamp;
  if( edit_time ) edit_timestamp = new_timestamp;

  // Wochentag anzeigen
  switch (edit_timestamp.wkday){

    case 0:
      lcd_string("So.");
      break;
    
    case 1:
      lcd_string("Mo.");
      break;
      
      
    case 2:
      lcd_string("Di.");
      break;
      
    case 3:
      lcd_string("Mi.");
      break;
      
    case 4:
      lcd_string("Do.");;
      break;
      
    case 5:
      lcd_string("Fr.");
      break;  
      
    case 6:
      lcd_string("Sa.");
      break;  

  }

}
// -----------------------------------------------------------

// -----------------------------------------------------------
// -- Ausgabe des Datum
void show_date(){

  // Auslesen der Daten aus dem Uhrenbaustein
  char ASCII[ASCII_L];

  get_time();        // Aktuelle Zeit
  edit_timestamp = timestamp;

  if( edit_time ) edit_timestamp = new_timestamp; 

  lcd_string( itoa(edit_timestamp.tday, ASCII, 10) );
  lcd_string( itoa(edit_timestamp.nday, ASCII, 10) );

lcd_string( "." );  

  lcd_string( itoa(edit_timestamp.tmon, ASCII, 10) );
  lcd_string( itoa(edit_timestamp.nmon, ASCII, 10) );
  lcd_string(".");              


}
// -----------------------------------------------------------

// -----------------------------------------------------------
// -- Ausgeben der Uhrzeit
void show_time(){

  // Auslesen der Daten aus Uhrenbaustein
  char ASCII[ASCII_L];

  if( !flash_time ) get_time();    // Aktuelle Zeit   
  edit_timestamp = timestamp;

  if( edit_time ) edit_timestamp = new_timestamp;

  
  lcd_string( itoa(edit_timestamp.thours, ASCII, 10) );  
  lcd_string( itoa(edit_timestamp.nhours, ASCII, 10) );
  lcd_string(":");              

  lcd_string( itoa(edit_timestamp.tmin, ASCII, 10) );
  lcd_string( itoa(edit_timestamp.nmin, ASCII, 10) );
  lcd_string(":");              

  lcd_string( itoa(edit_timestamp.tsec, ASCII, 10) );  
  lcd_string( itoa(edit_timestamp.nsec, ASCII, 10) );

}
// -----------------------------------------------------------

Die Interrupts (bis auf Tastatur-Interrupt) habe ich jetzt so geändert,
dass sie nur noch Flags setzen.
Wenn ich die Funktionsaufrufe zum Anzeigen auslasse, konnte den Fehler
bisher nicht reproduzieren. Diese Funktionen werden aber auch regülärzum
Anzeigen der Uhrzeit im Hauptprogramm genutzt.Dort gab es damit noch nie
Probleme.

*constant ist doch im Funktionskopf definiert
> [...] (int *constant, [...]
Reicht das nicht, oder habe ich deine Frage falsch verstanden?

Die Struktur für die Zeit ist folgende:

// Zeit-Struktur
struct time_struc{
  int thours;
  int nhours;

  int tmin;
  int nmin;

  int tsec;
  int nsec;

  int tday;
  int nday;

  int tmon;
  int nmon;

  int wkday;

  int year;
};

An *constant wird nur die Adresse eines Elements der Struktur übergeben.

Hannes Eilers schrieb:

> Die Variablen sind im übrigen alle int.

Und alle Zugriffe im gezeigten Code nicht nicht atomar... s.
Interrupt

> char ASCII[ASCII_L];

Ich sehe keine Definition von ASCII_L. Reicht der Platz für das spätere
itoa()?

Diese lokale Variable belastet den Stack. Wenn der Stack bereits eng ist
und eine ISR angesprungen wird, kann die ISR der Todesstoss sein. Ich
würde die versuchsweise global anlegen. Wieviel RAM braucht dein
Programm ungefähr (=> AVR-GCC => avr-size)?

> Und alle Zugriffe im gezeigten Code nicht nicht atomar...
Daran habe ich gar nicht gedacht. Werde den Interrupt während des
Zugriffs auf die Variabeln mal ausschalten.

#define ASCII_L 9

Ich brauche aber ind er Regel weniger Platz. Könnte das also nich
optimieren. Ist noch so hoch, weil ich beim "anlegen" der Def. nicht
wusste,wie viel Platz ich brauche.
Das Programm benötigt inzwischen 86,3% (7072 Byte) des
Programmspeichers.

Die Sache mit den Einzelvariablen für die Buttons finde ich keine so
gute Idee. Die würde ich alle beisammen in einem uint8_t lassen (du bist
überhaupt mit int ziemlich freigiebig!)

Wenn dir zb hier

       btn_menue_key = 1;
        btn_menue = 0;
        btn_enter = 0;
        btn_up = 0;
        btn_down = 0;

ein Interrupt dazwischenknallt, der einzelne btn_xxx Variablen setzt,
dann können, je nachdem wo der Interrupt auftritt, Tastendrücke
verlorengehen.

Hannes Eilers schrieb:

> Das Programm benötigt inzwischen 86,3% (7072 Byte) des
> Programmspeichers.

Das ist die Größe im Flash also .text plus .data (plus
Spezialsektionen)? diese Zahl ist weniger interessant.

Wie groß ist der statische Bedarf an RAM, also .data plus .bss?
RAM-Größe minus statischer Bedarf ist ca. max. Stackgröße. Wenn der
statische Bedarf in Richtung >3/4 des RAM geht, fange ich an, die Sache
kritisch zu beäugen.

Hannes Eilers schrieb:

> Das Programm benötigt inzwischen 86,3% (7072 Byte) des
> Programmspeichers.

Das kriegt man schon noch runter.
Du bist da an einigen gezeigten Stellen ziemlich suboptimal unterwegs.
Du hast zb einige Zahlenausgaben und benutzt dafür jedesmal die Sequenz

  lcd_string( itoa(edit_timestamp.thours, ASCII, 10) );

Warum nicht für Zahlenausgabe eine eigene Funktion machen

void lcd_number( int num )
{
  char buf[10];

  itoa( num, buf, 10 );
  lcd_string( buf );
}

und dann an den verwendenden Stellen

  lcd_number( edit_timestamp.thours );

die Funktion braucht zwar auch etwas Platz, aber dafür sparst du an der
Stelle der Verwendung Code ein. Und da du die Funktion oft verwendest
spart das dann in Summe mehr Code ein als durch die zusätzliche Funktion
aufgebraucht wird.

Aber im Grunde ist die Menge des verwendeten Flash uninteressat. Die
Data Section ist das interessante.

> du bist überhaupt mit int ziemlich freigiebig!
Das stimmt,ich finde esso übersichtlicher.DerDatenspeicher ist
gerademalbei 35% da geht also nich was. Aber man könnte das sicherlich
ändern und optimieren.

>je nachdem wo der Interrupt auftritt, Tastendrücke verlorengehen
Wäre ja nicht so schlimm,man müsste halt nochmal auf die Taste drücken
:-)

Was meinst du denn,könnte es Probleme mit dem Stack und der ASCII_L
geben?

Ich werde mal versuchen den Code weiterb zu optimieren. Vielleicht leigt
es daran. Ich werde aber trotzdem noch etwas Platz für dasProg brauchen,
da noch einige Funktionen fehlen.

Hannes Eilers schrieb:
>> du bist überhaupt mit int ziemlich freigiebig!
> Das stimmt,ich finde esso übersichtlicher.

Was ist da dran übersichtlicher? gar nichts!

struct time_struc{
  uint8_t thours;
  uint8_t nhours;

  uint8_t tmin;
  uint8_t nmin;

  uint8_t tsec;
  uint8_t nsec;

  uint8_t tday;
  uint8_t nday;

  uint8_t tmon;
  uint8_t nmon;

  uint8_t wkday;

  int year;
};

ist genauso übersichtlich wie deine Version. In der verwendenden
Programmierung gibt es keinen Unterschied, ausser das du dir um atomaren
Zugriff auf einzelne Member der Struktur keine Gedanken mehr zu machen
brauchst.

Lass es dir noch einmal eindringlich gesagt sein:
Auf einem 8-Bit µC, wie einem Mega8, ist der Datentyp uint8_t oder
int8_t immer deine erste Wahl!
* das ist der Datentyp mit dem der µC am besten umgehen kann
* alles andere erfordert mehr Aufwand als notwendig
* du hast bei allen anderen Typen immer das Problem, dass du
  Zugriff unter Umständen atomar absichern musst.
* du blockierst unnötigerweise Speicher, der dir an anderer Stelle
  manchmal schmerzlich fehlen kann


(Warum gibt es eigentlich in einem Datum jeweils einen t und einen n
Eintrag? Das erscheint mir nicht wirklich logisch)


>>je nachdem wo der Interrupt auftritt, Tastendrücke verlorengehen
> Wäre ja nicht so schlimm,man müsste halt nochmal auf die Taste drücken
> :-)

Doch das ist schlimm.

>
> Was meinst du denn,könnte es Probleme mit dem Stack und der ASCII_L
> geben?

Bei 35% RAM Verbrauch: unwahrscheinlich.
Da wird wohl irgendwo ein Pointer im Wald stehen.

Hannes Eilers schrieb:

> Ich werde mal versuchen den Code weiterb zu optimieren.

Schlechte Strategie.
Durch optimieren wird es selten besser, solange das Problem nicht 'Out
of memory' lautet.

Hannes Eilers schrieb:
> Die Variablen sind im übrigen alle int.

Damit verschenkst Du schonmal ein Großteil des Speicherplatzes, oder hat
bei Dir die Stunde 65535 Minuten?

Nimm besser uint8_t, wos ausreicht.


Peter

Also ich habe jetzt alle Variabeln angepasst, was mir mächtig Platz
spart.
Hierfür erstmal danke!

>(Warum gibt es eigentlich in einem Datum jeweils einen t und einen n Eintrag? Das
erscheint mir nicht wirklich logisch)

Das t ist für die Zehnerstelle das n für die Einerstelle.Ich brauche die
einzeln um die Dtaen hinterher in den Uhrenbaustein zu schieben, will
die Stellen leider einzeln haben.

Meine Tastur habe ich jetzt wie folgt realisiert:

// ----------------------------------------------------------
// Interrupt für Tastatur
ISR(INT_KEYBRD){

  keybrd = twi_read( IO_ADDR, 0x00 );

}
// ----------------------------------------------------------

In den Funktionen wird jetzt der Hex-Wert von keybrd abgefragt:

void m_set_time_constant(uint8_t *constant,uint8_t i,uint8_t x,uint8_t time){
  
  //Einstellen einer Zeitkonstante

  keybrd = 0x00;

  while( keybrd != 0x10 && keybrd != 0x08 ){

    if( keybrd == 0x04 && *constant < i ){
      *constant = *constant + 1;
      keybrd = 0x00;
    }

    if( keybrd== 0x02 && *constant > 0 ){
      *constant = *constant - 1;
      keybrd = 0x00;
    }

    set_cursor(x, 1);

    // Zeit oder Datum anzeigen?
    if( time ) show_time();
    else {

      show_wkday();
      show_date();

    }

    if( keybrd == 0x10 ) return;

    _delay_ms(100);

  }

  keybrd = 0x00;

  return;

}

Bisher hebt das aber das Problem mit dem Aufhängen nicht auf.

Hannes Eilers schrieb:

>>(Warum gibt es eigentlich in einem Datum jeweils einen t und einen n Eintrag?
Das
> erscheint mir nicht wirklich logisch)
>
> Das t ist für die Zehnerstelle das n für die Einerstelle.Ich brauche die
> einzeln um die Dtaen hinterher in den Uhrenbaustein zu schieben, will
> die Stellen leider einzeln haben.

Das ist jetzt aber nicht dein Ernst, oder?
(Und dann noch für Zahlen die nur von 0 bis 9 laufen können einen
eigenen int :-)

Du zwirbelst dir hier eine Menge 'Ungemach' auf, nur damit du die
Zerlegung einer Zahl in Zehner und Einer direkt beim Verschicken an den
Uhrenbaustein einsparst?

Hast du Zehner und Einer, dann errechnet sich die dadurch dargestellte
Zahl durch:

   Zahl = 10 * Zehner + Einer

und die Umkehrung, die Zerlegung ist

   Zehner = Zahl / 10
   Einer = Zahl - 10 * Zehner;

Problem gelöst und du brauchst beim Erhöhen/Erniedrigen der Uhrezeit,
sowie bei Vergleichen oder sonstigen Rechnereien keine 'Durch die Brust
von hintern ins Auge' Aktionen mit Zehnern und Einern machen.

Einzig bei der Ausgabe auf LCD muss man ein wenig darauf achten, dass
führende Nullen mit ausgegeben werden. Aber da die Ausgabe in einer
einzigen Funktion gesammelt ist, ist auch das kein wirkliches Problem
:-)

Jetzt versteh ich auch, warum da so viele lcd_string( itoa ...
Konstruktionen sind

Hannes Eilers schrieb:

> Bisher hebt das aber das Problem mit dem Aufhängen nicht auf.

Das Problem wird auch nicht in dieser Funktion sein.
Was du siehst, sind die Symptome. Die Ursache kann ganz woanders
stecken.

Hannes Eilers schrieb:
> Bisher hebt das aber das Problem mit dem Aufhängen nicht auf.

Nur Stochern im Nebel bringts eben nicht.
Poste endlich mal was compilierbares als Anhang!

Aber mach erstmal die tausende Leerzeilen weg, ich hab schon Brandblasen
am Zeigefinger vom vielen Scrollen.


Peter

Es sind mehrere Dateien.

Der AVR soll am Ende folgendes können(damit ihr versteht was ich mit dem
Code eig. bezwekcne will):

- Anzeige von Uhrzeit und Datum (ohne Jahr) im Wechsel (4sec.)
- Alarmfunktion
- Ansteuerung von RGB-LEDs über RGB-Werte (inkl-. Helligkeit)
- Anzeige der Daten auf einem LCD
- programmierbarer Alarm (best.Wochentage, Tägl. oder aus)
- Steuerung eines MP3-Player (über Simulation eines Tastendrucks)

Dazu habe ich einen PCF8583 und einen PCA9555 am I2C.
Letzterer übernimmt die Ports für den MP3-Player und ide Tastatur. Beide
sind über Interrupts angeschlossen.

Ich bin euch dankbar für die vielen Tipps, wie man etwas besser oder
einfacher realisieren kann.
Aber jetzt alles zu überarbeiten ist nicht Zweck dieses Forum, das muss
ich schon selber machen *freu, seufz* ;-)
Wenn jemand also einen Fehler findet, der dazu führen könnte, dass sich
der AVR aufhängt wäre ich ihm sehr dankbar.

p.s.: Ja,Wecker wird ander geschrieben :-))

Kurzer Zwischenbericht:

Du arbeitest mit malloc. Damit sind alle Aussagen bezüglich
Speicherverbrauch und den 35% zu Makulatur verkommen. Die Schlacht um
einen möglichen Stacküberlauf ist damit wieder eröffnet.

-> Versuch ohne malloc auszukommen.
Ein Menüsystem muss nicht die Menüs zur Laufzeit zusammenbauen. Das kann
man auch statisch machen.
Und dann stimmen die vom Compiler ausgeworfenen Zahlen bezgl.
Speicherverbrauch wieder und sind aussagekräftig.

in menu.h

>      if( keybrd == 0x08 && *function  ){

       if( keybrd == 0x08 && function  ){

> Das kann man auch statisch machen.
Ich möchte jedoch gerne die Menüstruktur als Liste machen, dann bietet
sich dieses Verfahrne doch an.
Wie würdest du es denn machen, dammit die Menüeinträge als Liste nicht
erst zur Laufzeit angelegt werden? Hab schon drüber nachgedacht sie ins
EEPROM zu legen, aber das ist auch nicht gerade so komfortabel.

Das Menü lässt sich auch prima aufrufen und durchblättern. Nur, wenn ich
die Funktion zum Verändern von Werten aufrufe hackts manchmal. manchmal
funktioniert es aber auch.

Hannes Eilers schrieb:
>> Das kann man auch statisch machen.
> Ich möchte jedoch gerne die Menüstruktur als Liste machen, dann bietet
> sich dieses Verfahrne doch an.

Kannst du doch.
Leg meinetwegen die einzelnen Knoten statisch an und verkette sie zur
Programmlaufzeit.

Aber verzichte auf malloc!

Also z.B. so?:

struct menueP m1, m2, m3, m4;

  // -----------------------------
  new_menueP("01 Zeit - Datum", &m1);
  new_menueP("02 Wecker", &m2);
  new_menueP("03 Farbe", &m3);
  new_menueP("04 Musik", &m4);

  m1.next = m3.prev = &m2;
  m2.next = m4.prev = &m3;
  m3.next = m1.prev = &m4;
  m4.next = m2.prev = &m1;

  start = &m1;
  // -----------------------------

Der Aufruf des ersten Elements funktioneirt auch. Das nächste in der
Liste wird aber irgendwie nicht richtig gefunden, es tauchen dann als
bezeichnung auf dem Display nur Zeichenfragmente auf (da scheint der
Pointer irgendwie nicht richtig zu sein).

Hier

    while( !keybrd ){

      set_cursor(7,2);
      lcd_string(">");

      // ggf.Funktion aufrufen
      if( keybrd == 0x08 && *function  ){

        lcd_clear();

        (*function)(tmp -> fdata);

        keybrd = 0x20;

      }

      // Warten auf Benutzereingabe
      if( keybrd == 0x10 ) tmp = tmp -> parent;
      if( keybrd == 0x08 && !function ) tmp = tmp -> child;
      if( keybrd == 0x04 ) tmp = tmp -> prev;
      if( keybrd == 0x02 ) tmp = tmp -> next;

solltest du unbedingt noch 'function' nachziehen. Sonst kann es dir
passieren, dass du die falsche Funktion aufrufst. Am besten jedoch,
eliminerst du die Variable function und greifst immer über tmp zu
(und bitte gewöhn dir an dieser Stelle die Leerzeichen wieder ab. Der
Pointer und der Member auf den über den Pointer zugegriffen werden soll,
gehören zusammen. Der -> ist Trenner genug.
Ich bin auch versucht zu sagen: Gewöhn dir auch das weiterschreiben in
einer Zeile nach einem if wieder ab. Die abhängige Anweisung kommt
eingerückt eine Zeile tiefer. Dann sieht man auch aus 3m Entfernung, das
da irgendetwas besonderes passiert)

Auch wimmeln mir da ein bischen zuviele HEX-Konstanten rum. Mach dir
dafür ein paar #define und lesbare Namen.

// Funktionen für die Menünavigation
void menue() {

 struct menueP *tmp = start;

  while( tmp ) {

    keybrd = 0x00;

    lcd_bg( 1 );      // Displaybeleuchtung an
    lcd_clear();
    lcd_home();
    lcd_string( tmp->name );

    while( !keybrd ) {
      set_cursor(7,2);
      lcd_string(">");

      if( ( keybrd & KEY_ENTER ) ) {
        if( tmp->function ) {
          lcd_clear();
          (*tmp->function)( tmp->fdata );
          keybrd = 0x20;   // was ist 0x20?
        }
        else
          tmp = tmp->child;
      }

      else if( keybrd & KEY_PARENT )
        tmp = tmp->parent;

      else if( keybrd & KEY_PREV )
        tmp = tmp->prev;

      else if( keybrd & KEY_NEXT )
        tmp = tmp->next;
    }
  }
}

Muss das wirklich sein, dass da bei jedem Durchlauf durch die äussere
while-Schleife das Display gelöscht und neu beschrieben wird? Das wird
ganz schön flackern :-)

Hannes Eilers schrieb:
> Also z.B. so?:

Genau


> Der Aufruf des ersten Elements funktioneirt auch. Das nächste in der
> Liste wird aber irgendwie nicht richtig gefunden, es tauchen dann als
> bezeichnung auf dem Display nur Zeichenfragmente auf (da scheint der
> Pointer irgendwie nicht richtig zu sein).

Da hast du irgendeinen Fehler gemacht.
Allerdings ist der ganze 'start' Pointer suspekt. Den braucht kein
Mensch.
Übergib der menue Funktion die Adresse des ersten Menüknotens und gut
ists.
Du scheinst höllische Angst davor zu haben, Argumente an Funktionen zu
übergeben. Statt dessen hast du dann eine Menge globale Variablen, die
den Sacheverhalt auch nicht besser machen.

OK, habe jetzt alles geändert.
Navigation durch die Menüs funktioniert auch.

.data + .bss +.noinit = 79,6%
.text + .data +.bootloader = 61,1%


Ich habe zwar nciht so viel Ahnung wie viele Daten man maximal haben
sollte, damit der Stack nicht voll ist aber ~80% sind wohl zu viel oder?

[edit]
Achso, lasse ich die Untermenüs weg, sind es statt ~80% nur ~15%.
Ev. könnte man die Hauptnavigation statisch machen und die Unterpunkte
dynamisch generieren, dass kostet mehr Programmzeilen aber erheblich
weniger Platz auf dem Stack oder?
[/edit]

Hannes Eilers schrieb:
>

> struct menueP m1, m2, m3, m4;
> 
>   // -----------------------------
>   new_menueP("01 Zeit - Datum", &m1);
>   new_menueP("02 Wecker", &m2);
>   new_menueP("03 Farbe", &m3);
>   new_menueP("04 Musik", &m4);
> 
>   m1.next = m3.prev = &m2;
>   m2.next = m4.prev = &m3;
>   m3.next = m1.prev = &m4;
>   m4.next = m2.prev = &m1;
> 
>   start = &m1;
>   // -----------------------------
> 

Wozu die Speicherverschwendung mit dem .next und .prev?
Du willst doch immer der Reihe nach zählen, dann zähl doch einfach nur
den Index:

#define MENUES 4
struct menueP m[MENUES];
uint8_t menue_idx = 0;
// ...
  next = menue_idx >= (MENUES - 1) ? menue_idx + 1 : 0;
  prev = menue_idx ? menue_idx - 1 : MENUES - 1;

Peter

Hannes Eilers schrieb:

> Ich habe zwar nciht so viel Ahnung wie viele Daten man maximal haben
> sollte, damit der Stack nicht voll ist aber ~80% sind wohl zu viel oder?

Welcher Prozessor?

80% ist schon eine Menge Holz.

> [edit]
> Achso, lasse ich die Untermenüs weg, sind es statt ~80% nur ~15%.
> Ev. könnte man die Hauptnavigation statisch machen und die Unterpunkte
> dynamisch generieren, dass kostet mehr Programmzeilen aber erheblich
> weniger Platz auf dem Stack oder?
> [/edit]

Den meisten Platz werden die Texte brauchen.
Jeweils ein 17-er char Array für einen Text ist schon ziemlich viel.
Mach da mal einen char* Pointer rein und setze den auf den in der
Erzeugungsroutine auf den konstanten Text den du bekommst. Das sollte
den Speicher schon mal drücken. Wenn das immer noch nicht reicht könnte
man die Texte ins Flash auslagern. Das geht immer noch relativ billig
ohen allzuviele Änderungen im Programm.

Wie hoch bist du jetzt mit dem Speicherverbrauch?

struct menueP {
  const char * name;
  
  void (*func)(uint8_t);  
  uint8_t fdata;

  uint8_t prevIndex;
  uint8_t nextIndex;
  struct menueP* child;
};

struct menuP soundMenu[] = {
   { "MP3-Player",       NULL, 0,   2, 1,  NULL },
   { "Lautstärke",       NULL, 0,   0, 2,  NULL },
   { "Weck-Song",        NULL, 0,   1, 0,  NULL },
};

struct menuP colorMenu[] = {
   { "Farbe setzen",     NULL, 0,   5, 1,  NULL },
   { "auto Farbwechsel", NULL, 0,   0, 2,  NULL },
   { "Weckfarbe",        NULL, 0,   1, 3,  NULL },
   { "auto. WeFarbWechs",NULL, 0,   2, 4,  NULL },
   { "norm. Helligkeit", NULL, 0,   3, 5,  NULL },
   { "Weck-Helligkeit",  NULL, 0,   4, 0,  NULL },
};

struct menuP alarmMenu[] = {
    ...
};

struct menuP timeMenu[] = {
   { "11 Zeit",          m_time, 0, 1, 1,  NULL },
   { "12 Datum",         m_date, 0, 0, 0,  NULL },
};

struct menuP mainMenu[] = {
   { "01 Zeit - Datum",  NULL, 0,   3, 1,  timeMenu  },
   { "02 Wecker",        NULL, 0,   0, 2,  alarmMenu },
   { "03 Farbe",         NULL, 0,   1, 3,  colorMenu },
   { "04 Musik",         NULL, 0,   2, 0,  soundMenu },
}; 

so würde ich das Menüsystem aufbauen. Wenn du den parent noch haben
willst, dann mach in die Menüstruktur noch den Pointer dafür rein und
füll den Parent Pointer dynamisch in der Menü Funktion aus. Wenn in ein
Childmenü gewechselt wird, wird der parent Pointer in diesem Childmenü
auf den Meneüpointer gesetzt, der für den Aufruf gesorgt hat.

Diese Darstellung oben hat auch den Vorteil gegenüber einer dynamischen
Allokierung, dass man einen besseren Überblick über den Menüaufbau hat,
als wie wenn sich der Aufbau in 40 Zeilen Code verbergen.

> Mach da mal einen char* Pointer rein und setze den auf den in der
> Erzeugungsroutine auf den konstanten Text den du bekommst.

Damit komme ich auf 53,2%
µC ist ein ATmega8-16

Fehler tritt aber immer noch auf.

Hannes Eilers schrieb:
>> Mach da mal einen char* Pointer rein und setze den auf den in der
> Erzeugungsroutine auf den konstanten Text den du bekommst.
>
> Damit komme ich auf 53,2%
> µC ist ein ATmega8-16

OK.
Damit sollte man auf der sicheren Seite sein.

> Fehler tritt aber immer noch auf.
Es wäre einfacher, wenn ich hier das Zeug nachstellen könnte :-)

So bleibt nur: Alle Pointer Zugriffe absuchen. Sicherheitshalber immer
auf NULL Pointer testen.

Nachmal zum Fehlerbild.
Was machst du genau nach dem Einschalten, damit man zum Fehler kommt
(und wie macht sich der Fehler bemerkbar?)

Na dann mal ran.(Das wird ein Spaß!) seufz

Solche Abfragen

if( keybrd == 0x04 && *constant < i ){
    **************

sind nicht sehr schlau. Du hast deine Tasten bitcodiert. Jede Taste hat
ihr eigenes Bit. Das kannst du benutzen um defensiv zu programmieren

 if( ( keybrd & 0x04 ) && *constant < i )

Der Unterschied:
Bei deiner Version MUSS keybrd exakt 0x04 sein. Ist irgendein anderes
Bit aus irgendwelchem Grund gesetzt, dann geht deine Version nicht mehr.
Bei meiner Version wird getestet, ob das zur Taste gehörende Bit gesetzt
ist. Diese Version funktioniert also auch dann noch, wenn in keybrd
irgendein anderes Bit zusätzlich gesetzt wurde. Bei den erhöh und
erniedrig-Tasten ist das wahrscheinlich egal, aber die Tasten für
'Aussteigen' können davon profitieren, weil dein Programm dann auch aus
einer 'kleinen' Fehlersituation heraus noch sinnvoll arbeiten kann.

Hannes Eilers schrieb:
> Na dann mal ran.(Das wird ein Spaß!) *seufz*

Nochmal:
Was musst du exakt machen, damit es zum Fehler kommt?
Und wie sieht der Fehler aus?

(und poste nochmal deinen jetzigen Code)

Hannes Eilers schrieb:
> Na dann mal ran.(Das wird ein Spaß!) *seufz*



Na ja.
So schlimm auch wieder nicht.
Du hast ein LCD.
Das kannst du benutzen um dir aus dem Programm heraus, Statusausgaben
machen zu lassen. Wenn dein Programm nicht mehr auf Tastendrücke
reagiert, aber in einer Schleife sein müsste, ist es zb eine gute Idee
innerhalb dieser Schleife sich die keybrd Variable an einer Stelle im
LCD ausgeben zu lassen.
usw. Wenn du nicht weißt was dein Programm tut, dann verpass ihm
Ausgaben, so dass dir das Programm erzählt was es gerade tut :-)

Also der Fehler muss irgendwo in der Schleife liegen.
Außerhalb tritt nie ein Fehler auf.

Wenn ich aber innerhalb einfach mal wild auf die Tasten drücke, bleibt
der AVR stehen und erhöht den Wert nicht mehr. Der Interrupt wird auch
nicht mehr aufgerufen.
verwende ich jetzt aber Statusmeldungen um die Fehlerquelle zu
lokalisieren, hängt er sich nicht mehr auf.

Ich habe daher einmal das delay am Ende auf 250ms gesetzt (ist ja eine
kaum spürbare Zeit) und siehe da: Kein Fehler mehr.
Wenn ich gar kein delay rein mache hängt er sich im übrigen sofort auf.

Auf jeden Fall habe ich es heute geschafft, dass mein Code viel weniger
Platz weg nimmt (jetzt ~53%).

ISR(INT_KEYBRD){

  keybrd = twi_read( IO_ADDR, 0x00 );

Oh oh oh !

Wenn dann im Mainprogramm auch I2C gemacht wird, krachts.
I2C ist quasi ein Device, wie z.B. das LCD. Ein Transfer darf nicht
durch eine anderen unterbrochen werden.

Wenn Du z.B. in Word eine Datei öffnest, kann da auch kein anderer
schreiben. Es würde krachen, daher blockiert der PC Devices und Dateien,
bis der anfordernde Prozess fertig ist.


Man sollte für I2C auch immer ein Timeout aufsetzen (1-10ms). Falls mal
ne Störung erfolgt und der Bus sich verklemmt, würde sonst Dein Program
für immer hängen.


Peter

> Oh oh oh !
Das wird nur vor der Aktivierung der Interrupts aufgerufen, da der
PCA9555 nach dem Start bereits einen Interrupt erzeugt. Er geht nämlich
davon aus, dass an den Pins ein High liegen soll, tut's aber nciht >>
Interrupt.
Dann beschreibe ich ihn, dass Low normal ist udn nur bei high ein
interrupt kommen soll.
Der erste Interrupt verschwindet aber erst, nachdem sich der Status des
Pins ändert oder man ihn einmal ausliest.
Da sollte es also keine Probleme geben.



Habe jetzt einmalmit dem Eintritt in's Menü die Timer ausgeschaltet
(bzw. die Interrupts dafür deaktiviert), dann lässt sich das delay noch
ein wenig nach unten korrigieren.

Hannes Eilers schrieb:
> Habe jetzt einmalmit dem Eintritt in's Menü die Timer ausgeschaltet
> (bzw. die Interrupts dafür deaktiviert), dann lässt sich das delay noch
> ein wenig nach unten korrigieren.

Dann scheine ich ja richtig zu liegen.

Ich weiß zwar nicht, welches Delay Du meinst, aber für ein sicheres
Programmkonzept darf davon nicht die Funktion abhängen.

Ich würde dieses gefährliche Konzept in jedem Fall ändern.
Die Verwendung einer nicht reentranten Funktion in Main und Interrupt
ist pfui-bäh und wird Dir auch immer wieder auf die Füße fallen.

Setze einfach im Interrupt ein Bit und im Main wird dieses getestet und
dann an sicherer Stelle das I2C aufgerufen.
Tastendrücke sind nichts eiliges, da ist kein Interrupt nötig.


Peter

> Die Verwendung einer nicht reentranten Funktion in Main und Interrupt ist >
pfui-bäh
Da muss ich zustimmen. Ich habe das jetzt geändert, was den Programmcode
etwas größer macht, da ja jede Funktion die laufend auf Tastatureingaben
wartet eine Zeile mehr braucht, aber dafür läuft jetzt alles stabil und
ohne Fehler (auch ohne delays).

Manchmal vergisst man eben diese elementaren (wichtigen!) Dinge.
Danke für eure Hilfe.