Resetting Rotary Encoder value after it reaches a max value and a certain amount of time passes

Using Arduino Programming Questions
1

Hello,

I'm working on a project for educational purposes, where I can control the speed of a DC fan (Wathai 12V brushless DC fan) using PWM and a Rotary Encoder (Taiss/AB 2 phase Incremental Rotary Encoder). I'm using a TM1637 4 digit LED display to show the value of the rotary encoder while its changing, I've limited the encoder to go only from 0 to 255, and I've mapped it on the TM1637 so it shows values from 0 to 1200. The codes works and I'm able to control the fan with the encoder. When the encoder reaches 255 the fan speed is at its max, which is what I wanted, but I would like to reset the encoder value back to zero ( which would take the fan to its lowest speed) if the encoder has been on 255 after 5 minutes, but I'm not sure where to add the instructions or how to do so.

Help would be greatly appreciated!

#include <Arduino.h>
#include <TM1637Display.h>
#define CLK 8
#define DIO 9
TM1637Display display(CLK, DIO);
// Define rotary encoder pins
#define ENC_A 2
#define ENC_B 3


volatile int counter = 0;

void setup() {
  display.setBrightness(7);
  // Set encoder pins and attach interrupts
  pinMode(ENC_A, INPUT_PULLUP);
  pinMode(ENC_B, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(ENC_A), read_encoder, CHANGE);
  attachInterrupt(digitalPinToInterrupt(ENC_B), read_encoder, CHANGE);

  // Start the serial monitor to show output
  Serial.begin(115200);
}

void loop() {
  static int lastCounter = 0;

  // If count has changed print the new value to serial
  if (counter != lastCounter) {
    //Serial.println(counter);
    lastCounter = counter;
  }
  int V = map(counter, 0, 255, 0, 1200);
  display.showNumberDecEx(V, 0b01000000, false);
  analogWrite(11, counter);
}

void read_encoder() {
  // Encoder interrupt routine for both pins. Updates counter
  // if they are valid and have rotated a full indent

  static uint8_t old_AB = 3;                                                                  // Lookup table index
  static int8_t encval = 0;                                                                   // Encoder value
  static const int8_t enc_states[] = { 0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0 };  // Lookup table

  old_AB <<= 2;  // Remember previous state

  if (digitalRead(ENC_A)) old_AB |= 0x02;  // Add current state of pin A
  if (digitalRead(ENC_B)) old_AB |= 0x01;  // Add current state of pin B

  encval += enc_states[(old_AB & 0x0f)];

  // Update counter if encoder has rotated a full indent, that is at least 4 steps
  if (encval > 3) {  // Four steps forward

    int changevalue = 1;
    counter = counter + changevalue;  // Update counter
    encval = 0;

    if (counter >= 255) {
      counter = 255;
    }
  } else if (encval < -3) {  // Four steps backward
    int changevalue = -1;
    counter = counter + changevalue;  // Update counter
    encval = 0;

    if (counter < 0) {
      counter = 0;
    }
  }
}
2

Looks like 1200, not 12, from your code?

This variable is unused. Was it intended for the 5 minute timeout you describe?

It's not safe to use read counter, or any variable except maybe byte/char variables, without protecting your code against interrupts occurring during the reading.

From this, I guess you found this code on the web and didn't write it, and don't understand it.

Dealing with interrupts is not a beginner level project. Why not use one of the various encoder libraries that have been written, and extensively tested, to help beginners like yourself?

2 Likes
3

I agree with @PaulRB that avoiding explicit use of interrupts is generally a good idea, an idea that is Good in inverse proportion to how much you know about doing.

Here

  analogWrite(11, counter);
}

when you write the new counter value to the fan output, you could check to see if it below some threshold value, and if so, reset a timer:

  analogWrite(11, counter);

  if (counter < 11250)
    runTimer = millis();
}

"Resetting a timer" is simply storing the current time. That is all.

Then, in a totally independent top level code section in the loop, check if the timer has expired. Here an expired timer is one that has fallen behind the current time by a sufficiently number of milliseconds. Check if it has been too long since the fan was seen to be running at a speed under the threshold:

  analogWrite(11, counter);

  if (counter < 1125)
    runTimer = millis();

// has it been running too fast for too long? 
  if (millis() - runTimer > 300000) {

// here take corrective action
// reset the counter to zero and maybe
// write zero to the fan output

  }
}

The above assumes the existence of a timer, in this context a timer is simply an unsigned long integer that is in scope. Easiest would be a global variable, one that is declared outside any function. Usually those appear at the top of the sketch.

I can't look closer at your code to fill in the commented block.

a7

4

Hello @PaulRB, thank you for your reply.

My mistake I meant 1200 not 12.

unsigned long _lastIncReadTime = micros();

That variable was used to skip some numbers when rotating the encoder to fast, since I'm not using it I removed it, but I forgot to delete the variable declaration. I'll make sure to update my code.

Can I ask, what do you mean when you say that it is not safe to use read counter?
And yes, I'm a total newbie over here!
I found the rotary encoder code on youtube and I've been using it because it has been the most precise, I used some libraries, like Encoder.h and RotaryEncoder.h but I had some trouble with numbers being skipped, and with that code the rotary encoder works perfectly. I'm not sure if this is due to the type of encoder that is used.

5

Why not set the counter to zero when time is up: counter = 0;

6

You didn't say what type of Arduino you are using, so I will assume Uno R3.

8-bit Arduino like Uno R3 deal with data 1 byte at a time. The variable counter is an int which is 2 bytes. So reading or updating counter takes 2 instructions. There is always a small chance that, if you are turning the encoder at that time, an interrupt could occur between those 2 instructions. The interrupt routine updates counter when the main code has only read or updated half of it, so can read or update a corrupted value. That's why it's important to disable interrupts before reading or updating a variable that the interrupt routine updates, and re-enable interrupts as quickly as possible afterwards to minimise the chance that an interrupt is missed.

7

What if the encoder value is 254 or 253? It is allowed to remain forever?

8

if you're just trying to in/decrease a value, you just need to see that the encoder changed value and in which direction.

a common approach is each is to recognize a change wheneve the Clk goes high and look at the Data to see in which direction.

using the encoder to recognize deltas implies that the value of something is never incremented above its max and never decremented below zero

9

This.

I assumed the test would be running faster than some threshold, like 97.5 percent top speed.

And yes, forever at, say, 95 percent.

a7

10

Hello @alto777, thank you for your reply.

I went through your answer and modified my code. I added the timer you mentioned as follows:

 analogWrite(11, counter);

  if (counter >= 255) {
    runTimer = millis();

    if (runTimer - previousTimer > 20000) {
      counter = 0;
      previousTimer = runTimer;
    }
  }

The code is working, once my counter is set to 255 and 20,000 ms have passed, the counter is set back to 0 and the speed of the fan also is at its minimum, which is what I wanted. The problem now is that since millis continues running, the "runTimer" keeps becoming larger, but the "previous timer" stopped, and if I wait for a minute the "runTimer" has become so large that when I set the counter back to 255, the interval is already larger than 20,000ms and it immediately resets the counter to 0.
I hope what I'm saying is understandable.

First round:
Counter = 255
runTimer: Reaches 20,000
previousTime: 0
Counter goes back to 0 and all is well.

Second round(After I waited a minute):
Counter = 255
runTimer: Already at 60,000, (already more than 20,000 ms larger than previousTimer)
previousTimer: 20,000
Counter immediately goes back to 0.

Below is my current code, I really appreciate all the help!

#include <Arduino.h>
#include <TM1637Display.h>
#define CLK 8
#define DIO 9
TM1637Display display(CLK, DIO);
// Define rotary encoder pins
#define ENC_A 2
#define ENC_B 3

volatile int counter = 0;

unsigned long runTimer = 0;
unsigned long previousTimer = 0;

void setup() {
  display.setBrightness(7);
  // Set encoder pins and attach interrupts
  pinMode(ENC_A, INPUT_PULLUP);
  pinMode(ENC_B, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(ENC_A), read_encoder, CHANGE);
  attachInterrupt(digitalPinToInterrupt(ENC_B), read_encoder, CHANGE);

  // Start the serial monitor to show output
  Serial.begin(115200);
}

void loop() {
  static int lastCounter = 0;

  // If count has changed print the new value to serial
  if (counter != lastCounter) {
    //Serial.println(counter);
    lastCounter = counter;
  }

  int V = map(counter, 0, 255, 0, 1200);
  display.showNumberDecEx(V, 0b01000000, false);
  analogWrite(11, counter);

  if (counter >= 255) {
    runTimer = millis();

    if (runTimer - previousTimer > 20000) {
      counter = 0;
      previousTimer = runTimer;
    }
  }
  Serial.println(runTimer);
  Serial.println(previousTimer);
}


void read_encoder() {
  // Encoder interrupt routine for both pins. Updates counter
  // if they are valid and have rotated a full indent

  static uint8_t old_AB = 3;                                                                  // Lookup table index
  static int8_t encval = 0;                                                                   // Encoder value
  static const int8_t enc_states[] = { 0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0 };  // Lookup table

  old_AB <<= 2;  // Remember previous state

  if (digitalRead(ENC_A)) old_AB |= 0x02;  // Add current state of pin A
  if (digitalRead(ENC_B)) old_AB |= 0x01;  // Add current state of pin B

  encval += enc_states[(old_AB & 0x0f)];

  // Update counter if encoder has rotated a full indent, that is at least 4 steps
  if (encval > 3) {  // Four steps forward

    int changevalue = 1;
    counter = counter + changevalue;  // Update counter
    encval = 0;

    if (counter >= 255) {
      counter = 255;
    }
  } else if (encval < -3) {  // Four steps backward
    int changevalue = -1;
    counter = counter + changevalue;  // Update counter
    encval = 0;

    if (counter < 0) {
      counter = 0;
    }
  }
}
11

I'm at the beach, so I can't study your code. But I can see immediately a difference to what I thought I wrote. If it is what I wrote, I got it enough slightly wrong for you to do a test.

  if (counter >= 255) {
    runTimer = millis();

    if (runTimer - previousTimer > 20000) {
      counter = 0;
      previousTimer = runTimer;
    }
  }

OK, it is not what I wrote. I took what I told you to do, and added what you tried to add to it, you just got bit creative, maybe thinking you needed to change it:

  if (counter < 255)
    runTimer = millis();

  if (millis() - runTimer > 300000) {
    counter = 0;
  }

This relies on attaining and maintaining 255, the very end of your range, which I have always thought was fraught.

Try the above, but consider

  if (counter < 250)
    runTimer = millis();   // move the timer up if the RPM is low enough.

                           // see if it has been too fast for too long
  if (millis() - runTimer > 20000) {
    counter = 0;
  }

which expresses a desire to stop when the speed has been 250 or greater for 20 seconds.

I don't know what, if any, roll previousTimer was meant to play. The pattern I wrote is entirely independent of anything else your code does. If you don't run the motor slow enough, the timer is never moved along and eventually expires, shutting it down.

a7

12

Here's a hacked version. It uses polling, not interrupts. It has the logic I write for your reset idea. It should work on your hardware.

I commented out the display I do not have.


//

# define MAX  25
# define TOOLONG  5000    // life too short

#define CLK 8
#define DIO 9

//TM1637Display display(CLK, DIO);
// Define rotary encoder pins

# define ENC_A 2
# define ENC_B 3

int counter = 0;


unsigned long runTimer = 0;

void setup() {
  //display.setBrightness(7);
  // Set encoder pins and attach interrupts
  pinMode(ENC_A, INPUT);  // pullup? not needed for mine
  pinMode(ENC_B, INPUT);


  // Start the serial monitor to show output
  Serial.begin(115200);
  Serial.println("Hi Mom!\n");
}

void loop() {
  static int lastCounter = 0;

  myEncoder();

// test just the encoder, uncomment return
// return;

  // If count has changed print the new value to serial
  if (counter != lastCounter) {
    Serial.println(counter);
    lastCounter = counter;
  }

// test encoder to counter logic, uncomment return
// return;

  int V = map(counter, 0, MAX, 0, 1200);
  //display.showNumberDecEx(V, 0b01000000, false);

  analogWrite(11, counter);

  if (counter < MAX)
    runTimer = millis();

  if (millis() - runTimer > TOOLONG) {
    Serial.println("                      Bzzzt!");
    counter = 0;
  }
}

void myEncoder() {
  static int lastClk = HIGH;

  int newClk = digitalRead(ENC_A);
  if (newClk != lastClk) {
    // There was a change on the CLK pin
    lastClk = newClk;
    int dtValue = digitalRead(ENC_B);
    if (newClk == LOW && dtValue == HIGH) {
      Serial.println("   CW");
      counter++; if (counter > MAX) counter = MAX;
    }
    if (newClk == LOW && dtValue == LOW) {
      Serial.println("CCW");
      if (counter) counter--;
    }
  }
}

I did get the interrupts to work after a fashion. I haven't time to see how it actually feels. The polled version is crisp and snappy enough.

Your code, except my changes, and a few tweaks of no consequence; matters of style I could not keep my hands from doing as the orders were coming from my elbows:

// now back to interrupts
//

# define MAX  25
# define TOOLONG  5000    // life too short

#define CLK 8
#define DIO 9

//TM1637Display display(CLK, DIO);
// Define rotary encoder pins

# define ENC_A 2
# define ENC_B 3

volatile int counter = 0;


unsigned long runTimer = 0;

void setup() {
  //display.setBrightness(7);
  // Set encoder pins and attach interrupts
  pinMode(ENC_A, INPUT);  // pullup? not needed for mine
  pinMode(ENC_B, INPUT);

  attachInterrupt(digitalPinToInterrupt(ENC_A), read_encoder, CHANGE);
  attachInterrupt(digitalPinToInterrupt(ENC_B), read_encoder, CHANGE);


  // Start the serial monitor to show output
  Serial.begin(115200);
  Serial.println("Hi Mom!\n");
}

void loop() {


  noInterrupts();
  int myCounter = counter;
  interrupts();

  read_encoder();

// test just the encoder, uncomment return
// return;

  // If count has changed print the new value to serial
  static int lastCounter = -1;  // no it does not!
  if (myCounter != lastCounter) {
    Serial.println(myCounter);
    lastCounter = myCounter;
  }

// test encoder to counter logic, uncomment return
// return;

  int V = map(counter, 0, MAX, 0, 1200);
  //display.showNumberDecEx(V, 0b01000000, false);

  analogWrite(11, counter);

  if (counter < MAX)
    runTimer = millis();

  if (millis() - runTimer > TOOLONG) {
    Serial.println("                      Bzzzt!");
    counter = 0;
  }
}

void myEncoder() {
  static int lastClk = HIGH;

  int newClk = digitalRead(ENC_A);
  if (newClk != lastClk) {
    // There was a change on the CLK pin
    lastClk = newClk;
    int dtValue = digitalRead(ENC_B);
    if (newClk == LOW && dtValue == HIGH) {
      Serial.println("   CW");
      counter++; if (counter > MAX) counter = MAX;
    }
    if (newClk == LOW && dtValue == LOW) {
      Serial.println("CCW");
      if (counter) counter--;
    }
  }
}


void read_encoder() {
  // Encoder interrupt routine for both pins. Updates counter
  // if they are valid and have rotated a full indent

  static uint8_t old_AB = 3;                                                                  // Lookup table index
  static int8_t encval = 0;                                                                   // Encoder value
  static const int8_t enc_states[] = { 0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0 };  // Lookup table

  old_AB <<= 2;  // Remember previous state

  if (digitalRead(ENC_A)) old_AB |= 0x02;  // Add current state of pin A
  if (digitalRead(ENC_B)) old_AB |= 0x01;  // Add current state of pin B

  encval += enc_states[(old_AB & 0x0f)];

  // Update counter if encoder has rotated a full indent, that is at least 4 steps
  if (encval > 3) {  // Four steps forward

    int changevalue = 1;
    counter = counter + changevalue;  // Update counter
    encval = 0;

    if (counter >= MAX) {
      counter = MAX;
    }
  } else if (encval < -3) {  // Four steps backward
    int changevalue = -1;
    counter = counter + changevalue;  // Update counter
    encval = 0;

    if (counter) --counter = 0;
  }
}

Adjust MAX and TOOLONG. Uncomment the display stuff.

HTH

a7

13

Edit: A different rotary encoder worked fine with the interrupt version below, and a different different encoder worked with INPUT_PULLUP and was entirely usable but not 100 percent of what it should be. I didn't test all those with the polling version. All are cheap KY-040 counterfeit knockoff.


I was fixing a shortcoming in the management of the volatile variable which is counter and I noticed that my encoder wasn't functioning very well. This sketch has a few more small changes, and it works somewhat better, but for my encoder the polled version works much better.

This is the corrected interrupt version:

// now back to interrupts
// edit: fix voaltile access completely

# define MAX  25
# define TOOLONG  5000    // life too short

#define CLK 8
#define DIO 9

//TM1637Display display(CLK, DIO);
// Define rotary encoder pins

# define ENC_A 2
# define ENC_B 3

volatile int counter = 0;


unsigned long runTimer = 0;

void setup() {
  //display.setBrightness(7);
  // Set encoder pins and attach interrupts
  pinMode(ENC_A, INPUT);  // pullup? not needed for mine
  pinMode(ENC_B, INPUT);

  attachInterrupt(digitalPinToInterrupt(ENC_A), read_encoder, CHANGE);
  attachInterrupt(digitalPinToInterrupt(ENC_B), read_encoder, CHANGE);


  // Start the serial monitor to show output
  Serial.begin(115200);
  Serial.println("Hi Mom!\n");
}

void loop() {
// grab a copy of the counter
  noInterrupts();
  int myCounter = counter;
  interrupts();

  read_encoder();

// test just the encoder, uncomment return
// return;

  // If count has changed print the new value to serial
  static int lastCounter = -1;  // no it does not!
  if (myCounter != lastCounter) {
    Serial.println(myCounter);
    lastCounter = myCounter;
  }

// test encoder to counter logic, uncomment return
// return;

  int V = map(myCounter, 0, MAX, 0, 1200);
  //display.showNumberDecEx(V, 0b01000000, false);

  analogWrite(11, myCounter);

  if (myCounter < MAX)
    runTimer = millis();

  if (millis() - runTimer > TOOLONG) {
    Serial.println("                      Bzzzt!");

    noInterrupts();
    counter = 0;
    interrupts();
  }
}

void read_encoder() {
  // Encoder interrupt routine for both pins. Updates counter
  // if they are valid and have rotated a full indent

  static uint8_t old_AB = 3;                                                                  // Lookup table index
  static int8_t encval = 0;                                                                   // Encoder value
  static const int8_t enc_states[] = { 0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0 };  // Lookup table

  old_AB <<= 2;  // Remember previous state

  if (digitalRead(ENC_A)) old_AB |= 0x02;  // Add current state of pin A
  if (digitalRead(ENC_B)) old_AB |= 0x01;  // Add current state of pin B

  encval += enc_states[(old_AB & 0x0f)];

  // Update counter if encoder has rotated a full indent, that is at least 4 steps
  if (encval > 3) {  // Four steps forward

    counter++;  // Update counter

    if (counter >= MAX) {
      counter = MAX;
    }
    encval = 0;
  } else if (encval < -3) {  // Four steps backward
    if (counter) counter--;
    encval = 0;
  }
}

I would move the encoder handling to a competent library now that you've had the fun of playing with that kind of code.

Or use the polling. As long as your loop runs freely, polling is effective.

a7

14

Thank you so much for your help, I really appreciate it!