Some improvements, primarily concerning naming and constness.

Dhcp.cpp

@@ -9,7 +9,15 @@
 #include "Arduino.h"
 #include "util.h"
 
-int DhcpClass::beginWithDHCP(uint8_t *mac, unsigned long timeout, unsigned long responseTimeout)
+DhcpClass::~DhcpClass() {
+	delete[] _dhcpLocalIp;
+	delete[] _dhcpSubnetMask;
+	delete[] _dhcpGatewayIp;
+	delete[] _dhcpDhcpServerIp;
+	delete[] _dhcpDnsServerIp;
+}
+
+int DhcpClass::beginWithDHCP(uint8_t *mac, const unsigned long timeout, const unsigned long responseTimeout)
 {
     _dhcpLeaseTime=0;
     _dhcpT1=0;
@@ -132,7 +140,7 @@ void DhcpClass::presend_DHCP()
 {
 }
 
-void DhcpClass::send_DHCP_MESSAGE(uint8_t messageType, uint16_t secondsElapsed)
+void DhcpClass::send_DHCP_MESSAGE(const uint8_t messageType, const uint16_t secondsElapsed)
 {
     uint8_t buffer[32];
     memset(buffer, 0, 32);
@@ -251,7 +259,7 @@ void DhcpClass::send_DHCP_MESSAGE(uint8_t messageType, uint16_t secondsElapsed)
     _dhcpUdpSocket.endPacket();
 }
 
-uint8_t DhcpClass::parseDHCPResponse(unsigned long responseTimeout, uint32_t& transactionId)
+uint8_t DhcpClass::parseDHCPResponse(const unsigned long responseTimeout, uint32_t& transactionId)
 {
     uint8_t type = 0;
     uint8_t opt_len = 0;

Dhcp.h

@@ -167,19 +167,20 @@ class DhcpClass {
   int request_DHCP_lease();
   void reset_DHCP_lease();
   void presend_DHCP();
-  void send_DHCP_MESSAGE(uint8_t, uint16_t);
+  void send_DHCP_MESSAGE(const uint8_t, const uint16_t);
   void printByte(char *, uint8_t);
 
-  uint8_t parseDHCPResponse(unsigned long responseTimeout, uint32_t& transactionId);
+  uint8_t parseDHCPResponse(const unsigned long responseTimeout, uint32_t& transactionId);
 public:
   IPAddress getLocalIp();
   IPAddress getSubnetMask();
   IPAddress getGatewayIp();
   IPAddress getDhcpServerIp();
   IPAddress getDnsServerIp();
 
-  int beginWithDHCP(uint8_t *, unsigned long timeout = 60000, unsigned long responseTimeout = 4000);
+  int beginWithDHCP(uint8_t *, const unsigned long timeout = 60000, const unsigned long responseTimeout = 4000);
   int checkLease();
+  ~DhcpClass();
 };
 
 #endif

Dns.cpp

@@ -11,43 +11,6 @@
 //#include <stdlib.h>
 #include "Arduino.h"
 
-
-#define SOCKET_NONE	255
-// Various flags and header field values for a DNS message
-#define UDP_HEADER_SIZE	8
-#define DNS_HEADER_SIZE	12
-#define TTL_SIZE        4
-#define QUERY_FLAG               (0)
-#define RESPONSE_FLAG            (1<<15)
-#define QUERY_RESPONSE_MASK      (1<<15)
-#define OPCODE_STANDARD_QUERY    (0)
-#define OPCODE_INVERSE_QUERY     (1<<11)
-#define OPCODE_STATUS_REQUEST    (2<<11)
-#define OPCODE_MASK              (15<<11)
-#define AUTHORITATIVE_FLAG       (1<<10)
-#define TRUNCATION_FLAG          (1<<9)
-#define RECURSION_DESIRED_FLAG   (1<<8)
-#define RECURSION_AVAILABLE_FLAG (1<<7)
-#define RESP_NO_ERROR            (0)
-#define RESP_FORMAT_ERROR        (1)
-#define RESP_SERVER_FAILURE      (2)
-#define RESP_NAME_ERROR          (3)
-#define RESP_NOT_IMPLEMENTED     (4)
-#define RESP_REFUSED             (5)
-#define RESP_MASK                (15)
-#define TYPE_A                   (0x0001)
-#define CLASS_IN                 (0x0001)
-#define LABEL_COMPRESSION_MASK   (0xC0)
-// Port number that DNS servers listen on
-#define DNS_PORT        53
-
-// Possible return codes from ProcessResponse
-#define SUCCESS          1
-#define TIMED_OUT        -1
-#define INVALID_SERVER   -2
-#define TRUNCATED        -3
-#define INVALID_RESPONSE -4
-
 void DNSClient::begin(const IPAddress& aDNSServer)
 {
     iDNSServer = aDNSServer;
@@ -139,12 +102,12 @@ int DNSClient::getHostByName(const char* aHostname, IPAddress& aResult)
                         while ((wait_retries < 3) && (ret == TIMED_OUT))
                         {
                             ret = ProcessResponse(5000, aResult);
-                            wait_retries++;
+							++wait_retries;
                         }
                     }
                 }
             }
-            retries++;
+			++retries;
         }
 
         // We're done with the socket now
@@ -216,7 +179,7 @@ uint16_t DNSClient::BuildRequest(const char* aName)
             // And then write out the section
             iUdp.write((uint8_t*)start, end-start);
         }
-        start = end+1;
+		start = end + 1;
     }
 
     // We've got to the end of the question name, so
@@ -234,9 +197,9 @@ uint16_t DNSClient::BuildRequest(const char* aName)
 }
 
 
-uint16_t DNSClient::ProcessResponse(uint16_t aTimeout, IPAddress& aAddress)
+uint16_t DNSClient::ProcessResponse(const uint16_t aTimeout, const IPAddress& aAddress)
 {
-    uint32_t startTime = millis();
+	const uint32_t startTime = millis();
 
     // Wait for a response packet
     while(iUdp.parsePacket() <= 0)
@@ -251,7 +214,7 @@ uint16_t DNSClient::ProcessResponse(uint16_t aTimeout, IPAddress& aAddress)
     //uint8_t header[DNS_HEADER_SIZE]; // Enough space to reuse for the DNS header
     union {
         uint8_t  byte[DNS_HEADER_SIZE]; // Enough space to reuse for the DNS header
-        uint16_t word[DNS_HEADER_SIZE/2];
+		uint16_t word[DNS_HEADER_SIZE / 2];
     } header;
 
     // Check that it's a response from the right server and the right port
@@ -297,7 +260,7 @@ uint16_t DNSClient::ProcessResponse(uint16_t aTimeout, IPAddress& aAddress)
     }
 
     // Skip over any questions
-    for (uint16_t i =0; i < htons(header.word[2]); i++)
+	for (uint16_t i = 0; i < htons(header.word[2]); ++i)
     {
         // Skip over the name
         uint8_t len;
@@ -316,7 +279,7 @@ uint16_t DNSClient::ProcessResponse(uint16_t aTimeout, IPAddress& aAddress)
         } while (len != 0);
 
         // Now jump over the type and class
-        for (int i =0; i < 4; i++)
+		for (int i =0; i < 4; ++i)
         {
             iUdp.read(); // we don't care about the returned byte
         }
@@ -327,7 +290,7 @@ uint16_t DNSClient::ProcessResponse(uint16_t aTimeout, IPAddress& aAddress)
     // type A answer) and some authority and additional resource records but
     // we're going to ignore all of them.
 
-    for (uint16_t i =0; i < answerCount; i++)
+	for (uint16_t i = 0; i < answerCount; ++i)
     {
         // Skip the name
         uint8_t len;
@@ -370,7 +333,7 @@ uint16_t DNSClient::ProcessResponse(uint16_t aTimeout, IPAddress& aAddress)
         iUdp.read((uint8_t*)&answerClass, sizeof(answerClass));
 
         // Ignore the Time-To-Live as we don't do any caching
-        for (int i =0; i < TTL_SIZE; i++)
+		for (int i =0; i < TTL_SIZE; ++i)
         {
             iUdp.read(); // we don't care about the returned byte
         }
@@ -394,7 +357,7 @@ uint16_t DNSClient::ProcessResponse(uint16_t aTimeout, IPAddress& aAddress)
         else
         {
             // This isn't an answer type we're after, move onto the next one
-            for (uint16_t i =0; i < htons(header_flags); i++)
+			for (uint16_t i =0; i < htons(header_flags); ++i)
             {
                 iUdp.read(); // we don't care about the returned byte
             }

Dns.h

@@ -31,11 +31,48 @@ class DNSClient
 
 protected:
     uint16_t BuildRequest(const char* aName);
-    uint16_t ProcessResponse(uint16_t aTimeout, IPAddress& aAddress);
+	uint16_t ProcessResponse(const uint16_t aTimeout, const IPAddress& aAddress);
 
     IPAddress iDNSServer;
     uint16_t iRequestId;
     EthernetUDP iUdp;
+
+	private:
+	static constexpr uint8_t SOCKET_NONE = 255;
+	// Various flags and header field values for a DNS message
+	static constexpr uint8_t UDP_HEADER_SIZE          = 8;
+	static constexpr uint8_t DNS_HEADER_SIZE          = 12;
+	static constexpr uint8_t TTL_SIZE                 = 4;
+	static constexpr uint8_t QUERY_FLAG               = (0);
+	static constexpr uint8_t RESPONSE_FLAG            = (1<<15);
+	static constexpr uint8_t QUERY_RESPONSE_MASK      = (1<<15);
+	static constexpr uint8_t OPCODE_STANDARD_QUERY    = (0);
+	static constexpr uint8_t OPCODE_INVERSE_QUERY     = (1<<11);
+	static constexpr uint8_t OPCODE_STATUS_REQUEST    = (2<<11);
+	static constexpr uint8_t OPCODE_MASK              = (15<<11);
+	static constexpr uint8_t AUTHORITATIVE_FLAG       = (1<<10);
+	static constexpr uint8_t TRUNCATION_FLAG          = (1<<9);
+	static constexpr uint8_t RECURSION_DESIRED_FLAG   = (1<<8);
+	static constexpr uint8_t RECURSION_AVAILABLE_FLAG = (1<<7);
+	static constexpr uint8_t RESP_NO_ERROR            = (0);
+	static constexpr uint8_t RESP_FORMAT_ERROR        = (1);
+	static constexpr uint8_t RESP_SERVER_FAILURE      = (2);
+	static constexpr uint8_t RESP_NAME_ERROR          = (3);
+	static constexpr uint8_t RESP_NOT_IMPLEMENTED     = (4);
+	static constexpr uint8_t RESP_REFUSED             = (5);
+	static constexpr uint8_t RESP_MASK                = (15);
+	static constexpr uint8_t TYPE_A                   = (0x0001);
+	static constexpr uint8_t CLASS_IN                 = (0x0001);
+	static constexpr uint8_t LABEL_COMPRESSION_MASK   = (0xC0);
+	// Port number that DNS servers listen on
+	static constexpr uint8_t DNS_PORT                  = 53;
+
+	// Possible return codes from ProcessResponse
+	static constexpr uint8_t SUCCESS                   = 1;
+	static constexpr uint8_t TIMED_OUT                 = -1;
+	static constexpr uint8_t INVALID_SERVER            = -2;
+	static constexpr uint8_t TRUNCATED                 = -3;
+	static constexpr uint8_t INVALID_RESPONSE          = -4;
 };
 
 #endif

Ethernet.cpp

@@ -3,9 +3,9 @@
 #include "Dhcp.h"
 
 IPAddress EthernetClass::_dnsServerAddress;
-DhcpClass* EthernetClass::_dhcp = NULL;
+DhcpClass* EthernetClass::_dhcp = nullptr;
 
-int EthernetClass::begin(uint8_t *mac, unsigned long timeout, unsigned long responseTimeout)
+int EthernetClass::begin(uint8_t *mac, const unsigned long timeout, const unsigned long responseTimeout)
 {
 	static DhcpClass s_dhcp;
 	_dhcp = &s_dhcp;
@@ -18,8 +18,8 @@ int EthernetClass::begin(uint8_t *mac, unsigned long timeout, unsigned long resp
 	SPI.endTransaction();
 
 	// Now try to get our config info from a DHCP server
-	int ret = _dhcp->beginWithDHCP(mac, timeout, responseTimeout);
-	if (ret == 1) {
+	int result = _dhcp->beginWithDHCP(mac, timeout, responseTimeout);
+	if (result == 1) {
 		// We've successfully found a DHCP server and got our configuration
 		// info, so set things accordingly
 		SPI.beginTransaction(SPI_ETHERNET_SETTINGS);
@@ -30,10 +30,10 @@ int EthernetClass::begin(uint8_t *mac, unsigned long timeout, unsigned long resp
 		_dnsServerAddress = _dhcp->getDnsServerIp();
 		socketPortRand(micros());
 	}
-	return ret;
+	return result;
 }
 
-void EthernetClass::begin(uint8_t *mac, IPAddress ip)
+void EthernetClass::begin(uint8_t *mac, const IPAddress ip)
 {
 	// Assume the DNS server will be the machine on the same network as the local IP
 	// but with last octet being '1'
@@ -42,7 +42,7 @@ void EthernetClass::begin(uint8_t *mac, IPAddress ip)
 	begin(mac, ip, dns);
 }
 
-void EthernetClass::begin(uint8_t *mac, IPAddress ip, IPAddress dns)
+void EthernetClass::begin(uint8_t *mac, const IPAddress ip, const IPAddress dns)
 {
 	// Assume the gateway will be the machine on the same network as the local IP
 	// but with last octet being '1'
@@ -51,13 +51,13 @@ void EthernetClass::begin(uint8_t *mac, IPAddress ip, IPAddress dns)
 	begin(mac, ip, dns, gateway);
 }
 
-void EthernetClass::begin(uint8_t *mac, IPAddress ip, IPAddress dns, IPAddress gateway)
+void EthernetClass::begin(uint8_t *mac, const IPAddress ip, const IPAddress dns, const IPAddress gateway)
 {
 	IPAddress subnet(255, 255, 255, 0);
 	begin(mac, ip, dns, gateway, subnet);
 }
 
-void EthernetClass::begin(uint8_t *mac, IPAddress ip, IPAddress dns, IPAddress gateway, IPAddress subnet)
+void EthernetClass::begin(uint8_t *mac, const IPAddress ip, const IPAddress dns, const IPAddress gateway, const IPAddress subnet)
 {
 	W5100.init();
 	SPI.beginTransaction(SPI_ETHERNET_SETTINGS);
@@ -77,11 +77,11 @@ void EthernetClass::begin(uint8_t *mac, IPAddress ip, IPAddress dns, IPAddress g
 
 int EthernetClass::maintain()
 {
-	int rc = DHCP_CHECK_NONE;
-	if (_dhcp != NULL) {
+	int result = DHCP_CHECK_NONE;
+	if (_dhcp != nullptr) {
 		// we have a pointer to dhcp, use it
-		rc = _dhcp->checkLease();
-		switch (rc) {
+		result = _dhcp->checkLease();
+		switch (result) {
 		case DHCP_CHECK_NONE:
 			//nothing done
 			break;
@@ -100,34 +100,34 @@ int EthernetClass::maintain()
 			break;
 		}
 	}
-	return rc;
+	return result;
 }
 
 IPAddress EthernetClass::localIP()
 {
-	IPAddress ret;
+	IPAddress address;
 	SPI.beginTransaction(SPI_ETHERNET_SETTINGS);
-	W5100.getIPAddress(ret.raw_address());
+	W5100.getIPAddress(address.raw_address());
 	SPI.endTransaction();
-	return ret;
+	return address;
 }
 
 IPAddress EthernetClass::subnetMask()
 {
-	IPAddress ret;
+	IPAddress address;
 	SPI.beginTransaction(SPI_ETHERNET_SETTINGS);
-	W5100.getSubnetMask(ret.raw_address());
+	W5100.getSubnetMask(address.raw_address());
 	SPI.endTransaction();
-	return ret;
+	return address;
 }
 
 IPAddress EthernetClass::gatewayIP()
 {
-	IPAddress ret;
+	IPAddress result;
 	SPI.beginTransaction(SPI_ETHERNET_SETTINGS);
-	W5100.getGatewayIp(ret.raw_address());
+	W5100.getGatewayIp(result.raw_address());
 	SPI.endTransaction();
-	return ret;
+	return result;
 }
 
 

Ethernet.h

@@ -17,14 +17,14 @@ class EthernetClass {
 	// Initialise the Ethernet shield to use the provided MAC address and
 	// gain the rest of the configuration through DHCP.
 	// Returns 0 if the DHCP configuration failed, and 1 if it succeeded
-	static int begin(uint8_t *mac, unsigned long timeout = 60000, unsigned long responseTimeout = 4000);
+	static int begin(uint8_t *mac, const unsigned long timeout = 60000, const unsigned long responseTimeout = 4000);
 	static int maintain();
 
 	// Manaul configuration
-	static void begin(uint8_t *mac, IPAddress ip);
-	static void begin(uint8_t *mac, IPAddress ip, IPAddress dns);
-	static void begin(uint8_t *mac, IPAddress ip, IPAddress dns, IPAddress gateway);
-	static void begin(uint8_t *mac, IPAddress ip, IPAddress dns, IPAddress gateway, IPAddress subnet);
+	static void begin(uint8_t *mac, const IPAddress, const ip);
+	static void begin(uint8_t *mac, const IPAddress ip, const IPAddress dns);
+	static void begin(uint8_t *mac, const IPAddress ip, const IPAddress dns, IPAddress gateway);
+	static void begin(uint8_t *mac, const IPAddress ip, const IPAddress dns, const IPAddress gateway, const IPAddress subnet);
 
 	static IPAddress localIP();
 	static IPAddress subnetMask();