Pages: [1]   Go Down
Author Topic: Sketch works with one button but not 2  (Read 399 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 8
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi All,

I've been trying to write a sketch that controls a motor. One button for up, one for down. I am using the Ardunio Motor Shield to control the motor.
I have everything connected to do with the hardware. If I run the sketch with just one button coded it works as intended, and if I code for just the other that works too, but when without changing any hardware I code for both it does not! The indicating lights come on on the shield but nothing comes from the motor.
The motor is powered by a wall power supply
To add a second button I have re-named, but basically just duplicated the code that makes one button work

Code:
#define LED 13
#define BUTTON 5
#define DOWN 6
#define PWA 3
#define DIRA 12

int val = 0;

int old_val = 0;

int va = 0;

int old_va = 0;

int state = 0;



void setup()
{
  pinMode(LED, OUTPUT);
  pinMode(BUTTON, INPUT);
  pinMode(DOWN, INPUT);
  pinMode(PWA, OUTPUT);
  pinMode(DIRA, OUTPUT);
}

void loop()
{
  val = digitalRead(BUTTON);
  va = digitalRead(DOWN);
 
  if((val == HIGH) && (old_val))
  {
    state = 1 - state;
    delay(10);
  }
 
  if((va == HIGH) && (old_va))
  {
    state = 2 - state;
    delay(10);
  }
 
  old_va = va;
  old_val = val;
 
  if(state == 1)
  {
    digitalWrite(LED, HIGH);
    digitalWrite(DIRA, LOW);
 analogWrite(PWA, 255);
  }else{
    digitalWrite(LED, LOW);
    analogWrite(PWA, 0);
  }
 
  if(state == 2)
  {
    digitalWrite(LED, LOW);
    digitalWrite(DIRA, HIGH);
 analogWrite(PWA, 255);
  }else{
    digitalWrite(LED, LOW);
    analogWrite(PWA, 0);
  }
 
}
Logged

Sydney, Australia
Offline Offline
Edison Member
*
Karma: 27
Posts: 1178
Big things come in large packages
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Couple of things that you can try:
1. Make your variable names more descriptive. For example if the button is UP, then variables associated should have up in the name. It makes it easier to read for everybody.
2. You should use Serial.Print() statements to look at what your state variable is, because if it is not 1 or 2 the code will do nothing. Printing stuff out is a good way to understand the way that your program is working and can all be removed at the end.
3. You have delays in the code that don't appear to be needed. This won't make it stop working but is just unnecessary.
« Last Edit: August 21, 2012, 09:38:56 pm by marco_c » Logged

Arduino libraries http://arduinocode.codeplex.com
Parola hardware & library http://parola.codeplex.com

0
Offline Offline
God Member
*****
Karma: 2
Posts: 854
Arduino rocks!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset



Your code is atrocious to try and read.  Even stuff like your if statements, why the explicit "== HIGH" on only one branch?

I'm pretty sure your problem is that you're setting your state variable wrong.  I don't think

state = 1 - state;
state = 2 - state;

is doing what you think it is.  But because of how poorly written your code is, it's hard to say.

Your if's and else's make no sense.  If state = 1, doesn't that guarantee the last thing you do is execute the else for state = 2 so you'll never even see the result of your state = 1 portion?

Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 8
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Thank you for your suggestions, I did them and it now works! I appreciate your points on making the code clearer and less messy.  smiley
Logged

Pages: [1]   Go Up
Jump to: