#define chA 2
#define chB 3
volatile long counter = 0;
volatile long mod_count= 0;
int PPR = 1000;
int prevTime = 0;
int intrvl = 500;
int timeelapsed;
int rpm;
const int pinButton1 = 7;
const int pinButton2 = 6;
byte prevState = 0;
char currOption;
bool looprunning;
float vel;
float angle = 0;
void isrA(){
if(digitalRead(chB) == LOW){
counter++;
}else{
counter--;
}
}
void isrB(){
if(digitalRead(chA) == HIGH){
counter++;
}else{
counter--;
}
}
void setup(){
Serial.begin(9600);
pinMode(pinButton1, INPUT);
pinMode(pinButton2, INPUT);
attachInterrupt(0,isrA,RISING);
attachInterrupt(1,isrB,RISING);
Serial.print("Checking sensor. Press any button and rotate CW or CCW");
}
void loop(){
byte currState1 = digitalRead(pinButton1);
byte currState2 = digitalRead(pinButton2);
currOption = 0;
if(currState1 != prevState){
currOption = '1';
while(currOption == '1'){
calibrate();
}
}
if(currState2 != prevState){
currOption = '2';
while(currOption == '2'){
countRPM();
}
}
}
void calibrate(){
mod_count = counter % PPR;
angle = mod_count*360/PPR;
Serial.print("angle = ");
Serial.println(angle);
if(angle == -359.0 || angle == 359.0){
Serial.print("Sensor checked!\nPress other button to count RPM");
currOption = 0;
}
}
void countRPM(){
int currTime = millis();
timeelapsed = currTime - prevTime; //finding total time for one rev
rpm =((counter/1000)/timeelapsed)*60000; //calculating the rpm
vel = (2*3.14*rpm)/60;
prevTime = currTime;
Serial.print("RPM = ");
Serial.print(rpm);
Serial.print("\tVelo = ");
Serial.println(vel);
}
Hi everyone!
with the code I am trying to build a project to count RPM from an incremental encoder and later I am planning to use tft lcd screen to display the result. Now, I am using button to start and stop loop. So, far I can script the code as posted. First, I am checking the sensor if it is working (calibrate function). Once it is done, then move to counting the RPM. The issue showed up as I ran the countRPM function, it won't calculate anything inside, only printing. I tested to print the total count and it worked. I am assuming that the problem might be the time-related or millis related calculation. Does anybody have experience or advice?
Appreciate your help
you need a critical section (disable interrupts) to copy the counter value and then work from that copy otherwise. It's possible that the value will be changing whilst you do the math
I am using Arduino UNO at the moment. What I understood from your advice that I could use something like detachInterrupt or other function or make counter as a local variable? I am pretty much beginner here.
the easiest way would be to use the encoder library then you only need to focus on the maths
what I'm saying is
you have an ISR modifying the counter variable that you rightly defined as being volatile. that part is OK (assuming the ISR is doing the right thing, I did not check)
but in the loop you call countRPM() which reads the value of counter without any protection. you are not guaranteed that this operation
rpm =((counter/1000)/timeelapsed)*60000; //calculating the rpm
is going to be atomic and it's possible that he value of counter will be changed whilst you do the maths
on an AVR, the easiest solution is to stop the interrupts just the time to make a copy of the counter
noInterrupts(); // see https://www.arduino.cc/reference/fr/language/functions/interrupts/nointerrupts/
long counterCopy = counter;
interrupts();
and then use the copy to do the maths
rpm =((counterCopy/1000)/timeelapsed)*60000; //calculating the rpm
you might have issues with int on a UNO as it's only 16 bits. you might want to make rpm a long ➜ use
rpm =((counterCopy/1000l)/timeelapsed)*60000l; //calculating the rpm
to make sure the maths are done using a long and also because this is int math, you might want to multiply before you divide as (3/100)*100 is going to be 0 as (3/100) is 0 or go to floating point math
rpm =((counterCopy/1000.0)/timeelapsed)*60000.0; //calculating the rpm
Thank you for your explanation and should I put this section of the code
noInterrupts(); // see https://www.arduino.cc/reference/fr/language/functions/interrupts/nointerrupts/
long counterCopy = counter;
interrupts();
in the loop section or inside the function? I tried to put that on the loop section and it showed some values, eventhough it is not showing the correct rpm and velocity, but at least it showed something. Thank you very much, Sir!
I also used the shared volatile variable without using the noInterrupt but the values are okay. Does it have any particular math/calculation case which can disturb the variable?
You will say that currOption is shared with calibrate(), and that it will be updated sometime that calibrate() is called, thus satisfying the while condition and causing it to exit.
I will say that it is a bad design pattern. The scope of currOption is obfuscated, and you have to go read the source for calibrate() to find out how and why it changes. Thus you could simply return a value to indicate the fact:
Using a strict equality with float usually is a bad idea, because its limited accuracy can lead to false negative check result.
This is true and If I rotate the encoder two fast it might skip those values, but my unexperienced mind can only think that this is the one works. That is why I'm posting the whole code and open to any ideas or suggestions. Thank you for your advice
This is great. It has once crossed my mind to use return but I don't really get the idea. If I adjust the code you gave, the main loop can be just the same, can't it?
Appreciate your help