dc motor not responding to limit switches

Hi all,

Not sure if this is the right place to put this but as part of a project I have a DC motor connected via an adafruit motor shield v2.3 (Using DC Motors | Adafruit Motor Shield V2 | Adafruit Learning System). It is supposed to run until a limit switch is pressed but it doesn’t seem to work. I’ve tried printing the results to my monitor and it all works fine. I think it might be something to do with the code I’m using for my motor. Does anyone know what’s wrong?

Code:

#include <Wire.h>
#include <Adafruit_MotorShield.h>
#include "utility/Adafruit_MS_PWMServoDriver.h" 
#define solenoidPin 7 //pin for solenoid
#define topPin 2 //pin for top switch
#define bottomPin 4 // pin for bottom switch

const int ldrPin = A0; //pin for 
bool isUp = false; //defining state of the door
// Create the motor shield object with the default I2C address
Adafruit_MotorShield AFMS = Adafruit_MotorShield();
// Or, create it with a different I2C address (say for stacking)
//Adafruit_MotorShield AFMS = Adafruit_MotorShield(0x61);
//Adafruit_MotorShield AFMS = Adafruit_MotorShield(0x57);


Adafruit_DCMotor *myMotor = AFMS.getMotor(1);

int counter = 0;
unsigned long previousMillis = 0;
unsigned long currentMillis; // this is part of something that was recommended by someone as an alternative to using delay() to run the program every second. I'm not rxactly sure how it works

 
void setup ()
{
  pinMode(7, OUTPUT); //setting the solenoid pins as an output
  pinMode(4, INPUT); //setting the switch pins to inputs 
  pinMode(2, INPUT);

  Serial.begin(9600);
  
  pinMode(ldrPin, INPUT); //setting the ldrPin to input

  Serial.println("Starting AMS");
  AFMS.begin();
  Serial.println(" AMS started, setup routine finished");
  myMotor->setSpeed(150);
  digitalWrite(solenoidPin, HIGH);
}
void pullSolenoid () { //subroutine for pulling the solenoid back
  digitalWrite(solenoidPin, LOW);
}
void releaseSolenoid () { //subroutine for releasing solenoid
  digitalWrite(solenoidPin, HIGH);
}
void doorUp () { //subroutine for lifitng the door up
  pullSolenoid();
  delay(500);
  myMotor->run(FORWARD);
  delay(1000);
  myMotor->run(RELEASE);
  delay(500);
  releaseSolenoid();
  /*Serial.println("pull solenoid");
  delay(500);
  Serial.println("run motor");
  delay(500);
  Serial.println("release solenoid");*/
  if (digitalRead(topPin) == LOW) {
    myMotor->run(FORWARD);
    //Serial.println("run motor");
    delay(500);
  }
  else if (digitalRead(topPin) == HIGH){
    myMotor->run(RELEASE);
    //Serial.println("stop motor");
    //delay(500);
  }
  Serial.println("Can get out of loop");
}
void doorDown () {
  Serial.println("entered loop");
  if (digitalRead(bottomPin) == LOW) {
    //myMotor->run(BACKWARD);
    Serial.println("run motor");
    delay(500);
  }
  else if (digitalRead(bottomPin) == HIGH) {
    //myMotor->run(RELEASE);
    Serial.println("stop motor");
    delay(500);
  }
  Serial.println("Can get out of loop");
}

void checkUp () { //subroutine checks if the door needs to go up 
  int ldrStatus = analogRead(ldrPin);
  if (ldrStatus > 400) {
    doorUp();
    isUp = true;
    delay(2000);
  }
  else {
    delay(2000);
  }
}
void checkDown () { //subroutine checks if the door needs to go down
  int ldrStatus = analogRead(ldrPin);
  if (ldrStatus < 300) {
    doorDown();
    isUp = false;
    delay(2000);
  }
  else {
    delay(2000);
  }
}
void loop ()
{
  int ldrStatus = analogRead(ldrPin); //check the brightness 
  Serial.println(ldrStatus); //print the brightness to the monitor
  
  currentMillis = millis();

  if (currentMillis - previousMillis >= 1000) {

    previousMillis = currentMillis;
    if (isUp == true) { //check if the door is up and then if it needs to go down
      checkDown();
      delay(1000);
    }
    else if(isUp == false) { // check if the door is down and then if it needs to go up
      checkUp();
      delay(1000);
    }
    else {
      delay(1000);
    }
    

  }
}

Wiring diagram, please.

Particularly show how the limit switches are connected. Do they have pull up or down resistors?

And what exactly does "it doesn't seem to work" mean? What actually happens?

Steve

This is the way I have wired the switches: BITE SIZE ARDUINO – 3 PIN SNAP-ACTION LEVER SWITCH – Killer Robotics . My motor is connected to the motor shield at M1. The motor shield is connected to the arduino at A4, A5, SDA, SCL, 5v and GND. What I mean mean by 'it doesn't seem to work' is that the motor just doesn't stop when the switch is pressed.

A4 and A5 are also SDA resp. SCL. You can't connect to all 4 pins.

A couple of things to note. If you want your code to be more responsive, you need to get rid of all those delay()s you have sprinkled throughout your code. A much better approach would be to implement a state machine and then, depending on which state you are in, you do what needs to be done (e.g. WAITING, DOOR_OPENING, DOOR_CLOSING, etc.)

If you are testing a boolean value with if(), you don't need to test the other state with elseif().

    if (isUp == true) { //check if the door is up and then if it needs to go down
      checkDown();
      delay(1000);
    }
    else if (isUp == false) { // check if the door is down and then if it needs to go up
      checkUp();
      delay(1000);
    }
    else {
      delay(1000);
    }

The final else() clause will never be executed.

    if (isUp == true) { //check if the door is up and then if it needs to go down
      checkDown();
      delay(1000);
    }
    else {
      checkUp();
      delay(1000);
    }

blh64:
A couple of things to note. If you want your code to be more responsive, you need to get rid of all those delay()s you have sprinkled throughout your code. A much better approach would be to implement a state machine and then, depending on which state you are in, you do what needs to be done (e.g. WAITING, DOOR_OPENING, DOOR_CLOSING, etc.)

If you are testing a boolean value with if(), you don't need to test the other state with elseif().

    if (isUp == true) { //check if the door is up and then if it needs to go down

checkDown();
      delay(1000);
    }
    else if (isUp == false) { // check if the door is down and then if it needs to go up
      checkUp();
      delay(1000);
    }
    else {
      delay(1000);
    }



The final else() clause will never be executed.


if (isUp == true) { //check if the door is up and then if it needs to go down
      checkDown();
      delay(1000);
    }
    else {
      checkUp();
      delay(1000);
    }

I've only got a couple of weeks left to do this. Is this strictly necessary to fix the motor problem or can I do this at some other point?

Use Serial Monitor and add some Serial.print of strategic data along loop. Thaat will be like Your eye into the code.

Railroader:
Use Serial Monitor and add some Serial.print of strategic data along loop. Thaat will be like Your eye into the code.

I've done that and it all seems to be working as it should be, which is what made me think it was the code I was using for the motor that was the problem.

What does the test prints say?
I see one weekness in the code. You only sample the isUp once per second, when You call the functions checkUp, or checkDown. What about doing it in loop?

Plasterboard:
I've only got a couple of weeks left to do this. Is this strictly necessary to fix the motor problem or can I do this at some other point?

If you don't have time to do it right, how do you have time to do it again? Not taking the time to do it right due to time constraints is not a good thing

Railroader:
What does the test prints say?
I see one weekness in the code. You only sample the isUp once per second, when You call the functions checkUp, or checkDown. What about doing it in loop?

It displayed the stuff that is currently commented out in the doorUp and doorDown subroutines, as it was supposed to.

blh64:
If you don't have time to do it right, how do you have time to do it again? Not taking the time to do it right due to time constraints is not a good thing

Its a thing that I'm doing for school, I just need to get it running to some degree in the next couple of weeks. I can improve it as much as I want afterwards.

Okey. Your test prints dosen't give any clear message. Maybe they can be enlarged, sharpened. Does the end switch respond if You manualy keeps it activated for some 10 seconds or more?
Have You tried my suggestion of sampling the endswitch in loop?

Railroader:
Okey. Your test prints dosen't give any clear message. Maybe they can be enlarged, sharpened. Does the end switch respond if You manualy keeps it activated for some 10 seconds or more?
Have You tried my suggestion of sampling the endswitch in loop?

By 'end switch' do you mean the bottom switch? If so, it will respond when I'm just using the serial print but when using the motor, it will not. I'm not sure what you mean in your sampling suggestion.

To make the motor stop I think I need to use myMotor->run(RELEASE);. This is why I’m trying to use an if loop, is there a way that I might be able to use a while loop if that would be a better option?

Hi,
Do you know if your limit switches are influencing your code?

Can you write some test code with serial prints to JUST show your limit switch states and see if they change when you manually operate them?
Don’t have anything in the code related to the motor driver, start with a blank code template.

Thanks… Tom… :slight_smile:

So I’ve just tried taking everything out of the code and just having a motor and a switch.

#include <Wire.h>
#include <Adafruit_MotorShield.h>
#include "utility/Adafruit_MS_PWMServoDriver.h"
Adafruit_MotorShield AFMS = Adafruit_MotorShield(); 
Adafruit_DCMotor *myMotor = AFMS.getMotor(1);
#define switchPin 2
void setup() {
  // put your setup code here, to run once:
  AFMS.begin();
  myMotor->setSpeed(150);
  pinMode(switchPin, INPUT);

}

void loop() {
  // put your main code here, to run repeatedly:
  if (digitalRead(switchPin) == LOW) {
    myMotor->run(FORWARD);
  }
  else if (digitalRead(switchPin) == HIGH) {
    myMotor->run(RELEASE);
  }
  

}

The motor will now stop when I press the switch so I’m not sure what it is in the other code that doesn’t make it work.

Hi,
Just a hint, instead of continually going back and read a pin all through your code, only read the pin at the start of the loop, then let your code act on that input.
Update the input each time you start the loop.

In other words take a snapshot of ALL the inputs you can at the top of the loop, store their value in a variables and use that variable value where needed in your code.

#include <Wire.h>
#include <Adafruit_MotorShield.h>
#include "utility/Adafruit_MS_PWMServoDriver.h"
Adafruit_MotorShield AFMS = Adafruit_MotorShield();
Adafruit_DCMotor *myMotor = AFMS.getMotor(1);
#define switchPin 2
bool switchState;
void setup()
 {
  AFMS.begin();
  myMotor->setSpeed(150);
  pinMode(switchPin, INPUT);
}


void loop() 
{
  switchState = digitalRead(switchPin);
  if (switchState == LOW) {
    myMotor->run(FORWARD);
  }
  else  // if switchState is not LOW then it must be HIGH
  {
    myMotor->run(RELEASE);
  }
}

Tom… :slight_smile:

TomGeorge:
Hi,
Do you know if your limit switches are influencing your code?

Can you write some test code with serial prints to JUST show your limit switch states and see if they change when you manually operate them?
Don’t have anything in the code related to the motor driver, start with a blank code template.

Thanks… Tom… :slight_smile:

What do you mean by ‘influence’? Do you mean that the arduino is actually registering the switch presses, then yes. As I have said in the previous post, with everything else removed the motor will stop when the switch is pressed. In my original code, when I’m just using serial print it says that the switches are being pressed.

Plasterboard:
By 'end switch' do you mean the bottom switch? If so, it will respond when I'm just using the serial print but when using the motor, it will not. I'm not sure what you mean in your sampling suggestion.

Read those switches in loop, not "faar away", once per second, in those 2 functions.