Wert des ADC Wandlers wird immer sofort ausgegeben (ATMega 2561, AVR-Studio 4)

[Fabian B. (Gast)]

Hallo liebe Community,

ich bin langsam kreativ an meinen Grenzen und weiß nicht mehr weiter.
Es geht um folgendes, ich möchte zuerst, dass ein analoger Wert von 
einem Potentiometer eingelesen wird. Danach soll nach einem Startbefehl 
eine Variable (in einer ISR) von maximal (1024 digits) zu dem 
eingelesenen Wert dekrementiert werden.

Wenn der Anlauf durchgelaufen ist, soll danach der Wert des 
Potentiometers normal eingelesen werden.

Zuerst der Code:

volatile int ALPHAsoll  =    0;
volatile int ALPHAIst  =    1023;
volatile int zaehler4 =     0; 
volatile int sanftanlauf_durch   =   0;

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++

int main (void)
.
.
.
while(1)
.
.
if (sanftanlauf_durch == 0)
{  
  ALPHA = (((ALPHAmax - ALPHAmin) / 1023) * (ALPHAIst) + ALPHAmin); //ALPHAIst wird im Timer 4 dekrementiert
}
else
{
  ALPHA = (((ALPHAmax - ALPHAmin) / 1023) * (ADC_Lesen())+ ALPHAmin);
}
.
.
.
if(ButtonPressed(0, PINA, 2, 500))  //an-/ausschalten
{
    if(gezuendet) //aus
    {
     Hier werden LED geschaltet
    }
    else  //an
    {
        ALPHAsoll = ADC_Lesen();
   ALPHAIst = 1023;
   sanftanlauf_durch=0;
    TCNT4 = 0;
    zaehler4 = 0;
        gezuendet = 1;
    }

++++++++++++++++++++++++++++++++++++++++++++
.
.
.

ISR (TIMER4_COMPA_vect)
{
zaehler4 ++;           // Zähler für Sanftanlauf

if (ALPHAsoll <= ALPHAIst)
{
  ALPHAIst = 1023 - zaehler4;           // Jeden Interrupt soll AlPHaIst an AlPHASoll näher herangetragen werden
  
}
else
{
  sanftanlauf_durch = 1;    // Sanftanlauf wird beendet
}

TCNT4 = 0;      // Timer 4 wird wieder auf 0 gesetzt
}
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

/*Analog-Digital-Konvertierung*/
int ADC_Lesen(void)
{
ADCSRA |=(1<<ADEN);       //ADC aktivieren
ADCSRA |= (1<<ADSC);              //nächste Umwandlung starten
  
    while (ADCSRA & (1<<ADSC));    //warten bis die Umwandlung abgeschlossen ist

  ADCSRA &= ~(1<<ADEN);      //ADC deaktivieren
return ADCW;        //Ergebnis von 0 bis 1023 ausgeben
}

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Der Code macht auch alles schön was er soll. Nur leider halt nicht alles 
;)

Zwei Probleme hab ich:

1) Könnt ihr mir sagen, warum ALPHAIst den Wert aus der ISR zur Main 
überträgt, obwohl ich der ISR keine Adresse übergeben habe

2) Wenn ich den Controller aktiviere (Buttenpressed ok) dann zeigt er 
mir zuerst den Wert an der am Potentiometer (ADC_Lesen()) eingestellt 
ist, springt danach auf AlPHAIst = 1023 und verkleinert ALPHAIst solange 
bis er auf ALPHAsoll (ADC_Lesen()) trifft.

Alles wäre so schön wenn er diesen allerersten Impuls auf (ADC_Lesen()) 
nicht machen würde.

Achja mit ALPHA wird der Mikrocontroller für die Anwendung die schon 
läuft auf unser 50 Hz Netz synchronisiert.
Damit muss irgendein ALPHA Wert zwingend zurückgegeben werden.

Ich habe irgendwie schon alles ausprobiert und bekomme das Problem 
dieses ersten ADC_Lesen impulses nicht in den Griff.

Entweder bin ich zu blöd oder der ATMega ist zu stark (Aber die Nerven 
sind weg ^^^;))

[Karl H. (kbuchegg)]

Keine Ahnung was du da rumrechnest. Im Moment tu ich mir extrem schwer 
deinen Code mit der Beschreibung dessen, was du erreichen willst, in 
Einklang zu bringen. Das kommt mir alles deutlich zu kompliziert vor.

[Karl H. (kbuchegg)]

In der ISR
>

>  ...
>   if (ALPHAsoll <= ALPHAIst)
>   {
>     ALPHAIst = 1023 - zaehler4;           // Jeden Interrupt soll AlPHaIst an AlPHASoll näher herangetragen werden
>   
>   }
>

was spricht gegen

ISR (TIMER4_COMPA_vect)
{
  if( !sanftanlauf_durch )
  {
    if (ALPHAsoll < ALPHAIst)
      ALPHAIst--;
    else if (ALPHAsoll > ALPHAIst)
      ALPHAIst++;
    else
      sanftanlauf_durch = 1;
  }

  TCNT4 = 0;      // Timer 4 wird wieder auf 0 gesetzt
}

Mit jedem ISR Aufruf wird ALPHAIst ein Stückchen näher an ALPHAsoll 
herangeführt.

Was du da allerdings in der Hauptschleife inszenierst, ist mir nicht 
wirklich klar.

(anstatt dem TCNT4 auf 0 setzen, wäre ein CTC Modus die bessere Wahl, 
aber das ist momentan ein Nebenschauplatz).

[Fabian B. (Gast)]

Hallo Karl-Heinz,

danke für die schnelle Antwort.
Das mit der komplizierten Beschreibung tut mir leid.
Ich probier es nochmal anders:

Das Problem liegt nicht unbedingt in der ISR. (Es spricht absolut nichst 
gegen deine Möglichkeit)

Der ALPHAIst wird ja verkleinert und das geänderte Ergebnis auch 
ausgegeben bis sanftanlauf_durch ist.

Leider gibt er auch direkt am Anfang das Signal aus, welches man 
eingestellt hat. Und nicht den maximalen Wert 1023, damit gewährleistet 
ist, das immer ein Maximaler ALPHA Wert zuerst ausgegeben wird und 
danach die entsprechend kleineren.

Ich weis gerade nicht wie ich das Problem näher einkreisen könnte.

Und ja der CTC Modus wäre das selbe.

[Karl H. (kbuchegg)]

So wie ich das sehe, hast du doch im main() im Prinzip eine 
Statemaschine vorliegen, oder nicht.
Die hat 3 Zustände
* vor dem Start
* nach dem Start, aber der Softanlauf läuft noch
* nach dem Softanlauf

Dann programmier das doch auch so

#define PRE_START  0
#define SOFTSTART  1
#define REGULAR    2

uint8_t state;

int main()
{
  ....

  state = PRE_START;

  while( 1 )
  {
    switch( state )
    {

      case PRE_START:
        ALPHAsoll = ADC_Lesen();
        ALPHAist = ALPHA = 1024;

        if(ButtonPressed(0, PINA, 2, 500))
          state = SOFTSTART;
        break;

      case SOFTSTART:
        ALPHA = ALPHAist;   // ALPHAist wird von der Timer ISR runtergezählt
        if( ALPHA == ALPAsoll )
          state = REGULAR;
        break;

      case REGULAR:
        ALPHA = ADC_Lesen();
        break;
    }

    ... mach was mit ALPHA (eventuell unter berücksichtigung, dass
    ... der state nicht mehr PRE_START sein darf. Keine Ahnung, kenn ja
    ... deine Anwednung nicht
  }
}

Das wäre für mich recht naheliegend.
Wenn dieser Softwanlauf nur ein einziges mal gebraucht wird, dann könnte 
man das auch nach dem Muster vereinfachen

int main()
{
  ....

  // Vorgeplänkel. AUf den Start warten
  do
  {
     ADC einlesen und als ALPHAsoll speichern
     ALPHA gleich 1024 ausgeben
  } while( ! ButtonPressed( ... ) )

  // Softanlauf
  ALPHAist = 1024;
  while( ALPHAist != ALPHAsoll )
    ALPHAist ausgeben;

  // Softanlaufphase ist vorbei. reguläre Arbeit aufnehmen
  while( 1 )
  {
    ALPHA = ADC_Lesen();
    ...
  }
}

IMHO hast du dich dadurch verzettelt, dass du zuviel Logik in ein paar 
Zeilen Code quetschen willst. Dazu benutzt du dann 25 Flags und 
Hilfsvariablen und irgendwann durchschaut man dann deren Zusammenspiel 
nicht mehr.

[Fabian B. (Gast)]

Ok warte,

ich gehe nochmal in mich und versuche es so deutlich wie möglich zu 
formulieren.

Ich lese beim buttonpress in der Main den Wert der aus dem Poti kommt = 
den ADC zurückgibt (ADCW). Und obwohl ich dem Prozessor sage nimm den 
Wert den ich dir berechne im ISR(Timer4_CompA_vect) und erst wenn ich 
durch bin nimm den ADCW. Macht er den allersten Zugriff auf den ADCW. 
Und das allerschlimmste das macht er nur 4 von 10 mal.

Ich bekomme diese Gewissheit von einem Oszilloskop welches ich so 
eingestellt habe das es bei einer horizontalen Abweichung von einem 
Messwert einen single Shot (single seq) mir ein Bild macht welches ich 
mir dann anschauen kann.

Vielleicht wird es klarer wenn ich die Bilder hier auch poste.

Wie man sieht soll der ALPHA den roten Impuls von der Mittellinie nach 
links verschieben. Gelb ist das Netz, Blau die Nulldurchgangserkennung.

Und irgendwie schaffe ich es nicht dem Compiler zu sagen, nehme den 
maximalen möglichen Digit (1023) der 1.6 ms nach Netzdurchgang kommt und 
verkleinere diesen Digitwert bis dorthin wo du stehen sollst.

Konnte ich das so klarer Darstellen?

[Karl H. (kbuchegg)]

Fabian B. schrieb:

> Und irgendwie schaffe ich es nicht dem Compiler zu sagen, nehme den
> maximalen möglichen Digit (1023) der 1.6 ms nach Netzdurchgang kommt und
> verkleinere diesen Digitwert bis dorthin wo du stehen sollst.
>
> Konnte ich das so klarer Darstellen?

Ja.
Und jetzt schau dir noch mal die Codesnippets an, die ich im letzten 
Post gemacht habe. Beide Varianten lösen IMHO genau dieses Problem, 
indem sie nicht alles in 3 Zeilen Code zu quetschen versuchen, sondern 
ganz einfach deine Anlaufphase in 3 voneinander getrennte Phasen 
aufteilen und dort explizit und leicht verständlich die jeweils 
relevanten Dinge abhandeln. Was immer das dann auch in der jeweiligen 
Phase sein mag.

[Fabian B. (Gast)]

Hi Karl-Heinz,

das ist richtig.

Die drei cases existieren und an die Case funktion an sich habe ich noch 
gar nicht gedacht.

Ich programmiere diese mal aus und hoffe das das Problem dadurch gelöst 
ist.

Bis dahin danke schon mal für die Idee.

Und ich halte euch/dich auf dem laufenden.

Lg Fabian

[Fabian B. (Gast)]

Hallo Community,
lieber Karl Heinz,

die Idee mit der Switch Case Funktion war super. Das Programm ist 
dadurch viel intuitiver erfassbar. Leider existiert das Problem mit den 
falschen Zündimpulsen (Bilder beim Post heute Mittag beachten) immer 
noch. So dass er manchmal (bisher 12 von 20) Zündungen richtig macht und 
manche (20-12=8 ^^) falsch.

###########################################################
#define VOR_START 0
#define HOCHLAUF 1
#define NORMAL   2
+++++++++++++++++++++++++++++++++++++++++++++++
while(1)
{
  switch (STATUS)
  {
    case VOR_START: 
          
      ALPHAsoll = ADC_Lesen();
      ALPHA    = ALPHAmax;

                  if(Buttonpressed(...))
                  {
    STATUS = HOCHLAUF;
    TCNT4 =     0;
    zaehler4 =     0;
    ALPHAIst =   1023;
                   }
    break;
      
    case HOCHLAUF:
        
            
ALPHA = (((ALPHAmax - ALPHAmin) / 1023) * (ALPHAIst) + ALPHAmin);  

    if (ALPHAIst == ALPHAsoll)
      STATUS = NORMAL;

    break;

    case NORMAL:          
      
         Berechne ALPHA über ADCW 
                 break;
}

Mir ist leider auch schleierhaft wieso er immer noch (jedoch nur ab- und 
an) den falschen ADCW wert sofort schreibt obwohl es jetzt klar ist, das 
der Compiler das so nicht tuen sollte.

Bin für jeden Hinweis dankbar auch gerne in die Richtung "Wieso braucht 
die ISR keine Schreibberechtigung um den Wert von ALPHAIst zu verändern"
(Irgendwie hab ich so ein Bauchgefühl das es daran liegt)

Lg Fabian

[Karl H. (kbuchegg)]

Fabian B. schrieb:

> Bin für jeden Hinweis dankbar auch gerne in die Richtung "Wieso braucht
> die ISR keine Schreibberechtigung um den Wert von ALPHAIst zu verändern"


Weil das eine globale Variable ist.
Jeder der will, darf drann rummachen.

> (Irgendwie hab ich so ein Bauchgefühl das es daran liegt)

das einzige was daran problematisch ist, ist das Problem, dass du da 
einen atomaren Zugriff drumm herum legen musst, weil das eine 16 Bit 
Variable ist und der AVR die nicht in einem Rutsch lesen kann.
d.h. du solltest tunlichst in der Hauptschleife so was

    ....
    cli();
    Wert = ALPHAist;
    sei();
    mit 'Wert' weiterrechnen
    ...

machen, anstelle von direkten Zugriffen auf ALPHAist.

[Fabian B. (Gast)]

Danke

Probier ich gleich aus.
Leider sieht es bisher so aus als ob das das Rätsel Lösung noch nicht 
ist. Falls noch weitere Ideen herumgeistern immer her damit.

Vielen Dank für alles bisher

Lg Fabian

[Karl H. (kbuchegg)]

Sowas

ALPHA = (((ALPHAmax - ALPHAmin) / 1023) * (ALPHAIst) + ALPHAmin);

ist zum Beispiel eine ausgesprochen schlechte Idee.
Divisionen immer soweit wie möglich nach rechts in den Berechnungen 
schieben! Hier verlierst du extrem an Genauigkeit. Das ist eine 
Integer-Division! Die produziert keine Nachkommastellen! Das Ergebnis 
von 7/5 ist 1 und nicht 1.4

[spess53 (Gast)]

Hi

>int ADC_Lesen(void)
>{
>ADCSRA |=(1<<ADEN);       //ADC aktivieren

>....

>  ADCSRA &= ~(1<<ADEN);      //ADC deaktivieren
>return ADCW;        //Ergebnis von 0 bis 1023 ausgeben
}

Datenblatt:

A normal conversion takes 13 ADC clock cycles. The first conversion 
after the ADC is switched on (ADEN in ADCSRA is set) takes 25 ADC clock 
cycles
in order to initialize the analog circuitry. When the bandgap reference
voltage is used as input to the ADC, it will take a certain time for
the voltage to stabilize. If not stabilized, the first value read after
the first conversion may be wrong.

Schalte den ADC einmal in der Initialisierung an und lass dann die 
Finger von ADEN.

MfG Spess

[Fabian B. (Gast)]

Hallo Karl-Heinz,

ja das mit der Integer Rechnung ist mir bekannt. Ist jedoch erstmal kein 
Problem, da ALPHA eine Wert zwischen 20000 und 75000 bekommt, da sind 
dann die Kommazahlen rein rechnerisch nicht vorhanden.

Hallo Spess53 ein guter Hinweis. Die Stelle im Datenblatt kenne ich. Ich 
hatte euch vorenthalten das bei der Initialisierung das ADSC Bit mit 
gesetzt wird und damit schon eine Umwandlung läuft, dessen Wert dann 
durch die nächste Umwandlung in meinem Hauptprogramm wieder 
überschrieben wird.

Hm... ich hatte gedacht das das wichtige Bit das ADSC Bit ist, denn 
damit wird die Umwandlungen gestartet und man danach das ADEN Bit wieder 
löschen kann (Das ADC Register somit wieder deaktiviert [Hintegrund: 
Wenn man die Werkbank verlässt, schaltet man doch das Licht aus, oder 
habe ich mir damit ein eigenes Bein gestellt])?

Lg Fabian

[Fabian B. (Gast)]

Ok,

Update:

Auch wenn ich das ADEN Bit nur in der Initialisierung setzte und von der 
ADC Hardware das ADSC Bit rücksetzen lasse, komme ich immer noch nicht 
an den allerersten Wert des ADCW der mal (52%) bei max anfängt (richtig) 
und mal (48% im groben)bei dem eingestellten Wert vom Poti (falsch).

Gut der Fehler wird durch eure Tipps immer näher eingekreist, leider ist 
er immer noch nicht erschlagen.

Um mal Starship Troopers zu zitieren: "Ich hasse Bugs"

Somit sieht es so aus:

Durch switch case Funktion ist das ganze Intuitiv erfassbar.
Durch das einmalige setzen von ADEN ist auch diese Fehlerquelle behoben.

Ich hoffe nicht das aus Starship Troopers die unendliche Geschichte 
wird.

Lg