My Interrupt is not updating my value to my main, how should i set up my Interrupt?

Question

Im trying to build a program that has a scannerlight with at least 5 leds and one button connected to an external interrupt pin. A single button press (and release) to start the scanner light. Pressing (and releasing) stops the scanner light again (and so on...).

I have a ground side switch on PD2
i have my LEDS on pin PD3,PD4,PD5,PD6 and PD7.
Im useing ATMega328P

I know my towering light works when i press on the button the towering light stops but when i press again it feels like it doesnt return the value to 1.

My code:

#ifndef F_CPU                   
#define F_CPU 1000000UL         
#endif

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

#define BIT_IS_CLEAR(byte, bit) (!(byte & (1 << bit)))

volatile int value = 1;
int main(void)
{
    DDRD = 0b111110000;
    DDRD &= ~(1 << PD2);        // clear DDRD bit 2, sets PD2 (pin 4) for input
    PORTD |= (1 << PD2);        // set PD2/INT0 (pin 4) internal pull-up resistor
    
    
    PCICR = 0b00000100;
    PCMSK2 = 0b00000100;
    sei();
    
    while (value==1) //when value is 1 it should start having a towerlight
    {
        PORTD= 0x80;
        _delay_ms(15000);
        PORTD= 0x40;
        _delay_ms(15000);
        PORTD= 0x20;
        _delay_ms(15000);
        PORTD= 0x10;
        _delay_ms(15000);
        PORTD= 0x08;
        _delay_ms(15000);
        
    }

}


ISR(PCINT2_vect)
{

    if(BIT_IS_CLEAR(PIND, PD2) & value==1) {            // if switch is pressed (logic low)
        value=0;
        } else if(value == 0) { 
        value=1;
        } else {
        // ideally should never get here, but may occasionally due to timing
    }
}```

Regarding reading switches from interrupt-triggered pins, [see this](https://stackoverflow.com/a/32647699/584518). Not something I would recommend for beginners. — Lundin, Sep 14 '20 at 09:14
Also you can replace the macro with a simple `if((PIND & PD2)==0 && value)`, perfectly readable to all C programmers. — Lundin, Sep 14 '20 at 09:19
shouldn't this: `DDRD = 0b111110000;` be: `DDRD = 0b11111000;` as there are only 8 bits in the register — user3629249, Sep 15 '20 at 07:54
regarding: `DDRD = 0b111110000; DDRD &= ~(1 << PD2);` didn't the first statement already set pin PD2 to 0, indicating it is an input? — user3629249, Sep 15 '20 at 08:09
Thanks for the insight guys, i didnt know you could use those commands like that. You thought me something i didnt know :) — Guess, Sep 18 '20 at 12:51

score 1 · Answer 1 · answered Sep 13 '20 at 22:21

1

You are using a bitwise AND where it should be a logical AND. Change this

if(BIT_IS_CLEAR(PIND, PD2) & value==1)

to this

if(BIT_IS_CLEAR(PIND, PD2) && (0 != value))

Comparing for non-zero as opposed to equal to 1 guards against value being corrupted; since you want a zero or non-zero condition. Adding brackets makes the intention very clear. Yoda comparisons (putting the constant on the left hand side) prevents accidental assignment.

One other thing to consider is what you're doing to debounce the switch - in an analogue fashion with R-C+Schmitt or monostable, or in a digital fashion where you have a timer routine that samples the input every so often and counts "how many 1s or 0s over the last, say, 16 samples"?

I find that edge triggered interrupts don't work particularly well for manual switch inputs.

answered Sep 13 '20 at 22:21

Den-Jason

2,395
1
22
17

3

Actually any mechanical button bounces, producing an undetermined number of edges. After going through several different algorithms over the years, I finally use just a slow **polling** cycle of 20ms to 100ms, depending on the system. I never needed to debounce further. – the busybee Sep 14 '20 at 06:09
1

@thebusybee Yep I have pretty much the same conclusion, a 10-20ms cyclic timer just comparing the value with the previous one works fine in most use-cases. It's only when you have some particularly mission-critical button that you need to sample more frequently. However, the OP is not debouncing at all and that won't work. – Lundin Sep 14 '20 at 09:13
yeah I poll based on a timer not actually use the button gpio pin edge changes. – old_timer Sep 15 '20 at 15:23
I tried useing a OP-amp to make a schmitt trigger. But gave in the end the same result. How ever when i used EIMSK my buttons didnt really need a debounce, and gave me the result i wanted. – Guess Sep 18 '20 at 12:49

score 1 · Accepted Answer · answered Sep 15 '20 at 15:20

1

regarding:

while (value==1)
{
    ....
}

When value is 0 the loop is exited, execution then exits the program. This is a serious logic flaw in the program

answered Sep 15 '20 at 15:20

user3629249

16,402
1
16
17

Thanks! This was my problem i made a if statement inside the while loop. – Guess Sep 18 '20 at 12:48

My Interrupt is not updating my value to my main, how should i set up my Interrupt?

2 Answers2