Problem beim Stellen der Zeit

#3968841
Lesenswert?

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.
1
void set_time(volatile uint32_t *hr, volatile uint32_t* min, volatile uint32_t *sec) {
2
  char temp[8];
3
  int i;
4
  volatile uint32_t *array[] = {hr, min, sec};
5
  uint32_t range[] = {24, 60, 60};
6
  set_led(LED_RED, ON);
7
  
8
  for (i=0; i<3; i++) {
9
    while(!button_enter()) {
10
      if(button_up()) {
11
        (*(array[i]))++;
12
      }
13
      if(button_down()) {
14
        if(*(array[i]) > 0)
15
          (*(array[i]))--;
16
      }
17
      *(array[i]) %= range[i];
18
      sleep(10);
19

20
      sprintf(temp, "%ld:%ld:%ld", *hr, *min, *sec);
21
      lcd_clearline(1);
22
      lcd_setcursor(0, 1);
23
      lcd_string(temp);
24
    }
25
    sleep(100);
26
  }
27
  set_led(LED_RED, OFF);
28
}
29

30

31
void set_current_time() {
32
  cli();
33
  set_time(&hr, &min, &sec);
34
  sei();
35
}
36

37
void set_up_time() {
38
  set_time(&up_hr, &up_min, &up_sec);
39
}
Grüsse,

Christophe
#3968851
Lesenswert?

1
  char temp[8];
2
...
3
      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.
#3968855
Lesenswert?

Karl Heinz schrieb:
>
1
>   char temp[8];
2
> ...
3
>       sprintf(temp, "%ld:%ld:%ld", *hr, *min, *sec);
4
>
>
> 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.
1
  char temp[15];   // nicht kleckern. klotzen! Der Speicher wird nur kurzfristig
2
                   // gebraucht, solange die Funktion läuft. Danach ist
3
                   // er wieder frei.
4
                   // Aber deine Programmstabilität wird es dir danken
5
                   // wenn du nicht gleich bei jeder Debug Ausgabe einen
6
                   // Array Overflow provozierst.
7
...
8
      sprintf(temp, "%02ld:%02ld:%02ld", *hr, *min, *sec);
#3968864
Lesenswert?

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.
#3968870
Lesenswert?

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.
1
void set_time(volatile uint8_t *hr, volatile uint8_t* min, volatile uint8_t *sec) {
2
  char temp[8];
3
  int i;
4
  volatile uint8_t *array[] = {hr, min, sec};
5
  uint8_t range[] = {24, 60, 60};
6
  set_led(LED_RED, ON);
7
  
8
  for (i=0; i<3; i++) {
9
    while(!button_enter()) {
10
      if(button_up()) {
11
        (*(array[i]))++;
12
      }
13
      if(button_down()) {
14
        if(*(array[i]) > 0)
15
          (*(array[i]))--;
16
      }
17
      *(array[i]) %= range[i];
18
      sleep(10);
19

20
      sprintf(temp, "%02d:%02d:%02d", *hr, *min, *sec);
21
      lcd_clearline(1);
22
      lcd_setcursor(0, 1);
23
      lcd_string(temp);
24
    }
25
    sleep(100);
26
  }
27
  set_led(LED_RED, OFF);
28
}
29

30
/* 
31
 * ===  FUNCTION  ======================================================================
32
 *         Name:  set_current_time
33
 *  Description:  set the current time
34
 * =====================================================================================
35
 */
36
void set_current_time() {
37
  cli();
38
  set_time(&hr, &min, &sec);
39
  sei();
40
}
#3968874
Lesenswert?

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 :/

Antwort schreiben

Bitte melde dich an, um einen Beitrag zu schreiben.

oder

Mit Google-Account einloggen

Die Registrierung ist kostenlos und dauert nur eine Minute.

Jetzt registrieren