Running a dc motor with encoder

Hi everyone.

I was trying to run my dc motor and stop it at a specific location using encoder. Then run it back to its initial position.

I am using this dc motor with this motor shield.

This is the code I have written

#define encoder0PinA_M1  2
#define encoder0PinB_M1  22
int EnablePin = 8;
int PWMPin1 = 3;
int PWMPin2 = 11;

volatile signed int encoder0Pos = 0;
void setup() 
{ 
  pinMode(encoder0PinA_M1, INPUT); 
  pinMode(encoder0PinB_M1, INPUT); 
  pinMode(EnablePin, OUTPUT);
  pinMode(PWMPin1, OUTPUT);
  pinMode(PWMPin2, OUTPUT);
  attachInterrupt(0, doEncoder, CHANGE);  
  Serial.begin (9600);
}
void doEncoder() 
{
  if (digitalRead(encoder0PinA_M1) == digitalRead(encoder0PinB_M1)) 
  {
    encoder0Pos++;
  } 
  else {
    encoder0Pos--;
  }
}

void rotateforward()
{
  digitalWrite(EnablePin, HIGH);
  analogWrite(PWMPin2,0);
  analogWrite(PWMPin1,40);
  
}
void rotatebackward()
{
  digitalWrite(EnablePin, HIGH);
  analogWrite(PWMPin2,40);
  analogWrite(PWMPin1,0);
  
}
void motorSTOP()
{
  digitalWrite(EnablePin, LOW);
  analogWrite(PWMPin2,0);
  analogWrite(PWMPin1,0);

}
void loop()
{
 rotateforward();
  if(encoder0Pos >= 40)
  {
    motorSTOP();
  }
rotatebackward();
  if(encoder0Pos>=0)
  {
    motorSTOP();
  }
}

Above code does not work. My motor does not move. However if I remove this part:

rotatebackward();
  if(encoder0Pos>=0)
  {
    motorSTOP();
  }

It runs successfully and stops at 40 count.

I don't know where I am going wrong. I even tried adding delays between the two steps, which results to motor simply moving forward and not stopping at 40 counts.

Thanks

if() does not cause the loop to stop and wait for that condition to become true. You could use a while() loop instead. Or a do..until().

You are calling both rotateforward() and rotatebackward() every time through loop - that's not sensible is it?

You need a variable to hold state so you know whether you are going forwards or backwards.

You only need to call rotateforward() when the state changes to FORWARDs, ditto backwards. Check the
encoder each time round loop to see if the state needs to change, and if so, change it. Simple.

Also you should protect the reading of encoder0Pos volatile variable, since it is more than one byte
and being written by your ISR:

int read_encoder ()
{
  noInterrupts () ;
  int res = encoder0Pos () ; // read the volatile protected from interrupts
  interrupts () ;
  return res ;
}

If you don't do this occasionally you'll get a corrupt value reading encoder0Pos when the ISR runs between
byte reads.

Thanks MarkT and MorganS.

MorganS: I will give it a try thanks.

MarkT :Since initial encoderpos is zero, Motor state won't change until I move it forward or backward. I am getting confused. Can you explain it with a small pseudo code?