I'm building, to start with, an rpm monitor, using a Hall-sensor. The sensor is hooked to interrupt 0 and is working correctly. What I want to do is display RPM, based on 2 pulses per rotation. For that I got this piece of code:
//Hall sensor interrupt pins
int frontWSP = 0;
//Actual wheel speed in RPM
volatile float frontSpeed = 0;
//Time elapsed since last pulse
volatile unsigned long lastFrontPulse = 0;
//Number of magnets in 1 rotation: recommendation is not to use more than 6 magnets per wheel.
//At 360km/h you get 396 reads/second, so about 2.5ms per pulse, which is pretty much :)
long numberOfPulsesFront = 2;
volatile long pulseCount = 0;
volatile long tpp = 0;
volatile long tpr = 0;
void setup() {
attachInterrupt(frontWSP, frontPulse, RISING);
lcd.print("Initialized!");
delay(1000);
}
void loop() {
Serial.begin(9600);
printWSP();
delay(1000);
}
//Return RPM
long calculateRPM(long lastPulse, long numberOfPulsesPerRotation) {
//Time per pulse
tpp = millis() - lastPulse; //LINE1
//Time per rotation
tpr = tpp*numberOfPulsesPerRotation; //LINE2
return 60000/tpr; //LINE3
}
void frontPulse() {
pulseCount++;
frontSpeed = calculateRPM(lastFrontPulse, numberOfPulsesFront);
lastFrontPulse = millis();
}
//Wheel speed in RPM will be cast to int for display reasons
void printWSP() {
String smsg = "Front: ";
smsg += (int)frontSpeed;
Serial.println(smsg);
Serial.println(lastFrontPulse);
Serial.println(pulseCount);
Serial.println(tpp);
}
If I run this, I get
Front: 0
0
0
0
on the serial output (and yes, the Hall sensor is getting triggered :)).
In the code are lines commented as LINE1, LINE2 and LINE3. If I comment LINE1 and LINE2 out, and replace tpr by a fixed value on LINE3, it all goes well. Well, that is, there are numbers being written. This made me think that calculateRPM is buggy for some reason. After some fiddling with the variables and replacing them by fixed values, I had some intermittent success, although I didn't really discover a pattern yet. This made me think that there must some sort of obvious issue with typing.
My programming background is limited to VB6, .NET (C#) and a tiny bit of Java. All these languages aren't that sensitive when it comes to typing. Is anyone able to spot the mistake? And maybe elaborate a bit so I won't make the same one next time?
Literals (60000) are treated as ints, in the absence of any contrary directives. 60000 is not in the range of values that fit in an int. You need to tack L on the end, to tell the compiler to treat the value as a long.
calculateRPM() should not be called from the interrup handler. It should only be called when needed, in printWSP.
The only thing happening in the interrupt handler is incrementing pulseCount;
This causes the RPM count to be really inaccurate though, since I'm only using 2 magnets at the moment, with a refresh of 100ms. The ultimate application is a traction control system, so accuracy needs to be really high.
I'm gonna run a quick test with 6 magnets and see how that improves things
One of the first things I'd look at was calculating revolutions per second, not revolutions per minute.
Timing the interval between pulses using micros() instead of millis() might help, too.
PS: There's a reason why I recommended L, not l. It's very hard to distinguish between 60000l and 600001, for some fonts, but distinguishing between 60000L and 600001 is easy.
It's impossible to say why the displayed RPM is inaccurate without seeing the rest of the sketch, in particular how you are calling calculateRPM. I agree with Paul's advice to use micros() rather than millis() to do the timing. I'd do something like this:
The reason the RPM is inaccurate is because of the low refresh (100ms isn't really enough time to get an accurate count from 2 magnets). I don't think that doing the calculations using micros would improve things, since the time between pulses is never less than 2ms.
I'll try some more of the suggestions here and get back later.
It's impossible to say why the displayed RPM is inaccurate without seeing the rest of the sketch, in particular how you are calling calculateRPM.
PS: There's a reason why I recommended L, not l. It's very hard to distinguish between 60000l and 600001, for some fonts, but distinguishing between 60000L and 600001 is easy.
sylvaingirard:
The reason the RPM is inaccurate is because of the low refresh (100ms isn't really enough time to get an accurate count from 2 magnets). I don't think that doing the calculations using micros would improve things, since the time between pulses is never less than 2ms.
You didn't say you were calling calcRPM every 100mS (that's why I said I couldn't tell what was wrong without seeing the whole sketch).
To get better accuracy, you could have just 1 magnet and record the time returned by micros() every N pulses for some suitable N, like this:
static unsigned long lastTime;
const int N = 10;
void frontPulse() {
pulseCount++;
if (pulseCount == N) {
unsigned long now = micros();
frontSpeed = (60e6 * N)/(now - lastTime);
pulseCount = 0;
lastTime = now;
}
}
