Problem with RPM Photo Interrupter/Motor Speed

Hi,

I'm putting an rpm sensor on the end of a motor using a photo interrupter, and I'm having issues getting accurate feedback when I adjust the motor speed that I'm reading the rpm on. At full speed, I get a stable rpm value, about 3400 rpm -which seems accurate, but when I slow the motor speed down to anything below a 255 analogWrite value, the rpm values jump inconsistently from about 16000 to 172000 rpm, regardless of the speed. Any thoughts?

volatile byte counts;
unsigned long rpm;
unsigned long timeold;
long prevTime;

int motorPin = 9;
int analogPin = A0;
int analogVal;
int N = 2; //number of blades on fan
int pwm;

void setup() {

  Serial.begin(9600);
  attachInterrupt(digitalPinToInterrupt(2), rpm_fun, RISING);
  //  attachInterrupt(0, rpm_fun, RISING);
  counts = 0;
  rpm = 0;
  timeold = 0;
}


void loop(){

  pwm = 90;
  analogWrite(motorPin, pwm);

  if (counts >= 100) {
    long interval = millis() - prevTime;
    prevTime = millis();
    //Update RPM every 20 counts, increase this for better RPM resolution,
    //decrease for faster update
    rpm = (60 / N) * 1000 / (millis() - timeold) * counts;
    timeold = millis();
    counts = 0;
    Serial.print("OutputVal= ");
    Serial.print(pwm);
    Serial.print(" RPM= ");
    Serial.print(rpm, DEC);
    Serial.print(" millis = ");
    Serial.println(interval);
    //    adjRPM(2600);
  }
}

void rpm_fun() {
  counts++;
}

I think you need to save the value of counts to another working variable and reset the value something like this

latestCounts = counts;
counts = 0;
prevTime = millis();
rpm = (60 / N) * 1000 / (millis() - timeold) * latestCounts;
// etc

That will ensure that the rest of the code interferes as little as possible with the counting.

However I suggest you use a different approach - measure the time for each revolution using micros().

This code is derived from a project where I control the speed of a small DC motor. It is not complete but it should give you the idea.

volatile unsigned long isrMicros;
unsigned long latestIsrMicros;
unsigned long previousIsrMicros;
volatile boolean newISR = false;


void loop() {
   if (newISR == true) {
     previousIsrMicros = latestIsrMicros; // save the old value
     noInterrupts(); // pause interrupts while we get the new value
        latestIsrMicros = isrMicros;
        newISR = false;
     interrupts();
     microsThisRev = latestIsrMicros - previousIsrMicos;
}

void myISR() {
   isrMicros = micros();
   newISR = true;
}

...R