Commands: Added connect command, fixed reconnect behaviour and scanning

Implemented command Connect (0x0d = 13), i.e. set WiFi credentials based
on the index of the network in the last scan result and the passphrase
supplied by the user.

For this is was necessary to refactor Connection and the Scan command,
since scanning can not be done while connection attempts are being made,
and we formerly always tried to reconnect when receiving a disconnect
event. It was somewhat pointless to keep trying to reconnect forever if
the connection kept failing due to wrong credentials as well.

Thus we now actively manage the automatic reconnect in Connection in the
member variable m_reconnecting, and only set it to true once a connection
is actually established. The disconnect() method now disconnects and
sets m_reconnecting to false, so it's an explicit request to disconnect
and *not* try to reconnect until a successive connect() call results in
an actual connection.

Implemented scanNetworks() in Connection to first disconnect(), call
WiFi.scanNetworks() and then connect() again to make sure scanning for
networks always works. There seem to be some quirks related to scanning
and multiple contradicting workarounds are suggested here:

espressif/arduino-esp32#3294

These workarounds involve manual delays, which seems a bit voodoo, so
I have decided that simply disconnecting before a scan and reconnecting
again once it is done is the most pragmatic approach here.

Scan: Now use connection->scanNetworks() instead of directly calling
WiFi.scanNetworks(). Also added responding with "no networks found"
like the previous firmware does, additionally logging whether the scan
failed in the first place or whether it succeeded and there are really
no networks available (see comment).

Data: Added method field(index) to access 0x01-separated fields. This is
the equivalent to the getValue()-function in th previous firmware.
Also renamed appendSeparated() to appendField().

Author: hbekel

wic64/CMakeLists.txt

@@ -13,6 +13,7 @@ idf_component_register(
     SRCS "commands/httpGet.cpp"
     SRCS "commands/getIP.cpp"
     SRCS "commands/scan.cpp"
+    SRCS "commands/connect.cpp"
     SRCS "commands/echo.cpp"
     SRCS "display.cpp"
     SRCS "webserver.cpp"

wic64/commands/commands.h

@@ -5,6 +5,7 @@
 #include "httpGet.h"
 #include "getIP.h"
 #include "scan.h"
+#include "connect.h"
 #include "echo.h"
 
 namespace WiC64 {
@@ -13,6 +14,7 @@ namespace WiC64 {
         WIC64_COMMAND(0x0f, HttpGet),
         WIC64_COMMAND(0x06, GetIP),
         WIC64_COMMAND(0x0c, Scan),
+        WIC64_COMMAND(0x0d, Connect),
         WIC64_COMMAND(0xff, Echo),
     };
 }

wic64/commands/connect.cpp

@@ -0,0 +1,22 @@
+#include "connection.h"
+#include "connect.h"
+
+#include "WiFi.h"
+
+namespace WiC64 {
+    const char* Connect::TAG = "CONNECT";
+
+    extern Connection *connection;
+
+    void Connect::execute(void) {
+        uint8_t indexInLastScanResult = atoi(request()->argument()->field(0));
+        const char* password = request()->argument()->field(1);
+
+        const String& ssid = WiFi.SSID(indexInLastScanResult).c_str();
+
+        connection->connect(ssid.c_str(), password);
+
+        response()->copy("Wlan config changed");
+        responseReady();
+    }
+}

wic64/commands/connect.h

@@ -0,0 +1,15 @@
+#ifndef WIC64_CONNECT_H
+#define WIC64_CONNECT_H
+
+#include "command.h"
+
+namespace WiC64 {
+    class Connect : public Command {
+        public:
+            static const char* TAG;
+
+            using Command::Command;
+            void execute(void);
+    };
+}
+#endif // WIC64_CONNECT_H

wic64/commands/scan.cpp

@@ -1,28 +1,49 @@
 #include "scan.h"
+#include "connection.h"
 #include "utilities.h"
 
 #include "WiFi.h"
 
 namespace WiC64 {
-
     const char* Scan::TAG = "SCAN";
 
+    extern Connection *connection;
+
     void Scan::execute(void) {
         int num_networks;
 
-        ESP_LOGD(TAG, "Scanning for WiFi networks...");
+        ESP_LOGI(TAG, "Scanning for WiFi networks...");
 
-        num_networks = WiFi.scanNetworks();
+        num_networks = connection->scanNetworks();
         num_networks = MIN(num_networks, 15);
 
-        for(uint8_t i=0; i<num_networks; i++) {
+        // The previous firmware only checked for num_networks == 0
+        // and returned "no networks found". We also test <= 0
+        // and issue the appropriate log message, but we still just
+        // return "no networks found", which remains true in either
+        // case, although it's not entirely accurate.
+
+        if (num_networks <= 0 ) {
+            if (num_networks < 0) {
+                ESP_LOGE(TAG, "Scan failed with error code %d", num_networks);
+            }
+            else {
+                ESP_LOGW(TAG, "Scan successful, still 0 networks found");
+            }
+            response()->copy("no networks found");
+        }
+        else {
+            ESP_LOGI(TAG, "Scan successful, %d networks found", num_networks);
+
+            for(uint8_t i=0; i<num_networks; i++) {
 
-            ESP_LOGD(TAG, "Network %d: [%s] %ddbm",
-                i, WiFi.SSID(i).c_str(), WiFi.RSSI(i));
+                ESP_LOGD(TAG, "Network %d: [%s] %ddbm",
+                    i, WiFi.SSID(i).c_str(), WiFi.RSSI(i));
 
-            response()->appendSeparated(String(i));
-            response()->appendSeparated(WiFi.SSID(i));
-            response()->appendSeparated(String(WiFi.RSSI(i)));
+                response()->appendField(String(i));
+                response()->appendField(WiFi.SSID(i));
+                response()->appendField(String(WiFi.RSSI(i)));
+            }
         }
         responseReady();
     }

wic64/connection.cpp

@@ -41,6 +41,8 @@ namespace WiC64 {
         display->RSSI(WiFi.RSSI());
         display->ip("0.0.0.0");
         display->status("Connected");
+
+        connection->reconnecting(true);
     }
 
     void Connection::onDisconnected(WiFiEvent_t event, WiFiEventInfo_t info) {
@@ -49,9 +51,13 @@ namespace WiC64 {
         display->SSID(getStoredSSID());
         display->RSSI(WiFi.RSSI());
         display->ip("0.0.0.0");
-        display->status("Connecting...");
 
-        connection->connect();
+        if (connection->reconnecting()) {
+            display->status("Reconnecting...");
+            connection->connect();
+        } else {
+            display->status("Disconnected");
+        }
     }
 
     void Connection::onGotIpAddress(WiFiEvent_t event, WiFiEventInfo_t info) {
@@ -62,15 +68,24 @@ namespace WiC64 {
         display->ip(connection->ipAddress());
     }
 
-    bool Connection::isConnected() {
-        return WiFi.isConnected();
-    }
-
     void Connection::connect() {
         ESP_LOGI(TAG, "Connecting to WiFi network...");
         WiFi.begin();
     }
 
+    void Connection::disconnect(void) {
+        reconnecting(false);
+        WiFi.disconnect();
+    }
+
+    void Connection::connect(const char* ssid, const char* password) {
+        ESP_LOGI(TAG, "Connecting with WiFi credentials SSID [%s] Passphrase [%s]",
+            ssid, password);
+
+        disconnect();
+        WiFi.begin(ssid, password);
+    }
+
     const char* Connection::ipAddress() {
         static char ip[16+1];
         strncpy(ip, WiFi.localIP().toString().c_str(), 16);
@@ -82,4 +97,14 @@ namespace WiC64 {
         strncpy(mac, WiFi.macAddress().c_str(), 18);
         return mac;
     }
+
+    uint16_t Connection::scanNetworks(void) {
+        uint16_t num_networks;
+
+        disconnect();
+        num_networks = WiFi.scanNetworks();
+        connect();
+
+        return num_networks;
+    }
 }

wic64/connection.h

@@ -8,19 +8,26 @@ namespace WiC64 {
         public: static const char* TAG;
 
         private:
+            bool m_reconnecting = false;
+
             static String getStoredSSID(void);
             static void onConnected(WiFiEvent_t event, WiFiEventInfo_t info);
             static void onDisconnected(WiFiEvent_t event, WiFiEventInfo_t info);
             static void onGotIpAddress(WiFiEvent_t event, WiFiEventInfo_t info);
 
         public:
-            Connection();
+            Connection(void);
+            bool reconnecting(void) { return m_reconnecting; }
+            void reconnecting(bool reconnecting) { m_reconnecting = reconnecting; }
 
-            bool isConnected(void);
-            void connect(void);
+            const char* ipAddress(void);
+            const char* macAddress(void);
 
-            const char* ipAddress();
-            const char* macAddress();
+            uint16_t scanNetworks(void);
+
+            void connect(void);
+            void connect(const char* ssid, const char* passphrase);
+            void disconnect(void);
     };
 
 }

wic64/data.cpp

@@ -32,24 +32,50 @@ namespace WiC64 {
         m_data[m_size] = '\0';
     }
 
-    void Data::copyFrom(const char *c_str) {
+    void Data::copy(const char *c_str) {
         m_data = transferBuffer;
         m_size = strlen(c_str);
         strncpy((char*) transferBuffer, c_str, m_size+1);
     }
 
-    void Data::appendSeparated(const String& string, const char separator) {
+    void Data::appendField(const String& string) {
         size_t len = string.length();
         memcpy(m_data + m_index, string.c_str(), len);
 
         m_index += len;
-        m_data[m_index] = separator;
+        m_data[m_index] = '\1';
         m_index++;
         m_size += len + 1;
     }
 
-    void Data::appendSeparated(const String &string) {
-        appendSeparated(string, '\1');
+    const char* Data::field(uint8_t index) {
+        static char value[256];
+
+        char* begin = (char*) m_data;
+        char* end = begin;
+        char* previous_end;
+
+        for (uint16_t i=0; i<=index; i++) {
+            previous_end = end;
+
+            if ((end = strstr(end, "\01")) != NULL) {
+                if (index == i) {
+                    begin = previous_end;
+
+                    uint8_t size =
+                        (end - (char*) m_data) -
+                        (begin - (char*) m_data);
+
+                    memcpy(value, begin, size);
+                    value[size] = '\0';
+
+                    return value;
+                }
+                end++;
+            }
+        }
+        value[0] = '\0';
+        return value;
     }
 
     void Data::queue(QueueHandle_t queue, uint16_t size) {

wic64/data.h

@@ -27,10 +27,10 @@ namespace WiC64 {
 
             void wrap(uint8_t* data, uint16_t size);
             void wrap(const char* c_str);
-            void copyFrom(const char* c_str);
+            void copy(const char* c_str);
 
-            void appendSeparated(const String& string, const char separator);
-            void appendSeparated(const String& string);
+            void appendField(const String& string);
+            const char* field(uint8_t index);
 
             uint16_t size() { return m_size; }
 

wic64/wic64.cpp

@@ -10,6 +10,7 @@
 #include "version.h"
 #include "commands/httpGet.h"
 #include "commands/scan.h"
+#include "commands/connect.h"
 
 #include "esp_log.h"
 
@@ -61,5 +62,6 @@ namespace WiC64 {
         esp_log_level_set(Command::TAG, loglevel);
         esp_log_level_set(HttpGet::TAG, loglevel);
         esp_log_level_set(Scan::TAG, loglevel);
+        esp_log_level_set(Connect::TAG, loglevel);
     }
 }