reducing program size

hello, i am trying to use enc28j60 with arduino so i came across this code which is most suitable for my project

// A simple web server to turn 2 LED on or off

#include "etherShield.h"
#include "ETHER_28J60.h"

int outputPin = 6; // LED1 to pin 6
int anotherOutputPin = 7; // LED2 to pin 7

static uint8_t mac[6] = {
  0x54, 0x55, 0x58, 0x10, 0x00, 0x24};   // this just needs to be unique for your network, 

static uint8_t ip[4] = {
  192, 168, 1, 15}; // IP address for the webserver

static uint16_t port = 80; // Use port 80 - the standard for HTTP

ETHER_28J60 e;

void setup()
{ 
  e.setup(mac, ip, port);
  pinMode(outputPin, OUTPUT);
  pinMode(anotherOutputPin, OUTPUT);
}

void loop()
{
  char* params;
  if (params = e.serviceRequest())
  {
    e.print("<h1><a href='/?led1=off&led2=off'>Arduino Web Remote</a></h1>");
    if (strcmp(params, "?led1=on&led2=off") == 0)
    {
      digitalWrite(outputPin, HIGH);
      digitalWrite(anotherOutputPin, LOW);
      e.print("<a href='?led1=off&led2=off'><button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED1 IS ON</button></a><a href='?led1=on&led2=on'>
<button style='border: 1px solid #000; border-left: 10px solid #000' type='button'>LED2 IS OFF</button></a>");
    }
    else if (strcmp(params, "?led1=off&led2=on") == 0)
    {
      digitalWrite(outputPin, LOW);
      digitalWrite(anotherOutputPin, HIGH);
      e.print("<a href='?led1=on&led2=on'><button style='border: 1px solid #000; border-left: 10px solid #000' type='button'>LED1 IS OFF</button></a><a href='?led1=off&led2=off'>
<button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED2 IS ON</button></a>");
    }
    else if (strcmp(params, "?led1=off&led2=off") == 0)
    {
      digitalWrite(outputPin, LOW);
      digitalWrite(anotherOutputPin, LOW);
      e.print("<a href='?led1=on&led2=off'><button style='border: 1px solid #000; border-left: 10px solid #000' type='button'>LED1 IS OFF</button></a><a href='?led1=off&led2=on'>
<button style='border: 1px solid #000; border-left: 10px solid #000' type='button'>LED2 IS OFF</button></a>");
    }
    else if (strcmp(params, "?led1=on&led2=on") == 0)
    {
      digitalWrite(outputPin, HIGH);
      digitalWrite(anotherOutputPin, HIGH);
      e.print("<a href='?led1=off&led2=on'><button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED1 IS ON</button></a><a href='?led1=on&led2=off'>
<button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED2 IS ON</button></a>");
    }
    e.respond();
  }
}

for two leds it has 2x2=4 combinations.
i want to control 5 leds so using else ifs i can actually extend the code but for 5 leds using the else if's it would take 5x5= 25 loops and the program size would be very big..

so i am trying to make it as a function so i can pass 5 parameters, the state of all the 5 leds ie on or off
the problem is it has bit of html and i am not sure how to proceed so if any one is kind enough please guide me to build a 5 led webserver using the code :slight_smile:

Does it have to use parameters such as:
?led1=off&led2=off

Would it not be far easier to have the webserver use this sort of format:
?leds=00010

Where a 1 represents that an led in the given position is on, and a 0 means off.

That way it would just be a case of

#include "etherShield.h"
#include "ETHER_28J60.h"

static uint8_t mac[6] = {
  0x54, 0x55, 0x58, 0x10, 0x00, 0x24};   // this just needs to be unique for your network, 

static uint8_t ip[4] = {
  192, 168, 1, 15}; // IP address for the webserver

static uint16_t port = 80; // Use port 80 - the standard for HTTP

ETHER_28J60 e;

#define MAXLEDS 5
byte outputPins[MAXLEDS] = {2,3,4,5,6}; //This would use digital2 to digital6, but you could choose any
void setup(){

  e.setup(mac, ip, port);

  for(byte led = 0; led < 8; led++){
    pinMode(led,OUTPUT);
  }
}

void loop()
{
  char* params;
  if (params = e.serviceRequest())
  { 
    int i = strcspn (params,"01"); //Find the first '1' or '0' in the parameters string
    byte led = 0;
    while (params[i] == '0' || params[i] == '1'){
      //keep counting through params while it is a 1 or a 0.
      
      digitalWrite(led, (params[i] == '1') ? HIGH : LOW); //Set the corresponding LED to high or low depending on what was sent
      led++; //Next LED.
      if(led >= MAXLEDS ){
        break; //Run out of LEDs to change, so finish loop.
      }
      i++; //move to next character in the parameters
    }
  }
}

You'd still have to figure out how to send back the correct HTML code for the buttons, but you could compile the HTML string during the while loop and then send it at the end.

You could write a simple string parser to look for "led = <on|off>", and by extracting the number x and the state (on or off), you could cope with very many LEDs

Rule of thumb,

Anytime you see a group of similarly named variables with incremental numeric suffixes like Led1, Led2, Led3, Led4, that is a strong indication that you should convert that group of variables into a single array like Led[4], thus allowing you to eliminate repetitious/redundant code.

thank you guys, i will use array and i am gonna reduce the parameters to ?led=01000

i will try to understand tc worlds program but in meanwhile i got this idea

if (strcmp(params, "?led=10") == 0)
    {
     function(1,0);
    }

function(boolean l1, boolean l2)
{
  digitalWrite(led[1], l1);
  digitalWrite(led[2], l2);
  e.print("<a href='?led=00'><button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED1 IS ON</button></a><a href='?led=11'>
<button style='border: 1px solid #000; border-left: 10px solid #000' type='button'>LED2 IS OFF</button></a>");
}

the problem is with e.print, i don't know how to introduce it into a function...
now is there any way to make arduino recognize and print the state stored in the boolean???? i mean instead of the ON and OFF can i type something like l1 and it will print value as ON if boolean is true and OFF when boolean is false???

You could try something like this:

void yourFunctionName(boolean l1, boolean l2)
{
  digitalWrite(led[1], l1);
  digitalWrite(led[2], l2);
  char on[] = "ON";
  char off[] = "OFF";
  char string[270] = {0};
  sprintf(string,"<a href='?led=00'><button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED1 IS %s</button></a><a href='?led=11'>
<button style='border: 1px solid #000; border-left: 10px solid #000' type='button'>LED2 IS %s</button></a>",  (l1?on:off),(l2?on:off));
  e.print(string);
}

Note, sprintf() is used to create c strings from other variables or literal strings.
So by using the %s type, it will replace the %s by a given cstring. In this case they are used like: LED1 IS %s
Then by using the conditional (l1?on:off), it will replace the %s by the string stored in the variable on[] if l1 is high, or with off[] if it is low.

Though there is probably a more efficient way of doing it.

thanks tc world
On off is settled but in the parameters it would still print the same thing and not work as a function, so i am trying to replace the with strings

void yourFunctionName(boolean l1, boolean l2)
{
boolean le1=l1;
boolean le2=l2;
  digitalWrite(led[1], l1);
  digitalWrite(led[2], l2);
  char on[] = "ON";
  char off[] = "OFF";
  char string[270] = {0};
  sprintf(string,"<a href='?led=%s%s'><button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED1 IS %s</button></a><a href='?led=%s%s'>
<button style='border: 1px solid #000; border-left: 10px solid #000' type='button'>LED2 IS %s</button></a>",(le1?1:0),(le2?1:0),(l1?on:off),(l2?on:off));
  e.print(string);
}

so by using above code i guess it would print both the parameters as ?led=10 and ?led=10, but i have to print the opposite of what is stored in the string then what do i do?? i mean instead of %s if i use a string which i call it %os and code would be

sprintf(string,"LED1 IS %s
LED2 IS %s
",(le1?1:0),(le2?1:0),(l1?on:off),(l2?on:off));

then i guess it would work fine rite? so how to implement this what i am calling as %os which prints the opposite of what is stored:)

No, that wouldn't work.

What it does is parse through the string. The first % takes the first value, the next % the next value, and so on.

You can use things like:

%d is an integer
%c is a char
%f is a float
%s is a string.

Example:
sprintf(buffer,"This is a decimal:%d, this a char:%c, this a string:%s", 10,'1',"ON");
Would result in:
"This is a decimal:10, this a char:1, this a string:ON"

Oh right, I understand what is supposed to happen now:

void yourFunctionName(boolean l1, boolean l2)
{
  digitalWrite(led[1], l1);
  digitalWrite(led[2], l2);
  char on[] = "ON";
  char off[] = "OFF";
  char string[270] = {0};
  sprintf(string,"<a href='?led=%d%d'><button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED1 IS %s</button></a><a href='?led=%d%d'>
<button style='border: 1px solid #000; border-left: 10px solid #000' type='button'>LED2 IS %s</button></a>", !l1,l2,(l1?on:off),l1,!l2,(l2?on:off));
  e.print(string);
}

See my last post.

Here was some quick test code:

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
  yourFunctionName(false,false);
  yourFunctionName(true,false);
  yourFunctionName(true,true);
  yourFunctionName(false,true);
}

void loop() {
  // put your main code here, to run repeatedly: 
  
}

void yourFunctionName(boolean l1, boolean l2)
{
  char on[] = "ON";
  char off[] = "OFF";
  char string[270] = {0};
  sprintf(string,"<a href='?led=%d%d'><button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED1 IS %s</button></a><a href='?led=%d%d'>
<button style='border: 1px solid #000; border-left: 10px solid #000' type='button'>LED2 IS %s</button></a>", !l1,l2,(l1?on:off),l1,!l2,(l2?on:off));
  Serial.println(string);
}

This is the result from the Serial monitor:

LED1 IS OFF
LED2 IS OFF

LED1 IS ON
LED2 IS OFF

LED1 IS ON
LED2 IS ON

LED1 IS OFF
LED2 IS ON

Does that look about right?

(Oh, and feel free to call me Tom, not TCWORLD :))

thanks tom :smiley:

yes that exactly is the same thing which is from the original sketch :smiley: perfect :smiley:

so for 5 leds i guess there would be 5 booleans and an array of size 5 to hold the led pins and would the text for the sprintf would the sketch size come under 8kb or maybe 16kb?? and will the ram be sufficient?

yes i will write the code and then again consult, but i am just asking of curiosity if this is feasible tom or shall i look into some other programing technique?

Well the issue is going to be that for a string that long, there may not be enough RAM for it. Which means you are going to have to look at using PROGMEM.

Alternatively, is it possible to send each button one by one?
e.g.

  sprintf(string,"<a href='?led=%d%d'><button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED1 IS %s</button></a>", !l1,l2,(l1?on:off));
  e.print(string);
  sprintf(string,"<a href='?led=%d%d'>
<button style='border: 1px solid #000; border-left: 10px solid #000' type='button'>LED2 IS %s</button></a>",l1,!l2,(l2?on:off));
  e.print(string);

If so, then this could work:

#include "etherShield.h"
#include "ETHER_28J60.h"

static uint8_t mac[6] = {0x54, 0x55, 0x58, 0x10, 0x00, 0x24};   // this just needs to be unique for your network, 

static uint8_t ip[4] = {192, 168, 1, 15}; // IP address for the webserver

static uint16_t port = 80; // Use port 80 - the standard for HTTP

ETHER_28J60 e;

byte LED[5] = {2,3,4,5,6}; //This would use digital2 to digital6, but you could choose any
void setup(){

  e.setup(mac, ip, port);

  for(byte i = 0; i < 5; i++){
    pinMode(LED[i],OUTPUT);
  }
}

void loop()
{
  char* params;
  if (params = e.serviceRequest())
  { 
    int startIndex = strcspn (params,"01"); //Find the first '1' or '0' in the parameters string
    for(byte i = 0; i < 5; i++){ //Work through each button in turn
      char ledStatus[6] = {0};
      strncpy(ledStatus, params + startIndex, 5); //Extract the 5 led statuses to a char array
      digitalWrite(LED[i],(ledStatus[i] == '1')?true:false); //Output to the correct LED
      ledStatus[i] = (ledStatus[i] == '1')?'0':'1'; //Swap the status for sending back in the HTML
      sprintf(string,"<a href='?led=%s'>
<button style='border: 1px solid #ff0000; border-left: 10px solid #ff0000' type='button'>LED1 IS %s</button></a>", ledStatus,((ledStatus[i] == '1')?"ON":"OFF"));
      e.print(string);
    }
  }
}

hmmm yeah, so maybe i could remove the colours of the buttons and gain some space :slight_smile:

and i seriously dont know how it would work if i send in the buttons one after other as u mentioned coz i have no idea about html ! :slight_smile:
ill give it a try and then if it works i guess the problem is solved and thanks :D, if it isnt then again u only have to help me :smiley:

also eeprom is only half kb and i dont think it is that much helpful :slight_smile:

Just noticed a bug, you need to add:

char string[150] = {0};

at the start of the loop();