Programm läuft nicht Verbesserungsvorschläge?

[usi (Gast)]

Hallo Leute!

Habe folgendes Progamm geschrieben. Leider läuft es nicht und lässt sich 
auch nicht debuggen.

Hat jemand eine Idee, was man besser machen könnte, bzw. was hier falsch 
läuft?

Vielen DANK!

Gruss

#include <avr/io.h>

//xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

// Define_Fix_Values

   double Freq_clk_psc = 16000000;
  
   double DUTY_CYCLE;
   double Freq_psc_cycle;
   double Per_psc_cycle;
   double DEAD_TIME;      
   double ON_TIME_0; 
   double ON_TIME_1; 
  
  
   unsigned int ADC_average;//ADC_Input_Mittelwert
   unsigned int  ADC_Vin;//ADC_Input_Voltage
  
   unsigned int resolution = 1024;//10_bit_ADC
   unsigned int Vref = 5000; // 1/1000V (mV)
   unsigned int max_current = 500; // A
   unsigned int target_current = 400; // A
   unsigned int scopeband; //ADC_Bereichsbreite
  
   unsigned int LEM_current;
   
  
//xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx


void init_pwm_value(void)
{
  
  DUTY_CYCLE = 0.50; // in %
  DEAD_TIME = 0.000008; // in sec.
  Freq_psc_cycle = 30000; // in Hz
  
  Per_psc_cycle = 1/Freq_psc_cycle;
  ON_TIME_0 = DUTY_CYCLE * Per_psc_cycle;
  
};

// Power_Stage_Controll_2_Initalisation
void init_psc2(void)
{
  
  PSOC2 = (1<<POEN2A)|(1<<POEN2B); // PSC_Waveform_Generator_To_Output

  OCR2SA = (ON_TIME_0/2)*Freq_clk_psc;
  OCR2RB = ((Per_psc_cycle/2)*Freq_clk_psc)-1;
  OCR2SB = (DEAD_TIME*Freq_clk_psc)+OCR2SAH;
  
}

// Ramp_Mode_Initialisation
void init_ramp_mode(void)
{
  PCNF2 |= (1<<PMODE21)|(1<<PMODE20)|(1<<POP2)|(1<<PALOCK2); // Center_Aligned_Mode // Slow_Clock_Input (PCLKSEL2 = 0)
            
}

void init_adc(void)
{
  unsigned int result;
  
  ADMUX &= ~((0<<REFS1)|(0<<REFS0));//External_Referencevoltage_used
  ADMUX &= ~(0<<ADLAR);//Right_Adjust_ADC_result     
  ADCSRA |= (1<<ADPS2)|(1<<ADPS1)|(1<<ADPS0);//ADC_Prescaler_Division_128 = 125kHz
  ADCSRA |= (1<<ADEN); //ADC_enable
  DIDR0 = 0xFF;
  DIDR1 = 0xFF;
  ADCSRA |= (1<<ADSC); //one_ADC_conversation
  while (ADCSRA & (1<<ADSC)); // waiting_for_conversation_end
  result = ADCW; // Dummy_readout
}

unsigned int ADC_read()
{
    ADMUX |= (1<<MUX3)|(1<<MUX0);//Select ADC_9     
  ADCSRA |= (1<<ADSC); //one_ADC_conversation
  while (ADCSRA & (1<<ADSC)); // waiting_for_conversation_end
  return (ADCW);
}

unsigned int ADC_average_read()
{
  int i;
  unsigned long int sum = 0;
  
  for(i=0; i<=32; i++)
  {
    sum = sum+ADC_read();
  }  
  return(unsigned int)(sum/33);
}  
     
void DAC_PWM_output(void)
{
  
  
  if(LEM_current <= max_current)
  {
    PCTL2 |= (1<<PCCYC2)|(1<<PRUN2); // PWM_Output_enable
      
  }
  else
  {
    PCTL2 &= ~((1<<PCCYC2)|(1<<PRUN2)); // PWM_Output_disable
      
  }
  
    DAC = ADC_average;
    DACON &= ~(1<<DALA);
    DACON |= (1<<DAOE)|(1<<DAEN); //DAC_starting_conversation
    
}

int main(void) 
{ 
  
  scopeband = (unsigned int)Vref/resolution;
  
  
  init_pwm_value();
  init_psc2();
  init_ramp_mode();
  init_adc();  
  
  
  
  ADC_average = ADC_average_read();
  ADC_Vin = (unsigned int)ADC_average*scopeband;
  LEM_current = (unsigned int)((ADC_Vin-2.5)/0.00222);
  DAC_PWM_output();
  
  while(1)
  {
  
  }  

  return 0;
}

[Karl H. (kbuchegg)]

usi schrieb:
>
> Hat jemand eine Idee, was man besser machen könnte,

Was du zb besser machen könntest ist schon mal eine ordentliche und 
vernünftige Fehlerbeschreibung!

> und lässt sich auch nicht debuggen.

Das glaub ich nicht.
Blöd drann ist man natürlich, wenn man bei der Konzeption davon ausging, 
der Programmier-Superstar zu sein, der keine Debug-Möglichkeit braucht. 
Alle anderen sehen sich ein paar Pins vor, über die sie sich vom 
Programm aus Werte ausgeben lassen können. Ob das jetzt UART, LCD, ein 
paar LED oder sonst irgendwas ist, spielt erst mal nicht so sehr eine 
Rolle. Die Hauptsache das Programm hat ein Möglichkeit mir die Inhalte 
von Variablen irgendwie mitzuteilen.

Und dann hat man somit seine Debugmöglichkeit.

Und dann schreibt man nitürlich nicht ein Programm in einem Rutsch, 
sondern in Etappen, die man möglichst bald und möglichst vollstänbdig 
testet. Dann fallen die Teile, die nicht funktionieren, sofort auf - es 
sind die paar Zeilen Code, die man als letztes geschrieben hat. Dann 
steht man auch nicht mit einem kompletten Programm da und hat nicht den 
Hauch einer Ahnung, wo man mit Fehlersuche anfangen könnte.

[Karl H. (kbuchegg)]

>   ADMUX &= ~(0<<ADLAR);//Right_Adjust_ADC_result

Merk dir eines

   0<<ADLAR

ist nie ein sinnvolles Konstrukt!

eine 0 kannst du bis zum St. Nimmerleinstag nach rechts oder nach links 
schieben - das Ergebnis ist nach wie vor 0.

[usi (Gast)]

Hallo!

Debugger JTAG ice mk II ist vohanden. Die Variablen lass ich mir direkt 
im Studio anzeigen.

Breakpoint habe ich bei der while-Schleife im main() gesezt.

Wenn ich in den Debuggmodus wechsle, lässt sich das Programm mit F10 
nicht durchlaufen, da im Studio unten links die ganze zeit "Running" 
steht.

[usi (Gast)]

Ok, diese ADMUX Nullbeschreibung hab ich schon mal rausgelöscht.
DANKE!

[Gregor B. (Gast)]

usi schrieb:
> Breakpoint habe ich bei der while-Schleife im main() gesezt.

Ja, und?

Was steht in der While? Antwort: Nichts.
Der geht also in die Whileschleife und springt dort unbedingt an den 
Anfang der While-Schleife. Was willst Du da noch debuggen?

[usi (Gast)]

ADC_average = ADC_average_read();
  ADC_Vin = (unsigned int)ADC_average*scopeband;
  LEM_current = (unsigned int)((ADC_Vin-2.5)/0.00222);
  DAC_PWM_output();

den Teil hab ich jetzt mal in die while-Schleife übernommen

[usi (Gast)]

in der while-Schleife tritt folgender Fehler auf

[Leichte Abhilfe (Gast)]

usi schrieb:
> in der while-Schleife tritt folgender Fehler auf

Fehler:  Da ist kein Sourcecode drin.
Abhilfe: Was reinschreiben.

[usi (Gast)]

>Fehler:  Da ist kein Sourcecode drin.
>Abhilfe: Was reinschreiben.

in wie fern steht hier kein Sourccode drin?

[Maxx (Gast)]

Du willst was debuggen, wofür du den Quelltext nicht hast.

Jetzt geh doch einfach mal nen Schritt zurück und setz den Breakpoint am 
Anfang deiner main() bevor du irgendwas anderes machst und guck was von 
da an passiert

[Cyblord -. (cyblord)]

Ah eine neue Vorgehensweise. Man ist mit seinem Programm nicht 
zufrieden, dann klatscht man es halt mal rein und wartet auf 
"Verbesserungsvorschläge". Ohne Fehlerbeschreibung, ja und ganz ohne 
Beschreibung was das Probramm eigentlich tun sollte. Kann man sich ja 
ausm Code rauslesen, gelle. Unglaublich.

Und wenn man Konstruke sieht, welche eine 0 mit << rumschieben wollen, 
dann wurde doch hier der fünfte Schritt vor dem ersten gemacht. Also 
bevor man mit PWM rumpfuscht, einfach mal Bitoperationen lernen.

gruß cyblord

[Hans M. (hansilein)]

while(1)
  {

  }

ist eine Endlosschleife, die nichts tut.
Wir wissen leider nicht was Du wirklich machen wolltest.

[usi (Gast)]

Hallo!

Weis jemand wie man im Studio 5 die Library libm.a einstellen, bzw. 
benutzen kann?

DANKE

[usi (Gast)]

Ich habe mal ein bischen nachgelesen, dieser Fehler könnte auch durch 
einen fehlenden Linkerzusatz entstehen.

Meine Frage, wie kann man diese Zusätze im Studio einstellen?

Gruss

[Walter T. (nicolas)]

cyblord ---- schrieb:
> [...] sieht, welche eine 0 mit << rumschieben wollen, [...]

Also ich bin jetzt nicht der Mega-Programmierer, aber solche Konstrukte 
nutze ich durchaus auch, um für mich selbst zu dokumentieren, daß ich an 
ein bestimmtes Bit in einem Register durchaus gedacht habe, es aber mit 
Absicht NICHT verändere, z.B.:

 UCRSR0B |= (1<<RXEN0) | (0<<TXEN0);

da sehe ich auf einen Blick, daß ich "TXEN" bewußt nicht setze und nicht 
etwa vergessen habe...

Just my 2 cents...

Nicolas

P.S.: Was keineswegs die Praxis "mein Code geht nicht, guckt mal nach!" 
gutheißen soll.

[Cyblord -. (cyblord)]

Nicolas S. schrieb:
> cyblord ---- schrieb:
>> [...] sieht, welche eine 0 mit << rumschieben wollen, [...]
>
> Also ich bin jetzt nicht der Mega-Programmierer, aber solche Konstrukte
> nutze ich durchaus auch, um für mich selbst zu dokumentieren, daß ich an
> ein bestimmtes Bit in einem Register durchaus gedacht habe, es aber mit
> Absicht NICHT verändere, z.B.:
>

>  UCRSR0B |= (1<<RXEN0) | (0<<TXEN0);
>

> da sehe ich auf einen Blick, daß ich "TXEN" bewußt nicht setze und nicht
> etwa vergessen habe...

Naja also 1<<RXEN0) definiert ja eine Bitmaske. Wohingegen (0<<TXEN0) 
wieder genau 0 produziert. Zwar auch eine Maske aber sinnlos. Was soll 
damit dokumentiert werden? Wofür gibts Kommentare? Lieber einen 
Kommentar als hanebüchenen Code welcher offensichtlich Unsinnig ist.
Denn dein Code impliziert dass du denkst, dieses Konstrukt würde das 
TXEN0 Bit auf 0 setzen. Was natürlich nicht stimmt. Damit stiftest du 
Verwirrung. Auch bei dir selbst wenn du den Code später mal anguckst.

gruß cyblord

[Walter T. (nicolas)]

Tja, daran erkennt man eben, daß ich kein Profi bin und deshalb nie auf 
die Idee käme, daß eine bitweise Veroderung ein Bit löschen könne.

[Patrick W. (pawi777)]

usi schrieb:
> double Freq_clk_psc = 16000000;
>
>    double DUTY_CYCLE;
>    double Freq_psc_cycle;
>    double Per_psc_cycle;
>    double DEAD_TIME;
>    double ON_TIME_0;
>    double ON_TIME_1;

DAFUQ?!
Weisst du überhaupt was du da schreibst? Double ist im avr-gcc das selbe 
wie float, und das kann man NIEMALS direkt in ein Register schreiben! 
Für sowas nimmt man integer, zb uint16_t. Wenn du zahlen grösser als ca 
65k(2^16-1) brauchst, nimm uint32_t oder uint64_t.

Am besten schreibst du alle integer so wie in der ersten Spalte:

(u)int8_t  = (Un)Signed Char
(u)int16_t = (Un)signed Int
(u)int32_t = (Un)signed Long
(u)int64_t = (Un)Signed Long Long

[Daniel (Gast)]

Nicolas S. schrieb:
> Tja, daran erkennt man eben, daß ich kein Profi bin und deshalb nie auf
> die Idee käme, daß eine bitweise Veroderung ein Bit löschen könne.

:-)) full ack...
Ich mache es wie Du mit dem (0<<RXEN), aus dem gleichen Grund - 
paradoxerweise bin ich der Meinung, diese Methode hier im Forum als 
"gute Praxis" gelernt zu haben :-o

[Edson (Gast)]

cyblord ---- schrieb:
> Naja also 1<<RXEN0) definiert ja eine Bitmaske. Wohingegen (0<<TXEN0)
> wieder genau 0 produziert. Zwar auch eine Maske aber sinnlos. Was soll
> damit dokumentiert werden? Wofür gibts Kommentare? Lieber einen
> Kommentar als hanebüchenen Code welcher offensichtlich Unsinnig ist.
> Denn dein Code impliziert dass du denkst, dieses Konstrukt würde das
> TXEN0 Bit auf 0 setzen. Was natürlich nicht stimmt. Damit stiftest du
> Verwirrung. Auch bei dir selbst wenn du den Code später mal anguckst.

Anscheinend akzeptierst du nur Dinge, die deinem eigenen Kopf 
entspringen. Deshalb ist es aber noch lange nicht falsch. Wo wird 
impliziert, dass ein Bit auf 0 gesetzt wird? Genau - nirgends, weil wir 
ja wissen dass eine ODER-Verknüpfung mit 0 keinen Einfluss hat.

Nicolas schrieb:
> Tja, daran erkennt man eben, daß ich kein Profi bin und deshalb nie auf
> die Idee käme, daß eine bitweise Veroderung ein Bit löschen könne.

Nö, daran (an cyblords Reaktion) erkennt man nur dass manch 
Möchtegern-Profi Regeln aufstellt, die es in der realen Weil nicht gibt 
und auch nicht braucht.

[Cyblord -. (cyblord)]

Was denn für Regeln? "Lasse falsche und sinnlose Konstrukte einfach 
weg"? Ja diese Regel habe ich nicht aufgestellt, aber man fährt bestimmt 
nicht schlecht wenn man sich daran hält. Sollte aber eigentlich 
selbstverständlich sein.

gruß cyblord

[Karl H. (kbuchegg)]

Daniel schrieb:

> Ich mache es wie Du mit dem (0<<RXEN), aus dem gleichen Grund -
> paradoxerweise bin ich der Meinung, diese Methode hier im Forum als
> "gute Praxis" gelernt zu haben :-o

Ist ein zwei-schneidiges Schwert und hängt unter anderem auch immer 
davon ab, wer es schreibt.
Man sieht hier im Forum gar nicht so wenige Neulinge, die diese 
Schreibweise benutzen in der vermeintlichen Hoffnung, ein Bit so auf 0 
zu setzen


Wenn er andeuten möchte, dass ADLAR nicht gesetzt sein soll, dann 
spricht nichts gegen

  ADMUX &= ~(1<<ADLAR);//Right_Adjust_ADC_result

aber

  ADMUX &= ~(0<<ADLAR);//Right_Adjust_ADC_result

ist einfach nur eine 0-Operation, bei der man sich fragt: Tippfehler 
oder doch kein Tippfehler? Auf der einen Seite suggeriert das Ansprechen 
des ADLAR Bits, dass er es aktiv beeinflussen möchte (um es auf 0 zu 
setzen), auf der anderen Seite würde aber ein einmal gesetztes ADLAR Bit 
hier mit Sicherheit nicht 0 gesetzt werden, denn die ganze Anweisung ist 
eine 0-Operation, die keinen Effekt hat!

(Jetzt mal ganz vom nicht wirklich passenden Kommentar abgesehen)

Dasselbe hier

  ADMUX &= ~((0<<REFS1)|(0<<REFS0));//External_Referencevoltage_used

Effektiv passiert hier nämlich nichts. Einmal gesetzte Refernz-Spannungs 
Bits werden dadurch NICHT verändert! Sie werden nicht 0 gesetzt und auch 
nicht auf 1. Sie werden überhaupt NICHT verändert! WEnn ich aber etwas 
sowieso nicht verändere, weder in der einen Richtung noch in der 
anderen, dann ist es sicherlich besser, es einfach wegzulassen. Das 
erspart dann auch das Rätselraten, ob das Absicht ist oder ob hier ein 
Fehler gemacht wurde.

Bei

  ADCSRA |= (1<<ADPS2) | (0<<ADPS1) | (1<<ADPS0);

würde ich persönlich die Verwendung eines 0<<xy Konstrukts ok finden. 
Denn hier dokumentiere ich, das ich mir bewusst bin, dass es dieses Bit 
gibt, das ich es aber bewusst nicht setzen möchte (allerdings wird es 
dadurch auch nicht zurück gesetzt. In einer Form

  ADCSRA = 0;
  ADCSRA |= (1<<ADPS2) | (0<<ADPS1) | (1<<ADPS0);

würde ich das aber akzeptabel finden.

[Cyblord -. (cyblord)]

Das Problem ist einfach wenn jemand außer dem Autor den Code anschaut. 
Niemand wird verstehen was dort gemacht wurde. Man würde nicht davon 
ausgehen das dies nur zu Dokumentationszwecken da ist also würde man 
sich überlegen was der Autor tun wollte und eben einen Fehler vermuten. 
Also in der realen Welt eher ungeeignet, für zuhause, wenn man der 
einzige ist der den Code anguckt, dann ok. Aber warum sowas angewöhnen?

Lustiges zum Thema:

http://images.cryhavok.org/d/2471-1/WTFs+per+Minute.jpg

[Edson (Gast)]

cyblord ---- schrieb:
> Das Problem ist einfach wenn jemand außer dem Autor den Code anschaut.
> Niemand wird verstehen was dort gemacht wurde. Man würde nicht davon
> ausgehen das dies nur zu Dokumentationszwecken da ist also würde man
> sich überlegen was der Autor tun wollte und eben einen Fehler vermuten.

Das Problem sehe ich ehrlich gesagt nicht. Das (nicht gesetzte) Bit hat 
in der Hardware eine bestimmte Bedeutung - vor einem Blick ins 
Datenblatt ist es reines Rätselraten, ob hier ein Fehler vorliegt oder 
nicht. Den einen, den wahren Stil gibt_es_nicht, wer fremden 
Sourcecode lesen muss ist immer angehalten, nicht jede ihm unbekannte 
Schreibweise als Fehler zu interpretieren sondern diese 
nachzuvollziehen.
Die "gefährlichen" Fehler verstecken sich in der Semantik, und diese 
muss man verstanden haben. Sich an Schreibweisen aufzuhängen lenkt da 
nur vom Wesentlichen ab.

[Karl H. (kbuchegg)]

Edson schrieb:

> Das Problem sehe ich ehrlich gesagt nicht. Das (nicht gesetzte) Bit hat
> in der Hardware eine bestimmte Bedeutung

Das Problem ist, dass an dieser Stelle das Bit weder gesetzt noch nicht 
gesetzt wird. Es wird weder in die eine Richtung noch in die andere 
Richtung verändert. Es passiert einfach nichts mit dem Bit. War es 
vorher gelöscht, dann ist es auch nachher glöscht. War es vorher 
gesetzt, dann ist es auch nachher gesetzt.
Anstelle von

  ADMUX &= ~((0<<REFS1)|(0<<REFS0));//External_Referencevoltage_used
  ADMUX &= ~(0<<ADLAR);//Right_Adjust_ADC_result     
  ADCSRA |= (1<<ADPS2)|(1<<ADPS1)|(1<<ADPS0);//ADC_Prescaler_Division_128 = 125kHz
  ADCSRA |= (1<<ADEN); //ADC_enable

hätte er genausogut

   i = i;
   j = j;
  ADCSRA |= (1<<ADPS2)|(1<<ADPS1)|(1<<ADPS0);//ADC_Prescaler_Division_128 = 125kHz
  ADCSRA |= (1<<ADEN); //ADC_enable

schreiben können. Der Effekt ist genau 100% derselbe.

[Karl H. (kbuchegg)]

Aber lasst uns jetzt nicht um die Schreibweise streiten.

Die Fragen, die sich daraus ergeben
* was genau versteht er jetzt unter 'funktioniert nicht'
* ist eine externe Referenzspannung angeschlossen?
* Hat er sich schon mal den Messwert angesehen?
* Und warum wird die Messung eigentlich nur ein einziges mal gemacht
  und nicht in der Hauptschleife, so wie man das für eine
  koninuierlche Operation eigentlich erwarten würde?

[Unplenked (Gast)]

usi schrieb:
> Hallo Leute!
>
> Habe folgendes Progamm geschrieben. Leider läuft es nicht und lässt sich
> auch nicht debuggen.
>
> Hat jemand eine Idee, was man besser machen könnte, bzw. was hier falsch
> läuft?
> Vielen DANK!

Also ansonsten bist du aber gesund?

Wenn du nicht erzählst, was das Programm tun soll, wie soll jemand 
anderes dann feststellen, was falsch ist oder nicht?

Gewollt kann vieles sein, auch z.B.:    while(1);
... wenn ich z.B. darauf warte, dass man mir den Saft abdreht.

[Unplenked (Gast)]

cyblord ---- schrieb:
> Ah eine neue Vorgehensweise.

Reg' dich nicht auf cybi, der Kerl kommt wahrscheinlich aus der Arduino 
Ecke ;-)       (Vorsicht, Ironie kann dick machen.)

[Unplenked (Gast)]

Nicolas S. schrieb:
> Also ich bin jetzt nicht der Mega-Programmierer, aber solche Konstrukte
>
> nutze ich durchaus auch, um für mich selbst zu dokumentieren, daß ich an
>
> ein bestimmtes Bit in einem Register durchaus gedacht habe, es aber mit
>
> Absicht NICHT verändere, z.B.: UCRSR0B |= (1<<RXEN0) | (0<<TXEN0);

Was spricht denn dagegen, dies in einem Kommentar zu dokumentieren:
// Ich, Nicolas, will das verdammte TXENO mit Absicht nicht verändern

... über den Textinhalt kann man sich freilich streiten  ;-)

Es scheint aber unter Hobbyprogrammierern, und solchen, die es werden 
wollen, eine starke Aversion gegen eine ordentliche Dokumentation des 
Programm durch Kommentare zu geben.

[Edson (Gast)]

Unplenked schrieb:
> Es scheint aber unter Hobbyprogrammierern, und solchen, die es werden
> wollen, eine starke Aversion gegen eine ordentliche Dokumentation des
> Programm durch Kommentare zu geben.

Weder deine bisherigen noch dieser Beitrag lassen im entferntesten 
darauf schliessen, dass du einen Profi vom Hobbyisten unterscheiden 
könntest. Also quatsch nicht...

kbuchegg schrieb:
>Aber lasst uns jetzt nicht um die Schreibweise streiten.

Ok, weil du es bist. ;)

[Walter T. (nicolas)]

Unplenked schrieb:
> Es scheint aber unter Hobbyprogrammierern, und solchen, die es werden
> wollen, eine starke Aversion gegen eine ordentliche Dokumentation des
> Programm durch Kommentare zu geben.

Stimmt, vor allem wenn sich mit der Zeit herausstellt, daß Kommentar und 
Source immer weiter auseinanderdriften, was oft daran liegt, daß viele 
Sachen im unfertigen Zustand bleiben und der letzte Schliff für immer 
ausbleibt...
Dann lieber Kommentierung so knapp wie möglich und den Rest möglichst 
selbstdokumentierend.

Ich habe nie behauptet, daß das perfekt oder professionell ist. Aber es 
ist definitiv nicht immer gedanken- oder sinnlos.

[Peter D. (peda)]

Karl Heinz Buchegger schrieb:
> ADMUX &= ~((0<<REFS1)|(0<<REFS0));//External_Referencevoltage_used
>   ADMUX &= ~(0<<ADLAR);//Right_Adjust_ADC_result
>   ADCSRA |= (1<<ADPS2)|(1<<ADPS1)|(1<<ADPS0);//ADC_Prescaler_Division_128 = 
125kHz
>   ADCSRA |= (1<<ADEN); //ADC_enable

Ich halte von dieser AND/ORerei nichts. Man muß dann immer wissen, wie 
die Bits vorher gesetzt waren.
Ich kann mir das nicht merken, deshalb weise ich immer alle Bits zu, 
z.B.:

#define LOAD_ADCSRA ( 1<<ADEN                          /* enable ADC */ \
                    | 1<<ADSC                          /* start conversion */ \
                    | 1<<ADPS2 | 1<<ADPS1 | 0<<ADPS0 ) // 8MHz/64 = 125kHz

#define LOAD_ADMUX  ( 0<<REFS1 | 0<<REFS0              /* external reference */ \
                    | 0 )                              // select ADC0
...
  ADMUX = adc_idx + LOAD_ADMUX;                        // ADC0 ... ADC5
  adc_sum[adc_idx] += ADC;
  ADCSRA = LOAD_ADCSRA;                                // start next conversion

Und bei der Zuweisung setzt das 0 schieben das entsprechende Bit auch 
wirklich auf 0.


Peter

[Cyblord -. (cyblord)]

Nicolas S. schrieb:

> Stimmt, vor allem wenn sich mit der Zeit herausstellt, daß Kommentar und
> Source immer weiter auseinanderdriften, was oft daran liegt, daß viele
> Sachen im unfertigen Zustand bleiben und der letzte Schliff für immer
> ausbleibt...
> Dann lieber Kommentierung so knapp wie möglich und den Rest möglichst
> selbstdokumentierend.

Völlig korrekte Aussage.

Sowenig Kommentare wie nötig. Ein Kommentar an einer Stelle ist 
grundsätzlich ein Eingeständniss dass hier der Code obskur ist. Aber 
(0<<X) ist bereits obskur und darum sollte man es weglassen, oder aber 
wenn man es denn benötigt, einen kurzen Kommentar dazu.

gruß cyblord

[Daniel (Gast)]

Karl Heinz Buchegger schrieb:
> In einer Form  ADCSRA = 0;
>   ADCSRA |= (1<<ADPS2) | (0<<ADPS1) | (1<<ADPS0);
> würde ich das aber akzeptabel finden.

Ok, ich gebe Dir recht, ich habe oben die reine Zuweisung und die 
Veroderung mit dem gelesenen Wert über einen Kamm geschoren.

Also so akzeptal

ADCSRA = (1<<ADPS2) | (0<<ADPS1) | (1<<ADPS0)

und so nicht so gut

ADCSRA |= (1<<ADPS2) | (0<<ADPS1) | (1<<ADPS0)

[Stefan K. (Gast)]

Karl Heinz Buchegger schrieb:
> Merk dir eines
>    0<<ADLAR
> ist nie ein sinnvolles Konstrukt!

Hi Karl-Heinz...

Muss kein Fehler sein... Das mach ich auch so.
Das "0<<ADLAR" ist hier wahrscheinlich nur als ein Platzhalter gedacht.
Es lässt halt das Bit auf "0", aber ggf. kann man schnell ne "1" aus der 
"0" machen und man kann sofort lesen welche Bits "0" und welche "1" 
sind.

[Karl H. (kbuchegg)]

Stefan K. schrieb:
> Karl Heinz Buchegger schrieb:
>> Merk dir eines
>>    0<<ADLAR
>> ist nie ein sinnvolles Konstrukt!
>
> Hi Karl-Heinz...
>
> Muss kein Fehler sein... Das mach ich auch so.
> Das "0<<ADLAR" ist hier wahrscheinlich nur als ein Platzhalter gedacht.
> Es lässt halt das Bit auf "0",

Siehs dir bitte im ganzen Zusammenhang an.

 ADMUX &= ~(0<<ADLAR);

Dieses Statement verändert NICHTS in ADMUX. Weder wird ADLAR auf 0 
gesetzt, noch wird es auf 1 gesetzt. Das ist ein Statement ohne Effekt. 
Da steht

 ADMUX = ADMUX & 0xFF;

und das man ein 8 Bit Register jederzeit mit 0xFF verunden kann und doch 
nur wieder den vorherigen Zustand erhält, ist auch nicht geheimnisvoll. 
IM Endeffekt steht da

 ADMUX = ADMUX;

Das ist aber eine Trivialität, von der man sich fragen muss: Will ich 
das da wirklich stehen haben, oder verschleiert mir da die komplizierte 
Schreibweise nicht eigentlich, dass hier absolut nichts passiert?

Wenn ich mir die Option offen halten möchte anzudeuten, dass ich mich 
nicht um ADLAR kümmere, dann lieber so

  //  ADMUX &= ~(1<<ADLAR);      don't care, must be handled by caller

> und man kann sofort lesen welche Bits "0" und welche "1" sind.

Und genau da ist der Trugschluss. Dieses Statement suggeriert, dass da 
etwas passiert. In Wirklickeit aber verändert es nichts. Gar nichts. 
Nimmt man das Statement raus, verändert sich das Programm in keinster 
Weise. Bis zum letzten Bit ist alles ohne oder mit diesem Statement 
identisch.

Man kann natürlich sein Programm mit Statements alla

   i = i;
   j = j;

   // mach was

   k = k;
   j == 5;

zupflastern, wenn man möchte. Aber wie sinnvoll ist das?

[Fläck (Gast)]

Karl Heinz Buchegger schrieb:
> ADMUX &= ~(0<<ADLAR);
>
> Dieses Statement verändert NICHTS in ADMUX. Weder wird ADLAR auf 0
> gesetzt, noch wird es auf 1 gesetzt. Das ist ein Statement ohne Effekt.
> Da steht
>
>  ADMUX = ADMUX & 0xFF;

und wenn sowas nu noch auf ein Register angewendet wird in dem ein noch 
zu verwendendes Interrupt Flag stehen könnte, is die Ka... am dampfen, 
oder?