Uno + Motor Shield. Motor not behaving as it should.

This sketch is to open a door to let my chickens in/out of the coop based on when it is light/dark. Most of this code is from Dave Naves (http://davenaves.com/blog/interests-projects/chickens/chicken-coop/arduino-chicken-door/).

I had to update it a little to add the proper commands for the motor shield.

The issue is that when I want to open the door, the motor stops as soon as it sees the bottom switch open. It should keep going until it hits the top switch.

It works as it should when closing (keeps going until it hits the bottom switch).

Any ideas why it’s working like that and how to fix?

// libraries

  #include <SimpleTimer.h>                     // load the SimpleTimer library to make timers, instead of delays & too many millis statements
  #include "MotorDriver.h"

//pins
  const int photocellPin = A0;                 // photocell connected to analog 0  
  const int bottomSwitchPin = 2;              // bottom switch is connected to pin 26
  const int topSwitchPin = 4;                 // top switch is connected to pin 27
  const int coopDoorOpenLed = 7;              // led set to digital pin 40
  const int coopDoorClosedLed = 8;            // led set to digital pin 41

// photocell
  int photocellReading;                        // analog reading of the photocel
  int photocellReadingLevel;                   // photocel reading levels (dark, twilight, light)

// reed switches top and bottom of coop door

// top switch

  int topSwitchPinVal;                   // top switch var for reading the pin status
  int topSwitchPinVal2;                  // top switch var for reading the pin delay/debounce status
  int topSwitchState;                    // top switch var for to hold the switch state

// bottom switch

  int bottomSwitchPinVal;                // bottom switch var for reading the pin status
  int bottomSwitchPinVal2;               // bottom switch var for reading the pin delay/debounce status
  int bottomSwitchState;                 // bottom switch var for to hold the switch state

// just need 1 SimpleTimer object
  SimpleTimer coopTimer;
// ************************************** the setup **************************************

void setup(void) {
  Serial.begin(9600);

// coop door leds
  pinMode (coopDoorOpenLed, OUTPUT);                // enable coopDoorOpenLed = output
  pinMode (coopDoorClosedLed, OUTPUT);              // enable coopDoorClosedLed = output

// coop door switches
// bottom switch
  pinMode(bottomSwitchPin, INPUT);                   // set bottom switch pin as input
  digitalWrite(bottomSwitchPin, HIGH);               // activate bottom switch resistor

// top switch
  pinMode(topSwitchPin, INPUT);                      // set top switch pin as input
  digitalWrite(topSwitchPin, HIGH);                  // activate top switch resistor  

  // timed actions setup
  coopTimer.setInterval(5000, readPhotoCell);      // read the photocell every 5 seconds

}

// functions

// operate the coop door

// photocell to read levels of exterior light

 void readPhotoCell() { // function to be called repeatedly - per cooptimer set in setup

    photocellReading = analogRead(photocellPin);
      Serial.print(" Photocel Analog Reading = ");
      Serial.println(photocellReading);

//  set photocel threshholds
        if (photocellReading >= 0 && photocellReading <= 700) {
        photocellReadingLevel = '1';
        Serial.print(" Photocel Reading Level:");
        Serial.println(" - Dark");
      }
          if (photocellReading  >= 1000 ) {
            photocellReadingLevel = '3';
            Serial.print(" Photocel Reading Level:");
            Serial.println(" - Light");
       }
     }

//debounce bottom reed switch

  void debounceBottomReedSwitch() { 

    //debounce bottom reed switch
    bottomSwitchPinVal = digitalRead(bottomSwitchPin);        // read input value and store it in val
    delay(100);
    bottomSwitchPinVal2 = digitalRead(bottomSwitchPin);       // read input value again to check or bounce

      if (bottomSwitchPinVal == bottomSwitchPinVal2) {        // make sure we got 2 consistant readings!
        if (bottomSwitchPinVal != bottomSwitchState) {        // the switch state has changed!
          bottomSwitchState = bottomSwitchPinVal;
        }
        Serial.print (" Bottom Switch Value: ");             // display "Bottom Switch Value:" 
        Serial.println(digitalRead(bottomSwitchPin));        // display current value of bottom switch;
      }
    }

 // debounce top reed switch
  void debounceTopReedSwitch() {

    topSwitchPinVal = digitalRead(topSwitchPin);               // read input value and store it in val
    delay(100);
    topSwitchPinVal2 = digitalRead(topSwitchPin);              // read input value again to check or bounce

      if (topSwitchPinVal == topSwitchPinVal2) {               // make sure we got 2 consistant readings!
        if (topSwitchPinVal != topSwitchState) {               // the button state has changed!
          topSwitchState = topSwitchPinVal;
        }
        Serial.print (" Top Switch Value: ");                // display "Bottom Switch Value:" 
        Serial.println(digitalRead(topSwitchPin));           // display current value of bottom switch;
      }
    }

  void closeCoopDoorMotorB() {  
    if (bottomSwitchPinVal == 1) {
      motordriver.init(MOTORA);
      motordriver.setSpeed(100,MOTORA);
      motordriver.goBackward();
        if (bottomSwitchPinVal == 0) {                         // if bottom reed switch circuit is closed
        motordriver.stop();
        Serial.print(" Coop Door Closed");
     }
   }
  }

  void openCoopDoorMotorB() {
    if (topSwitchPinVal == 1) { 
      motordriver.init(MOTORA);
      motordriver.setSpeed(235,MOTORA);
      motordriver.goForward(); 
       if (topSwitchPinVal == 0) {                   // if top reed switch circuit is not closed
         motordriver.stop();
         Serial.print(" Coop Door Open");
     }
   } 
  }
     
  void doCoopDoor(){

      if (photocellReadingLevel  == '1') {              // if it's dark
        if (photocellReadingLevel != '3') {           // if it's not light 
            debounceTopReedSwitch();                    // read and debounce the switches
            debounceBottomReedSwitch();
            closeCoopDoorMotorB();                      // close the door
        }
      }
      else if (photocellReadingLevel  == '3') {             // if it's light
          if (photocellReadingLevel != '1') {           // if it's not dark
             debounceBottomReedSwitch();
             debounceTopReedSwitch();                    // read and debounce the switches
             openCoopDoorMotorB();                       // Open the door
          }
         
      }
    }
  
//  coop door status: red if open, green if closed, blinking red if stuck 

void doCoopDoorLed() {
  if (bottomSwitchPinVal == 0) {                         // if bottom reed switch circuit is closed
    digitalWrite (coopDoorClosedLed, HIGH);              // turns on coopDoorClosedLed (green)
    digitalWrite (coopDoorOpenLed, LOW);                 // turns off coopDoorOpenLed (red)
  }
  else if(topSwitchPinVal == 0) {                        // if top reed switch circuit is closed 
    digitalWrite (coopDoorClosedLed, LOW);               // turns off coopDoorClosedLed (green)
    digitalWrite (coopDoorOpenLed, HIGH);                // turns on coopDoorOpenLed (red)
  }
  //    else if (topSwitchPinVal != 0)  && if (bottomSwitchPinVal != 0) {                // if bottom and top reed switch circuits are open 
  //            doCoopDoorLedError();                         // blink the coopDoorOpenLed
  //         }
  //         }
  else {
    digitalWrite (coopDoorClosedLed, LOW);              // turns off coopDoorClosedLed (green)
    digitalWrite (coopDoorOpenLed, LOW);                // turns off coopDoorOpenLed (red)
  }
}

// ************************************** the loop **************************************
void loop() {
  //  polling occurs
  coopTimer.run();
  doCoopDoor();
  doCoopDoorLed();

}

The first question whenever there is an issue with switches is "How did you wire them?". The second is why so f**king complicated. Use the internal pullup resistor and connect one leg to ground and one leg to the digital pin.

HIGH is not pressed. LOW (like the top of the switch) is pressed.

      if (photocellReadingLevel  == '1') {              // if it's dark
        if (photocellReadingLevel != '3') {           // if it's not light

If the value IS 1, it certainly isn't 3.

You have WAY too much for diagnosing the problem with one switch.

PaulS:
The first question whenever there is an issue with switches is "How did you wire them?". The second is why so f**king complicated. Use the internal pullup resistor and connect one leg to ground and one leg to the digital pin.

HIGH is not pressed. LOW (like the top of the switch) is pressed.

      if (photocellReadingLevel  == '1') {              // if it's dark

if (photocellReadingLevel != '3') {          // if it's not light



If the value IS 1, it certainly isn't 3. 

You have WAY too much for diagnosing the problem with one switch.

I do see that is kind of redundant now... Got that removed.

I am using the internal pullup on pins 2 and 4 (bottom & top) switch with the other leg connected to ground.

I am using the motor shield since I am driving a 12V 1A motor.

Connections used

A0 = Photocell resistance
2 = bottom switch (NO reed switch)
4 = top switch (NO reed switch)
7 = LED output
8 = LED output

Forgot to add that when it shuts off when opening, it then continues to pulse very rapidlydoesn't move the motor, but LED on the motor shield blink). This pulsing changes with the delay for debouncing the switches.

You need to write a simple sketch that moves the door from open to closed, waits, and then moves it from closed to open, and waits again. Repeat over and over.

Either you are not reading the switches properly or there is something wrong with the motor wiring. Forget about the light sensor, etc. until the door opens and closes properly.

You need ONE function to debounce any number of switches. Don't duplicate code. That way lies madness. Hmmm, you seem to be experiencing madness.

Thanks for the help cleaning up my sketch. I will portion out the sketch into smaller pieces and see what I can figure out.

Here's the fritz... pretty simple. I do have a fuse before any connections on the battery positive as well... 10k ohm resistors for all 3.

Also… here is the slightly cleaned up code…

// libraries

  #include <SimpleTimer.h>                     // load the SimpleTimer library to make timers, instead of delays & too many millis statements
  #include "MotorDriver.h"

//pins
  const int photocellPin = A0;                 // photocell connected to analog 0  
  const int bottomSwitchPin = 2;              // bottom switch is connected to pin 26
  const int topSwitchPin = 4;                 // top switch is connected to pin 27
  const int coopDoorOpenLed = 7;              // led set to digital pin 40
  const int coopDoorClosedLed = 8;            // led set to digital pin 41

// photocell
  int photocellReading;                        // analog reading of the photocel
  int photocellReadingLevel;                   // photocel reading levels (dark, twilight, light)

// reed switches top and bottom of coop door

// top switch

  int topSwitchPinVal;                   // top switch var for reading the pin status
  int topSwitchPinVal2;                  // top switch var for reading the pin delay/debounce status
  int topSwitchState;                    // top switch var for to hold the switch state

// bottom switch

  int bottomSwitchPinVal;                // bottom switch var for reading the pin status
  int bottomSwitchPinVal2;               // bottom switch var for reading the pin delay/debounce status
  int bottomSwitchState;                 // bottom switch var for to hold the switch state

// just need 1 SimpleTimer object
  SimpleTimer coopTimer;
// ************************************** the setup **************************************

void setup(void) {
  Serial.begin(9600);

// coop door leds
  pinMode (coopDoorOpenLed, OUTPUT);                // enable coopDoorOpenLed = output
  pinMode (coopDoorClosedLed, OUTPUT);              // enable coopDoorClosedLed = output

// coop door switches
// bottom switch
  pinMode(bottomSwitchPin, INPUT);                   // set bottom switch pin as input
  digitalWrite(bottomSwitchPin, HIGH);               // activate bottom switch resistor

// top switch
  pinMode(topSwitchPin, INPUT);                      // set top switch pin as input
  digitalWrite(topSwitchPin, HIGH);                  // activate top switch resistor  

  // timed actions setup
  coopTimer.setInterval(5000, readPhotoCell);      // read the photocell every 5 seconds

}

// functions

// operate the coop door

// photocell to read levels of exterior light

 void readPhotoCell() { // function to be called repeatedly - per cooptimer set in setup

    photocellReading = analogRead(photocellPin);
      Serial.print(" Photocel Analog Reading = ");
      Serial.println(photocellReading);

//  set photocel threshholds
        if (photocellReading >= 0 && photocellReading <= 700) {
        photocellReadingLevel = '1';
        Serial.print(" Photocel Reading Level:");
        Serial.println(" - Dark");
      }
          if (photocellReading  >= 1000 ) {
            photocellReadingLevel = '3';
            Serial.print(" Photocel Reading Level:");
            Serial.println(" - Light");
       }
     }

//debounce bottom reed switch

  void debounceReedSwitches() { 

    //debounce bottom reed switch
    bottomSwitchPinVal = digitalRead(bottomSwitchPin);        // read input value and store it in val
    delay(100);
    bottomSwitchPinVal2 = digitalRead(bottomSwitchPin);       // read input value again to check or bounce

      if (bottomSwitchPinVal == bottomSwitchPinVal2) {        // make sure we got 2 consistant readings!
        if (bottomSwitchPinVal != bottomSwitchState) {        // the switch state has changed!
          bottomSwitchState = bottomSwitchPinVal;
        }
        Serial.print (" Bottom Switch Value: ");             // display "Bottom Switch Value:" 
        Serial.println(digitalRead(bottomSwitchPin));        // display current value of bottom switch;
      }
 

 // debounce top reed switch
  topSwitchPinVal = digitalRead(topSwitchPin);               // read input value and store it in val
    delay(100);
    topSwitchPinVal2 = digitalRead(topSwitchPin);              // read input value again to check or bounce

      if (topSwitchPinVal == topSwitchPinVal2) {               // make sure we got 2 consistant readings!
        if (topSwitchPinVal != topSwitchState) {               // the button state has changed!
          topSwitchState = topSwitchPinVal;
        }
        Serial.print (" Top Switch Value: ");                // display "Bottom Switch Value:" 
        Serial.println(digitalRead(topSwitchPin));           // display current value of bottom switch;
      }
  }

  void doCoopDoor(){

      if (photocellReadingLevel  == '1') {              // if it's dark
          debounceReedSwitches();                    // read and debounce the switches
          if (bottomSwitchPinVal == 1) {
            motordriver.init();
            motordriver.setSpeed(100,MOTORA);
            motordriver.goBackward();
              if (bottomSwitchPinVal == 0) {                         // if bottom reed switch circuit is closed
              motordriver.stop();
              Serial.print(" Coop Door Closed");
     }
   }
  }
        else if (photocellReadingLevel  == '3') {             // if it's light
          debounceReedSwitches();                    // read and debounce the switches
          if (topSwitchPinVal == 1) { 
            motordriver.init();
            motordriver.setSpeed(235,MOTORA);
            motordriver.goForward(); 
              if (topSwitchPinVal == 0) {                   // if top reed switch circuit is not closed
              motordriver.stop();
              Serial.print(" Coop Door Open");
     }
   } 
  }
 }
  
//  coop door status: red if open, green if closed, blinking red if stuck 

void doCoopDoorLed() {
  if (bottomSwitchPinVal == 0) {                         // if bottom reed switch circuit is closed
    digitalWrite (coopDoorClosedLed, HIGH);              // turns on coopDoorClosedLed (green)
    digitalWrite (coopDoorOpenLed, LOW);                 // turns off coopDoorOpenLed (red)
  }
  else if(topSwitchPinVal == 0) {                        // if top reed switch circuit is closed 
    digitalWrite (coopDoorClosedLed, LOW);               // turns off coopDoorClosedLed (green)
    digitalWrite (coopDoorOpenLed, HIGH);                // turns on coopDoorOpenLed (red)
  }
  //    else if (topSwitchPinVal != 0)  && if (bottomSwitchPinVal != 0) {                // if bottom and top reed switch circuits are open 
  //            doCoopDoorLedError();                         // blink the coopDoorOpenLed
  //         }
  //         }
  else {
    digitalWrite (coopDoorClosedLed, LOW);              // turns off coopDoorClosedLed (green)
    digitalWrite (coopDoorOpenLed, LOW);                // turns off coopDoorOpenLed (red)
  }
}

// ************************************** the loop **************************************
void loop() {
  //  polling occurs
  coopTimer.run();
  doCoopDoor();
  doCoopDoorLed();

}

I think I found the answer. Total noob problem... Motor shield was using a pin that I was using as an LED output. Got that cleaned up and will test when I get home later today!

Working now! Great to have the project done and I feel really dumb for spending so much time thinking/tinkering before realizing the issue.