rpm reading problem !!!

hello every one
I am trying to write a code to measure the fan rpm by using half effect sensor . After many attempts I couldn't write it by my self , then I go directly to YouTube and find many experiences exactly like what I want . All experience I found measure rpm in a good way , but when I use the code in my Arduino Uno , I get a random measurements Wich it doesn't seem to be an rpm measurements ... some thing like
rpm=4375
rpm=1473
rpm=3754
rpm=5385
rpm=2648 ... etc.

What did I miss ?
I use KY 024 half effect sensor .
The black wire (sensor wire ) is in pin 2

The code is as follows :

int hallsensor=2;
volatile byte count;
unsigned int rpm;
unsigned long passedtime;
void isr(){
count = count + 1;
}
void setup(){

Serial.begin(9600);
attachInterrupt(0,isr,RISING);
pinMode(hallsensor,INPUT);
count=0;
rpm=0;
passedtime=0;
}

void loop(){
// put your main code here, to run repeatedly:
delay(1000);
detachInterrupt(0);
rpm=30*1000/(millis() - passedtime)*count;
passedtime=millis();
count=0;
Serial.print("RPM=");
Serial.print(rpm);
attachInterrupt(0,isr,RISING);

}

any idea of this problem , and why I didn't get a real rpm measurements ?

You are doing lots of things wrong.
You only have a byte for the count make it an int.
Do not detach and reattached interrupts. Attach them once in the setup function and then turn on and off interrupts in the main program when you read the count.
Use floating point variables in the calculation of RPM.

I assume you didn't write this. Just look for better code. That isn't even worth fixing.

Somebody used REPORT TO MODERATOR instead of REPLY :grinning:

Yes
It's me
I am new here
and also newbie in Arduino
so ... I need your help !!

Please read and follow the sticky topics on top of the forum.

Then tell us what you want to do, not how you think that other people have solved your problem.

aarg:
I assume you didn't write this. Just look for better code. That isn't even worth fixing.

aarg:
I assume you didn't write this. Just look for better code. That isn't even worth fixing.

what is the best. code for my case in your opinion .

DrDiettrich:
Please read and follow the sticky topics on top of the forum.

Then tell us what you want to do, not how you think that other people have solved your problem.

I just want to read the rpm of fan by using arduino and hall effect sensor .

Every week there is at least one tachometer (RPM) related post here, with all the comments and fixes. Sometimes there are several. Just use the forum search feature to find them. If you can’t find what you need, because there is some special aspect that is troubling you, come back and we’ll be happy to help. Did you try Google? Maybe, “arduino RPM hall effect” or “arduino fan speed hall effect”.

ok aarg

I'll search in the forum try to find something match my case , other wise I'll back to my post to find what I am looking for .

by the way ...
I didn't wrote the code , I found it in YouTube channel .

alihnet:
ok aarg

I'll search in the forum try to find something match my case , other wise I'll back to my post to find what I am looking for .

by the way ...
I didn't wrote the code , I found it in YouTube channel .

And it is worth every penny you paid for it!

Paul

Do you even understand what I told you in reply #1?

The reply that counts is #2. You have missed a lot, the code is junk, and you need to start over. For all that, you can get results like you show, even though the code is essentially kosher. This is down to the behaviour of the sensor. I believe it is fixed by averaging readings, or simply reading over a longer period.

Grumpy_Mike:
Do you even understand what I told you in reply #1?

dear Grumpy_Mike

I just understand a part of your reply , Wich talk about using byte instead of int And the section Wich talk about using floating point variables in the calculation of RPM.
But I missed detach and reattached interrupts.

because I am newbie in Arduino

I appreciate your Interesting about my project

would you clarify more about the errors in my previous code .

thanks again

To whom it may concern

I found another code to measure rpm as the writer say , but I get same rpm reading in the previous code
the reading of rpm was as follows :
rpm=4370
rpm=1836
rpm=4758
rpm=5384
rpm=-1294
rpm=600
and so on

the code is

float rev=0;
int rpm,count=0;
int oldtime=0;
int time;
 
void isr() //interrupt service routine
{
rev++;
}
 
void setup() {
 
 Serial.begin(9600);
attachInterrupt(0,isr,CHANGE);  //attaching the interrupt
 
}
 
void loop() {
 
delay(1000);
detachInterrupt(0);           //detaches the interrupt
time=millis()-oldtime;        //finds the time 
rpm=(rev/time)*60000;         //calculates rpm
oldtime=millis();             //saves the current time
rev=0;
Serial.print("RPM=");
 Serial.println(rpm); 
attachInterrupt(0,isr,RISING);
 
 

 
}

what is the error i have got

need your help please ...

If you have no codings skills you have to try several examples from other people. If you try two or three other codes you'll find a much better solution.

The only help for your current code: drop it, it's not usable for anything but wasting time.

DrDiettrich:
If you have no codings skills you have to try several examples from other people. If you try two or three other codes you'll find a much better solution.

The only help for your current code: drop it, it's not usable for anything but wasting time.

sorry ... I have little knowledge about writing codes , so I am here to get help for my issue

Connect your hall effect to pin 2 and try this simple sketch:

unsigned long start;
const byte inPin = 2;
volatile unsigned long pulses;//the number of the pulses
unsigned int fin = 1000, rpm;
unsigned long count, lastCount;

void setup() {
  // initialize serial:
  Serial.begin(9600);
  pinMode(inPin,INPUT);
  attachInterrupt(0, readPulse, FALLING);
  
}

void loop() {
  if(millis() - start > fin)
  {
    start = millis();
    noInterrupts();  
    count = pulses;
    interrupts();
    rpm = (count - lastCount) * 60;
    lastCount = count;
    Serial.println(rpm);
  }
}
 
void readPulse()
{
  ++pulses;
}

JCA34F:
Connect your hall effect to pin 2 and try this simple sketch:

unsigned long start;

const byte inPin = 2;
volatile unsigned long pulses;//the number of the pulses
unsigned int fin = 1000, rpm;
unsigned long count, lastCount;

void setup() {
  // initialize serial:
  Serial.begin(9600);
  pinMode(inPin,INPUT);
  attachInterrupt(0, readPulse, FALLING);
 
}

void loop() {
  if(millis() - start > fin)
  {
    start = millis();
    noInterrupts(); 
    count = pulses;
    interrupts();
    rpm = (count - lastCount) * 60;
    lastCount = count;
    Serial.println(rpm);
  }
}

void readPulse()
{
  ++pulses;
}

thank you JCA34F for your interesting ...

I try the sketch as you refer , but the rpm always reed ( rpm = 0 )

do you tried this sketch by your self ?

thanks again

But I missed detach and reattached interrupts.

You attach interrupts to a function like the last line of the setup function in the above code.

Then instead of the detach / attach stuff you did in the loop then inhibit the interrupts from having any effect like this bit of the loop.

 noInterrupts();  
    count = pulses;
    interrupts();

If there is anything in an answer you do not understand from anyone on this forum you should ask specifically about it. Otherwise from this end it looks like you are ignoring the advice.

When given advice to change the code and it doesn’t work like you want say what it does say what you want it to do and most importantly post your attempt at the changes ( all the code ).