PID with encoder issues

I’m trying to write some code to operate a relay on a current value and set value (a PID). I have the PID working correctly, but I want to use a rotary encoder to control the set value.

The problem I have is that the ‘PID’ process in the loop is quite slow (I think this is mainly caused by reading temperature from a DS18B20)

As the code is slow, the LCD screen has a delay when changing the rotary encoder position.

here is my code

  • I’ve removed the code for the PID as its quite long, and replaced it with ‘delay(3000);’ to simulate the delay it causes

//LCD
#include <Wire.h> // Comes with Arduino IDE
#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE); // Set the LCD I2C address

// push button switch
int inPin = 4; // the number of the input pin
int state = HIGH; // the current state of the output pin
int reading; // the current reading from the input pin
int previous = LOW; // the previous reading from the input pin
long time = 0; // the last time the output pin was toggled
long debounce = 200; // the debounce time, increase if the output flickers

// rotary switch
const int PinCLK = 2; // Used for generating interrupts using CLK signal
const int PinDT = 3; // Used for reading DT signal
volatile boolean encChanged;
volatile long encPosition = 0;
volatile boolean up;

void isr() { // Interrupt service routine is executed when a HIGH to LOW transition is detected on CLK
volatile boolean CLK = digitalRead(PinCLK);
volatile boolean DT = digitalRead(PinDT);
up = ((!CLK && DT) || (CLK && !DT));
if (!up)
encPosition++;
else
encPosition–;

if (encPosition < 0)
{
encPosition = 0;
}

else if (encPosition > 100)
{
encPosition = 100;
}

encChanged = true;
delay(10);
}

void setup() {

// push button switch
pinMode(inPin, INPUT_PULLUP);
// rotary switch
pinMode(PinCLK, INPUT);
pinMode(PinDT, INPUT);
attachInterrupt(0, isr, FALLING); // interrupt 0 is always connected to pin 2 on Arduino UNO
Serial.begin(9600);
Serial.println(“Start”);
// LCD
lcd.begin(20,4);
lcd.setCursor(0,0); //Start at character 4 on line 0
lcd.print(" Sous Vide");

}

void loop() {
//push button
reading = digitalRead(inPin);
if (reading == HIGH && previous == LOW && millis() - time > debounce) {
buttonPress();
}

previous = reading;
//rotary
if (encChanged) { // do this only if rotation was detected
encChanged = false; // do NOT repeat IF loop until new rotation detected
Serial.print("Count = ");
Serial.println(encPosition);
}

delay(3000); // to simulate PID process

lcd.setCursor(0,0);
lcd.print("Encoder: ");
lcd.print(encPosition);
lcd.setCursor(0,1);
lcd.print("Button State: ");
lcd.print(state);

}

void buttonPress()
{
if (state == HIGH)
{
state = LOW;
Serial.println(state);
}
else

{
state = HIGH;
Serial.println(state);
}

time = millis();
}

Never use delay in an interrupt. Also your interrupt is doing too much work. Dwight

PID taking 3 seconds seems extreme I have a balancing bot that triggers PID code every 10 miliseconds with time to spare the 3 second delay is causing all kinds of trouble with your button reading you will need to hold the button for up to 3 seconds for it to read. I have more advanced version that can trigger on both rise and fall of both data and clock pins for 4X the resolution but I an guessing you are using the encoder for adjustments and don’t want or need the extra accuracy. I will post the code if you would like to see it :slight_smile:

Also how long does it take to read your DS18B20 Temperature sensor? I would guess < 10 microseconds?
I would like to help you with the PID 3 Second thing.

I simplified your encoder code with notes and added some no delay() delays to help with spamming the LCD Display and serial ports causing the UNO to possibally crash.

/ LCD
#include <Wire.h>  // Comes with Arduino IDE
#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);  // Set the LCD I2C address

// push button switch
int inPin = 4;         // the number of the input pin
int state = HIGH;      // the current state of the output pin
int reading;           // the current reading from the input pin
int previous = LOW;    // the previous reading from the input pin
long time = 0;         // the last time the output pin was toggled
long debounce = 200;   // the debounce time, increase if the output flickers

// rotary switch
const int PinCLK = 2;                   // Used for generating interrupts using CLK signal
const int PinDT = 3;                    // Used for reading DT signal
volatile boolean encChanged;
volatile long encPosition = 0;
volatile boolean up;

void isr()  {                    // Interrupt service routine is executed when a HIGH to LOW transition is detected on CLK
  //volatile boolean CLK = digitalRead(PinCLK);
  // You know that the Clock cycle is low because of  attachInterrupt(0, isr, FALLING);   // interrupt 0 is always connected to pin 2 on Arduino UNO
  // The Clock pin could bounce if it is a mechanical switch so use a small capacitor to dampen the possibility of bounce in the circuit.
  // The data pin hasn't changed for some time so if it is low the we are moving in one direction if it is high we are moving in the other

  if (digitalRead(PinDT))
    encPosition++;
  else
    encPosition--;

  encPosition = min(100, max(0, encPosition));
  encChanged = true;
}

void setup()  {

  // push button switch
  pinMode(inPin, INPUT_PULLUP);
  // rotary switch
  pinMode(PinCLK, INPUT); // if you are not providing power you may need to use INPUT_PULLUP
  pinMode(PinDT, INPUT); // if you are not providing power you may need to use INPUT_PULLUP
  attachInterrupt(0, isr, FALLING);   // interrupt 0 is always connected to pin 2 on Arduino UNO
  Serial.begin(9600);
  Serial.println("Start");
  // LCD
  lcd.begin(20, 4);
  lcd.setCursor(0, 0); //Start at character 4 on line 0
  lcd.print("     Sous Vide");

}

void loop()  {
   int t = 100; //10 time a second... I struggled with sending fast to test too often to the LCD display and Serial ports so I suggest a no delay spam timer :)
 
  //push button
  reading = digitalRead(inPin);
  if (reading == HIGH && previous == LOW && millis() - time > debounce) {
    buttonPress();
  }

  previous = reading;
  //rotary
  if (encChanged)  {        // do this only if rotation was detected
    encChanged = false;          // do NOT repeat IF loop until new rotation detected
    static unsigned long SpamTimerSerial;
    if ( (unsigned long)(millis() - SpamTimerSerial) >= (t)) {
      SpamTimerSerial = millis();
      Serial.print("Count = ");
      Serial.println(encPosition);
    }
  }

  //delay(3000);                  // to simulate PID process
  // the delay will mess up you push button you will have to hold the button for up to 3 seconds before it is recognized
  
  static unsigned long SpamTimerLCD;
  if ( (unsigned long)(millis() - SpamTimerLCD) >= (t)) {
    SpamTimerLCD = millis();
    lcd.setCursor(0, 0);
    lcd.print("Encoder:       ");
    lcd.print(encPosition);
    lcd.setCursor(0, 1);
    lcd.print("Button State:  ");
    lcd.print(state);
  }
}


void buttonPress()
{
  if (state == HIGH)
  {
    state = LOW;
    Serial.println(state);
  }
  else

  {
    state = HIGH;
    Serial.println(state);
  }

  time = millis();
}

Thank you for the help! Especially zhomeslice, that code looks much better.

I'll give it a go and report back..

Unfortunately the new rotary code doesn't seem to work too well.

I'm running it exactly as above. the encoder does work partially, i.e the value goes up if you turn one way and down if you turn the other. But its very unstable, even turning slowly it sometimes jumps 10 at a time, and occasionally the wrong way.

Do you have the rotary encoder filters as shown on page 2 of this encoder, or something similar?

PEC12R Rotary Encoder.pdf (367 KB)

CrossRoads: Do you have the rotary encoder filters as shown on page 2 of this encoder, or something similar?

CrossRoads is correct it sounds like you're missing the capacitor. It is critical that you add small capacitors on the data and clock terminals. a capacitor should be connected to ground as shown in the diagram. When the clock pin drops it bounces causing multiple readings to occur. Adding code to debounce is much more difficult than just adding a simple capacitor.

CrossRoads: Do you have the rotary encoder filters as shown on page 2 of this encoder, or something similar?

thanks CrossRoads, you were correct!

I only had some 100nF caps, not 10nF as in your link, but nevertheless it all seems to be working great!