Do not rely on ATParser library for CONNECT and CLOSE commands

Author: janjongboom

Diff for: ESP8266/ESP8266.cpp

@@ -51,18 +51,6 @@ bool ESP8266::startup(int mode)
 
     _parser.oob("+IPD", this, &ESP8266::_packet_handler);
 
-    _parser.oob("0,CONNECT", this, &ESP8266::_incoming_socket_opened0);
-    _parser.oob("1,CONNECT", this, &ESP8266::_incoming_socket_opened1);
-    _parser.oob("2,CONNECT", this, &ESP8266::_incoming_socket_opened2);
-    _parser.oob("3,CONNECT", this, &ESP8266::_incoming_socket_opened3);
-    _parser.oob("4,CONNECT", this, &ESP8266::_incoming_socket_opened4);
-
-    _parser.oob("0,CLOSED", this, &ESP8266::_incoming_socket_closed0);
-    _parser.oob("1,CLOSED", this, &ESP8266::_incoming_socket_closed1);
-    _parser.oob("2,CLOSED", this, &ESP8266::_incoming_socket_closed2);
-    _parser.oob("3,CLOSED", this, &ESP8266::_incoming_socket_closed3);
-    _parser.oob("4,CLOSED", this, &ESP8266::_incoming_socket_closed4);
-
     return success;
 }
 
@@ -253,6 +241,10 @@ void ESP8266::_packet_handler()
 
 int32_t ESP8266::recv(int id, void *data, uint32_t amount)
 {
+    bool exit_when_not_found = false;
+
+    bool is_incoming_socket = _incoming_socket_status[id];
+
     while (true) {
         // check if any packets are ready for us
         for (struct packet **p = &_packets; *p; p = &(*p)->next) {
@@ -281,10 +273,37 @@ int32_t ESP8266::recv(int id, void *data, uint32_t amount)
             }
         }
 
-        // Wait for inbound packet
-        if (!_parser.recv("OK")) {
+        if (exit_when_not_found) {
             return -1;
         }
+
+        // Here's a problem... recv() blocks for forever, but it might be that the underlying socket closes in the meantime.
+        // We know when it happens (due to monitoring the RX IRQ channel)
+        // but there's no way of signaling this thread and actually abort the request...
+
+        if (is_incoming_socket) {
+            int timeout = _parser.getTimeout();
+            _parser.setTimeout(1000);
+
+            if (!_parser.recv("OK")) {
+                _parser.setTimeout(timeout);
+
+                // socket gone
+                if (!_incoming_socket_status[id]) return NSAPI_ERROR_NO_SOCKET;
+
+                // otherwise, just continue trying to get data...
+                continue;
+            }
+        }
+        else {
+            // Wait for inbound packet
+            if (!_parser.recv("OK")) {
+                // so this is weird... the message just received by the parser could actually be one of ours (in TCPServer mode)...
+                // so do one more pass...
+                exit_when_not_found = true;
+                continue;
+            }
+        }
     }
 }
 
@@ -316,7 +335,7 @@ bool ESP8266::writeable()
     return _serial.writeable();
 }
 
-void ESP8266::attach(Callback<void()> func)
+void ESP8266::attach(Callback<void(int)> func)
 {
     _serial.attach(func);
 }
@@ -335,20 +354,68 @@ bool ESP8266::recv_ap(nsapi_wifi_ap_t *ap)
 
 bool ESP8266::bind(const SocketAddress& address)
 {
+    // we need an event queue to dispatch from serial RX IRQ -> non-IRQ thread
+    event_queue = new EventQueue();
+    event_thread = new Thread(osPriorityNormal, 2048);
+    if (!event_queue || !event_thread) {
+        return NSAPI_ERROR_NO_MEMORY;
+    }
+    event_thread->start(callback(event_queue, &EventQueue::dispatch_forever));
+
+    // buffer to store RX data in
+    rx_buffer = (char*)malloc(1024);
+    rx_ix = 0;
+    if (!rx_buffer) {
+        return NSAPI_ERROR_NO_MEMORY;
+    }
+
+    // clear incoming socket status
+    memset(_incoming_socket_status, 0, sizeof(_incoming_socket_status));
+
+    // attach to the serial
+    _serial.attach(callback(this, &ESP8266::attach_rx));
+
+    // and start the actual server
     return _parser.send("AT+CIPSERVER=1,%d", address.get_port())
         && _parser.recv("OK");
 }
 
-void ESP8266::_incoming_socket_opened(int8_t socket_id)
-{
-    printf("Incoming socket opened %d\n", socket_id);
+void ESP8266::process_command(char* cmd, size_t size) {
+    if (size == 9 /* 0,CONNECT */
+        && (cmd[0] >= '0' && cmd[0] <= '9')
+        && (cmd[1] == ',')
+        && (strcmp(&cmd[2], "CONNECT") == 0)) {
+
+        _incoming_socket_status[cmd[0] - '0'] = true;
+
+        _signalingCallback(ESP8266_SOCKET_CONNECT, cmd[0] - '0');
+    }
+    else if (size == 8 /* 0,CLOSED */
+        && (cmd[0] >= '0' && cmd[0] <= '9')
+        && (cmd[1] == ',')
+        && (strcmp(&cmd[2], "CLOSED") == 0)) {
 
-    _signalingCallback(ESP8266_SOCKET_CONNECT, socket_id);
+        _incoming_socket_status[cmd[0] - '0'] = false;
+
+        _signalingCallback(ESP8266_SOCKET_CLOSE, cmd[0] - '0');
+    }
+    free(cmd);
 }
 
-void ESP8266::_incoming_socket_closed(int8_t socket_id)
-{
-    printf("Incoming socket closed %d\n", socket_id);
+void ESP8266::attach_rx(int c) {
+    // store value in buffer
+    rx_buffer[rx_ix] = c;
+    rx_buffer[rx_ix + 1] = 0;
+
+    if (rx_ix > 0 && c == '\n') {
+        // got a whole command
+        char* cmd = (char*)calloc(rx_ix, 1);
+        memcpy(cmd, rx_buffer, rx_ix - 1);
+        event_queue->call(callback(this, &ESP8266::process_command), cmd, rx_ix - 1);
+
+        rx_ix = 0;
+        return;
+    }
 
-    _signalingCallback(ESP8266_SOCKET_CLOSE, socket_id);
+    rx_ix++;
 }

Diff for: ESP8266/ESP8266.h

@@ -218,9 +218,10 @@ class ESP8266
     */
     bool bind(const SocketAddress& address);
 
-    void ping();
-
 private:
+    void attach_rx(int);
+    void process_command(char*, size_t);
+
     BufferedSerial _serial;
     ATParser _parser;
     Callback<void(SignalingAction, int)> _signalingCallback;
@@ -232,28 +233,20 @@ class ESP8266
         // data follows
     } *_packets, **_packets_end;
     void _packet_handler();
-    void _incoming_socket_opened(int8_t);
-    void _incoming_socket_closed(int8_t);
     bool recv_ap(nsapi_wifi_ap_t *ap);
 
     char _ip_buffer[16];
     char _gateway_buffer[16];
     char _netmask_buffer[16];
     char _mac_buffer[18];
 
-    // The CONNECTED OOB messages start with %d, and patching the ATParser is not worth it
-    // Max. connections is 5 according to ESP8266_SOCKET_COUNT, if that changes, this has to change too
-    void _incoming_socket_opened0() { _incoming_socket_opened(0); }
-    void _incoming_socket_opened1() { _incoming_socket_opened(1); }
-    void _incoming_socket_opened2() { _incoming_socket_opened(2); }
-    void _incoming_socket_opened3() { _incoming_socket_opened(3); }
-    void _incoming_socket_opened4() { _incoming_socket_opened(4); }
-
-    void _incoming_socket_closed0() { _incoming_socket_closed(0); }
-    void _incoming_socket_closed1() { _incoming_socket_closed(1); }
-    void _incoming_socket_closed2() { _incoming_socket_closed(2); }
-    void _incoming_socket_closed3() { _incoming_socket_closed(3); }
-    void _incoming_socket_closed4() { _incoming_socket_closed(4); }
+    // TCPServer mode needs a separate thread to dispatch RX IRQ commands from
+    EventQueue* event_queue;
+    Thread* event_thread;
+    char* rx_buffer;
+    size_t rx_ix;
+    bool _incoming_socket_status[5];
+
 };
 
 #endif

Diff for: ESP8266Interface.cpp

@@ -37,7 +37,7 @@
 
 // ESP8266Interface implementation
 ESP8266Interface::ESP8266Interface(PinName tx, PinName rx, bool debug)
-    : _esp(tx, rx, callback(this, &ESP8266Interface::signal), false)
+    : _esp(tx, rx, callback(this, &ESP8266Interface::signal), debug)
 {
     memset(_ids, 0, sizeof(_ids));
     memset(_cbs, 0, sizeof(_cbs));
@@ -208,7 +208,11 @@ int ESP8266Interface::socket_close(void *handle)
 
 int ESP8266Interface::socket_bind(void *handle, const SocketAddress &address)
 {
-    return _esp.bind(address);
+    int bind_res = _esp.bind(address);
+    if (bind_res < -3000) return bind_res;
+    if (bind_res != 1) return NSAPI_ERROR_DEVICE_ERROR;
+
+    return NSAPI_ERROR_OK;
 }
 
 int ESP8266Interface::socket_listen(void *handle, int backlog)
@@ -232,13 +236,7 @@ int ESP8266Interface::socket_connect(void *handle, const SocketAddress &addr)
 
 int ESP8266Interface::socket_accept(void *server, void **socket, SocketAddress *addr)
 {
-    printf("socket_accept\n");
-
     while (1) {
-        // to get the OOB messages we need to send something to the ESP8266 so the ATParser has time to
-        // grab the packets. Send a bogus message to the module...
-        _esp.get_firmware_version();
-
         // now go through the _incoming_sockets array and see if there are unattached sockets...
         for (size_t ix = 0; ix < ESP8266_SOCKET_COUNT; ix++) {
             // printf("_incoming_sockets[ix] socket=%p connected=%d accepted=%d\n",
@@ -254,7 +252,7 @@ int ESP8266Interface::socket_accept(void *server, void **socket, SocketAddress *
             }
         }
 
-        wait_ms(20); // socket_accept is blocking
+        wait_ms(20);
     }
     return 0;
 }
@@ -277,7 +275,10 @@ int ESP8266Interface::socket_recv(void *handle, void *data, unsigned size)
     _esp.setTimeout(ESP8266_RECV_TIMEOUT);
 
     int32_t recv = _esp.recv(socket->id, data, size);
-    if (recv < 0) {
+    if (recv < -3000) {
+        return recv;
+    }
+    else if (recv < 0) {
         return NSAPI_ERROR_WOULD_BLOCK;
     }
 
@@ -334,12 +335,11 @@ void ESP8266Interface::event() {
 }
 
 void ESP8266Interface::signal(SignalingAction action, int socket_id) {
-    printf("ESP8266Interface::signal %d %d\n", action, socket_id);
-
     if (action == ESP8266_SOCKET_CONNECT) {
+        printf("ESP8266::SOCKET CONNECT %d\n", socket_id);
         if (_ids[socket_id]) {
             // this should not be possible...
-            printf("ESP8266_SOCKET_CONNECT for socket that already exists...\n");
+            // printf("ESP8266_SOCKET_CONNECT for socket that already exists...\n");
             // return;
         }
 
@@ -359,6 +359,8 @@ void ESP8266Interface::signal(SignalingAction action, int socket_id) {
         _incoming_sockets[socket_id].accepted = false;
     }
     else if (action == ESP8266_SOCKET_CLOSE) {
+        printf("ESP8266::SOCKET CLOSE %d\n", socket_id);
+
         // Q: should we be able to delete the socket here? probably segfaults if held in user code
         struct esp8266_socket *socket = _incoming_sockets[socket_id].socket;
         if (!socket || !socket->connected) {
@@ -367,10 +369,9 @@ void ESP8266Interface::signal(SignalingAction action, int socket_id) {
         }
 
         socket->connected = false;
+        _ids[socket_id] = false;
         _incoming_sockets[socket_id].accepted = false;
         _incoming_sockets[socket_id].socket = NULL;
-        _ids[socket_id] = false;
-
-        // delete socket?
+        delete socket;
     }
 }

Diff for: ESP8266Interface.h

@@ -283,6 +283,7 @@ class ESP8266Interface : public NetworkStack, public WiFiInterface
         struct esp8266_socket* socket;
         bool accepted;
      } _incoming_sockets[ESP8266_SOCKET_COUNT];
+
 };
 
 #endif