Probleme mit externem Interrupt

[onion]

Hallo,
wer kann mir sagen warum mein kleines Testprogramm mit der auskommentierten Zeile 53 statt Zeile 54 anders Läuft. (Zeile mit --> <-- markiert)

Was ich eigentlich erreichen wollte:
5 Mal blinken dann in Power Down Sleep Mode gehen. Aufwecken durch Tastendruck/externen Interrupt und alles wieder von vorne.

Irgendwie macht der externe Interrupt nie so genau was ich eigentlich will.

So hier der Code:

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

#define TIMER1_END_VALUE 1
#define TIME_TILL_SLEEP 5

uint8_t volatile sleep_count;

ISR(TIMER1_COMPA_vect);
void init(void);

int main(void)
{
init();
sei();
set_sleep_mode(SLEEP_MODE_PWR_DOWN); //Sleep Mode setzen

while (1)
{
if(sleep_count == TIME_TILL_SLEEP)
{
sleep_count = 0;
sleep_mode(); //einschlafen
}
}

return 0;
}

ISR(TIMER1_COMPA_vect)
{
static uint16_t run_led_active_count = 0;

run_led_active_count++;
if(run_led_active_count == 900)
{
PORTD = (1<<PIND3);
}
if(run_led_active_count >= 1000)
{
run_led_active_count = 0;
sleep_count++;
PORTD = 0x00;
}
}

ISR(PCINT_vect)
{
//Interrupt Mask Register setzen für externen Interrupt

//--> GIMSK |= ~(1 << PCIE); <--
GIMSK = 0x00;

GIMSK |= (1 << PCIE);
PORTD ^= (1 << PD0);
}

void init()
{
//Datenrichtung für die Ausgänge
DDRD = (1 << DDD0) | (1 << DDD1) | (1 << DDD2) | (1 << DDD3) | (1 << DDD4) | (1 << DDD5) | (1 << DDD6);
DDRB = (1 << DDB0);

//Setup Timer 1
OCR1A = TIMER1_END_VALUE; //Compare Value
TCCR1B = (1<<WGM12); //CTC Mode
TIMSK |= (1<<OCIE1A); //Output Compare A Match Interrupt Enable
TCCR1B |= (1<<CS12) | (1<<CS10); //Timer1 aktivieren

//setup external interrupts
PCMSK |= (1 << PCINT2) | (1 << PCINT3) | (1 << PCINT4);
GIMSK |= (1 << PCIE);
}

[onion]

Ok, ich glaube das muss GIMSK &= ~(1 << PCIE); heißen statt GIMSK |= ~(1 << PCIE);
Verstehe nur grad nicht wieso man mit &= einzelne bits löschen kann ...

[vklaffehn]

Moin!
Um ein bit zu löschen, reicht '&=' nicht ganz aus, Sinn macht das erst mit dem ~(1<<PCIE), das erste macht eine Und-verknüpfung, mit dem zweiten setzt man ein Bit und invertiert quasi das Ergebnis, so daß man also eine UND-Verknüpfung mit einem gelöschten Bit macht, da nun aber in einem Operanden das entsprechende Bit nicht gesetzt ist, kommt dort immer 0 raus. beim '|=' (bitweises Oder) würde hier, wenn das Bit nur in einem Operand, in dem Fall evtl. auch in GIMSK schon vorher gesetzt ist, eine 1 rauskommen, das Bit bliebe also gesetzt...


MfG
Volker

[onion]

Ah, ok. Mir war/ist nur nicht ganz klar dass erst das AND kommt und dann das NOT.
Achja man lernt doch nie aus.

[vklaffehn]

Moin!
Naja, eigentlich kommt erst das NOT und dann das AND :-)
Wenns nicht schon so spät wäre, würd ich mir jetzt noch überlegen, ob das eigentlich einen Unterschied macht ... :-)
MfG

[onion]

Hm, mal überlegen:

GIMSK &= ~(1 << PCIE);

entspricht meiner Meinung nach nun

GIMSK = ~( GIMSK & (1 << PCIE) );

und nicht

GIMSK = GIMSK & ( ~(1 << PCIE) );

Ok, jetzt erklär mir noch mal wie du das meinst, erst kommt das NOT und dann das AND.

[sternst]

GIMSK &= ~(1 << PCIE);
ist
GIMSK = GIMSK & ( ~(1 << PCIE) );

Definitiv!

[onion]

Jo, heute morgen fiel mir das dann auch ein. War wohl doch etwas zu spät gestern...

Das funktioniert ja deshalb weil ja quasi die bits von dem ganzen Wort gekippt werden und nicht nur das eine geshiftete bit, also NOT(00100000) --> 11011111
und dann wird es verundet. Ist ja klar, dass dann das entsprechende bit gelöscht wird.
Ok, trotzdem vielen Dank für eure Antworten.

[onion]

So, hab jetzt noch mal ein konkretes Problem mit dem externen Interrupt.
Und zwar funktioniert der irgendwie nur jedes zweite Mal korrekt. Bzw. wenn ich den benutze um den AVR wieder aufzuwecken geht das nur einmal. Beim zweiten Mal schläft er dann für immer.
Woran kann das jetzt liegen?
Hier mal mein dazugehöriger Code. Das ganze soll mal ne Eieruhr werden


/*
* Folgender Code ist für eine Eieruhr basierend auf einem
* Atmega tiny2313. Der Code darf frei weitergegeben verändert
* zerstückelt oder sonst wie von jedem verwendet werden.
* Über eine kurz Email (jsemail@gmx.de) würde ich mich freuen.
*/

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

#define F_CPU 1000000 // Clock in Hz
#include <util/delay.h>

#define TIMER1_END_VALUE 1 //Values for Timer Compare Match
#define TIMER0_END_VALUE 5
#define TIME_TILL_PWR_DOWN 10 //Time in blinks till going to Power Down Mode

//*** global variables ***
//global state
uint8_t volatile current_time = 1;
uint8_t volatile time_set = 1;

enum global_state {STATE_SLEEP, STATE_SET, STATE_RUN};
enum global_state volatile current_state = STATE_SET;
enum global_state volatile new_state = STATE_SET;


//function prototypes
void init(void);
void change_state(void);

int main(void)
{
init();

//Haupt(endlos)schleife
while (1)
{
if(new_state != current_state)
{
change_state();
}
sleep_mode(); //einschlafen
}
return 0;
}

ISR(TIMER0_COMPA_vect)
{
static uint8_t debounced_button_state; // Debounced state of the switches
static uint8_t button_plus_state; // for button debouncing
static uint8_t button_minus_state;
static uint8_t button_start_state;
uint8_t debounced_button_state_old;
uint8_t button_press; // bit 1 on rising edge (button down)
static uint8_t set_led_active_count;
static uint8_t pwr_down_count;

//read button states
button_plus_state = (button_plus_state<<1) | ((PINB & (1<<PINB2))>>2);
button_minus_state = (button_minus_state<<1) | ((PINB & (1<<PINB3))>>2);
button_start_state = (button_start_state<<1) | ((PINB & (1<<PINB4))>>2);

//run debounce algorithm and check for rising edge
debounced_button_state_old = debounced_button_state;
debounced_button_state = ((button_plus_state && 0x0F)<<PINB2) | ((button_minus_state && 0x0F)<<PINB3) | ((button_start_state && 0x0F)<<PINB4);
button_press = (debounced_button_state ^ debounced_button_state_old) & debounced_button_state;

if(debounced_button_state == ((1<<PINB2) | (1<<PINB3)))
{
time_set = 1;
}

set_led_active_count++;
if(set_led_active_count == 50)
{
PORTD = time_set;
}
else
{
if(set_led_active_count >= 80)
{
PORTD = 0x00;
set_led_active_count = 0;
pwr_down_count++;
if(pwr_down_count == TIME_TILL_PWR_DOWN)
{
pwr_down_count = 0;
new_state = STATE_SLEEP;
return;
}
}
}
if(button_press)
{
pwr_down_count = 0;

switch(button_press)
{
case(1<<PINB2) :
if(time_set > 1)
{
time_set--;
}
break;
case(1<<PINB3) :
if(time_set < 60)
{
time_set++;
}
break;
case(1<<PINB4) :
current_time = time_set;
PORTD = 0x00;
new_state = STATE_RUN;
break;
default :
break;
}
}
return;
}

ISR(TIMER1_COMPA_vect)
{
static uint16_t run_led_active_count = 0;
static uint16_t run_beep_count;

if(current_time == 0)
{
run_beep_count++;
if(run_beep_count <= 50)
{
PORTB ^= (1<<PINB0);
}
if(run_beep_count == 100)
{
run_beep_count = 0;
}
return;
}

run_led_active_count++;
if(run_led_active_count == 900)
{
PORTD = current_time;
}
if(run_led_active_count >= 1000)
{
run_led_active_count = 0;
PORTD = 0x00;
current_time--;
if(GIMSK ^ (1 << PCIE))
{
PCMSK |= (1 << PCINT4); //externe Interrupts auf Taster 4 (ACHTUNG: nicht sauber es hier zu machen statt in change_state()...)
GIMSK |= (1 << PCIE); //externe Interrupts erst hier aktivieren
}
}
return;
}

ISR(PCINT_vect)
{
GIMSK &= ~(1<<PCIE); //externe Interrupts deaktivieren
_delay_ms(200);
new_state = STATE_SET;
}

void init()
{
//Datenrichtung für die Ausgänge
DDRD = (1 << DDD0) | (1 << DDD1) | (1 << DDD2) | (1 << DDD3) | (1 << DDD4) | (1 << DDD5) | (1 << DDD6);
DDRB = (1 << DDB0);

//Setup Timer 1
OCR1A = TIMER1_END_VALUE; //Compare Value
TCCR1B = (1<<WGM12); //CTC Mode
TIMSK |= (1<<OCIE1A); //Output Compare A Match Interrupt Enable

//Setup Timer 0
OCR0A = TIMER0_END_VALUE; //Compare Value
TCCR0A = (1<<WGM01); //CTC Mode
TCCR0B |= (1<<CS02) | (1<<CS00); //Teiler

TIMSK |= (1<<OCIE0A); //Enable Interrupt Output Compare

current_time = 1;
sei();
return;
}

void change_state()
{
cli();
switch(new_state)
{
case(STATE_SLEEP) :
set_sleep_mode(SLEEP_MODE_PWR_DOWN); //Sleep Mode auf Power Down setzen

TCCR1B &= ~((1<<CS12) | (1<<CS11) | (1<<CS10)); //alle Timer deaktivieren
TCCR0B &= ~((1<<CS02) | (1<<CS01) | (1<<CS00));

PCMSK |= (1 << PCINT2) | (1 << PCINT3) | (1 << PCINT4); //externe Interrupts aktivieren zum Aufwecken
GIMSK |= (1 << PCIE);
break;
case(STATE_SET) :
set_sleep_mode(SLEEP_MODE_IDLE); //Sleep Mode auf Idle setzen
GIMSK &= ~(1<<PCIE); //externe Interrupts deaktivieren
PCMSK &= ~((1 << PCINT2) | (1 << PCINT3) | (1 << PCINT4)); //externe Interrupts aktivieren zum Aufwecken
TCCR1B &= ~((1<<CS12) | (1<<CS11) | (1<<CS10)); //Timer1 deaktivieren
TCCR0B |= (1<<CS02) | (1<<CS00); //Timer0 aktivieren
break;
case(STATE_RUN) :
set_sleep_mode(SLEEP_MODE_IDLE); //Sleep Mode auf Idle setzen
TCCR0B &= ~((1<<CS02) | (1<<CS01) | (1<<CS00)); //Timer0 deaktivieren
TCCR1B |= (1<<CS12) | (1<<CS10); //Timer1 aktivieren
break;
default :
break;
}
current_state = new_state;
sei();
return;
}

[onion]

Ok, jetzt hab ich den Fehler endlich gefunden. Warum kommt man immer wenn man nach endloser Suche um Hilfe bittet dann doch selbst auf die Lösung...?

Also man muss in der Serviceroutine auch die bits im PCMSK löschen sonst wird der Interrupt wohl sozusagen zwischengespeichert, wenn ich mir das richtig überlegt habe.