PWM vermutlich falsche Werte

[Jacob2]

Hi,
ich hab mich heut zum ersten mal mit PWM beschäftigt und wollte daher eine LED auf- und abdimmen (langsam blinken). Dazu habe ich nach dem Tutorial bei mikrocontroller.net folgenden Code geschrieben:


#include <avr/io.h>
#ifndef F_CPU
/* Definiere F_CPU, wenn F_CPU nicht bereits vorher definiert
(z.B. durch Übergabe als Parameter zum Compiler innerhalb
des Makefiles). Zusätzlich Ausgabe einer Warnung, die auf die
"nachträgliche" Definition hinweist */
#warning "F_CPU war noch nicht definiert, wird nun mit 1 MHz definiert"
#define F_CPU 1000000UL /* Quarz mit 1.0000 Mhz */
#endif
#include <util/delay.h>


int main (void)
{
uint8_t vergleichswert = 0;
DDRB |= (1<<PB1); //PB1 auf Ausgang
TCCR1A = 0;
TCCR1A |= (1<<WGM10) | (1<<COM1A1); //Nichtinvertierende PWM
TCCR1B = 0;
TCCR1B = (1<<CS12) | (1<<CS10); //Takt CK/1024
while(1)
{
while (vergleichswert < 255) //LED heller (Vergleichswert größer)
{
OCR1A = vergleichswert;
vergleichswert++;
_delay_ms(1); // warten, damit es nicht rasend geht
}
while(vergleichswert > 0) //LED dunkler (Vergleichswert kleiner)
{
OCR1A = vergleichswert;
vergleichswert--;
_delay_ms(1);
}
}
}


Die LED blinkt aber völlig willkürlich herum. Ich denke, dass das an falschen Werten bei Takt, Obergrenze oder so liegt. Da ich da eben keine Erfahrung habe, kann mir jemand Werte nennen, bei denen eine LED (ist mit minus am Mikrocontroller angeschlossen) einigermaßen flüssig auf- und abgedimmt wird? Damit ich erstmal einen Ausgangspunkt habe, an dem ich dann weiter rumspielen kann...
Oder gibt es Fehler im Code???

[sternst]

Die PWM-Frequenz ist viel zu niedrig, lass erstmal den Vorteiler weg.
Auf der anderen Seite ist die Zeit zwischen den einzelnen Dimmschritten zu kurz, so siehst du ja kaum was. Ändere die Delays mal in 10 ms.

[lowtzow]

hallo

versuchs mal so, du hast einen zugriff auf das 16bit OCR1A Register gemacht, hast jedoch nur ein 8bit pwm auch deine variable lässt auf 8 bit schließen (uint8_t vergleichswert)
mit OCR1L, kannst du nur auf die ersten 8Bit zugreiffen, sollte so funktionieren ;-)

zu den 1ms delay, die könnte man auch in einer ISR mit einem 1ms timer verbauen

mfg
low





#include <avr/io.h>
#ifndef F_CPU
/* Definiere F_CPU, wenn F_CPU nicht bereits vorher definiert
(z.B. durch Übergabe als Parameter zum Compiler innerhalb
des Makefiles). Zusätzlich Ausgabe einer Warnung, die auf die
"nachträgliche" Definition hinweist */
#warning "F_CPU war noch nicht definiert, wird nun mit 1 MHz definiert"
#define F_CPU 1000000UL /* Quarz mit 1.0000 Mhz */
#endif
#include <util/delay.h>


int main (void)
{
uint8_t vergleichswert = 0;
DDRB |= (1<<PB1); //PB1 auf Ausgang
TCCR1A = 0;
TCCR1A |= (1<<WGM10) | (1<<COM1A1); //Nichtinvertierende PWM
TCCR1B = 0;
TCCR1B = (1<<CS12) | (1<<CS10); //Takt CK/1024
while(1)
{
while (vergleichswert < 255) //LED heller (Vergleichswert größer)
{
OCR1L = vergleichswert;
vergleichswert++;
_delay_ms(1); // warten, damit es nicht rasend geht
}
while(vergleichswert > 0) //LED dunkler (Vergleichswert kleiner)
{
OCR1L = vergleichswert;
vergleichswert--;
_delay_ms(1);
}
}
}

[sternst]

Mit dem Zugriff auf das OCR-Register hat das nichts zu tun. Die PWM-Frequenz ist einfach viel zu niedrig. Mit einem Systemtakt von 1MHz und einem Vorteiler von 1024 beträgt die gerade mal 3,8Hz. Klar, dass das nichts mit "Helligkeit" zu tun hat, sondern als "wildes Blinken" wahrgenommen wird.

Und nur das Low-Byte zu beschreiben, ist eine ziemlich gefährliche Sache (und damit ein reichlich schlechter Tipp), denn damit wird nämlich auch der Inhalt des temporären High-Bytes geschrieben. Und wenn man da selber nichts rein geschrieben hat, wird das ins OCR-High-Byte geschrieben, was da halt noch vom letzten 16-Bit-Zugriff (z.B. Lesen des Zählers) drinsteht.

[lowtzow]

ja stimmt, ein teiler 8 solle reichen oder keinen.

ich würde trozdem nicht einer 8bit zahl eine 16bit übergeben, vergleichen (wie auch immer), wer bei so kleinen sachen sowas macht kann sich bei großen projekten dumm und dämlich suchen

[sternst]

ich würde trozdem nicht einer 8bit zahl eine 16bit übergeben, vergleichen (wie auch immer), wer bei so kleinen sachen sowas macht kann sich bei großen projekten dumm und dämlich suchenMan muss es hier aber machen, damit das temporäre High-Byte-Register genullt wird. Wenn man es ganz korrekt machen will, macht man halt noch einen Cast dazu.
OCR1 = (uint16_t)vergleichswert;

[Ceos]

in dem falle hast du recht, da er explizit in das lowbyte schreibt, es reicht wenn man einfach nur OCR1 schreibt, DANN muss man nicht casten! wenn ich einen 8bit wert in einen 16bit wert speichere werden durch den compiler BEIDE variablen als 16bit wert betrachtet und die 8bit variable mit 0en aufgefüllt!

aber allein schon die abfrage if(vergleichwert < 255) ist IMMER wahr, da eine 8bit zahl nie größer werden kann

[sast]

Ich dachte immer 255<255 ist falsch ;o)

sast

[sternst]

in dem falle hast du recht, da er explizit in das lowbyte schreibt, es reicht wenn man einfach nur OCR1 schreibt, DANN muss man nicht casten! wenn ich einen 8bit wert in einen 16bit wert speichere werden durch den compiler BEIDE variablen als 16bit wert betrachtet und die 8bit variable mit 0en aufgefüllt!Das "muss" bezog sich auch nicht auf den Cast, sondern auf das Beschreiben des kompletten 16-Bit-Registers.

[Jacob2]

aber allein schon die abfrage if(vergleichwert < 255) ist IMMER wahr, da eine 8bit zahl nie größer werden kann
größer nicht, aber gleich groß oder?


Leider konnte ich aus eurer Diskussion nicht ganz heraushören, wie das mit dem 16bit/8bit geändert werden muss...

Einfach "vergleichswert" als 16bit vereinbaren?

Edit: Es funktioniert jetzt aber eigentlich relativ gut... nur an der Stelle wo die while(1) Schleife wieder von vorne anfängt, bzw. wenn die LED am hellsten ist (Kommentare im obigen Post stimmen nicht, erst dunkler, dann heller) muss irgendwoher ein etwas längerer high Impuls kommen, denn die LED wird sehr kurz dunkel! woran kann das liegen?

[lowtzow]

ich würde trozdem nicht einer 8bit zahl eine 16bit übergeben, vergleichen (wie auch immer), wer bei so kleinen sachen sowas macht kann sich bei großen projekten dumm und dämlich suchenMan muss es hier aber machen, damit das temporäre High-Byte-Register genullt wird. Wenn man es ganz korrekt machen will, macht man halt noch einen Cast dazu.
OCR1 = (uint16_t)vergleichswert;

man kann auch aufs OCR1L zugreiffen, was bei einem 8bit pwm ohne probleme funktioniert! und was über den 8bit is wurscht, weil weiter läuft ein 8bit pwm eh nicht!

[Ceos]

[quote]Ich dachte immer 255<255 ist falsch ;o) [quote] arrgh recht haste ich hab jetzt iwie aus 255 in meinen augen 256 gemacht :p

du musst das ganze register beschreiben wie sternst schon gesagt hat also statt OCR1AL einfach OCR1A schreiben, ich würde dennoch dazu raten den vergleichswert 16bit groß zu machen

dann musst du noch den prescaler runtersetzen, höchstens 64 erstmal, theoretisch müsste deine LED 255ms lang immer dunkler werden und dann 255ms lang wieder heller, also 2 mal in der sekunde hel und dunkel werden