Problem beim Stellen der Zeit

[Christophe S. (cschmol)]

Hallo,

Ich bin gerade dabei, den uC in meiner Schaltung zu programmieren.
Eine Teilfunktion ist das Stellen der aktuellen Zeit, sowie das Stellen 
einer "Weckzeit".

Die aktuelle Zeit wird per Interrupt Routine aktualisiert.
Ich habe mir dafür eine Hilfsfunktion geschrieben, aber das Stellen der 
aktuellen Zeit funktioniert nicht zuverlässig, manchmal bleibt dann das 
ganze Programm einfach hängen, und ich weiss nicht woran das liegt.
Das Stellen der "Weckzeit" funktioniert jederzeit problemlos.

Vielleicht könnt ihr ja mal drüber schauen, und erkennt, wo der Fehler 
liegt.

void set_time(volatile uint32_t *hr, volatile uint32_t* min, volatile uint32_t *sec) {
  char temp[8];
  int i;
  volatile uint32_t *array[] = {hr, min, sec};
  uint32_t range[] = {24, 60, 60};
  set_led(LED_RED, ON);
  
  for (i=0; i<3; i++) {
    while(!button_enter()) {
      if(button_up()) {
        (*(array[i]))++;
      }
      if(button_down()) {
        if(*(array[i]) > 0)
          (*(array[i]))--;
      }
      *(array[i]) %= range[i];
      sleep(10);

      sprintf(temp, "%ld:%ld:%ld", *hr, *min, *sec);
      lcd_clearline(1);
      lcd_setcursor(0, 1);
      lcd_string(temp);
    }
    sleep(100);
  }
  set_led(LED_RED, OFF);
}


void set_current_time() {
  cli();
  set_time(&hr, &min, &sec);
  sei();
}

void set_up_time() {
  set_time(&up_hr, &up_min, &up_sec);
}

Grüsse,

Christophe

[Karl H. (kbuchegg)]

uint32_t für Stunde, Minute, Sekunde?

Mit Verlaub, aber hast du einen Knall?
Dir ist schon bewusste, dass du deinen µC damit völlig sinnloserweise 
unnötige Mehrarbeit aufbürdest.

[Karl H. (kbuchegg)]

  char temp[8];
...
      sprintf(temp, "%ld:%ld:%ld", *hr, *min, *sec);

das geht sich mit einem 8-er Array nicht aus.
Du hast auf die 0-Terminierung vergessen.

Auf der einen Seite schmeisst du mit den Bytes für Stunde, Minute, 
Sekunde nach dem Muster "Was kostet die Welt" um, auf der anderen Seite 
sparst du hier zu viel und provozierst einen Array Overflow, der dir 
Gott weiss Was in deinem Programm zerstört.

[Karl H. (kbuchegg)]

Karl Heinz schrieb:
>

>   char temp[8];
> ...
>       sprintf(temp, "%ld:%ld:%ld", *hr, *min, *sec);
>

>
> das geht sich mit einem 8-er Array nicht aus.
> Du hast auf die 0-Terminierung vergessen.

Und im Sinne deiner Anwender solltest du dafür Sorge tragen, dass die 
Anzeige bei einem Wechsel von 9 auf 10 (bzw. umgekehrt) nicht dauernd 
hin und her tanzt. Bei Zeiten kommt dir zu Gute, dass die Leute gewohnt 
sind, dass hier normalerweise mit führenden 0-en jede Angabe auf 
2-stellig aufgebohrt wird.

  char temp[15];   // nicht kleckern. klotzen! Der Speicher wird nur kurzfristig
                   // gebraucht, solange die Funktion läuft. Danach ist
                   // er wieder frei.
                   // Aber deine Programmstabilität wird es dir danken
                   // wenn du nicht gleich bei jeder Debug Ausgabe einen
                   // Array Overflow provozierst.
...
      sprintf(temp, "%02ld:%02ld:%02ld", *hr, *min, *sec);

[Karl H. (kbuchegg)]

Karl Heinz schrieb:

> Mit Verlaub, aber hast du einen Knall?

Sorry. Das nehm ich vom Wortlaut her zurück. Aber nicht von der Sache 
der Datentypen. uint32 für einen Wert, dessen Wertebereich von 0 bis 60 
geht. Kopfschüttel.

[Christophe S. (cschmol)]

Hallo,

danke schon mal für die schnellen Antworten.
Ich habe mir eure Bemerkungen zu Herze genommen und umgesetzt.
Ich habe jetzt alles auf uint8_t und "%02d" umgesetzt.

Leider tut das Programm immer noch nicht das was es soll.

Nämlich: Beim Einstellen der Stunden der aktuellen Zeit passiert einfach 
gar nichts, bei Minuten und Sekunden klappt es dann.

[Karl H. (kbuchegg)]

Christophe Privat schrieb:

> Leider tut das Programm immer noch nicht das was es soll.


Hast du das ARray grösser gemacht?

Das ist definitiv ein Fehler!

Du hast 6 Zeichen für die Ziffern.
2 Zeichen für die Doppelpunkte
und 1 Zeichen für die abschliessende String 0-Terminierung.

Macht zusammen minimal eine Array Länge von 9.

[Christophe S. (cschmol)]

ja, ich habe das array jetzt sicherheitshalber auf 20 Zeichen erweitert.

[Karl H. (kbuchegg)]

Christophe Privat schrieb:

> Nämlich: Beim Einstellen der Stunden der aktuellen Zeit passiert einfach
> gar nichts, bei Minuten und Sekunden klappt es dann.


Moment.
Im Eröffnungsposting war das aber noch: mein Programm hängt sich auf.

Wie sieht die Funktion jetzt aus?

[Christophe S. (cschmol)]

jetzt sieht die Funktion so aus.
ja, am anfang bin ich auch aus der Funktion nicht mehr rausgekommen.
Jetzt klappt das schon mal, nur werden die Stunden einfach nicht 
inkrementiert/dekrementiert.

void set_time(volatile uint8_t *hr, volatile uint8_t* min, volatile uint8_t *sec) {
  char temp[8];
  int i;
  volatile uint8_t *array[] = {hr, min, sec};
  uint8_t range[] = {24, 60, 60};
  set_led(LED_RED, ON);
  
  for (i=0; i<3; i++) {
    while(!button_enter()) {
      if(button_up()) {
        (*(array[i]))++;
      }
      if(button_down()) {
        if(*(array[i]) > 0)
          (*(array[i]))--;
      }
      *(array[i]) %= range[i];
      sleep(10);

      sprintf(temp, "%02d:%02d:%02d", *hr, *min, *sec);
      lcd_clearline(1);
      lcd_setcursor(0, 1);
      lcd_string(temp);
    }
    sleep(100);
  }
  set_led(LED_RED, OFF);
}

/* 
 * ===  FUNCTION  ======================================================================
 *         Name:  set_current_time
 *  Description:  set the current time
 * =====================================================================================
 */
void set_current_time() {
  cli();
  set_time(&hr, &min, &sec);
  sei();
}

[Karl H. (kbuchegg)]

Christophe Privat schrieb:

> void set_time(volatile uint8_t *hr, volatile uint8_t* min, volatile 
> uint8_t *sec) {
>   char temp[8];

              ^
              |

<räusper>

[Christophe S. (cschmol)]

Mann mann mann, das kam wieder rein durch meine undo's, sorry.
Mit 20 statt 8 funktioniert jetzt auch alles so wie es soll.

Vielen Dank für eure Hilfe, und sorry für den Fehler.

Jetzt weiss ich auch, wieso die Stunden dann nicht geupdated wurden.

Die Parameter werden ja von hinten nach vorne auf den Stack gepush'd.

Dann hat natürlich der sprintf die Adresse der Stunde überschrieben :/