Zu schnelles Blinken

[Alex (Gast)]

Hallo zusammen. Für einen Test einer neuen Schaltung wollte ich ein 
Programm schreiben, welches im Sekundentakt eine LED blinken lässt.
Doch die LED flackert nur so, anstatt im Sekundentakt zu blinken.
Ich habe dem Timer1 das Comparematch Interrupt programmiert und lasse 
ihn beim gewünschten Zählerstand zurücksetzen. Das Interrupt lässt die 
LED blinken.
Den Wert habe ich wie folgt gerechnet für das OCR1A 
(Vergleichsverzeichnis):
1'000'000 (FCPU) / 256 (Prescaler) = 3900

Was mache ich falsch?

#ifndef F_CPU
#define F_CPU 1000000
#endif
#define DDRLED DDRC
#define PORTLED PORTC
#define LED PC1
#define COMPAREAT 3900

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

int main(void)
{
  cli();
  DDRLED |= 1 << LED;
  PORTLED |= 1 << LED;
  
  TCCR1A = 0x00;
  TCCR1B = (1 << CS12) | (1 << WGM12);
  TIMSK = (1 << OCIE1A);
  OCR1AH = (uint8_t)(8 >> COMPAREAT);
  OCR1AL = (uint8_t) COMPAREAT;
  sei();
  
    while(1)
    {
        
    }
  return 0;
}

ISR (TIMER1_COMPA_vect)
{
  if (PORTLED & (1 << LED))
  {
    PORTLED &= ~(1 << LED);
  }
  else
  {
    PORTLED |= 1 << LED;
  }
}

Gruss Alex

[Alex (Gast)]

Danke für die schnelle Hilfe^^ bin selbst auf den Fehler gekommen :)

OCR1AH = (uint8_t)(8 >> COMPAREAT);

muss natürlich:

OCR1AH = (uint8_t)(COMPAREAT >> 8);

lauten

Gruss Alex

[Karl H. (kbuchegg)]

Jetzt hast du gesehen, was da alles passieren kann und in Zukunft 
schreibst du ganz einfach

  OCR1A = COMPAREAT;

Es gibt für dich keinen Grund, dich um so "lästige" Details wie das 
Aufteilen der Bytes an dieser Stelle kümmern zu müssen, wenn der 
Compiler das genausogut für dich erledigen kann.

PS: der gcc hat für alle Registerpärchen, die aus jeweils einem H und 
einem L Register bestehen, eine entsprechende Definition mit. Lass 
einfach immer den Buchstaben H bzw. L weg und tu so als ob das ein 16 
Bit Register wäre an das du ganz einfach zugreifst bzw. von dem du ganz 
einfach liest. Den Rest macht der Compiler.

[Alex (Gast)]

Das ist für jeden Fall wirklich gültig???

Wieso ist davon im Avr-Gcc Tutorial nichts vermerkt?
http://www.mikrocontroller.net/articles/AVR-GCC-Tutorial#16-Bit_Register_.28ADC.2C_ICR1.2C_OCR1x.2C_TCNT1.2C_UBRR.29


Da steht sogar, dass man es einzeln behandeln MUSS.

Bei anderen Registern, wie zum Beispiel Baudraten-Register, liegen High- 
und Low-Teil nicht direkt nebeneinander im SFR-Bereich, so dass ein 
16-Bit Zugriff nicht möglich ist und der Zugriff zusammengebastelt 
werden muss:


Ist das dann neu? Kann ich in Zukunft also darauf verzichten?
Und wie sieht es mit Lesezugriffen aus? Das selbe?
Und beim Zugriff auf manche 16Bit Register sollte man ja die Interrupts 
deaktivieren. Da muss man sich dann trotzdem noch drum kümmern? In der 
Art:
cli();
temp = OCR1A;
sei();

Gruss Alex

[Karl H. (kbuchegg)]

Alex schrieb:

> Da steht sogar, dass man es einzeln behandeln MUSS.

Erwischt. An das Baudratenregister hab ich nicht mehr gedacht.

> Und wie sieht es mit Lesezugriffen aus? Das selbe?

Jep.

> Und beim Zugriff auf manche 16Bit Register sollte man ja die Interrupts
> deaktivieren. Da muss man sich dann trotzdem noch drum kümmern?

Ja. Das hat sich nicht verändert. Wenn eine Chance besteht, dass ein 
Register in einer ISR verändert werden kann, dann gilt genau das gleiche 
wie für jede andere 16-Bit Variable: Um atomares Lesen zu garantieren 
muss das Auslesen unter Interrupt Sperre erfolgen.

> In der
> Art:
> cli();
> temp = OCR1A;
> sei();


Die ATOMIC_xx Makros sind eleganter
http://www.nongnu.org/avr-libc/user-manual/group__util__atomic.html