Forum: Mikrocontroller und Digitale Elektronik Volatile in ISR und main() geht nicht


von Karsten K. (karsten42)


Lesenswert?

Moin Moin liebe Forumsmitglieder.

Mein erster Beitrag und damit schon mal sorry für eine vielleicht 
einfache Frage und herzlichsten Dank an diejenigen die unermüdlich Hilfe 
leisten ( Hut ab! ).

Ich möchte unter anderem ein paar multiplexer ( SN74595 ) ansteuern 
deren daten über ein globales array im Zugriff liegen. Die Multiplexer 
werden mit einem Timer (Timer 0) alle 4ms auf Stand gebracht indem eine 
funktion aufgerufen wird die auf das evtl. aktualisierte array 
zugegreift.
Die Idee ist, irgendwo im code das array zu ändern und das dann durch 
die ISR des Timers abarbeiten zu lassen.

Jetzt möchte ich aber auch das array innerhalb einer anderen ISR ( Timer 
1 ) verändern. Dazu ist dann das globale array auf volatile zu 
definieren da es innerhalb einer ISR und im main() geändert wird.

Mit
1
unsigned char mux[MUX_SIZE];
 global funktioniert das updaten in ISR TM1 tadellos jedoch eine 
änderung im main() bleibt unbeachtet.

Mit
1
volatile unsigned char mux[MUX_SIZE;
 und folgerichtig in mux_out()
1
volatile unsigned char *muxp = mux + MUX_SIZE;
 funktioniert das automatische updaten in main() via TM0, aber nicht 
mehr über ISR TIM1.

Ich bin da echt ratlos....
Hat jemand von euch eine Idee was hier falsch ist??

1000000000000 * 10 ^ 12 Dank für konstruktive Hilfe

Karsten
1
// Defines
2
#define MUX_SIZE   2  // Number of installed MUX ( 74LS595 )
3
4
#define REED1     2  // Multiplexer # for reed relais
5
#define MX_RPAON   1  // Mux OUT for reed PA on
6
#define MX_LPAON   4  // Mux OUT for 
7
8
#define INIT    1  // State for Power max
9
#define PWRMAX    2  // State for Power max
10
11
// Prototypes
12
void init(void);
13
void mux_out(void);
14
15
// Global vars
16
unsigned char mux[MUX_SIZE];
17
int8_t state = INIT;
18
19
20
int
21
main(void)  {
22
23
  init(); // initialize timer / mux ....
24
  sei();
25
26
  while(1)  {
27
28
     if(state == PWRON)  {
29
30
    ....
31
    state = PWRMAX;  // initiate blink of MX_LPAON via TIM1
32
    // at ISR TM1 the changed  state will update the mux array
33
    // at ISR TM0 the mux_out() function is calle to update the
34
    // multiplexer outputs.
35
    ....
36
37
    cli();
38
    mux[2] |= (1 << MX_LPAON);
39
    state = RUN;
40
    sei();
41
    // try atomic. Switch LPAON to off and update state. On next cycle of
42
    // TM1 the blink modeof MX_LPAON is off. With next TM0 the mux_out()
43
      // function will update the multiplexer outputs.
44
      }
45
46
  ....
47
    if(key_press_short(1 << KEY_PAON))  {
48
  ....
49
    mux[REED1] |= (1 << MX_RPAON);
50
    // This update should be active with next TH0 cycle.
51
     }
52
  }
53
54
}
55
56
// ISR called every 4ms
57
ISR(TIM0_COMPA_vect)  {
58
59
  ....
60
  mux_out();
61
}
62
63
64
// ISR called every 100ms
65
ISR(TIM1_COMPA_vect)  {
66
67
  ...
68
  if(state == PWRMAX)
69
    mux[REED1] ^= (1 << MX_LPAON);
70
}
71
72
73
// write mux array bytes bit per bit to multiplexer
74
void
75
mux_out(void)  {
76
77
  unsigned char* muxp = mux + MUX_SIZE;
78
79
  .....
80
}

von Oliver (Gast)


Lesenswert?

volatile ist bei Feldern nicht erforderlich, die optimiert der Compiler 
(noch?) nicht weg.

Das Problem könnte aber daran liegen, daß du regelmässig auf mux[2] 
ausserhalb des Feldes liest und schreibst. Sowas kommt nie gut ;-)

In C beginnt der Feldindex nunmal bei 0.

Oliver

von Stefan B. (stefan) Benutzerseite


Lesenswert?

> #define MUX_SIZE   2  // Number of installed MUX ( 74LS595 )
> unsigned char mux[MUX_SIZE];

> volatile unsigned char *muxp = mux + MUX_SIZE;

Gibt einen Bufferoverflow beim Zugriff auf *muxp.
muxp zeigt auf das unsigned char hinter dem Array mux.

>    mux[2] |= (1 << MX_LPAON);

dito Bufferoverflow.

von Karsten K. (karsten42)


Lesenswert?

Whaaaa! DOPPEL SCHÄM!!!!

Ich hab das aus dem original code falsch übertragen.... Es heisst 
natürlich:
1
#define MUX_SIZE 3

Somit ist das array auch richtig definiert und werte werden richtig 
zugewiesen.
Volatile ist also nicht bei array's notwendig? Gut, dann begreife ich 
aber nicht, warum eine Zuweisung im scope von main() auf das array in 
form von
1
mux[2] = 0xf;
 nicht zu einem update via ISR TM0 call auf mux_out() führt... :-(

Daaaanke für die überaus schnelle und aufmerksamen Antworten!

Karsten

von Peter (Gast)


Lesenswert?

ist es nicht wichtiger state als volatil zu kennzeichen?

von Joachim (Gast)


Lesenswert?

Hallo

Bitte alles zeigen, was mux_out macht!
Bis jetzt wird nur ein Zeiger auf das Ende von mux gesetzt.

Gruß
Joachim

von Karsten K. (karsten42)


Lesenswert?

Hallo Peter,

Die Variable <state> wird in der ISR aber nur gelesen und nicht 
zugewiesen. Daher war ich davon ausgegangen das diese nicht als volatile 
zu definieren ist. Aber du hast recht glaube ich: Denn im AVR-GCC 
tutorial steht, dass auch beim lesen von variablen in einer ISR die im 
main() scope verändert werden diese mit volatile zu definieren sind.

Daaanke für diesen überaus guten Tipp! Ich werde das heute abend gleich 
ausprobieren und das Ergebnis mitteilen.

Gruß
Karsten

von Falk B. (falk)


Lesenswert?

@  Karsten Kankowski (karsten42)

>Volatile ist also nicht bei array's notwendig?

DOCH!!! Auch wenn es OFT ohne geht, ist es NICHT sicher!
Hab ich auf dem GCC schon erlebt, dass erst das volatile das korrekte 
Ergebnis liefert (UART TX Interrupt).

MfG
Falk

von Karsten K. (karsten42)


Lesenswert?

Hey Falk,

Du hast Recht!! nach langem ausprobieren ( was mir absolut widerstrebt ) 
habe ich nun eine Lösung meines Problems gefunden.
Das globale array muss wie folgt definiert werden:
1
volatile unsigned char mux[MUX_SIZE];
In der funktion mux_out() muss der char pointerebenfalls volatile 
definiert werden ( sonst beschwert sich der Compiler ):
1
volatile unsigned char* muxp = mux + MUX_SIZE;

Damit aber nun ein update von mux[] im scope von main() und ISR TIM1 
auch funktioniert, musste ich den Funktionsaufruf aus der ISR TM0 in die 
ISR TM1 legen.
1
ISR(TIM1_COMPA_vect)  {
2
3
  ...
4
  if(state == PWRMAX)
5
    mux[REED1] ^= (1 << MX_LPAON);
6
  ...
7
8
  mux_out();
9
}


Mir ist aber absolut nicht klar warum! Und ich bin darüber ziemlich 
unerfreut, funktioniert doch etwas im Code von dem ich ausgehen kann das 
es noch diverse Seiteneffekte hat. Ich hab dazu ein ungutes Gefühl ( wie 
auf Sand gebaut ).

Vielleicht hat ja jemand aus dem Forum eine plausible Erklärung dazu...


Vielen herzlichen Dank aber schon mal an alle die sich beteildigt haben!

Gruß
Karsten

von (prx) A. K. (prx)


Lesenswert?

Ich bin auf die Schnelle nicht bis ins Detail rein, aber wenn muxp 
sowohl in ISR als auch im Hauptprogramm verwendet wird, dann kann es gut 
sein, dass diese Variable nicht nur auf "volatile" zeigen muss, sondern 
auch selbst "volatile" sein muss, also
  volatile unsigned char *volatile muxp = mux + MUX_SIZE;

Apropos: Direkt hinter das letzte Element zu zeigen ist solange legal, 
wie nur damit verglichen nicht aber darüber zugegriffen wird.

von Karl H. (kbuchegg)


Lesenswert?

A. K. schrieb:
> Ich bin auf die Schnelle nicht bis ins Detail rein, aber wenn muxp
> sowohl in ISR als auch im Hauptprogramm verwendet wird, dann kann es gut
> sein, dass diese Variable nicht nur auf "volatile" zeigen muss, sondern
> auch selbst "volatile" sein muss, also
>   volatile unsigned char *volatile muxp = mux + MUX_SIZE;

in diesem konkreten Fall nicht.
Da mux seine Adresslage nicht ändert und auch MUX_SIZE eine Konstante 
ist, könnte man höchstens argumentieren, dass
1
   volatile unsigned char *const muxp = mux + MUX_SIZE;

korrekt wäre.

Die Sache mit dem Array.
Auch wenn es unwahrscheinlich ist, so sollte man natürlich das Array als 
ganzes volatile machen. Die Tatsache, dass es sich hier um ein Array 
handelt, darf doch keine Rolle spielen! volatile bedeutet ja 
schliesslich: keinerlei Annahmen über den Inhalt einer Speicherzelle 
machen und lokale Optimierungen auf dieser Speicherzelle aussen vor 
lassen. Natürlich ist es unwahrscheinlich, dass der Compiler es schafft 
ein komplettes Array in CPU-Registern zu halten und damit eine 
Optimierung aufzubauen. Aber unwahrscheinlich bedeutet nicht, dass es 
unmöglich ist. Wird in einer Schleife ein einzelnes Element aus dem 
Array benutzt, dann kann der Optimizer sehr wohl zuschlagen und dieses 
Element in einem Register halten.

Das fiese ist ja: ohne volatile kann etwas funktionieren, aber es muss 
nicht. Ob und wie hängt immer von den Details des Optimizers ab. Und dem 
möchte man sich ganz sicher nicht auslieferern.

von Karsten K. (karsten42)


Lesenswert?

Moin Moin,

Ich habe *muxp als volatile definiert weil der compiler korrekter weise 
die Zuweisung von
1
*muxp auf volatile unsignbed char mux[]
bemängelte.

Mir ist soweit klar, dass variablen die in ISR gelesen/beschrieben 
werden auf volatile zu setzen sind. Folgerichtig habe ich das array 
mux[] entsprechend definiert.

Mein Unverständnis liegt darin, dass der scope für Änderungen in mux[] 
nur in main() und in der ISR TIM1 liegt.
Oder klarer:
Rufe ich die funktion mux_out() in der ISR TIM0 auf, so sind Änderungen 
am array mux[] nur im scope main() sichtbar, aber eben nicht in der ISR 
TIM1!

Gruß
Karsten

Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.