[SOLVED] Ethernet speed riddle (or String speed?) (with w5100)

After using surferTims patch for w5100.h, I got my keep-alive sensor value server up and running. And right away stumple into the next issue. The time it takes.

First I suspected the analogRead.
Then I suspected the ethernet shield and the many single "client.print()"
Then I suspected the String library by itself.
But I just cannot nail it down to any of this.

Here is my code (PLEASE NOTE THE TIME MEASURES IN THE COMMENTS)

/*
  Keep Alive Server socket
  
  Receives one-char-instructions what to send back next
  Closes the socket when receiving a "X"
  
 Circuit:
 * Ethernet shield attached to pins 10, 11, 12, 13
 * Analog inputs attached to pins A0 through A5 (optional)
 
 created 2011 (based on the Webserver example)
 by Thomas Schuett
 */
#define FASTADC 1

// defines for setting and clearing register bits
#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif
#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif



#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[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
byte ip[] = { 192,168,0, 111 };
//byte gateway[] = {192,168,0,1};	
//byte subnet[] = { 255, 255, 255, 0 };

//String responseStr; // then nothing works any more !?!?
  
// Initialize the Ethernet server library
// with the IP address and port you want to use 
// (port 80 is default for HTTP):
Server server(80);


void setup()
{
// from the forum:
#if FASTADC
  // set prescale to 16
  sbi(ADCSRA,ADPS2) ;
  cbi(ADCSRA,ADPS1) ;
  cbi(ADCSRA,ADPS0) ;
#endif

  // start the Ethernet connection and the server:
  Ethernet.begin(mac, ip);
  server.begin();
  delay(1000);
  blink();
}

void loop()
{
  // listen for incoming clients
  Client client = server.available();
  if (client) {
    //blink();
  
    while (client.connected()) {
      
      if (client.available()) {
        //blink2();
        
        char c = client.read();
        client.println(c);

        if (c == 'A') { // needs 14 ms
          // output the value of each analog input pin
          for (int analogChannel = 0; analogChannel < 6; analogChannel++) {
            client.print("analog input ");
            client.print(analogChannel);
            client.print(" is ");
            client.print(analogRead(analogChannel)); // takes around 3 ms each ??
            client.println();
          }
        }

        if (c == 'B') { // needs 33 ms (!!!!!!)
          String response111 = String("");
          for (int analogChannel = 0; analogChannel < 6; analogChannel++) {
            response111 += "analog input ";
            response111 += analogChannel;
            response111 += " is ";
            response111 += analogRead(analogChannel);
            response111 += "\n";
          }
          client.println(response111);
        }

        if (c == 'C') { // needs 5 ms
          String response111 = String("");
          for (int analogChannel = 0; analogChannel < 6; analogChannel++) {
            response111 += "analog input ";
            response111 += analogChannel;
            response111 += " is ";
            response111 += analogRead(analogChannel);
            response111 += "\n";
          }
          client.println("xxx");
        }

        if (c == 'D') {  // 33 ms
          String response111 = String("");
          for (int analogChannel = 0; analogChannel < 6; analogChannel++) {
            response111 += "analog input ";
            response111 += analogChannel;
            response111 += " is ";
            response111 += "xxx";
            response111 += "\n";
          }
          client.println(response111);
        }

        if (c == 'E') {  // 33 ms
          String response111 = String("");
          for (int analogChannel = 0; analogChannel < 6; analogChannel++) {
            response111 += "analog input ";
            response111 += analogChannel;
            response111 += " is ";
            response111 += "xxx";
            response111 += " ";
          }
          client.println(response111);
        }

        if (c == 'F') { // needs 3 to 4 ms
          String response111 = String("");
          for (int analogChannel = 0; analogChannel <    1   ; analogChannel++) {
            response111 += analogRead(analogChannel);
            response111 += " ";
          }
          client.println(response111);
        }

        if (c == 'G') { // needs 8 ms
          String response111 = String("");
          for (int analogChannel = 0; analogChannel < 6; analogChannel++) {
            response111 += "---";
            response111 += " ";
          }
          client.println(response111);
        }

        if (c == 'H') { // needs 2 ms
          client.println("a");
        }

        if (c == 'I') { // needs 3 to 4 ms
          client.println("a b b b b b b b b b b b b b b b b b b b b b b b b b b b b b b b b b c");
        }

        if (c == 'J') { // needs 2 ms
          client.println("a\nb\nb\nb\nb\nb\nb\nc\n");
        }


        client.println("END.");

        if (c == 'X') {
          break;
        }

      }
    }
    // give the other side time to receive the data
    delay(1);
    // close the connection:
    client.stop();
  }
}

And in case you want to test it by yourself, here is the client side, written in Java:

import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.net.Socket;


public class LoopThread extends Thread {
	public boolean stop = false;
	
//	public String sendChars = "AIIIA";
	public String sendChars = "J";
	int whichChar = 0;
	
	public void run() {
		
		Socket s = null;
		OutputStreamWriter sout =null;
		try {
			s = new Socket("192.168.0.111",80);
			InputStream in = s.getInputStream();
			OutputStream out = s.getOutputStream();
			sout = new OutputStreamWriter(out);
			BufferedInputStream bin = new BufferedInputStream(in);
			int cnt = 0;
			do {
				cnt++;
				System.out.println("======================= "+cnt);
				boolean ende = false;
				String c = sendChars.substring(whichChar,whichChar+1);
				whichChar++;
				if (whichChar == sendChars.length()) whichChar = 0;
				long startTime = System.currentTimeMillis();
				sout.write(c);
				sout.flush();
				while (! ende && ! stop) {
					String line = readLine(bin);
					System.out.print(line);
					
					if (line.indexOf("END.") > -1) {
						ende = true;
					}
				}
				long stopTime = System.currentTimeMillis();
				System.out.println("Time used: "+(stopTime-startTime)+ " ms");

				try { Thread.sleep(20); } catch (InterruptedException e) {	}
			} while (! stop);
			sout.write("X");
			sout.flush();
		} catch (Throwable e) {
			e.printStackTrace();
		}

		try {
			try { Thread.sleep(20); } catch (InterruptedException e) {	}
			s.close();
		} catch (IOException e) {
			e.printStackTrace();
		}
		
		System.out.println("\n\n==== ENDE ====");
		
	}
	
	
	private String readLine(BufferedInputStream bin) throws IOException {
		StringBuffer buf = new StringBuffer();
		int l = 0;
		do {
			l = bin.read();
			if (l > -1) {
				buf.append((char) l);
			}
			else {
				break;
			}
		} while ((char) l != '\n');
		if (buf.length() > 0) return buf.toString();
		else throw new IOException("input stream closed");
	}

}

and

import java.io.IOException;


public class Loop {

	static boolean stop = false;
	
	public static void main(String[] args) {
		LoopThread t = new LoopThread();
		t.start();
		
		try {
			System.in.read();
		} catch (IOException e) {
			e.printStackTrace();
		}
		t.stop = true;
	}
	
}

Does anyone understand the measured timings?

(BTW: Yes, I saw the thread "Arduino Ethernet Shield Speed", but that turned out to be due to serial debug output delays.)

O.k., that was a bit much to read. Let me boil it down to 3 versions:

if (c == 'A') { // needs 14 ms
// output the value of each analog input pin
for (int analogChannel = 0; analogChannel < 6; analogChannel++) {
client.print("analog input ");
client.print(analogChannel);
client.print(" is ");
client.print(analogRead(analogChannel)); // takes around 3 ms each ??
client.println();
}
}

if (c == 'B') { // needs 33 ms (!!!!!!)
String response111 = String("");
for (int analogChannel = 0; analogChannel < 6; analogChannel++) {
response111 += "analog input ";
response111 += analogChannel;
response111 += " is ";
response111 += analogRead(analogChannel);
response111 += "\n";
}
client.println(response111);
}

if (c == 'C') { // needs 5 ms
String response111 = String("");
for (int analogChannel = 0; analogChannel < 6; analogChannel++) {
response111 += "analog input ";
response111 += analogChannel;
response111 += " is ";
response111 += analogRead(analogChannel);
response111 += "\n";
}
client.println("xxx");
}

So in "A" I do many client.print() calls, and it takes 14 ms to send all 6 sensor values.

In "B" I collect the whole response in a single String, and send this string out over ethernet.
Wow - It takes much longer now !?!?!? It needs 33 ms. So the String class may be bad? Or the analogReads all much more back-to-back compared to "a" ?

But no - in "C" I use the String class too, and collect all sensor values back-to-back too, but I just don't send it out over Ethernet - and it takes only 5 ms!!

How can this be ???

Isn't this just telling you that client.println() consumes some time, but that once it's sent to the ethernet shield, the shield will put it on the wire in parallel to other activities. So you can be reading analog and accumulating data for your next print. Your B version waits until the end to send ~120 chars. A version takes advantage of the parallelism, C version tells you client.println() takes a while.

wildbill:
Isn't this just telling you that client.println() consumes some time, but that once it's sent to the ethernet shield, the shield will put it on the wire in parallel to other activities.

No, I am sorry this can not explain the timings:
In "C" we see, that the collection of the whole response takes 5 ms. But the only difference to "B" is, that B sends out this string via Ethernet. As B needs 33 ms, and the first part (equals to C) takes only 5 ms, the sending via ethernet needs 28 ms in "B". But this is much more than the whole thing (including sending out everything over ethernet) takes only 14 ms in "A".

Have you tried your speed test using client.write()? I use the write function not documented in the manual. It seems to perform pretty good. I have not tested it tho. I load everything up into outBuffer of size bufferSize, then send it in one call.

client.write(outBuffer,bufferSize);

          String response111 = String("");

Please explain this statement. Construct a string object containing no characters. Then, use the assignment operator to set another string to that copy. Then, delete the string object.

          String response111 = "";

Does exactly the same thing without the need to call a constructor and destructor.

When speed is your goal, stupid should not be part of your coding practices.

SurferTim:
Have you tried your speed test using client.write()?

No I have not by now, but it seems a good idea. Even more, as just found out (with tcpdump), that in variant "A" each digit of each sensor value comes in its own ethernet package, and even worse: In variant "B" each single letter (!!!) comes in its own ethernet package.

@PaulS: Stupid is, who looks for microseconds, when milliseconds are in question.

I am surprised at the result of variant B. I thought it would go in one packet.

Please let me know how the write function does in comparison. I know that
client.write(byte)
sends each byte in a packet, but according to my router, that dropped to a single packet by using
client.write(byte*,int);

ADD: You might want to try another variant of write in variant B. Try replacing
client.println(response1111);
with
client.write(response1111);

That does the same as my client.write() above, actually this:
client.write(response1111,strlen(response1111));

The client.write(byte*,int) sends all the characters in the byte array to the w5100 transmit buffer, then sends the w5100 the "send" command. That should send all that in one packet. Should...

SurferTim:
ADD: You might want to try another variant of write in variant B. Try replacing
client.println(response1111);
with
client.write(response1111);

That does the same as my client.write() above, actually this:
client.write(response1111,strlen(response1111));

Actually I am not a C programmer (I do java, perl and shell script).

The client.write(response111); resulted in
error: no matching function for call to 'Client::write(String&)'

and the 2nd gave:
error: cannot convert 'String' to 'const char*' for argument '1' to 'size_t strlen(const char*)'

I know there have to be some "*" or some "&" here and there, but this is exactly the reason why I don't do C programming (except in cases like this, when it just must be).

So would you mind to assist, please? :slight_smile:

I do not use strings and append. I use character buffers and sprintf. That works with client.write(). Compile and run this if you need an example.

char outBuffer[64];
int testInt = 0;
int myInt = 100;

void setup()
{
  Serial.begin(9600);
}

void loop()
{
  sprintf(outBuffer,"testInt is %d and myInt is %d\n",testInt,myInt);
  Serial.print(outBuffer);
  testInt += 1;
  myInt -= 1;
  delay(1000);
}

Then you would use outBuffer just like in the serial call. It works.
client.write(outBuffer);

[quote author=SurferTim link=topic=73170.msg550883#msg550883 date=1316902612]
  sprintf(outBuffer,"testInt is %d and myInt is %d\n",testInt,myInt);

Oh yes, thats it. I was at strcpy, but saw no possibility to put an offset to the destination. Also the non-standars arduino "String" was somehow a problem (no .size(), no .c_str(),...).

I will try it today and mail back. I m sure it'll run like hell :slight_smile:

One C question again: Can I sprintf to an offset in outBuffer? Can I say sprintf(outBuffer+14, ... for example? Is this a "proper" way to do it?

Sure. Like any array. This starts at outBuffer array position 8:
sprintf(&outBuffer[8],"This is a test");
and this too
sprintf(outbuffer+8,"This is a test");

Since you are using a zero terminated string ( I don't), you can use strlen() to find the new "append" position.

Thomas33:
I will try it today and mail back. I m sure it'll run like hell :slight_smile:

And it does :slight_smile: :

       if (c == 'B') { // needs 10 to 11 ms for all 16 analogValues :-)
          char a[1024];
          for (int analogChannel = 0; analogChannel < 16; analogChannel++) {
            sprintf(a+24*analogChannel,"analog input %2d is %4d\n", analogChannel,analogRead(analogChannel));
          }    
          client.write(a);
        }

Of course, the extra packets for the command echo and the "END" can be included too, but this whould have damaged the speed comparision.

BTW: The FASTADC stuff seems to save around 0.1 ms for each analogRead.

So I can put a SOLVED on this topic. Thanks SurferTim again!!

(Next will be moving to WLAN, but this will be a new big thing.)

Glad I could help. :slight_smile:

Just a warning about sprintf(), since you may soon want to convert int to float for voltage and the like.
There is no %f parameter. sprintf will not convert a float to a string.
But you can convert them:
http://arduino.cc/forum/index.php/topic,72682.0.html

SurferTim:
Just a warning about sprintf() ... There is no %f parameter. sprintf will not convert a float to a string.

Oh. Thanks for the hint, may be it saves me some headache later on.

BTW: A brief SUMMARY on the thread issue:

  • don't use client.print(stringObject) !! (as of arduino version 22)
    It will send each letter in its own ethernet package.
    (Maybe the client.print method should be extended to handle stringObjects in a appropriate way)

  • the new timings still may not be, what someone might expect from a 10 Mbit/s connection. But this is not a bug, but due to the SPI speed limitation, as the ethernet shield is connected via SPI.

have fun!
Thomas

(Maybe the client.print method should be extended to handle stringObjects in a appropriate way)

Probably, but it's better not to even use the String class.