Shaft RPM tachometer output fluctuates

[ztugdriver]

Hello
First time posting and extremely new to Arduino. Ive created the code below using some available code from resources online and modifying it to suit my application.

I am trying to make a display that reads the shaft RPM of a welding positioned. The shaft rotates between 0-5 rpm. I have the display working and the RPM showing correctly but it fluctuates a little +/- 1-2 rpm. I need it to be pretty accurate. Im using an Uno board and a 16x2 I2C display. My input is coming from a optocoupler that has a n encoder disc with 8 gaps (I also have a 20 gap disc). As I said its working pretty good I would just like to see a more steady read out with less fluctuation. I don't believe the motor is fluctuating in its RPM but I have no way to verify this. Any input would be greatly appreciated.

#include <Wire.h>
#include <LiquidCrystal_I2C.h>

#define ENCODER_PIN 3
#define ENCODER_N 8.0

LiquidCrystal_I2C MyLCD(0x27, 16, 2);  // Creates I2C LCD Object With (Address=0x27, Cols=16, Rows=2)

unsigned long T1 = 0, T2 = 0, T;
bool MeasDone = 0;
float Motor_RPM = 0.00;

void INT0_ISR(void) {
  if (MeasDone) {
    T2 = micros();
    T = T2 - T1;
    MeasDone = 0;
  } else {
    T1 = micros();
    MeasDone = 1;
  }
}

void setup() {
  MyLCD.init();
  MyLCD.backlight();
  pinMode(ENCODER_PIN, INPUT);
  attachInterrupt(digitalPinToInterrupt(ENCODER_PIN), INT0_ISR, RISING);
}
void loop() {
  Motor_RPM = (60000000) / (T * ENCODER_N);
  MyLCD.clear();
  MyLCD.setCursor(0, 0);
  MyLCD.print("Table RPM:");
  Motor_RPM = (float)Motor_RPM / 1;  // Convert RPM to float with 2 decimal places
  MyLCD.setCursor(0, 1);
  MyLCD.print(Motor_RPM, 2);
  delay(1000);
}

`

[xfpd]

Use the ISR to indicate an RPM event occurred, only. When outside the ISR, check the RPM event flag and process it.

[edit] Variables used in ISR should be typed as volatile... and interrupts should be disabled at the entry of the ISR, and enabled when exiting of the ISR.

volatile - Arduino Reference

noInterrupts() - Arduino Reference

[Paul_KD7HB]

@ztugdriver

unsigned long T1 = 0, T2 = 0, T;

For one thing, these all need to be "volatile" as they a re all multiple bytes long and will likely be in the process of being changed by your interrupt code while loop() is processing them.

I addition, you need to turn off interrupts while in the loop() and move all the variables associated with the interrupt code to new global variables and turn interrupts back on.
Then continue the loop() code using the values in the new global variables. Otherwise the interrupt code and your loop() code will be interfering with the other guy;s variables and chaos will happen.

Look at other sample code in the IDE to see how they use interrupts.

[ztugdriver]

Thank you for the info. I really have no idea to apply what you have suggested but Im giving it a try. It took me 2 weeks to figure out what I have so far. LOL

[JCA34F]

Its going to be difficult to get a steady reading with only 40 pulses per minute (one every 1.5 seconds). Have you tried the 20 slot wheel? You could try a "rolling average" algorithm but that would slow down the response time to changes in RPM.

[ztugdriver]

I have a 20 and a 100. I will try both and see if I can get better results. Ive tried to apply what others have suggested above but its a bit out of my understanding at the moment. IM trying to figure this coding out but man its like drinking water from a fire hose.

[johnerrington]

@ztugdriver

The shaft rotates between 0-5 rpm. I have the display working and the RPM showing correctly but it fluctuates a little +/- 1-2 rpm. I need it to be pretty accurate.

I'd not consider a random error of +/-2 in a range of 5 to be pretty accurate.

If the rpm is 5 then you have 5* 8 changes per minute or 1.5 seconds between rising edges.
So after the ISR executes you have a value of about 1,500,000 in T
and you calculate

Motor_RPM = (60,000,000) /1,500,000 * 8;

giving 5.000 for rpm.

It would be useful at that point to serial print the value you have calculated.

@ztugdriver

man its like drinking water from a fire hose.

The only value common to the interrupt routine and the loop{} is T. lets make all the variables in the ISR volatile. (explanation here Explanation of Volatile?)

volatile unsigned long T1 = 0, T2 = 0, T;
volatile bool MeasDone = 0;

In your loop you should do the update process only when the ISR has triggered.
So use the MeasDone flag

if(MeasDone){

Motor_RPM = (60000000) / (T * ENCODER_N);
  MyLCD.clear();
  MyLCD.setCursor(0, 0);
  MyLCD.print("Table RPM:");
  Motor_RPM = (float)Motor_RPM / 1;  // Convert RPM to float with 2 decimal places
  MyLCD.setCursor(0, 1);
  MyLCD.print(Motor_RPM, 2);

and lose the delay.

[PaulRB]

@xfpd

Variables used in ISR should be typed as volatile...

@Paul_KD7HB

these all need to be "volatile"

No. Only variables, like T, that are updated inside an ISR, and read or updated outside an ISR, need to be declared volatile. Variables used only inside the ISR, like T1, T2, don't need to be volatile. In fact, they could be local to the ISR, but declared static so their values are not lost between calls.

[PaulRB]

@ztugdriver

void INT0_ISR(void) {
  if (MeasDone) {
    T2 = micros();
    T = T2 - T1;
    MeasDone = 0;
  } else {
    T1 = micros();
    MeasDone = 1;
  }
}

I think this code is only timing every other gap, effectively reducing the encoder to 4 gaps. It could be re-written to measure every gap, allowing two gaps to be measured and the average taken, while still maintaining the same rate of readings.

@ztugdriver

I don't believe the motor is fluctuating in its RPM but I have no way to verify this.

Would you not notice a difference the the quality of the welding joints along the length of the weld, if that were true?

I can only guess what is being welded here. Metal pipes are sometimes made from long metal sheets, welded in a spiral manner. Something like that?

[DaveX]

How fast is the shaft supposed to vary? Is it set by some other controller to a constant speed and then supposed to operate at that speed for seconds? minutes? hours?

With a quality wheel, an 8 slot encoder at a steady 5RPM should give you a rising edge every 1.5 seconds. Even if you just measured it in a plain loop with millis() you should be able to get 0.1% precision out of the system, much higher than the 0.5% accuracy of the Arduino clock.

You could measure the intervals between the rising edges and also between sucessive falling edges to get alternating esitmaites of time at 0.75 sec updates.

[ztugdriver]

So I have applied what I could to my code and it seems to be working better. It still fluctuates a bit about 1 rpm. Im not sure if my bench setup is holding the encoder disc in a concentric axis enough for the optocoupler to get a consistent reading. Ive added serial.print to the code and have monitored MeasDone in the serial plotter and I can see the pulse rate vary slightly. I don't know if this is due to the way it calculating or from the inconsistency of my setup. It is pretty steady though and I think close enough for my application. To clarify what Im doing I am making a welding positioner that rotates from 0-5 RPM so that I can make very consistent welds on round objects. I need to change the RPM depending on the diameter of the piece being welded. The object will be rotated by a AC induction geared motor which is controlled by its own motor speed controller. I haven't made the positioner yet Im just trying to learn Arduino and apply this setup to measure the table rotation once I get it fabricated. Im more of a hands on guy than a programmer. Needless to say Im much better at fabrication than writing code. I know I have a lot to learn but this project has opened my eyes to how difficult this is and how much time is invested in learning coding. Now I know why my son hated his coding classes in high school.
The new code is below and if any of you have suggestions Im all ears. I don't know how to make T volatile. Thank you all for your input I really appreciate it.`

#include <Wire.h>
#include <LiquidCrystal_I2C.h>
 
#define ENCODER_PIN 2
#define ENCODER_N   20.0
 
LiquidCrystal_I2C MyLCD(0x27, 16, 2); // Creates I2C LCD Object With (Address=0x27, Cols=16, Rows=2)
 
volatile unsigned long T1 = 0, T2 = 0, T;
volatile bool MeasDone = 0;
float Motor_RPM = 0.00;
 
void INT0_ISR(void) {
  if (MeasDone) {
    T2 = micros();
    T = T2 - T1;
    MeasDone = 0;
  } else {
    T1 = micros();
    MeasDone = 1;
  }
}
void setup()
{
  Serial.begin(9600);
  MyLCD.init();
  MyLCD.backlight();
  pinMode(ENCODER_PIN, INPUT);
  attachInterrupt(digitalPinToInterrupt(ENCODER_PIN), INT0_ISR, RISING);
}
void loop()
{if(MeasDone)
  Motor_RPM = (60000000) / (T * ENCODER_N);
  MyLCD.setCursor(0, 0);
  MyLCD.print("Motor RPM:");
  Motor_RPM = (float)Motor_RPM / 1; // Convert RPM to float with 2 decimal places
  MyLCD.setCursor(0, 1);
  MyLCD.print(Motor_RPM, 2);
  Serial.print(MeasDone);
  Serial.println();

  
}

`

[JCA34F]

@ztugdriver

rotated by a AC induction geared motor

What is the AC motor's RPM when the table is turning at 5 RPM?

[ztugdriver]

The AC induction gear motor is spinning at 1850 rpm but it’s output shaft it 27 rpm at full speed. The output shaft will have a 25 tooth spur gear on it which will drive a 150 tooth spur gear that is attached to the table that the object I’m welding we rest on. That table will rotate between 0-5 rpm. Through the gear reduction im able to ge the 27 rpm down to 0-5 rpm. LMK if you have any other questions.

I’m intending on measuring the table rpm by connecting the encoder disc to the end of the shaft that the table and 150 tooth gear is attached to. Here is a pic of the ac motor.

[JCA34F]

Is it possible to mount your encoder wheel on the 27 RPM shaft?

[GoForSmoke]

If you put a a bar magnet across the end of the shaft center and position a linear Hall sensor (analog device, not the Hall switch) right over that center, aligned crossways like the magnet, it reads magnetic field strength along that axis. As the shaft rotates the reads cycle sinusoidally, this is how high end joystick axes are read, pots get dirty, magnets don't. You can detect motion < 1 degree and begin to respond 8x per millisecond with an Uno by using non-blocking code.

[DaveX]

Your code is very dependent upon the ".0" in this line:

@ztugdriver

#define ENCODER_N   20.0

If it isn't there, (for example "8" or "100" then this line will do integer math and give you whole numbers like for RPM:

@ztugdriver

  Motor_RPM = (60000000) / (T * ENCODER_N);

You can force the RHS to always calculate in real numbers no matter what format you use to identify the slots on the encoder by making the 60000000 a real number with 600000000.0 or 60e6.

[GoForSmoke]

If you measure micros the roundoff will be less significant.

We use millivolts to step around fractional volts for example, if our working unit becomes microvolts we can lose 3 places and still have mV.

IEEE floating point has a dirt in sand problem.

[ztugdriver]

Relocating the encoder disc is definitely possible do you think it would be a better approach than the slower rotating table shaft?

[DaveX]

Measure which in micros? The OP's code measures time in micros:

@ztugdriver

void INT0_ISR(void) {
  if (MeasDone) {
    T2 = micros();
    T = T2 - T1;
    MeasDone = 0;
  } else {
    T1 = micros();
    MeasDone = 1;
  }
}

So maybe do the RPM calc in milliRPMs with something like this?

 Motor_mRPM = (60000000) / ((T/1000) * ENCODER_N);

or measure time in millis()?

Arduino doesn't seem to make it easy to render milliUnits with a decimal point other than with floating point.

[JCA34F]

It would give 6 times the pulse rate, 120 pulses per revolution instead of 20. Might result in less jitter.