Reading inputs with interrupts

Hey, i'm trying to read an input using an interrupt. I have 4 external switches, all wired into a 4 input or. The output of this 4 input NOR goes into pin 2 to trigger the interrupt. What I need is for the interrupt to instantaneously read the switches and see which is high (the interrupt will be triggered by a LOW signal, therefore when the switches are on, the NOR gate will go low).

What I need though is that the read value can never be zero, if the value hasn't changed, it stays as it was. It only changes if one of the other switches is pressed at the time of interrupt.

I had a go at the code but once the interrupt had been initially done, the variable value would stay at the initial value, irregardless of the other switches.

I'm using direct port read to read all the switches at once.

Sorry if this is a tad hard to understand, it is half 2 in the morning haha

int pin;
int flag;

void setup(){
  Serial.begin(9600);
  attachInterrupt(0, string, LOW);
  DDRC = DDRC | B00000000;  
}

void loop(){
 
Serial.println(pin);
delay(10);}

void string(){
  flag=0;
while(flag==0){
  if(pin==0){
    pin=PINC;
    }
  else{
    flag=1;
  }
}

Not sure if it's the problem you are seeing but when using global variables inside ISR functions, you must declare the variables as volatile.

So

int pin;
int flag;

should be

volatile int pin;
volatile int flag;

See if that changes the symptoms you are seeing.

Lefty

You probably want to set the interrupt to trigger on FALLING rather than LOW (which I fear might continuously fire if the pin isn't reset to high during the interrupt handler)

Hey, cheers for the help. After trying both of these, neither have solved the problem unfortunately, I think the problem lies in the code. What I want to ensure is that the variables can never equal zero and they only change if a newly read value is not zero. Since i'm reading port C all at once, the bit number will lie with the corresponding input, e.g. A0 will produce 1, A1 will produce 2, A2 will produce 4, A3 will produce 8 and so on.

I know that the state of the switch will change very fast but it just doesn't seem to read a new state one the interrupt occurs.

Anyway I can sort this?

Thanks,

void string(){
  flag=0;
while(flag==0){
  if(pin==0){
    pin=PINC;
    }
  else{
    flag=1;
  }
}

This code is poorly indented and completely uncommented. You need to correct both of those issues before we can help you.

Apologies there Paul! I've rewritten the code with all comments and correct indentation (Sorry about that, it was a rushed job.)

I've figured now that if I read port C and store it in the pin variable, before doing my while loop, it does exactly what I want it to do but unfortunately, about 50% of the time, jams up the arduino (probably stuck in a loop somewhere) where I have to tap one of the switches multiple times for it to exit the loop. The new code is:

volatile int pin;
volatile int flag;

void setup(){
  Serial.begin(9600);   //Begin serial transmission
  attachInterrupt(0, string, FALLING);   //Create interrupt which is on pin 2 using the interrupt routine called string on the falling edge of the trigger
  DDRC = DDRC | B00000000;    //Change all of port C to inputs, Port C being tha analog ins
}

void loop(){
  Serial.println(pin);   //Print the current pin value to serial monitor
  delay(10);   //Sending delay as to not fill the serial buffer
}

void string(){   //Interrupt routine when pin 2 is on the falling edge
  flag=0;    //Initially set the while flag to zero as to ensure the loop will always function
  pin=PINC;    //Initially read the value of the pin
  while(flag==0){   //Make a while loop dependent on whether the flag is equal to zero or not. Flag can be either zero or one.
    pin=PINC;    //Read the port and store the value in the volatile variable called pin
    if(pin==0){    //If the value of pin is zero, read the value again, this ensures that no zero values can be used for pin (apart from when the device is initially turned on)
      pin=PINC;
    }
    else{
      flag=1;    //If the value of pin is not zero, flag now equals 1 and the while loop will end presenting pin to the loop section of code as a value.
    }
  }
}

Once again, My apologies that Paul!

The first thing I get stuck at is why the function is called string(). It certainly doesn't create a string, do any processing with strings, nor is it in any way a string handling function.

I'm wondering what the purpose of the while loop is. Apparently one of the switches connected to port C triggered the interrupt. pin is set to the byte defined by PINC. So, theoretically, you now know which switch triggered the interrupt.

What is the purpose of the while loop?

volatile int pin;
volatile int flag;

You probably can live with using unsigned char type on those variables.

  DDRC = DDRC | B00000000;    //Change all of port C to inputs, Port C being tha analog ins

ORing 0s does nothing. To clear the bits, you want to AND 0s instead.

  while(flag==0){   //Make a while loop dependent on whether the flag is equal to zero or not. Flag can be either zero or one.

Be careful with using while inside of an isr: you don't want to hang your code there.

Yeh, its just called string because the switches are switched with a piece of string in the real world so I thought i'd just call the routine string, not string in the sense of C.

I've included a schematic diagram of whats happening in the real world, there are pull down resistors on each of the switches, it was just hard to draw them here.

The while loop is included to ensure that there is never a non zero reading (apart from when the arduino is first activated) should I include some form of millis based time out so that if more than 100ms has passed, flag automatically = 0 or just not use a while loop at all?

About the OR'ing, I was just going off the port manipulation arduino tutorial.

string interrupter.png

its just called string because the switches are switched with a piece of string in the real world

Interesting 1, readability 0.


Rob

just not use a while loop at all?

That loop makes no sense to me. Sounds like its intend is to detect an idle condition (all of PINC is low). You obviously can detect that in your code by testing pin.

Hey, cheers for all the replies.

The while loop is just there to ensure that a zero reading is never produced. The interrupt will only occur when one of the switches is pressed. The only way that the interrupt will have a moment when port c is zero would be if the nor gate was malfunctioning. Since my idea doesn't seem to be working properly, would anybody be able to send me in the correct direction with a small snippet of code?

Remembering, the pin variable can only be zero once the arduino is initialized, for the duration of the arduino being on, the pin variable should be a value corresponding to which switch was last on while the port was being checked.

Thanks,

The while loop is just there to ensure that a zero reading is never produced.

Which should be dealt with in loop(), not in the ISR. The interrupt will only be triggered if a switch is pressed. There is no reason, or way, to then wait in the ISR until a switch is pressed.

Since my idea doesn't seem to be working properly, would anybody be able to send me in the correct direction with a small snippet of code?

Sure:

void string()
{   //Interrupt routine when pin 2 is on the falling edge
  pin=PINC;    //Initially read the value of the pin
}

Hi Paul, thank you for that. So I see that the ISR is only reading the pins which makes sense actually (when the nor gate is low, one switch HAS to be on so theoretically there should always be a present value on portc) how do I then within the loop make sure that the pin value is never zero?

Cheers,

How to implement it will depend on how your code uses "pin".

As is, I would initialize pin to an impossible value: impossible in the sense that your input pins could never read such a value. For example, if your 4 pins are connected to the lower 4 bits, I would initialize pin to the reverse of that.

#define PIN_MASK 0x0f //pins connected to the lowest four bits
...
unsigned char pin = ~PIN_MASK; //initialize pin

ISR():
  pin = PINC & PIN_MASK; //read pin
  flag = 1; //indicating new data

...
main():
  if (flag) { //new data has arrived
    flag = 0; //reset flag
    do_something_with(pin); //process pin
...

pyrohaz:
how do I then within the loop make sure that the pin value is never zero?

Initialise it to a non-zero value and never set it to zero.