uint32_t Problem

[Markus Wien (Gast)]

Hallo zusammen!
Zunächst einmal vielen Dank für dieses Forum. Konnte in den letzten 
Jahre immer wieder auf jede Menge nützliche Tips zurückgreifen und viel 
lernen.
Jetzt komme ich aber nicht mehr weiter. Ich möchte gerne eine Art "Uhr" 
in den Controller einbauen. Jede Millisekunde wird die ISR aufgerufen 
und inkrementiert die Variable __Time_ms um 1. In der 
Haupt-While-Schleife wird überprüft, ob 500ms vergangen sind. Ist dem 
so, dann wird eine LED aktiviert bzw. deaktiviert.

Jedoch blinkt die LED nicht regelmäßig. Ab und zu sind es nicht die 
veranschlagten 500ms, sondern weniger, bis sich der Status der LED 
ändert. Hab ich was offensichtliches übersehen?

Anbei der Code:

/****************/
/* Bibliotheken */
/****************/

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include "clock.h"
#include "led.h"


/****************/
/*  Prototypen  */
/****************/

void initAll (void);



/*********************/
/* Globale Variablen */
/*********************/

volatile uint32_t __Time_ms = 0;



/********************************************/
/*-------PROGRAMMCODE-----------------------*/
/********************************************/

/*****************/
/* MAIN-Funktion */
/*****************/

int main (void)
{
  uint32_t oldTime=0;
  uint8_t ledStatus=0;


  initAll();

  
  while (1)
  {
    if ((oldTime + 500) < CLOCK_SystemTime())
    {
      if (ledStatus)
      {
        LED_OnOff(0);
        ledStatus = 0;
      }
      else
      {
        LED_OnOff(1);
        ledStatus = 1;
      }
      oldTime = CLOCK_SystemTime();
    }    
  }

  return 1;
}



/****************/
/*  Funktionen  */
/****************/

void initAll (void)
{  
  LED_Init();
  
  TIMSK0|=1<<TOIE0);
  TCCR0A |= (1<<CS01) | (1<<CS00); //Set Timer0 prescaler 64

  sei();
}





/***********************************************************/
/*-------------INTERRUPT SERVICE ROUTINEN------------------*/
/***********************************************************/

ISR (TIMER0_OVF_vect)
{
  __Time_ms++;
  TCNT0 = 5;
}

[Johannes M. (johnny-m)]

Erstens ist in obigem Code ein Syntaxfehler drin, weshalb das so sicher 
gar nicht kompiliert. Außerdem hast Du als Anwender keine Bezeichner zu 
vergeben, die mit Unterstrichen beginnen (und erst recht keine, die mit 
zwei Unterstrichen beginnen). Diese Bezeichner sind für das System 
(Compiler und lib) reserviert!

Abgesehen davon ist es für sinnvolle Hilfe essentiell erforderlich, zu 
wissen, um welchen Controller es überhaupt geht. Das gehört in den 
Betreff und steht auch in den Forenregeln, deren Kenntnis auch Du vor 
dem Absenden bestätigen musstest!

[Johannes M. (johnny-m)]

Dass das ungenau wird (wenn es denn überhaupt funktioniert), wundert 
mich übrigens nicht wirklich. Timer-Reload macht man beim AVR einfach 
nicht. Jeder AVR hat mindestens einen Timer, der mindestens eine 
Compare-Einheit besitzt. Und wenn man die im CTC-Modus betreibt, dann 
wird das ganze auch genau.

[Jörg W. (dl8dtl) (Moderator)]

Johannes M. wrote:

> Dass das ungenau wird (wenn es denn überhaupt funktioniert), wundert
> mich übrigens nicht wirklich. Timer-Reload macht man beim AVR einfach
> nicht.

Derartige Codeschnipsel stammen vermutlich noch aus der Zeit vor 10
Jahren, da der AT90S1200 in der Tat noch kein CTC hatte...

Wenn überhaupt, dann sollte der timer reload übrigens die erste
Operation innerhalb der ISR sein.

[Johann L. (gjlayde)]

Markus Wien wrote:
> volatile uint32_t __Time_ms = 0;
>
> ISR (TIMER0_OVF_vect)
> {
>   __Time_ms++;
>   ...
> }

Race condition auf __Time_ms? Wo/wie das gelesen ist, kann man nur 
raten...

Johann

[Peter (Gast)]

@Johann L.
>Race condition
kannst du das etwas genauer erleutern? Denn innerhalb der ISR kann das 
Programm ja nicht uterbrochen werden, wo soll da ein Problem entstehen?

[Johannes M. (johnny-m)]

Peter wrote:
> @Johann L.
>>Race condition
> kannst du das etwas genauer erleutern? Denn innerhalb der ISR kann das
> Programm ja nicht uterbrochen werden, wo soll da ein Problem entstehen?
Das Problem ensteht dann, wenn im Hauptprogramm auf die Variable 
zugegriffen wird und während des Zugriffs ein Interrupt 
dazwischenhaut. Der Zugriff auf eine 32-Bit-Variable ist eine ziemlich 
langwierige Sache.

Ich verstehe auch absolut nicht, wozu Du bei den Zeiten einen 
32-Bit-Integer brauchst.

Die Frage nach dem Controller-Typ ist übrigens immer noch offen...



EDIT:
Sorry, hatte nicht gesehen, dass Du nicht der OP bist...

[(prx) A. K. (prx)]

Johannes M. wrote:

> Ich verstehe auch absolut nicht, wozu Du bei den Zeiten einen
> 32-Bit-Integer brauchst.

Wenn man Zeitabhängigkeiten wie beispielsweise Timeouts über eine 
System-Uptime implementiert, dann ist das durchaus sinnvoll.

Man sollte dann allerdings für den Lesezugriff auf diese Variable eine 
eigene kleine Zugriffsfunktion implementieren, die den Zugriff bei 
abgeschalteten Interrupts durchführt.

[Peter (Gast)]

@Johann L.
> Das Problem ensteht dann, wenn im Hauptprogramm auf die Variable
> zugegriffen wird und während des Zugriffs ein Interrupt
> dazwischenhaut. Der Zugriff auf eine 32-Bit-Variable ist eine ziemlich
> langwierige Sache.
Und wo wird im Hauptprogramm auf die Variable zugegriffen? Ich kann 
nicht sehen wo die Variable überhaupt verwendet wird. Also kann es auch 
gut möglich sein das er vor dem zugriff die interups ausschaltet und 
dann wieder einschaltet.

[Karl H. (kbuchegg)]

Peter wrote:
> @Johann L.
>> Das Problem ensteht dann, wenn im Hauptprogramm auf die Variable
>> zugegriffen wird und während des Zugriffs ein Interrupt
>> dazwischenhaut. Der Zugriff auf eine 32-Bit-Variable ist eine ziemlich
>> langwierige Sache.
> Und wo wird im Hauptprogramm auf die Variable zugegriffen? Ich kann
> nicht sehen wo die Variable überhaupt verwendet wird.

Genau deswegen hat es Johann L. ja auch als Frage formuliert.

[Johannes M. (johnny-m)]

A. K. wrote:
>> Ich verstehe auch absolut nicht, wozu Du bei den Zeiten einen
>> 32-Bit-Integer brauchst.
>
> Wenn man Zeitabhängigkeiten wie beispielsweise Timeouts über eine
> System-Uptime implementiert, dann ist das durchaus sinnvoll.
Hast natürlich Recht, ich hatte mir das Programm grad nicht noch mal 
angeschaut...

[Johannes M. (johnny-m)]

Peter wrote:
> @Johann L.
>> Das Problem ensteht dann, wenn im Hauptprogramm auf die Variable
>> zugegriffen wird und während des Zugriffs ein Interrupt
>> dazwischenhaut. Der Zugriff auf eine 32-Bit-Variable ist eine ziemlich
>> langwierige Sache.
> Und wo wird im Hauptprogramm auf die Variable zugegriffen? Ich kann
> nicht sehen wo die Variable überhaupt verwendet wird.
Möglicherweise in der geheimen Funktion "CLOCK_SystemTime()"...

[Markus Wien (Gast)]

Herzlichen Dank für die vielen hilfreichen Tips.
Den Interrupt der Uhr hab ich nun umgestellt (Danke an Jörg und 
Johannes). Macht die Sache etwas leichter und es spart Zeit.
Zugegebenermaßen hab ich das mit dem Code-Beispiel etwas verbockt. Ist 
z.T. aus anderen Files entnommen (daher auch die __Time_ms und die 
"geheimnisvolle" Funktion CLOCK_SystemTime) und zusammengebastelt. Hab 
es danach nicht mehr versucht zu kompilieren (mea culpa).

Den entscheidenden Hinweis gab schließlich Johann mit "race_condition". 
Die Lösung kam dann von Peter, A.K. Johannes und all den anderen.
Hab jetzt vor der Abfrage der SystemZeit den Interrupt deaktiviert und 
siehe da, es läuft. Kein zufälliges Flackern der LED mehr.

Werde das ganze jetzt noch etwas aufhübschen und wieder in den alten 
Code einbetten. Tausend Dank an alle.

Zu guter letzt: Es war ein AT90CAN128-Controller und die Uhr brauche ich 
zur exakten Abfrage von ms-Zeiten, die z.T. größer als 64 Sekunden sind. 
Daher kein uint16.


Anbei noch der Code:

////////////////////////////
/**************************/
/*                        */
/*      TEST              */
/*                        */
/**************************/
////////////////////////////



/****************/
/* Bibliotheken */
/****************/

#include <avr/io.h>
#include <avr/interrupt.h>
#include "led.h"


/****************/
/*  Prototypen  */
/****************/

void initAll (void);
uint32_t CLOCK_SystemTime2(void);


/*********************/
/* Globale Variablen */
/*********************/

volatile uint32_t systemTime_ms;


/********************************************/
/*--------PROGRAMMCODE----------------------*/
/********************************************/

/*****************/
/* MAIN-Funktion */
/*****************/

int main (void)
{

  uint8_t ledStatus=0;
  uint32_t oldTime=0;


  initAll();

  
  while (1)
  {
    if ((oldTime + 400) <= CLOCK_SystemTime2())
    {
      if (ledStatus)
      {
        LED_OnOff(0);
        ledStatus = 0;
      }
      else
      {
        LED_OnOff(1);
        ledStatus = 1;
      }
      oldTime = CLOCK_SystemTime2();
    }
  }

  return 1;
}



/****************/
/*  Funktionen  */
/****************/

void initAll (void)
{  
  LED_Init();  

  
  TCCR0A |= (1<<WGM01);
  TIMSK0 |= (1<<OCIE0A);
  TCCR0A |= (1<<CS01) | (1<<CS00);
  OCR0A = 250;

  systemTime_ms = 0;

  sei();
}

uint32_t CLOCK_SystemTime2(void)
{
  uint32_t buffer;
  cli();
  buffer = systemTime_ms;
  sei();
  return buffer;
}




/**************************************************/
/*----------INTERRUPT SERVICE ROUTINEN------------*/
/**************************************************/

ISR (TIMER0_COMP_vect)
{
  systemTime_ms++;
}