Trouble controlling servo with 'if' and 'for' statements for speed control

Hello all.

I been learning Arduino programming for a couple months now. Just Youtube videos and piecing together references, and I've seen a lot of success .

But I'm getting hung up on this problem:

I'm writing a sketch to open and close a set of blinds, and I want the servo to run slower to avoid loud and jerky movement.

I know I can control servo speeds with a 'for' count, and I know I can control the servo position with 3 push buttons assigned to 0, 90 and 180 degrees with 'if' statements.

But when I try to combine them, so that I can choose a position with a button AND have it move at a reduced speed, it doesn't seem to work.

I want to select one of 3 positions with push buttons, but I want the servo to move at a slower more controlled speed.

Here is the code I'm trying to make work:

#include <Servo.h>
Servo servoMotor1;
int pos=0;

const int button0=2;
const int button1=3;
const int button2=4;

int button0State;
int button1State;
int button2State;

int servoDelay=50;

void setup() {
Serial.begin(9600);
servoMotor1.attach(9);
pos=0;                      //bring servo to 0 position at reset
servoMotor1.write(pos);     
}
void loop() {
  
  button0State=digitalRead(button0);    //  button calling for 0 degree position
  button1State=digitalRead(button1);    //  button calling for 90 degree position
  button2State=digitalRead(button2);    //  buttin calling for 180 degree position
  
  int lastPos= servoMotor1.read();    //record the last postion written to servo 
 
  Serial.print("Button 1 =");         // serial printout for debugging purposes
  Serial.println(button0State);
  Serial.print("Button 2 =");
  Serial.println(button1State);
  Serial.print("Button 3 =");
  Serial.println(button2State);
  Serial.println("  ");
  Serial.println(lastPos);
  Serial.println("  ");

 /////////////////////////////////////////////////
                                          //  If a button is pressed & the servo
  if(button0State==HIGH && lastPos !=0){  //  isn't already at the angle that button
    if(lastPos <=95){                     //  would call for , it then checks to see if the
      for (pos=90; pos>=0; pos=pos-1) {   //  current position (actually the last position 
  servoMotor1.write(pos);                 //  written) is less than 95. 
  delay(servoDelay);                      //  Since there are only 3 possible positions 
      }                                   //  0, 90, and 180; then anything less than 95, but 
    }                                     //  NOT 0 has to be 90 and a request to move
                                          //  from 90 to 0 is made.
      else {                              //  Otherwise, it's 180 degrees and the 'else' 
        for (pos=180; pos>=0; pos=pos-1) {//  statment request a move from 180 to 0
  servoMotor1.write(pos);                 //  
  delay(servoDelay);                      //  All the buttons are setup in this manner
        }                                 //
     }                                    //
  }                                       //
 /////////////////////////////////////////////////
  
  if(button1State==HIGH && lastPos !=90){
    if(lastPos <=89){
      for (pos=0; pos>=90; pos=pos+1) {
  servoMotor1.write(pos);
  delay(servoDelay);
      }
    }
      else {
        for (pos=180; pos>=0; pos=pos-1) {
  servoMotor1.write(pos);
  delay(servoDelay);
        }
     }
  }
 /////////////////////////////////////////////////
  
if(button2State==HIGH && lastPos !=180){
    if(lastPos <=89){
      for (pos=0; pos>=180; pos=pos+1) {
  servoMotor1.write(pos);
  delay(servoDelay);
      }
    }
      else {
        for (pos=90; pos>=180; pos=pos+1) {
  servoMotor1.write(pos);
  delay(servoDelay);
        }
     }
  }
 
}

Unfortunately, this results in the servo moving to the assigned 0 degree position (as requested in setup) and remaining unresponsive from there.

I've confirmed my 'lastPos' and button assignments are accurate via the serial monitor and using them in different sketches that , for instance , don't include the 'for' count to slow the servo and simply designate a position and works as expected.

This doesn't return and errors when compiling and I can read though it step by step and I can't find the hole in it's logic.

Any assistance would be appreciated.
Thank you.

I ran the IDE's auto-format feature on your code to make it easier to read.

#include <Servo.h>
Servo servoMotor1;
int pos = 0;

const int button0 = 2;
const int button1 = 3;
const int button2 = 4;

int button0State;
int button1State;
int button2State;

int servoDelay = 50;

void setup() {
  Serial.begin(9600);
  servoMotor1.attach(9);
  pos = 0;                    //bring servo to 0 position at reset
  servoMotor1.write(pos);
}
void loop() {

  button0State = digitalRead(button0);  //  button calling for 0 degree position
  button1State = digitalRead(button1);  //  button calling for 90 degree position
  button2State = digitalRead(button2);  //  buttin calling for 180 degree position

  int lastPos = servoMotor1.read();   //record the last postion written to servo

  Serial.print("Button 1 =");         // serial printout for debugging purposes
  Serial.println(button0State);
  Serial.print("Button 2 =");
  Serial.println(button1State);
  Serial.print("Button 3 =");
  Serial.println(button2State);
  Serial.println("  ");
  Serial.println(lastPos);
  Serial.println("  ");

  /////////////////////////////////////////////////
  //  If a button is pressed & the servo
  if (button0State == HIGH && lastPos != 0) { //  isn't already at the angle that button
    if (lastPos <= 95) {                  //  would call for , it then checks to see if the
      for (pos = 90; pos >= 0; pos = pos - 1) { //  current position (actually the last position
        servoMotor1.write(pos);                 //  written) is less than 95.
        delay(servoDelay);                      //  Since there are only 3 possible positions
      }                                   //  0, 90, and 180; then anything less than 95, but
    }                                     //  NOT 0 has to be 90 and a request to move
    //  from 90 to 0 is made.
    else {                              //  Otherwise, it's 180 degrees and the 'else'
      for (pos = 180; pos >= 0; pos = pos - 1) { //  statment request a move from 180 to 0
        servoMotor1.write(pos);                 //
        delay(servoDelay);                      //  All the buttons are setup in this manner
      }                                 //
    }                                    //
  }                                       //
  /////////////////////////////////////////////////

  if (button1State == HIGH && lastPos != 90) {
    if (lastPos <= 89) {
      for (pos = 0; pos >= 90; pos = pos + 1) {
        servoMotor1.write(pos);
        delay(servoDelay);
      }
    }
    else {
      for (pos = 180; pos >= 0; pos = pos - 1) {
        servoMotor1.write(pos);
        delay(servoDelay);
      }
    }
  }
  /////////////////////////////////////////////////

  if (button2State == HIGH && lastPos != 180) {
    if (lastPos <= 89) {
      for (pos = 0; pos >= 180; pos = pos + 1) {
        servoMotor1.write(pos);
        delay(servoDelay);
      }
    }
    else {
      for (pos = 90; pos >= 180; pos = pos + 1) {
        servoMotor1.write(pos);
        delay(servoDelay);
      }
    }
  }
}

I would totally re-organize the sketch to remove the for loops entirely. Use the blink-without-delay method to make a step towards the target once every 50 milliseconds.

#include <Servo.h>
Servo servoMotor1;
int posActual = 0;
int posTarget = 0;

const int button0 = 2;
const int button1 = 3;
const int button2 = 4;

int button0State;
int button1State;
int button2State;

unsigned long servoDelay = 50; //milliseconds - sets the speed of movement

void setup() {
  Serial.begin(9600);
  servoMotor1.attach(9);
  moveServoToTarget();
}

void loop() {

  readButtons();

  printButtons();

  calculateTargetPosition();

  moveServoToTarget();
  
}

void readButtons() {

  button0State = digitalRead(button0);  //  button calling for 0 degree position
  button1State = digitalRead(button1);  //  button calling for 90 degree position
  button2State = digitalRead(button2);  //  buttin calling for 180 degree position

}

void printButtons() {

  Serial.print("Button 1 =");         // serial printout for debugging purposes
  Serial.println(button0State);
  Serial.print("Button 2 =");
  Serial.println(button1State);
  Serial.print("Button 3 =");
  Serial.println(button2State);
  Serial.println("  ");
  Serial.println(posActual);
  Serial.println("  ");

}

void calculateTargetPosition() {
  /////////////////////////////////////////////////
  //  If a button is pressed, change the target position we're moving towards
  //  Question: what happens if multiple buttons are held down?
  if (button0State == HIGH ) { 
    posTarget = 0;
  }                                       

  if (button1State == HIGH) {
    posTarget = 90;
  }

  if (button2State == HIGH) {
    posTarget = 180;
  }
}

void moveServoToTarget() {
  /////////////////////////////////////////////////
  //  Now move the servo towards the target
  //  But don't do this on every loop - do it at a defined interval
  static unsigned long lastServoMove;
  if(millis()-lastServoMove > servoDelay) {
    if(posTarget < posActual) posActual--; //only move by one degree increments
    if(posTarget > posActual posActual++;
    servoMotor1.write(posActual); 
    lastServoMove = millis(); //record when we moved
  }
}

I also split the code into separate functions which do separate tasks.

 for (pos = 0; pos >= 90; pos = pos + 1) {

When you initialize 'pos' to zero, the 'pos >= 90' will immediately fail and no movement will happen. I think you want 'pos <= 90'.

I'm naturally lazy so I'd just use VarSpeedServo.h instead of Servo.h and let that do all the slowing down for me.

See GitHub - netlabtoolkit/VarSpeedServo: Arduino library for servos that extends the standard servo.h library with the ability to set speed, and wait for position to complete

Steve

MorganS:
I would totally re-organize the sketch to remove the for loops entirely. Use the blink-without-delay method to make a step towards the target once every 50 milliseconds.
....
I also split the code into separate functions which do separate tasks.

Thanks you very much. Not only does this resolve my problem, I look forward to dissecting the code to further my own understanding . As I said, I've only been at this for a few weeks and I possess no prior experience with coding , so I can understand if my code appeared primitive.

Also, thank you to everyone else who contributed to the thread. This was very helpful.

Just to satisfy my curiosity and help me better understand the sketch coding; can anyone help me understand where the disconnect was happening within my code (other than the < 90 typo) . I'm probably not going to get much better at this if I don't learn to debug my own code.

Was I just too Inceptionny with the statements?

I don't have any ego in this, just want to learn to identify the cracks in my own work.

THANK AGAIN EVERYONE!

johnwasser:

 for (pos = 0; pos >= 90; pos = pos + 1) {

When you initialize 'pos' to zero, the 'pos >= 90' will immediately fail and no movement will happen. I think you want 'pos <= 90'.

NVM, this was it all along.

I made this correction throughout (not just on the line quoted), and my original code works as expected.

Thank you very much johnwasser!