Logic Programming Problem

Hi,

I a am a newbie and trying to create a Sketch that would allow me to turn on and off two pins ( to LEDS) via the internet. Got the internet part all working, but my programing logic for the pins is only partially working. I can only turn on and off LED1 by its self, when I try to turn on LED2 (seperate from LED1) both LEDs come on, and when I turn off LED2, both LED 1 and LED 2 turn off. I am using a bunch of “if” statements and I know I must be doing something wrong in the use of the IF statement. Any help would be appreciated.

/////// Eithernet Control of Pin 4 and Pin 6
/////// Using LEDs to be replaced by relays
////// Turn Pin on and off indivigually

#include <SPI.h>
#include <Ethernet.h>

int led1 = 4; // LED connected to digital pin 4
int led2 = 6; // LED connected to digital pin 6

byte mac = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED }; //physical mac address
byte ip = { 192, 168,2,3 }; // ip in lan
byte gateway = { 24, 0, 95, 22 }; // internet access via router
byte subnet = { 255, 255, 255, 0 }; //subnet mask
EthernetServer server(8081); //server port

String readString;

//////////////////////

void setup(){

pinMode(led1, OUTPUT); //pins selected to control
pinMode(led2, OUTPUT);
//start Ethernet
Ethernet.begin(mac, ip, gateway, subnet);
server.begin();
//the pin for the servo co
//enable serial data print
Serial.begin(9600);
Serial.println(“server LED test 1.0”); // so I can keep track of what is loaded
}

void loop(){
// Create a client connection
EthernetClient client = server.available();
if (client) {
while (client.connected()) {
if (client.available()) {
char c = client.read();

//read char by char HTTP request
if (readString.length() < 100) {

//store characters to string
readString += c;
//Serial.print(c);
}

//if HTTP request has ended
if (c == ‘\n’) {

///////////////
Serial.println(readString); //print to serial monitor for debuging

client.println(“HTTP/1.1 200 OK”); //send new page
client.println(“Content-Type: text/html”);
client.println();

client.println("");
client.println("");
client.println("");
client.println("");
client.println("");
client.println(“LED Control”);
client.println("");
client.println("");
client.println(“

LED Remote Control/H1>”);
client.println("
");
client.println("
");

client.println("<a href="/?lighton"">Turn On LED");
client.println("<a href="/?lightoff"">Turn Off LED
“);
client.println(”
");

client.println("<a href="/?lighton2"">Turn On LED2");
client.println("<a href="/?lightoff2"">Turn Off LED2
");

client.println("");
client.println("");

delay(1);
//stopping client
client.stop();

////////////// /////////Pin Logic

///////////////////// control arduino pin 6

if(readString.indexOf("?lighton") >0)//checks for on

{digitalWrite(led1, HIGH); // set pin 6 high
Serial.println(“Led1 On”);
}
if(readString.indexOf("?lightoff") >0)//checks for off

{digitalWrite(led1, LOW); // set pin 6 low
Serial.println(“Led1 Off”);
}

///////////////////// control arduino pin 4
if(readString.indexOf("?lighton2") >0)//Checks for on
{
digitalWrite(led2, HIGH); // set pin 4 high
Serial.println(“Led2 On”);
}

if(readString.indexOf("?lightoff2")>0)//checks for off

{digitalWrite(led2, LOW); // set pin 4 low
Serial.println(“Led2 Off”);
}

//clearing string for next read
readString="";
}
}}}}

 }
    }}}}

Bad. Bad. Bad. Every } goes on a line by itself.

Now, look at your instructions. lighton and lighton2. Don't you see that the string lighton2 contains lighton? Can you think of a way yo fix the "problem"?

Hello and welcome :slight_smile:

I never used the String class, I can only suggest to replace it with standard char arrays and then you use strcmp for comparison, it's really not harder to work with...

But for your problem, I'm not sure but I think it would work if you place them in this order and use "else if":

if(readString.indexOf("?lighton2") >0)//Checks for on
{
    digitalWrite(led2, HIGH);    // set pin 4 high
    Serial.println("Led2 On");
}
          
else if(readString.indexOf("?lightoff2")>0)//checks for off
{
    digitalWrite(led2, LOW);    // set pin 4 low
    Serial.println("Led2 Off");
}

else if(readString.indexOf("?lighton") >0)//checks for on
{
    digitalWrite(led1, HIGH);    // set pin 6 high
    Serial.println("Led1 On");
}

else if(readString.indexOf("?lightoff") >0)//checks for off
{
    digitalWrite(led1, LOW);    // set pin 6 low
    Serial.println("Led1 Off");
}

And please next time indent your code correctly and put it in a code box :wink:

The problem is that if lighton2 is in the sentence, then index of is also going to find lighton. It's just lighton2 without the 2. So when you send lighton2, the first indexOf see's lighton and that part gets run then the indexOf(lightOn2) sees that is true in the exact same place just one letter longer, so run that part too.

Thanks to all,

Yup it was confused with Lighton vs Lighton2. Changed Lighton to Lighton1 and it worked like a charm.

New code below

client.println("<a href="/?lighton1"">Turn On LED");
client.println("<a href="/?lightoff1"">Turn Off LED
“);
client.println(”
");

client.println("<a href="/?lighton2"">Turn On LED2");
client.println("<a href="/?lightoff2"">Turn Off LED2
");

client.println("");
client.println("");

delay(1);
//stopping client
client.stop();
////////////// Pin Logic
///////////////////// control arduino pin 6
if(readString.indexOf("?lighton1") >0)//checks for on

{digitalWrite(led1, HIGH); // set pin 6 high
Serial.println(“Led1 On”);
}
if(readString.indexOf("?lightoff1") >0)//checks for off

{digitalWrite(led1, LOW); // set pin 6 low
Serial.println(“Led1 Off”);
}
///////////////////// control arduino pin 4
if(readString.indexOf("?lighton2") >0)//Checks for on
{
digitalWrite(led2, HIGH); // set pin 4 high
Serial.println(“Led2 On”);
}

if(readString.indexOf("?lightoff2")>0)//checks for off

{digitalWrite(led2, LOW); // set pin 4 low
Serial.println(“Led2 Off”);
}

//clearing string for next read
readString="";
}
}}}}

The next step is to understand how to name states as well as form items, so you get back led1=on, led2=off, etc.