interrupts stop working after a few cycles[Solved]

Interrupts stop working after cycles(how many cycles seems to be random). Program looks good so I don’t understand why. Here is my code, Please help.

int countUp = 9;
int countDown = 10;
int powerReset = 11;//future use
int bounceDuration0=1000;    
int bounceDuration1=1000;    
volatile int bounceTime0=0;
volatile int bounceTime1=0;
volatile int waveCount=0;
volatile int resetCounter=0;
int lastWaveCount=0;
//##################################################################
void setup() {
 pinMode(4, INPUT);
 pinMode(countUp, OUTPUT);
 pinMode(countDown, OUTPUT);
 pinMode(powerReset, OUTPUT);//future use
 attachInterrupt(0, interruptHandler0, RISING);
 attachInterrupt(1, interruptHandler1, RISING);
 digitalWrite(countUp,LOW);
 digitalWrite(countDown,LOW);
 digitalWrite(powerReset,LOW);//future use
 Serial.begin(9600);//debugging
}
//##################################################################
void loop() {
 if(waveCount>lastWaveCount){
   digitalWrite(countUp,HIGH);
   delay(1000);
   digitalWrite(countUp,LOW);
   lastWaveCount=waveCount;
   Serial.println(waveCount);//debugging
 }
 if(resetCounter==1){
   for(int i=0;i<waveCount;i++){
     digitalWrite(countDown,HIGH);
     delay(250);
     digitalWrite(countDown,LOW);
     delay(250);
     Serial.println(i);//debugging
   }
   waveCount=0;
   resetCounter=0;
 }
 Serial.println(waveCount);//debugging
}
//##################################################################
void interruptHandler0(){
 if(millis()>bounceTime0){
   waveCount+=1;
   bounceTime0=millis()+bounceDuration0;
 }
}
void interruptHandler1(){
 if(millis()>bounceTime1){
   resetCounter=1;
   bounceTime1=millis()+bounceDuration1;
 }
}
volatile int bounceTime0=0;
volatile int bounceTime1=0;

What type does millis() return? Here's a hint. It's not int.

That won’t matter, the low 16 bits of millis() is still a cycling counter value.

One issue with the code is that you can’t expect to read a volatile int (or long)
without inhibiting interrupts first (other wise it could get garbled if the
interrupts happens half way through reading it).

So use noInterrupts() and interrupts () around accesses of multi-byte variables
from the main code.

But the most suspicious thing is the use of a volatile in a for loop condition:

    for(int i=0;i<waveCount;i++){

Looks like chasing a moving target.

You probably meant

    noInterrupts () ;
    int N = waveCount ;  // read atomically, once before looping
    interrupts () ;
    for(int i=0;i<N;i++){  // N doesn't change

excellent idea reading waveCount into a non-volatile variable before looping. I will make that change.

I found that the main reason it failed was that I did not reset lastWaveCount to zero when resetting. if(waveCount<lastWaveCount) would never be true after first cycle of interrupt1.

works now. here is my revised code.

int countUp = 9;
int countDown = 10;
int powerReset = 11;//future use
int bounceDuration0=1000;    
int bounceDuration1=1000;    
volatile unsigned long bounceTime0=0;
volatile unsigned long bounceTime1=0;
volatile int waveCount=0;
volatile int resetCounter=0;
int lastWaveCount=0;
//##################################################################
void setup() {
 pinMode(4, INPUT);
 pinMode(countUp, OUTPUT);
 pinMode(countDown, OUTPUT);
 pinMode(powerReset, OUTPUT);//future use
 attachInterrupt(0, interruptHandler0, RISING);
 attachInterrupt(1, interruptHandler1, RISING);
 digitalWrite(countUp,LOW);
 digitalWrite(countDown,LOW);
 digitalWrite(powerReset,LOW);//future use
 Serial.begin(9600);//debugging
}
//##################################################################
void loop() {
 noInterrupts();
 int n=waveCount;
 interrupts();
 if(n>lastWaveCount){
   digitalWrite(countUp,HIGH);
   delay(1000);
   digitalWrite(countUp,LOW);
   lastWaveCount=n;
   Serial.println(waveCount);//debugging
 }
 if(resetCounter==1){
   for(int i=0;i<waveCount;i++){
     digitalWrite(countDown,HIGH);
     delay(250);
     digitalWrite(countDown,LOW);
     delay(250);
     Serial.println(i);//debugging
   }
   waveCount=0;
   lastWaveCount=0;
   resetCounter=0;
 }
 Serial.println(waveCount);//debugging
}
//##################################################################
void interruptHandler0(){
 if(millis()>bounceTime0){
   waveCount+=1;
   bounceTime0=millis()+bounceDuration0;
 }
}
void interruptHandler1(){
 if(millis()>bounceTime1){
   resetCounter=1;
   bounceTime1=millis()+bounceDuration1;
 }
}

thank you very much for the input. :)

How can I reset millis so I don't have failures when it rolls over?

johnnyreb1129: How can I reset millis so I don't have failures when it rolls over?

Why would you want to?

Please, please remember to use code tags when posting code.

You can even do it retrospectively (hint)

johnnyreb1129: How can I reset millis so I don't have failures when it rolls over?

Your watch rolls over twice a day. When was the last time that caused you a problem?

Removing power from the Arduino, and then powering it back up generally resets millis() just fine.

This board will run 24-7 and receive inputs in about 90 second intervals for 3-4 hours, up to twice a day. I need to reset millis() to avoid rollover during one of those 3-4 hour time periods as my debounce function uses millis(). If millis() rolled over at the wrong time then millis() would always be less than bounceTime and my variables would not be incremented. I have therefore cycled a normally closed relay at the end of the reset operation to cycle power to the board, resetting millis() at the end of each session. Thanks for the input. Here is the revised code.

int countUp = 9;//increment relay coil output N.O.
int countDown = 10;//decrement relay coil output N.O.
int powerReset = 11;//power relay coil output N.C.
int bounceDuration0=2000;
int bounceDuration1=2000;
volatile unsigned long bounceTime0=0;
volatile unsigned long bounceTime1=0;
volatile int waveCount=0;
volatile int resetCounter=0;
int lastWaveCount=0;
int countTemp=0;
//##################################################################
void setup() {
 pinMode(4, INPUT);//not used, set as input because ground trace runs across this pin
 pinMode(countUp, OUTPUT);
 pinMode(countDown, OUTPUT);
 pinMode(powerReset, OUTPUT);
 attachInterrupt(0, interruptHandler0, RISING);//increment input
 attachInterrupt(1, interruptHandler1, RISING);//reset input
 digitalWrite(countUp,LOW);
 digitalWrite(countDown,LOW);
 digitalWrite(powerReset,LOW);
}
//##################################################################
void loop() {
 noInterrupts();
 countTemp=waveCount;//set current wave count to non-volatile variable
 interrupts();
 if(countTemp>lastWaveCount){//cycles increment relay if increment input detected
   digitalWrite(countUp,HIGH);
   delay(1000);
   digitalWrite(countUp,LOW);
   lastWaveCount=countTemp;
 }
 if(resetCounter==1){//if reset input detected, cycles decrement relay == number of increments
   noInterrupts();
   countTemp=waveCount;//set current wave count to non-volatile variable
   interrupts();
   for(int i=0;i<countTemp;i++){//cycle decrement relay
     digitalWrite(countDown,HIGH);
     delay(200);
     digitalWrite(countDown,LOW);
     delay(200);
   }
   waveCount=0;    //
   lastWaveCount=0;//reset variables
   resetCounter=0; //
   digitalWrite(powerReset,HIGH);//cyle power to board resetting millis()
 }
}
//##################################################################
void interruptHandler0(){//increment interrupt handler
 if(millis()>bounceTime0){
   waveCount+=1;
   bounceTime0=millis()+bounceDuration0;
 }
}
void interruptHandler1(){//reset interrupt handler
 if(millis()>bounceTime1){
   resetCounter=1;
   bounceTime1=millis()+bounceDuration1;
 }
}

I need to reset millis() to avoid rollover during one of those 3-4 hour time periods

No, you do not. You don't need to reset your watch when it rolls over while you are at work/school, do you?

Properly written code, involving ONLY subtraction of times. works just fine when rollover occurs.

Also, if you find that you need more buttons later, in the process of trying to add them and finding that you have run out of interrupts you might discover that you don't really need to use interrupts at all for this task.

PaulS: No, you do not. You don't need to reset your watch when it rolls over while you are at work/school, do you?

Properly written code, involving ONLY subtraction of times. works just fine when rollover occurs.

Then how do you recommend I rewrite my debounce functions to avoid the problem. I am willing to take constructive criticism but please offer suggestions with it.

aarg:
Also, if you find that you need more buttons later, in the process of trying to add them and finding that you have run out of interrupts you might discover that you don’t really need to use interrupts at all for this task.

I probably don’t NEED to use interrupts, but I am trying to learn more about interrupts on a simple project so I can use them more effectively in the future.

johnnyreb1129: Then how do you recommend I rewrite my debounce functions to avoid the problem. I am willing to take constructive criticism but please offer suggestions with it.

He already told you. Use subtraction only.

Try using:

if(millis()-bounceTime0 >bounceDuration){

How about this?

void interruptHandler0(){//increment interrupt handler
  if(abs(millis()-lastMillis0)>bounceDuration0){
    waveCount+=1;
    lastMillis0=millis()
  }
}

There is no need for abs().

Henry_Best: Try using:

if(millis()-bounceTime0 >bounceDuration){

Thank you.