Will This Code Work?

Forum 2005-2010 (read only) Software Syntax & Programs
1

To work or not to work. That is the question.

#define LED1 13
#define LED2 12

#define S1 7
#define S2 6
#define S3 5
#define S4 4

#define R1 3

int val1=0;
int val2=0;

int sw1=0;
int sw2=0;
int sw3=0;
int sw4=0;


void setup(){
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(S1, INPUT);
  pinMode(S2, INPUT);
  pinMode(S3, INPUT);
  pinMode(S4, INPUT);
  pinMode(R1, INPUT);

}

void loop(){
  do
  {
    digitalWrite(LED1, LOW);
    digitalWrite(LED2, LOW);
    sw1 = digitalRead(S1);
    sw2 = digitalRead(S2);
    sw3 = digitalRead(S3);
    sw4 = digitalRead(S4);
   int val2=0;
  }
  while ( sw1 == HIGH );
  
  while ( true )
  {
    val1 = digitalRead(R1);
    if (R1 == HIGH and val2 == 0){
      int val2 = 1;
      digitalWrite(LED1, HIGH);
      delay(50);
      digitalWrite(LED1, LOW);
      
    }
    if (R1 == HIGH and val2  == 1) {
      int val2 = 0;
      digitalWrite(LED2, HIGH);
      delay(50);
      digitalWrite(LED2, LOW);
    }
    
  }
    

    


  while ( sw2 == HIGH );
  
  while ( true )
  {
    val1 = digitalRead(R1);
    if (val1 == HIGH and val2 == 0) {
      int val2=1;
    }
    if (val1 == HIGH and val2 == 1) {
      int val2=2;
      digitalWrite(LED1, HIGH);
      delay(50);
      digitalWrite(LED1, LOW);
   
    }
    if (val1 == HIGH and val2 == 2) {
      int val2=3;
    }
    if (val1 == HIGH and val2 == 3) {
     int val2=0;
    digitalWrite(LED2, HIGH );
    delay(50);
    digitalWrite(LED2, LOW);
    }    
            
  }
  
  while ( sw3 == HIGH);
  
  while ( true );
  {
    val1 = digitalRead(R1);
    if (val1 == HIGH and val2 == 0) {
      int val2=1;
    }
    if (val1 == HIGH and val2 == 1) {
      int val2=2;
    }
    if (val1 == HIGH and val2 == 2) {
      int val2=3;
      digitalWrite(LED1, HIGH);
      delay(50);
      digitalWrite(LED1, LOW);
    }
    if (val1 == HIGH and val2 == 3) {
      int val2=4;
    }
    if (val1 == HIGH and val2 == 4) {
      int val2=5;
    }
    if (val1 == HIGH and val2 == 5) {
      int val2=0;
      digitalWrite(LED2, HIGH);
      delay(50);
      digitalWrite(LED2, LOW);
    }

  }
  
  while (sw4 == HIGH);
  while ( true );
  {
    val1 = digitalRead(R1);
    if (val1 == HIGH and val2 == 0); {
      int val2=1;
    }
    if (val1 == HIGH and val2 == 1); {
      int val2=2;
    }
    if (val1 == HIGH and val2 == 2); {
      int val2=3;
    }
    if (val1 == HIGH and val2 == 3); {
      int val2=4;
      digitalWrite(LED1, HIGH);
      delay(50);
      digitalWrite(LED1, LOW);
    }
    if (val1 == HIGH and val2 == 4); {
      int val2=5;
    }
    if (val1 == HIGH and val2 == 5); {
      int val2=6;
    }
    if (val1 == HIGH and val2 == 6); {
      int val2=7;
    }
    if (val1 == HIGH and val2 == 7); {
      int val2=0;
      digitalWrite(LED2, HIGH);
      delay(50);
      digitalWrite(LED2, HIGH);
    }
  }
  
    

   
  }
2

Will it work? Maybe.

Will it do what you want? Don't know. You didn't say what you want it to do.

Will it all get executed? No.

  while ( true )
  {
    val1 = digitalRead(R1);
    if (R1 == HIGH and val2 == 0){
      int val2 = 1;
      digitalWrite(LED1, HIGH);
      delay(50);
      digitalWrite(LED1, LOW);
      
    }
    if (R1 == HIGH and val2  == 1) {
      int val2 = 0;
      digitalWrite(LED2, HIGH);
      delay(50);
      digitalWrite(LED2, LOW);
    }
  }

This is an infinite loop, with no way to break out of it. None of the following code will be executed.

val2 is initialized to 0. It may also be set to 0 in the preceding loop. But, it can never be anything but 0.

R1 is a constant (pin number). It is assigned a value of 3. That is not equal to HIGH, so you have an infinite loop containing two blocks of code for which the if conditions can never be true. All that it will do, then, is read the digital pin over and over again.

So, I think I'll reconsider my first comment. No, it won't work.

[edit]You have a global variable named val2, and several local variables of the same name. Not a good programming practice.[/edit]

3

Im not sure I quite get what you mean. BTW all of the "HIGH"s in the code got changed to "1"

4

He basically says:
yes, it works.

There is a giant but, however.
As PaulS tried to explain, it will surely not do what you want it to do.
Does it work -> does it run? yes
Does it work -> does it do what you want it to? no
This is due to poor and incorrect coding, such as the use of while(true) within your void loop() function.

5

That was asked:
http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1268363396/4#4
and the answer I got is what I used.
If the program is wrong please tell me how to fix it.

6

The program is wrong. But, before we can tell you how to fix it, you need to tell us what it is supposed to do.

I mentioned several things, in my previous post. You have infinite loops. You put data in one variable, and compare another. You are mixing pin numbers with data that was supposed to be read from that pin. You have local variables with the same name as global variables.

What else is wrong is hard to say without knowing what the program is supposed to do.

Just one final word of advice. I never write more than 20 to 30 lines of code before testing what I just wrote. I write code for a living, and I'm pretty good at it. But, I write a little, and test. Then, I write some more and test some more. Then, I look at making functions smaller, usually by making more of them. I borrow code form other applications I've written. I know that code works, so I don't need to spend a lot of time testing it, because it's already been tested.

You should verify that you can read a button state, by writing something to the serial monitor. That tells you that the button is wired correctly, that the right pin is being referenced, and that the right code is being used to read the pin, and to decide that the button is, or is not pressed, and that the code is responsive to button presses.

You should write code that does nothing but define the structure of the program. Use Serial.print (or turn an LED on or off) to demonstrate that you got to a particular place in the code, and verify that you got there as a result of specific, repeatable actions.

One does not try to build a house by starting with the roof. The foundation comes first. You have an entire house here, but the foundation is not correct. It will be more difficult to fix this now, that it would have been to build it piece by piece, testing each piece.

7

It is supposed to take the status from the switch on pin 3 and then, if switch 1 is on, make the 2 leds blink alternating every time it is hit, if switch 2 is on every other time, etc. etc.

8

It is supposed to take the status from the switch on pin 3 and then, if switch 1 is on, make the 2 leds blink alternating every time it is hit, if switch 2 is on every other time, etc. etc.

Let's break that statement down a little:

It is supposed to take the status from the switch on pin 3 and

For what purpose?

then, if switch 1 is on, make the 2 leds blink alternating every time it is hit

It? Every time what is hit?

You start out by reading the status of all 4 switches in a loop, while switch one is HIGH. The code continues only when switch one is released. Is that compatible with what you want to do?

These may seem like nit-picking comments and questions, but the code is going to do exactly what you tell it to do. Nothing more, nothing less. It is important that you think logically (like a computer) about what you want to do.

For instance, a statement like this is easy to code:

While switch one is held down, light the red LED. Make sure the LED turns off when switch one is released.

#define LEDPIN 13
#define SWITCHPIN 2
void loop()
{
   if(digitalRead(SWITCHPIN) == HIGH)
   {
      digitalWrite(LEDPIN, HIGH);
   }
   else
   {
      digitalWrite(LEDPIN, LOW);
   }
}

Try forgetting about the code you already have. Write out what you want the program to do, without using etc., etc. Don't make any assumptions about what we know. Tell us everything that needs to be done. State all timing-related issues (while this is happening, after that happens, etc.). Being able to explain to us what you are trying to do will make it much easier for you to write the program in the end.

9

It meaning the switch connected to pin 3.