help with remote relay project (formerly html button problem)

hi guys,
this project is at it's very start, the end result should be a network controlled Boiler (and maybe other devices in the future) with some advanced features.
i'm currently trying to get the basic functions to work before i start adding advanced ones.

the current setup is an arduino MEGA with an Ethernet Shield and a LED on pin 2 for testing.

the problem is turning the LED off. turning it on works fine, but in order to turn it off i need to click the button twice.
i looked at the HTTP requests via serial monitor and it seems that the LED changes only when the referrer is the "OFF" page. so when the LED is ON, one click brings up a page that seems like the "ON" page but has the "OFF" page's URL, only then the (second) button click will turn the LED off.

i suspect it has more to do with the HTML part then the Arduino code because that's the part i'm least familiar with (learning as i go) but i'm not sure.

what do you think?
thanks in advance!

my sketch:

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

byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
IPAddress ip(192,168,123,223); 
EthernetServer server(12345);  
String HTTP_req;         
boolean LED_status = false;  
void setup()
{
    Ethernet.begin(mac, ip); 
    server.begin();         
    Serial.begin(9600);   
    pinMode(2, OUTPUT);       
}
void loop()
{
    EthernetClient client = server.available(); 
    if (client) {  // got client?
        boolean currentLineIsBlank = true;
        while (client.connected()) {
            if (client.available()) { 
                char c = client.read(); 
                HTTP_req += c; 
                if (c == '\n' && currentLineIsBlank) {
                    Toggle();
                    client.println("HTTP/1.1 200 OK");
                    client.println("Content-Type: text/html");
                    client.println("Connection: close");
                    client.println();
                    client.println("<!DOCTYPE html>");
                    client.println("<html>");
                    client.println("<head>");
                    client.println("<title>Arduino Boiler Control</title>");
                    client.println("</head>");
                    client.println("<body>");            
                    client.println("<p>Click to switch Boiler on and off.</p>");
                    Button(client);
                    client.println("</form>");
                      if (LED_status){
                    client.println("<h1>Boiler is ON</h1>");
                    } else client.println("<h1>Boiler is OFF</h1>");
                    client.println("</body>");
                    client.println("</html>");
                    Serial.print(HTTP_req);
                    HTTP_req = "";
                    break;
                }
                if (c == '\n') {
                    currentLineIsBlank = true;
                } 
                else if (c != '\r') {
       
                    currentLineIsBlank = false;
                }
            } 
        } 
        delay(1); 
        client.stop(); 
    } 
}

void Toggle(){
     if (HTTP_req.indexOf("ON?") > -1) {
         LED_status = true;
         digitalWrite(2, HIGH);
     }
    else if (HTTP_req.indexOf("OFF?") > -1){
         LED_status = false;
         digitalWrite(2, LOW);
    } 
}

void Button(EthernetClient cl)
{
    if (LED_status) {
        cl.println("<form method=\"get\" action=\"/OFF\">");
        cl.println(" <button type=\"submit\">Turn OFF</button>");
    }
    else { 
       cl.println("<form method=\"get\" action=\"/ON\">");
       cl.println(" <button type=\"submit\">Turn ON</button>");  
  }
}

If a client sends a GET request, you call Toggle(), regardless of whether the request was an initial connection or a form submit request. Toggle() is a useless name, because that function does NOT toggle anything.

Why is the form open tag sent in Button() while the form close tag is sent in loop()? The entire form should be sent in the function.

Serial.print()ing the GET request would be a useful thing to do, as would showing us the output.

the "toggle" function is called that for lack of a better fitting name at the time. (you can notice a "LED_status" variable even though there's no LED in the project - these are just temporary names)
the "toggle" function wasn't originally there, it was originally a part of the "button" function and moved where as part of trial and error for fixing the discussed problem.

this is what i get when accessing the webpage for the first time :

GET / HTTP/1.1
Host: 192.168.123.223:12345
Connection: keep-alive
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3

GET /favicon.ico HTTP/1.1
Host: 192.168.123.223:12345
Connection: keep-alive
Accept: */*
User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3

first click on the ON button (LED turns ON) :

GET /ON? HTTP/1.1
Host: 192.168.123.223:12345
Connection: keep-alive
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31
Referer: http://192.168.123.223:12345/
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3

GET /favicon.ico HTTP/1.1
Host: 192.168.123.223:12345
Connection: keep-alive
Accept: */*
User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3

second click on the OFF button (LED stays ON) :

GET /OFF? HTTP/1.1
Host: 192.168.123.223:12345
Connection: keep-alive
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31
Referer: http://192.168.123.223:12345/ON?
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3

GET /favicon.ico HTTP/1.1
Host: 192.168.123.223:12345
Connection: keep-alive
Accept: */*
User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3

third click on the OFF button (LED turns OFF):

GET /OFF? HTTP/1.1
Host: 192.168.123.223:12345
Connection: keep-alive
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31
Referer: http://192.168.123.223:12345/OFF?
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3

GET /favicon.ico HTTP/1.1
Host: 192.168.123.223:12345
Connection: keep-alive
Accept: */*
User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3

I think you've already found the problem: the code in Toggle() is detecting the "ON?" contained in the Referer field, not in the request URL. Due to the precedence in Toggle(), it only detects "OFF?" when neither the URL nor the Referer contains "ON?". Since you don't seem to make any use of the rest of the HTTP request, perhaps you could throw away everything after the first line? Alternative look for a string which is unique to the URL field, such as "GET /ON?" etc.

simple server code that will turn a pin on/off.

//zoomkat 4-1-12
//simple button GET for servo and pin 5
//for use with IDE 1.0
//open serial monitor to see what the arduino receives
//use the \ slash to escape the " in the html, or use ' instead of " 
//address will look like http://192.168.1.102:84 when submited
//for use with W5100 based ethernet shields

#include <SPI.h>
#include <Ethernet.h>
#include <Servo.h> 
Servo myservo;  // create servo object to control a servo 

byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED }; //physical mac address
byte ip[] = { 192, 168, 1, 102 }; // ip in lan
byte gateway[] = { 192, 168, 1, 1 }; // internet access via router
byte subnet[] = { 255, 255, 255, 0 }; //subnet mask
EthernetServer server(84); //server port

String readString; 

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

void setup(){

  pinMode(5, OUTPUT); //pin selected to control
  //start Ethernet
  Ethernet.begin(mac, ip, gateway, gateway, subnet);
  server.begin();

  myservo.write(90); //set initial servo position if desired
  myservo.attach(7);  //the pin for the servo control
  //enable serial data print 
  Serial.begin(9600); 
  Serial.println("server servo/pin 5 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("<HTML>");
          client.println("<HEAD>");
          client.println("<TITLE>Arduino GET test page</TITLE>");
          client.println("</HEAD>");
          client.println("<BODY>");

          client.println("<H1>Zoomkat's simple Arduino button</H1>");
          
          client.println("<a href=\"/?on\">ON</a>"); 
          client.println("<a href=\"/?off\">OFF</a>"); 

          client.println("</BODY>");
          client.println("</HTML>");
 
          delay(1);
          //stopping client
          client.stop();

          ///////////////////// control arduino pin
          if(readString.indexOf("on") >0)//checks for on
          {
            myservo.write(40);
            digitalWrite(5, HIGH);    // set pin 5 high
            Serial.println("Led On");
          }
          if(readString.indexOf("off") >0)//checks for off
          {
            myservo.write(140);
            digitalWrite(5, LOW);    // set pin 5 low
            Serial.println("Led Off");
          }
          //clearing string for next read
          readString="";

        }
      }
    }
  }
}

PeterH,
looking only in the first line of the request works, Thanks. hopefully it won't restrict me when trying some advanced options (if so, i'll try the other method).

after solving the html button problem i need some assistance with a physical button problem. (i'll continue in this thread so as not to open multiple ones)

I've added an interrupt to toggle the LED at a button press. (I've never used interrupts, so any general advice is welcome)
the button is debounced via a R-C circuit and a schmitt-inverter (74hc14).

the problem is that after a few button presses (sometimes just one) the system seems to stall, not responding to button presses or html requests.
is there a chance that the button bounces still register, even though it doesn't show in the serial monitor? (i added a Serial.println inside the interrupt to see when the button is pressed).
what can cause this behavior?

this is the code:

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

byte mac[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
IPAddress ip(192,168,123,223); 
EthernetServer server(12345);  
String HTTP_req;         
volatile boolean LED_status = false;  

void setup()
{
    Ethernet.begin(mac, ip); 
    server.begin();         
    Serial.begin(9600);   
    pinMode(2, OUTPUT); 
    attachInterrupt(1, Toggle, RISING); 
}

void loop()
{
    EthernetClient client = server.available(); 
    if (client) {
        boolean currentLineIsBlank = true;
        while (client.connected()) {
            if (client.available()) { 
                char c = client.read(); 
                HTTP_req += c; 
                if (c == '\n') {
                    client.println("HTTP/1.1 200 OK");
                    client.println("Content-Type: text/html");
                    client.println("Connection: close");
                    client.println();
                    client.println("<!DOCTYPE html>");
                    client.println("<html>");
                    client.println("<head>");
                    client.println("<title>Arduino Boiler Control</title>");
                    client.println("</head>");
                    client.println("<body>");            
                    client.println("<p>Click to switch Boiler on and off.</p>");
                    readGET(); //identify the request
                    Button(client); //display button according to current status
                    executeLED(); //change pin according to request
                    client.println("</form>");
                    if (LED_status){ //display status message according to current status
                    client.println("<h1>Boiler is ON</h1>");
                    } else client.println("<h1>Boiler is OFF</h1>");
                    client.println("</body>");
                    client.println("</html>");
                    Serial.print(HTTP_req);
                    HTTP_req = "";
                    break;
                }
                if (c == '\n') {
                    currentLineIsBlank = true;
                }else if (c != '\r') {
                    currentLineIsBlank = false;
                      }
            } 
        } 
        delay(1); 
        client.stop(); 
    } 
}

void Toggle(){ //interrupt function - toggles the pin and status indicator
Serial.println("POP"); //for debugging, to see if button bounces
LED_status=!LED_status;
executeLED();
}

void readGET(){
     if (HTTP_req.indexOf("ON?") > -1) {
         LED_status = true;
     }else if (HTTP_req.indexOf("OFF?") > -1){
         LED_status = false;
    } 
}

void executeLED(){
  if (LED_status) {
    digitalWrite(2, HIGH);
  }else digitalWrite(2, LOW);
}

void Button(EthernetClient cl)
{
    if (LED_status) {
        cl.println("<form method=\"get\" action=\"/OFF\">");
        cl.println(" <button type=\"submit\">Turn OFF</button>");
    }else { 
       cl.println("<form method=\"get\" action=\"/ON\">");
       cl.println(" <button type=\"submit\">Turn ON</button>");  
  }
}

It is inadvisable to print from within an interrupt routine.

Here's why: draining the serial port is also done at interrupt time, and interrupts are off during your interrupt routine. If you call Serial.print() and it happens to be the call that overflows the internal 64-byte buffer, the print() will hang waiting for buffer space to become available. (Precisely the symptom you describe.)

-br

I've added an interrupt to toggle the LED at a button press. (I've never used interrupts, so any general advice is welcome)

Why? Isn't polling the switch good enough?

                    readGET(); //identify the request

You are in the middle of sending a response to a GET request. Does it make sense to now read what the client wanted?

Serial.print() in an ISR is a really bad idea. Don't do it, unless you are willing to risk the Arduino locking up.

the problem is that after a few button presses (sometimes just one) the system seems to stall, not responding to button presses or html requests.

I'm not surprised.

Haha, in trying to monitor and debug i actually caused bugs myself :fearful:
thanks. it seems to work fine now but i think bounces still register, i guess that's a topic for the hardware forum.

i moved the readGET() function to a point before the transmission starts. thanks.

as to why use interrupts? there are going to be more triggers and i thought it was the way to go, i might be mistaken. i think interrupts are more elegant and give you more flexibility, am i wrong?