From bc0f3c4fe165aafe60c4cda254e3d16bb6be69ec Mon Sep 17 00:00:00 2001 From: amcewen Date: Tue, 28 Dec 2010 15:16:42 +0000 Subject: [PATCH 1/4] Fixes to UDP so that it no longer has socket 0 hardcoded - all part of issue #436. UdpClass::begin now finds the first available free socket, or fails if they're all in use. UdpClass::stop added to release the socket once it is no longer needed. The global Udp object has also been removed and the examples updated to provide their own instance. Finally, in testing I noticed that the UdpNtpClient didn't print leading 0s if the minute or second was a single-digit, so have taken the opportunity to provide a simple fix for it. --- libraries/Ethernet/Udp.cpp | 30 ++++++++++++++++--- libraries/Ethernet/Udp.h | 7 +++-- .../UDPSendReceiveString.pde | 2 ++ .../examples/UdpNtpClient/UdpNtpClient.pde | 11 +++++++ 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/libraries/Ethernet/Udp.cpp b/libraries/Ethernet/Udp.cpp index 17eaac69c..b349317d7 100644 --- a/libraries/Ethernet/Udp.cpp +++ b/libraries/Ethernet/Udp.cpp @@ -32,10 +32,25 @@ #include "Udp.h" /* Start UDP socket, listening at local port PORT */ -void UdpClass::begin(uint16_t port) { +uint8_t UdpClass::begin(uint16_t port) { + if (_sock != MAX_SOCK_NUM) + return 0; + + for (int i = 0; i < MAX_SOCK_NUM; i++) { + uint8_t s = W5100.readSnSR(i); + if (s == SnSR::CLOSED || s == SnSR::FIN_WAIT) { + _sock = i; + break; + } + } + + if (_sock == MAX_SOCK_NUM) + return 0; + _port = port; - _sock = 0; //TODO: should not be hardcoded socket(_sock, SnMR::UDP, _port, 0); + + return 1; } /* Send packet contained in buf of length len to peer at specified ip, and port */ @@ -129,8 +144,15 @@ port = myPort; return ret; } +/* Release any resources being used by this UdpClass instance */ +void UdpClass::stop() +{ + if (_sock == MAX_SOCK_NUM) + return; + close(_sock); + EthernetClass::_server_port[_sock] = 0; + _sock = MAX_SOCK_NUM; +} -/* Create one global object */ -UdpClass Udp; diff --git a/libraries/Ethernet/Udp.h b/libraries/Ethernet/Udp.h index 5b88d3e06..51c045f20 100644 --- a/libraries/Ethernet/Udp.h +++ b/libraries/Ethernet/Udp.h @@ -45,7 +45,8 @@ private: uint16_t _port; // local port to listen on public: - void begin(uint16_t); // initialize, start listening on specified port + UdpClass() : _sock(MAX_SOCK_NUM) {}; + uint8_t begin(uint16_t); // initialize, start listening on specified port. Returns 1 if successful, 0 if there are no sockets available to use int available(); // has data been received? // C-style buffer-oriented functions @@ -56,8 +57,8 @@ public: // readPacket that fills a character string buffer int readPacket(char *, uint16_t, uint8_t *, uint16_t &); + // Finish with the UDP socket + void stop(); }; -extern UdpClass Udp; - #endif diff --git a/libraries/Ethernet/examples/UDPSendReceiveString/UDPSendReceiveString.pde b/libraries/Ethernet/examples/UDPSendReceiveString/UDPSendReceiveString.pde index cfe1b58b5..6f694d250 100644 --- a/libraries/Ethernet/examples/UDPSendReceiveString/UDPSendReceiveString.pde +++ b/libraries/Ethernet/examples/UDPSendReceiveString/UDPSendReceiveString.pde @@ -35,6 +35,8 @@ unsigned int remotePort; // holds received packet's originating port char packetBuffer[UDP_TX_PACKET_MAX_SIZE]; //buffer to hold incoming packet, char ReplyBuffer[] = "acknowledged"; // a string to send back +// A UDP instance to let us send and receive packets over UDP +UdpClass Udp; void setup() { // start the Ethernet and UDP: diff --git a/libraries/Ethernet/examples/UdpNtpClient/UdpNtpClient.pde b/libraries/Ethernet/examples/UdpNtpClient/UdpNtpClient.pde index 0b3832b5b..c28e1e02e 100644 --- a/libraries/Ethernet/examples/UdpNtpClient/UdpNtpClient.pde +++ b/libraries/Ethernet/examples/UdpNtpClient/UdpNtpClient.pde @@ -37,6 +37,9 @@ const int NTP_PACKET_SIZE= 48; // NTP time stamp is in the first 48 bytes of the byte packetBuffer[ NTP_PACKET_SIZE]; //buffer to hold incoming and outgoing packets +// A UDP instance to let us send and receive packets over UDP +UdpClass Udp; + void setup() { // start Ethernet and UDP @@ -80,8 +83,16 @@ void loop() Serial.print("The UTC time is "); // UTC is the time at Greenwich Meridian (GMT) Serial.print((epoch % 86400L) / 3600); // print the hour (86400 equals secs per day) Serial.print(':'); + if ( ((epoch % 3600) / 60) < 10 ) { + // In the first 10 minutes of each hour, we'll want a leading '0' + Serial.print('0'); + } Serial.print((epoch % 3600) / 60); // print the minute (3600 equals secs per minute) Serial.print(':'); + if ( (epoch % 60) < 10 ) { + // In the first 10 seconds of each minute, we'll want a leading '0' + Serial.print('0'); + } Serial.println(epoch %60); // print the second } // wait ten seconds before asking for the time again From ca07ac18f3502dc1929100cd5896e1cebb1bead2 Mon Sep 17 00:00:00 2001 From: amcewen Date: Sat, 1 Jan 2011 21:42:23 +0000 Subject: [PATCH 2/4] Update to the fix for Issue #436 - UdpClass renamed to UDP and the constructor moved into the .cpp to prevent compilation errors in certain conditions if w5100.h hasn't been included before Udp.h --- libraries/Ethernet/Udp.cpp | 21 +++++++++++-------- libraries/Ethernet/Udp.h | 4 ++-- .../UDPSendReceiveString.pde | 2 +- .../examples/UdpNtpClient/UdpNtpClient.pde | 2 +- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/libraries/Ethernet/Udp.cpp b/libraries/Ethernet/Udp.cpp index b349317d7..07c454df2 100644 --- a/libraries/Ethernet/Udp.cpp +++ b/libraries/Ethernet/Udp.cpp @@ -31,8 +31,11 @@ #include "Ethernet.h" #include "Udp.h" +/* Constructor */ +UDP::UDP() : _sock(MAX_SOCK_NUM) {} + /* Start UDP socket, listening at local port PORT */ -uint8_t UdpClass::begin(uint16_t port) { +uint8_t UDP::begin(uint16_t port) { if (_sock != MAX_SOCK_NUM) return 0; @@ -56,13 +59,13 @@ uint8_t UdpClass::begin(uint16_t port) { /* Send packet contained in buf of length len to peer at specified ip, and port */ /* Use this function to transmit binary data that might contain 0x00 bytes*/ /* This function returns sent data size for success else -1. */ -uint16_t UdpClass::sendPacket(uint8_t * buf, uint16_t len, uint8_t * ip, uint16_t port){ +uint16_t UDP::sendPacket(uint8_t * buf, uint16_t len, uint8_t * ip, uint16_t port){ return sendto(_sock,(const uint8_t *)buf,len,ip,port); } /* Send zero-terminated string str as packet to peer at specified ip, and port */ /* This function returns sent data size for success else -1. */ -uint16_t UdpClass::sendPacket(const char str[], uint8_t * ip, uint16_t port){ +uint16_t UDP::sendPacket(const char str[], uint8_t * ip, uint16_t port){ // compute strlen const char *s; for(s = str; *s; ++s); @@ -72,7 +75,7 @@ uint16_t UdpClass::sendPacket(const char str[], uint8_t * ip, uint16_t port){ } /* Is data available in rx buffer? Returns 0 if no, number of available bytes if yes. * returned value includes 8 byte UDP header!*/ -int UdpClass::available() { +int UDP::available() { return W5100.getRXReceivedSize(_sock); } @@ -82,7 +85,7 @@ int UdpClass::available() { /* NOTE: I don't believe len is ever checked in implementation of recvfrom(),*/ /* so it's easy to overflow buffer. so we check and truncate. */ /* returns number of bytes read, or negative number of bytes we would have needed if we truncated */ -int UdpClass::readPacket(uint8_t * buf, uint16_t bufLen, uint8_t *ip, uint16_t *port) { +int UDP::readPacket(uint8_t * buf, uint16_t bufLen, uint8_t *ip, uint16_t *port) { int packetLen = available()-8; //skip UDP header; if(packetLen < 0 ) return 0; // no real data here if(packetLen > (int)bufLen) { @@ -131,21 +134,21 @@ int UdpClass::readPacket(uint8_t * buf, uint16_t bufLen, uint8_t *ip, uint16_t * } /* Read a received packet, throw away peer's ip and port. See note above. */ -int UdpClass::readPacket(uint8_t * buf, uint16_t len) { +int UDP::readPacket(uint8_t * buf, uint16_t len) { uint8_t ip[4]; uint16_t port[1]; return recvfrom(_sock,buf,len,ip,port); } -int UdpClass::readPacket(char * buf, uint16_t bufLen, uint8_t *ip, uint16_t &port) { +int UDP::readPacket(char * buf, uint16_t bufLen, uint8_t *ip, uint16_t &port) { uint16_t myPort; uint16_t ret = readPacket( (byte*)buf, bufLen, ip, &myPort); port = myPort; return ret; } -/* Release any resources being used by this UdpClass instance */ -void UdpClass::stop() +/* Release any resources being used by this UDP instance */ +void UDP::stop() { if (_sock == MAX_SOCK_NUM) return; diff --git a/libraries/Ethernet/Udp.h b/libraries/Ethernet/Udp.h index 51c045f20..c801ee277 100644 --- a/libraries/Ethernet/Udp.h +++ b/libraries/Ethernet/Udp.h @@ -39,13 +39,13 @@ #define UDP_TX_PACKET_MAX_SIZE 24 -class UdpClass { +class UDP { private: uint8_t _sock; // socket ID for Wiz5100 uint16_t _port; // local port to listen on public: - UdpClass() : _sock(MAX_SOCK_NUM) {}; + UDP(); uint8_t begin(uint16_t); // initialize, start listening on specified port. Returns 1 if successful, 0 if there are no sockets available to use int available(); // has data been received? diff --git a/libraries/Ethernet/examples/UDPSendReceiveString/UDPSendReceiveString.pde b/libraries/Ethernet/examples/UDPSendReceiveString/UDPSendReceiveString.pde index 6f694d250..f27290f54 100644 --- a/libraries/Ethernet/examples/UDPSendReceiveString/UDPSendReceiveString.pde +++ b/libraries/Ethernet/examples/UDPSendReceiveString/UDPSendReceiveString.pde @@ -36,7 +36,7 @@ char packetBuffer[UDP_TX_PACKET_MAX_SIZE]; //buffer to hold incoming packet, char ReplyBuffer[] = "acknowledged"; // a string to send back // A UDP instance to let us send and receive packets over UDP -UdpClass Udp; +UDP Udp; void setup() { // start the Ethernet and UDP: diff --git a/libraries/Ethernet/examples/UdpNtpClient/UdpNtpClient.pde b/libraries/Ethernet/examples/UdpNtpClient/UdpNtpClient.pde index c28e1e02e..22bc754c5 100644 --- a/libraries/Ethernet/examples/UdpNtpClient/UdpNtpClient.pde +++ b/libraries/Ethernet/examples/UdpNtpClient/UdpNtpClient.pde @@ -38,7 +38,7 @@ const int NTP_PACKET_SIZE= 48; // NTP time stamp is in the first 48 bytes of the byte packetBuffer[ NTP_PACKET_SIZE]; //buffer to hold incoming and outgoing packets // A UDP instance to let us send and receive packets over UDP -UdpClass Udp; +UDP Udp; void setup() { From 81b6c69f295fc58f928f758916540d1daf5d4bc9 Mon Sep 17 00:00:00 2001 From: amcewen Date: Sun, 2 Jan 2011 22:21:24 +0000 Subject: [PATCH 3/4] An improved patch for the Client part of issue 416 (adding a multi-byte read). This one moves all of the checking into recv, so that single-byte reads also benefit. It also returns -1 if there's no data available unless we've reached EOF, in which case it returns 0. --- libraries/Ethernet/Client.cpp | 17 ++++++++++++++--- libraries/Ethernet/Client.h | 1 + libraries/Ethernet/utility/socket.cpp | 14 +++++++++----- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/libraries/Ethernet/Client.cpp b/libraries/Ethernet/Client.cpp index 3b1084f0d..51cb5eb14 100644 --- a/libraries/Ethernet/Client.cpp +++ b/libraries/Ethernet/Client.cpp @@ -77,14 +77,25 @@ int Client::available() { int Client::read() { uint8_t b; - if (!available()) + if ( recv(_sock, &b, 1) ) + { + // recv worked + return b; + } + else + { + // No data available return -1; - recv(_sock, &b, 1); - return b; + } +} + +int Client::read(uint8_t *buf, size_t size) { + return recv(_sock, buf, size); } int Client::peek() { uint8_t b; + // Unlike recv, peek doesn't check to see if there's any data available, so we must if (!available()) return -1; ::peek(_sock, &b); diff --git a/libraries/Ethernet/Client.h b/libraries/Ethernet/Client.h index f0080bdf6..8725f158a 100644 --- a/libraries/Ethernet/Client.h +++ b/libraries/Ethernet/Client.h @@ -17,6 +17,7 @@ public: virtual void write(const uint8_t *buf, size_t size); virtual int available(); virtual int read(); + virtual int read(uint8_t *buf, size_t size); virtual int peek(); virtual void flush(); void stop(); diff --git a/libraries/Ethernet/utility/socket.cpp b/libraries/Ethernet/utility/socket.cpp index 628bca808..57cdf0d4c 100644 --- a/libraries/Ethernet/utility/socket.cpp +++ b/libraries/Ethernet/utility/socket.cpp @@ -146,14 +146,18 @@ uint16_t send(SOCKET s, const uint8_t * buf, uint16_t len) */ uint16_t recv(SOCKET s, uint8_t *buf, uint16_t len) { - uint16_t ret=0; - - if ( len > 0 ) + // Check how much data is available + uint16_t ret = W5100.getRXReceivedSize(s); + if (ret > len) { - W5100.recv_data_processing(s, buf, len); - W5100.execCmdSn(s, Sock_RECV); ret = len; } + + if ( ret > 0 ) + { + W5100.recv_data_processing(s, buf, ret); + W5100.execCmdSn(s, Sock_RECV); + } return ret; } From 983d8af8140f09fd3975d5deef64244102912550 Mon Sep 17 00:00:00 2001 From: amcewen Date: Sun, 2 Jan 2011 22:49:11 +0000 Subject: [PATCH 4/4] Final changes for the Client part of issue 416, which actually include the corrent return values. This should have been in the previous commit, but I'm still getting my head round git. --- libraries/Ethernet/Client.cpp | 2 +- libraries/Ethernet/utility/socket.cpp | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/libraries/Ethernet/Client.cpp b/libraries/Ethernet/Client.cpp index 51cb5eb14..96d5c7eed 100644 --- a/libraries/Ethernet/Client.cpp +++ b/libraries/Ethernet/Client.cpp @@ -77,7 +77,7 @@ int Client::available() { int Client::read() { uint8_t b; - if ( recv(_sock, &b, 1) ) + if ( recv(_sock, &b, 1) > 0 ) { // recv worked return b; diff --git a/libraries/Ethernet/utility/socket.cpp b/libraries/Ethernet/utility/socket.cpp index 57cdf0d4c..cad54a5e3 100644 --- a/libraries/Ethernet/utility/socket.cpp +++ b/libraries/Ethernet/utility/socket.cpp @@ -148,7 +148,22 @@ uint16_t recv(SOCKET s, uint8_t *buf, uint16_t len) { // Check how much data is available uint16_t ret = W5100.getRXReceivedSize(s); - if (ret > len) + if ( ret == 0 ) + { + // No data available. + uint8_t status = W5100.readSnSR(s); + if ( s == SnSR::LISTEN || s == SnSR::CLOSED || s == SnSR::CLOSE_WAIT ) + { + // The remote end has closed its side of the connection, so this is the eof state + ret = 0; + } + else + { + // The connection is still up, but there's no data waiting to be read + ret = -1; + } + } + else if (ret > len) { ret = len; }