Arduino/Relay control, digitalWrite problems

Hey guys, this place is really great. I am currently working on a project that is basically remote controlled Christmas lights. I have an app on my Android phone that sends a specific byte of data depending on the button pressed. (in my case i have 4 relays, so 1-4 for each relay, 5 for random flashing, 6 for all relays closed and 7 for all relays open) The Arduino receives the data via a bluetooth receiver. My idea was as follows, i want to be able to control the relay turning on and off, just with sending the byte '1' and if it is on, it turns off, and the other way around. This is my code to do so.

if (command == 1) //Relay 1
{
while (command != 7)
{
if (digitalRead(pinB) == LOW) //Checks if the relay is closed
{
digitalWrite(pinB,HIGH); //The relay stays on
}
else
{
digitalWrite(pinB,LOW); //The relay turns off
}
}
}

And the first relay closes perfectly fine, but when i send a second '1' it does nothing. Also, i have this code in it as well to turn all of the relays off...
if (command == 7)
{
while (command != 6)
{
digitalWrite(pinA, LOW);
digitalWrite(pinB, LOW);
digitalWrite(pinC, LOW);
digitalWrite(pinD, LOW);
}
}
but even when i send the byte '7' to the Arduino, the first relay stays closed. This is confusing me thoroughly. The only thing i can think of is the Arduino isn't taking the second the second byte at all, it is too focused on the first one received. Any tips? I can post the full code if needed. Thanks so much.

I suggest you post all of your code, and enclose it in [ code ] and [ /code ] tags. But before you do that, re-read the sticky "Read this before posting a programming question".

Thanks for the push to re-read the sticky, my fault. Here is the full code in the correct format. I'm not worried about the random flashing part for right now.

const int pinA = 4;
const int pinB = 5;
const int pinC = 6;
const int pinD = 7;
byte command;
int randomPin;  //for use in the random function
const int rHigh = 1000;  // sets the upper limit for the random delays
const int rLow = 200; // sets the lower limit for the random delyas

void setup()
{
  pinMode (pinA, OUTPUT); // set the relays pin as an output
  pinMode (pinB, OUTPUT); // set the relays pin as an output
  pinMode (pinC, OUTPUT); // set the relays pin as an output
  pinMode (pinD, OUTPUT); // set the relays pin as an output
  Serial.begin(9600); // initialize serial communications:
}

void loop()
{ 
  digitalWrite (pinA, LOW);  //Shuts off relay A at the beginning of the program and after the stop command is sent 
  digitalWrite (pinB, LOW);  //Shuts off relay B at the beginning of the program and after the stop command is sent
  digitalWrite (pinC, LOW);  //Shuts off relay C at the beginning of the program and after the stop command is sent
  digitalWrite (pinD, LOW);  //Shuts off relay D at the beginning of the program and after the stop command is sent

  if ( Serial.available() > 0 )     // Check if a command has been sent.
  {
    command = Serial.read();    // write the serial command value   
    
    if (command == 1)  //Relay 1
    {
    while (command != 7)
    {
     if (digitalRead(pinA) == LOW) //Checks if the relay is closed
     {
      digitalWrite(pinA,HIGH); //The relay stays on
      }
     else
     {
      digitalWrite(pinA,LOW); //The relay turns off
     }
    }
  }


    if (command == 2)  //Relay 2
    {
    while (command != 7)
    {
     if (digitalRead(pinB) == LOW) //Checks if the relay is closed
     {
      digitalWrite(pinB,HIGH); //The relay stays on
      }
     else
     {
      digitalWrite(pinB,LOW); //The relay turns off
     }
    }
  }
    
    if (command == 3)  //Relay 3
    {
    while (command != 7)
    {
     if (digitalRead(pinC) == LOW) //Checks if the relay is closed
     {
      digitalWrite(pinC,HIGH); //The relay stays on
      }
     else
     {
      digitalWrite(pinC,LOW); //The relay turns off
   }
    }
    }

    if (command == 4)  //Relay 4
    {
     while (command != 7)
    {
     if (digitalRead(pinD) == LOW) //Checks if the relay is closed
     {
      digitalWrite(pinD, HIGH); //The relay stays on
      }
     else
     {
      digitalWrite(pinD, LOW); //The relay turns off
     }
    }
   }

    if (command == 5)  //Random flashing
    {
      while (command != 7) //Until stop command is sent
      {
        randomPin = random(pinA, pinD + 1); //Selects a random relay
        digitalWrite(randomPin, HIGH);      //Turns it on
        delay(random(rLow, rHigh));          //Stays on for a random time period
        digitalWrite(randomPin, LOW);       //Turns off
        delay(random(rLow, rHigh));          //Optional off delay
      } 
    }

    if (command == 6)
    {
    while (command != 7)
    {
    digitalWrite(pinA, HIGH);
    digitalWrite(pinB, HIGH);
    digitalWrite(pinC, HIGH);
    digitalWrite(pinD, HIGH);
    }
    }

    if (command == 7)
    {
    while (command != 6)
    {
    digitalWrite(pinA, LOW);
    digitalWrite(pinB, LOW);
    digitalWrite(pinC, LOW);
    digitalWrite(pinD, LOW);
    }
    }

  }
}
if (command == 7)
    {
    while (command != 6)
    {
    digitalWrite(pinA, LOW);
    digitalWrite(pinB, LOW);
    digitalWrite(pinC, LOW);
    digitalWrite(pinD, LOW);
    }
    }

but even when i send the byte '7' to the Arduino, the first relay stays closed. This is confusing me thoroughly. The only thing i can think of is the Arduino isn't taking the second the second byte at all, it is too focused on the first one received. Any tips? I can post the full code if needed. Thanks so much.

Not surprising. If command == 7 it will always !=6 so you will be stuck in that while loop. The while loop needs command == 6 to exit. You don't need that loop at all.

It sounds like a switch statement might be a better idea.

The command variable never gets updated inside of any of your while loops, so they'll loop forever.

If the code to drive the relays is intended to happen once, get rid of the while loops. If it's intended to happen over and over again (like a blink), then it should be outside of the if statement that checks for available serial data.

If you look at the very beginning of your loop() function you are turning off all the relays. As the loop() is always...well looping... continuously...., I think this might be fooling with your desired relay logic states. If even needed this initial set up state of the relays should probably be in the setup() function.

Loop() keeps on loopin, sounds like a good song chorus!

And I agree a switch statement might go further to let you express and see what you want to happen.

Lefty

The command variable never gets updated inside of any of your while loops, so they'll loop forever.
If the code to drive the relays is intended to happen once, get rid of the while loops. If it's intended to happen over and over again (like a blink), then it should be outside of the if statement that checks for available serial data.

I think i can say i understand that. Well the intent was for it to happen once, then stay how it is until another command that involves that pin is received. So, if pinA becomes HIGH, i want it to stay HIGH, until i turn all pins LOW, I set them to go randomly, or I send the '1' byte again and set it to be LOW. What i'm trying not to do is make it so that if pinA is HIGH and it receives the byte '2', pinA becomes LOW and pinB goes HIGH.

3tuxedo:

What i'm trying not to do is make it so that if pinA is HIGH and it receives the byte '2', pinA becomes LOW and pinB goes HIGH.

So then just put that in each case. If it's '1', turn pinA HIGH, pinB,C and D Low. If it's '2', pinB to HIGH, pinA, C, D to LOW, etc.

So then just put that in each case. If it's '1', turn pinA HIGH, pinB,C and D Low. If it's '2', pinB to HIGH, pinA, C, D to LOW, etc.

But see, wouldn't that turn off pinA if a '2' is received to turn pinB HIGH? If pinA is HIGH, and a '2' is received, that shouldn't affect it, it should only affect pinB.

3tuxedo:

So then just put that in each case. If it's '1', turn pinA HIGH, pinB,C and D Low. If it's '2', pinB to HIGH, pinA, C, D to LOW, etc.

But see, wouldn't that turn off pinA if a '2' is received to turn pinB HIGH? If pinA is HIGH, and a '2' is received, that shouldn't affect it, it should only affect pinB.

So then don't turn off pinA and just turn on pinB...

So then don't turn off pinA and just turn on pinB...

Easy enough! Thanks so much.
How would I go about writing the code so that in the switch statement, if the pin is already on, turn it off, and if it is already off turn it on? Just an if statement inside of the case?

The command variable never gets updated inside of any of your while loops, so they'll loop forever.

I think that may be the over arching problem. Do i need the command variable in setup? Or still in the loop just updated each time? I am unsure of how to do that if it's right.

Do i need the command variable in setup?

No.

Or still in the loop just updated each time?

Something inside the while loop, or external to the loop() function (such as an interrupt service routine) must update the variable(s) that the while loop is monitoring, if the while loop is ever to end.

Easy enough! Thanks so much.
How would I go about writing the code so that in the switch statement, if the pin is already on, turn it off, and if it is already off turn it on? Just an if statement inside of the case?

You could store each relay state in a variable and toggle both the relay and the variable if you want. You could read the pin's state directly and take action.

I get the idea that you might just turn off all the relays right before you turn on what you want.

You could store each relay state in a variable and toggle both the relay and the variable if you want. You could read the pin's state directly and take action.

So depending on the state a different function, or statement, or what have you, will take action to the relay. That sounds like it may possibly work, considering i have absolutely 0 experience with interrupt service routines. Although that is probably a way to do it as well. Do you mind sharing an idea of how you were thinking of achieving this?

You can find a good example of using a variable to keep a state in the blink without delay sketch.

Ha, well this is embarrassing. When I attempted to upload my updated code, i forgot that my Bluetooth receiver was plugged in, and on. That was stopping the code from being uploaded to the Arduino. Thanks to everyone who gave input and suggestions to help my naivety. So here is my present code...

int pinA = 4;
int pinB = 5;
int pinC = 8;
int pinD = 7;
byte command = 0;
const int rHigh = 800;
const int rLow = 200;
int randomPin;
dud
void setup() 
{                
  pinMode(pinA, OUTPUT);
  Serial.begin(9600);
}

void loop() 
{
  if (Serial.available() > 0)
  {
  command = Serial.read();
  switch (command)
    {
     case 1:
      if (digitalRead(pinA) == LOW)
      {
      digitalWrite(pinA, HIGH);
      }
      else
      {
      digitalWrite(pinA, LOW);
      }
      break;
     case 2:
      if (digitalRead(pinB) == LOW)
      {
      digitalWrite(pinB, HIGH);
      }
      else
      {
      digitalWrite(pinB, LOW);
      }
      break;
     case 3:
      if (digitalRead(pinC) == LOW)
      {
      digitalWrite(pinC, HIGH);
      }
      else
      {
      digitalWrite(pinC, LOW);
      }
      break;
     case 4:
      if (digitalRead(pinD) == LOW)
      {
      digitalWrite(pinD, HIGH);
      }
      else
      {
      digitalWrite(pinD, LOW);
      }
      break;
     case 5:
      do
        {
        randomPin = random(pinA, pinD + 1);
        digitalWrite(randomPin, HIGH);
        delay(random(rLow, rHigh));
        digitalWrite(randomPin, LOW);
        }
        while (command == 5);
      break;
     case 6:
      digitalWrite(pinA, HIGH);
      digitalWrite(pinB, HIGH);
      digitalWrite(pinC, HIGH);
      digitalWrite(pinD, HIGH);
      break;
     case 7:
      digitalWrite(pinA, LOW);
      digitalWrite(pinB, LOW);
      digitalWrite(pinC, LOW);
      digitalWrite(pinD, LOW);
      break;
    }
  }
}

Now the problem I am facing is two-fold. The 'random' portion of the code (case 5) is unstoppable unless I press the reset button. How would i make it so sending a 6 or a 7 would exit the random flashing by turning all relays on or off, but until then, it would stay in a loop? And lastly, for some odd reason case 3 doesn't let me turn off the relay when i send a 3 again, does anyone see a coding error i missed?

The 'random' portion of the code (case 5) is unstoppable unless I press the reset button.

Because you haven't put anything in the while loop to break out of it. You've been told repeatedly now that you need to do that.

And lastly, for some odd reason case 3 doesn't let me turn off the relay when i send a 3 again, does anyone see a coding error i missed?

You need to be pressing the key when the 3 arrives. That hardly seems likely.

Reading the state of the switches only when there is serial data to read, and it is some specific value is wrong. You need to read the state of the switches (all of them) independently of whether there is serial data, or what that value is.

Second, you really need to change the value in command only when there is serial data, but execute the switch statement on EVERY pass through loop.

Third, you need to read up on the difference between while and do/while. Where you have the do/while loop should be a while loop IF there needs to be one at all. I'm not convinced that there should be one there at all.