Interrupts

[bogdan_adrian91]

Hello guys , i have made a schetch to read rpm using a hall effect sensor and t works fine on its own .

I have another schetch that uses 4 buttons to set an interval ( Min, Max ) and move a servo in the interval . This also works fine on its own.
But when i tried to merge them i found out that the schetch was faulty and i cannot find the problem.My servo should move +/- 1 position untill the position is in the interval.

I am thinking that the interrupt is messing it up ?! is there a simpler way to measure RPM ?
I will use the schetch to measure RPM lower than 150 .
here is the code for rpm measurement:

#include <MicroView.h>
int sensorPin = 2; //hall effect
int counter = 0;
int ledpin = 13;
float start, finished;
float elapsed, time;
float revs;
float revolution;

void setup()
{
  uView.begin();			// start MicroView
  uView.clear(PAGE);

  // setup serial - diagnostics -
  Serial.begin(115200);
  // setup pins
  pinMode(sensorPin, INPUT);
  pinMode(ledpin, OUTPUT);
  // setup interript
  attachInterrupt(0, RPM, RISING);
  start = millis();

}

void RPM()
{
  int sensorValue = digitalRead(sensorPin);
  //lcd.clear();
  elapsed = millis() - start;
  start = millis();
  float revs = 60000.0 / elapsed ;
  float revolution = elapsed / 1000;
  int revo = round(revs); ;

  uView.setCursor(0, 0);	// set cursor to 0,0
  uView.print("RPM:");	// display Mid
  uView.display();

  uView.setCursor(22, 0);	// set cursor to 0,0
  uView.print(revo);	// display Mid
  uView.display();


  if (elapsed < 1200) {}
  else {}


  if (sensorValue == 0) {
    digitalWrite (ledpin, HIGH);
  }
  else {
    digitalWrite(ledpin, LOW);
  }
  //lcd.setCursor(3, 1);
  if (elapsed > 1200) {
    revs == 0;
  }
}

void loop()
{
  elapsed = millis() - start;

  int sensorValue = digitalRead(sensorPin);


  if (sensorValue == 0) {
    digitalWrite (ledpin, HIGH);
  }
  else {
    digitalWrite(ledpin, LOW);
  }
  //lcd.setCursor(3, 1);
  if (elapsed > 1200) {
    revs == 0;
  }


}

[septillion]

bogdan_adrian91:
My measured rpm will not go over 150 rpm .

That's because you ignore the first rule of interrups, they should be short and fast. And here you go and print all sort of things and even do float(!!!) calculations in the interrupt! But it should be more like

volatile unsigned long hallMillis;
volatile bool hallFlag;

void hallInterrupt(){
  hallMillis = millis();
  hallFlag = true;
}

And handle the rest in the loop.

And keep in mind, with millis() only has a resolution of 1ms so with higher RPM will give you a reasonable error. So maybe you want to use timers to get the timing. Maybe just the capture mode of the timer.

[bogdan_adrian91]

septillion:
That's because you ignore the first rule of interrups, they should be short and fast. And here you go and print all sort of things and even do float(!!!) calculations in the interrupt! But it should be more like

I meant i will be using it to measure RPM < 150 .

[system]

 if (elapsed > 1200) {
    revs == 0;
  }

Oops.

[Paul_B]

Using interrupts for events of this nature - 2½ per second, or even 100 times that - it is quite absurd to use interrupts in the first place. You should only ever be polling.

Second rule of interrupts: Interrupts should not be used unless you really understand what they are in all respects. If you do not understand them, just don't use them and life will be much happier. Mind you, this clearly is subject to the Dunning-Kruger effect.

[Robin2]

Nice link, Paul.

...R

[bogdan_adrian91]

Sarcastic comment aside
I should not use interrupts ? Okay thean.
I will count 10 000 milis in the setup and than in a function count how many pulses i have in a period, example: how many consecutive low,high pulses there are in 10 000 milis.
And then just multiply this value by 6 and i will have my rpm ?
is this okay given that i will be measuring a maximum of 150 rpm ?

[system]

Have you fixed the problems I pointed out?
I think you do it twice.

[Robin2]

bogdan_adrian91:
I should not use interrupts ?

It seems reasonable to use interrupts for RPM counting - but the ISR code should be very simple - perhaps just

void RPMcounter() {
    pulseCount ++;
}

In other words it just adds one to a counter for every interrupt.

The rest of the "intelligence" should be outside the ISR. For example

void checkPulseRate() {
   if (millis() - lastReadMillis > 1000) {
      lastReadMillis += 1000;
      prevPulseCount = latestPulseCount; // save the last value
      noInterrupts();  // in case the number changes while we are copying it
         latestPulseCount = pulseCount;
      interrupts(); 
      pulsesThisSecond = latestPulseCount - prevPulseCount;
   }
}

Define all the timing and counting variables as unsigned long

One reason to use interrupts for pulse counting is that the duration of the pulse may be too short to capture reliably using polling even though the number of pulses per second is low.

...R

[CrossRoads]

I think data type volatile needs to be declared at the top of the sketch so data in the ISR can be accessed outside of the ISR, as in:
unsigned int volatile pulsecount; // for counts up to 65535 (2^16 -1), or unsigned long (2^32 -1)
syntax might need a little tweaking, been a while since I had an interrupt that used variables in my code.

[Robin2]

CrossRoads:
syntax might need a little tweaking, been a while since I had an interrupt that used variables in my code.

Have you spotted something wrong with my code in Reply #8?
If so please let me know so I can correct it.

I agree that the variable pulseCount should be defined at the top of the program as

volatile unsigned long pulseCount;

The word "volatile" is used to let the compiler know that the variable will change even though the compiler might think it won't.

...R

[CrossRoads]

No, I was referring to the syntax I used in the volatile declaration. I don't have a way to confirm it where I am.

[bogdan_adrian91]

Hello, i have done the following and it works fine tested with a geared motor and a magnet attached to the tire up to a certain point .

problem 1. After 1 minute or so i get a '1' in front of my rpm but it's only there for roughly 1 rotation.
problem 3. Instead of reading 72 it sometimes adds another nomber at the end example. 768 instead of just 76 .

Thank you very much.

#include <MicroView.h>


volatile unsigned long rpmcount ;
int rpm;

unsigned long timeold;
//+++++++++++++++++++++++
int hallPin = 2;         //hall sensor on digital pin 2

void setup()
{  
   Serial.begin(9600);
  uView.begin();			// start MicroView 
  uView.clear(PAGE);
  pinMode(hallPin, INPUT);

 
  attachInterrupt(0, rpm_fun, RISING);
  rpmcount = 0;
  rpm = 0;
  timeold = 0;
}

void loop()
{
        rpmCal();
        
        uView.setCursor(20,20);	// set cursor to 0,0
	uView.print(rpm);	// display Mid
	uView.display();         
}

void rpmCal(){
   if (rpmcount >= 2) {
    //Update RPM every 2 counts, increase this for better RPM resolution,
    //decrease for faster update
    rpm = 30*1000/(millis()-timeold)*rpmcount;
    timeold = millis();
    rpmcount = 0;
  }
}
void rpm_fun()
{
  rpmcount++;
 
}

[Robin2]

You have not followed my example in Reply #8.

Look at how I used noInterrupts() and a separate variable to get a copy of the value being produced by the ISR.

There should be no need to set the variable rpmcount to 0. Just get the difference as I have illustrated.

...R

[bogdan_adrian91]

@Robin2 , i have also done what you have kindly explained but i cannot get it to calculate rotations per minute

#include <MicroView.h>


volatile unsigned long pulseCount ;
unsigned long lastReadMillis ;
unsigned long prevPulseCount ;
unsigned long latestPulseCount ;
unsigned long pulsesThisSecond;
volatile unsigned long rpm ;
int hallPin = 2;         //hall sensor on digital pin 2

void setup()
{
  Serial.begin(9600);
  uView.begin(); // start MicroView
  uView.clear(PAGE);
  pinMode(hallPin, INPUT);


  attachInterrupt(0, RPMcounter, RISING);
  pulseCount = 0;
  lastReadMillis = 0;
  prevPulseCount = 0;
  latestPulseCount = 0;

}
void loop() {
  checkPulseRate() ;
  uView.setCursor(10, 0); // set cursor to 0,0
  uView.print(pulseCount); // display Mid
  uView.display();
  uView.setCursor(0, 10); // set cursor to 0,0
  uView.print("Rpm:"); // display Mid
  uView.print(rpm);
  uView.display();
  uView.setCursor(10, 25); // set cursor to 0,0
  uView.print("latestPulseCount:"); // display Mid
  uView.print(latestPulseCount);
  uView.display();


}

void checkPulseRate() {
  if (millis() - lastReadMillis > 1000) {
    lastReadMillis += 1000;
    prevPulseCount = latestPulseCount; // save the last value
    noInterrupts();  // in case the number changes while we are copying it
    latestPulseCount = pulseCount;
    interrupts();
    pulsesThisSecond = latestPulseCount - prevPulseCount;
    // rpm = 30*1000/(millis() - lastReadMillis)*latestPulseCount;
  }
  if (latestPulseCount >= 2) {
    rpm = (60000 * pulseCount) / (millis() - lastReadMillis);
    lastReadMillis = millis();


  }
}


void RPMcounter() {
  pulseCount ++;
}

[anon57585045]

Paul__B:
Using interrupts for events of this nature - 2½ per second, or even 100 times that - it is quite absurd to use interrupts in the first place. You should only ever be polling.

I said this once. Then I was reminded that the pulse width of a rotation sensor can be incredibly short at higher RPMs.

[Robin2]

bogdan_adrian91:
@Robin2 , i have also done what you have kindly explained but i cannot get it to calculate rotations per minute

I'm not surprised. This piece of code makes no sense in the context of the stuff that precedes it - how could you think it does

if (latestPulseCount >= 2) {
    rpm = (60000 * pulseCount) / (millis() - lastReadMillis);
    lastReadMillis = millis();
  }

I don't think you have said how many pulses there are per revolution, but supposing there is 10 and supposing the shaft is rotating at 120 rpm or 2 rps then the variable pulsesThisSecond will hold the value 20. To convert that to RPM you need this calculation

pulsesThisSecond * 60 / pulses per revolution

...R

[bogdan_adrian91]

I knew that the code was not correct ,it was left there from my last attempt,which was made out of frustration.

How can i know how many pulses per revolution i will have ? The shaft will be rotating at anywhere between 1 up to 150 rotations per minute.
I guess it will have between 0 and 3(max) pulses per second .

This is what the code does now .

[Robin2]

bogdan_adrian91:
How can i know how many pulses per revolution i will have ? The shaft will be rotating at anywhere between 1 up to 150 rotations per minute.
I guess it will have between 0 and 3(max) pulses per second .

The critical thing is how many pulses there are per revolution - but you have not said.

If you have between 0 and 3 pulses per second I don't think you will get useful RPM info just by counting the pulses. You will need to measure the time between pulses with code something like this

void updateRPM() {
    if (newPulse == true) {
        noInterrupts()
            newPulse = false;
            latestPulseMicros = pulseMicros;
        interrupts();
        revolutionMicros = latestPulseMicros - prevPulseMicros;
        prevPulseMicros = latestPulseMicros;
        // then calculate RPM from the time taken for one revolution
    }
}


void pulseTimer() {
    pulseMicros = micros();
    newPulse = true;
}

This would replace the functions checkPulseRate() and RPMcounter()

If your principal purpose is to control the speed I think it would make sense to do so based on the value in revolutionMicros without bothering to convert it to RPM.

...R

[bogdan_adrian91]

I just said that 0-3 pulses by presuming my motor will run at a top speed of 180 rpm. So i just did 180/60 and ended up with 3 rps . This might be irrelevant.

My project envolves the following :

1. Read Rpm from a shaft that has a magnet attached to it .
2. Set a rotation interval using 4 buttons ( Rmin, Rmax )
3. Use a servo to control the rotations,make sure the Rpm is in the given interval Rmin,Rmax.
   The servo has a potentiometer mechanically sealed to it that controls the motor shaft speed .
   Basically i am trying to make an automatic system .