self-written encoder class is malfunctioning

Hello,

In a nutshell–I’m using two PWM motors, and am trying to get the Arduino to react after certain encoder values are reached.

So I started off with this code below. It doesn’t have the encoders as a class, and it was working just fine. The value 475 is used because it’s the number of ‘ticks’ for one wheel rotation.

 /* Read Quadrature Encoder
   Connect Encoder to Pins encoder0PinA, encoder0PinB, and +5V.

   Sketch by max wolf / www.meso.net .... except it's been modified by yours truly :)
   v. 0.1 - very basic functions - mw 20061220
*/

//  9/21/18
//motor A (right)
int enA = 5; //enable A, pin 7 on the Motor Module
int in1 = 42; //btw, this is mapping pins on the Motor Module (ie. the one labeled IN1) to pins on the Arduino (number 6)
int in2 = 43;

//motor B (left)
int enB = 6;
int in3 = 44; 
int in4 = 45;


int right_speed;
int left_speed;
char SIDE;

int valR;
int encoderRPinA = 35;
int encoderRPinB = 34;
int encoderRPos = 0;
int encoderRPinALast = LOW;
int nR = LOW;
//---------------------

int valL;
int encoderLPinA = 37; //yellow
int encoderLPinB = 36; //white
int encoderLPos = 0;
int encoderLPinALast = LOW;
int nL = LOW;

void setup() {
  pinMode (encoderRPinA, INPUT);
  pinMode (encoderRPinB, INPUT);
  pinMode (encoderLPinA, INPUT);
  pinMode (encoderLPinB, INPUT);
  Serial.begin(9600);

  pinMode(enA, OUTPUT);
  pinMode(in1, OUTPUT);
  pinMode(in2, OUTPUT);

  pinMode(enB, OUTPUT);
  pinMode(in3, OUTPUT);
  pinMode(in4, OUTPUT);

//this sets it so that the robot is moving forward (ie. both wheels in same direction)
  digitalWrite(in1, LOW);
  digitalWrite(in2, HIGH);
  digitalWrite(in3, LOW);
  digitalWrite(in4, HIGH);
}

void loop() {
  nR = digitalRead(encoderRPinA);
  if (((encoderRPinALast == LOW) && (nR == HIGH))) {
    if (digitalRead(encoderRPinB) == LOW) {
      encoderRPos--;
    } else {
      encoderRPos++;
    }
 //   Serial.println(encoderRPos);
  }
//------------------------------

  nL = digitalRead(encoderLPinA);
  if (((encoderLPinALast == LOW) && (nL == HIGH))) {
    if (digitalRead(encoderLPinB) == LOW) {
      encoderLPos--;
    } else {
      encoderLPos++;
    }
  //  Serial.println(encoderLPos);
  }

  

 if(abs(encoderRPos) < abs(475))
  {
    analogWrite(enB, 90);
    //Serial.println("hooray!");
  }
else if(abs(encoderRPos) > abs(475))
{
   analogWrite(enB, 0);
  // Serial.println("boo!");
}
  encoderRPinALast = nR;
//------------------------------------------

  if(abs(encoderLPos) < 475)
  {
    analogWrite(enA, 90);
  }
else if(abs(encoderLPos) > 475)
{
   analogWrite(enA, 0);

}
  encoderLPinALast = nL;

//analogWrite(enA, 50);
//analogWrite(enB, 50);
}

Okay so then I wanted to turn the encoders into a class so that I didn’t have to write these huge blocks of code. This was my translation:

class Encoder{
 private:
 int encoderPinA; //yellow wire
 int encoderPinB; //white wire
 int encoderPos = 0; 
 int encoderPinALast = LOW;
 int n = LOW;

 public:
 Encoder (int pinA, int pinB){
 encoderPinA = pinA;
 encoderPinB = pinB;
 pinMode(encoderPinA, INPUT);
 pinMode(encoderPinB, INPUT);
 
 }

 int limit(int lim){ //tells if the wheel has moved more than it's supposed to
  n = digitalRead(encoderPinA);
  if (((encoderPinALast == LOW) && (n == HIGH))) {
    if (digitalRead(encoderPinB) == LOW) {
      encoderPos--;
    } else {
      encoderPos++;
    }
    Serial.println(encoderPos);
  }
//------
  if(abs(encoderPos) < abs(lim))
  {
    return 0; //false; it hasn't moved more than it's supposed to
  }
  else if(abs(encoderPos) > abs(lim))
  {
    return 1; //true, it has reached/surpassed the limit
    }
     encoderPinALast = n;
 }
};

Encoder right_enc(35, 34);
Encoder left_enc(37, 36);

void setup() {
 Serial.begin(115200);
 Serial.println("Starting up...");
}

void loop() {
left_enc.limit(200); //this is basically just getting the above function to run so I can see whether or not it's working
}

But instead of neatly printing out the encoder values as I turn the wheel, this is what happens: I will turn the wheel in the (-) direction, and suddenly the values will race away to -inf. I will turn the wheel in the (+) direction, and the values will race away to +inf. Eventually as I continue to do this, the values will just pick a direction on their own and continue to race away.

I couldn’t find any difference in setup between the two codes (but maybe that’s the issue). Please advise on how to get the second code to work properly.

Thanks!

the values will race away to -inf

That would be because you are not properly detecting a change in the encoder state.

Under what circumstances will this line be executed?

    encoderPinALast = n;

jremington:
Under what circumstances will this line be executed?

    encoderPinALast = n;

In case you don't get it, I think jremington is saying you always return from the function above this line, so it never reaches this line.

I would expect the line “encoderPinALast = n;” would be executed when

abs(encoderPos) == abs(lim)

Since the OP tests for both GT and LT but allows the EQ case to fall through. Maybe it is because the return value is undefined in this case? Hard to say without seeing how it’s used.

Most of the time, the function returns without updating encoderPinALast, which means it does not correctly detect a change in encoder state.

Ah, now I see said the blind man. Thank you.

Great example of how to create a bug with lax/poor/sloppy/careless statement order.

And the other notable issue:

    pinMode (encoderPinA, INPUT);
    pinMode (encoderPinB, INPUT);

No pullups - the vast majority of encoders out there are open-collector, so INPUT_PULLUP is
the mode to use.

Hi guys, thanks so much for the responses! I see the error now :).

One more q–so all the encoder values are read in and printed to the Serial Monitor correctly when I have baud rate set to 115200, but I know the standard is 9600. If I don’t have the encoder values printing, can a 9600 baud rate still cause problems?

I’m using a 9600 baud rate in some other code that I would like to incorporate this encoder code into (would prefer not to have to change everything), but sometimes the code just skips over the encoder ‘command’ entirely.

Here’s what it looks like:
(What I’m trying to do is get the robot to navigate autonomously, so I have a sensor class, motor class, etc in there as well).

//11/14/18

//created 9/2118 by Kanwal Ahmad
//not original code; this is a remix of Filename: NeuroRobot_Original_Code provided by Alex Kaiser
//plus some other stuff I guess :)

class Motor{
 private:
 int pin_enable; //this is the thing that you set the speed with
 int pin_forward; //basically each motor has two of these pins, 
 int pin_backward; //and you use them to reverse polarity and switch direction
 
 public:
 Motor(int enable, int one, int two){
 pin_enable = enable;
 pin_forward = one;
 pin_backward = two;
 //in the old code they essentially combined these last two steps with something fancy
 //oh well, this is easier to understand lol
 }
 void motorSpeed(int Speed){
 if(Speed > 0 || Speed == 0){ //here, I'm asking which way to set the polarity, with the idea being that I can input pos. or neg. speeds into the function
 digitalWrite(pin_forward, HIGH);
 digitalWrite(pin_backward, LOW);
 analogWrite(pin_enable, Speed);
 }
 else{
 digitalWrite(pin_forward, LOW);
 digitalWrite(pin_backward, HIGH);
 analogWrite(pin_enable, -Speed);
 }
 }
};

//Reverse Polarity protector is plugged into Pin A12 on the MEGA shield
Motor right_motor(6, 42, 43);
Motor left_motor(7, 44, 45);
char SIDE;
int right_speed;
int left_speed;

void setSpeeds(int bothSpeed){
 right_motor.motorSpeed(bothSpeed);
 left_motor.motorSpeed(bothSpeed);
 }

void turn(int bothSpeed)
{
 right_motor.motorSpeed(bothSpeed);
 left_motor.motorSpeed(-bothSpeed);
}

//__________________________________________________
//encoder setup
class Encoder{
 private:
 int encoderPinA; //yellow wire
 int encoderPinB; //white wire
 int encoderPos = 0; 
 int encoderPinALast = LOW;
 int n = LOW;

 public:
 Encoder (int pinA, int pinB){
 encoderPinA = pinA;
 encoderPinB = pinB;
 pinMode(encoderPinA, INPUT);
 pinMode(encoderPinB, INPUT);
 }

 int limit(int lim){ //tells if the wheel has moved more than it's supposed to
  n = digitalRead(encoderPinA);
  if (((encoderPinALast == LOW) && (n == HIGH))) {
    if (digitalRead(encoderPinB) == LOW) {
      encoderPos--;
    } else {
      encoderPos++;
    }
 //   Serial.println(encoderPos);
  }
       encoderPinALast = n;
//------
  if(abs(encoderPos) < abs(lim))
  {
    return 0; //false; it hasn't moved more than it's supposed to
  }
  else if(abs(encoderPos) > abs(lim))
  {
    return 1; //true, it has reached/surpassed the limit
    }
 }
};
Encoder right_enc(35, 34);
Encoder left_enc(37, 36);

//____________________________________________________
//now let's start working with the sensors
//set up the LongRange sensor class "Sensor"
class Sensor{
 private:
 int trigPin;
 int echoPin;
 long duration;
 int distance;

void trigger(){
 //clears the trigpin
 digitalWrite(trigPin, LOW);
 delayMicroseconds(2);
 digitalWrite(trigPin, HIGH);
 delayMicroseconds(2);
 digitalWrite(trigPin, LOW);
 }

public:
 Sensor(int trig, int echo){
 trigPin = trig;
 echoPin = echo;

pinMode(trigPin, OUTPUT);
pinMode(echoPin, INPUT);
 }
 
 int dist() {
 trigger(); //i literally floundered for the longest time BC I FORGOT THIS AAAAH
 
 //reads the echopin and returns the sound wave travel time in ms
 duration = pulseIn(echoPin, HIGH);
 //calculate the distance
 distance = duration*.034/2;

return distance;
 }
};

//now let's instantiate the actual sensors (yay!)
//it's (trig, echo)
Sensor fl(52, 53); //ie back left 40, 41
Sensor fm(18, 19);
Sensor fr(16, 17);

byte rx_byte = 0; // stores received byte
//___________________________________________________

void adjust(){
Serial.println(fm.dist()); //so middle sensor
Serial.println(fl.dist()); //so left sensor
Serial.println(fr.dist()); //so right sensor
Serial.println("-------");

int tooclose = 10;
int justright = 20;
int goaway = 5;

delay(100);

if(fm.dist() > tooclose && fl.dist() > goaway && fr.dist() > goaway)
{
 Serial.println("FRONT");
 rx_byte = 'F';
 setSpeeds(100);
}
 else if(fl.dist() < tooclose && fl.dist() < fr.dist())
 {
 Serial.println("RIGHT");
 rx_byte = 'R';
 while(left_enc.limit(100) == 0 || right_enc.limit(100) == 0){
 turn(120);
 //Serial.println("enc");
 }
 }
 else if(fr.dist() < tooclose)
 {
 Serial.println("LEFT");
 rx_byte = 'L';
 while(left_enc.limit(100) == 0 ||right_enc.limit(100) == 0){
 turn(-120);
 // Serial.println("enc");
 }
 }
  else
 {
 Serial.println("BACK");
 rx_byte = 'B';
 while(left_enc.limit(200) == 0 || right_enc.limit(200) == 0){
 setSpeeds(-70);
 // Serial.println("enc");
}
 }

/*
if(fm.dist() > justright)
{
  rx_byte = '3';
}
else if (fm.dist() > tooclose)
{rx_byte = '2';
}
else if (fm.dist() > goaway)
{rx_byte = '1';
}
else
{rx_byte = '0';
}
*/
}
//___________________________________________________

void readSpeed() //a random function I wrote before
 {
 char SIDE = Serial.read(); //read a character that will determine which motor speed to set in next if-loops
 int in = Serial.parseInt(); //now read the motor speed
 if (SIDE == 'R'){ //R designates the right motor
 right_motor.motorSpeed(in);
 right_speed = in;
 }
 else if (SIDE == 'L') { //L designates the left motor
 left_motor.motorSpeed(in);
 left_speed = in;
 }
 else if (SIDE == 'B'){
 setSpeeds(in);
 right_speed = in;
 left_speed = in;
 }
 Serial.print("Right: ");
 Serial.println(right_speed, DEC); //print out the new speeds (ie. check that the correct one was changed)
 Serial.print("Left: ");
 Serial.println(left_speed, DEC);
 Serial.println("--------------");
 delay(10);
}
//_____________________________________________________

void setup() {
 Serial.begin(9600);
 Serial.println("Starting up...");
  Serial3.begin(9600); // serial port 3
  //setSpeeds(100);
}

void loop() {
 adjust();
 if (Serial3.available()) {
 Serial3.write(rx_byte);
 }
}

so I’m not printing anything specifically related to the encoders onto the Serial Monitor, but I am printing what direction the robot needs to move (for ex. RIGHT, etc). Originally the encoders would engage only sporadically (fixed by not printing anything… or so it seems), but now the sensors aren’t ‘sensing’ that the robot needs to turn except sporadically.

Do I need to use attachInterrupt somewhere? (How is that done)? Or does it look like its another issue?

Thanks again, everybody :).

 if (Serial3.available()) {

Serial3.write(rx_byte);
}

Really? If there is a byte available, don't read it but send some other byte back? Then it is still available.

To print the values twice per second, do this....

static unsigned long lastPrinted;
const unsigned long printInterval = 500; //milliseconds
if(millis() - lastPrinted > printInterval) {
  lastPrinted = millis();
  Serial.print("whatever");
}

Really? If there is a byte available, don't read it but send some other byte back? Then it is still available.

I think the code should be:

if (Serial3.availableForWrite() > 0) {
 Serial3.write(rx_byte);
 }