Output timing issue

Using Arduino Programming Questions
1

Hello, I'm just learning and struggling a bit with a programming issue. What I am trying to do is control a motor. Essentially I want the motor to run for 3 seconds, stop for 3 seconds in between limit switches. Once it reaches a limit switch, I want it to reverse and go in the other direction. I have the hardware part figured out. Just having a bit of trouble with the code. Here is what I have:

const int onButton = 7;
const int openLimit = 2;
const int closeLimit = 4;// the pin that the pushbutton is attached to
const int motorOpen = 8;
const int motorClose = 13;// the pin that the LED is attached to

// Variables will change:

int openLimitState;
int lastOpenLimitState;// current state of the button
int closeLimitState;
int lastCloseLimitState;
int onButtonState;

void setup() {
// initialize the button pin as a input:
pinMode(openLimit, INPUT);
// initialize the LED as an output:
pinMode(closeLimit, INPUT);
pinMode(motorOpen, OUTPUT);
pinMode(motorClose, OUTPUT);
pinMode(onButton, INPUT);

}

void loop() {
// read the pushbutton input
onButtonState = digitalRead(onButton);

if (onButtonState == HIGH) {

openLimitState = digitalRead(openLimit);
closeLimitState = digitalRead(closeLimit);

// compare the buttonState to its previous state
if (openLimitState == HIGH && closeLimitState == HIGH) {
digitalWrite(motorOpen, LOW);
digitalWrite(motorClose, LOW);
}

else if (openLimitState == HIGH) {
// if the current state is HIGH then the button
// wend from off to on:
motorRunClose();
}

else if (closeLimitState == HIGH) {
motorRunOpen();
}
else {
motorRunClose();

}
}
else if (onButtonState == LOW) {
digitalWrite (motorOpen, LOW);
digitalWrite (motorOpen, LOW);
}
}

void motorRunOpen() {
while (openLimitState == LOW) {
digitalWrite (motorOpen, HIGH);
delay (3000);
digitalWrite (motorOpen, HIGH);
delay (3000);
digitalWrite (motorOpen, HIGH);
delay (3000);
}
}

void motorRunClose() {
while (closeLimitState == LOW) {
digitalWrite (motorClose, HIGH);
delay (3000);
digitalWrite (motorClose, HIGH);
delay (3000);
digitalWrite (motorClose, HIGH);
delay (3000);
}
}

Essentially what happens is if the on button attached to input 7 is high, the 13 output just stays on all the time and ignores any other input. What am I doing wrong?

Thanks in advance for any guidance!

2 
const int closeLimit = 4;// the pin that the pushbutton is attached to
const int motorClose = 13;// the pin that the LED is attached to
int lastOpenLimitState;// current state of the button

  // initialize the button pin as a input:
  pinMode(openLimit, INPUT);
  // initialize the LED as an output:
  pinMode(closeLimit, INPUT);


  // read the pushbutton input                                                                                                                                                                                                                                                             
  onButtonState = digitalRead(onButton);

Read the stickies
Code tags
Your comments should match reality...

3

And to answer the question, drop the delay()'s. In a delay() the Arduino does nothing which includes checking the limit switches.... Have a look at Blink without delay :wink:

4

You are using too many "if conditional statements" that are interlocking each others. With different condition in these "if statements", you should use them separately for easy to troubleshoot/trace faults.

Good luck and have fun.

5 
if (onButtonState == HIGH) {

Will only be true as long as the button is held down.

 // compare the buttonState to its previous state
  if (openLimitState == HIGH && closeLimitState == HIGH) {
     digitalWrite(motorOpen, LOW);
     digitalWrite(motorClose, LOW);
   }

Only checks if the current state is HIGH, no comparison with previous state

Your define these:

int lastOpenLimitState;// current state of the button
int lastCloseLimitState;

but never use them.

You write a HIGH to motorOpen, wait 3 seconds, write HIGH to the already HIGH motorOpen, wait 3 seconds then again write HIGH to the already HIGH motorOpen

void motorRunOpen() {
  while (openLimitState == LOW) {
  digitalWrite (motorOpen, HIGH);
  delay (3000);
  digitalWrite (motorOpen, HIGH);
  delay (3000);
  digitalWrite (motorOpen, HIGH);
  delay (3000);
  }
}
6

And once again, the boilerplate question when I see "pinMode(pin,INPUT) then,
"if(digitalRead(pin) == HIGH)".

Do you have a pulldown resistor connected from GND to the input pin? :slight_smile:

7

Thanks for the help all. So I've tried to break this down in pieces. Hardware is all good. I do have a resistor from ground to the input pin so floating.

I've gotten rid of the delay() function and am trying the whole state machine thing. Just using LEDs for now to get the program figured out.

Why would this not work?

int ledPin = 13; // the number of the LED pin
int limitPin = 4; // the number of the limit switch pin
int onButton = 7; // the number of the on button pin
int ledState = LOW; // ledState used to set the LED
int onButtonState = LOW; // used to set the state of the on button pin
int previousOnButtonState = LOW;
unsigned long previousMillis = 0; // will store last time LED was updated
long OnTime = 3050; // milliseconds of on-time
long OffTime = 1250; // milliseconds of off-time
int closeLimitSwitchStatus = 0; // used to set the state of the close limit switch

void setup()
{
// set the digital pin as output:
pinMode(ledPin, OUTPUT);
pinMode(limitPin, INPUT);
pinMode(onButton, INPUT);
}

void loop() {
// if the on button is high, then run the motorRunClose function.
if (onButtonState == HIGH) {
onButtonState = previousOnButtonState;
motorRunClose();
}
}

void motorRunClose() {

//check the status of the close limit input and start timing.
closeLimitSwitchStatus = digitalRead(limitPin);
unsigned long currentMillis = millis();

// if close limit input is not high and the timer is less than the OnTime variable, turn led on.
if ((closeLimitSwitchStatus =! HIGH) && (currentMillis - previousMillis <= OnTime)) {
ledState = HIGH;
previousMillis = currentMillis;
digitalWrite(ledPin, ledState);
}
// if the close limit input is high and the timer is less than the OffTime variable, turn led off.
if ((closeLimitSwitchStatus == HIGH) && (currentMillis - previousMillis <= OffTime)) {
ledState = LOW;
previousMillis = currentMillis;
digitalWrite(ledPin, ledState);
}
}

8

Now place it in code tags please. Like asked before :wink:

9

Sorry I wasn't sure what that meant but I think I know now. Here we go! :wink:

Thanks for the help all. So I've tried to break this down in pieces. Hardware is all good. I do have a resistor from ground to the input pin so floating.

I've gotten rid of the delay() function and am trying the whole state machine thing. Just using LEDs for now to get the program figured out.

Why would this not work?

int ledPin =  13;      // the number of the LED pin
int limitPin = 4;     // the number of the limit switch pin
int onButton = 7;    // the number of the on button pin
int ledState = LOW;  // ledState used to set the LED
int onButtonState = LOW; // used to set the state of the on button pin
int previousOnButtonState = LOW;
unsigned long previousMillis = 0;        // will store last time  LED was updated
long OnTime = 3050;           // milliseconds of on-time
long OffTime = 1250;          // milliseconds of off-time
int closeLimitSwitchStatus = 0; // used to set the state of the close limit switch

void setup() 
{ 
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);   
  pinMode(limitPin, INPUT);    
  pinMode(onButton, INPUT);
}

void loop() {
  // if the on button is high, then run the motorRunClose function.
 if (onButtonState == HIGH) {
  onButtonState = previousOnButtonState;
  motorRunClose();
 }
}

void motorRunClose() {

  //check the status of the close limit input and start timing.
  closeLimitSwitchStatus = digitalRead(limitPin);
  unsigned long currentMillis = millis();

  
// if close limit input is not high and the timer is less than the OnTime variable, turn led on.
  if ((closeLimitSwitchStatus =! HIGH) && (currentMillis - previousMillis <= OnTime)) {
    ledState = HIGH;
    previousMillis = currentMillis;
    digitalWrite(ledPin, ledState);
  }
  // if the close limit input is high and the timer is less than the OffTime variable, turn led off.
  if ((closeLimitSwitchStatus == HIGH) && (currentMillis - previousMillis <= OffTime)) {
    ledState = LOW;
    previousMillis = currentMillis;
    digitalWrite(ledPin, ledState);
  }
}
10

onButtonState never gets assigned a value from any "button". It is initialised as LOW and then never changes. How were you expecting it to change? The compiler doesn't magically know that this is connected to any pin.

You check if it currently is HIGH and if it is, you set it back to LOW, since previousOnButtonState also never gets updated from its initial setting.