link sends user to code page?

Hi everyone,
I'm making a web based sprinkler control system using an Arduino UNO R3, and ethernet shield, and an 8-relay board. It's all working fine and I have some code written up which was working great until now. I have a button that turns all the relays on, and a button that turns all relays off. I also have buttons for each individual relay to turn on or off. Out of nowhere, when I was testing the program, I clicked on the "turn all on" button and it turns all the relays on (like its supposed to do) but right after that it redirects me to a page of HTML code. It looks like this:

ton>Turn On</button><a>

Sprinkler 2 is <b><font color=red></b>OFF</font>
<a href="./?PIN3=T"><button>Turn On</button><a>

Sprinkler 3 is <b><font color=red></b>OFF</font>
<a href="./?PIN4=T"><button>Turn On</button><a>

Sprinkler 4 is <b><font color=red></b>OFF</font>
<a href="./?PIN5=T"><button>Turn On</button><a>

Sprinkler 5 is <b><font color=red></b>OFF</font>
<a href="./?PIN6=T"><button>Turn On</button><a>

Sprinkler 6 is <b><font color=red></b>OFF</font>
<a href="./?PIN7=T"><button>Turn On</button><a>

Sprinkler 7 is <b><font color=red></b>OFF</font>
<a href="./?PIN8=T"><button>Turn On</button><a>

Sprinkler 8 is <b><font color=red></b>OFF</font>
<a href="./?PIN9=T"><button>Turn On</button><a>





Sprinkler 1 is <b><font color=red></b>OFF</font>
<a href="./?
PIN2=T
"><button>Turn On</button><a>

<<a href=HTTP/1.1 200 OK
Content-Type: text/html

<title>SprinkDuino</title>
<h1 color=red style=text-align:center;>Welcome to SprinkDuino!</h1>



<a href="./?PINA=T">TURN ALL ON<a>



<a href="./?PINA=F">TURN ALL OFF<a>



Sprinkler 1 is <b><font color=green></b>ON</font>
<a href="./?PIN2=F"><button>Turn Off</button><a>

Sprinkler 2 is <b><font color=red></b>OFF</font>
<a href="./?PIN3=T"><button>Turn On</button><a>

Sprinkler 3 is <b><font color=red></b>OFF</font>
<a href="./?PIN4=T"><button>Turn On</button><a>

Sprinkler 4 is <b><font color=red></b>OFF</font>
<a href="./?PIN5=T"><button>Turn On</button><a>

Sprinkler 5 is <b><font color=red></b>OFF</font>
<a href="./?PIN6=T"><button>Turn On</button><a>

Sprinkler 6 is <b><font color=red></b>OFF</font>
<a href="./?PIN7=T"><button>Turn On</button><a>

Sprinkler 7 is <b><font color=red></b>OFF</font>
<a href="./?PIN8=T"><button>Turn On</button><a>

Sprinkler 8 is <b><font color=red></b>OFF</font>
<a href="./?PIN9=T"><button>Turn On</button><a>

FF</font>
<a href="./?PIN2=T"><button>Turn On</button><a>

Sprinkler 2 is <b><font color=red></b>OFF</font>
<a href="./?PIN3=T"><button>Turn On</button><a>

Sprinkler 3 is <b><font color=red></b>OFF</font>
<a href="./?PIN4=T"><button>Turn On</button><a>

Sprinkler 4 is <b><font color=red></b>OFF</font>
<a href="./?PIN5=T"><button>Turn On</button><a>

Sprinkler 5 is <b><font color=red></b>OFF</font>
<a href="./?PIN6=T"><button>Turn On</button><a>

Sprinkler 6 is <b><font color=red></b>OFF</font>
<a href="./?PIN7=T"><button>Turn On</button><a>

Sprinkler 7 is <b><font color=red></b>OFF</font>
<a href="./?PIN8=T"><button>Turn On</button><a>

Sprinkler 8 is <b><font color=red></b>OFF</font>
<a href="./?PIN9=T"><button>Turn On</button><a>





Sprinkler 1 is <b><font color=red></b>OFF</font>
<a href="./?
PIN2=T
"><button>Turn On</button><a>

<<a href=HTTP/1.1 200 OK
Content-Type: text/html
....
....
....

...and its pretty long. I'm guessing this is the output code from the Arduino, but I can't figure out whats going wrong. I didn't change any code between the time it was working and started doing this. Hopefully one of you has some idea of what's going on!

Here's my program code:

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

// Enter a MAC address and IP address for your controller below.
// The IP address will be dependent on your local network:
byte mac[] = { 0x90, 0xA2, 0xDA, 0x0D, 0x98, 0x26 };
IPAddress ip(192,168,1,134);

// Initialize the Ethernet server library
// with the IP address and port you want to use 
// (port 80 is default for HTTP):
EthernetServer server(80);

int pin[8] = {2,3,4,5,6,7,8,9};
String readString = String(30);
String state[8] = String(3);
String pinOn[8] = String(6);
String pinOff[8] = String(6);

void setup()
{
  // start the Ethernet connection and the server:
  Ethernet.begin(mac, ip);
  server.begin();
  
  //Sets relay pins (2-9) as output
  for(int i = 2; i < 10; i++){
   pinMode(i, OUTPUT); 
  }
  for(int i = 2; i < 10; i++){
   digitalWrite(i, HIGH); 
   state[i-2] = "OFF";
  }
  pinOn[0] = "PIN2=T";
  pinOn[1] = "PIN3=T";
  pinOn[2] = "PIN4=T";
  pinOn[3] = "PIN5=T";
  pinOn[4] = "PIN6=T";
  pinOn[5] = "PIN7=T";
  pinOn[6] = "PIN8=T";
  pinOn[7] = "PIN9=T";
  
  pinOff[0] = "PIN2=F";
  pinOff[1] = "PIN3=F";
  pinOff[2] = "PIN4=F";
  pinOff[3] = "PIN5=F";
  pinOff[4] = "PIN6=F";
  pinOff[5] = "PIN7=F";
  pinOff[6] = "PIN8=F";
  pinOff[7] = "PIN9=F";
}

void loop()
{
  // listen for incoming clients
  EthernetClient client = server.available();
  if (client) {
    // an http request ends with a blank line
    boolean currentLineIsBlank = true;
    while (client.connected()) {
      if (client.available()) {
        char c = client.read();
        // if you've gotten to the end of the line (received a newline
        // character) and the line is blank, the http request has ended,
        // so you can send a reply

        if (readString.length() < 30) {
          readString.concat(c);
        }

        if (c == '\n' && currentLineIsBlank) {
          // send a standard http response header
          int PIN = readString.indexOf("PIN");
          
          for(int i = 0; i < 8; i++){
            if (readString.substring(PIN,PIN+6) == pinOn[i]) {
              digitalWrite(pin[i], LOW);           
              state[i] = "ON";
            }
            else if (readString.substring(PIN,PIN+6) == pinOff[i]) {
              digitalWrite(pin[i], HIGH);
              state[i] = "OFF";
            } 
          }
          if(readString.substring(PIN,PIN+6) == "PINA=F") {
            for(int i = 0; i < 8; i++){
             digitalWrite(pin[i], HIGH);
             state[i] = "OFF";
            }
          }
          else if(readString.substring(PIN,PIN+6) == "PINA=T") {
            for(int i = 0; i < 8; i++){
             digitalWrite(pin[i], LOW);
             state[i] = "ON"; 
            }
          }
          client.println("HTTP/1.1 200 OK");
          client.println("Content-Type: text/html");
          client.println();
          client.println("<title>SprinkDuino</title>");
          client.println("<h1 color=red style=text-align:center;>Welcome to SprinkDuino!</h1>");
          client.println("

");
          
          client.println("<a href=\"./?PINA=T\"><button>TURN ALL ON</button><a>");
          client.println("

");
          client.println("<a href=\"./?PINA=F\"><button>TURN ALL OFF</button><a>");
          client.println("

");
          
          for(int i = 2; i < 10; i++){
           client.print("Sprinkler ");
           client.print(i-1);
           client.print(" is "); 
           if(state[i-2] == "ON"){
             client.print("<b><font color=green></b>");
             client.print(state[i-2]);
             client.print("</font>");
             client.print("
");
           }
           else if(state[i-2] == "OFF"){
             client.print("<b><font color=red></b>");
             client.print(state[i-2]);
             client.print("</font>");
             client.print("
"); 
           }
           if (state[i-2] == "ON") {
              client.print("<a href=\"./?");
              client.print(pinOff[i-2]);
              client.print("\"><button>Turn Off</button><a>");
            }
            else {
              client.print("<a href=\"./?");
              client.print(pinOn[i-2]);
              client.print("\"><button>Turn On</button><a>");
            }
           client.print("

");
          }
          break;
        }
        if (c == '\n') {
          // you're starting a new line
          currentLineIsBlank = true;
        } 
        else if (c != '\r') {
          // you've gotten a character on the current line
          currentLineIsBlank = false;
        }
      }
    }
    // give the web browser time to receive the data
    delay(1);
    readString = "";
    // close the connection:
    client.stop();
  }
}

Thanks in advance!
-Ryan

Alright, I had a microSD card in the ethernet shield but didn't program the Arduino to use it (I just put it there for future use). I took it out and now my program works fine! Hmmm...

Please note that, at present, the String library has bugs as discussed here and here.

In particular, the dynamic memory allocation used by the String class may fail and cause random crashes.

I recommend reworking your code to manage without String. Use C-style strings instead (strcpy, strcat, strcmp, etc.).

Also you may want to keep the strings in program memory, eg. change:

           client.print("Sprinkler ");

to:

           client.print(F("Sprinkler "));

And so on for all the numerous other places.

Thanks, I'll definitely change that! Is there supposed to be a fix for that soon?

We are begging them to fix it. However no joy yet.

Even if there were a fix, it's still a risky proposition to use dynamic allocation with so little SRAM. Heap fragmentation may well bite you, leading to bugs that don't manifest for quite a while after the sketch has started and debugging such things would be tricky and frustrating.

debugging such things would be tricky and frustrating.

Not to mention unnecessary. Static char arrays work just fine for 99.999% of the stuff that people use String for.

I haven't used C in a while since most things I do are in C++, would these be appropriate conversions?

int pin[8] = {2,3,4,5,6,7,8,9};
char readString;
char *state[8];
char *pinOn[8];
char *pinOff[8];
char *pinTime[8];

I keep getting an error at this line:

        if (sizeof(readString) < 30) {
          strcat(readString, c);

Am I using the correct functions to get the length and for concatenation? It's saying "Invalid conversion from 'char' to '*char' " when it gets to the concatenation.

EDIT:Also, should I do this every time I print something, even if its just HTML code?:

client.print(F("Sprinkler "));

Thanks!

char readString;

readString is one char. That's a pretty short string...

        if (sizeof(readString) < 30) {
          strcat(readString, c);

The sizeof() function tells you how big an array (or scalar) variable is, not how much it currently holds. For char arrays that hold string, strlen() tells you that. Of course, you have to have an array in the first place.

char *state[8];
char *pinOn[8];
char *pinOff[8];
char *pinTime[8];

Arrays of pointers are probably NOT what you want.

Also, should I do this every time I print something, even if its just HTML code?:

That's not HTML code. It's text. Plain and simple. The fact that some other process assigns special meaning to the text is irrelevant. Constant text being output should be wrapped by the F() macro.

Sorry, I was l little confused...well I still am lol.
So then the declaration should look like this, correct?:

static char readString[30];
static char state[8];
static char pinOn[8];
static char pinOff[8];

What I don't get about this is how can I make a static char array that holds a word in each index? The way I have it means that 'state' is going to be a character array that holds 8 single charactors, doesn't it? Would I have to make a two dimensional array for each index to hold 5 characters such as "hello"?

What I don't get about this is how can I make a static char array that holds a word in each index?

For that you need either a 2D array of chars or an array of pointers.

If you go with the array of pointers, keep in mind that you must, at some point, make those pointers point to something BEFORE you try to store a word where the pointer points.

Do you mean I need to declare the variable and initialize it with something other than the words?

static char *pinOn[8] = {"PIN2=T","PIN3=T","PIN4=T","PIN5=T","PIN6=T","PIN7=T","PIN8=T","PIN9=T"};

^^So this wouldn't work?

I don't have much experience with 2D arrays...would you recommend it over using an array of pointers?

^^So this wouldn't work?

Yes, it would. The memory that the pointer points to is being allocated as part of the variable declaration because you provided initializers.

char *ptrs[5];
strcat(ptrs[0],"Hello");
strcat(ptrs[1],"World");
strcat(ptrs[2],"FAIL!")'

would not. These pointers do not point to anywhere. Trying to write data at the memory location that they point to will fail.

Ahh alright that makes sense now, thanks! I just have a couple other quick questions regarding C-style strings...

I changed this:

if (readString.length() < 30) {
          readString.concat(c);
        }

to this:

if (strlen(readString) < 30) {
          strcat(readString, &c);
        }

What I want is to concatenate the char c onto readString but it gave me errors unless I wrote it as the address of c, which isn't going to do what i want is it?
EDIT: I figured out I could just make a new char array and concatenate it as follows:

        char c = client.read();
        char newC[2];
        newC[0] = c;
        newC[1] = '\0';
        // if you've gotten to the end of the line (received a newline
        // character) and the line is blank, the http request has ended,
        // so you can send a reply

        if (strlen(readString) < 30) {
          strcat(readString, newC);
        }

^^This should be fine, right?

I changed this:

int PIN = readString.indexOf("PIN");

to this:

int PIN = *strrchr(readString, 'PIN');

It is supposed to set PIN to the index of where "PIN" is in the readString (which is actually a char array now, not a string). I looked up how to do this with C strings and this is the only function I thought would work.

And this:

          for(int i = 0; i < 8; i++){
            if (readString.substring(PIN,PIN+6) == pinOn[i]) {          //Turns current pin on if strings match
              digitalWrite(pin[i], LOW);           
              state[i] = "ON";
            }

to this:

          for(int i = 0; i < 8; i++){
            if (substr(PIN,PIN+6,readString) == pinOn[i]) {          //Turns current pin on if strings match
              digitalWrite(pin[i], LOW);           
              state[i] = "ON";
            }

Which is supposed to take a substring of readString starting at the index of PIN, and is six characters long (or should it be seven to include the null character?) then compare it to another string.

Thanks for your help, I've been working on these for hours and can't seem to get them right. I'm sure some of it has to do with pointers, which I always get a little confused by.

        char c = client.read();
        char newC[2];
        newC[0] = c;
        newC[1] = '\0';

That's a roundabout way of doing it. Assuming you declare readstring as an array:

char readstring [50];

All you need is the current size in a variable, eg.

int readlength;

Each time you get a new character, if the string isn't full, you put it at the end, and add a 0x00 byte, eg.

readstring [readlength++] = c;  // append to string, increment count
readstring [readlength] = 0;  // terminating null

^^This should be fine, right?

It's OK. It's far more complicated than it needs to be.

int pos = strlen(readString);
if(pos < 30)
{
   readString[pos++] = c;
   readString[pos] = '\0';
}

Does the same thing, in less code.

I changed this:...to this:...It is supposed to set PIN to the index of where "PIN" is in the readString

There are several problems with the changed code. First the character 'PIN' will never appear in readString. The string "PIN" might. The output of strrchr() is a pointer to where the last occurrence of a character is in a string. That pointer could be subtracted from a pointer to the first character of the string, to get the relative position. It should not be used in the way you are trying to use it.

The strrchr() function is not the correct function to be using to locate a string in a string. The strstr() function is.

Frankly, it's hard to help you with general parsing questions. Seeing the string(s) you want to parse, and helping parse those specific strings would be far easier.

You might want to look at the strtok function.

static char readString[30];

...

int pos = strlen(readString);

if(pos < 30)
{
  readString[pos++] = c;
  readString[pos] = '\0';
}

Better check for < 29 in that case. The null takes a position, so you have room for 29 characters.

PaulS:
Frankly, it's hard to help you with general parsing questions. Seeing the string(s) you want to parse, and helping parse those specific strings would be far easier.

I based my program off the example Arduino server code, and to be honest I'm actually still confused on that part. Here is my program(still a little messy and unfinished):

#include <SPI.h>
#include <Ethernet.h>
#include <LiquidCrystal.h>
#include <Time.h>
#include <TimeAlarms.h>

byte mac[] = { 0x90, 0xA2, 0xDA, 0x0D, 0x98, 0x26 };
IPAddress ip(192,168,1,134);
// Initialize the Ethernet server library
// with the IP address and port you want to use 
// (port 80 is default for HTTP):
EthernetServer server(80);

int pin[8] = {2,3,4,5,6,7,8,9};
static char readString[30];
static char *state[8] = {"OFF","OFF","OFF","OFF","OFF","OFF","OFF","OFF",};
static char *pinOn[8] = {"PIN2=T","PIN3=T","PIN4=T","PIN5=T","PIN6=T","PIN7=T","PIN8=T","PIN9=T"};
static char *pinOff[8] = {"PIN2=F","PIN3=F","PIN4=F","PIN5=F","PIN6=F","PIN7=F","PIN8=F","PIN9=F"};
/* LCD */
LiquidCrystal lcd(14,15,16,17,18,19);
int backLight = 1;
void setup()
{
  // start the Ethernet connection and the server:
  Ethernet.begin(mac, ip);
  server.begin();
  
  //Time setup
  setTime(7,30,0,11,22,12);
  
  //LCD setup
  pinMode(backLight, OUTPUT);
  digitalWrite(backLight, HIGH);
  lcd.begin(20, 4);
  lcd.clear();
  lcd.setCursor(0,0);
  lcd.print(F("SprinkDuino"));
  delay(5000);
  lcd.clear();
  
  //Sets relay pins (2-9) as output
  for(int i = 2; i < 10; i++){
   pinMode(i, OUTPUT); 
  }
  for(int i = 2; i < 10; i++){
   digitalWrite(i, HIGH);          //HIGH is off
  }
}

void loop()
{
  digitalClockDisplay();
  Alarm.delay(1000);
  
  EthernetClient client = server.available();                // listen for incoming clients
  if (client) {
    boolean currentLineIsBlank = true;                       // an http request ends with a blank line
    while (client.connected()) {
      if (client.available()) {
        
        char c = client.read();
        int pos = strlen(readString);
        if(pos < 29)
        {
           readString[pos++] = c;
           readString[pos] = '\0';
        }
        if (c == '\n' && currentLineIsBlank) {          
          for(int i = 0; i < 8; i++){
            if (strstr(readString, "PIN") == pinOn[i]) {          //Turns current pin on
              digitalWrite(pin[i], LOW);           
              state[i] = "ON";
            }
            else if (strstr(readString, "PIN") == pinOff[i]) {    //Turns current pin off
              digitalWrite(pin[i], HIGH);
              state[i] = "OFF";
            } 
          }
          if(strstr(readString, "PIN") == "PINA=F") {              //Turns all pins off
            for(int i = 0; i < 8; i++){
             digitalWrite(pin[i], HIGH);
             state[i] = "OFF";
            }
          }
          else if(strstr(readString, "PIN") == "PINA=T") {       //Turns all pins on
            for(int i = 0; i < 8; i++){
             digitalWrite(pin[i], LOW);
             state[i] = "ON"; 
            }
          }
          }
          client.println(F("HTTP/1.1 200 OK"));
          client.println(F("Content-Type: text/html"));
          client.println();
          client.println(F("<title>SprinkDuino</title>"));
          client.println(F("<h1 color=red style=text-align:center;>Welcome to SprinkDuino!</h1>"));
          client.print(F("<table border=0><tr>"));
          client.println(F("<td><a href=\"./?PINA=T\"><button><font size=4>TURN ALL <font color=green>ON</font></font></button><a></td>"));
          client.println(F("<td><a href=\"./?PINA=F\"><button><font size=4>TURN ALL <font color=red>OFF</font></font></button><a></td>"));
          client.println(F("<tr>"));
          
          for(int i = 2; i < 10; i++){                    //Prints state of sprinkler pin
           client.print(F("<td>"));
           client.print(F("<font size=4>Sprinkler "));
           client.print(i-1);
           client.print(F(" is ")); 
           if(state[i-2] == "ON"){
             client.print(F("<b><font color=green>"));
             client.print(state[i-2]);
             client.print(F("</font></b></font>"));
             client.print(F("</td>"));
           }
           else {
             client.print(F("<b><font color=red>"));
             client.print(state[i-2]);
             client.print(F("</font></b></font>"));
             client.print(F("</td>"));
           }
           if (state[i-2] == "ON") {                      //Turns sprinkler pin on or off
              client.print(F("<td>"));
              client.print(F("<a href=\"./?"));
              client.print(pinOff[i-2]);
              client.print(F("\"><button>Turn Off</button><a>"));
              client.print(F("</td>"));
              client.print(F("<tr>"));
            }
            else {
              client.print(F("<td>"));
              client.print(F("<a href=\"./?"));
              client.print(pinOn[i-2]);
              client.print(F("\"><button>Turn On</button><a>"));
              client.print(F("</td>"));
              client.print(F("<tr>"));
            }          
          client.print(F("</table>"));
          break;
        }
        if (c == '\n') {
          // you're starting a new line
          currentLineIsBlank = true;
        } 
        else if (c != '\r') {
          // you've gotten a character on the current line
          currentLineIsBlank = false;
        }
      }
    }
    // give the web browser time to receive the data
    delay(1);
    readString[0] = '\0';
    // close the connection:
    client.stop();
  }
}

void digitalClockDisplay()
{
  lcd.clear();
  lcd.setCursor(0,0);
  lcd.print(F("Time"));
  lcd.setCursor(0,1);
  lcd.print(hour());
  printDigits(minute());
  printDigits(second());
}

void printDigits(int digits)
{
  lcd.print(F(":"));
  if(digits < 10)
    lcd.print('0');
  lcd.print(digits);
}

So if you want pin 2 to turn on, you would click the button which appends "PIN2=T" onto the end of the URL. readString is then set to PIN2=T and compared to pinOn[0]. If it is true pin 2 is set to LOW, turning the relay on. I just don't fully get what 'c' is doing...I'm guessing since it is expected to be the new line character '\n', once you press the button the program will interpret the URL and set the readString accordingly?

Sorry to be jumping around to different topics, but the program is now crashing similar to how it was previously. When I open up the page, it repeats the following over and over (with a few buttons that I couldn't paste here):

Welcome to SprinkDuino!
	
Sprinkler 1 is OFF	
HTTP/1.1 200 OK Content-Type: text/html

Since I'm not using strings anymore, I'm guessing it's some kind of memory leak? Could it also be because I'm trying to do too much formatting with the HTML?

            if (strstr(readString, "PIN") == pinOn[i]) {          //Turns current pin on

This is comparing addresses, not what they point to.

More like:

            if (strcmp (strstr(readString, "PIN"), pinOn[i]) == 0) {          //Turns current pin on

You could simplify your parsing quite a bit. Make the name a number, like "2", not a value like "PIN2"
Make the value a string like "T;" or "F;", instead of "T" or "F".

Then, the URL will contain something like "?2=T;". You can store the whole server response in the char array.

Then, use strtok(), with ? as the delimiter. Throw that value away. Call strtok() again with NULL as the string to parse and "=" as the delimiter. The token will be "2". That's easy to turn into a number. Do so. Use that number as the array index

Then, call strtok() again with NULL and ";". The token will be "T" or "F". Use that to decide what to do with the known pin number.