Update Ethernet.cpp

Code cleanup, standardize formatting, added nullptr checks to help stability, reorganized to eliminate local variables to save stack space.

Author: rowingdude

libraries/Ethernet/src/Ethernet.cpp

@@ -1,37 +1,54 @@
 #include <EthernetC33.h>
 #include <EthernetClock.h>
-/*
- * The old implementation of the begin set a default mac address:
- * this does not make any sense.
- * Default mac address is in the hardware, when lwip start that mac
- * address is passed to lwip
- * If mac address needs to be changed then call the appropriate function
- * of lwIpIf before to get the interface 
- */
+
+/* -------------------------------------------------------------------------- */
+void CEthernet::initializeTimer() {
+/* -------------------------------------------------------------------------- */
+	
+    if (!ethernetTimer) {
+        ethernetTimer = new (std::nothrow) EthernetClock();
+	if (ethernetTimer) {
+	    ethernetTimer->start();
+	    delay(2);
+        }
+    }
+}
+
 
 /* -------------------------------------------------------------------------- */
 int CEthernet::begin(unsigned long timeout, unsigned long responseTimeout) {
 /* -------------------------------------------------------------------------- */  
 
-  ethernetTimer = new EthernetClock();
-  ethernetTimer->start();
-  delay(2);
-  (void)responseTimeout;
+    initializeTimer();
+    (void)responseTimeout;
 
-  int rv = 0;
+    ni = CLwipIf::getInstance().get(NI_ETHERNET);
+    if (ni) {
+        ni->DhcpSetTimeout(timeout);
+        return static_cast<int>(ni->DhcpStart());
+    }
+    return 0;
+}
 
+/* -------------------------------------------------------------------------- */  
+int CEthernet::configureStaticIP(const IPAddress& local_ip, const IPAddress& dns_server, const IPAddress& gateway, const IPAddress& subnet) {
+/* -------------------------------------------------------------------------- */  
+    initializeTimer();
+    
+    if (ni) {
+        ni->config(local_ip, gateway, subnet);
+    } else {
+        ni = CLwipIf::getInstance().get(NI_ETHERNET, local_ip, gateway, subnet);
+        if (!ni) return 0;
+    }
 
-  ni = CLwipIf::getInstance().get(NI_ETHERNET);
-  if(ni != nullptr) {
-    ni->DhcpSetTimeout(timeout);
-    rv = (int)ni->DhcpStart();
-  }
-  
-  return rv;
+    ni->DhcpNotUsed();
+    CLwipIf::getInstance().addDns(dns_server);
+    return 1;
 }
 
 /* -------------------------------------------------------------------------- */
-int CEthernet::begin(IPAddress local_ip) {
+int CEthernet::begin(const IPAddress& local_ip) {
 /* -------------------------------------------------------------------------- */
   // Assume the DNS server will be the machine on the same network as the local IP
   // but with last octet being '1'
@@ -41,7 +58,7 @@ int CEthernet::begin(IPAddress local_ip) {
 }
 
 /* -------------------------------------------------------------------------- */
-int CEthernet::begin(IPAddress local_ip, IPAddress dns_server) {
+int CEthernet::begin(const IPAddress& local_ip, const IPAddress& dns_server) {
 /* -------------------------------------------------------------------------- */  
   // Assume the gateway will be the machine on the same network as the local IP
   // but with last octet being '1'
@@ -51,56 +68,42 @@ int CEthernet::begin(IPAddress local_ip, IPAddress dns_server) {
 }
 
 /* -------------------------------------------------------------------------- */
-int CEthernet::begin(IPAddress local_ip, IPAddress dns_server, IPAddress gateway) {
+int CEthernet::begin(const IPAddress& local_ip, const IPAddress& dns_server, const IPAddress& gateway) {
 /* -------------------------------------------------------------------------- */  
   IPAddress subnet(255, 255, 255, 0);
   return begin(local_ip, dns_server, gateway, subnet);
 }
 
 /* -------------------------------------------------------------------------- */
-int CEthernet::begin(IPAddress local_ip, IPAddress dns_server, IPAddress gateway, IPAddress subnet) {
+int CEthernet::begin(const IPAddress& local_ip, const IPAddress& dns_server, const IPAddress& gateway, const IPAddress& subnet) {
 /* -------------------------------------------------------------------------- */  
 
-  ethernetTimer = new EthernetClock();
-  ethernetTimer->start();
-  delay(2);
-  
-  if (ni != nullptr) {
-    ni->config(local_ip, gateway, subnet);
-  } else {
-    ni = CLwipIf::getInstance().get(NI_ETHERNET, local_ip, gateway, subnet);
-    if (ni == nullptr) {
-      return 0;
-    }
-  }
-
-  /* If there is a local DHCP informs it of our manual IP configuration to prevent IP conflict */
-  ni->DhcpNotUsed();
-  CLwipIf::getInstance().addDns(dns_server);
-  return 1;
+  return configureStaticIP(local_ip, dns_server, gateway, subnet);
 }
 
 /* -------------------------------------------------------------------------- */
-void CEthernet::setDNS(IPAddress dns_server) {
+inline void CEthernet::setDNS(const IPAddress& dns_server) {
 /* -------------------------------------------------------------------------- */  
   CLwipIf::getInstance().addDns(dns_server);
 } 
 
 /* -------------------------------------------------------------------------- */
-int CEthernet::begin(uint8_t *mac, unsigned long timeout, unsigned long responseTimeout) {
+int CEthernet::begin(const uint8_t* mac, unsigned long timeout, unsigned long responseTimeout) {
 /* -------------------------------------------------------------------------- */  
   CLwipIf::getInstance().setMacAddress(NI_ETHERNET, mac);
   return begin(timeout, responseTimeout);
 }
 
 /* -------------------------------------------------------------------------- */
-int CEthernet::begin(uint8_t *mac_address, IPAddress local_ip) {
+int CEthernet::begin(const uint8_t* mac_address, const IPAddress& local_ip) {
 /* -------------------------------------------------------------------------- */
   // Assume the DNS server will be the machine on the same network as the local IP
   // but with last octet being '1'
-  IPAddress dns_server = local_ip;
-  dns_server[3] = 1;
-  return begin(mac_address, local_ip, dns_server);
+   
+  static const uint8_t GATEWAY_OFFSET = 1;
+  IPAddress gateway(local_ip);
+  gateway[3] = GATEWAY_OFFSET;
+  return begin(mac_address, local_ip, dns_server, gateway);
 }
 
 /* -------------------------------------------------------------------------- */
@@ -114,111 +117,92 @@ int CEthernet::begin(uint8_t *mac_address, IPAddress local_ip, IPAddress dns_ser
 }
 
 /* -------------------------------------------------------------------------- */
-int CEthernet::begin(uint8_t *mac_address, IPAddress local_ip, IPAddress dns_server, IPAddress gateway) {
+CEthernet::begin(const uint8_t* mac_address, const IPAddress& local_ip, const IPAddress& dns_server, const IPAddress& gateway) {
 /* -------------------------------------------------------------------------- */  
-  IPAddress subnet(255, 255, 255, 0);
-  return begin(mac_address, local_ip, dns_server, gateway, subnet);
+  static const IPAddress DEFAULT_SUBNET(255, 255, 255, 0);
+  return begin(mac_address, local_ip, dns_server, gateway, DEFAULT_SUBNET);
 }
 
 /* -------------------------------------------------------------------------- */
-int CEthernet::begin(uint8_t *mac, IPAddress local_ip, IPAddress dns_server, IPAddress gateway, IPAddress subnet, unsigned long timeout, unsigned long responseTimeout) {
+int CEthernet::begin(const uint8_t* mac, const IPAddress& local_ip, const IPAddress& dns_server, const IPAddress& gateway, const IPAddress& subnet, unsigned long timeout, unsigned long responseTimeout) {
 /* -------------------------------------------------------------------------- */  
-  CLwipIf::getInstance().setMacAddress(NI_ETHERNET, mac);
-  return begin(local_ip, dns_server, gateway, subnet);
+    CLwipIf::getInstance().setMacAddress(NI_ETHERNET, mac);
+    return begin(local_ip, dns_server, gateway, subnet);
 }
 
 /* -------------------------------------------------------------------------- */
-EthernetLinkStatus CEthernet::linkStatus() {
+EthernetLinkStatus CEthernet::linkStatus() const {
 /* -------------------------------------------------------------------------- */  
-  if(ni != nullptr) {
-    return (!CLwipIf::getInstance().isEthInitialized()) ? Unknown : (ni->isLinkUp() ? LinkON : LinkOFF);
-  }
-  return Unknown;
+    if (!ni) return Unknown;
+    if (!CLwipIf::getInstance().isEthInitialized()) return Unknown;
+    return ni->isLinkUp() ? LinkON : LinkOFF;
 }
 
 /* -------------------------------------------------------------------------- */
-EthernetHardwareStatus CEthernet::hardwareStatus() {
+EthernetHardwareStatus CEthernet::hardwareStatus() const {
 /* -------------------------------------------------------------------------- */
   return EthernetLwip;
 }
 
 /* -------------------------------------------------------------------------- */
 int CEthernet::disconnect() {
 /* -------------------------------------------------------------------------- */
-  ethernetTimer->stop();
-  delete(ethernetTimer);
-  ethernetTimer = NULL;
-  return 1;
+    if (ethernetTimer) {
+        ethernetTimer->stop();
+        delete ethernetTimer;
+        ethernetTimer = nullptr;
+    }
+    ni = nullptr;
+    return 1;
 }
 
 /* -------------------------------------------------------------------------- */
 int CEthernet::maintain() {
 /* -------------------------------------------------------------------------- */  
-  int rc = DHCP_CHECK_NONE;
-
-  if (ni != NULL) {
-    //we have a pointer to dhcp, use it
-    rc = ni->checkLease();
-    switch (rc) {
-      case DHCP_CHECK_NONE:
-        //nothing done
-        break;
-      case DHCP_CHECK_RENEW_OK:
-      case DHCP_CHECK_REBIND_OK:
-	//_dnsServerAddress = _dhcp->getDnsServerIp();
-        break;
-      default:
-        //this is actually a error, it will retry though
-        break;
-    }
-  }
-  return rc;
+  return ni ? ni->checkLease() : DHCP_CHECK_NONE;
 }
 
-/*
- * This function updates the LwIP stack and can be called to be sure to update
- * the stack (e.g. in case of a long loop).
- */
+/* -------------------------------------------------------------------------- */  
 void CEthernet::schedule(void) {
-  if (ni != NULL) {
-    ni->task();
-  }
+/* -------------------------------------------------------------------------- */  
+  if (ni) ni->task();
 }
 
-
-
-uint8_t *CEthernet::MACAddress(void) {
-  CLwipIf::getInstance().getMacAddress(NI_ETHERNET, mac_address);
-  return mac_address;
+/* -------------------------------------------------------------------------- */
+const uint8_t* CEthernet::MACAddress() const {
+/* -------------------------------------------------------------------------- */	
+    CLwipIf::getInstance().getMacAddress(NI_ETHERNET, const_cast<uint8_t*>(mac_address));
+    return mac_address;
 }
 
-void CEthernet::MACAddress(uint8_t *mac) {
-  CLwipIf::getInstance().getMacAddress(NI_ETHERNET, mac);
+/* -------------------------------------------------------------------------- */
+void CEthernet::MACAddress(uint8_t* mac) const {
+/* -------------------------------------------------------------------------- */
+    CLwipIf::getInstance().getMacAddress(NI_ETHERNET, mac);
 }
 
-IPAddress CEthernet::localIP() {
-  if(ni != nullptr) {
-      return IPAddress(ni->getIpAdd());   
-   }
-   return IPAddress((uint32_t)0);
+/* -------------------------------------------------------------------------- */
+IPAddress CEthernet::localIP() const {
+/* -------------------------------------------------------------------------- */
+    return ni ? IPAddress(ni->getIpAdd()) : IPAddress(static_cast<uint32_t>(0));
 }
 
-IPAddress CEthernet::subnetMask() {
-  if(ni != nullptr) {
-      return IPAddress(ni->getNmAdd());   
-   }
-   return IPAddress((uint32_t)0);
+/* -------------------------------------------------------------------------- */
+IPAddress CEthernet::subnetMask() const {
+/* -------------------------------------------------------------------------- */
+    return ni ? IPAddress(ni->getNmAdd()) : IPAddress(static_cast<uint32_t>(0));
 }
 
-IPAddress CEthernet::gatewayIP() {
-  if(ni != nullptr) {
-      return IPAddress(ni->getGwAdd());   
-   }
-   return IPAddress((uint32_t)0);
+/* -------------------------------------------------------------------------- */
+IPAddress CEthernet::gatewayIP() const {
+/* -------------------------------------------------------------------------- */
+    return ni ? IPAddress(ni->getGwAdd()) : IPAddress(static_cast<uint32_t>(0));
 }
 
-IPAddress CEthernet::dnsServerIP() {
-  return CLwipIf::getInstance().getDns();
+/* -------------------------------------------------------------------------- */
+IPAddress CEthernet::dnsServerIP() const {
+/* -------------------------------------------------------------------------- */
+    return CLwipIf::getInstance().getDns();
 }
 
 CEthernet Ethernet;