Static int in Interruptroutine - bin ich doof?

[Alex V. (bastel_alex)]

Hallo,
ich habe eben für eine kleine Bastelei ein altes Projekt etwas 
zusammengekürzt und umgeschrieben. Ganz einfach: Wenn eine Klangschwelle 
überschritten wird, soll ein Vibro angehen.

code:

/*************************************************************************

   Hardware
   
   prozessor:   ATtiny13A
   clock:      9.6Mhz internal RC oscillator

   PIN1   RESET            
   PIN2   PORTB3/ADC3      
   PIN3   PORTB4/ADC2      Vibro Out
   PIN4   GND

   PIN5   PORTB0/OC10      
   PIN6   PORTB1       
   PIN7   PORTB2/ADC1      Mic in
   PIN8   VCC

*************************************************************************/
#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdlib.h>
#include <util/delay.h> 

// sampling frequency

#define F_CPU 9.6E6 


static int test = 0;
static int toggle = 0;




void vibro_on()
{
  PORTB |= (1<<PINB4);
}

void vibro_off()
{
  PORTB &= ~(1<<PINB4);
}

/*************************************************************************

   adc interrupt

*************************************************************************/

SIGNAL (TIM0_OVF_vect)
{
   uint16_t x;

   // read ADC
   x =   ADCL;
   x += (ADCH << 8);
   x=x>>2; // ADC result with 8 bit resolution

  if (x>50)
  {
    test=1;
  }  
  
  

}
/*************************************************************************

   main

*************************************************************************/
int main(void)
{
  //************** init Timer ************************************

   TCCR0B=(1<<CS01) | (1<<CS00) ; // Prescaling = 64, 

   // interrupt mask register: enable timer1 overflow 
   TIMSK0 |= (1 << TOIE0);
   sei();

   // Ports
   DDRB=(1<<PINB4); // Pin 3 as output ( PB4, vibro)   
   

   // ADC_Clk=F_CPU/128 ==> fs~75kHz, free runing mode
   ADCSRA = (1<<ADEN) | (1<<ADATE) |(1<<ADPS2) | (1<<ADPS1) | (1<<ADPS0);

   // VCC as reference and ADC1 as input
   ADMUX=(0<<REFS0) | (0<<MUX1) | (1<<MUX0);

   //timer0 overflow as adc trigger
   ADCSRB |= (1 << ADTS2 );

   // clear interrupt flag and start first conversion
    ADCSRA |= (1 << ADIF) | (1<<ADSC);
  
  while(1)
   {
        
      if (test==1)
      {
        vibro_on();
        
      }        
    
   }
}

Das Problem (pease help): die Bedingung test==1 wird nie true. Der Code 
ansonsten funktioniert -  wenn ich in der Interrupt-Routine in der 
if(x>50) - Bedingung vibro_on() setze, dann läufts.
Bin ich blöd? Ja. Woran liegts?

[Karl H. (kbuchegg)]

test muss volatile sein

FAQ: Was hat es mit volatile auf sich

[Michael F. (startrekmichi)]

änder mal folgendes:

static volatile int test = 0;

static sagt für globale Variablen, dass sie nur innerhalb dieses 
Source-Files verwendet wird.
volatile sagt, dass sich die Variable ändern kann, ohne dass der 
Compiler das bei der Analyse erkennen kann. Und von Interrupts weiß der 
nix -> er fragt die Variable in der main() wenn überhaupt nur einmal ab.

[Alex V. (bastel_alex)]

Dankesehr, das war natürlich die Lösung! :)
Komisch nur, dass in meinem alten Projekt aus dem ich den code genommen 
habe alles nur mit statics funktioniert hat. Da war der Compiler wohl 
aus irgendeinem Grunde schlau genug!

[Karl H. (kbuchegg)]

Alex v. L. schrieb:
> Dankesehr, das war natürlich die Lösung! :)
> Komisch nur, dass in meinem alten Projekt aus dem ich den code genommen
> habe alles nur mit statics funktioniert hat. Da war der Compiler wohl
> aus irgendeinem Grunde schlau genug!

Wahrscheinlich war in deinem alten Projekt noch mehr in der Schleife. 
Lies dir den Link durch. Die Optimierung hängt logischerweise auch davon 
ab, was sonst noch so in der Schleife los ist, d.h. ob die 
Register-Optimierung überhaupt möglich ist oder ob die Register auch 
noch für etwas anderes gebraucht werden.

Es zeigt sich die alte Regel:
Nur weil etwas zu funktionieren scheint, bedeutet das noch lange nicht, 
dass es auch richtig ist.
Durch Testen kann man nur die Anwesenheit von Fehlern nachweisen, nicht 
jedoch die Abwesenheit.

[Karl H. (kbuchegg)]

Im übrigen solltest du um Datentypen wie int einen Bogen machen, wenn du 
nicht wirklich 16 Bit brauchst.
Solange du mit einem Zahlenbereich 0..255 auskommst, solltest du 
unbedingt uint8_t nehmen.

In deinem

int test;

steckt nämlich noch eine 2-te Falle. Gut, bei dir in deinem Programm 
jetzt nicht, aber grundsätzlich schon. Auf einen int kann der Prozessor 
nicht atomar zugreifen (in einem Rutsch) sondern er muss das auf 2 mal 
machen. Wenn jetzt genau zwischen den beiden Zugriffen die ISR ausgelöst 
wird und innerhalb der ISR die Variable verändert wird, dann hat der 
unterbrochene Code unter Umständen ein Problem: er hat einen int 
gelesen, dessen beiden Bytes von verschiedenen Werten stammen. Und das 
kann sich dann auch so auswirken, dass in dieser Kombination Werte 
entstehen, die eigentlich ungültig sind.

Anhand des Dezimalsystems erklärt:
In deiner Variablen können nur Werte von 0 bis 100 stehen (inklusive). 
Und jetzt nehmen wir mal an, der µC müsste die Tasuender und Hunderter 
getrennt von den Zehnern und Einern einlesen (also es sind 2 
Lesezugriffe notwendig). Das Programm liest also die 1 für 100. Und noch 
ehe es den Rest lesen kann, kommt die ISR zum Zug und ändert den Wert 
auf 99. Wird danach der 2.te Lesezugriff gemacht, dann liest dein 
Hauptprogramm natürlich für Zehner und Einer die 99 aus. Die 1 aus den 
Hundertern hatte es ja schon vor der Unterbrechung gelesen. 
Zusammengenommen hat also dein Programm den Wert .... 199 aus der 
Variablen ausgelesen. Und das war nicht im Sinne des Erfinders.

Daher: Immer den kleinsten Datentyp nehmen der passt. Und wenn geht 
einen uint8_t. Denn der ist 1 Byte groß und kann in einem Rutsch aus dem 
Speicher gelesen werden. Und somit gibt es da auch kein Problem mit dem 
atomaren Zugriff. Denn 1 Byte lesen ist trivialerweise atomar.

[Alex V. (bastel_alex)]

Danke für die Anmerkung und deine Zeit! Ich versuche sie in Zukunft zu 
beherzigen!