RPS Encoder issue

I am trying to calculate RPS and display to an LCD but am getting odd results.

RPS is showing up as 6.00 instantly when its not rotating. PulseCounter is displaying 993 or 994. Neither update when spinning the encoder. The “updateEncoder” block functions properly and appears to show 150 Pulses on a 360 degree rotation.

I am using this 300 CPR HKT22 Encoder (datasheet).

The “simulatedEncoder” section is to fake a motor revolution spinning and adding a pulse at a quick pace (1 pulse each 1000 micros).

Any help is greatly appreciated.
Thanks

//Encoder 
int encoderPin1 = 2;
int encoderPin2 = 3;
volatile int lastEncoded = 0;
volatile long encoderValue = 0;
long lastencoderValue = 0;
int lastMSB = 0;
int lastLSB = 0;

//fake Timer

int pulseInterval = 1000; //simulating encoder ++ every 1000 micros 

int pulseCounter = 0; //add up pulses
int prevPulseCounter = 0; // 

int motorSteps = 400; //per rev
float microSteps = 2; //1/2 micro steps = 2x pulses req'd
int pulsesPerRev = motorSteps * microSteps; // likely 800 pulses per rev
int encFullCycle = 150; //# of encoder pulses per full rev
float RPS; //= pulseCounter/pulsesPerRev; figured out later
unsigned long updateMillis = 1000; //Calculate RPS every 1 sec
unsigned long prevMillis = 0;
unsigned long prevMicros = 0;

#include <LiquidCrystal.h>
const int rs = 12, en = 11, d4 = 7, d5 = 6, d6 = 5, d7 = 4;
LiquidCrystal lcd(rs, en, d4, d5, d6, d7);

void setup() {
  Serial.begin (9600);
  lcd.begin(16, 2); 
  
  pinMode(encoderPin1, INPUT_PULLUP); 
  pinMode(encoderPin2, INPUT_PULLUP);
  digitalWrite(encoderPin1, HIGH); //turn pullup resistor on
  digitalWrite(encoderPin2, HIGH); //turn pullup resistor on

  //call updateEncoder() when any high/low changed seen
  //on interrupt 0 (pin 2), or interrupt 1 (pin 3) 
  attachInterrupt(0, updateEncoder, CHANGE); 
  attachInterrupt(1, updateEncoder, CHANGE);
}

void loop(){ 
  simulatedEncoder();
  displayCount();
}

void simulatedEncoder(){
  if (micros() - prevMicros >= pulseInterval){ //simulated encoder spinning
    prevMicros = micros();
    pulseCounter ++; //increment every 1000 micros
  }
}

void displayCount(){
    if (millis() - prevMillis >= updateMillis){ //update display & calc every second
      prevMillis = millis();
      RPS = pulseCounter/encFullCycle;
     
      lcd.setCursor(0, 0);
      lcd.print(RPS);
      lcd.setCursor(0, 1);
      lcd.print(pulseCounter);
//    lcd.print(encoderValue);

      pulseCounter = 0; // reset counter
  }
}
   
void updateEncoder(){
  int MSB = digitalRead(encoderPin1); //MSB = most significant bit
  int LSB = digitalRead(encoderPin2); //LSB = least significant bit

  int encoded = (MSB << 1) |LSB; //converting the 2 pin value to single number
  int sum  = (lastEncoded << 2) | encoded; //adding it to the previous encoded value

  if(sum == 0b1101 || sum == 0b0100 || sum == 0b0010 || sum == 0b1011) encoderValue ++;
  if(sum == 0b1110 || sum == 0b0111 || sum == 0b0001 || sum == 0b1000) encoderValue --;

  lastEncoded = encoded; //store this value for next time
//  pulseCounter ++; // increment when receives single pulse
}

I assume that RPS is Rotations Per Second?

If that is the case, how are you calculating it without an RTC? Not trying to pick, just trying to figure out what your script is doing.

Firstly your timing is out:

change

void simulatedEncoder(){
  if (micros() - prevMicros >= pulseInterval){ //simulated encoder spinning
    prevMicros = micros();
    pulseCounter ++; //increment every 1000 micros
  }
}

to

void simulatedEncoder(){
  if (micros() - prevMicros >= pulseInterval){ //simulated encoder spinning
    prevMicros += pulseInterval;   // don't lose random microseconds
    pulseCounter ++; //increment every 1000 micros
  }
}

The lcd code may also affect the timing, the technique above will automatically catch up if ever delayed more than pulseInterval.

Also change:

void displayCount(){
    if (millis() - prevMillis >= updateMillis){ //update display & calc every second
      prevMillis = millis();

to

void displayCount(){
    if (millis() - prevMillis >= updateMillis){ //update display & calc every second
      prevMillis += updateMillis;

for same reason.

This should get the 993/994’s to be 1000+/-1

Your ISR can be improved:

void updateEncoder(){
  int MSB = digitalRead(encoderPin1); //MSB = most significant bit
  int LSB = digitalRead(encoderPin2); //LSB = least significant bit

  int encoded = (MSB << 1) |LSB; //converting the 2 pin value to single number
  int sum  = (lastEncoded << 2) | encoded; //adding it to the previous encoded value

  if(sum == 0b1101 || sum == 0b0100 || sum == 0b0010 || sum == 0b1011) encoderValue ++;
  if(sum == 0b1110 || sum == 0b0111 || sum == 0b0001 || sum == 0b1000) encoderValue --;

  lastEncoded = encoded; //store this value for next time
//  pulseCounter ++; // increment when receives single pulse
}

to

void updateEncoder(){
  byte encoded = (digitalRead(encoderPin1) << 1) | digitalRead(encoderPin2);
  encoded = encoded ^ (encoded >> 1) ; // convert Gray code to phase (0..3)

  byte diff = encoded - lastEncoded ;  //numerical difference between current and last phase
  diff &= 3 ; // ensure phase in range 0..3, really a number of quadrants to turn.
  lastEncoded = encoded ;

  if (diff == 3)    // check sign of 2 bit 2's complement phase difference value
    encoderValue ++ ;
  else if (diff == 1) 
    encoderValue -- ;
}

Note that reading encoderValue in the main part of the program requires a critical section
as its more than one byte:

  noInterrupts();
  long my_value = encoderValue ;  // dont allow ISR to run inbetween reads of the 4 bytes of the var.
  interrupts() ;

Oh wow, this is great. @Capt-Nemo, I am calculating RPS by polling the # of pulses every 1 sec and dividing the two. I suppose there is a margin of error but not sure how much it is or if it should be considered. I am hoping for accuracy to the 100th second (Ex 9.98 RPS).

@MarkT, Thank you. The timer change make sense and wouldn't have thought of that. The "fake timer" seems to function properly, though my attempt to switch to the encoder breaks as the total pulse count isn't being added up.

I've read the ref material but am not entirely following the "diff&= 3" explanation. Probably because I don't understand the patterns the encoder are sending. I found this article helpful. I gather this section of code is looking for 2 HIGH channels and if so incrementing the encoder to 2. Are there bounce issues? Or is it pretty much this type of a pattern?

When both are HIGH, should I not be adding "pulseCounter ++;"? Would this be here (see below) or at the end of the updateEncoder function IF both are HIGH?

if (diff == 3){    // check sign of 2 bit 2's complement phase difference value
    encoderValue ++ ;
    pulseCounter ++ ;
  }

The note re noInterrupts makes sense, though didn't have luck with where to place it. I believe it would be here:

void displayCount(){
    if (millis() - prevMillis >= updateMillis){ //update display & calc every second
      prevMillis += updateMillis;

      noInterrupts();
      pulseCounter = encoderValue ;  // dont allow ISR to run inbetween reads of the 4 bytes of the var.
      interrupts() ;
      
      RPS = pulseCounter/encFullCycle;

Any tips on where to put the noInterrupts()?

I believe everything is working well after adding “pulseCounter ++” at the end of updateEncoder and “pulseCounter = 0” at the end of the displayCount loop.

Changing encFullCycle to a float and value of 150.0 also allows decimal values for RPS.

Here’s the code for others should they need it:

int encoderPin1 = 2;
int encoderPin2 = 3;
volatile int lastEncoded = 0;
volatile long encoderValue = 0;
//fake Timer int pulseInterval = 1000; //simulating encoder ++ every 1000 micros 
//fake timer unsigned long prevMicros = 0;

int pulseCounter = 0; //add up pulses
float encFullCycle = 150.0; //# of encoder pulses per full rev
float RPS; //= pulseCounter/pulsesPerRev; figured out later

int motorSteps = 400; //per rev
float microSteps = 4; //1/2 micro steps = 2x pulses req'd
int pulsesPerRev = motorSteps * microSteps; // likely 800 pulses per rev

unsigned long updateMillis = 1000; //Calculate RPS every 1 sec
unsigned long prevMillis = 0;

#include <LiquidCrystal.h>
const int rs = 12, en = 11, d4 = 7, d5 = 6, d6 = 5, d7 = 4;
LiquidCrystal lcd(rs, en, d4, d5, d6, d7);

void setup() {
  Serial.begin (9600);
  lcd.begin(16, 2); 
  pinMode(encoderPin1, INPUT_PULLUP); 
  pinMode(encoderPin2, INPUT_PULLUP);
  digitalWrite(encoderPin1, HIGH); //turn pullup resistor on
  digitalWrite(encoderPin2, HIGH); //turn pullup resistor on
  attachInterrupt(0, updateEncoder, CHANGE); 
  attachInterrupt(1, updateEncoder, CHANGE);
}

void loop(){ 
//  simulatedEncoder();
  displayCount();
}

void updateEncoder(){
  byte encoded = (digitalRead(encoderPin1) << 1) | digitalRead(encoderPin2);
  encoded = encoded ^ (encoded >> 1) ; // convert Gray code to phase (0..3)

  byte diff = encoded - lastEncoded ;  //numerical difference between current and last phase
  diff &= 3 ; // ensure phase in range 0..3, really a number of quadrants to turn.
  lastEncoded = encoded ;

  if (diff == 3){    // check sign of 2 bit 2's complement phase difference value
    encoderValue ++ ;
  }
  else if (diff == 1) {
    encoderValue -- ;
  }

  pulseCounter ++; //***NEW  
}

void displayCount(){
    if (millis() - prevMillis >= updateMillis){ //update display & calc every second
      prevMillis += updateMillis;
      RPS = pulseCounter/encFullCycle;

      lcd.setCursor(0, 0);
      lcd.print("RPS: "); lcd.print(RPS, 3);
      lcd.setCursor(0, 1);
      lcd.print("Pulses: "); lcd.print(pulseCounter);
      pulseCounter = 0; // reset counter ***NEW
  }
}
   
//void simulatedEncoder(){
//  if (micros() - prevMicros >= pulseInterval){ //simulated encoder spinning
//    prevMicros += pulseInterval;   // don't lose random microseconds
//    pulseCounter ++; //increment every 1000 micros
//  }
//}