I like the way you used interrupts in your code. My version does not use interrupts and the displayed rpm is not stable.
I didn't realize you can stop the millis() clock with interrupts. My current PWM of 25khz is implemented using timers, the TCCR1A, TCCR1B, ICR1 stuff. Will interrupts screw up the PWM signal?
Thanks
millis() returns the value of an internal variable that is updated every 1.024 milliseconds via
TIMER0_OVF_vect ISR. Pin 2 external interrupt is handled by the
INT0_vect ISR which Arduino hides behind the
attachInterrupt() call. Assuming 1000 RPM fan, that's 16.6 times per second or an interrupt every 60 milliseconds.
The
noInterrupts() call stops all ISRs from executing. All timer interrupts and the external interrupt raise a flag when their interrupt occurs (see:
TIFR0:TOV, TIFR1, EIFR). And will run their ISR (and clear the flag) as soon as the call to
interrupts() re-enables global interrupts.
But we shouldn't stay in this interrupts-disabled state too long otherwise subsequent interrupts will be missed (flag still set, ISR yet to run when a newer interrupt arrives).
TIMER0 is set and forget by Arduino runtime initialization.
TIMER1, the one you are using, should also be set and forget until the need to change the PWM value. I'm not sure of the need to call any ISR for TIMER1 (
ICR1?).
So by keeping the interrupts-disabled state to a minimum (kept way below min(1ms,16ms)+overhead), you shouldn't lose any interrupts. And
TIMER1 should still be outputting the same PWM even if the CPU is in an ISR. It'll just need to finish before you can change to a new PWM value.
I've updated the code below to do the bare minimum amount of work; just take a snapshot of the vars (5 assignments + 1.5us interrupt overhead, I think). That way any slower math and floating point operations can be taken out of the critical section. Also, the rate is in RPM to avoid a divide by zero (in the last version).
EDIT: We don't stop the millis() clock; just hold-off updating it by preventing the ISR to run briefly.
EDIT2: Localized the rate calculating logic to the getTachRateInRPM() function.
EDIT3: Confirmed that the code works. It just needed a cast to float in the function return.
const byte interruptPin = 2; // or 3 would work too
volatile int counts = 0;
unsigned long last = 0;
void incCount() {
counts++;
}
void setup() {
pinMode(interruptPin, INPUT_PULLUP); // tach output needs pullup to MCU Vcc
attachInterrupt(digitalPinToInterrupt(interruptPin), incCount, FALLING);
}
float getTachRateInRPM() {
noInterrupts(); // stop the count and millis() update for a brief moment
unsigned long now = millis();
unsigned long lastLast = last;
int lastCounts = counts;
last = now;
counts = 0;
interrupts(); // carry on counting
// may need to divide counts var by n if the fan generates n counts per revolution.
return (float)lastCounts / (last - lastLast) * 1000 * 60;
}
void loop() {
float rate = getTachRateInRPM();
delay(1000); // do other stuff or wait some brief period of time before calculating rate again
}