Trouble stopping a servo loop. Advice requested.

Hello all. I will try to keep this concise.

There was a project I was working on that involved cannibalizing another product for my needs. My friend said I could do more, and better building it myself. He bought me an arduino kit to begin practicing. I've never done electronics or any programming before so this is all new to me and I've finally hit a wall that I haven't been able to get over.

I've tried finding various examples online etc and can't seem to find anything that applies to my specific circumstances that is solving my issue.

The goal:

  1. Have two servos run a loop going from point a, to b, to c with a delay between each move.
  2. Have a button I can press that will interrupt the a/b/c/ loop, move the servos to a specific point and hold them there until the button is released whereby they go back to the a/b/c loop.
  3. Set up 3 seperate IR range/motion sensors that will automatically interrupt the a/b/c loop and again move the servos to a specific point and hold them there until they are no longer receiving input.

I built the loop to make the servos follow the a/b/c loop (though the delay is shorter than in the final version and one of the steps is blocked out for testing purposes).

I have the button coding in and it works... sort of. When I press the button it will instantly move both servos to 90 degrees, however if the a/b/c loop is on the first step, it will finish the loop moving to the second step (and obviously the third if it wasn't blocked out) where it will then return to 90 degrees and hold it until I release the button where it will return to the a/b/c cycle.

No matter how I change the interrupt and coding it always behaves that way. I can't seem to figure out how to get it to just 'stop' the a/b/c loops, move the servos to 90 degrees and hold them there until I release the button without it finishing the whole a/b/c loop.

I'm assuming I will run into the same issue once I get the ir's into the code as well.

Any brighter minds out there see what is wrong with my code?

This is the most current version I'm working on:

#include <Servo.h>

const int buttonPin = 2; // pushbutton pin
Servo servoL;             // create servo object
Servo servoR;             // create servo object

int buttonState = 0;      // variable for reading the pushbutton states
int eardly = 1000;        // use for setting delay
volatile int pos;
volatile int state = LOW;

void setup() {
  Serial.begin(9600);
  Serial.println("DualServo_Button_Interrupt4_break.ino");
  pinMode(buttonPin, INPUT); // initialize the pushbutton pin as an input
  pinMode(0, INPUT_PULLUP);
  servoL.attach(6);  // define the servo pin
  servoR.attach(9);  // define the servo pin
  attachInterrupt(0, button, FALLING);
}

void loop()
{
  buttonpress();
}

void button()
{ 
  servoL.write(90);   // tell left servo to turn to 90 degrees
  servoR.write(90);   // tell right servo to turn to 90 degrees
}

void earauto(){
  servoR.write(0);  // tell right servo to turn to 0 degrees
  servoL.write(270);  // tell left servo to turn to 270 degrees
  delay (eardly);       // delay x seconds
  servoR.write(270);  // tell right servo to turn to 270 degrees
  servoL.write(0);  // tell left servo to turn to 0 degrees
  delay (eardly);       // delay x seconds
  // servoR.write(90);  // tell right servo to turn to 90 degrees
  // servoL.write(90);   // tell left servo to turn to 90 degrees
  // delay(eardly);        // delay x seconds
}
void buttonpress(){
  buttonState = digitalRead(buttonPin);  // read button state
  while (buttonState == HIGH)           // button state is up
  {
    earauto();    // servo loop for auto-movement
    break;
  }
  buttonState = digitalRead(buttonPin);  // read button state
  if (buttonState == LOW)
{      
  servoL.write(90);   // tell left servo to turn to 90 degrees
  servoR.write(90);   // tell right servo to turn to 90 degrees
}
}

Any advice or feedback would be greatly appreciated.

Thank you.

Hi,
Without reading to deeply into your code, you have a delay() function. This will block the controller from reading inputs and writing to outputs while it is running.
So delay(1000); will give you a 1Sec delay, but also basically stop your code for 1Sec.

Look in the IDE examples for Blink without Delay.
It will show how to get around this blocking.

Thanks.. Tom.. :slight_smile:

Okay thank you, that helps a little. Now I need to figure out how to actually use millis and make them work with servo's as the blink without delay doesn't seem to translate well to making servo's function, or follow an a/b/c loop.

The funny thing is I could 'cheat' by just increasing the delay time, since the servos will only be moving once every 2 minutes or so on the final product, but I don't want to cheat. I want to learn how to do this right but man, as someone who's never programmed before in any language, this is making my gray matter throb uncomfortably and my eyes cross lol.

Anyway this is the current mess trying with the millis approach:

#include <Servo.h>

const int buttonPin = 2;           // pushbutton pin
Servo servoL;                      // create servo object
Servo servoR;                      // create servo object
unsigned long previousMillis = 0;  //store last time servo updated
unsigned long interval = 1000;        //interval at which to move
int buttonState = 0;               // variable for reading the pushbutton states
volatile int pos;
volatile int state = LOW;

void setup() {
  Serial.begin(9600);
  Serial.println("DualServoMillisTEST.ino");
  pinMode(buttonPin, INPUT); // initialize the pushbutton pin as an input
  pinMode(0, INPUT_PULLUP);
  servoL.attach(6);  // define the servo pin
  servoR.attach(9);  // define the servo pin
  attachInterrupt(0, button, FALLING);
}

void loop()
{
  buttonpress();
}

void button()
{ 
  servoL.write(90);   // tell left servo to turn to 90 degrees
  servoR.write(90);   // tell right servo to turn to 90 degrees
}

void earauto(){
  unsigned long currentMillis = millis();
  if (currentMillis - previousMillis >= interval){
    previousMillis = currentMillis;
  servoR.write(0);  // tell right servo to turn to 0 degrees
  servoL.write(270);  // tell left servo to turn to 270 degrees
  if (currentMillis - previousMillis >= interval){
    previousMillis = currentMillis;
  servoR.write(270);  // tell right servo to turn to 270 degrees
  servoL.write(0);  // tell left servo to turn to 0 degrees
  }
  }
  // servoR.write(90);  // tell right servo to turn to 90 degrees
  // servoL.write(90);   // tell left servo to turn to 90 degrees
  // delay(eardly);        // delay x seconds
}
void buttonpress(){
  buttonState = digitalRead(buttonPin);  // read button state
  while (buttonState == HIGH)           // button state is up
  {
    earauto();    // servo loop for auto-movement
    break;
  }
  buttonState = digitalRead(buttonPin);  // read button state
  if (buttonState == LOW)
{      
  servoL.write(90);   // tell left servo to turn to 90 degrees
  servoR.write(90);   // tell right servo to turn to 90 degrees
}
}

They just sit there not moving 'twitching' angrily.

Maybe the demo Several Things at a Time or Planning and Implementing a Program will help.

In both of them there are servos and buttons and millis()

...R

pinMode(buttonPin, INPUT); // initialize the pushbutton pin as an input

Did you mean:

pinMode(buttonPin, INPUT_PULLUP); // initialize the pushbutton pin as an input

?

pinMode(0, INPUT_PULLUP);

Pin 0 is the Serial RX pin.

So I've got it, sort of. I read some of the links provided (thanks all) and did a few google searches and found some things that have helped me recode.

To test I only used one servo to start, and it's working almost exactly how I want. It goes to 270, then 90, then 0 degrees. Any time I press the button, it stops whatever it's doing, goes to 90 degrees and holds until I release the button.

I still need to integrate the right servo and see if I can get them both working together and in sync, but for now I was wondering if someone could look at my sketch and help with 2 questions:

  1. When the servo gets to 270, it hangs for about 10 seconds before moving to 90, where it hangs for 1, then to 0 where it hangs for 1 before going back to 270 where it again hangs for 10. This is my first time using millis so I'm not sure where I went wrong with that particular part of it.

  2. I notice I can still hear the servo 'running' while it's hanging. This wasn't happening with the delay. It would move, go quiet until the delay was over, move, rinse and repeat. This makes me concerned about battery longevity as in addition to two running servors, I'm going to have 3 ir emitters and 3 receivers constantly running. Any advice?

Thanks all for the help so far, it's gone a long way and I really appreciate it.

#include <Servo.h>
const int buttonPin = 2;           // pushbutton pin
Servo servoL;                      // create servo object
int stateL;
Servo servoR;                      // create servo object
int stateR;
unsigned long lastTime;  //store last time servo updated
int buttonState = 0;               // variable for reading the pushbutton states
volatile int pos;
volatile int state = LOW;

void setup() {
  Serial.begin(9600);
  Serial.println("DualButtonMillis.ino");
  pinMode(buttonPin, INPUT_PULLUP); // initialize the pushbutton pin as an input
  pinMode(2, INPUT_PULLUP);
  servoL.attach(6);  // define the servo pin
  servoR.attach(9);  // define the servo pin
  attachInterrupt(0, buttonpress, FALLING);
}

void buttonpress() {
  buttonState = digitalRead(buttonPin);  // read button state
  while (buttonState == HIGH)           // button state is up
  {
    earauto();    // servo loop for auto-movement
    break;
  }
  buttonState = digitalRead(buttonPin);  // read button state
  if (buttonState == LOW)
  {
    servoL.write(90);   // tell left servo to turn to 90 degrees
    servoR.write(90);   // tell right servo to turn to 90 degrees
  }
}

void loop()
{
  buttonpress();
}

void earauto() {
  switch (stateL) {
    case 0:
      //for (pos = 0; pos < 270; pos += 1)
      if (pos >= 270) {
        stateL = 1;
        lastTime = millis();
        break;
      }
      //  delay(30);
      if (millis() - lastTime >= 30) {
        servoL.write(pos++);  // pos += 1)
        lastTime += 30;
      }
      break;

    case 1:
      // delay(1000);
      if (millis() - lastTime >= 1000) {
        stateL = 2;
        lastTime = millis();
        pos = 270;
      }
      break;

    case 2:
      // for (pos = 270; pos >= 90; pos -= 1)
      if (pos < 90) {
        stateL = 3;
        lastTime = millis();
        break;
      }
      //  delay(1000);
      if (millis() - lastTime >= 1000) {
        servoL.write(pos--);
        lastTime += 30;
      }
      break;

    case 3:
    // for (pos = 90; pos >= 0; pos -= 1)
    if (pos < 0) {
      stateL = 4;
      lastTime = millis();
      break;
    }
      // delay(1000);
      if (millis() - lastTime >= 1000) {
        servoL.write(pos--);
        lastTime += 30;
      }
      break;

    case 4:
      // delay(1000);
      if (millis() - lastTime >= 1000) {
        stateL = 0;
        lastTime = millis();
        pos = 0;
      }
      break;
  }
}

You need to dispense with the interrupt handler for reading the switch. There is nothing about a human pressing the switch that needs nanosecond response times.

You are doing far too much in your interrupt handler.

  while (buttonState == HIGH)           // button state is up
  {
    earauto();    // servo loop for auto-movement
    break;
  }

It is rather stupid to have a while loop that will only iterate once at a maximum.

void loop()
{
  buttonpress();
}

Why are you calling the interrupt handler?

Get rid of those delays in the function called from the interrupt handler. delay() has NO place in in an interrupt service routine EVEN AS A COMMENT!

That seems to be an awfully complicated way to do something simple. What about something like this

void loop() {
   readButton();
   updatePosition()
   moveServo();
}

void readButton() {
   // leave this for homework
   // sets or clears buttonPressed
}

void updatePosition() {
   if (buttonPressed == true) {
       if (millis - prevUpdateMillis >= updateInterval) {
           prevUpdateMillis += updateInterval;
           curPos += servoStepSize;
           if (curPos > maxPos) {
               curPos = maxPos;
           }
       }
}

void moveServo() {
   myServo.write(curPos);
}

...R

Hey guys,

I appreciate all the feedback you've given so far, it's helped me clean up my code and get some things working a lot better, but I'm still having a timing issue.

I want the servo to hit 3 spots, pause for X second (x=the same at each spot), then move to the next location. The final pause will be much longer but for now I've got it set to 1000 millis for testing purposes.

I want it to go from 270degrees pause for 1 second, move to 90degrees pause for 1 second move to 0degrees pause for one second then back to 270 and repeat the whole cycle.

Currently when it hits 90, it pauses properly, 0, pauses properly but at the 270degree mark it pauses for 10 seconds and I cannot figure out why.

Anyone who is more experienced using millis for timing mind looking this over and telling me what looks wrong or what I can change?

Thank you.

void earauto() {
  switch (stateL) {
    case 0:
      //for (pos = 0; pos < 270; pos += 1)
      if (pos >= 270) {
        stateL = 1;
        lastTime = millis();
        break;
      }
      
      if (millis() - lastTime >= 30) {
        servoL.write(pos++);  // pos += 1)
        lastTime += 30;
      }
      break;

    case 1:
      
      if (millis() - lastTime >= 1000) {
        stateL = 2;
        lastTime = millis();
        pos = 270;
      }
      break;

    case 2:
      // for (pos = 270; pos >= 90; pos -= 1)
      if (pos < 90) {
        stateL = 3;
        lastTime = millis();
        break;
      }
      
      if (millis() - lastTime >= 1000) {
        servoL.write(pos--);
        lastTime += 30;
      }
      break;

    case 3:
    // for (pos = 90; pos >= 0; pos -= 1)
    if (pos < 0) {
      stateL = 0;
      lastTime = millis();
      break;
    }
      
      if (millis() - lastTime >= 1000) {
        servoL.write(pos--);
        lastTime += 30;
      }
      break;

  }
}

Currently when it hits 90, it pauses properly, 0, pauses properly but at the 270degree mark it pauses for 10 seconds and I cannot figure out why.

There are probably clues there. Without seeing the complete code, we would have to guess at variable types, etc. I hate guessing games.

Oh sorry about that. Anyway here's the current codes.

Let me explain the final goal because I don't think I've been entirely clear and I apologize.

The final goal is to have 2 servo's that automatically move on their own on a timer but remain in sync.

ServoL: 270 (pause) 90 (pause) 0 (pause)
ServoR: 0 (pause) 90 (pause) 270 (pause)

So when L is at 270, I need R at 0, then both to move to 90 at the same time, then while L is moving to 0, R to 270.

I want to be able to interrupt this auto movement with various inputs.

A button press that will move both servos to 90 degrees and hold them there until the button is let go, and 3 ir range detectors the also move the servos to set position and hold them there until the object moves out of range.

So with the single servo, the code works fine except for the delay it hits at 270 degrees.

So here are two codes I'm working on, in the first code, it goes through one cycle then freezes.

#include <Servo.h>
const int buttonPin = 2;                // pushbutton pin
Servo servoL;                           // create LEFT servo object
int stateL;
Servo servoR;                           // create RIGHT servo object
int stateR;
unsigned long lastTime;                 // store last time servo updated
int buttonState = 0;                    // variable for reading pushbotton states
int pos = 0;                           // set LEFT servo position
int pos2 = 0;                           // set RIGHT servo position

void setup() {
  Serial.begin(9600);
  Serial.println("DualButtonMillis2.ino");
  pinMode(buttonPin, INPUT);            // initialize the pushbutton as an input
  servoL.attach(6);                     // define LEFT servo pin
  servoR.attach(9);                     // define RIGHT servo pin
}

void buttonpress() {
  buttonState = digitalRead(buttonPin); // read button state
  while (buttonState == HIGH)           // button state is down
  {
    earauto();
    break;
  }
  buttonState = digitalRead(buttonPin); // read button state
  if (buttonState == LOW)
  {
    servoL.write(90);                   // move LEFT servo to 90 degrees
    servoR.write(90);                   // move RIGHT servo to 90 degrees
  }
}

void earauto() {
  switch (stateL);
  switch (stateR) {
    case 0:
      // for (pos = 0; pos < 270; pos += 1)
      if (pos >= 270);
      // for (pos2 = 0; pos2 < 0; pos2 -=1)
      if (pos2 <= 0)  {
        stateL = 1;
        stateR = 1;
        lastTime = millis();
        break;
      }
      
      if (millis() - lastTime >= 30) {
        servoL.write(pos++);  // pos += 1
        servoR.write(pos--);  // pos -= 1
        lastTime += 30;
      }
      break;

    case 1:
     
      if (millis() - lastTime >= 1000) {
        stateL = 2;
        stateR = 2;
        lastTime = millis();
        pos = 270;
        pos2 = 0;
      }
      break;

    case 2:
      // for (pos = 270; pos >= 90; pos -= 1) for (pos2 = 0; pos <= 90; pos += 1)
      if (pos < 90);
      if (pos2 > 90) {
        stateL = 3;
        stateR = 3;
        lastTime = millis();
        break;
      }
      
      if (millis() - lastTime >= 1000) {
        servoL.write(pos--);
        servoR.write(pos2++);
        lastTime += 30;
      }
      break;

    case 3:
    // for (pos = 90; pos >= 0; pos -= 1) for (pos2 = 90; pos2 <= 270; pos += 1)
    if (pos < 0);
    if (pos2 >270) {
      stateL = 0;
      stateR = 0;
      lastTime = millis();
      break;
    }
      
      if (millis() - lastTime >= 1000) {
        servoL.write(pos--);
        servoR.write(pos2++);
        lastTime += 30;
      }
      break;
  }
}

void loop()
{
  buttonpress();
}

Here's a second code I have where they don't freeze and continue to cycle, but due to the delay issues I cannot tell if they are moving in sync or not.

#include <Servo.h>
const int buttonPin = 2;                // pushbutton pin
Servo servoL;                           // create LEFT servo object
int stateL;
Servo servoR;                           // create RIGHT servo object
int stateR;
unsigned long lastTime;                 // store last time servo updated
int buttonState = 0;                    // variable for reading pushbotton states
int pos = 0;                           // set LEFT servo position
int pos2 = 0;                           // set RIGHT servo position

void setup() {
  Serial.begin(9600);
  Serial.println("DualButtonMillis2.ino");
  pinMode(buttonPin, INPUT);            // initialize the pushbutton as an input
  servoL.attach(6);                     // define LEFT servo pin
  servoR.attach(9);                     // define RIGHT servo pin
}

void buttonpress() {
  buttonState = digitalRead(buttonPin); // read button state
  while (buttonState == HIGH)           // button state is down
  {
    earauto();                          
    break;
  }
  buttonState = digitalRead(buttonPin); // read button state
  if (buttonState == LOW)
  {
    servoL.write(90);                   // move LEFT servo to 90 degrees
    servoR.write(90);                   // move RIGHT servo to 90 degrees
  }
}

void earauto() {
  switch (stateL) {
    case 0:
      //for (pos = 0; pos < 270; pos += 1)
      if (pos >= 270) {
        stateL = 1;
        lastTime = millis();
        break;
      }
      
      if (millis() - lastTime >= 30) {
        servoL.write(pos++);  // pos += 1)
        lastTime += 30;
      }
      break;

    case 1:
      
      if (millis() - lastTime >= 1000) {
        stateL = 2;
        lastTime = millis();
        pos = 270;
      }
      break;

    case 2:
      // for (pos = 270; pos >= 90; pos -= 1)
      if (pos < 90) {
        stateL = 3;
        lastTime = millis();
        break;
      }
      
      if (millis() - lastTime >= 1000) {
        servoL.write(pos--);
        lastTime += 30;
      }
      break;

    case 3:
    // for (pos = 90; pos >= 0; pos -= 1)
    if (pos < 0) {
      stateL = 0;
      lastTime = millis();
      break;
    }
      
      if (millis() - lastTime >= 1000) {
        servoL.write(pos--);
        lastTime += 30;
      }
      break;
  }
  
  switch (stateR) {
    case 0:
      if (pos2 <= 0) {
        stateR = 1;
        lastTime = millis();
        break;
      }
      if (millis() - lastTime >= 30) {
        servoR.write(pos2--);  // pos -= 1)
        lastTime += 30;
      }
      break;

    case 1:
      if (millis() - lastTime >= 1000) {
        stateR = 2;
        lastTime = millis();
        pos2 = 0;
      }
      break;

    case 2:
      // for (pos = 0; pos <= 90; pos += 1)
      if (pos2 > 90) {
        stateR = 3;
        lastTime = millis();
        break;
      }
      if (millis() - lastTime >= 1000) {
        servoR.write(pos2++);
        lastTime += 30;
      }
      break;

    case 3:
    // for (pos = 90; pos <= 270; pos += 1)
    if (pos2 < 270) {
      stateR = 0;
      lastTime = millis();
      break;
    }
      if (millis() - lastTime >= 1000) {
        servoR.write(pos2++);
        lastTime += 30;
      }
      break;
  }
}

void loop()
{
  buttonpress();
}

EDIT: I also realized that on (pos2++) movements, it's slow and stutter. I swapped servos to see if it was an issue with the servo and it's not. For some reason the (pos2++) is slow and suttery where the (pos2--) and all the movements on servoL are smooth.

ServoL: 270 (pause) 90 (pause) 0 (pause)Do your servos have a range of 0 to 270 degrees ?

UKHeliBob:

ServoL: 270 (pause) 90 (pause) 0 (pause)

Do your servos have a range of 0 to 270 degrees ?

Yes sir. Just to test I changed the 270 to 180 and delay issue is still there.

  while (buttonState == HIGH)           // button state is down
  {
    earauto();
    break;
  }

It is pointless to use a while statement and then force it to iterate at most once. A simple if statement is sufficient.

      if (pos >= 270);

Well, there's a useful bit of code. NOT!

Your first but of code calls earauto() many many times, as long as the switch is pressed.

You REALLY need to look at the state change detection example. Set a properly named variable to true when the switch BECOMES pressed.

When the properly named variable is true, call earauto() over and over. When earauto() finally gets the servo to the last position, it should set the properly named variable's value to false, so loop() stops calling earauto().

EDIT: I also realized that on (pos2++) movements, it's slow and stutter. I swapped servos to see if it was an issue with the servo and it's not. For some reason the (pos2++) is slow and suttery where the (pos2--) and all the movements on servoL are smooth.

Is that true for both servos?

Just to test I changed the 270 to 180 and delay issue is still there.

I was not suggesting that your problem was caused by trying to move the servo to 270 degrees. As a matter of interest, what type of servos do you have and how are they powered ?

      if (millis() - lastTime >= 1000) {
        servoR.write(pos2++);
        lastTime += 30;

Why test whether 1000 milliseconds has passed then add 30 to the previous time ?

Elsewhere in the program you have

     if (millis() - lastTime >= 30) {
        servoL.write(pos++);  // pos += 1)
        lastTime += 30;

Your code would be much more readable and easy to maintain if you used well named constants rather than magic numbers.

PaulS:
Is that true for both servos?

No. Either servo connected to servoL works fine. Either servo as servoR stutters.

UKHeliBob:
I was not suggesting that your problem was caused by trying to move the servo to 270 degrees. As a matter of interest, what type of servos do you have and how are they powered ?

Not sure what kind they are. Just a couple servos I had from the original project I was going to cannibalize and just using them for now to test the code and see if I can get it working as intended. They're powered by the Arduino board.

They're powered by the Arduino board.

Well, now we know what the problem is. The Arduino can not power servos.

Hi,

Can you please post a copy of your circuit, in CAD or a picture of a hand drawn circuit in jpg, png?
Including how you are powering your servos, a picture of the servos would be appreciated, or part numbers.

Thanks... Tom.. :slight_smile: