Timerinterrupt kommt nur einmal

[Michael R. (zeroeightfifteen)]

Hallo
ich will mit dem Timer1 ca alle 4 s einen Interrupt auslösen.
doch der Interrupt kommt nur einmal am anfang und dann nie wieder. was 
habe ich da falsch gemacht? muss ich den Timer wieder manuell auf 0 
setzen?

#include <stdlib.h>
#include <avr/io.h>
#include <avr/interrupt.h>
#include <avr/signal.h>
#include <avr/pgmspace.h>

#include "uart.h"

#ifndef F_CPU
#define F_CPU 16000000UL
#endif

#define UART_BAUD_RATE      19200

ISR( TIMER1_OVF_vect );

int main(void)
{

    TCCR1A = 0x00;
    TCCR1B = 0x1d; // Prescaler 1024

   TIMSK = ( 1 << TOIE1 );         // Overflow Interrupt einschalten

  DDRB = 0x00;
  PORTB = 0xff;
    DDRC = 0x00;
  PORTC = 0xff;
    DDRD |= (1<<PD7);

uart_init( UART_BAUD_SELECT(UART_BAUD_RATE,F_CPU) );


sei();

for(;;)
  {

  }


}


ISR( TIMER1_OVF_vect )
{

if ( PORTD & (1<<PORTB7) ) PORTD &= ~ (1<<PORTD7);    //Ausgang toggeln 
um interrupt zu sehen
else PORTD |= (1<<PD7);

unsigned char Tasterstellung = 0;            //Tastenstellung auf 0 
setzen

if ( PINB & (1<<PINB0) ) Tasterstellung |= (1<<0);
else Tasterstellung &= ~(1 << 0);

if ( PINB & (1<<PINB1) ) Tasterstellung |= (1<<1);
else Tasterstellung &= ~(1 << 1);

if ( PINB & (1<<PINB2) ) Tasterstellung |= (1<<2);
else Tasterstellung &= ~(1 << 2);

if ( PINB & (1<<PINB3) ) Tasterstellung |= (1<<3);
else Tasterstellung &= ~(1 << 3);

if ( PINB & (1<<PINB4) ) Tasterstellung |= (1<<4);
else Tasterstellung &= ~(1 << 4);

if ( PINB & (1<<PINB5) ) Tasterstellung |= (1<<5);
else Tasterstellung &= ~(1 << 5);

if ( PINC & (1<<PINC0) ) Tasterstellung |= (1<<6);
else Tasterstellung &= ~(1 << 6);

if ( PINC & (1<<PINC1) ) Tasterstellung |= (1<<7);
else Tasterstellung &= ~(1 << 7);

uart_putc((unsigned char)Tasterstellung );
}

[Matthias (Gast)]

Erstmal hast du nen tippfehhler in der Zeile, wo du den Ausgang 
toogelst.
Das stört aber soweit nicht..
Ich würde dir raten, diese Zeile (zum Ausgang toggeln) mal direkt UNTER 
die zeile mit dem uart_putc.. zu setzten.
Weil, abgesehen das man Tastenabfragen anders realisiert, kann ich 
soweit nix feststellen. Weil somit siehst du ob der µC die ISR komplett 
durchläuft..
Weil ich denke das der ISR garnicht beendet wird (anhand deiner 
Erklärung)

[Malte _. (malte)]

> muss ich den Timer wieder manuell auf 0 setzen?
Ja. Manche Timer eines AVRs (zB Timer2 beim ATMEGA8) besitzen aber die 
Möglichkeit dies automatisch zu machen (dafür muss dann ein passendes 
Bit gesetzt werden).

[Karl H. (kbuchegg)]

Und nimm mal den Prototypen für die ISR raus.
ISR Funktionen werden vom Compiler sowieso anders
behandelt. Die ganze ISR Geschichte ist sowieso sehr fragil,
ich könnte mir vorstellen, dass das den Compiler 'verwirrt' hat.

[johnny.m (Gast)]

> if ( PORTD & (1<<PORTB7) ) PORTD &= ~ (1<<PORTD7);    //Ausgang toggeln
> um interrupt zu sehen
> else PORTD |= (1<<PD7);
Sowas macht man (abgesehen von den oben bereits angesprochenen Fehlern) 
mit Exklusiv-ODER:

PORTD ^= 1 << PORTD7;

[johnny.m (Gast)]

Übrigens:

Tasterstellung = (PINB & 0x3F) | (PINC << 6);

sollte genau das machen, was in Deiner ISR der ganze if-else-Kram macht, 
und das in nur einer einzigen Zeile... (nicht getestet, deshalb ohne 
Gew(a)ehr...)

Und zu der Anmerkung von Karl Heinz:
Prototypen braucht man nur dann, wenn eine Funktion vor ihrer Definition 
bereits aufgerufen werden können soll. Da ISRs grundsätzlich nicht aus 
dem Programm heraus aufrufbar sind, brauchts da auch keinen Prototypen. 
Ich habe jetzt nicht ausprobiert, was der Compiler in Deinem Fall draus 
machen würde, aber es besteht zumindest die Möglichkeit, dass er den 
Prototypen als leere ISR deutet und diese auch einsetzt, was dazu führen 
würde, dass bei Auftreten eines Interrupts gar nichts passiert.

[Michael R. (zeroeightfifteen)]

Danke schon mal für eure Hilfe. was ist denn an ISR so schlecht. ich 
habe mir schon mehrere Beispiele angesehen, die auch ISR verwenden. Wie 
kann ich das sonst realisieren?

Das mit der Tasterstellung habe ich auch noch nicht ganz verstanden. Ich 
will ja jedes Bit dieses Chars einzelln mit jedem Taster setzten oder 
rücksetzen.

[johnny.m (Gast)]

> was ist denn an ISR so schlecht.
Wer sagt denn, dass an ISR irgendwas schlecht ist? Was meinst Du damit? 
Ich habe Dir nur eine Möglichkeit gezeigt, wie Du die ganzen 
if-else-Abfragen in eine einzige Codezeile packen kannst und dadurch 
Dein Programm kürzer, effizienter und übersichtlicher wird.

Falls Du meinen Vorschlag nicht verstanden hast, hier noch mal ne kurze 
Erklärung:
Du hast anscheinend Taster an PortB 5...0 und an PortC 1..0 hängen und 
willst die Zustände dieser Taster in einer Variablen speichern und 
ausgeben. Hier mal die beiden Ports mit den Tastern:

        7   6   5   4   3   2   1   0
PortB   X   X   T5  T4  T3  T2  T1  T0

PortC   X   X   X   X   X   X   T7  T6

Sind summasummarum 8 Taster, habe sie zwecks Anschaulichkeit schon mal 
durchnummeriert, so wie sie hinterher auch in Deiner Varibale sortiert 
sind. Die Xe kennzeichnen Bits, die anderweitig belegt sind und uns für 
die Auswertung nicht interessieren. Wenn Du die Zustände der einzelnen 
Bits in einer Variable speichern möchtest, kannst Du natürlich in einer 
elend langen if-else-Abfrage jeden Taster einzeln überprüfen und das 
entsprechende Bit in der Variable setzen oder löschen, wie Du es ja auch 
oben gemacht hast. Das ist aber erstens fürchterlich viel Code, zweitens 
geht es wesentlich einfacher und kürzer. Wenn Du die Register PINC und 
PINB einliest und den Inhalt von PINC um 6 Bit-Stellen nach links 
schiebst, steht da folgendes (PortB und PortC sind hier nur Platzhalter 
und haben nichts mit den Registern PORTB und PORTC zu tun!):

        7   6   5   4   3   2   1   0
PortB   X   X   T5  T4  T3  T2  T1  T0

PortC   T7  T6  0   0   0   0   0   0

Durch das Linksschieben werden in der unteren Zeile von hinten Nullen 
eingefügt. Wenn Du jetzt noch dafür sorgst, dass bei den beiden nicht 
benutzten Bits von PortB ebenfalls Nullen stehen, nämlich indem Du die 
Bits, die eigentlich interessieren, mit einer bitweisen UND-Verknüpfung 
("&") maskierst, kommt das raus:

        7   6   5   4   3   2   1   0
PortB   X   X   T5  T4  T3  T2  T1  T0
  &     0   0   1   1   1   1   1   1     (0x3F)
---------------------------------------
        0   0   T5  T4  T3  T2  T1  T0

Jetzt kannst Du die beiden Teilausdrücke mit einem bitweisen ODER ("|") 
verknüpfen und erhältst als Ergebnis

        0   0   T5  T4  T3  T2  T1  T0
   |    T7  T6  0   0   0   0   0   0
----------------------------------------
        T7  T6  T5  T4  T3  T2  T1  T0

Also alle Tasterzustände in 8 Bit, schön in Reih und Glied 
hintereinander. Damit hast Du 8 if-else-Abfragen gespart...

[Karl H. (kbuchegg)]

> was ist denn an ISR so schlecht

Gar nichts.

> Wie kann ich das sonst realisieren?

Das ist schon ok.
Nur implementieren musst du es noch richtig.

Ohne getestet zu haben:

#include <stdlib.h>
#include <avr/io.h>
#include <avr/interrupt.h>
#include <avr/signal.h>
#include <avr/pgmspace.h>

#include "uart.h"

#ifndef F_CPU
#define F_CPU 16000000UL
#endif

#define UART_BAUD_RATE      19200

ISR( TIMER1_OVF_vect )
{
  unsigned char Tasterstellung = 0;      // Tastenstellung auf 0 setzen
  unsigned char i;

  PORTD ^= 1 << PD7;

  // Deine tasterauswertung ist so nicht grundsätzlich falsch.
  // Nur: so ist das kürzer 

  Tasterstellung = ( PINB & 0b00111111 ) |
                   ( ( PINC & 0b00000011 ) << 6 );

  uart_putc( Tasterstellung );
}

int main(void)
{
  TCCR1A = 0x00;
  TCCR1B = 0x1d; // Prescaler 1024

  TIMSK = ( 1 << TOIE1 );         // Overflow Interrupt einschalten

  DDRB = 0x00;
  PORTB = 0xff;
  DDRC = 0x00;
  PORTC = 0xff;
  DDRD |= (1<<PD7);

  uart_init( UART_BAUD_SELECT(UART_BAUD_RATE,F_CPU) );

  sei();

  for(;;)
  {
  }
}

Probiers mal so rum.

Der uart_putc in der ISR schmeckt mir gar nicht. Auf der anderen
Seite hast du 1024 mal 65536 Zyklen für eine Ausgabe Zeit.
Das sollte dicke reichen. Bei 16 Mhz sind das immerhin knapp
4 Sekunden.

[johnny.m (Gast)]

Hier der Vollständigkeit halber noch mal Deine "neue" ISR mit den beiden 
Änderungsvorschlägen von mir:

ISR(TIMER1_OVF_vect)
{
    unsigned char Tasterstellung;
    PORTD ^= 1 << PORTD7; //PortD.7 toggeln
    Tasterstellung = (PINB & 0x3F) | (PINC << 6); //Taster einlesen
}

Gibs zu: Es ist ein bisschen kürzer, als Deine Originalversion. Und das 
Beste: Es tut genau das selbe!

[Michael R. (zeroeightfifteen)]

Danke nochmal. Meine Variante war doch ein bisschen zu lange. So ist es 
sicher besser und auch übersichtlicher.
Aber das mit der Implementierung habe ich doch alles gemacht oder nicht? 
Der Timer läuft nun auch.  Ich habe statt
TCCR1A = 0x00;
TCCR1B = 0x1d; // Prescaler 1024
dies hier geschrieben:

TCCR1B |= (1<<CS12)|(1<<CS10);


Nun muss ich die Leds wieder auf PortB und C darstellen aber das schaffe 
ich heute noch.