Go Down

Topic: Rotary encoder and servo working together (Read 929 times) previous topic - next topic

dukereduker

Hi I'm 15 and with a team working on an autonomous robotic sailboat. To make the sails trim autonomously we have constructed a wind vane mainly consisting an incremental rotary encoder. What I am trying to do now is make the position of the servo match the position of the rotary encoder. I'm not getting any errors while compiling but it simply just doesn't do anything. Here's the code I have constructed/modified so far any help would be very very much appreciated, so thank you in advance.

Code: [Select]
#include <Servo.h>
//these pins can not be changed 2/3 are special pins
int encoderPin1 = 2;
int encoderPin2 = 3;

Servo myservo;

int servopos = 0;

volatile int lastEncoded = 0;
volatile long pos = 0;

long lastencoderValue = 0;

int lastMSB = 0;
int lastLSB = 0;

void setup() {
  Serial.begin (9600);
 
  myservo.attach(9);

  pinMode(encoderPin1, INPUT);
  pinMode(encoderPin2, INPUT);

  digitalWrite(encoderPin1, HIGH); //turn pullup resistor on
  digitalWrite(encoderPin2, HIGH); //turn pullup resistor on

  //call updateEncoder() when any high/low changed seen
  //on interrupt 0 (pin 2), or interrupt 1 (pin 3)
  attachInterrupt(0, updateEncoder, CHANGE);
  attachInterrupt(1, updateEncoder, CHANGE);

}

void loop(){
 
if(servopos < 0)
 
for(servopos > pos; servopos = pos; pos -=1){
  myservo.write(servopos * -1);
  delay(5);
}

for(servopos < pos; servopos = pos; pos +=1){
  myservo.write(servopos * -1);
  delay(5);
}
;


if(servopos > 0)
 
for(servopos > pos; servopos = pos; servopos -=1){
  myservo.write(servopos);
  delay(5);
}

for(servopos < pos; servopos = pos; servopos +=1){
  myservo.write(servopos);
  delay(5);
}
;


  Serial.println(pos);
  delay(1000); //just here to slow down the output, and show it will work  even during a delay
  Serial.println(servopos);
  delay(1000);
}


void updateEncoder(){
  int MSB = digitalRead(encoderPin1); //MSB = most significant bit
  int LSB = digitalRead(encoderPin2); //LSB = least significant bit

  int encoded = (MSB << 1) |LSB; //converting the 2 pin value to single number
  int sum  = (lastEncoded << 2) | encoded; //adding it to the previous encoded value

  if(sum == 0b1101 || sum == 0b0100 || sum == 0b0010 || sum == 0b1011) pos ++;
  if(sum == 0b1110 || sum == 0b0111 || sum == 0b0001 || sum == 0b1000) pos --;

  lastEncoded = encoded; //store this value for next time
 
}

UKHeliBob

Your for loops look very odd.  Can you explain what you think they are doing ?

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

nicoverduin

#2
Jun 08, 2013, 11:35 pm Last Edit: Jun 09, 2013, 10:43 am by nicoverduin Reason: 1
Shouldn't this
Code: [Select]

for(servopos > pos; servopos = pos; servopos -=1){


be

Code: [Select]

for(servopos > pos; servopos != pos; servopos -=1){
Met vriendelijke groet / kindest regards
Nico Verduin
www.verelec.nl

dukereduker

I will repost in a bit with more explination on what I think the code is doing. But if you're talking about the for loops I think that if the servo position is lesser than the rotary encoder position then it should raise by incriments of 1 until it reaches it and vise versa. I think the problem may be that I am using a continuous rotation servo not a standard servo so this code is doing what it should be doing for a standard servo I think but could somebody help me adust it to a continuous rotation servo?

UKHeliBob

For loops require 3 elements, an initialisation, a condition and an increment (or decrement)

The initialization happens first and only once. Each time through the loop, the condition is tested; if it's true, the statement block in braces, and the increment is executed, then the condition is tested again. When the condition becomes false, the loop ends.

Looking at one of your for loops
Code: [Select]
for(servopos < pos; servopos = pos; pos +=1)
The initialisation is done by comparing 2 variables and the answer will be either 0 or 1, but it is not assigned to a variable
The condition sets one variable to another instead of comparing them
The increment adds one to a variable that has not been initialised in the for loop

What you need to do is to compare servopos and pos to determine which way the servo needs to move and then, separately from the comparison move the servo appropriately using a for loop
Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

nicoverduin

Also you cannot write negative values to the servo. This piece of code is from the servo library:
Code: [Select]

void Servo::write(int value)
{
  if(value < MIN_PULSE_WIDTH)
  { // treat values less than 544 as angles in degrees (valid values in microseconds are handled as microseconds)
    if(value < 0) value = 0;
    if(value > 180) value = 180;
    value = map(value, 0, 180, SERVO_MIN(), SERVO_MAX());
  }
  this->writeMicroseconds(value);
}
Met vriendelijke groet / kindest regards
Nico Verduin
www.verelec.nl

scottyjr

Where, in the loop, is the encoder value used? - Scotty

dukereduker

Code: [Select]
#include <Servo.h>
//these pins can not be changed 2/3 are special pins
int encoderPin1 = 2;
int encoderPin2 = 3;

Servo myservo;

int servopos = 0;

volatile int lastEncoded = 0;
volatile long pos = 0;

long lastencoderValue = 0;

int lastMSB = 0;
int lastLSB = 0;

void setup() {
  Serial.begin (9600);
 
  myservo.attach(9);

  pinMode(encoderPin1, INPUT);
  pinMode(encoderPin2, INPUT);

  digitalWrite(encoderPin1, HIGH); //turn pullup resistor on
  digitalWrite(encoderPin2, HIGH); //turn pullup resistor on

  //call updateEncoder() when any high/low changed seen
  //on interrupt 0 (pin 2), or interrupt 1 (pin 3)
  attachInterrupt(0, updateEncoder, CHANGE);
  attachInterrupt(1, updateEncoder, CHANGE);

}

void loop(){
 
if(servopos < 0) //Accounting for negative values
 
for(servopos > pos; servopos != pos; servopos -=1){ //when the postition of the servo is great than that of the rotary encoder
                                                     //the position of the servo will decrease by increments of 1 until they are equal.
  myservo.write(servopos * -1 + 90); //+90 because the center position of the rotary encoder is 0 but the center position of the
                                         //servo is 90
  delay(5);
}

for(servopos < pos; servopos != pos; servopos +=1){
  myservo.write(servopos * -1 + 90);
  delay(5);
}
;


if(servopos > 0) //Accounting for positive values
 
for(servopos > pos; servopos != pos; servopos -=1){
  myservo.write(servopos + 90);
  delay(5);
}

for(servopos < pos; servopos != pos; servopos +=1){
  myservo.write(servopos + 90);
  delay(5);
}
;


  Serial.println(pos);
  delay(1000); //just here to slow down the output, and show it will work  even during a delay
  Serial.println(servopos);
  delay(1000);
}


void updateEncoder(){
  int MSB = digitalRead(encoderPin1); //MSB = most significant bit
  int LSB = digitalRead(encoderPin2); //LSB = least significant bit

  int encoded = (MSB << 1) |LSB; //converting the 2 pin value to single number
  int sum  = (lastEncoded << 2) | encoded; //adding it to the previous encoded value

  if(sum == 0b1101 || sum == 0b0100 || sum == 0b0010 || sum == 0b1011) pos ++;
  if(sum == 0b1110 || sum == 0b0111 || sum == 0b0001 || sum == 0b1000) pos --;

  lastEncoded = encoded; //store this value for next time
 
}



Scotty: the variable pos is the position of the encoder.

nicoverduin: I am only multiplying by -1 when the value is negative. So either way I'm writing a positive value to the servo (I think, correct me if I'm wrong)

HeliBob: Before I try what you suggested could you take a look at this revised code and check if it would suffice please?

UKHeliBob

Take the comparisons between servopos and pos out of the for loops.  They are not doing what you think.

As I said in a previous post, what you need to do is to compare servopos and pos to determine which way the servo needs to move and then, separately from the comparison move the servo appropriately using a for loop.

Try this
Code: [Select]
if (servopos > pos)  //decide which way to move
  {
    for ( ; servopos >= pos; servopos--)  //servopos is already set so the initialisation is not necessary
    {
      myservo.write(servopos);
    }
  }

The first part of the for loop, the initialisation, is omitted because servopos already exists and has a value
Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

dukereduker

Ok here is the product so far after all of your wonderful help. Note that I just set the values of pos and servopos to 90 so I didn't keep having to write +90 after everything.

Code: [Select]
#include <Servo.h>
//these pins can not be changed 2/3 are special pins
int encoderPin1 = 2;
int encoderPin2 = 3;

Servo myservo;

int servopos = 90;

volatile int lastEncoded = 0;
volatile long pos = 90;

long lastencoderValue = 0;

int lastMSB = 0;
int lastLSB = 0;

void setup() {
  Serial.begin (9600);
 
  myservo.attach(9);

  pinMode(encoderPin1, INPUT);
  pinMode(encoderPin2, INPUT);

  digitalWrite(encoderPin1, HIGH); //turn pullup resistor on
  digitalWrite(encoderPin2, HIGH); //turn pullup resistor on

  //call updateEncoder() when any high/low changed seen
  //on interrupt 0 (pin 2), or interrupt 1 (pin 3)
  attachInterrupt(0, updateEncoder, CHANGE);
  attachInterrupt(1, updateEncoder, CHANGE);

}

void loop(){
 
if(pos < 0) //Accounting for negative values
 
if (servopos > pos)  //decide which way to move
  {
    for ( ; servopos >= pos; servopos--)  //servopos is already set so the initialisation is not necessary
    {
      myservo.write(servopos);
    }
  }

if (servopos < pos)  //decide which way to move
  {
    for ( ; servopos <= pos; servopos++)  //servopos is already set so the initialisation is not necessary
    {
      myservo.write(servopos);
    }
  };


if(pos > 0) //Accounting for positive values

if (servopos > pos)  //decide which way to move
  {
    for ( ; servopos >= pos; servopos--)  //servopos is already set so the initialisation is not necessary
    {
      myservo.write(servopos);
    }
  }

if (servopos < pos)  //decide which way to move
  {
    for ( ; servopos <= pos; servopos++)  //servopos is already set so the initialisation is not necessary
    {
      myservo.write(servopos);
    }
  }
;


  Serial.println("pos:");
  Serial.println(pos);
  Serial.println("servopos:");
  Serial.println(servopos);
  delay(2000);
}


void updateEncoder(){
  int MSB = digitalRead(encoderPin1); //MSB = most significant bit
  int LSB = digitalRead(encoderPin2); //LSB = least significant bit

  int encoded = (MSB << 1) |LSB; //converting the 2 pin value to single number
  int sum  = (lastEncoded << 2) | encoded; //adding it to the previous encoded value

  if(sum == 0b1101 || sum == 0b0100 || sum == 0b0010 || sum == 0b1011) pos ++;
  if(sum == 0b1110 || sum == 0b0111 || sum == 0b0001 || sum == 0b1000) pos --;

  lastEncoded = encoded; //store this value for next time
 
}

UKHeliBob

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

dukereduker

Yes it works very well! The only thing I have changed since posting that code is that I made the increments for the for loops and reading the encoder because one revolution for the encoder is 36 not 360.

Go Up