Speedometer displaying high values

Hello!
Recently I've been working on an Arduino Nano based speedometer for bicycles. it basically measures the rotations of a wheel (by using a Hall effect sensor which outputs LOW when a magnet is near) and converts it to speed. The speedometer, however, outputs a constant speed when the bicycle is not moving AKA no signal from the Hall effect sensor. there are no problems with the wiring or hardware, I've checked multiple times. The program uses an LCD to display the speed. I have figured out the constant speed that is output is somehow related to the integers wheelMagnets and the delay() in loop(). The speed decreases if wheelMagnets is increased and the same with the delay() in the loop(). Thank you for your time! P.S I am not quite good at programming and am of a young age yet.

#include<math.h>
#include<LiquidCrystal.h>
float wheelCirc = 2.2352;
float velocity;
float speed;
float speedcast;
float cadence;
int revs;
float wheelStart;
float wheelElapsed;
float cadStart;
float distance;
int distancerounded;
float distancecast;
float cadElapsed;
int wheelMagnets = 4;   //num of magnets on wheel
int cadMagnets = 4; //num of magnets on gear
const int rs = 9,en = 8,d4 = 7,d5 = 6,d6 = 5, d7 = 4;
LiquidCrystal lcd(rs,en,d4,d5,d6,d7);
int wheelSens = 2;
int revSens = 3;

void setup() {
  lcd.begin(16,2);
  pinMode(wheelSens,INPUT_PULLUP);
  pinMode(revSens,INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(wheelSens),speedCalc,FALLING);
  attachInterrupt(digitalPinToInterrupt(revSens),revCalc,FALLING);
  Serial.begin(9600);
}

void loop() {
  speedCalc();
  revCalc();
  lcd.print(speed);
  lcd.print(" km/h");
  lcd.setCursor(0,1);
  lcd.print(revs); 
  lcd.print(" RPM");
  delay(250);
  lcd.clear();
  Serial.println(speed);
}

void speedCalc(){
  wheelElapsed = millis()-wheelStart;
  wheelStart = millis();
  velocity = (3600*wheelCirc/wheelElapsed)/wheelMagnets;
  speedcast = int(velocity*10+0.5);
  speed = speedcast/10;
  distance + wheelCirc/wheelMagnets;
  distancerounded = int(distance+0.5);
}

void revCalc(){
  cadElapsed = millis()-cadStart;
  cadStart = millis();
  cadence = (60000/cadElapsed)/cadMagnets;
  revs = int(cadence+0.5);
}

Code.ino (1.35 KB)

Variables shared between interrupt and non-interrupt context should be qualified as "volatile".
I'm also inclined to say your interrupt service routines are doing too much work.

Can you please explain and/or tell me the changes I should make?

Here is a good place to start.

This line in your interrupt routine calculates a value then does nothing with it:

  distance + wheelCirc/wheelMagnets;

Why are you calling your interrupt routines from within loop()? Those functions are for servicing interrupts, and will be executed when the interrupt occurs.

darth_chikoo:
Can you please explain and/or tell me the changes I should make?

Change all the variables that are used inside and outside of an ISR ( Interrupt Service Routine ) to the volatile type. That is put the word volatile in front of the variable deceleration. For example:-

float speed;
should be 
volatile float speed;

That is just one example do that with all variables used inside AND outside an ISR

Also as the speedCalc and revCalc functions are called by interrupts they should NOT be called in the loop function.

While reading variables of more than one byte long outside an ISR then you need to :-

  1. Disable the interrupts
  2. Read or use the variable
  3. Enable the interrupts.

Do you know that millis() will not advance while inside an ISR?

Yes I do know, I just use them as a way to get the time frame between two LOW signals.

david_2018:
Why are you calling your interrupt routines from within loop()? Those functions are for servicing interrupts, and will be executed when the interrupt occurs.

I deleted those lines - and it did solve the problem and the meter now starts off at zero. However now the ISR is only triggered upon a LOW signal from the sensor and so the speed does not update if there is no signal. Therefore if I were to stop instantly, the speed would not be zero as it wouldn't update. Everything else seems fine and this is the only shortcoming it seems. Could you suggest a way around this? Perhaps a timeout that turns the values to zero?

  lcd.print(" RPM");
  delay(250);
  lcd.clear();

That is silly and will cause the LCD to flicker. Remove the delay and the clear.
Only update the parts of the display that actually change. Do the fixed part of the display in the setup function.

Therefore if I were to stop instantly, the speed would not be zero as it wouldn't update.

So you have a volatile variable in the ISR to simply count when that function is entered.
Then you look at that variable in the loop function and if it is non zero you clear it and update the display.
If it is zero for the first time you make a note of the time from the millis() function and set a flag saying it was timing.
When the current millis() reaches some threshold, say a second, you then display the zero speed.
In normal operation the clearing of the count variable will also be accompanied by the clearing of the timing flag.

Hi.

Looks like your learning a lot about the use of interrupts.

You might like to explore the PulseIn() function. This returns the duration the HE sensor is high (or low). That value can be directly converted to rpm. The code is trivial.

John.

The problem with using pulse in is it is blocking code and you could wait forever for the pulse to arrive, unless you specify a timeout.

Well, my sketch has nothing else to do, so nothing is being blocked.

I use a 5 second timeout -- if the HE state doesn't change for 5 seconds the sketch assumes a speed of zero.

All works very satisfactorily.

John.