Atmega328P Analog-Read (sei()) "friert Controller ein"

[Chris K. (lesso)]

Hallo Forum,

ich habe ein Problem mit meiner Schaltung/meinem Code.
Eigentlich möchte ich von einem Potentiometer analog lesen um die 
Blinkgeschwindigkeit einer LED zu beeinflussen.
Das funktioniert nicht, da die Variable "speed" nie geändert wird.
Nach dem ersten Durchlauf der "while" in der Funktion "loop()" hängt 
sich mein Atmega auf. Wenn ich den Funktionsaufruf von "sei()" entferne, 
läuft der Loop erwartungsgemäß durch.

Der Poti funktioniert einwandfrei (getestet mit Multimeter).
Falls es hilft, kann ich auf ein Foto meiner Schaltung hochladen, aber 
ich vermute den Fehler eher im Code selbst.

Habt ihr eine Idee?

Grüße, Chris

Hier mein Code:

#ifndef F_CPU
#define F_CPU 16000000UL
#endif


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

void loop(void);
int read_analog();

int speed = 0;

int main(void) {
  DDRD = (1<<PORTD0) | (1<<PORTD1) | (1<<PORTD2);
  ADMUX = (1 << REFS0) | (1 << MUX2) | (1 << MUX0);
  ADCSRA = (1 << ADEN ) | (1 << ADIE) | (1 << ADPS0) | (1 << ADPS1) | (1 << ADPS2);
  DIDR0 = (1 << ADC5D);
  read_analog();
  sei(); // <--- problem
  loop();
  return 0;
}

void delay_ms_own(int ms) {
  while(ms > 0) {
    _delay_ms(1);
    ms--;
  }
}

void loop() {
  while (1) {
    PORTD |= (1<<PORTD2 );
    delay_ms_own(100);
    PORTD &= ~(1<<PORTD2);
    delay_ms_own(100);
    PORTD |= (1<<PORTD2 );
  }
}

int read_analog() {
  ADCSRA |= (1 << ADSC);
  return 0;
}

ISR(ADC_vect) {
  speed = ADC;
  read_analog();
}

[Thomas E. (thomase)]

Er stürzt ab, wenn der Interrupt ausgeführt wird. Da die richtige ISR 
vorhanden ist, tippe ich mal auf einen falsch eingestellten Controller.

[S. Landolt (Gast)]

> hängt sich mein Atmega auf
Wie äußert sich das?

> da die Variable "speed" nie geändert wird
Und wie das? Ich sehe nur eine Zuweisung an speed, aber sonst nichts, 
keine Abfrage darauf bzw. Weiterverarbeitung.

[Brummbär (Gast)]

Ich werde trotzdem schonmal "volatile" ins Rennen.

[Hubert G. (hubertg)]

Du vermischt hier Arduino Code mit normalen C-Code.
Im main wird alles nur einmal aufgerufen, da gibt es kein return, dann 
kommt eine for oder while Schleife, je nach belieben.

[Stefan F. (Gast)]

Da wir das gerade erst hatten:

Hast du eventuell beim linken die MCU Angabe vergessen? In diesem Fall 
würde unter anderem die Sprungtabelle für die ISR Routinen fehlen.

[Stefan F. (Gast)]

> Du vermischt hier Arduino Code mit normalen C-Code.

Der Arduino Style ist deutlich zu sehen, aber er hat keine Arduino 
Library eingebunden und keine Arduino Funktion verwendet.

Das "return 0" am Ende der main() Funktion ist fachlich korrekt, wenn 
auch beim avr-gcc ausnahmsweise unnötig.

>> hängt sich mein Atmega auf
> Wie äußert sich das?

Ist das nicht offensichtlich? Er wird wohl an PD2 eine LED angeschlossen 
haben, die aufhört, zu blinken.

[Chris K. (lesso)]

Thomas E. schrieb:
> Er stürzt ab, wenn der Interrupt ausgeführt wird. Da die richtige ISR
> vorhanden ist, tippe ich mal auf einen falsch eingestellten Controller.

Das hilft mit leider nicht weiter. Was genau kann da falsch eingestellt 
sein?

S. Landolt schrieb:
> Wie äußert sich das?

Der Inhalt der "while"-Schleife in der Funktion "loop()" wird nur einmal 
ausgeführt und nicht wie durch die "while(1)" zu erwarten unendlich oft.
Sprich: die LED schaltet sich an, nach 100 Millisekunden aus, nach 
weiteren 100ms wieder an und danach bleibt sie angeschalten.

S. Landolt schrieb:
> Und wie das? Ich sehe nur eine Zuweisung an speed, aber sonst nichts,
> keine Abfrage darauf bzw. Weiterverarbeitung.

Ich würde die Variable "speed" gerne verwenden. Den lesenden Zugriff 
baue ich ein sobald es funktioniert. Man könnte zB. sowas machen:

while (1) {
  PORTD |= (1<<PORTD2 );
  delay_ms_own(speed);
  PORTD &= ~(1<<PORTD2);
  delay_ms_own(speed);
}

Das bringt mir allerdings aktuell nichts, weil sich der Controller wie 
gesagt aufhängt. Dass die Variable derzeit nur geschrieben, aber nicht 
gelesen wird, ist mir klar.

Stefan U. schrieb:
> Da wir das gerade erst hatten:
>
> Hast du eventuell beim linken die MCU Angabe vergessen? In diesem Fall
> würde unter anderem die Sprungtabelle für die ISR Routinen fehlen.

Ich bin ziemlich unerfahren. Kannst du mir sagen wie ich das in Atmel 
Studio 7 erreichen kann?

Stefan U. schrieb:
> Ist das nicht offensichtlich? Er wird wohl an PD2 eine LED angeschlossen
> haben...

Korrekt. An PD2 ist eine LED.

[S. Landolt (Gast)]

> LED ... an, nach 100 Millisekunden aus, nach
> weiteren 100ms wieder an
Das ist doch aussagekräftiger als "friert ein" und "hängt sich auf".

Auch wenn ich mir darauf keinen Reim machen kann; sollte die ADC-ISR 
wirklich, wie von zweien vermutet, in einen Reset münden, so kann doch 
der ADC nicht so lange benötigen.

[Stefan F. (Gast)]

>> Hast du eventuell beim linken die MCU Angabe vergessen?

> Ich bin ziemlich unerfahren. Kannst du mir sagen wie ich das in
> Atmel Studio 7 erreichen kann?

Gar nicht. Das kann Dir nur passieren, wenn du "manuell" auf der 
Kommandozeile compilierst oder ein fehlerhaftes Makefile verwendest. 
Atmel Studio benutzt normalerweise kein Makefile.

Und du hast ganz sicher den richtigen Mikrocontroller-Typ für den 
Compiler eingestellt? Gibt der Compiler irgendwelche Hinweise dazu aus, 
oder gar Warnmeldungen?

Hast du AVCC angeschlossen? Hast du vielleicht Abblock-Kondensatoren 
(nahe zum Mikrocontroller) vergessen?

[Thomas E. (thomase)]

Chris K. schrieb:
> Das hilft mit leider nicht weiter. Was genau kann da falsch eingestellt
> sein?

Hast du das schon kontrolliert?

Wenn du für einen anderen Controller kompilierst, und da wärest du 
wahrlich nicht der erste, wird eine für deinen Controller nicht passende 
Interrupt-Vektor-Tabelle angelegt. Dann springt das Programm sonstwo 
hin, nur nicht in die ADC-ISR.

[Chris K. (lesso)]

Ich habe einen Teil des Codes falsch kopiert:
Der Inhalt der "while" sieht eigentlich so aus:

if(speed) {
  PORTD |= (1<<PORTD2 );
  delay_ms_own(speed);
  PORTD &= ~(1<<PORTD2);
  delay_ms_own(speed);
}

Ich habe das "if" absichtlich nicht mit kopiert, weil ich es für 
bedeutungslos hielt. Der Ausdruck wird sowieso immer "true" sein, war 
meine Annahme.

Nach wiederholtem Programmieren des Controllers blinkt jetzt gar nichts 
mehr.
Wenn ich allerdings die "if" zu

if(1) //always true

ändere und alles andere gleich lasse, funktioniert alles wie erwartet.
Die LED blinkt und durch das Verstellen des Potentiometer schneller oder 
langsamer.

Wieso löst die "if" nicht auf "true" auf wenn ich nur "speed" als 
Condition angebe?

[S. Landolt (Gast)]

Kommt jetzt Brummbär zum Zuge mit dem 'volatile'?

[Chris K. (lesso)]

S. Landolt schrieb:
> Kommt jetzt Brummbär zum Zuge mit dem 'volatile'?

Das war's.

volatile int speed = 0;

löst das Problem.

Kann mir das jemand genauer erläutern?
Ich verstehe nicht wieso hier ein Unterschied entsteht.

[S. Landolt (Gast)]

Sorry, ich kenne es nur vom Hörensagen, kann kein C.

[Sascha W. (sascha-w)]

Chris K. schrieb:
> S. Landolt schrieb:
>> Kommt jetzt Brummbär zum Zuge mit dem 'volatile'?
>
> Das war's.
>

> volatile int speed = 0;
>

> löst das Problem.
>
> Kann mir das jemand genauer erläutern?
> Ich verstehe nicht wieso hier ein Unterschied entsteht.
Ohne volatile bekommt der Code in loop nichts davon mit das der Wert 
in einer ISR geändert wurde. Mit volatile wird der Wert jedesmal von 
der entsprechenden Speicherstelle neu geholt.

Sascha

[Thomas E. (thomase)]

Chris K. schrieb:
> Ich habe einen Teil des Codes falsch kopiert:
> Der Inhalt der "while" sieht eigentlich so aus:

Also haben wir hier die ganze Zeit auf falschem Code rumgeraten?

Kopf - Tisch. Dreimal.

[S. Landolt (Gast)]

> Kopf - Tisch. Dreimal.
Soviel zum Thema "offensichtlich".

[Np R. (samweis)]

Der Compiler weiß nicht, dass speed in der ISR geändert wird, denn er 
kann nicht erkennen, wann oder ob die ISR überhaupt ausgeführt wird.

Für den Compiler steht da also nur:

int speed = 0;
...

if(speed) {
...

Das kann er auch gleich wegoptimieren.

Das "volatile" sagt ihm, dass speed auch außerhalb von main geändert 
werden kann.

[S. Landolt (Gast)]

Schon, aber wie passt das zu:

Chris K. schrieb:
> Wenn ich allerdings die "if" zu
> if(1) //always true
> ändere und alles andere gleich lasse, funktioniert alles wie erwartet.
> Die LED blinkt und durch das Verstellen des Potentiometer schneller oder
> langsamer.

Hier kommt doch speed angeblich irgendwie rüber, ganz ohne volatile.

[Chris K. (lesso)]

Thomas E. schrieb:
> Chris K. schrieb:
>> Ich habe einen Teil des Codes falsch kopiert:
>> Der Inhalt der "while" sieht eigentlich so aus:
>
> Also haben wir hier die ganze Zeit auf falschem Code rumgeraten?
>
> Kopf - Tisch. Dreimal.

S. Landolt schrieb:
>> Kopf - Tisch. Dreimal.
> Soviel zum Thema "offensichtlich".

Entschuldigt. Das war klar fehlendes Wissen meinerseits.
Aber dafür fragt man schließlich in einem Forum.
Auch wenn mein initialer Post das Problem nicht dargestellt hat.

np r. schrieb:
> Der Compiler weiß nicht, dass speed in der ISR geändert wird, denn
> er
> kann nicht erkennen, wann oder ob die ISR überhaupt ausgeführt wird.
>
> Für den Compiler steht da also nur:int speed = 0;
> ...
>
> if(speed) {
> ...
>
> Das kann er auch gleich wegoptimieren.
>
> Das "volatile" sagt ihm, dass speed auch außerhalb von main geändert
> werden kann.


Danke für die Klärung. Das macht natürlich Sinn.

[Np R. (samweis)]

S. Landolt schrieb:
> Schon, aber wie passt das zu:
...

Bei if (speed) kann der Compiler, wenn seiner Ansicht nach speed = 0 
ist, das gesamte if-Statement wegwerfen - es wird ja sowieso nie 
ausgeführt.

Bei if(1) muss er das If-Statement und den Code darin behalten.
Darin wird speed verwendet. Und in der ISR geändert. In diesem Falle: 
Glück gehabt.

[Np R. (samweis)]

Chris K. könnte ja mal die Größe des kompilierten Programms vergleichen, 
eimmal mit if(speed) (ohne volatile), einmal mit if(1) und einmal mit 
if(speed) und volatile.

[Thomas E. (thomase)]

Chris K. schrieb:
> Aber dafür fragt man schließlich in einem Forum.

Ja, aber mit dem kompletten Code.

Zu den typischen Anfängerfehlern gehört das Weglassen/Vergessen von 
"volatile" und, das allerdings nicht nur bei Anfängern, das generelle 
Ausschließen von Fehlern: "Nein, daran kann es gar nicht liegen." Oder: 
"Daß da ein Fehler liegt, hätte ich niemals gedacht."
Fatal ist es, wenn beides auch noch zusammenkommt.