Actually Disabling Timer Interrupts

I'm trying to get the 3 interrupts to run only 5 or 6 times, but every time it triggers far more than 6. I think that I'm doing something stupid, but I can't get the dang thing to turn off. It's driving me bonkers

//3 is interrupt for Timing
//4 through 11 are Time dependent receivers
//3-11 Pins 12-19

//Curent Serial Byte(Letter or Number)
char incomingByte = 0;
//Previous Byte in the Serial Chain
char previousByte = 0;

//lets us do multiple cycles without interruption
bool busy = false;

//Iterator for Interrupt Values
volatile unsigned int maincount = 0;

volatile unsigned int n=0;
//Time of High Period or Timer Duration
volatile unsigned int timerhigh;

//Output Pin for Timer Waveform
volatile unsigned int waveout;


//Time for Timer Delay in milliseconds, default tim
volatile unsigned int groundtime = 0;

//Receiver Pins in an Array, except for 3, which serves as 0 source reference
int pins[9] =
{
  3,4,5,6,7,8,9,10,11
};


//Prototype
void offset_output(int incomingByte);

void rising()
{   
  // set up Timer 1
  TCCR1A = 0;  // normal mode
  TCCR1B = bit(WGM12) | bit(CS11);  // CTC, scale to clock / 8
  OCR1A = timerhigh;          // time before timer fires
  TIMSK1 = bit (OCIE1A);            // interrupt on Compare A Match
}


//Sets Pin Low, then sets up another timer interrupt for the jump back to high
ISR(TIMER1_COMPA_vect)
{
    //Brings Pin Low for Keypress
    //Serial.println("We're in Timer 1");
    pinMode(waveout,OUTPUT);
    digitalWrite(waveout, LOW); 

    //Stop Timer 1
     TCCR1A = 0;
     TCCR1B = 0;                      // stop timer
     TIMSK1 = 0;                      // cancel timer interrupt

     //Disables Interrupts Before Messing
    cli();
   // set up Timer 2
    TCCR2A = 0;  // normal mode
    TCCR2B = 0;
    //  TCCR2A = bit(WGM21) | bit(CS21);  // CTC, scale to clock / 8
    TCCR2A = (1 << WGM21); //Enables CTC for timer 
    
    TCCR2B |= (1 << CS21); 
    TCCR2B |= (1 << CS22); //Sets 256 bit prescaler
    TCCR2B |= (0 << CS20); 
    OCR2A = groundtime;    // time before rising edge
    TIMSK2 = (1 << OCIE2A);
    //Cancel Rising Interrupt on D3
    EIFR = bit (INTF1); 
    //Reinables interrupts
    sei();
 

}

//Timer Interrupt for Low Period
ISR(TIMER2_COMPA_vect)
{
    //Serial.println("WE'RE IN TIMER2");
    //Brings Output pin high
    digitalWrite(waveout, HIGH);
      
    //Stop Timer 2
    TCCR2A = 0;
    TCCR2B = 0;                      // stop timer
    TIMSK2 = 0;                      // cancel timer interrupt  
    
    //Cancel Rising Interrupt on D3
     EIFR = bit (INTF1);   
    n++;
    busy = true;
}

void setup() {
  // put your setup code here, to run once:

  //Sets all the pins for output use
  for(int i=0;i<8;i++)
  {
    pinMode(pins[i], INPUT);
  }
  
  //Sets Reference Pin as Input
  pinMode(3,INPUT);
  
  
  //Begins Serial
  Serial.begin(115200);
  
  //Cancels Timer 1
  TCCR1A = 0;  // normal mode
  TCCR1B = 0;  // stop timer
  TIMSK1 = 0;   // cancel timer interrupt
  
  //Cancels Timer 2
   TCCR2A = 0;
   TCCR2B = 0;                      // stop timer
   TIMSK2 = 0;                      // cancel timer interrupt  



}

void loop() {
  //Time of Ground Drop(2 mS), Universal for all letters and offsets
  groundtime = 125;     // spark time (125 * 16uS) = 2 mS
 if (n >= 3){
   n=0;
   busy = false;
   detachInterrupt(1);
   
     //Cancels Timer 1
  TCCR1A = 0;  // normal mode
  TCCR1B = 0;  // stop timer
  TIMSK1 = 0;   // cancel timer interrupt
  
  //Cancels Timer 2
   TCCR2A = 0;
   TCCR2B = 0;                      // stop timer
   TIMSK2 = 0;                      // cancel timer interrupt  


 }
    if((Serial.available()> 0) && busy == false );
    sei(); // enable interrupts
    {
       //Reads Serial Value and assigns to incomingByte
       incomingByte = Serial.read();

    
  //  noInterrupts ();  // atomic change of the time amount

   //In this area, we'll set the output pin to be used by the timing interrupt, 
   //and the time delay off the default waveform present on pin 4
   offset_output(incomingByte);
   //We can use switch case arrangement, don't know if that's the optimal setup though    

  //Attaches Interrupt to Send Letter!
  attachInterrupt(1,rising,RISING);
    }
 
}

void offset_output(int incomingByte){
     switch(incomingByte)
   {
     default:
     //Do Nothing
     break;
  
     //Cases FOR ALL THE LETTERS
     case 'a':
       break;
     case 'b':
       break;
     case 'c':
       break;
     case 'd':
       break;
     case 'e':
       break;
     case 'f':
       break;
     case 'g':
       break;
     case 'h':
       break;
     case 'i':
       break;
     case 'j':
     break;
     case 'k':
     break;
     case 'l':
     break;
     case 'm':
     break;
     case 'n':
     break;
     case 'o':
     break;
     case 'p':
     break;
     case 'q':
     break;
     case 'r':
     //Output pin 17 Frequency Pin 10
     //microseconds
       timerhigh=8000;
       waveout=9;
       break;
     case 's':
       break;
     case 't':
     break;
     case 'u':
     break;
     case 'v':
     break;
     case 'w':
     break;
     case 'x':
     break;
     case 'y':
     break;
     case 'z':
     break;
     
     //Cases FOR ALL THE NUMBERS
     case '0':
     break;
     case '1':
     break;
     case '2':
     break;
     case '3':
     break;
     case '4':
     break;
     case '5':
     break;
     case '6':
     break;
     case '7':
     break;
     case '8':
     break;
     case '9':
     break;
     
     //Cases FOR ALL THE PUNCUATION
     case '.':
     break;
     case ',':
     break;
     case '!':
     break;
     case '"':
     break;
     case '#':
     break;
     case '

:
    break;
    case '%':
    break;
  }
}

ISR(TIMER1_COMPA_vect)
{
...
//Disables Interrupts Before Messing
cli(); // <<<<<<<<<< Interrupts are already disabled. Remove this line.
...
//Reinables interrupts
sei(); // <<<<<<<<<< REALLY BAD IDEA. REMOVE THIS LINE.
}

Removed those, no change in the program function

    if((Serial.available()> 0) && busy == false );

Oops.

Hey BadCoding, could you elaborate on what's wrong with that line? Serial .available() returns true when there's nothing in the buffer, Serial.read() emptys the buffer, and && is the and operator, what's wrong with that line?

benl11235:
Serial .available() returns true when there's nothing in the buffer, Serial.read() emptys the buffer, and && is the and operator...

None of which has any affect because you put a semicolon at the end.

In general its best to use a volatile boolean to communicate with the ISR when you
want it to stop doing stuff. If you mask an interrupt and the condition happens then
you unmask that interrupt it runs straightaway even if the condition was months ago.

An interrupt source sets a bit when its due, and that's only cleared when the handler
runs or you manually clear it. For instance timer1 overflow interrupt is flagged up
in the TOV1 bit in TIFR1 register - you don't appear to touch any of these interrupt
flag bits, just the mask bits - at best these can only delay an interrupt not cancel it.

Using your own boolean you have precise control of when the ISR does stuff, and not
have to know anything about any of the flag and mask bits.

You can also tidy up this:

   switch(incomingByte)
   {
     default:
     //Do Nothing
     break;
  
     //Cases FOR ALL THE LETTERS
     case 'a':
       break;
     case 'b':
       break;
     case 'c':
       break;
     case 'd':
       break;
     case 'e':
       break;
     case 'f':
       break;
     case 'g':
       break;
     case 'h':
       break;
     case 'i':
       break;
     case 'j':
     break;
     case 'k':
     break;
     case 'l':
     break;
     case 'm':
     break;
     case 'n':
     break;
     case 'o':
     break;
     case 'p':
     break;
     case 'q':
     break;
     case 'r':
     //Output pin 17 Frequency Pin 10
     //microseconds
       timerhigh=8000;
       waveout=9;
       break;
     case 's':
       break;
     case 't':
     break;
     case 'u':
     break;
     case 'v':
     break;
     case 'w':
     break;
     case 'x':
     break;
     case 'y':
     break;
     case 'z':
     break;
     
     //Cases FOR ALL THE NUMBERS
     case '0':
     break;
     case '1':
     break;
     case '2':
     break;
     case '3':
     break;
     case '4':
     break;
     case '5':
     break;
     case '6':
     break;
     case '7':
     break;
     case '8':
     break;
     case '9':
     break;
     
     //Cases FOR ALL THE PUNCUATION
     case '.':
     break;
     case ',':
     break;
     case '!':
     break;
     case '"':
     break;
     case '#':
     break;
     case '

to just

   switch(incomingByte)
   {
     default:
     //Do Nothing
     break;

     case 'r':
     //Output pin 17 Frequency Pin 10
     //microseconds
       timerhigh=8000;
       waveout=9;
       break;

   }

:
    break;
    case '%':
    break;
  }


to just

§DISCOURSE_HOISTED_CODE_1§

Your so far of the beam with the idea of "Disabling Timer Interrupts" that I want some of what your on (it's been a real bad day). Tell is what you are real trying to do with out any of your ideas of how to do it.

If you you do we can set you on the right track. I'm a pro programmer (first program written in 1974) and I can't think of any reason to ever disable the timers.

Mark

Thanks guys, I actually do need all those cases, I just hadn't filled out all of them yet. I'm trying to replicate a waveform with a certain offset from a reference waveform. The waveform goes to ground for 2ms and then back up for a period of 16ms. This allows me to pretend to be the keyboard on an old typewriter. My latest trouble is that the data collected from the quotes tends to stop abruptly. I'll create another post more specific to my problem, but here's the code in case you see it.
Arduino Code

Quotefile