Fixing compiler warnings due to stronger compile-time checks (#69)

* Fixing issues uncovered by stronger compiler warnings
* Adding const qualifier for parameter 'char * fmt' -> 'const char * fmt' thereby eliminating -Wwrite-strings warnings
* Adding missing cases to switch statements
* Removing unused variable 'index'
* Reording initialisation order of ArduinoIoTCloud in order to eliminate '-Wreorder' warnings
* Adding const qualifier for parameter 'char * fmt' -> 'const char * fmt' thereby eliminating -Wwrite-strings warnings
* Prefixing 'connection' with an underscore -> '_connection' in order to be consistent with all other member variables
* Preventing multiple definitions of SECRET_PASS mutually overwriting one another by prefixing the correct connection type
* Changing type of timeout intervals to type unsigned long - otherwise there is a type inconsistency and probable bug when checking the connection time interval
* Removing duplicate ERROR case
* Adding missing cases and simplifying code
* Changing type of timeout intervals to type unsigned long - otherwise there is a type inconsistency and probable bug when checking the connection time interval
* Using default clause for missing switch statements (no constant defined for state DISCONNECTING)
* Fix -Wreorder error
* Simplifying WifiConnectionManager
* Replacing '#warning' with '#pragma message'
* Fixing comparison between unsigned and signed type
* Ignoring error stemming from warning about deprecated functions
* Rectifying signed/unsigned comparison
* char * arrays should be compared via strcmp
* Commenting out unused parameters in order to prevent -Wunused-parameter
* Removing unused variable connection buffer
* Adding missing switch cases
* Correct initlisation of struct tm t
* Correct printf specifier for unsigned long it is %lu
* Adding missing initializer for tm_isdst
* Performing cast in order to guarantuee correct comparison
* Fixing formatting
* Disabling '-Wunused-variable' to prevent false positives
* Removing prefix 'WIFI_' and 'GSM_' because Arduino Create is looking for 'SECRET_SSID', 'SECRET_PASS', etc. when importing a example project into Arduino Create
* Correcting comment
* Removing prefix of parameter 'newState' wherever used, since prefixes usually indicate class members
* Consting parameter 'newState' - defensive programming

Author: aentinger

examples/ArduinoIoTCloud_LED_switch/arduino_secrets.h

@@ -1,15 +1,15 @@
-/*
-  Fill in your login credentials:
+#include <ConnectionManager.h>
 
-  The following lines are used for WiFi enabled boards (MKR1000, MKR WiFi 1010)
-*/
-#define SECRET_SSID "YOUR_WIFI_NETWORK_NAME"
-#define SECRET_PASS "YOUR_WIFI_PASSWORD"
-/*
-  If you prefer using a MKR GSM 1400 comment the lines above and uncommet the following.
-  PIN, APN, Login and Password are supplied by your Cellular Data provider.
-*/
-#define SECRET_PIN ""
-#define SECRET_APN ""
-#define SECRET_LOGIN ""
-#define SECRET_PASS ""
+/* MKR1000, MKR WiFi 1010 */
+#if defined(BOARD_HAS_WIFI)
+  #define SECRET_SSID "YOUR_WIFI_NETWORK_NAME"
+  #define SECRET_PASS "YOUR_WIFI_PASSWORD"
+#endif
+
+/* MKR GSM 1400 */
+#if defined(BOARD_HAS_GSM)
+  #define SECRET_PIN ""
+  #define SECRET_APN ""
+  #define SECRET_LOGIN ""
+  #define SECRET_PASS ""
+#endif

examples/ArduinoIoTCloud_Travis_CI/arduino_secrets.h

@@ -1,9 +1,15 @@
-/* (MKR1000, MKR WiFi 1010) */
-#define SECRET_SSID "YOUR_WIFI_NETWORK_NAME"
-#define SECRET_PASS "YOUR_WIFI_PASSWORD"
+#include <ConnectionManager.h>
 
-/* MKR GSM 1400 comment the lines above and uncommet the following. */
-#define SECRET_PIN ""
-#define SECRET_APN ""
-#define SECRET_LOGIN ""
-#define SECRET_PASS ""
+/* MKR1000, MKR WiFi 1010 */
+#if defined(BOARD_HAS_WIFI)
+  #define SECRET_SSID "YOUR_WIFI_NETWORK_NAME"
+  #define SECRET_PASS "YOUR_WIFI_PASSWORD"
+#endif
+
+/* MKR GSM 1400 */
+#if defined(BOARD_HAS_GSM)
+  #define SECRET_PIN ""
+  #define SECRET_APN ""
+  #define SECRET_LOGIN ""
+  #define SECRET_PASS ""
+#endif

examples/GSM_Cloud_Blink/GSM_Cloud_Blink.ino

@@ -4,22 +4,22 @@
 
 #ifndef SECRET_PIN
   #define SECRET_PIN ""
-  #warning "You need to define SECRET_PIN in tab/arduino_secrets.h"
+  #pragma message "You need to define SECRET_PIN in tab/arduino_secrets.h"
 #endif
 
 #ifndef SECRET_APN
   #define SECRET_APN ""
-  #warning "You need to define SECRET_PIN in tab/arduino_secrets.h"
+  #pragma message "You need to define SECRET_PIN in tab/arduino_secrets.h"
 #endif
 
 #ifndef SECRET_USER_NAME
   #define SECRET_USER_NAME ""
-  #warning "You need to define SECRET_USER_NAME in tab/arduino_secrets.h"
+  #pragma message "You need to define SECRET_USER_NAME in tab/arduino_secrets.h"
 #endif
 
 #ifndef SECRET_PASSWORD
   #define SECRET_PASSWORD ""
-  #warning "You need to define SECRET_PASSWORD in tab/arduino_secrets.h"
+  #pragma message "You need to define SECRET_PASSWORD in tab/arduino_secrets.h"
 #endif
 
 String cloudSerialBuffer = ""; // the string used to compose network messages from the received characters
@@ -76,7 +76,7 @@ void handleString() {
 void sendString(String stringToSend) {
   // send the characters one at a time
   char lastSentChar = 0;
-  for (int i = 0; i < stringToSend.length(); i++) {
+  for (unsigned int i = 0; i < stringToSend.length(); i++) {
     lastSentChar = stringToSend.charAt(i);
     CloudSerial.write(lastSentChar);
   }

examples/WiFi_Cloud_Blink/WiFi_Cloud_Blink.ino

@@ -4,12 +4,12 @@
 
 #ifndef SECRET_WIFI_NAME
   #define SECRET_WIFI_NAME ""
-  #warning "You need to define SECRET_WIFI_NAME in tab/arduino_secrets.h"
+  #pragma message "You need to define SECRET_WIFI_NAME in tab/arduino_secrets.h"
 #endif
 
 #ifndef SECRET_PASSWORD
   #define SECRET_PASSWORD ""
-  #warning "You need to define SECRET_PASSWORD in tab/arduino_secrets.h"
+  #pragma message "You need to define SECRET_PASSWORD in tab/arduino_secrets.h"
 #endif
 
 String cloudSerialBuffer = ""; // the string used to compose network messages from the received characters
@@ -66,7 +66,7 @@ void handleString() {
 void sendString(String stringToSend) {
   // send the characters one at a time
   char lastSentChar = 0;
-  for (int i = 0; i < stringToSend.length(); i++) {
+  for (unsigned int i = 0; i < stringToSend.length(); i++) {
     lastSentChar = stringToSend.charAt(i);
     CloudSerial.write(lastSentChar);
   }

src/ArduinoIoTCloud.cpp

@@ -52,18 +52,18 @@ static unsigned long getTime() {
 }
 
 ArduinoIoTCloudClass::ArduinoIoTCloudClass() :
+  _connection(NULL),
   _thing_id(""),
   _bearSslClient(NULL),
   _mqttClient(NULL),
-  connection(NULL),
+  _lastSyncRequestTickTime(0),
   _stdinTopic(""),
   _stdoutTopic(""),
   _shadowTopicOut(""),
   _shadowTopicIn(""),
   _dataTopicOut(""),
   _dataTopicIn(""),
   _otaTopic(""),
-  _lastSyncRequestTickTime(0),
   _on_sync_event_callback(NULL),
   _on_connect_event_callback(NULL),
   _on_disconnect_event_callback(NULL) {
@@ -83,7 +83,7 @@ ArduinoIoTCloudClass::~ArduinoIoTCloudClass() {
 }
 
 int ArduinoIoTCloudClass::begin(ConnectionManager *c, String brokerAddress, uint16_t brokerPort) {
-  connection = c;
+  _connection = c;
   Client &connectionClient = c->getClient();
   _brokerAddress = brokerAddress;
   _brokerPort = brokerPort;
@@ -131,8 +131,8 @@ int ArduinoIoTCloudClass::begin(Client& net, String brokerAddress, uint16_t brok
   if (_bearSslClient) {
     delete _bearSslClient;
   }
-  if (connection != NULL) {
-    _bearSslClient = new BearSSLClient(connection->getClient());
+  if (_connection != NULL) {
+    _bearSslClient = new BearSSLClient(_connection->getClient());
   } else {
     _bearSslClient = new BearSSLClient(*_net);
   }
@@ -141,8 +141,8 @@ int ArduinoIoTCloudClass::begin(Client& net, String brokerAddress, uint16_t brok
   _mqttClient = new MqttClient(*_bearSslClient);
 
   // Bind ArduinoBearSSL callback using static "non-method" function
-  if (connection != NULL) {
-    getTimeConnection = connection;
+  if (_connection != NULL) {
+    getTimeConnection = _connection;
     ArduinoBearSSL.onGetTime(getTime);
   }
 
@@ -211,12 +211,7 @@ bool ArduinoIoTCloudClass::disconnect() {
   return true;
 }
 
-void ArduinoIoTCloudClass::update(CallbackFunc onSyncCompleteCallback) {
-  // If user call update() without parameters use the default ones
-  update(MAX_RETRIES, RECONNECTION_TIMEOUT, onSyncCompleteCallback);
-}
-
-void ArduinoIoTCloudClass::update(int const reconnectionMaxRetries, int const reconnectionTimeoutMs, CallbackFunc onSyncCompleteCallback) {
+void ArduinoIoTCloudClass::update(int const /* reconnectionMaxRetries */, int const /* reconnectionTimeoutMs */, CallbackFunc onSyncCompleteCallback) {
   // Check if a primitive property wrapper is locally changed
   Thing.updateTimestampOnLocallyChangedProperties();
 
@@ -328,7 +323,6 @@ void ArduinoIoTCloudClass::handleMessage(int length) {
   String topic = _mqttClient->messageTopic();
 
   byte bytes[length];
-  int index = 0;
 
   for (int i = 0; i < length; i++) {
     bytes[i] = _mqttClient->read();
@@ -356,10 +350,10 @@ void ArduinoIoTCloudClass::requestLastValue() {
 }
 
 void ArduinoIoTCloudClass::connectionCheck() {
-  if (connection != NULL) {
-    connection->check();
+  if (_connection != NULL) {
+    _connection->check();
 
-    if (connection->getStatus() != NetworkConnectionState::CONNECTED) {
+    if (_connection->getStatus() != NetworkConnectionState::CONNECTED) {
       if (iotStatus == ArduinoIoTConnectionStatus::CONNECTED) {
         setIoTConnectionState(ArduinoIoTConnectionStatus::DISCONNECTED);
       }
@@ -416,15 +410,16 @@ void ArduinoIoTCloudClass::connectionCheck() {
   }
 }
 
-void ArduinoIoTCloudClass::setIoTConnectionState(ArduinoIoTConnectionStatus _newState) {
-  switch (_newState) {
+void ArduinoIoTCloudClass::setIoTConnectionState(ArduinoIoTConnectionStatus newState) {
+  iotStatus = newState;
+  switch (iotStatus) {
+    case ArduinoIoTConnectionStatus::IDLE:                                                                                  break;
     case ArduinoIoTConnectionStatus::ERROR:        debugMessage(DebugLevel::Error, "Arduino, we have a problem.");          break;
     case ArduinoIoTConnectionStatus::CONNECTING:   debugMessage(DebugLevel::Error, "Connecting to Arduino IoT Cloud...");   break;
     case ArduinoIoTConnectionStatus::RECONNECTING: debugMessage(DebugLevel::Error, "Reconnecting to Arduino IoT Cloud..."); break;
     case ArduinoIoTConnectionStatus::CONNECTED:    debugMessage(DebugLevel::Error, "Connected to Arduino IoT Cloud");       break;
     case ArduinoIoTConnectionStatus::DISCONNECTED: debugMessage(DebugLevel::Error, "Disconnected from Arduino IoT Cloud");  break;
   }
-  iotStatus = _newState;
 }
 
 void ArduinoIoTCloudClass::printDebugInfo() {

src/ArduinoIoTCloud.h

@@ -91,8 +91,9 @@ class ArduinoIoTCloudClass {
     inline void update() {
       update(NULL);
     }
-    void update(CallbackFunc onSyncCompleteCallback) __attribute__((deprecated)); /* Attention: Function is deprecated - use 'addCallback(ArduinoIoTCloudConnectionEvent::SYNC, &onSync)' for adding a onSyncCallback instead */
-
+    inline void update(CallbackFunc onSyncCompleteCallback) __attribute__((deprecated)) { /* Attention: Function is deprecated - use 'addCallback(ArduinoIoTCloudConnectionEvent::SYNC, &onSync)' for adding a onSyncCallback instead */
+      update(MAX_RETRIES, RECONNECTION_TIMEOUT, onSyncCompleteCallback);
+    }
     // defined for users who want to specify max reconnections reties and timeout between them
     inline void update(int const reconnectionMaxRetries, int const reconnectionTimeoutMs) {
       update(reconnectionMaxRetries, reconnectionTimeoutMs, NULL);
@@ -193,10 +194,10 @@ class ArduinoIoTCloudClass {
     ArduinoIoTConnectionStatus getIoTStatus() {
       return iotStatus;
     }
-    void setIoTConnectionState(ArduinoIoTConnectionStatus _newState);
+    void setIoTConnectionState(ArduinoIoTConnectionStatus newState);
   private:
     ArduinoIoTConnectionStatus iotStatus = ArduinoIoTConnectionStatus::IDLE;
-    ConnectionManager *connection;
+    ConnectionManager * _connection;
     static void onMessage(int length);
     void handleMessage(int length);
     ArduinoIoTSynchronizationStatus _syncStatus = ArduinoIoTSynchronizationStatus::SYNC_STATUS_SYNCHRONIZED;

src/EthernetConnectionManager.cpp

@@ -128,13 +128,21 @@ void EthConnectionManager::check() {
           debugMessage(DebugLevel::Info, "Connected");
         }
         break;
+      case NetworkConnectionState::DISCONNECTING: {
+          /* Do nothing */
+        }
+        break;
       case NetworkConnectionState::DISCONNECTED: {
           debugMessage(DebugLevel::Error, "Connection lost.");
           debugMessage(DebugLevel::Info, "Attempting reconnection");
           changeConnectionState(NetworkConnectionState::CONNECTING);
           //wifiClient.stop();
         }
         break;
+      case NetworkConnectionState::ERROR: {
+          /* Do nothing */
+        }
+        break;
     }
     lastConnectionTickTime = now;
   }
@@ -144,17 +152,17 @@ void EthConnectionManager::check() {
    PRIVATE MEMBER FUNCTIONS
  ******************************************************************************/
 
-void EthConnectionManager::changeConnectionState(NetworkConnectionState _newState) {
-  netConnectionState = _newState;
-  int newInterval = CHECK_INTERVAL_IDLE;
-  switch (_newState) {
-    case NetworkConnectionState::INIT:         newInterval = CHECK_INTERVAL_INIT;         break;
-    case NetworkConnectionState::CONNECTING:   newInterval = CHECK_INTERVAL_CONNECTING;   break;
-    case NetworkConnectionState::GETTIME:      newInterval = CHECK_INTERVAL_GETTIME;      break;
-    case NetworkConnectionState::CONNECTED:    newInterval = CHECK_INTERVAL_CONNECTED;    break;
-    case NetworkConnectionState::DISCONNECTED: newInterval = CHECK_INTERVAL_DISCONNECTED; break;
+void EthConnectionManager::changeConnectionState(NetworkConnectionState const newState) {
+  netConnectionState = newState;
+  switch (netConnectionState) {
+    case NetworkConnectionState::INIT:          connectionTickTimeInterval = CHECK_INTERVAL_INIT;          break;
+    case NetworkConnectionState::CONNECTING:    connectionTickTimeInterval = CHECK_INTERVAL_CONNECTING;    break;
+    case NetworkConnectionState::GETTIME:       connectionTickTimeInterval = CHECK_INTERVAL_GETTIME;       break;
+    case NetworkConnectionState::CONNECTED:     connectionTickTimeInterval = CHECK_INTERVAL_CONNECTED;     break;
+    case NetworkConnectionState::DISCONNECTED:  connectionTickTimeInterval = CHECK_INTERVAL_DISCONNECTED;  break;
+    case NetworkConnectionState::ERROR:         connectionTickTimeInterval = CHECK_INTERVAL_ERROR;         break;
+    default:                                    connectionTickTimeInterval = CHECK_INTERVAL_IDLE;          break;
   }
-  connectionTickTimeInterval = newInterval;
   lastConnectionTickTime = millis();
 }
 

src/EthernetConnectionManager.h

@@ -49,23 +49,23 @@ class EthConnectionManager : public ConnectionManager {
 
   private:
 
-    void changeConnectionState(NetworkConnectionState _newState);
+    void changeConnectionState(NetworkConnectionState const newState);
 
-    const int CHECK_INTERVAL_IDLE = 100;
-    const int CHECK_INTERVAL_INIT = 100;
-    const int CHECK_INTERVAL_CONNECTING = 500;
-    const int CHECK_INTERVAL_GETTIME = 100;
-    const int CHECK_INTERVAL_CONNECTED = 10000;
-    const int CHECK_INTERVAL_RETRYING = 5000;
-    const int CHECK_INTERVAL_DISCONNECTED = 1000;
-    const int CHECK_INTERVAL_ERROR = 500;
+    const unsigned long CHECK_INTERVAL_IDLE = 100;
+    const unsigned long CHECK_INTERVAL_INIT = 100;
+    const unsigned long CHECK_INTERVAL_CONNECTING = 500;
+    const unsigned long CHECK_INTERVAL_GETTIME = 100;
+    const unsigned long CHECK_INTERVAL_CONNECTED = 10000;
+    const unsigned long CHECK_INTERVAL_RETRYING = 5000;
+    const unsigned long CHECK_INTERVAL_DISCONNECTED = 1000;
+    const unsigned long CHECK_INTERVAL_ERROR = 500;
 
-    unsigned long lastConnectionTickTime, lastNetworkStep;
     uint8_t* mac;
     int ss_pin;
+    unsigned long lastConnectionTickTime, lastNetworkStep;
     EthernetClient ethClient;
     EthernetUDP udp;
-    int connectionTickTimeInterval;
+    unsigned long connectionTickTimeInterval;
 };
 
 #endif /* #ifdef BOARD_HAS_ETHERNET */