How to read an encoder and control a stepper motor without delay()

Hello all...
I want to read the A and B pins from an encoder using interrupts together with an OLED display used as a readout, much like a lathe digital readout. Then to control a stepper motor with the Arduino Motor Shield using buttons with set amounts of steps in ether direction. Both the encoder and stepper motor will be fitted to a linear actuator. I have looked at the playground.arduino.cc/Main/RotaryEncoders and have taken bit's to use, study and to try and learn the code. It has been great fun till I stumbled on getting both the encoder and stepper motor to work together. I cant have to much delay() commands in the code or it will miss the state changes made from the encoder. How can I do this? I would guess using the "millis()"?
After reading though Coding Academy "Linux Format" there is a fantastic bit about controlling the Arduino from an Android devise this would be the next step to learn instead of using buttons.

Parts in use are..

Arduino Mega 2560
Arduino Motor Shield,
SSD1306 - Adafruit - "OLED" [u]http://www.adafruit.com/datasheets/SSD1306.pdf[/u]
ENA1J-B28-L00128L - Bourns - "Encoder" http://docs-europe.electrocomponents.com/webdocs/008d/0900766b8008d78a.pdf
RS (191-8328) - "1.8DEG Stepper motor" http://docs-europe.electrocomponents.com/webdocs/001c/0900766b8001c018.pdf

I know this code is not %100 that is why I'm here. If I have missed anything out shout.

#include <Wire.h>
#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>
#define OLED_DC 48
#define OLED_CS 52
#define OLED_CLK 46
#define OLED_MOSI 44
#define OLED_RESET 50
Adafruit_SSD1306 display(OLED_MOSI, OLED_CLK, OLED_DC, OLED_RESET, OLED_CS);
#if (SSD1306_LCDHEIGHT != 32)
#endif
enum PinAssignments {
  encoderPinA = 21,
  encoderPinB = 20,
  clearButton = 42
};

volatile unsigned int encoderPos = 0;
unsigned int lastReportedPos = 1;

boolean A_set = false;
boolean B_set = false;
static boolean rotating=false;      // debounce management
int myValue;

const int ledPin =  43;      // the number of the LED pin

// Variables will change:
int ledState = LOW;             // ledState used to set the LED
long previousMillis = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 500;           // interval at which to blink (milliseconds)

void setup() {

  display.begin(SSD1306_SWITCHCAPVCC);
  display.display(); // show splashscreen
  delay(500);
  display.clearDisplay();

  pinMode(encoderPinA, INPUT); 
  pinMode(encoderPinB, INPUT); 
  pinMode(clearButton, INPUT);
  digitalWrite(encoderPinA, HIGH);  // turn on pullup resistor
  digitalWrite(encoderPinB, HIGH);  // turn on pullup resistor
  digitalWrite(clearButton, HIGH);
  attachInterrupt(2, doEncoderA, CHANGE);
  attachInterrupt(3, doEncoderB, CHANGE);
  pinMode(ledPin, OUTPUT);  

}

void loop() {


  display.setTextSize(2);
  display.setTextColor(WHITE);
  display.setCursor(0,0);
  display.print("EncoderPos");
  display.setTextSize(5);
  display.setTextColor(WHITE);
  display.print(myValue, DEC);
  display.display();
  display.clearDisplay();
  rotating = true;  // reset the debouncer

  if (lastReportedPos != encoderPos) {
    lastReportedPos = encoderPos;
  }
  if (digitalRead(clearButton) == LOW )  {
    encoderPos = 0;

  }
      (myValue = (encoderPos) * 1);
}

// Interrupt on A changing state
void doEncoderA(){
  // debounce
  if ( rotating ) delay (1);  // wait a little until the bouncing is done

  // Test transition, did things really change? 
  if( digitalRead(encoderPinA) != A_set ) {  // debounce once more
    A_set = !A_set;

    // adjust counter + if A leads B
    if ( A_set && !B_set ) 
      encoderPos += 1;

    rotating = false;  // no more debouncing until loop() hits again
  }
}

// Interrupt on B changing state, same as A above
void doEncoderB(){
  if ( rotating ) delay (1);
  if( digitalRead(encoderPinB) != B_set ) {
    B_set = !B_set;
    //  adjust counter - 1 if B leads A
    if( B_set && !A_set ) 
      encoderPos -= 1;

    rotating = false;
      unsigned long currentMillis = millis();
 
  if(currentMillis - previousMillis > interval) 
    // save the last time you blinked the LED 
    previousMillis = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
  }
}

Many Thanks,

      (myValue = (encoderPos) * 1);

I think this statement could use some more parentheses, don't you?

doEncoderA and doEncoderB are interrupt service routines. During those functions, you can NOT use delay(). Get them out of there.

I cant have to much delay() commands in the code or it will miss the state changes made from the encoder.

So, why are you trying to delay() in the ISR?

You shouldn't have delay calls in your encoder ISRs anyway. I normally poll encoders using either a tick interrupt or a task in a cooperative scheduler. That avoids tying up 2 interrupts and makes it easier to support several encoders. Polling at 1ms intervals is best for avoiding missed transitions, but you can typically get away with 2ms. You can find the code I use at GitHub - dc42/arduino: Reusable modules, drivers and patches for the Arduino platform.

Of course, if you don't have too much else going on (for example, you don't use any library functions that may block such as Serial.print), and there are no delay calls elsewhere in your code, then you can do the polling directly within loop() instead.

Some questions...
If the stepper and encoder are hooked to the same thing, do you even need the encoder? All it's going to tell you is how far the motor turned, but since it's a stepper motor you should know that anyway just by counting the steps.

Also, I see it's a quadrature encoder. Those are great when you don't know which way the thingy (technical term) is rotating. Is that the case here, or do you already know the direction? If so, then you can vastly simplify the encoder handling code (just count interrupts).

Other points
ISRs should be as short and simple as possible. Don't do delays, don't do library calls that take a long time. keep the logic short and simple. Ideally, you should just set flags or change counters and do the rest in the main loop.

In general, encoders don't need to be debounced. Every transition means something. There is the case of "jitter" but you can fix that by ignoring "double" interrupts (if you get A, A, A, A with no B's then this is probably jitter).

On the other hand, buttons DO need to be debounced, or they are unreliable as input. Adding debouncing code will complicate your sketch, but it's necessary. Look in the playground for stuff you can copy or use directly.

bugs/nits
The variable encoderPos is declared as an unsigned int. It is entirely possible that it could go below 0 depending on which way the encoder rotates. Also, if you get a lot of movement, it could overflow. Try using volatile long as the type instead. Change lastReportedPos to match.

You don't need an enum for the input pins. #define's work just fine for pin numbers in most cases.

Don't do clearDisplay in the main loop. It's not necessary and is probably not what you want. You could probably move some of the LCD writing to the setup function and just display the number in the main loop.

HTH,

Hi all thanks for your support,

How is this? I have to shoot, but I'll be hack on here tomorrow!

#include <Wire.h>
#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>
#define OLED_DC 48
#define OLED_CS 52
#define OLED_CLK 46
#define OLED_MOSI 44
#define OLED_RESET 50
Adafruit_SSD1306 display(OLED_MOSI, OLED_CLK, OLED_DC, OLED_RESET, OLED_CS);
#if (SSD1306_LCDHEIGHT != 32)
#endif
enum PinAssignments {
  encoderPinA = 21,
  encoderPinB = 20,
  clearButton = 42
};
volatile unsigned int encoderPos = 0;
unsigned int lastReportedPos = 1;
boolean A_set = false;
boolean B_set = false;

void setup() {
  display.begin(SSD1306_SWITCHCAPVCC);
  display.display();
  delay(500);
  display.clearDisplay();
  pinMode(encoderPinA, INPUT); 
  pinMode(encoderPinB, INPUT); 
  pinMode(clearButton, INPUT);
  digitalWrite(encoderPinA, HIGH);
  digitalWrite(encoderPinB, HIGH);
  digitalWrite(clearButton, HIGH);
  attachInterrupt(2, doEncoderA, CHANGE);
  attachInterrupt(3, doEncoderB, CHANGE);
}
void loop() {
  display.setTextSize(2);
  display.setTextColor(WHITE);
  display.setCursor(0,0);
  display.print("EncoderPos");
  display.setTextSize(5);
  display.setTextColor(WHITE);
  display.print(encoderPos, DEC);
  display.display();
  display.clearDisplay();
  if (lastReportedPos != encoderPos) {
    lastReportedPos = encoderPos;
  }
  if (digitalRead(clearButton) == LOW )  {
    encoderPos = 0;
  }
}

void doEncoderA(){
  A_set = !A_set;
  if ( A_set && !B_set ) 
    encoderPos += 1;
}
void doEncoderB(){
  if( digitalRead(encoderPinB) != B_set ) {
    B_set = !B_set;
    if( B_set && !A_set ) 
      encoderPos -= 1;
  }
}

If the stepper and encoder are hooked to the same thing, do you even need the encoder? All it's going to tell you is how far the motor turned, but since it's a stepper motor you should know that anyway just by counting the steps.

Telling the stepper to step, and having it actually step, are two different things. The encoder tells you whether it actually stepped.

Basically, I want automated control from the stepper via code and buttons. Manual control will also be required by the user to operate the linear actuator by hand at set parts, as the stepper can't sense any feed back to give to the user.

Hi click/HTH, thank you!
Both the stepper and encoder will be connected to the same shaft and the code/encoder will need to account for directional changes made by the user. I was using an inexpensive DPL12V2424A20FR (See PDF) from RS, I had to use denouncing in both circuit and code in the first bit of code I added. I left the denouncing code in when I obtained the new encoder I'm using now, it works really well but it still missed a few steps over a few turns going forwards and backwards, would this of been the delays in the code? The new code works really well now. I just need to work through the code regarding your last post.
Please please remember I have only just recently stared working with Arduino's and my code knowledge is limited. I'm still learning the do's and don't s too.

http://docs-europe.electrocomponents.com/webdocs/0ccf/0900766b80ccf883.pdf

How can I get a way with out using these?

#include <Wire.h>
#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>

--
  display.clearDisplay(); I took out, but had to pop it back in as the display.print wrote on top of the last write.

I will be back after a brake...

How can I do this?

Two approaches:

  1. use a state machine: this requires saving the previous state of the pins and determine the rotation based on the previous state + current state. Coupled with an array, this is incredibly fast.
  2. use an isr: so you don't miss a pulse.

When you combine the two, you can read the rotation easily into thousands of pulses per second.

Hi dhenry,
How would I implement your approaches into my existing code?

Many thanks,

Just thought I would add a picture of the setup as it is now..

http://www.circuitsathome.com/mcu/reading-rotary-encoder-on-arduino

This is approach 1:

const signed char encoder_sm[]={0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0};

unsigned char encoder_read(void) {
        static signed char encoder_output=0;
	static unsigned char ABs=0x00;

	ABs = (ABs << 2) & 0x0f;
        ABs |= ((ENC_PORT & ENC_A)?0x02:0x00) | ((ENC_PORT & ENC_B)?0x01:0x00);
        encoder_output += encoder_sm[ABs];
	return encoder_sm[ABs];				//return the relative value (+1 = clockwise, 0, -1 = counterclockwise)
        //return encoder_output;                             //alternatively, return the absolute value
}

Each time this routine is called, it reads off ENC_A / ENC_B on ENC_PORT and determines if the rotation is clock or counter-clockwise, or return the absolute value (via encoder_output).

This routine can be called in the main loop or in an isr:

volatile signed char enc_direction=0;
volatile signed char enc_output=0;
volatile unsigned char enc_flag=0;

ISR(PC/INTx) {
  house_cleaning();
	ABs = (ABs << 2) & 0x0f;
        ABs |= ((ENC_PORT & ENC_A)?0x02:0x00) | ((ENC_PORT & ENC_B)?0x01:0x00);
        encoder_output += encoder_sm[ABs];
	enc_direction = encoder_sm[ABs];				//return the relative value (+1 = clockwise, 0, -1 = counterclockwise)
  enc_flag = 1; //set a flag to indicate new data
}

You will need two isrs, one for A and one for B.

Your application code would be something like this:

  if (enc_flag) {  //new data off the encoder?
    enc_flag = 0; //reset the flag
    do_something_with_encoder_data();
  }

Hello,

Appolgies, I didn't have much luck with updating the code dh, I couldn't get it working sadly. However I have been using this link for a bit and I just cut, pasted and edited it and it works flawlessly.
Arduino Playground - RotaryEncoders - when I see the full code I can work it out, a bit. I get a bit lost looking at parts of code, sorry dh.

Now.. How can I get this all working nicely with the stepper motor, at the moment I'm thinking of getting a another Arduino to use it on it's own with the Motor Shield, then I don't have to worry about using delays to setup the speed. Suggestions? Nothing to extreme chaps.

#include <Wire.h>
#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>
#define OLED_DC 48
#define OLED_CS 52
#define OLED_CLK 46
#define OLED_MOSI 44
#define OLED_RESET 50
Adafruit_SSD1306 display(OLED_MOSI, OLED_CLK, OLED_DC, OLED_RESET, OLED_CS);
#if (SSD1306_LCDHEIGHT != 32)
#endif
enum PinAssignments {
  encoderPinA = 20,
  encoderPinB = 21,
  zeroButton = 42
};
volatile unsigned int encoderPos = 0;
unsigned int lastReportedPos = 1;
boolean A_set = false;
boolean B_set = false;
void setup() {
  pinMode(encoderPinA, INPUT); 
  pinMode(encoderPinB, INPUT); 
  pinMode(zeroButton, INPUT);
  digitalWrite(encoderPinA, HIGH);
  digitalWrite(encoderPinB, HIGH);
  digitalWrite(zeroButton, HIGH);
  attachInterrupt(2, doEncoderA, CHANGE);
  attachInterrupt(3, doEncoderB, CHANGE);
  display.begin(SSD1306_SWITCHCAPVCC);
}
void loop(){ 
  if (lastReportedPos != encoderPos) {
    lastReportedPos = encoderPos;
  }
  if (digitalRead(zeroButton) == LOW)  {
    encoderPos = 0;
  }
  display.setTextSize(2);
  display.setTextColor(WHITE);
  display.setCursor(0,0);
  display.print("EncoderPos");
  display.print(encoderPos, DEC);
  display.display();
  display.clearDisplay();
}
void doEncoderA(){
  A_set = digitalRead(encoderPinA) == HIGH;
  encoderPos += (A_set != B_set) ? +1 : -1;
}
void doEncoderB(){
  B_set = digitalRead(encoderPinB) == HIGH;
  encoderPos += (A_set == B_set) ? +1 : -1;
}

I get lost with all the "+=" "==" " ? +1 : -1" bit's...

 encoderPos += (A_set != B_set) ? +1 : -1;

==

if (A_set != B_set) 
  {
    encoderPos = encoderPos + 1;
  }
  else
  {
    encoderPos = encoderPos - 1;
  }
if (A_set != B_set) 
  {
    encoderPos = encoderPos + 1;
  }
  else
  {
    encoderPos = encoderPos - 1;
  }

=

if A_set "is not equal to" B_set = add 1 else subtract 1..

Thanks AWOL,

Not a problem. It is just different ways of skinning the same cat.

One question....

Why did this code not work, at all with my Arduino Motor Shield even when setting up the pins in the code?
http://www.circuitsathome.com/mcu/reading-rotary-encoder-on-arduino