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
    }
}```

Answer 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

Answer 2

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.