TCP Client not reading anything

I replaced my web page parser with just:

void serialEvent() {
char inChar = myTCP.read();
Serial.print(inChar);
}

In my loop(), I do:

while (myTCP.available()) {
serialEvent();
}

And I do not see the full contents of the web page like I used to. The first 604 chars are completely missing. I get 504 chars, with some missing in between and then nothing. The chars I do get are mostly good, except for some missing areas. And it consistently does the same thing over and over.

Using curl and wc, I find that the web page I am fetching is 2767 bytes long, so I am only getting a small part of the page now.

Did anyone see a recent change to TCP buffer management that might be causing this?

I’m also seeing issues with TCP Client reads… if anyone can run this example and tell me what I’m doing wrong I’d appreciate it. I’m basically missing chunks of data, and I’m waiting plenty of time to get further chunks from the server. I’ve even increased the timeout in http_get() to 5 seconds and got the same response.

NOTE: When you run this you have to have your serial monitor closed, wait for the blue LED to come on, then open your monitor and press enter. It will pull data down from http://graph.facebook.com/sparkdevices and then timeout, and return to the main loop where it flashes the led until the next 10 second mark, and repeats the request. One thing to note client.connected() is always true until we call client.stop() so I didn’t use that for a timeout… had to use an actual timer. In the future client.connected() should be show the socket closed when the server terminates the connection, which should be after all of the data is sent.

I’m also seeing weird things with Serial TX… I need to put delays in there after large amounts of data or I don’t seem to see the data appear in the serial monitor.

#include "application.h"

TCPClient client;

String http_get(char const * hostname, String path) {

  if (client.connect(hostname, 80)) {
    client.print("GET ");
    client.print(path);
    client.print(" HTTP/1.1\r\n");
    client.print("HOST: ");
    client.println(hostname);
    client.print("\r\n\r\n\r\n");
  } else {
    Serial.println("\r\n\r\nConnection Failed!");
    client.stop();
    return NULL;
  }

  int i = 0;
  Serial.println("\r\n\r\n\r\nReading Facebook Data......");
  uint32_t lastRead = millis();
  while ((millis() - lastRead) < 1000) {
    while (client.available()) {
      char c = client.read();
      Serial.print(c);
      if(i++ > 50) {
        delay(100);
        i = 0;
      }
      lastRead = millis();
    }
  }
  client.flush();
  client.stop();
  return "";
}

void setup() {
  pinMode(D7, OUTPUT);
  digitalWrite(D7, HIGH);
  Serial.begin(115200);
  while (!Serial.available()); // After Core D7 LED turns on, open serial monitor and press enter!
}

uint32_t nextTime = millis(); // next time to contact the server
int count = 19;
bool state = 1;
void loop() {
  if (millis() > nextTime) {
    nextTime = millis() + 500UL;
    if (++count == 20) {
      count = 0;
      http_get("graph.facebook.com", "/sparkdevices");
    } else { // toggle the led while we wait for 10 second mark
      state = !state;
      digitalWrite(D7, state);
    }
  }
}

This is what I get:

Reading Facebook Data......
HTTP/1.1 200 OK
Access-Control-Allow-Origin: *
Cache-Control: private, no-cache, no-store, must-revalidate
Content-Type: text/javascript; charset=UTF-8
ETag: "38d35b9d9ec8dd27ed2ce90819473c873663d089"
Expires: Sat, 01 Jan 2000 00:00:00 GMT
Pragma: no and pre-order the Spark Core and accessories at www.spark.io","category":"Computers\/technology","description":"Spark makes using and creating connected devices easy. The Spark Core an Arduino-compatible, Wi-Fi enabled development kit pairs with the Spark""},"products":"Spark Core, Spark Cloud","talking_about_count":395,"username":"sparkdevices","website":"http:\/\/www.spark.io","were_here_count":0,"id":"427120970660412","name":"Spark","link":"http:\/\/www.facebook

There is another thread where there is similar discussion and a patch to TCPClient that has fixed the problem for some. I don’t have time to test this myself, but I thought you would want to know.

I hope that @zachary can get this fix applied quickly–I am dead in the water right now.

That’s great guys, thanks a lot for the fast feedback and workarounds, in my case I was able to fix the code looping until available() returned something:

String http_get(char const* hostname, String path) {

    if (client.connect(hostname, 80)) {
            client.print("GET ");
            client.print(path);
            client.print(" HTTP/1.0\r\n");
            client.print("HOST: ");
            client.println(hostname);
            client.println();
            client.flush();
    } else {
            Serial.println("connection failed");
            client.stop();
            return NULL;
    }

    int bytes = 0;
    while(!bytes && client.connected()) {
	    bytes = client.available();
	    delay(200);
    }
	Serial.println("bytes:");
	Serial.println(bytes);

	for (unsigned int i = 0;  i  < bytes; i++) {
		char c = client.read();
			Serial.print(c);
		if (c == -1) {
		    	Serial.println("oops");
			break;
		}

		buffer[i] = c;
	
	}
	client.stop();

    String response(buffer);
    int bodyPos = response.indexOf("\r\n\r\n");
    if (bodyPos == -1) {
            Serial.println("can not find http get reponse body");
                            return NULL;
    }
    return response.substring(bodyPos+4);

}

B​Dub’s code seems more elegant as it doesn’t use delay that much and as he mentioned connected() always returns true which makes mine redundant. I’ll change to that if in the following weeks we don’t get the fixed merged into compile-server2.

@BDub not really answering but having a problem using your code.

I set up a web service that queries http://openweathermap.org/ and then maps temperature and conditions to RGB codes (I want to create a moodlight based on the weather.)

the web service url is: http://roman-mueller.ch/api/weather

As you can see it returns something like this:

"W-[000,000,139][144,238,144] - Temp: -2,3°C; Color [DarkBlue] - Condition: SkyIsClear; Color [LightGreen]"

So when I use your code from above just using my url instead of graph.facebook.com and /api/weather instead of /sparkdevices I get the following answer:

HTTP/1.1 200 OK
Cache-Control: no-cache
Pragma: no-cache
Content-Type: application/json; charset=utf-8
Expires: -1
Server: Microsoft-IIS/7.0
X-AspNet-Version: 4.0.30319
X-Powered-By: ASP.NET

Looks like I get all the header data but not my weather data… any idea?

1 Like

Yeah I don't know why everyone is messing around with TCP Clients right when it's currently broken, but it's popping up a lot today! See my post and a temporary fix by weakset here:

1 Like

Hey Guys,

Popping in to say I fixed some issues with the pre-processor stripping out stuff it shouldn’t. I’ve changed the approach it takes, so that shouldn’t be a side-effect anymore. Please keep sending me any code you find that doesn’t compile like you think it should, and we’ll add unit tests for it! :slight_smile:

Thanks!
David

2 Likes

I just want to report back a minor success. Remember in the early days we all noticed that TCPClient.available() was returning a boolean rather than a count of bytes? Well the recent changes fixed this and it must be related to the TCPClient problems.

My old code for the boolean case looked like this and extracted one byte per call of loop():

while (myTCP.available()) {
    serialEvent();
}

This code stopped working sometime last night when the new TCPClient rolled out.

Now my code looks like this:

int bytecount = myTCP.available();
while (bytecount>0) {
    serialEvent();
    bytecount--;
}

And this works much better.

I thought I was being a good cooperative-multitasking doobie by giving control back to the TCPClient every byte, but I think it does not like that! Now if the available() method says there are N bytes available, I process them all at once and only then give control back by letting loop() go round again. Maybe this prevents premature buffer reuse in the TCPClient, I don't know.

But it does work much better now--maybe you guys can try it too.

1 Like

Thanks @bko for the report! Here are the commits for the recent changes to TCPClient:

What does serialEvent() do? If you let it fall through your loop() over and over again, how do you know when you are done with the transfer?

Hi @zach, Thanks for the pointer to commits. Here is what I think is going on. The new code in TCPClient.available() checks the number of bytes received on line 131 and then sets the _offset to zero and the _remaining to the number of bytes. When TCPClient.read() is called, it checks if there are bytes remaining (_remaining > 0 on line 154) and if so, it gets the byte incrementing the _offset pointer and decrementing the _remaining count.

The problem arrises if I call TCPClient.available(), then call TCPClient.read(), and then call TCPClient.available() again. In the first call to available, _offset and _remaining are set correctly. I then get one byte with the call to read() and _offset and _remaining are updated. But then when I call available() again, the current values of _offset and _remaining are ignored and overwritten with _offset = 0 and _remaining = new bytes received.

I think the proper thing to do in TCPClient.available() is to add the number of new bytes received to the current value of _remaining and to only force _offset to zero when _remaining is zero.

I would love to send you a pull request, but I don’t have a build sandbox setup here.

I hope this helps!

Hi @BDub,

My serialEvent() is my one-character-at-a-time state-machine HTML/XML parser. It starts off with:

void serialEvent() {
    char inChar = myTCP.read();
    if (tagFlag==false && dataFlag==false && inChar == startMatch[matchPtr]) {
        tagFlag = true; 
....

So it consumes one char from the TCPClient.read() for every call. With my changes for the TCPClient.available(), it is called in a while loop for as many bytes are available. In my old code, I called it one or more times per loop() when available was true, since parsing the web/rss feed doesn’t have to be done quickly.

The problem seems to be that if you call TCPClient.available() and it tells you there are N bytes, you need to call TCPClient.read() N times before you call TCPClient.available() again, or you will lose data.

@zach @satishgn I don’t pretend to understand the low level driver yet but I have a rough view of what select() and recv() do, but what is the theory behind making TCPClient.available() check for available bytes AND read them into a buffer, vs. letting TCPClient.read() do the actual reading from low level stuff?

One reason I thought might be because you thought buffering 512 straight away is faster, and then let users peck away at the data in read() and buffered read() at their leisure.

One issue I see is currently we are not automatically getting our socket to close when the server is done sending data, so TCPClient.connected() is always true unless we call TCPClient.stop(); But we don’t really know when the data is done… so we don’t really know if we should try to close the socket ourselves or not. That said, there is a way to know when the server closes the socket… the recv() method will return -57 when it happens. I guess some things can be done in either as long as you’re polling them both and it doesn’t matter… but it seems confusing when you go read the code.

Anyhoo… if the server socket closing was detected, _sock could be set to SOCK_MAX_NUM and then TCPClient.connected() would be useful. Something like this:

uint32_t lastRead = millis();
  while (client.connected() && (millis() - lastRead < 3000)) {
    while (client.available()) {
      char c = client.read();
      lastRead = millis();
    }
  }
  client.stop(); // technically this is redundant if we don't timeout of the main while loop

This is pretty much necessary for chunked transfers, so you’ll know to keep waiting because there is another chunk possibly coming.

@BDub I'm getting a '0' at the end of the read, is that what I should be getting?

Reading Facebook Data......
HTTP/1.1 200 OK
Access-Control-Allow-Origin: *
Cache-Control: private, no-cache, no-store, must-revalidate
Content-Type: text/javascript; charset=UTF-8
ETag: "5d8d8fa86024882f78b93e63c06f69e3b44e81bf"
Expires: Sat, 01 Jan 2000 00:00:00 GMT
Pragma: no-cache
X-FB-Rev: 1100030
X-FB-Debug: 9rDexSyqdPgbWmPZ5O+FFCDeCOaQ4j1Q1F77B4D/TJQ=
Date: Wed, 29 Jan 2014 16:20:37 GMT
Transfer-Encoding: chunked
Connection: keep-alive

3f0
{"about":"Spark makes using and creating connected devices easy. Learn more and pre-order the Spark Core and accessories at www.spark.io","category":"Computers/technology","description":"Spark makes using and creating connected devices easy. The Spark Core an Arduino-compatible, Wi-Fi enabled development kit pairs with the Spark Cloud powered development platform to make creating internet-connected hardware a breeze now available for pre-order at: https://www.spark.io","is_published":true,"location":{"street":"","city":"Minneapolis","state":"MN","country":"United States","zip":""},"products":"Spark Core, Spark Cloud","talking_about_count":395,"username":"sparkdevices","website":"http://www.spark.io","were_here_count":0,"id":"427120970660412","name":"Spark","link":"http://www.facebook.com/sparkdevices","likes":2381,"cover":{"cover_id":537602636278911,"source":"http://sphotos-f.ak.fbcdn.net/hphotos-ak-prn1/s720x720/923349_537602636278911_656089984_n.jpg","offset_y":40,"offset_x":0}}
0

Yes that looks complete! What is that 0 or the 3f0 at the beginning?.. heck if I know. BTW I have a cool project with all of this code in mind… coming soon.

Try adding “Connection: close” to your request headers.

Does that get rid of the mystery ‘0’? What is that anyway?

The server is sending keep-alive connection data in pieces (Transfer-Encoding: chunked) and that “random” stuff is actually hexadecimal size of the next piece.

3f0 is 1008 in decimal and 0 means there is no more data coming this time.

Aha… makes sense… 1008 characters there. Except shouldn’t that be in the header as Content-Length: 1008 ? When I use Advanced Rest Client I see content-length: 574

Coffee’s code looked for "\r\n\r\n" to indicate the body. Is the first line of the body always the content length? I guess if so, we have an easy way to look for the end of the data, and we can close the socket ourselves.

Yeah, we are not really parsing the headers to understand how to read the data and handle the connection. I guess I have to go dig up some of my old HTTP browser code to see if I can ‘borrow’ from there. :smile: