ESP8266: Simplify flow control and enable per-socket reconnection

ccli8 · michalpasztamobica · commit 4405ab46b6a1 · 2020-01-09T12:48:22.000+02:00
1.  Fix 'spurious close' by adding close() in open(). 'spurious close' gets frequent and cannot ignore when send() changes to asynchronous. User can retry open() until 'spurious close' gets true.
2.  Allow only one actively sending socket because:
    (1) ESP8266 AT packets 'SEND OK'/'SEND FAIL' are not associated with socket ID. No way to tell them.
    (2) In original implementation, ESP8266::send() is synchronous, which implies only one actively sending socket.
3.  Register 'SEND OK'/'SEND FAIL' oobs, like others in ESP8266::ESP8266 constructor. Don't get involved in oob management with send status because ESP8266 modem possibly doesn't reply these packets on error case.
4.  Now that ESP8266::send() changes to asynchronous, drop the code with _parser.recv("SEND OK")/_parser.recv("SEND FAIL"). _parser.recv("SEND OK")/_parser.recv("SEND FAIL") and 'SEND OK'/'SEND FAIL' oobs both consume 'SEND OK'/'SEND FAIL' packets and complicate flow control.
diff --git a/components/wifi/esp8266-driver/ESP8266/ESP8266.cpp b/components/wifi/esp8266-driver/ESP8266/ESP8266.cpp
@@ -58,7 +58,7 @@ ESP8266::ESP8266(PinName tx, PinName rx, bool debug, PinName rts, PinName cts)
       _error(false),
       _busy(false),
       _reset_done(false),
-      _send_status(SEND_STATUS_OK),
+      _sock_sending_id(-1),
       _conn_status(NSAPI_STATUS_DISCONNECTED)
 {
     _serial.set_baud(MBED_CONF_ESP8266_SERIAL_BAUDRATE);
@@ -90,13 +90,18 @@ ESP8266::ESP8266(PinName tx, PinName rx, bool debug, PinName rts, PinName cts)
     _parser.oob("busy ", callback(this, &ESP8266::_oob_busy));
     // NOTE: documentation v3.0 says '+CIPRECVDATA:<data_len>,' but it's not how the FW responds...
     _parser.oob("+CIPRECVDATA,", callback(this, &ESP8266::_oob_tcp_data_hdlr));
+    // Register 'SEND OK'/'SEND FAIL' oobs here. Don't get involved in oob management with send status
+    // because ESP8266 modem possibly doesn't reply these packets on error case.
+    _parser.oob("SEND OK", callback(this, &ESP8266::_oob_send_ok_received));
+    _parser.oob("SEND FAIL", callback(this, &ESP8266::_oob_send_fail_received));
 
     for (int i = 0; i < SOCKET_COUNT; i++) {
         _sock_i[i].open = false;
         _sock_i[i].proto = NSAPI_UDP;
         _sock_i[i].tcp_data = NULL;
         _sock_i[i].tcp_data_avbl = 0;
         _sock_i[i].tcp_data_rcvd = 0;
+        _sock_i[i].send_fail = false;
     }
 
     _scan_r.res = NULL;
@@ -289,9 +294,7 @@ bool ESP8266::reset(void)
     tr_debug("reset(): Done: %s.", done ? "OK" : "FAIL");
 
     _clear_socket_packets(ESP8266_ALL_SOCKET_IDS);
-    _send_status = SEND_STATUS_OK;
-    _parser.remove_oob("SEND OK");
-    _parser.remove_oob("SEND FAIL");
+    _sock_sending_id = -1;
     set_timeout();
     _smutex.unlock();
 
@@ -515,9 +518,17 @@ nsapi_error_t ESP8266::open_udp(int id, const char *addr, int port, int local_po
     // process OOB so that _sock_i reflects the correct state of the socket
     _process_oob(ESP8266_SEND_TIMEOUT, true);
 
-    if (id >= SOCKET_COUNT || _sock_i[id].open) {
+    // Previous close() can fail with busy in sending. Usually, user will ignore the close()
+    // error code and cause 'spurious close', in which case user has closed the socket but ESP8266 modem
+    // hasn't yet. Because we don't know how long ESP8266 modem will trap in busy, enlarge retry count
+    // or timeout in close() isn't a nice way. Here, we actively re-call close() in open() to let the modem
+    // close the socket. User can re-try open() on failure. Without this active close(), open() can fail forever
+    // with previous 'spurious close', unless peer closes the socket and so ESP8266 modem closes it accordingly.
+    if (id >= SOCKET_COUNT) {
         _smutex.unlock();
         return NSAPI_ERROR_PARAMETER;
+    } else if (_sock_i[id].open) {
+        close(id);
     }
 
     for (int i = 0; i < 2; i++) {
@@ -566,9 +577,12 @@ nsapi_error_t ESP8266::open_tcp(int id, const char *addr, int port, int keepaliv
     // process OOB so that _sock_i reflects the correct state of the socket
     _process_oob(ESP8266_SEND_TIMEOUT, true);
 
-    if (id >= SOCKET_COUNT || _sock_i[id].open) {
+    // See the reason above with close()
+    if (id >= SOCKET_COUNT) {
         _smutex.unlock();
         return NSAPI_ERROR_PARAMETER;
+    } else if (_sock_i[id].open) {
+        close(id);
     }
 
     for (int i = 0; i < 2; i++) {
@@ -619,13 +633,14 @@ bool ESP8266::dns_lookup(const char *name, char *ip)
 nsapi_size_or_error_t ESP8266::send(int id, const void *data, uint32_t amount)
 {
     if (_sock_i[id].proto == NSAPI_TCP) {
-        if (_send_status == SEND_STATUS_PENDING) {
-            tr_debug("send(): Previous packet was not yet ACK-ed with SEND OK.");
-            return NSAPI_ERROR_WOULD_BLOCK;
-        } else if (_send_status == SEND_STATUS_FAILED) {
-            tr_debug("send(): Previous packet failed.");
-            _send_status = SEND_STATUS_OK;
-            return NSAPI_ERROR_DEVICE_ERROR;
+        if (_sock_sending_id >= 0 && _sock_sending_id < SOCKET_COUNT) {
+            if (!_sock_i[id].send_fail) {
+                tr_debug("send(): Previous packet (socket %d) was not yet ACK-ed with SEND OK.", _sock_sending_id);
+                return NSAPI_ERROR_WOULD_BLOCK;
+            } else {
+                tr_debug("send(): Previous packet (socket %d) failed.", id);
+                return NSAPI_ERROR_DEVICE_ERROR;
+            }
         }
     }
 
@@ -644,6 +659,10 @@ nsapi_size_or_error_t ESP8266::send(int id, const void *data, uint32_t amount)
     }
 
     _smutex.lock();
+    // Mark this socket is sending. We allow only one actively sending socket because:
+    // 1. ESP8266 AT packets 'SEND OK'/'SEND FAIL' are not associated with socket ID. No way to tell them.
+    // 2. In original implementation, ESP8266::send() is synchronous, which implies only one actively sending socket.
+    _sock_sending_id = id;
     set_timeout(ESP8266_SEND_TIMEOUT);
     _busy = false;
     _error = false;
@@ -685,44 +704,27 @@ nsapi_size_or_error_t ESP8266::send(int id, const void *data, uint32_t amount)
         goto END;
     }
 
-    //We might receive "busy s/p...", "SEND OK" or "SEND FAIL" from modem, so we need to check that also
-    _parser.oob("SEND FAIL", callback(this, &ESP8266::_oob_send_fail_received));
-    for (unsigned int i = send_ack_retries; i > 0; i--) {
-        if (!_parser.recv("SEND OK")) {
-            if (_error || _send_status == SEND_STATUS_FAILED) {
-                _parser.remove_oob("SEND FAIL");
-                goto END;
-            }
-            if (_busy) {
-                _busy = false;
-                tr_debug("send(): Busy, %d retries left...", i - 1);
-            }
-        } else {
-            ret = amount; // Got "SEND OK" - return number of bytes.
-            goto END;
-        }
-    }
-
-    // ESP8266 ACKed data over serial, but did not ACK with SEND OK or report any error.
-    _send_status = SEND_STATUS_PENDING;
-    _parser.oob("SEND OK", callback(this, &ESP8266::_oob_send_ok_received));
     ret = amount;
 
 END:
     _process_oob(ESP8266_RECV_TIMEOUT, true); // Drain USART receive register to avoid data overrun
 
     // error hierarchy, from low to high
-    if (_busy) {
+    // NOTE: We cannot return NSAPI_ERROR_WOULD_BLOCK when "Recv X bytes" has reached, otherwise duplicate data send.
+    if (_busy && ret < 0) {
         ret = NSAPI_ERROR_WOULD_BLOCK;
         tr_debug("send(): Modem busy.");
     }
 
     if (_error) {
+        // FIXME: Not sure clear or not of _error. See it as device error and it can recover only via reset?
+        _sock_sending_id = -1;
         ret = NSAPI_ERROR_CONNECTION_LOST;
         tr_debug("send(): Connection disrupted.");
     }
 
-    if (_send_status == SEND_STATUS_FAILED) {
+    if (_sock_i[id].send_fail) {
+        _sock_sending_id = -1;
         if (_sock_i[id].proto == NSAPI_TCP) {
             ret = NSAPI_ERROR_DEVICE_ERROR;
         } else {
@@ -732,6 +734,7 @@ nsapi_size_or_error_t ESP8266::send(int id, const void *data, uint32_t amount)
     }
 
     if (!_sock_i[id].open && ret < 0) {
+        _sock_sending_id = -1;
         ret = NSAPI_ERROR_CONNECTION_LOST;
         tr_debug("send(): Socket %d closed abruptly.", id);
     }
@@ -1005,14 +1008,6 @@ void ESP8266::_clear_socket_packets(int id)
         _sock_i[id].tcp_data_avbl = 0;
     }
 }
-void ESP8266::_clear_send_status(void)
-{
-    _smutex.lock(); // remove_oob doesn't use serial, but we don't want to race against it.
-    _send_status = SEND_STATUS_OK;
-    _parser.remove_oob("SEND OK");
-    _parser.remove_oob("SEND FAIL");
-    _smutex.unlock();
-}
 
 bool ESP8266::close(int id)
 {
@@ -1025,20 +1020,33 @@ bool ESP8266::close(int id)
                     _closed = false;
                     _sock_i[id].open = false;
                     _clear_socket_packets(id);
+                    // Closed, so this socket escapes from SEND FAIL status.
+                    if (id == _sock_sending_id) {
+                        _sock_sending_id = -1;
+                    }
+                    _sock_i[id].send_fail = false;
                     _smutex.unlock();
                     // ESP8266 has a habit that it might close a socket on its own.
+                    tr_debug("close(%d): socket close OK with UNLINK ERROR", id);
                     return true;
                 }
             } else {
                 // _sock_i[id].open set to false with an OOB
                 _clear_socket_packets(id);
+                // Closed, so this socket escapes from SEND FAIL status
+                if (id == _sock_sending_id) {
+                    _sock_sending_id = -1;
+                }
+                _sock_i[id].send_fail = false;
                 _smutex.unlock();
+                tr_debug("close(%d): socket close OK with AT+CIPCLOSE OK", id);
                 return true;
             }
         }
         _smutex.unlock();
     }
 
+    tr_debug("close(%d): socket close FAIL'ed (spurious close)", id);
     return false;
 }
 
@@ -1215,39 +1223,59 @@ void ESP8266::_oob_socket0_closed()
 {
     static const int id = 0;
     _sock_i[id].open = false;
-    _clear_send_status();
+    // Closed, so this socket escapes from SEND FAIL status
+    if (id == _sock_sending_id) {
+        _sock_sending_id = -1;
+    }
+    _sock_i[id].send_fail = false;
     tr_debug("_oob_socket0_closed(): Socket %d closed.", id);
 }
 
 void ESP8266::_oob_socket1_closed()
 {
     static const int id = 1;
     _sock_i[id].open = false;
-    _clear_send_status();
+    // Closed, so this socket escapes from SEND FAIL status
+    if (id == _sock_sending_id) {
+        _sock_sending_id = -1;
+    }
+    _sock_i[id].send_fail = false;
     tr_debug("_oob_socket1_closed(): Socket %d closed.", id);
 }
 
 void ESP8266::_oob_socket2_closed()
 {
     static const int id = 2;
     _sock_i[id].open = false;
-    _clear_send_status();
+    // Closed, so this socket escapes from SEND FAIL status
+    if (id == _sock_sending_id) {
+        _sock_sending_id = -1;
+    }
+    _sock_i[id].send_fail = false;
     tr_debug("_oob_socket2_closed(): Socket %d closed.", id);
 }
 
 void ESP8266::_oob_socket3_closed()
 {
     static const int id = 3;
     _sock_i[id].open = false;
-    _clear_send_status();
+    // Closed, so this socket escapes from SEND FAIL status
+    if (id == _sock_sending_id) {
+        _sock_sending_id = -1;
+    }
+    _sock_i[id].send_fail = false;
     tr_debug("_oob_socket3_closed(): %d closed.", id);
 }
 
 void ESP8266::_oob_socket4_closed()
 {
     static const int id = 4;
     _sock_i[id].open = false;
-    _clear_send_status();
+    // Closed, so this socket escapes from SEND FAIL status
+    if (id == _sock_sending_id) {
+        _sock_sending_id = -1;
+    }
+    _sock_i[id].send_fail = false;
     tr_debug("_oob_socket0_closed(): Socket %d closed.", id);
 }
 
@@ -1287,22 +1315,17 @@ void ESP8266::_oob_connection_status()
 
 void ESP8266::_oob_send_ok_received()
 {
-    tr_debug("_oob_send_ok_received called with _send_status %d", _send_status);
-    if (_send_status == SEND_STATUS_PENDING) {
-        _send_status = SEND_STATUS_OK;
-    }
-    _parser.remove_oob("SEND OK");
-    _parser.remove_oob("SEND FAIL");
+    tr_debug("_oob_send_ok_received called for socket %d", _sock_sending_id);
+    _sock_sending_id = -1;
 }
 
 void ESP8266::_oob_send_fail_received()
 {
-    tr_debug("_oob_send_fail_received called with _send_status %d", _send_status);
-    if (_send_status == SEND_STATUS_PENDING) {
-        _send_status = SEND_STATUS_FAILED;
+    tr_debug("_oob_send_fail_received called for socket %d", _sock_sending_id);
+    if (_sock_sending_id >= 0 && _sock_sending_id < SOCKET_COUNT) {
+        _sock_i[_sock_sending_id].send_fail = true;
     }
-    _parser.remove_oob("SEND FAIL");
-    _parser.remove_oob("SEND OK");
+    _sock_sending_id = -1;
 }
 
 int8_t ESP8266::default_wifi_mode()
diff --git a/components/wifi/esp8266-driver/ESP8266/ESP8266.h b/components/wifi/esp8266-driver/ESP8266/ESP8266.h
@@ -250,17 +250,6 @@ class ESP8266 {
     */
     nsapi_error_t open_tcp(int id, const char *addr, int port, int keepalive = 0);
 
-    /** ESP8266 send status.
-     *
-     * There is one send status per device, because the ACKs are just simple "SEND OK/FAIL"
-     * with no way to tell which socket is reporting back.
-     */
-    enum send_status {
-        SEND_STATUS_OK      = 0, /*!< device is able to send */
-        SEND_STATUS_PENDING = 1, /*!< previous send is pending for OK/FAIL */
-        SEND_STATUS_FAILED  = 2  /*!< previous send FAILed */
-    };
-
     /**
     * Sends data to an open socket
     *
@@ -455,7 +444,6 @@ class ESP8266 {
         // data follows
     } *_packets, * *_packets_end;
     void _clear_socket_packets(int id);
-    void _clear_send_status(void);
     int _sock_active_id;
 
     // Memory statistics
@@ -493,7 +481,7 @@ class ESP8266 {
     bool _error;
     bool _busy;
     bool _reset_done;
-    send_status _send_status;
+    int _sock_sending_id;
 
     // Modem's address info
     char _ip_buffer[16];
@@ -508,6 +496,7 @@ class ESP8266 {
         char *tcp_data;
         int32_t tcp_data_avbl; // Data waiting on modem
         int32_t tcp_data_rcvd;
+        bool send_fail;     // Received 'SEND FAIL'. Expect user will close the socket.
     };
     struct _sock_info _sock_i[SOCKET_COUNT];
 
