PWM funktion OCR1 atmega328p

[Robin G. (gawin)]

Hi leute,
ich brauchte PWM und da ich mich noch nicht so gut damit auskannte, hab 
ich erst mal nen paar tut's gelesen und ne Funktion für den Allgemeinen 
gebrauch geschrieben. Klappt auch alles super nur kommt mir die funktion 
nen bisschen aufgebläht vor und da dachte ich mir, vllt könnte mir 
jemand nen paar Tipps geben um die Funktion zu verkleinern?
Die Funktion ist im anhang.

Danke schonmal ;)
Gruß Robin

edit: seh gerade hab beim if/else "prescaler" einmal noch 64 statt 1024, 
kleiner copy/past fehler halt ;)

[Stefan W. (dl6dx)]

Hi Robin,

ich hab die Funktion mal etwas umgebaut:

void OCR1_PWM(boolean B,
        boolean inverted,
        boolean fast,
        uint8_t bit,
        uint16_t prescaler,
        uint16_t value)
  {
  if(!B)
    {
    DDRB |= (1 << DDB1);
    OCR1A = value;
    TCCR1A |= inverted ? (1 << COM1A1) | (1 << COM1A0) : (1 << COM1A1);
    }
  else
    {
    DDRB |= (1 << DDB2);
    OCR1B = value;
    TCCR1A |= inverted ? (1 << COM1B1) | (1 << COM1B0) : (1 << COM1B1) ;
    }

  switch(bit)
    {
    case 8:
      TCCR1A |= (1 << WGM10);
      break;
    case 9:
      TCCR1A |= (1 << WGM11);
      break;
    case 10;
      TCCR1A |= (1 << WGM11) | (1 << WGM10);
      break;
    default:
      // hier ggf. eine Fehlerbehandlung für ungültige Parameter einbauen
      break;
    }
    
  if(fast)
    {
    TCCR1A |= (1 << WGM12);
    }

  switch(prescaler)
    {
    case 0;
      TCCR1B |= (1 << CS10);
      break;
    case 8:
      TCCR1B |= (1 << CS11);
      break;
    case 64:
      TCCR1B |= (1 << CS11) | (1 << CS10);
      break;
    case 256:
      TCCR1B |= (1 << CS12);
      break;
    case 1024:
      TCCR1B |= (1 << CS12) | (1 << CS10);
      break;
    default:
      // hier ggf. eine Fehlerbehandlung für ungültige Parameter einbauen
      break;
    }
  }

Der Code sollte nach diff-vergleich äquivalent sein. Falls du Lust hast, 
das mal zu kompilieren, würde mich der Codeumfang im Vergleich 
interessieren.

Grüße

Stefan

[holger (Gast)]

Das mit den ganzen |= ist gefährlich. Bereits gesetzte
Bits werden so nicht mehr zurückgesetzt.

[Robin G. (gawin)]

@holger: Wie gehts denn besser?

@Stefan: hab es mal kompiliert und braucht in folgendem beispielt 22byte 
weniger ;)

int main(void){
  DDRC |= 0x03;
  int i;
  while(1){
    for(i=1;i<=63;i+=1){
      OCR1_PWM(0, 0, 0, 10, 64, i);
      _delay_ms(40);
    }
    for(i=63;i>=1;i-=1){
      OCR1_PWM(0, 0, 0, 10, 64, i);
      _delay_ms(40);
    }
  }
  return 0;
}

[Karl H. (kbuchegg)]

Robin Gerhartz schrieb:
> @holger: Wie gehts denn besser?

Indem du auch die jeweils relevanten Bits auf 0 setzt und nicht nur 
diejenigen auf 1, die du brauchst.

Oder auf Deutsch: Du hast da eine Annahme getroffen. Nämlich die, das 
die anderen Bits automatisch 0 sind. Zb. beim Prescaler. Ist als 
Prescaler 8 angegeben, dann setzt du CS11. Aber was ist mit den anderen 
Bits? In einer allgemein verwendbaren Funktion darfst du nicht davon 
ausgehen, dass diese automatisch 0 wären. Was ist wenn du die Funktion 2 
mal mit verschiedenen Prescalern aufrufst? Passiert dann immer noch das 
richtige?

  switch(prescaler)
    {
    ...


    case 8:
      TCCR1B &= ~( ( 1 << CS12 ) | ( 1 << CS10 ) );
      TCCR1B |= (1 << CS11);
      break;

    ....

jetzt tut es das.

Auch so

  TCCR1B &= ~( ( 1 << CS12 ) | ( 1 << CS11 ) ( 1 << CS10 ) );

  switch(prescaler)
    {
    ...


    case 8:
      TCCR1B |= (1 << CS11);
      break;

    ....

tut es das, ohne das du dich bei den Einzelfällen damit abquälen musst.

Wenn man einmal weiter denkt, dann wäre es vernünftig
* die 2. Variante zu benutzen
* das Rücksetzen der Prescaler Bits ganz am Anfang der Funktion zu 
machen.

Denn: Wenn der Timer bereits läuft und eine PWM generiert, dann ist 
nicht vorhersehbar was passieren wird, wenn man dem bereits laufenden 
Timer die PWM während des Betriebs umkonfiguriert.

Also

void OCR1_PWM(boolean B,
        boolean inverted,
        boolean fast,
        uint8_t bit,
        uint16_t prescaler,
        uint16_t value)
{
  TCCR1B &= ~( ( 1 << CS12 ) | ( 1 << CS11 ) ( 1 << CS10 ) );

  ....

So ist sicher gestellt, dass
* die Bits auf jeden Fall 0 sind
* der Timer angehalten ist, so dass man relativ gefahrlos die
  PWM Einstellung ändern darf.

[Robin G. (gawin)]

Gut hab die Bits vorher mal alle zurück gesetzt.
Und mir is noch auf gefallen das ich den Bit WGM12 im falschen Register 
setzen wollte, wurde auch korrigiert. Es braucht im folgendem Beispiel 
laut AVRDude auf dem µc 502bytes.

#include <avr/io.h>
#include <util/delay.h>
#include <m328p/OCR1.c>

int main(void){
  int i;
  while(1){
    for(i=1;i<=63;i+=1){
      OCR1_PWM(0, 0, 1, 10, 64, i);
      _delay_ms(40);
    }
    for(i=63;i>=1;i-=1){
      OCR1_PWM(0, 0, 0, 10, 64, i);
      _delay_ms(40);
    }
  }
  return 0;
}

Geht da noch was?

Gruß Robin

[Robin G. (gawin)]

So hab auch mal für die beiden anderen Timer auf dem Atmega328p ne 
Funktion geschrieben und default überall "8Bit" oder "ohne Prescaler" 
eingestellt.

Meine frage jetzt sind die so in Ordnung oder sollte ich noch was 
verändern?

Bespiel:

#include <avr/io.h>
#include <util/delay.h>
// #include <m328p/OCR0.c>
// #include <m328p/OCR1.c>
#include <m328p/OCR2.c>

int main(void){
  int i;
  while(1){
    for(i=10;i<=70;i+=2){
      OCR2_PWM(0, 0, 1, 32, i);
      _delay_ms(33);
    }
    for(i=70;i>=10;i-=2){
      OCR2_PWM(0, 0, 1, 32, i);
      _delay_ms(33);
    }
  }
  return 0;
}

OCR0 - avrdude: 398 bytes of flash verified
OCR1 - avrdude: 498 bytes of flash verified
OCR2 - avrdude: 470 bytes of flash verified