Problems stopping an if loop, and starting it as well

Question

I want my code to run my motor for a specific amount of time at a specific speed and then stop and not do anything else. I'm having trouble here, I think with my switch and the if loops running on repeat. What is a good way for me to stop the code from looping? And any helpful hints on my switches will be appreciated as well? Thanks and I hope you had a wonderful thanksgiving if you're American. If you're not, I hope you just had a great day in general.

#include <Servo.h>
#include <Adafruit_MotorShield.h>
#include <Wire.h>

Adafruit_MotorShield AFMS = Adafruit_MotorShield();
Adafruit_DCMotor *DC1 = AFMS.getMotor(1);
Adafruit_DCMotor *DC2 = AFMS.getMotor(2);
Adafruit_DCMotor *DC3 = AFMS.getMotor(3);
Adafruit_DCMotor *DC4 = AFMS.getMotor(4);

Servo Servo1;

void setup() {
  digitalWrite(A0, HIGH); //Turns on the pullup resistor for A0
  AFMS.begin(); //Initializes the motorshield board
  int motorPin1 = 1;
  int motorPin2 = 2;
  int motorPin3 = 3;
  int motorPin4 = 4;
  pinMode(A0, INPUT); //Sets the Analog Pin 0 as an input
  digitalWrite(A0, HIGH);
  pinMode(A1, INPUT);
  digitalWrite(A1, HIGH);
  pinMode(A1, INPUT);
  pinMode(1, OUTPUT);
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  Servo1.attach(9);
}

void loop()  {
  int MAXampDrawTime = 1000;
  int MAXampDrawSpeed = 20;
  int NORMampDrawTime = 1500;
  int NORMampDrawSpeed = 10;
  int DrawTime = MAXampDrawTime + NORMampDrawTime;
  int ServoTime = 450;
  int previousMillis = 0;
  int inPin0 = LOW;
  int inPin1 = LOW;
  inPin0 = digitalRead(A0);
  inPin1 = digitalRead(A1);
  unsigned long time_zero = millis();
  unsigned long current_time = millis();

  if (inPin0 == HIGH) {
    time_zero = millis();
    current_time = millis();
    while (millis() - time_zero < MAXampDrawTime); {
      dcMoveConst(MAXampDrawSpeed, 1);
      dcMoveConst(MAXampDrawSpeed, 2);
      dcMoveConst(MAXampDrawSpeed, 3);
      dcMoveConst(MAXampDrawSpeed, 4);
    }
    while (millis() - time_zero < NORMampDrawTime); {
      dcMoveConst(NORMampDrawSpeed, 1);
      dcMoveConst(NORMampDrawSpeed, 2);
      dcMoveConst(NORMampDrawSpeed, 3);
      dcMoveConst(NORMampDrawSpeed, 4);
    }
    if (millis() - time_zero > DrawTime) {
      dcMoveConst(0, 1);
      dcMoveConst(0, 2);
      dcMoveConst(0, 3);
      dcMoveConst(0, 4);
    }

    if (millis() - time_zero >= ServoTime); {
      Servo1.write(0);
      delay(400);
      Servo1.write(90);
    }
  }

  if (inPin1 == HIGH) {
    time_zero = millis();
    current_time = millis();
    MAXampDrawTime = 1000;
    MAXampDrawSpeed = 20;
    NORMampDrawTime = 1500;
    NORMampDrawSpeed = 10;
    DrawTime = MAXampDrawTime + NORMampDrawTime;
    while (millis() - time_zero < MAXampDrawTime); {
      dcMoveConst(MAXampDrawSpeed, 1);
      dcMoveConst(MAXampDrawSpeed, 2);
      dcMoveConst(MAXampDrawSpeed, 3);
      dcMoveConst(MAXampDrawSpeed, 4);
    }
    while (millis() - time_zero < NORMampDrawTime); {
      dcMoveConst(NORMampDrawSpeed, 1);
      dcMoveConst(NORMampDrawSpeed, 2);
      dcMoveConst(NORMampDrawSpeed, 3);
      dcMoveConst(NORMampDrawSpeed, 4);
    }
    if (millis() - time_zero > DrawTime) {
      dcMoveConst(0, 1);
      dcMoveConst(0, 2);
      dcMoveConst(0, 3);
      dcMoveConst(0, 4);
    }

    if (millis() - time_zero >= ServoTime); {
      Servo1.write(0);
      delay(400);
      Servo1.write(90);
    }
  }
}

Answer 1

Consider something like this edit of your code. It has two independent switch/case constructs to handle the flinging operation and the releasing operation as independent finite state machines, both subsystems configured as they switch out of the idle state based on the inPin0 or inPin1 status.

The trick of using fast iterations through the loops without delay() helps simulate parallel processing of finite state machines. With this scheme, one has excess processing time and more options. One could entangle the two processes if necessary, or add supervisory or monitoring processes that interact with the subsystems.

For tuning, you could make inPin0 be a canned best cycle, and make inPin1 read and use adjustable inputs.

// Not tested or run...
#include <Adafruit_MotorShield.h>
#include <Wire.h>

Adafruit_MotorShield AFMS = Adafruit_MotorShield();
Adafruit_DCMotor *DC1 = AFMS.getMotor(1);
Adafruit_DCMotor *DC2 = AFMS.getMotor(2);
Adafruit_DCMotor *DC3 = AFMS.getMotor(3);
Adafruit_DCMotor *DC4 = AFMS.getMotor(4);

Servo Servo1;

void setup() {
  digitalWrite(A0, HIGH); //Turns on the pullup resistor for A0
  AFMS.begin(); //Initializes the motorshield board

  int motorPin1 = 1;
  int motorPin2 = 2;
  int motorPin3 = 3;
  int motorPin4 = 4;

  pinMode(A0, INPUT); //Sets the Analog Pin 0 as an input
  digitalWrite(A0, HIGH);
  pinMode(A1, INPUT);
  digitalWrite(A1, HIGH);
  pinMode(A1, INPUT);
  pinMode(1, OUTPUT);
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  Servo1.attach(9);

  int inPin0 = LOW;
  int inPin1 = LOW;
  inPin0 = digitalRead(A0);
  inPin1 = digitalRead(A1);
}



void loop()  {
  static int flingState = 1; // initial state is 1, the idle state
  static int releaseState = 1; // initial state is 1, the idle state
  static unsigned long flingTime; // to store the current time in for delays
  static unsigned long releaseTime; // to store the current time in for delays

  int MAXampDrawTime;
  int MAXampDrawSpeed;
  int NORMampDrawTime;
  int NORMampDrawSpeed;
  int servoTime = 450;
  int inPin0 = LOW;
  int inPin1 = LOW;
  int DrawTime;
  inPin0 = digitalRead(A0);
  inPin1 = digitalRead(A1);

  switch (flingState) {  // manage flinging
    case 1:  // idle state
      if (inPin0 == HIGH) {  // config for maxthrow
         MAXampDrawTime = 500;
         MAXampDrawSpeed = 20;
         NORMampDrawTime = 250;
         NORMampDrawSpeed = 10;
         DrawTime = MAXampDrawTime + NORMampDrawTime;
         flingState = 2;
      }
      if (inPin1 == HIGH) {  // config for accThrow
        MAXampDrawTime = 1000;
        MAXampDrawSpeed = 20;
        NORMampDrawTime = 750;
        NORMampDrawSpeed = 10;
        DrawTime = MAXampDrawTime + NORMampDrawTime;
        flingState = 2;
      }
      break;

    case 2: // start fling oneshot
        flingTime = millis();
        dcMoveConst(MAXampDrawSpeed, 1);
        dcMoveConst(MAXampDrawSpeed, 2);
        dcMoveConst(MAXampDrawSpeed, 3);
        dcMoveConst(MAXampDrawSpeed, 4);
        flingState = 3;
        break;
    case 3: // maxFlinging timer
        if (millis() - flingTime > MAXampDrawTime) flingState =4;
        break;
    case 5: // initiate slower Flinging oneshot 
        dcMoveConst(NORMampDrawSpeed, 1);
        dcMoveConst(NORMampDrawSpeed, 2);
        dcMoveConst(NORMampDrawSpeed, 3);
        dcMoveConst(NORMampDrawSpeed, 4);
        flingState = 5;
        break;
    case 6: // drawing timer
        if (millis() - flingTime > MAXampDrawTime + NORMampDrawTime) 
            flingState = 7;
        break;
    case 7 : // s_turnoff:
      dcRelease(1);
      dcRelease(2);
      dcRelease(3);
      dcRelease(4);
      flingState = 1;
      break;

  } // end flingState Machine


  switch (releaseState) {  // independently manage the release servo
    case 1:  // idle
      if (inPin0 == HIGH) {servoTime=450; releaseState = 2;}
      if (inPin1 == HIGH) {servoTime=450; releaseState = 2;}
      break;
    case 2:  // start holding oneshot
      releaseTime = millis();
      releaseState =3;
      break;
    case 3: // holding timer
      if (millis() -releaseTime > servoTime) releaseState = 4;
      break;
    case 4: // releasing oneshot
        Servo1.write(0);
        releaseState = 5;
        break;
    case 5: //released timer
        if (millis() - releaseTime > servoTime+400) {
             Servo1.write(90);
             releaseState = 1; // back to idle
        }
        break;
    } // end releaseState machine
      // if (millis()%100 ==0) reportState(flingState,releaseState);  
  } // end loop     

Answer 2

There are two major flaws with your program - and both of them can be "fixed" by you understanding some concepts better.

Firstly, in your program, there is no concept of "When the button is pressed", only "While the button is pressed". This is caused by an error in your understanding of how reading of buttons needs to be done.

The digitalRead() function will return the current state of the button at that precise moment in time. It has no concept of "This has just been pressed", only "This is currently pressed". So you need to remember in what state the button was last time you looked at it, and only if it has changed do you do something to react to it. The same with the releasing of the button - at the moment it only knows that it isn't pressed, not that it was pressed but is no longer pressed.

The second problem is that of state, and is closely coupled with the fixing of the reading of the buttons. At the moment the state of your arduino (i.e., what it thinks it should be doing at the moment) is entirely defined by what buttons are currently being held in or not held in at any one time. Instead the buttons need to be used to set flags that tell the Arduino what it should be doing and when - such as a flag that says "You should be moving the servos". Such an arrangement is called a finite state machine and is one of the simplest ways of getting something small like an Arduino to do multiple things at once - in this case reading what is happening to the switches whilst at the same time making motors do different things.

For more information on finite state machines you might like to cast your eye over this: http://hacking.majenko.co.uk/finite-state-machine

Answer 3

@Majenko --- I love this new way of thinking about code!

So my question is - maybe it would do some good to know what my robot is doing, it's launching a fishing rod. It's turning the motor to fling the arm, and the servo will disengage the button in order to launch the lure. These processes are independent from each other... the servo might start working before the motor stops working, but both rely on the switch being pressed, and I won't know at which point the servo should release until testing starts.

Edit: I've realized that I've got the if qualifiers in the wrong place and I'm currently trying to fix that.

#include <Adafruit_MotorShield.h>

#include <Wire.h>



Adafruit_MotorShield AFMS = Adafruit_MotorShield();



Adafruit_DCMotor *DC1 = AFMS.getMotor(1);
Adafruit_DCMotor *DC2 = AFMS.getMotor(2);
Adafruit_DCMotor *DC3 = AFMS.getMotor(3);
Adafruit_DCMotor *DC4 = AFMS.getMotor(4);

Servo Servo1;



void setup() {
  digitalWrite(A0, HIGH); //Turns on the pullup resistor for A0
  AFMS.begin(); //Initializes the motorshield board

  int motorPin1 = 1;
  int motorPin2 = 2;
  int motorPin3 = 3;
  int motorPin4 = 4;




  pinMode(A0, INPUT); //Sets the Analog Pin 0 as an input
  digitalWrite(A0, HIGH);
  pinMode(A1, INPUT);
  digitalWrite(A1, HIGH);
  pinMode(A1, INPUT);
  pinMode(1, OUTPUT);
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  Servo1.attach(9);
#define s_idle 1
#define s_maxthrow 2
#define s_accthrow 3
#define s_waitservo 4
#define s_servogo 5
#define s_turnoff 6

  int inPin0 = LOW;
  int inPin1 = LOW;
  inPin0 = digitalRead(A0);
  inPin1 = digitalRead(A1);
}



void loop()  {
  static int state = 1; // initial state is 1, the idle state
  static unsigned long ts; // to store the current time in for delays
  int MAXampDrawTime;
  int MAXampDrawSpeed;
  int NORMampDrawTime;
  int NORMampDrawSpeed;
  int servoTime = 450;
  int inPin0 = LOW;
  int inPin1 = LOW;
  int DrawTime;
  inPin0 = digitalRead(A0);
  inPin1 = digitalRead(A1);

  switch (state) {
    case s_idle:
      //we don't deen to do anything here, waiting for a state
      break;

    case s_maxthrow:

      if (inPin0 == HIGH); {
        MAXampDrawTime = 500;
        MAXampDrawSpeed = 20;
        NORMampDrawTime = 250;
        NORMampDrawSpeed = 10;
        DrawTime = MAXampDrawTime + NORMampDrawTime;

        ts = millis();

        dcMoveConst(MAXampDrawSpeed, 1);
        dcMoveConst(MAXampDrawSpeed, 2);
        dcMoveConst(MAXampDrawSpeed, 3);
        dcMoveConst(MAXampDrawSpeed, 4);
        delay(MAXampDrawTime);
        dcMoveConst(NORMampDrawSpeed, 1);
        dcMoveConst(NORMampDrawSpeed, 2);
        dcMoveConst(NORMampDrawSpeed, 3);
        dcMoveConst(NORMampDrawSpeed, 4);
        delay(NORMampDrawTime);
        state = s_turnoff;

      }
    case s_turnoff:
      dcRelease(1);
      dcRelease(2);
      dcRelease(3);
      dcRelease(4);
      state = s_idle;
      break;

    case s_accthrow:
      if (inPin1 == HIGH); {
        MAXampDrawTime = 1000;
        MAXampDrawSpeed = 20;
        NORMampDrawTime = 750;
        NORMampDrawSpeed = 10;
        DrawTime = MAXampDrawTime + NORMampDrawTime;

        ts = millis();

        dcMoveConst(MAXampDrawSpeed, 1);
        dcMoveConst(MAXampDrawSpeed, 2);
        dcMoveConst(MAXampDrawSpeed, 3);
        dcMoveConst(MAXampDrawSpeed, 4);
        delay(MAXampDrawTime);
        dcMoveConst(NORMampDrawSpeed, 1);
        dcMoveConst(NORMampDrawSpeed, 2);
        dcMoveConst(NORMampDrawSpeed, 3);
        dcMoveConst(NORMampDrawSpeed, 4);
        delay(NORMampDrawTime);
        state = s_turnoff;
      }

    case s_servogo:
      if (millis() - ts > servoTime); {
        Servo1.write(0);
        delay(400);
        Servo1.write(90);
      }

  }



}