RPM with other inputs

I have 2 inputs being monitored at the same time. The fist is the speed of a shaft (rpm) and the second is a level indicator (switch on if low off if high level)

I can't copy paste from the serial monitor but the readings are a constant speed of 2420 rpm but when the level drops and the switch puts SeedState HIGH the RPM reading goes up to 5410 rpm with the same real speed

I cant find a reason so all help appreciated. code below.

  //-----------------------------------------------
  int LowSeedPin = 7;     // the number of the low seed pin
  int SeedState = 0;         // variable for reading the seed status
  volatile byte rpmcount;

   unsigned int rpm;
   unsigned long timeold;

 void setup()
 {
  Serial.begin(9600);
    attachInterrupt(0, rpm_fun, FALLING);

    rpmcount = 0;
    rpm = 0;
    timeold = 0;
   
    pinMode(LowSeedPin, INPUT);  // initialize the seed switch pin as an input:
    
 }

 void loop()
 {

  SeedState = digitalRead(LowSeedPin);     
  // read the state of seed level:


  if (SeedState == HIGH) {       
    // if seed low switch on the SeedState is HIGH:
       
    Serial.println("**************LOW SEED******* "); 
    SeedState = 0; 
  } 
  
  
   if (rpmcount >= 20) { 
     //Update RPM every 20 counts
     
     detachInterrupt(0);
     rpm =60000000/(micros() - timeold)*20;
     
     Serial.print("rpm      ");
     Serial.println(rpm,DEC);
     
     timeold = micros();
     rpmcount = 0;
     
     attachInterrupt(0, rpm_fun, FALLING);
   }
 }

 void rpm_fun()
 {
   rpmcount++;
  
 }

//-----------------------------------------------

Add a Serial.print of rpmcount before

Serial.print("rpm ");
Serial.println(rpm,DEC);

and see what you get.

Second there are 3 different errors in the line

rpm =60000000/(micros() - timeold)*20;

So Serial.print() micros()-timeold and do the calculation your self and compare the Arduinos result and your own!.

Mark

Mark
I have added Serial.print of rpmcount before

 Serial.print("rpm      ");
     Serial.println(rpm,DEC);

and get 20 every time

I am a bit thick, what are 3 different errors in the line

rpm =60000000/(micros() - timeold)*20;

I did Serial.print() micros()-timeold and got 491060 micros which divided 60000000 by 60000000/491060=122.1846 and then multiplied by 20 = 2443 arduino gave me 2442

Tim

Now do the thing with the button so that $SeedState" is high.

Mark

holmes4:
Now do the thing with the button so that $SeedState" is high.

Mark

Makes no difference, RPM holds steady until SeedStae changes then it wanders around

TimFr:

  SeedState = digitalRead(LowSeedPin);     

// read the state of seed level:

if (SeedState == HIGH) {      
   // if seed low switch on the SeedState is HIGH:
     
   Serial.println("*******LOW SEED ");
   SeedState = 0;
 }

What's this code intended to achieve? Doesn't it produce an huge amount of serial output? That may be slowing your sketch down to the point that you get rpmcount > 20, which would screw your calculations up. Why don't you calculate based on the actual revolution count, not assume the count is 20?

By the way, rpmcount is a poor choice of name since it does not hold or count 'rpms'.

The previous advice about the Serial.print() statements extending the sampling time is valid. However, instead of waiting a certain time and seeing how many pulses you count that time, a better way is to measure and store the time interval between interrupts, and calculate the rpm from that interval. Like this:

volatile unsigned long lastPulseTime;
volatile unsigned long pulseInterval;

void rpm_func()
{
  unsigned long now = micros();
  pulseInterval = now - lastPulseTime;
  lastPulseTime = now;
}

void loop()
{
  noInterrupts();
  unsigned long interval = pulseInterval;
  interrupts;
  unsigned int rpm = 60000000L/interval;
  ... // do something with 'rpm'
}

You don't need to call attachInterrupt() in loop(), just calling it in setup() is enough.

Missing from this example is code to detect when the the pulses have stopped, and set rpm to zero when that happens. I leave that to you to write.

dc42:
a better way is to measure and store the time interval between interrupts, and calculate the rpm from that interval.

It looks to me as if that is what the code aims to do (but timing twenty ish revolutions rather than one).

PeterH:

dc42:
a better way is to measure and store the time interval between interrupts, and calculate the rpm from that interval.

It looks to me as if that is what the code aims to do (but timing twenty ish revolutions rather than one).

Yes, you are quite right. However, the code doesn't actually measure the time between one pulse and one 20 pulses later. It measures the length of an interval which is known to contain at least 20 pulses, but may start some time before the first pulse and end some time after the last one. Even if the code is corrected to calculate the RPM from the actual number of pulses recorded in that interval (rather than assuming it is 20), it will still be subject to sampling error because of this. Measuring the time between successive interrupts avoids most of this sampling error, as long as interrupt latency is low.

DC42

I tried compiling your suggestion but came across several problems. I am newbie so don't understand everything yet. this is what I have got

volatile unsigned long lastPulseTime;
volatile unsigned long pulseInterval;
unsigned long now;
unsigned long interval;
unsigned int rpm;

 void setup()
 {
   Serial.begin(9600);
   attachInterrupt(0, rpm_fun, FALLING);


 }



void rpm_func()
{
   now = micros();
  pulseInterval = now - lastPulseTime;
  lastPulseTime = now;
}

void loop()
{
  noInterrupts();
   interval = pulseInterval;
  interrupts;
   rpm = 60000000L/interval;
  ... // do something with 'rpm'
}

but came across several problems.

But, I'm not going to tell you what they are. I want you to help me anyway.

Can you see how silly that sounds?

Sorry about that; the problems were that each time I tried to, compile I got 'Undeclared errors. so I added this

unsigned long now;
unsigned long interval;

Then I got rpm not defined so I added thisunsigned int rpm;
but still got rpm not defined.
What is the difference between undeclared and not defined?

TimFr:
What is the difference between undeclared and not defined?

There are two ways to play this. Either you post your actual code and the actual error messages you get when trying to compile it, or let us guess. Which way do you think is best?

Will post all in about 15 days as I have the arduino installed and wired in another machine application.
Wish I had 2, one to use & one to play with.
Will get back when finished using it.