Optical Encoder code

Using Arduino Programming Questions
1

Hey Guys,

I have been using some optical encoders to gather data from some motors. I was having trouble however, because the encoder count seemed to be gaining and losing value randomly throughout a motor move. I would rotate the motor shaft from 0-90 degrees, and the encoder count (on a 1200 PPR encoder) would be between roughly -6 - 6 at zero degrees, and 286-315 at 90 degrees (where as the actual values are 0, and 300. These are just example ranges of what I was getting, it was always +/- no more than 10ish from the real value.

I have done multiple experiments on my motors (Steppers) and have verified beyond any doubt that they are moving precisely where I ask them to.

I was very frustrated for a long time and even bought quadrature decoder chips (which didn't work out) and thought maybe my signal wires needed debounced. By chance one day I thought maybe it is a problem with my code (which I thought was very unlikely because of how simple the code is) but I copied and pasted an encoder reading program from the internet, and lo and behold it worked. The encoder count was going from 0-300 just as it should.

I get bothered when I don't know why something doesn't work so I was hoping someone would be able to look at the code I was using to tell me why it was randomly gaining/losing encoder counts. I will also include the code that -worked- if it is needed. The encoders have two open collector signal wires, and I was using an arduino due on the native port.

My code:

int PinA = 2;
int PinB = 3;
int counter_2 = 0;
volatile int counter = 0;
char in_char;
unsigned long my_time;
String send, info, tab = "\t";
unsigned long start, stop, elapsed;
int pin_a = 2, pin_b = 3;
void setup()
{
  SerialUSB.begin(9600);
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  start = millis();
  attachInterrupt(digitalPinToInterrupt(pin_a), Achange, RISING);
  attachInterrupt(digitalPinToInterrupt(pin_b), Bchange, RISING);
}

void loop()
{
  if (counter != counter_2)
  {
    noInterrupts();
    info = (String)counter;
    counter_2 = counter;
    interrupts();
    stop = millis();
    my_time = stop - start;
    send = my_time + tab + info;
    SerialUSB.println(send);
  }
}

void Achange()
{
  if (digitalRead(pin_b) == HIGH)
  {
    counter = counter - 1;
  }
  else
  {
    counter = counter + 1;
  }
}

void Bchange()
{
  if (digitalRead(pin_a) == LOW)
  {
    counter = counter - 1;
  }
  else
  {
    counter = counter + 1;
  }
}

Internet code that worked:

double val=0;
byte  PinA=6;
byte PinB=7;
int ASet;
int BSet;

void setup()
{
  SerialUSB.begin(9600);
  pinMode(PinA, INPUT);
  pinMode(PinB, INPUT);
  

  ASet = digitalRead(PinA);
  BSet = digitalRead(PinB);   // read the input pin
 
  attachInterrupt(PinA, INCRE, CHANGE);
  attachInterrupt(PinB, DECRE, CHANGE);
}

void loop()
{
  int time=millis()/1000; 
 if(time % 1==0)
  {
    SerialUSB.print(time);
    SerialUSB.print("  \t  ");
    SerialUSB.println(val);
    //val=0;
    delay(1000);
  }
}

void INCRE()
{
  ASet = digitalRead(PinA) == HIGH;
  val += (ASet != BSet) ? +1 : -1;
}

void DECRE()
{
  BSet = digitalRead(PinB) == HIGH;
  val += (ASet == BSet) ? +1 : -1;
}
2 
    info = (String)counter;

counter is an int. It makes NO sense to cast an int as a String.

3

PaulS:

    info = (String)counter;

counter is an int. It makes NO sense to cast an int as a String.

I am just doing this to be able to print all my info in one println statement. I combine it all as send=my_time+tab+info to effectively print out the time, followed by a tab, followed by the encoder count. Could this cause a problem?

4

My guess is that the problem is here

void loop()
{
  if (counter != counter_2)

You are reading those values without protecting them with noInterrupt()

I would always save the values from the ISR variables into working variables and then use the values in the working variables within loop(). For example

void loop() {
  noInterrupts();
    latestCounter = counter;
    latestCounter2 = counter2;
  interrupts();
  if (latestCounter != latestCounter2) {
    // etc

Actually your code makes another error because it reads counter and counter2 again and they then may have different values.

...R

5

I am just doing this to be able to print all my info in one println statement.

I am doing something stupid because I am lazy. Oh, well. Moving on.

6

Hi,
Can you post a link to the spec on your encoder, does it need pullup resistors?

Tom....... :slight_smile:

7

PaulS:
I am doing something stupid because I am lazy. Oh, well. Moving on.

Is it bad practice to do this? I only did it because the println functions on the arduino due's programming port are really slow compared to other arduinos and the native port (which I have recently switched to using instead, so there is no point in it anymore). I thought maybe sending one println statement instead of three would speed things up since there was going to be a lot of printing going on.

TomGeorge:
Hi,
Can you post a link to the spec on your encoder, does it need pullup resistors?

Tom....... :slight_smile:

Thanks for the reply Tom, the encoders were incredibly cheap chinese encoders that I couldn't find a datasheet for. I guess that is what I get for buying 10 dollar encoders. I found a forum online that explained how they were to be wired and yes, they did need pull-up resistors (I am using 2.2k pull ups). I would get you the model number but they are attached to my thesis robot in a way that makes it very difficult to reach without dis-assembly, I understand if there is nothing you can do to help because of that.

Robin2:
My guess is that the problem is here

void loop()

{
 if (counter != counter_2)




You are reading those values without protecting them with noInterrupt()

I would always save the values from the ISR variables into working variables and then use the values in the working variables within loop(). For example

void loop() {
 noInterrupts();
   latestCounter = counter;
   latestCounter2 = counter2;
 interrupts();
 if (latestCounter != latestCounter2) {
   // etc




Actually your code makes another error because it reads counter and counter2 again and they then may have different values.

...R

Thank you! That makes perfect sense.

8

Serial data is sent one byte at a time. It does not matter whether the data is buffered by three calls to print() (or println()) or 12 calls or one call. All that trying to do it in one call accomplishes is wasting resources.

9

PaulS:
Serial data is sent one byte at a time. It does not matter whether the data is buffered by three calls to print() (or println()) or 12 calls or one call. All that trying to do it in one call accomplishes is wasting resources.

I will be sure to get rid of it then, thanks for the help!

10