Speedometer with interrupts

Hi folks,

I am building a speedometer using a hall effect sensor. When the south pole of the magnet faces the sensor, the output is 5V.
When the north pole faces it, the output goes back to 0V. I am using an interrupt to make a speedometer, but it isn't working.
I put in a blink just to check, and that seems to work fine. Anyway, here is the code:

int diameter = 47.5;
int speedk;
unsigned long startHall = 0;
unsigned long stopHall = 0;
unsigned long elapsed;

void setup() {
  Serial.begin(9600);
  attachInterrupt(0, speedCalc, CHANGE);
  pinMode(13, OUTPUT);
}

void loop() {
  Serial.println(speedk);
  delay(1000);
}

void speedCalc() {
  if (digitalRead(2) == HIGH) {
    startHall = millis();
    digitalWrite(13, HIGH);
  }
  if (digitalRead(2) == LOW) {
    stopHall = millis();
    elapsed = stopHall - startHall;
    speedk = (((PI * diameter) / 2) / elapsed) / 100000;
    digitalWrite(13, LOW);
  }
}

Am I doing anything wrong, because my serial monitor only shows zeros?

I believe speedk needs to be declared volatile.

http://arduino.cc/en/Reference/AttachInterrupt
"You should declare as volatile any variables that you modify within the attached function."

The variables used in the ISR should be declared as volatile. You should do as little as possible in the ISR, such as incrementing a variable, and do the bulk of the work in the main loop assuming, of course, that you need an interrupt at all.

I think you are using the same pin for interrupt and for testing the value. I haven't checked the datasheet but I wonder if the value of a pin that triggers an interrupt remains unchanged for a subsequent read. Or perhaps you can get the value directly from the register associated with the interrupt.

Don't do two separate reads for high and for low - the value might change between the reads. Read the value into a variable and use that for your if clauses.

If a high always follows a low why bother with anything except the time between successive interrupts? And I think there is a special mode in the 328 for measuring that interval.

...R

I would trigger on the RISING edge only, that way you have half as many interrupts and you only have to measure the time between them instead of wasting processor bandwidth figuring out if it's rising or falling.

Hi,
thanks for the responses. I'm using digital pin 2 for the interrupt and also pin 13 for the on board led, but that is just to make sure the loop works. I changed the code so that the interrupt is more efficient and I have changed the variables used in the funciton to volatile, however this did not fix the problem, because I'm still just getting "0" on the serial monitor. Here is the code:

int diameter = 47.5;
volatile int speedk;
volatile unsigned long prevHall = 0;
volatile unsigned long elapsed;

void setup() {
  Serial.begin(9600);
  attachInterrupt(0, speedCalc, RISING);
  pinMode(13, OUTPUT);
}

void loop() {
  Serial.println(speedk);
  delay(1000);
  if (digitalRead(2) == LOW) {
    digitalWrite(13, LOW);
  }
}

void speedCalc() {
  elapsed = millis() - prevHall;
  prevHall = millis();
  speedk = (((PI * diameter) / 2) / elapsed) / 100000;
  digitalWrite(13, HIGH);
}

What am I doing wrong?

The problem may be that you have defined speedk as an integer but you are doing some complex maths that may give a floating point answer.

...R

I have changed it to a floating point, but I'm still getting just 0.00 now. Seriously, I have no idea what's going wrong. Is there anyone who can confirm that the code is correct. I am using a hall effect sensor on digital pin 2 for the interrupt.

int diameter = 47.5;
volatile int speedk;
volatile unsigned long prevHall = 0;
volatile unsigned long elapsed;

void setup() {
Serial.begin(9600);
attachInterrupt(0, speedCalc, RISING);
pinMode(13, OUTPUT);
}

void loop() {
Serial.println(speedk);
delay(1000);
if (digitalRead(2) == LOW) {
digitalWrite(13, LOW);
}
}

void speedCalc() {
elapsed = millis() - prevHall;
prevHall = millis();
speedk = (((PI * diameter) / 2.0) / elapsed) / 100000.0;
digitalWrite(13, HIGH);
}

check the red marks.

also, make a small sketch that only does the speedk calculation. once you get this running, add the rest of the code for leds, interrupts, etc.

Thanks,

I changed the two red bits at the bottom, but i'm not sure what you meant with the red and the black "int" at the top, for I already had that. I did change the black int to float though. It still isnt working and only displaying 0.00. I'm pretty sure the calculation is correct. Again, what's going wrong?

Try printing elapsed. What do you get ?

but i'm not sure what you meant with the red and the black "int" at the top

An int can't contain a fractional part, so it will be truncated to 47.

I would try very hard not to do floating point arithmetic in an interrupt.

I printed elapsed and that worked fine, so something must be going wrong with the calculation. Not sure what though...

Now make ((PI * diameter) / 2.0) into a const. No need to do the calculation every time.
Shouldn't the variables be floats, particularly when PI is not an integer of any kind ?

I still think that you should move the calculation out of the ISR and turn off interrupts whilst doing the calculations because reads from any numeric variables are not atomic and bytes within them could change whilst they are being read or updated.

What do you calculate should be the answer? If you take the elapsed number you're getting and plug it into the equation, what should the answer be? I'm guessing it is a very small number. Maybe too small for your accuracy of the print line.