Where do I post code for help with a large (29K) NTP/UDP and TCP/ip sketch

Hi SurferTim,
Using up the sockets was one of my thoughts but Parse.Packet() returns 0 when the NTP request fails. I also put a couple of Serial.print(_sock) and Serial.print(socket) lines into EthernetUDP.ccp and W5100.cpp. Only once did the sketch climb through the sockets going 1,2,3,1 in succession it never got to 4.As far as I can make out there isn't a socket zero as that is the value returned on failure.

I'm not convinced that is a socket problem because the NTP bit still fails even if you deliberately avoid making a web query on the server part.

How do you know that there is debris in the RX buffer? Looking at the library code Parse.Packet() looks for W5100.getRXReceivedSize(_sock) to be >0. a Serial.print here shows the received size as 56 which is to be expected and 0 when it isn't.

I've got this feeling that the three problems are interrelated but I'm at a loss to see or find where the clash is. The Web server file send loop goes round about 10,000 times a second when there are no web enquiries and about 300 then the PC is GETing info. This fills up the serial monitor/hyper terminal long before anything fails

Sorry but the forum webpage wont let me attach more than 4K of code and I don't have an FTP server to do it that way.

Do you have a SD card in the shield's slot? If so, remove it for a test.

Does the NTP code on this link work ok for you?

edit: I just checked the file attach under Additional Options. It certainly isn't bigger than 4Mbytes, is it?
Maximum attachment size allowed: 4096 KB, per post: 4

Hi SurferTim,

I'll try your code after lunch but I don't expect a problem from it as it is doing essentially the same thing as mine except that I'm an interrupt routine/counter to ask for an update every 20 min not mills(), the original system design was for this to be once a day but I've speeded the cycle up so I don't have to wait forever for things to fall over.

I've attached the code file and the HTML files. Your missing the Mystyle.css file but that is only window dressing anyway. The Setup.htm file allows you to set the max, min and default values for the various settings. The server checks they are valid settings before saving them in the EEPROM. (By the way I was reading the 4096Kb and thinking 4096 bytes is not a lot - new glasses required I guess)

You'll see that there is a one second interrupt that sets the bits of 'TickTock' which the main code then uses to update the readings from the sensors every second, update the state of the relays every 15 seconds and then on the 10min interval write data to a file. Then at the moment it should get a NTP update at the 20min - 23 second mark ie just before the next write to the SD card.

Enquiries 'GET' over the Ethernet from the PC to 192.168.1.65 invoke the Index.htm which uses the $Xy replacement flag to put the various temperatures and strings back to the client, you can also change the various thermostat parameters here. Hitting the 'Get saved thermal data' button at the bottom delivers you the current record file on the SD card straight to Excel so you can analyse it.

I'm using Fixed IP addresses as DHCP seems to add 4k to the sketch and I did think of using a RTC but the code for that and using 'Time'h' to set the clock also gave me more than 32k of code for the whole sketch. (Ideally there are a couple of things still to add, The ability to list the files on the card and chose which one to down load and to add more sensors but there is no point in redesigning the input board with MC14051 chips until the fundamental problem is solved. I know now that I would probably be better off with a mega board but there isn't room in the control box for one.)

TimeStampedTemperatureData40e.ino (31.8 KB)

index.htm (7.93 KB)

GetData.htm (459 Bytes)

response.htm (458 Bytes)

I think you are running out of sockets. I see where you read the first 12 characters of the GET request from the rx buffer, but I do not see where your code reads from the rx buffer until a blank(empty) line. That is "\r\n\r\n". I see an attempt to do that if the request is a POST. Even when I get that, I insure I have emptied the rx buffer. If you leave characters in that buffer, the socket does not close correctly.

This is the code I use. It has a proven way of reading the request. Feel free to borrow the parts you need.
http://playground.arduino.cc/Code/WebServerST

Hi SurferTim,
Well your NTP code ran all night without a hitch. So there is no problem with my broadband NTP service. Now for tweaking my own code to sort out the POST / GET requirements. couple of things I have noticed:

In my 'actionUpDate()' code iI've got

'if (client.findUntil("thermostat","\n\r")){' This would leave a '\n' in the buffer. I'll try turning that round to '\r\n' first and that bit should get to the end of the buffer.

I'm also wondering if a 'client.flush();' after the client.stop(); would do the trick at the end of the code that sorts out POST & GET

'if (client.findUntil("thermostat","\n\r")){' This would leave a '\n' in the buffer. I'll try turning that round to '\r\n' first and that bit should get to the end of the buffer.

Careful there. I would change that to "\r\n\r\n". If you don't read until the double cr/lf, you risk leaving characters in the rx buffer.

edit:
This is from the reference guide. Stream.findUntil() - Arduino Reference

findUntil() reads data from the stream until the target string of given length or terminator string is found.

If it finds the target string in the stream, it stops reading and leaves the rest of the characters in the rx buffer.

Well - if I rem out the part of my code that sorts the HTTP GET and POST then the NTP code keeps working. So clearly the part that sends and receives data to/from web pages is causing the problem.

Changing 'if (client.findUntil("thermostat","\n\r")){'

to 'if (client.findUntil("thermostat","\r\n\r\n")){'

Hasn't made any difference

Nor does including client.flush() just before client.stop()

I'm looking for a way to purge the W5100 RX buffer as it would appear that debris left there is clogging up the works. As far as I can make out client.find(..) and findUntil(..) do not advance the RX pointer but just returns a boolean value. Client.read and readBytesUntil do however consume the contents of the buffer but you have to do something with the values they return.

I'm contemplating putting a blind

while(client.available());{
client.read();
}
client.stop();

on the end of the code rather than the simple client.stop()

//Ethernet outputs here
 // listen for incoming clients
  byte requestType;  
  client = server.available();
  if (client) {
    while (client.connected()){
      if (client.available()) {
    /////////////////////////////////
    //Wink to show where we've got to  
    digitalWrite(7,LOW);
        memset(fileName, 0 ,sizeof(fileName)); //clear the inputBuffer
        if (client.readBytesUntil('/',fileName,MAX_PAGE_NAME_LEN)){
          if (strcmp(fileName,"GET ") == 0){
          requestType = 1 ; //search for 'GET'
          }
          else if (strcmp(fileName,"POST ") == 0){
          requestType = 2;  //search for 'POST'
          }
        //gather what comes after the '/'
        memset(fileName, 0 ,sizeof(fileName)); //clear the inputBuffer
        //EofN = 0;
          if( client.find("") ){
            fileBufferLen = 0;
            *fileName = 0;
            while(fileBufferLen < MAX_PAGE_NAME_LEN ){
              char c = client.read();
              if( c == 0 ){
                fileBufferLen = 0;   // timeout returns 0 !
              }
              else if((c == 32)||(c == 63)) {  // space character or ?
                fileName[fileBufferLen] = 0; // terminate the string
                break;
              }
              else{
                fileName[fileBufferLen++] = c;
              }
            }
            fileName[fileBufferLen] = 0;
            // Note: inputBuffer full before the closing post_string encountered
          }
          else fileBufferLen = 0;    //failed to find the prestring

        }//end if(client.readBytes
        if (requestType == 2){       //do a POST
          //skip the rest of the header and find the carriage return
          
          actionUpDate();           //check for key words and action
          }//end if (requestType == 2)
        // GET or POST Give the string asked for
        // no file name so give index
        if (fileBufferLen == 0) {
          
          strcpy(fileName,"index.htm");
          
        }//end if 
        sendFile(fileName);
        delay(1);
        
        
      }//end while (client.connected()
    }//end(client.avaialble .... for reading the POST/GET file required NB the buffer may not be empty!!
    delay(1);
    while(client.available());{       //make sure w5100 RX buffer is empty
        client.read();
        }
     client.stop();    
      }//end if (client..

digitalWrite(7, HIGH);


} //end 'loop'

Any body else tried anything like this?

Changing 'if (client.findUntil("thermostat","\n\r")){'

to 'if (client.findUntil("thermostat","\r\n\r\n")){'

Hasn't made any difference

Nor does including client.flush() just before client.stop()

client.flush() won't help. It is a send function that causes the tx buffer to write to the stream.

The blind read idea is a keeper tho. :wink:
I "blind read" until the \r\n\r\n, then call client.stop() while in the while(client.connected()) loop. That seems to close the socket correctly.

Changing 'if (client.findUntil("thermostat","\n\r")){'

to 'if (client.findUntil("thermostat","\r\n\r\n")){'

Hasn't made any difference

Then, clearly, the thing to do is to stop trying to actually use the data until you know what the data looks like. Forget about trying to use the data. Simply print it to the serial port, in multiple formats - char and hex. Only AFTER you KNOW what the data looks like can you successfully parse it.

Well

Yes I do know what is coming down the wire its everything from "HTTP1.1........... through to... keep connection live. \r\n\r\n"

If I eliminate all of it from the RXbuffer then the NTP part of the code works BUT the web page does not come up on the browser. it just sits there waiting. If I leave any of the '\r\n\r\n' in the buffer then the NTP code returns zero.

I guess I need to find a way of getting the TCP bit and the UDP bit to use different sockets.

Unfortunately simply moving

client = server.available();
clinet.connected();

to the front of the loop code doesn't stop UDP hijacking the TCP socket.

All ideas welcome

It doesn't hijack anything. It took me about 5 minutes to put a NTP client and a web server together in the same sketch. It is running right now. The NTP time is correct to the second, and the server works fine. I used the server code in a function called doServer() instead of loop(), and call it every iteration through loop() just before the NTP stuff.

I used this NTP code:

I used this server code:
http://playground.arduino.cc/Code/WebServerST
It is still running both just fine.

edit: On the NTP code, I replaced the 10 minute delay with a millis() time check so it wouldn't block there for 10 minutes.

Well folks I've built a piece of code that merges the two SurferTim offered up, and that works in its raw state. I've added my interrupt routine which just sets a flag and I've replaced the millis/delay counters with an routine in the loop that is triggered by the interrupt flag.
That did not work at first, untiI put a delay(1) between the trigger and the call to the NTP code.

So I now need to work out how to extract the GET ,POST, fileName, and the feedback of the values to be changed by the post instruction from the buffer (tBuff) into which the browser requests are written. Why was 64 bytes chosen for this buffer? The returned bytes from my browser IE10 run to 240+ from the 'HTTP/1.1'.....'bit through to the .....'keep connection alive\r\n\r\n'

You say the socket is not being hijacked but why when I put some Serial.print debug lines in the Library code di both the TCP and UDP bits of code nearly always come back as _sock = 1? Given that the server is talking to one IP address on port 80 and the UDP is talking to another IP on port 8888 surely they should be using two different sockets.

Why was 64 bytes chosen for this buffer?

That was all I needed for my request string. If you need more, feel free to make it bigger.

You say the socket is not being hijacked but why when I put some Serial.print debug lines in the Library code di both the TCP and UDP bits of code nearly always come back as _sock = 1? Given that the server is talking to one IP address on port 80 and the UDP is talking to another IP on port 8888 surely they should be using two different sockets.

What makes you think that? And you say "nearly always" as if occasionally they are not. If there is no port 80 request pending, the UDP function will use the first socket available.

I don't have a problem with the idea of sockets being reused provided that they are allocated and released properly and several sockets can be allocated to multiple overlapping activities. There must however be something wrong with the way this is done in the Ethernet library because in my case the UDP code is failing to return anything on the second and subsequent times of asking after there has been a web page request from the browser. The sequence goes.

1 Startup
2 Get NTP time stamp

Then in the loop
A Do control stuff
B Record Data
C Listen for Web browser request

If there is a request
a) Serve Page automatic Refresh every 15 seconds
b) React to any POST commands

D Once a day Get NTP time stamp (This is the bit that fails the second time it is supposed to get the time)
E Go round again

Activity D always works the first time in the loop even if it has been preceded by a browser request but not the second time round. If there is no browser request as the startup/ first turn of the loop then there is a 50/50 chance that it will return the time a few more times before it fails.

As for my 'nearly always' comment if left in the failed state it continues to fail, but once in a hundred or so times it will actually return the right time!!! I've speeded the requests up to once every 2 minutes recording the outcome with the data to the SD card. There is no pattern that I can ascertain to when it might throw up the right time.

If it did not work at all, or only worked on start up but not in the loop I would know there was a problem with my code, but its the works some of the time condition that is flumoxeing.

I just started my NTP/server sketch again. It has updated four times (every 10 minutes) and the times returned are correct, even after several server requests.

I did notice one thing about UDP. If you have a SD card in the slot, and do not disable or initialize it, the NTP time will be incorrect. I have a 2GB SanDisk in the slot for my test, but I disable it in setup.

I would post my sketch, but it is a bit difficult to understand. I have changed it since I wrote it to automatically adjust the millis time to get the millis "clock" adjusted to stay within a second of the NTP time over a few hours.

I've found a curious thing with my sketch and the Ethernet Library. I now believe that your original thought that I was running out of Sockets is right but not in the normal overload way. Yes I did have the last CRLF left in the buffer as well but once I cured that problem I still had a problem getting UDP time stamp updates.

As originally coded I put a Udp.begin(localPort); at the beginning of the code to get an NTP time stamp and a UDP.stop(); at the end of that segment of the code. As I was only looking to get a time stamp once a day it seemed reasonable to free up the socket that UDP was using for the rest of the time.

Now by doctoring the Server.available() and the UDP begin() to print out the socket number to the serial monitor showed that in on start up Server.available() claimed socket 0 and UDP claimed socket 1. However once running in the loop as soon as there was a browser request Server.available() hogged both sockets 0 &1, I suspect the reason for two sockets is that the browser starts asking for the Style.css file before its finished receiving the web page. If the sketch called for an updated time stamp it might get socket 0 or 1and it seems to be pure chance as to which it gets If it got socket 1 ie its original socket, the new time was returned any other socket and nothing comes back. I even tried loading the system with several browsers open on various PCs on the network. All 4 sockets were in use but UDP code was only interested in socket 1.

Going a stage further I deleted the Udp.stop() line and moved the Udp.begin() into the setup part of the code. Now on start up Server.available gets socket 0 UDP gets socket 1 and keeps it! Make a browser request for a web page and server.available uses sockets 0 and 2. With this set up the time stamps keep on coming with out any hickcups. So good coding practice of releasing resources when you don't need them creates a problem.

It is my belief that there is either something wrong with the way the server releases sockets or there is something wrong with the way UDP aquires a socket. Certainly there is a an incompatibility between the two.

The code has run for 4 days now with out a problem albeit that the UDP code is hogging one of the sockets, however I think this incompatibility between Server.available() and UDP.begin() should be treated as a bug that needs fixing.

The code has run for 4 days now with out a problem albeit that the UDP code is hogging one of the sockets, however I think this incompatibility between Server.available() and UDP.begin() should be treated as a bug that needs fixing.

OK, what is your suggestion? Would you prefer the UDP function to fail if all sockets are busy with the server?

If all the sockets are in use then what ever is calling for another socket, be it Server or UDP should fail to get one. What I've been showing is that once you top and bottom your code that sends and receives a NTP request with a Udp.begin()/Udp.stop(), the UDP fails even when there are sockets available and it is allocated one.

I haven't dug deep into the library code yet but I suspect that when a socket is allocated to UDP and that was or is still being used by the server it is not recognised as still being in use.

If all the sockets are in use then what ever is calling for another socket, be it Server or UDP should fail to get one.

I vote "no" on that. If you need more sockets, may I suggest the w5200.

edit: As a matter of fact, I was already trying to devise a way to reserve a socket for client communication while running a server.

edit2: After a little thought, it would be fine with me if
socket 0 was reserved for client tcp stuff
socket 1 was reserved for UDP stuff
socket 2 and up for the server. 2 if w5100 or 6 if w5200