Button Interrupts on 2560

So I have three buttons attached to 19/21 and 21 (interrupts 4,3,2) and they are wired according to this schematic: (without the led)
http://www.dave-auld.net/index.php?option=com_content&view=article&id=107:arduino-interrupts&catid=53:arduino-input-output-basics&Itemid=107

I dont know what is going on, I have tried rising/falling/high/low but this code does not seem to display anything to my lcd when I presss the buttons. I took my multimeter to the pins of one of the buttons very lightly, and then that interupt text appeared. It was really odd.
i wonder if anyone can spot my mistake, because it seems like there's a poltergeist in the room...

#include <Adafruit_GFX.h>
#include <Adafruit_PCD8544.h>
Adafruit_PCD8544 display = Adafruit_PCD8544(52, 51, 50, 48, 49);
#define NUMFLAKES 10
#define XPOS 0
#define YPOS 1
#define DELTAY 2
#define LOGO16_GLCD_HEIGHT 16 
#define LOGO16_GLCD_WIDTH  16 

int pbInLeft = 2;                  // Interrupt2 on pin 21
int pbInUse = 3;                  // Interrupt3 on pin 20
int pbInRight = 4;
void setup()   {                
  display.begin();

  display.setContrast(50);
  attachInterrupt(pbInRight, Right, FALLING);
  attachInterrupt(pbInUse, Use, FALLING);
  attachInterrupt(pbInLeft, Left, FALLING);
  // text display tests
  display.setTextSize(1);
  display.setTextColor(BLACK);
  display.setCursor(0,0);
  display.println("Hello, world!");
  display.setTextColor(WHITE, BLACK); // 'inverted' text
  display.println(3.141592);
  display.setTextSize(2);
  display.setTextColor(BLACK);
  display.print("0x"); 
  display.println(0xDEADBEEF, HEX);
  //display.display();
  delay(2000);
}
void loop() {
  int value = analogRead(15);
  display.clearDisplay();
  display.setCursor(0,0);
  display.println(value);
  display.display();
  delay(10);
}
void Right()
{
  static unsigned long last_interrupt_time = 0;
  unsigned long interrupt_time = millis();
  // If interrupts come faster than 200ms, assume it's a bounce and ignore
  if (interrupt_time - last_interrupt_time > 100)
  {
    display.clearDisplay();
    display.setCursor(0,0);
    display.println("Right");
    display.display();
    delay(1000);
  }
  last_interrupt_time = interrupt_time;
}

void Left()
{
  static unsigned long last_interrupt_time = 0;
  unsigned long interrupt_time = millis();
  // If interrupts come faster than 100ms, assume it's a bounce and ignore
  if (interrupt_time - last_interrupt_time > 100)
  {
    display.clearDisplay();
    display.setCursor(0,0);
    display.println("left");
    display.display();
    delay(1000);
  }
  last_interrupt_time = interrupt_time;
}
void Use()
{
  static unsigned long last_interrupt_time = 0;
  unsigned long interrupt_time = millis();
  // If interrupts come faster than 100ms, assume it's a bounce and ignore
  if (interrupt_time - last_interrupt_time > 100)
  {
    display.clearDisplay();
    display.setCursor(0,0);
    display.println("Use");
    display.display();
    delay(1000);
  }
  last_interrupt_time = interrupt_time;
}
[/code
sorry its long, theres not much there though!

You can't use delay() in an ISR. The code in an ISR is supposed to be fast. Really, really fast. Nothing in your ISRs bears any relationship to fast.

Clearing the display on an LCD takes time. Printing to an LCD takes time. A delay(), even if it did work, takes time. None of that stuff belongs in an ISR.

Your loop() function is not doing anything such that interrupts to handle, immediately, a switch press, are needed.

Is that what it is?
By ISR (I know what that is when i program avrs in AVS) I assume you mean the functions calls, i.e right left and use. I had wondered whether they are being called from the .
OK I'll set some flags and try that
Thanks

By ISR ... I assume you mean the functions calls, i.e right left and use.

ISR = Interrupt Service Routine - a function (routine) that is called to handle (service) an interrupt

yeah but for some reason I have/had assumed the ISR sets a flag to call the functions that I write, I didnt think it was calling it directly, hence putting alot of code in there was ok.....

yeah but for some reason I have/had assumed the ISR sets a flag to call the functions that I write, I didnt think it was calling it directly, hence putting alot of code in there was ok.....

The thing that generates the interrupt sets a flag that says that an interrupt (of a specific type) occurred. At the soonest opportunity, the highest priority interrupt is handled (not all interrupts are the same priority). Handling the interrupt means calling whatever ISR is registered to handle the specific interrupt.

That could include nothing (pin 2 goes HIGH but there is no RISING interrupt handler registered) or some system defined code (pin 0 goes HIGH indicating serial data is arriving) or some user defined code (pin 2 goes LOW and attachInterrupt() was used to register a handler for the FALLING event).

So, your ISR IS called, somewhat indirectly, when an interrupt occurs. There are plenty of interrupts that can happen - the clock ticking, serial data arriving, pins 2 or 3 (in the non-Mega varieties) going HIGH or LOW - and the system needs to be able to handle all of them before data gets lost. That is why an ISR needs to be fast.

Further interrupts are disabled while an ISR is running. This is why delay() can't be used in an ISR, since it relies in the clock ticking to know when to end, and the clock doesn't tick during your ISR. Serial data transmission also relies on interrupts which is why serial data doesn't get sent during an ISR. When the ISR returns, interrupts are enabled again.

Setting a flag in the ISR is the proper approach, but that means that the loop() function, or anything it calls, needs to check the flag often.

Since this is the case, interrupt handling is not as useful as a lot of people seem to think. For instance, attaching an interrupt handler to pin 2 will let your sketch know immediately if the switch attached to that pin is pressed or released.

The ISR then sets a flag to remember that this occurred.

Meanwhile, loop() called a function that contains delay(1200000UL). The fact that the flag is set is not going to be noticed for some (possibly quite long) period of time.

Once the delay ends, the sketch can then note that while it was taking a nap the switch was pressed. However, you can see that the action that the user expected when pushing the button is still delayed.

It's generally much better to write code that polls the switch often enough that interrupts are not needed. Of course, this means not using delay() and restructuring your program, but it makes your program react much more quickly to user/external input.