I'm trying to Interrupt with a switch. Can I do this?

Set up an interrupt (INT or IOC) and in the isr, set / clear a flag as approriate. In the main loop, process that flag.

dhenry, could you possibly break that down into what that'd translate to as far as a small snippet of code?

http://arduino.cc/en/Reference/AttachInterrupt

Seriously, as others have said - you have no need to use interrupts. Take a look at the blink without delay sketch supplied with the IDE. Take a look at Nick’s links above. For a first step, you need to adapt your servo code to do its thing without delays. Once you have that it’ll be fairly trivial to poll the switch to control the servo. Your latency will be considerably less than a second too.

I wrote this on my phone, so it is completely uncompiled or tested, but replaces the delays in the sweep example:

#include <Servo.h> 
Servo myservo; 

const int upperLimit = 180;
const int lowerLimit = 0;
int pos;
bool rampUp = true

unsigned long lastTime;  
unsigned long moveInterval = 15;

void setup() 
{ 
  myservo.write (pos);
  myservo.attach(9); 
} 
 
void loop ()
{
  unsigned long timeNow = millis ();
  if (timeNow - lastTime > moveInterval) {
    lastTime = timeNow;
    if (rampUp) {
      pos++;
      if (pos >= upperLimit) {
        rampUp = false;
      }
    } else {
      pos--;
      if (pos <= lowerLimit) {
        rampUp = true;
      }
    }
    myservo.write (pos);
  }
}

Sure. This would be a quick sketch and hopefully it will send you on the way:

//define pin connection here
#define INT_PIN    2      //int0 pin on pd2, active now (falling edge). do not change the number
//

volatile unsigned char key_pressed=0;  //flag for key pressed. 1=key pressed; 0=key not pressed

//int0 isr
ISR(INT0_vect) {
  if (key_pressed==0) {              //now, key is pressed
    key_pressed = 1;                 //set the flag
    //now active isr on the rising edge, to detect the release of the key
    EICRA = (EICRA & ~0x03) |        //preserve eicra bit 7..2
          (1<<ISC01) | (1<<ISC00)    //interrupt on int0's rising edge
          ;
  } else {                           //now, the key is released
    key_pressed = 0;                 //reset the flag
    EICRA = (EICRA & ~0x03) |        //preserve eicra bit 7..2
          (1<<ISC01) | (0<<ISC00)    //interrupt on int0's falling edge
          ;
  }    
}

void int0_init(void) {
   //set up direction register
  pinMode(INT_PIN, INPUT_PULLUP);    //int pin configure as input, pull-up activated
  
  //set up the isr
  EICRA = (EICRA & ~0x03) |          //preserve eicra bit 7..2
          (1<<ISC01) | (0<<ISC00)    //interrupt on int0's falling edge
          ;
  EIFR |= (1<<INTF0);                //clera int0 interrupt flag
  EIMSK |= (1<<INT0);                //activate int0 interrupt
 
}

void setup(void) {
  //initialize int0 isr
  int0_init();
  
  //active interrupts
  interrupts();
}

void loop(void) {
  if (key_pressed) {
    //key is pressed
    //do something with it
  } else {
    //key is not pressed
    //do something else
  }
}

It sets up int0 pin (digital 2) as external interrupt, active now. The code detects a falling edge, and sets key_pressed. After that, it detects a rising edge (key released) and clears key_pressed to signal that the key has been released.

In the main loop, you process key_pressed only.

The same function can be also coded with ioc and probably more cleanly.

You will need to rewrite your main loop function now so that it can process key_pressed.

Something like this will work:

#define LOOP_MAX  10      //how many times you want to run the loop before change servo position
#define LOOP_DLY  10      //delay in each loop.


...
void loop(void) {
  static unsigned short loop_counter = 0;

  if (key_pressed) {
    loop_counter += 1;       //increment loop counter
    if (loop_counter == LOOP_MAX) {  //time to advance the servo
      loop_counter = 0;       //reset loop counter
      //advance the servo
    }
  } else {
    loop_counter = 0;         //reset loop counter
    //stop the servo
  }
  delay(LOOP_DLY);          //delay the loop a little bit
}

As long as the key is pressed, the code counts the number of times it has been executed, and if it has reached the maximum number of execution (defined by LOOP_MAX), it moves the servo.

LOOP_DLY is to add some time to the loop so that LOOP_MAX does not need to be huge.

Something like this will respond to a key pressed quickly (no longer than LOOP_DLY).

Hope it helps.

Warning: untested code.

Thanks to all for the ideas. I'll see if I can get them to work and I'll have another look at Nick's tutorials as well.

The key is to keep the main loop short in execution. You do not want to tie it up for two long and I would think 10ms is the maximum.

You do not want to tie it up for two long and I would think 10ms is the maximum.

I'd say 10ms is more than 9ms too long for what you have to do.

ISR(INT0_vect) {

if (key_pressed==0) {              //now, key is pressed
    key_pressed = 1;                //set the flag
    //now active isr on the rising edge, to detect the release of the key
    EICRA = (EICRA & ~0x03) |        //preserve eicra bit 7…2
          (1<<ISC01) | (1<<ISC00)    //interrupt on int0’s rising edge
          ;
  } else {                          //now, the key is released
    key_pressed = 0;                //reset the flag
    EICRA = (EICRA & ~0x03) |        //preserve eicra bit 7…2
          (1<<ISC01) | (0<<ISC00)    //interrupt on int0’s falling edge
          ;
  }   
}

This is far too complex in my opinion. There is already a perfectly easy to use system in the Arduino library: attachInterrupt. If you do an attachInterrupt on a CHANGE interrupt all you have to do is test the key state in the ISR.

This register-manipulation is very processor-specific and not easy to understand.

I have example code on my page here:

http://www.gammon.com.au/interrupts

I hope you already looked at it. The first example shows detecting a switch press:

const byte LED = 13;
const byte BUTTON = 2;

// Interrupt Service Routine (ISR)
void pinChange ()
{
  if (digitalRead (BUTTON) == HIGH)
    digitalWrite (LED, HIGH);
  else
    digitalWrite (LED, LOW);
}  // end of pinChange

void setup ()
{
  pinMode (LED, OUTPUT);  // so we can update the LED
  digitalWrite (BUTTON, HIGH);  // internal pull-up resistor
  attachInterrupt (0, pinChange, CHANGE);  // attach interrupt handler
}  // end of setup

void loop ()
{
  // loop doing nothing 
}

The “change” interrupt can respond to switch-on or switch-off. In this case it finds the current state and flashes an LED. You can change that part to do what you want.

@dhenry: I do not agree with using low-level registers when someone has taken the trouble to write a library to simplify things for you, unless you can demonstrate a clear advantage in a particular case.

In this particular case of interrupting a motor, all you need to do however is test the switch in the main loop: simpler again!

void loop ()
  {
  if (digitalRead (STOP_SWITCH) == LOW)
    {
    // stop motor
    }  // end of if

  // other stuff
  }  // end of loop

Don't over-egg the pudding.

For // other stuff, see reply #24

Exactly! As long as "other stuff" does not use delay() the main loop can be very responsive to switch presses.

Therefore dhenry's suggestion:

  delay(LOOP_DLY);          //delay the loop a little bit

... is not necessary. No reason to throw in arbitrary delays.

“Don’t over egg the pudding…” I like that Nick. Sort of like keep it simple stupid but with better taste;) I like it.

I tried the “if” and “digitalRead” in the loops and it doesn’t stop if it’s in it’s travel and executing code. There are times where the servo is travelling and it takes about 30 seconds of code execution before it hits the digitalRead telling it to exit. So I’ve basically had to pepper my program with a mess of digitalRead samplings interspersed.

It’s sort of working but I’d like to find a way to have a little cleaner/more readable code.

But of course my loops have “delay” in them. It’s actually an important part of the way I’m running the servo. I need to be able to have control over how fast or slow it gets to where it’s going.

777funk: But of course my loops have "delay" in them.

Yes, but see reply #24. "Of course" you can get rid of them. Life will be happier thereafter.

Programming without delay():

http://www.gammon.com.au/blink

But of course my loops have "delay" in them.

The issue with long delays is that they are not "interruptable": you push a button and the mcu doesn't respond.

You can use interrupts to cure that, or you can "interruptize" your long delays:

void mydelay(unsigned int dly) {
  while (dly--) {
    if(digitalRead(int_key)) break; //allows int_key to break mydelay, active high
    delay(1);
  }
}

Essentially, you have programmed the key presses to have a higher priority than the delays, the same concept as interrupts but without the interrupts.

AVRs excel in part because of their vectored interrupts. No one should be afraid of using them.

dhenry: The issue with long delays is that they are not "interruptable": you push a button and the mcu doesn't respond.

You can use interrupts to cure that ...

Not exactly cure it. If you are inside a delay(10000) then an interrupt will not make it leave prematurely. Please word your responses carefully. Your suggestion of "mydelay" would indeed allow a "delay" to exit early, but it doesn't use interrupts.

AVRs excel in part because of their vectored interrupts. No one should be afraid of using them.

They are indeed useful when used appropriately. If I choose not to use them in a particular situation, it is not fear that will drive my decision-making.