Interrupt causing crash?

I'm trying to figure out how I want this to work - I think I want a button to stop the code and enter a config mode. Then I want the same button to select/deselect a series of variables that are scrolled through with a rotary knob. When a variable is selected, the rotary knob will edit the value until the button is pressed again.

Right now I'm just using the button to switch between two functions.

This code first executes the interrupt (prints "enter config") and enters the sweep function. When the button is pressed, sometimes it prints "enter config" and goes back to the middle of the sweep function, other times it just prints "Sweep Mode" 20 or so times and locks up, or something similar. I've tried changing the debounce time and adding the delays...

long debouncing_time = 15; //Debouncing Time in Milliseconds
volatile unsigned long last_micros = 0;

void setup()
{
...
attachInterrupt(1, debounce, RISING);
}

void loop()
{
  if (mode == false) {
    cw();
    }
  else {
    sweep();
  }
}

void debounce() {
  if((long)(micros() - last_micros) >= debouncing_time * 1000) {
    last_micros = micros();
    config();
  }
}

void config() {
  Serial.println("enter config");
  delay(200);
  mode = !mode;
  delay(50);
}

void cw()
{
  Serial.println(" CW Mode");
  while (digitalRead(BUTTON) == HIGH) {
  //keep doing this until button is pressed, would rather eliminate the digital read though.
}

void sweep()
{
  Serial.println(" Sweep Mode");  
  while (digitalRead(BUTTON) == HIGH) {
  //keep doing this until button is pressed, would rather eliminate the digital read though.
}

Calling "Serial.print" in interrupt context is a long way from being the most sensible thing you could do.
Get rid of it.

You can't use Serial from inside an interrupt routine.

Mark

 delay(200);

Oops. Mote/plank situation.

Do detach the interrupt at the beginning of the do_config routine (the rising edge has been caught already, you are in) and before you go out from the interrupt do attach it back..
For example:

..
void setup()
{
...
attachInterrupt(1, do_config, RISING);
}

void do_config() {
   detachInterrupt(1);
    config();
    //delay(5);  //you may debounce here when config() is short
   attachInterrupt(1, do_config, RISING);
   
  }

It would be better you do a simple set of a "change_mode flag" inside the config(), ie:

void config() {
  mode = !mode;
  changed_mode = 1;
}

and in the loop:

void loop()
{
  if (changed_mode == 1) {
    if (mode == false) {
      cw();
    }
    else {
      sweep();
    }
    changed_mode = 0;
  }   
}

Where to start....

What's probably happening is that you are getting an interrupt cascade: several interrupts that come in while you are still executing the interrupt function. This can cause some crazy errors with code that is not designed to be re-entrant, especially if the interrupts are really close together, as they would be with noisy input like a non-debounced button.

You are calling both delay() and Serial.print from interrupt context. Don't Do That. Interrupt routines should be as short as possible, do as little as possible and return immediately.

Your "debounce" routine is a bit primitive, and it's possible that you are still seeing noise from the
button after the debounce period. There are many better debounce algorithms. See this
Debouncing Contacts and Switches for some ideas.

Since the debounce period is in milliseconds, why not use millis() instead of micros()? Much less chance of roll-over problems that way.

In general it is a Bad Idea to attach a non-debounced input to an interrupt. You really want to
go with polling on this. Even if your loop takes several milliseconds, it will still seem to respond very quickly to human reactions.

There are several pieces of code in the playground that deal with debouncing, you could use one
of those and not have to write your own. Arduino Playground - HomePage

HTH,

I thought I had it working... I was able to cycle through the config function with each press of the button, consistently... but only the while (param == 0) loop would print anything. the rest would not respond to encoder (Also, the cw function stopped running the encoder correctly relative to another similar code? it steps in 2, 5, 14, 35, and gets stuck going one way - it is consistent though).

right now, it steps into param == 1 and hangs.

I'm just looking at the loop, debounce, and config functions, so feel free to ignore the rest.

//AD9850 DDS test

#include <SPI.h>

//#define DDS_CLOCK 125000000
#define DDS_CLOCK 124999000

#define  CLOCK  13  //pin 8 connections for DDS
#define  LOAD 10 //9 FQ_UD
#define  DATA  11 //10
#define  RESET 9  //11
#define BUTTON 3
int lastButtonState = HIGH;
//volatile char mode = 'cw';
int mode = 0;
int param = 6;

/* Encoder Library - Basic Example
 * http://www.pjrc.com/teensy/td_libs_Encoder.html
 *
 * This example code is in the public domain.
 */
#define ENCODER_DO_NOT_USE_INTERRUPTS
#include <Encoder.h>

/*
// This optional setting causes Encoder to use more optimized code,
// It must be defined before Encoder.h is included.
#define ENCODER_OPTIMIZE_INTERRUPTS
#include <Encoder.h>
*/

// Change these two numbers to the pins connected to your encoder.
//   Best Performance: both pins have interrupt capability
//   Good Performance: only the first pin has interrupt capability
//   Low Performance:  neither pin has interrupt capability
Encoder myEnc(4, 5);  //   avoid using pins with LEDs attached
long oldPosition  = -999;

long debouncing_time = 20; //Debouncing Time in Milliseconds
volatile unsigned long last_micros = 0;

  long F1 = 1;
  long F2 = 100;
  long fStep = 1;
  long freq = F1;
  long timeDwell = 300;
  int32_t tuning_word_F1 = (F1 * 0x100000000LL)/124999100L;
  int32_t tuning_word_F2 = (F2 * 0x100000000LL)/124999100L;
  int32_t tuning_word_fStep = (fStep * 0x100000000LL)/124999100L;
  int32_t tuning_word = (freq * 0x100000000LL)/124999100L;





void setup() {
  Serial.begin(115200);
  Serial.println("AD9850 DDS");
  pinMode(BUTTON, INPUT_PULLUP);
  pinMode (CLOCK, OUTPUT); 
  pinMode (LOAD,  OUTPUT); 
  pinMode (DATA,  OUTPUT); 
  pinMode (RESET, OUTPUT); 
  SPI.begin();
  SPI.setDataMode(SPI_MODE0);    // mode 0 seems to be the right one
  SPI.setClockDivider(SPI_CLOCK_DIV2); //try to go pretty fast
  SPI.setBitOrder(LSBFIRST);
  AD9850_init();
  AD9850_reset();
  SetFrequency(10000000);
  Serial.println("10,000,000 Hz");
  attachInterrupt(1, debounce, RISING);
  delay(100);
}

void loop() {
  Serial.println("loop");
  if (mode == 0) {
    config();
  }
  if (mode == 1) {
    cw();
  }
  if (mode == 2) {
    sweep();
  }
}

void debounce() {
  if((long)(micros() - last_micros) >= debouncing_time * 1000) {
    last_micros = micros();
    mode = 0;
    param = param + 1;
    if (param > 6) {
      param = 0;
    }
  }
}

void config() {
  Serial.println("Config Mode");
  Serial.println(param);
  Serial.println(mode);
  Serial.println("Param 0:");

    while (param == 0) {
    long newPosition = myEnc.read();
    if (newPosition < oldPosition) {
      mode = 1;
      Serial.println("Changed to CW mode");
      //cw();
    }
    if (newPosition > oldPosition) {
      mode = 2;
      Serial.println("Changed to Sweep mode");
      //sweep();
    }
    oldPosition = newPosition;
  }

  Serial.println("Param 1:");
  while (param == 1) {
    long newPosition = myEnc.read();
    if (newPosition != oldPosition) {
      oldPosition = newPosition;
        freq = freq - newPosition;
        Serial.print("Edit F1: ");
        Serial.println(long(freq));
    }
  }
  
  Serial.println("Param 2:");
  while (param == 2) {
    long newPosition = myEnc.read();
    if (newPosition != oldPosition) {
      oldPosition = newPosition;
        freq = freq - (250 * newPosition);
        Serial.print("Edit F2: ");
        Serial.println(long(freq));
    }
  }
  
  Serial.println("Param 3:");
  while (param == 3) {
    long newPosition = myEnc.read();
    if (newPosition != oldPosition) {
      oldPosition = newPosition;
        freq = freq - (250 * newPosition);
        Serial.print("Edit fStep: ");
        Serial.println(long(freq));
    }
  }
  
  Serial.println("Param 4:");
  while (param == 4) {
    long newPosition = myEnc.read();
    if (newPosition != oldPosition) {
      oldPosition = newPosition;
        timeDwell = timeDwell - newPosition;
        Serial.print("Edit Dwell: ");
        Serial.println(long(timeDwell));
    }
  }
  
  Serial.println("Param 5:");
  while (param == 5) {
    long newPosition = myEnc.read();
    if (newPosition != oldPosition) {
      oldPosition = newPosition;
        freq = freq - (250 * newPosition);
        Serial.print("Edit 5: ");
        Serial.println(long(freq));
    }
  }
  
  Serial.println("Param 6:");
  while (param == 6) {
    long newPosition = myEnc.read();
    if (newPosition != oldPosition) {
      oldPosition = newPosition;
      freq = freq - (250 * newPosition);
      Serial.print("Edit 6: ");
      Serial.println(long(freq));
    }
  }
}
    


void cw() {
  boolean updateLCD = true;
  //long freq = 1000000;
  while (mode == 1) {
    long newPosition = myEnc.read();
    if (newPosition != oldPosition) {
      oldPosition = newPosition;
      freq = freq - newPosition;
      int32_t tuning_word = (freq * 0x100000000LL)/124999100L;
      SetFrequency(tuning_word);
      updateLCD = true;
    }
    if (updateLCD) {
      Serial.println(" CW Mode");
      Serial.print(long (freq));
      Serial.println(" Hz");
      updateLCD = false;
    }
  }
}


void sweep() {
/*
  long F1 = 1;
  long F2 = 100;
  long fStep = 1;
  long freq = F1;
  long timeDwell = 300;
  int32_t tuning_word_F1 = (F1 * 0x100000000LL)/124999100L;
  int32_t tuning_word_F2 = (F2 * 0x100000000LL)/124999100L;  
  int32_t tuning_word_fStep = (fStep * 0x100000000LL)/124999100L;  
  int32_t tuning_word = (freq * 0x100000000LL)/124999100L;
*/
  Serial.println(" Sweep Mode");
  while (mode == 2) {
    long timeSweepStart = micros();
    for (int i = 0; i < 10; i++) {
    long timeStepStart = micros();
    while (tuning_word <= tuning_word_F2) {
      long timenow = micros();
      if ((timenow - timeStepStart) >= timeDwell) {
        timeStepStart = timenow;
        SetFrequency(tuning_word);
        tuning_word = tuning_word + tuning_word_fStep;
      }
     }
    tuning_word = tuning_word_F1;
    }
    long timeSweepStop = micros();
    long timeSweep = timeSweepStop - timeSweepStart;
    Serial.println(timeSweep);
  }
}


void SetFrequency(int32_t tuning_word) {
  PORTB = PORTB & B11111011; // clear pin 10
  SPI.transfer(tuning_word);
  SPI.transfer(tuning_word >> 8);
  SPI.transfer(tuning_word >> 16);
  SPI.transfer(tuning_word >> 24);
  SPI.transfer(0x0);
  PORTB = PORTB | B00000100; // set pin 10
}

void AD9850_init() {
  digitalWrite(RESET, LOW);
  digitalWrite(CLOCK, LOW);
  digitalWrite(LOAD, LOW);
  digitalWrite(DATA, LOW);
}

void AD9850_reset() {
  //reset sequence is:
  // CLOCK & LOAD = LOW
  //  Pulse RESET high for a few uS (use 5 uS here)
  //  Pulse CLOCK high for a few uS (use 5 uS here)
  //  Set DATA to ZERO and pulse LOAD for a few uS (use 5 uS here)
  // data sheet diagrams show only RESET and CLOCK being used to reset the device, but I see no output unless I also
  // toggle the LOAD line here.
  digitalWrite(CLOCK, LOW);
  digitalWrite(LOAD, LOW);
  digitalWrite(RESET, LOW);
  delay(5);
  digitalWrite(RESET, HIGH);  //pulse RESET
  delay(5);
  digitalWrite(RESET, LOW);
  delay(5);
  digitalWrite(CLOCK, LOW);
  delay(5);
  digitalWrite(CLOCK, HIGH);  //pulse CLOCK
  delay(5);
  digitalWrite(CLOCK, LOW);
  delay(5);
  digitalWrite(DATA, LOW);    //make sure DATA pin is LOW
  digitalWrite(LOAD, LOW);
  delay(5);
  digitalWrite(LOAD, HIGH);  //pulse LOAD
  delay(5);
  digitalWrite(LOAD, LOW);
  // Chip is RESET now
}

It would be a lot easier to follow the flow of your program if you:
Put each { on a new line,
Used Tools + Auto Format to fix the indenting,
Used appropriate amounts of white space (one line - no more, no less) between functions.

//   Low Performance:  neither pin has interrupt capability
Encoder myEnc(4, 5);  //   avoid using pins with LEDs attached

On a 328-based Arduino, neither of those pins has (external) interrupt capability. Why are you using them, specifically?

Hi Paul, I've now found the Auto Format function.

I'm not too concerned with a low performance encoder, if it misses a pulse here and there. But I've not yet seen the encoder need interrupts and have specified so in the code (#define ENCODER_DO_NOT_USE_INTERRUPTS). The encoder has two 'data' pins and the 328 only has 2 interrupts and it seemed I needed one of the interrupts for a button.

I tried moving the encoder to pins 2, and 4 and enabling interrupts. The encoder still works fine when in the config function when in the "while (param == 0)" loop, but still locks up when pressing the interrupt button and getting to the "while (param == 1)" loop.

It's also weird because it seems the interrupt fires by itself since it steps into config after running the loop once.

I'm also not sure if attaching an interrupt to a pin conflicts with any of the other code on the same pin, such as "pinMode(BUTTON, INPUT_PULLUP);"

I tried putting a 10k from VCC to the button pin and the code auto cycled through to "while (param == 6) and printed "Edit 6: 1" meaning the encoder was read.

I changed two things:

mode and param variables as volatile.
For some reason, if (newPosition != oldPosition) does not work in this code as it does in other very similar code, so I change it to, if (newPosition > oldPosition)...

I have some other idea how to do the config routine, but this works.

Thanks for the clues and help to get me there.

//AD9850 DDS test

#include <SPI.h>

//#define DDS_CLOCK 125000000
#define DDS_CLOCK 124999000

#define  CLOCK  13  //pin 8 connections for DDS
#define  LOAD 10 //9 FQ_UD
#define  DATA  11 //10
#define  RESET 9  //11
#define BUTTON 3 //interrupt 1
int lastButtonState = HIGH;  //not used, keep in case 2nd button
volatile int mode = 0;
volatile int param = 6;
long delta = 1;

/* Encoder Library - Basic Example
 * http://www.pjrc.com/teensy/td_libs_Encoder.html
 *
 * This example code is in the public domain.
 */

//#define ENCODER_OPTIMIZE_INTERRUPTS
//#define ENCODER_DO_NOT_USE_INTERRUPTS
#include <Encoder.h>

/*
// This optional setting causes Encoder to use more optimized code,
 // It must be defined before Encoder.h is included.
 #define ENCODER_OPTIMIZE_INTERRUPTS
 #include <Encoder.h>
 */

// Change these two numbers to the pins connected to your encoder.
//   Best Performance: both pins have interrupt capability
//   Good Performance: only the first pin has interrupt capability
//   Low Performance:  neither pin has interrupt capability
Encoder myEnc(2, 4);  //   avoid using pins with LEDs attached
long oldPosition  = -999;
long debouncing_time = 30; //Debouncing Time in Milliseconds
volatile unsigned long last_micros = 0;
long F1 = 1000; // Hz
long F2 = 10000000; // Hz
long fStep = 1000; // Hz
long freq = 10000000; // Hz
long timeDwell = 20; // us
int32_t tuning_word_F1 = (F1 * 0x100000000LL)/124999100L;
int32_t tuning_word_F2 = (F2 * 0x100000000LL)/124999100L;
int32_t tuning_word_fStep = (fStep * 0x100000000LL)/124999100L;
int32_t tuning_word = (freq * 0x100000000LL)/124999100L;

void setup() {
  Serial.begin(115200);
  Serial.println("AD9850 DDS");
  pinMode(BUTTON, INPUT_PULLUP);
  pinMode (CLOCK, OUTPUT); 
  pinMode (LOAD,  OUTPUT); 
  pinMode (DATA,  OUTPUT); 
  pinMode (RESET, OUTPUT); 
  SPI.begin();
  SPI.setDataMode(SPI_MODE0);    // mode 0 seems to be the right one
  SPI.setClockDivider(SPI_CLOCK_DIV2); //try to go pretty fast
  SPI.setBitOrder(LSBFIRST);
  AD9850_init();
  AD9850_reset();
  SetFrequency(10000000);
  Serial.println("10,000,000 Hz");
  attachInterrupt(1, debounce, RISING);
  delay(100);
}

void loop() {
  if (mode == 0) {
    config();
  }
  if (mode == 1) {
    cw();
  }
  if (mode == 2) {
    sweep();
  }
}

void debounce() {
  if((long)(micros() - last_micros) >= debouncing_time * 1000) {
    last_micros = micros();
    mode = 0;
    param = param + 1;
    if (param > 5) {
      param = 0;
    }
  }
}

void config() {
  Serial.println(" Config Mode");
  long newPosition = myEnc.read();
  oldPosition = newPosition;
  Serial.println("CW <----> SWEEP");
  while (param == 0) {
    long newPosition = myEnc.read();
    if (newPosition < oldPosition) {
      mode = 1;
      cw();
    }
    if (newPosition > oldPosition) {
      mode = 2;
      sweep();
    }
    oldPosition = newPosition;
  }
  Serial.print("F1: ");
  Serial.print(long(F1));
  Serial.println(" Hz");
  newPosition = myEnc.read();
  oldPosition = newPosition;
  while (param == 1) {
    long newPosition = myEnc.read();
    if (newPosition > oldPosition) {
      F1 = F1 + delta;
      if (F1 > 40000000) {
        F1 = 40000000;
      }
      Serial.print("F1: ");
      Serial.print(long(F1));
      Serial.println(" Hz");
    }
    if (newPosition < oldPosition) {
      F1 = F1 - delta;
      if (F1 < 0) {
        F1 = 0;
      }
      Serial.print("F1: ");
      Serial.print(long(F1));
      Serial.println(" Hz");
    }
    oldPosition = newPosition;
  }
  Serial.print("F2: ");
  Serial.print(long(F2));
  Serial.println(" Hz");
  newPosition = myEnc.read();
  oldPosition = newPosition;
  while (param == 2) {
    long newPosition = myEnc.read();
    if (newPosition > oldPosition) {
      F2 = F2 + delta;
      if (F2 > 40000000) {
        F2 = 40000000;
      }
      Serial.print("F2: ");
      Serial.print(long(F2));
      Serial.println(" Hz");
    }
    if (newPosition < oldPosition) {
      F2 = F2 - delta;
      if (F2 < 0) {
        F2 = 0;
      }
      Serial.print("F2: ");
      Serial.print(long(F2));
      Serial.println(" Hz");
    }
    oldPosition = newPosition;
  }
  Serial.print("Fstep: ");
  Serial.print(long(fStep));
  Serial.println(" Hz");
  newPosition = myEnc.read();
  oldPosition = newPosition;
  while (param == 3) {
    long newPosition = myEnc.read();
    if (newPosition > oldPosition) {
      fStep = fStep + delta;
      if (fStep > (F2 - F1)) {
        fStep = F2 - F1;
      }
      Serial.print("Fstep: ");
      Serial.print(long(fStep));
      Serial.println(" Hz");
    }
    if (newPosition < oldPosition) {
      fStep = fStep - delta;
      if (fStep < 0) {
        fStep = 0;
      }
      Serial.print("Fstep: ");
      Serial.print(long(fStep));
      Serial.println(" Hz");
    }
    oldPosition = newPosition;
  }
  Serial.print("Dwell: ");
  Serial.print(long(timeDwell));
  Serial.println(" us");
  newPosition = myEnc.read();
  oldPosition = newPosition;
  while (param == 4) {
    long newPosition = myEnc.read();
    if (newPosition > oldPosition) {
      timeDwell = timeDwell + delta;
      Serial.print("Dwell: ");
      Serial.print(long(timeDwell));
      Serial.println(" us");
    }
    if (newPosition < oldPosition) {
      timeDwell = timeDwell - delta;
      if (timeDwell < 0) {
        timeDwell = 0;
      }
      Serial.print("Dwell: ");
      Serial.print(long(timeDwell));
      Serial.println(" us");
    }
    oldPosition = newPosition;
  }
  Serial.print("Edit +");
  Serial.println(long(delta));
  long i = 0;
  newPosition = myEnc.read();
  oldPosition = newPosition;
  while (param == 5) {
    long newPosition = myEnc.read();
    if (newPosition > oldPosition) {
      i++;
      if (i > 6) {
        i = 6;
      }
      delta = pow(10, i) + 0.5;
      Serial.print("Edit +");
      Serial.println(long(delta));
    }
    if (newPosition < oldPosition) {
      i--;
      if (i < 0) {
        i = 0;
      }
      delta = pow(10, i) + 0.5;
      Serial.print("Edit +");
      Serial.println(long(delta));
    }
    oldPosition = newPosition;
  }
}

void cw() {
  Serial.println("CW Mode");
  Serial.print("CW Freq: ");
  Serial.print(long(freq));
  Serial.println(" Hz");
  long newPosition = myEnc.read();
  oldPosition = newPosition;
  while (mode == 1) {
    long newPosition = myEnc.read();
    if (newPosition > oldPosition) {
      freq = freq + delta;
      if (freq > 40000000) {
        freq = 40000000;
      }
      int32_t tuning_word = (freq * 0x100000000LL)/124999100L;
      SetFrequency(tuning_word);
      Serial.print("CW Freq: ");
      Serial.print(long(freq));
      Serial.println(" Hz");
    }
    if (newPosition < oldPosition) {
      freq = freq - delta;
      if (freq < 0) {
        freq = 0;
      }
      int32_t tuning_word = (freq * 0x100000000LL)/124999100L;
      SetFrequency(tuning_word);
      Serial.print("CW Freq: ");
      Serial.print(long(freq));
      Serial.println(" Hz");
    }
    oldPosition = newPosition;
  }
}

void sweep() {
  Serial.println(" Sweep Mode");
  tuning_word_F1 = (F1 * 0x100000000LL)/124999100L;
  tuning_word_F2 = (F2 * 0x100000000LL)/124999100L;
  tuning_word_fStep = (fStep * 0x100000000LL)/124999100L;
  while (mode == 2) {
    long timeSweepStart = micros();
    //for (int i = 0; i < 10000; i++) {
    long timeStepStart = micros();
    while (tuning_word <= tuning_word_F2) {
      long timenow = micros();
      if ((timenow - timeStepStart) >= timeDwell) {
        timeStepStart = timenow;
        SetFrequency(tuning_word);
        tuning_word = tuning_word + tuning_word_fStep;
      }
    }
    tuning_word = tuning_word_F1;
    //}
    long timeSweepStop = micros();
    long timeSweep = timeSweepStop - timeSweepStart;
    if (timeSweep < 1000) {
      Serial.print(timeSweep);
      Serial.println(" us");
    }
    else if (timeSweep < 1000000) {
      Serial.print(double(timeSweep)/1000);
      Serial.println(" ms");
    }
    else if (timeSweep >= 1000000) {
      Serial.print(double(timeSweep)/1000000);
      Serial.println(" s");
    }
  }
}

void SetFrequency(int32_t tuning_word) {
  PORTB = PORTB & B11111011; // clear pin 10
  SPI.transfer(tuning_word);
  SPI.transfer(tuning_word >> 8);
  SPI.transfer(tuning_word >> 16);
  SPI.transfer(tuning_word >> 24);
  SPI.transfer(0x0);
  PORTB = PORTB | B00000100; // set pin 10
}

...