Interrupt 0 on UNO restarting the program

Hi folks this is my program which was initially working fine but now when I click on the button that generated the interrupt it restarts the code and after it restarts the Button click works for some seconds and then again freezes and finally further clicking restarts the program:

#include <MemoryFree.h>

const int ledPin = 13; //On board LED
const int button = 2; //Interuppt generator when button is clicked

unsigned long prevValue , currValue;
int counter;
boolean checkBUTTON ;
float startTime, endTime;
long previousMicros = 0;

void clickReader() {
currValue = millis();
//Serial.println(“clicked!”);
if (currValue == 400){
currValue = 0;
}
if (currValue - prevValue > 150){
//Serial.println(“clicked!”);
counter++;
if (counter == 1){
startTime = millis() / 1000;
//Serial.print(startTime);
checkBUTTON = true;
digitalWrite(ledPin, counter);
}
else if(counter == 2){
counter = 0;
endTime = millis() / 1000;
//Serial.print(endTime);
checkBUTTON = false;
digitalWrite(ledPin, counter);
}
prevValue = currValue;
}
}

void setup() {

pinMode(ledPin, OUTPUT);
pinMode(button, INPUT);
digitalWrite(button, HIGH);
attachInterrupt(0, clickReader, RISING);
Serial.begin(9600);
Serial.println(“Booted”);
}

void loop() {

}

Doesn't sound like your internal pullup is working.
Do you see a good 5V level on the pin when it is not pushed, and then after the button is released?
Maybe try LOW instead of RISING.

Hi Sir,

Glad to see you!

I tried low but then it hangs the uC.

So is the pin coming back high then? Sounds like it is not.

Also, is it delay() that doesn't work in ISRs, or is it millis()?

startTime = millis() / 1000;
endTime = millis() / 1000;

You're doing integer calculations and then storing the result in float variables (startTime, endTime). If you want them to be float values, you'd need to change 1000 to 1000.0 so that the compiler will use a float calculation. (Since you don't do anything with the result, it's not obvious whether it's sensible to use floats here.)

You have Serial.println() statements in your interrupt handler. Although they're commented out, they should never have been there in the first place. Get rid of them straight away and never put them back. You must not do serial output in an interrupt handler.

The logic of looking for the value of millis() to be exactly equal to 400 seems pointless to me since the chances that you will managed to trigger the interrupt at exactly 400ms after startup seems negligible.

Using the value of an integer counter as an argument to digitalWrite is very poor practice, even though the counter happens to have values 0 and 1 in the cases where you use it. You could hard-code HIGH and LOW here with no impact on the functionality and avoid that nastiness.

You should know how to post code by now. Quotes are not the way.

Apart from that I don't see anything wrong with the code, and when I run it on my UNO R3 the LED toggles on and off every time I ground pin 2.

Thanks Peter

You should know how to post code by now. Quotes are not the way.

Did that accidentally

You have Serial.println() statements in your interrupt handler. Although they're commented out, they should never have been there in the first place. Get rid of them straight away and never put them back. You must not do serial output in an interrupt handler.

Yes I know they should be never be there I used them to know that whether at all the loop is working

thanks I come with the result back!

CrossRoads:
Doesn't sound like your internal pullup is working.
Do you see a good 5V level on the pin when it is not pushed, and then after the button is released?
Maybe try LOW instead of RISING.

Its usually a mistake to use LOW, since the interrupt will keep happening while the
pin is low.

I notice the code had calls (now commented out) to Serial inside the ISR - this
will hang the system, you cannot use Serial within an ISR. Similarly you cannot
call delay() - both these are implemented using interrupts themselves.

As the code stands now it looks OK, except for the glaring lack of "volatile"
declarations for all the variables used in the ISR - they all need to be volatile.

MarkT:
they all need to be volatile.

That's only the case if they need to be shared between the interrupt context and the main context. Here that isn't the case.