Cellular: Fix Gemalto/Cinterion socket open defer

Author: Ari Parkkila

UNITTESTS/features/cellular/framework/AT/at_cellularcontext/at_cellularcontexttest.cpp

@@ -473,7 +473,7 @@ TEST_F(TestAT_CellularContext, connect_disconnect_sync)
     network_cb_count = 0;
     // connect in sync mode, semaphore will return 0 so timeout is returned
     ASSERT_EQ(ctx.connect(), NSAPI_ERROR_TIMEOUT);
-    ASSERT_EQ(network_cb_count, 0);
+    ASSERT_EQ(network_cb_count, 1);
 
     my_AT_CTX ctx1(at, &dev);
     ctx1.attach(&network_cb);
@@ -486,7 +486,7 @@ TEST_F(TestAT_CellularContext, connect_disconnect_sync)
 
     ASSERT_EQ(ctx1.connect(),  NSAPI_ERROR_OK);
 
-    ASSERT_EQ(network_cb_count, 4);
+    ASSERT_EQ(network_cb_count, 5);
 
     ASSERT_EQ(ctx1.connect(), NSAPI_ERROR_IS_CONNECTED);
 

features/cellular/framework/targets/GEMALTO/CINTERION/GEMALTO_CINTERION_CellularStack.cpp

@@ -44,21 +44,6 @@ GEMALTO_CINTERION_CellularStack::~GEMALTO_CINTERION_CellularStack()
     _at.set_urc_handler("^SISR:", 0);
 }
 
-GEMALTO_CINTERION_CellularStack::CellularSocket *GEMALTO_CINTERION_CellularStack::find_socket(int sock_id)
-{
-    CellularSocket *sock = NULL;
-    for (int i = 0; i < SOCKET_MAX; i++) {
-        if (_socket[i] && _socket[i]->id == sock_id) {
-            sock = _socket[i];
-            break;
-        }
-    }
-    if (!sock) {
-        tr_error("Socket not found %d", sock_id);
-    }
-    return sock;
-}
-
 void GEMALTO_CINTERION_CellularStack::urc_sis()
 {
     int sock_id = _at.read_int();
@@ -175,56 +160,11 @@ nsapi_error_t GEMALTO_CINTERION_CellularStack::socket_close_impl(int sock_id)
 }
 
 nsapi_error_t GEMALTO_CINTERION_CellularStack::socket_open_defer(CellularSocket *socket, const SocketAddress *address)
-{
-    // host address (IPv4) and local+remote port is needed only for BGS2 which does not support UDP server socket
-    char sock_addr[sizeof("sockudp://") - 1 + NSAPI_IPv6_SIZE + sizeof("[]:12345;port=12345") - 1 + 1];
-
-    if (socket->proto == NSAPI_UDP) {
-        if (GEMALTO_CINTERION::get_module() != GEMALTO_CINTERION::ModuleBGS2) {
-            std::sprintf(sock_addr, "sockudp://%s:%u", address ? address->get_ip_address() : "", socket->localAddress.get_port());
-        } else {
-            std::sprintf(sock_addr, "sockudp://%s:%u;port=%u", address->get_ip_address(), address->get_port(), socket->localAddress.get_port());
-        }
-    } else {
-        if (address->get_ip_version() == NSAPI_IPv4) {
-            std::sprintf(sock_addr, "socktcp://%s:%u", address->get_ip_address(), address->get_port());
-        } else {
-            std::sprintf(sock_addr, "socktcp://[%s]:%u", address->get_ip_address(), address->get_port());
-        }
-    }
-
-    _at.cmd_start("AT^SISS=");
-    _at.write_int(socket->id);
-    _at.write_string("address", false);
-    _at.write_string(sock_addr);
-    _at.cmd_stop_read_resp();
-
-    _at.cmd_start("AT^SISO=");
-    _at.write_int(socket->id);
-    _at.cmd_stop_read_resp();
-
-    if (_at.get_last_error()) {
-        tr_error("Socket %d open failed!", socket->id);
-        _at.clear_error();
-        socket_close_impl(socket->id); // socket may already be open on modem if app and modem are not in sync, as a recovery, try to close the socket so open succeeds the next time
-        return NSAPI_ERROR_NO_SOCKET;
-    }
-
-    socket->created = true;
-    tr_debug("Cinterion open %d (err %d)", socket->id, _at.get_last_error());
-
-    return _at.get_last_error();
-}
-
-// To open socket:
-// 1. Select URC mode or polling mode with AT^SCFG
-// 2. create a GPRS connection profile with AT^SICS (must have PDP)
-// 3. create service profile with AT^SISS and map connectionID to serviceID
-// 4. open internet session with AT^SISO (ELS61 tries to attach to a packet domain)
-nsapi_error_t GEMALTO_CINTERION_CellularStack::create_socket_impl(CellularSocket *socket)
 {
     int connection_profile_id = CONNECTION_PROFILE_ID;
 
+    int retry_open = 1;
+retry_open:
     // setup internet session profile
     int internet_service_id = socket->id;
     bool foundSrvType = false;
@@ -285,6 +225,56 @@ nsapi_error_t GEMALTO_CINTERION_CellularStack::create_socket_impl(CellularSocket
         _at.cmd_stop_read_resp();
     }
 
+    // host address (IPv4) and local+remote port is needed only for BGS2 which does not support UDP server socket
+    char sock_addr[sizeof("sockudp://") - 1 + NSAPI_IPv6_SIZE + sizeof("[]:12345;port=12345") - 1 + 1];
+
+    if (socket->proto == NSAPI_UDP) {
+        if (GEMALTO_CINTERION::get_module() != GEMALTO_CINTERION::ModuleBGS2) {
+            std::sprintf(sock_addr, "sockudp://%s:%u", address ? address->get_ip_address() : "", socket->localAddress.get_port());
+        } else {
+            std::sprintf(sock_addr, "sockudp://%s:%u;port=%u", address->get_ip_address(), address->get_port(), socket->localAddress.get_port());
+        }
+    } else {
+        if (address->get_ip_version() == NSAPI_IPv4) {
+            std::sprintf(sock_addr, "socktcp://%s:%u", address->get_ip_address(), address->get_port());
+        } else {
+            std::sprintf(sock_addr, "socktcp://[%s]:%u", address->get_ip_address(), address->get_port());
+        }
+    }
+
+    _at.cmd_start("AT^SISS=");
+    _at.write_int(socket->id);
+    _at.write_string("address", false);
+    _at.write_string(sock_addr);
+    _at.cmd_stop_read_resp();
+
+    _at.cmd_start("AT^SISO=");
+    _at.write_int(socket->id);
+    _at.cmd_stop_read_resp();
+
+    if (_at.get_last_error()) {
+        tr_error("Socket %d open failed!", socket->id);
+        _at.clear_error();
+        socket_close_impl(socket->id); // socket may already be open on modem if app and modem are not in sync, as a recovery, try to close the socket so open succeeds the next time
+        if (retry_open--) {
+            goto retry_open;
+        }
+        return NSAPI_ERROR_NO_SOCKET;
+    }
+
+    socket->created = true;
+    tr_debug("Cinterion open %d (err %d)", socket->id, _at.get_last_error());
+
+    return _at.get_last_error();
+}
+
+// To open socket:
+// 1. Select URC mode or polling mode with AT^SCFG
+// 2. create a GPRS connection profile with AT^SICS (must have PDP)
+// 3. create service profile with AT^SISS and map connectionID to serviceID
+// 4. open internet session with AT^SISO (ELS61 tries to attach to a packet domain)
+nsapi_error_t GEMALTO_CINTERION_CellularStack::create_socket_impl(CellularSocket *socket)
+{
     if (socket->proto == NSAPI_UDP) {
         if (GEMALTO_CINTERION::get_module() != GEMALTO_CINTERION::ModuleBGS2) {
             return socket_open_defer(socket);
@@ -335,13 +325,13 @@ nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_sendto_impl(Cellul
         }
     }
     if (!socket->started || !socket->tx_ready) {
-        tr_debug("Socket %d would block (started %d, tx %d)", socket->id, socket->started, socket->tx_ready);
+        tr_debug("Socket %d send would block (started %d, tx %d)", socket->id, socket->started, socket->tx_ready);
         return NSAPI_ERROR_WOULD_BLOCK;
     }
 
     if (size > UDP_PACKET_SIZE) {
-        tr_warn("Sending UDP packet size %d (max %d)", size, UDP_PACKET_SIZE);
-        size = UDP_PACKET_SIZE;
+        tr_error("sendto size %d (max %d)", size, UDP_PACKET_SIZE);
+        return NSAPI_ERROR_PARAMETER;
     }
 
     _at.set_at_timeout(FAILURE_TIMEOUT);
@@ -413,13 +403,12 @@ nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_recvfrom_impl(Cell
     if (!socket->rx_avail) {
         _at.process_oob(); // check for ^SISR URC
         if (!socket->rx_avail) {
-            tr_debug("Socket %d would block", socket->id);
+            tr_debug("Socekt %d recv would block", socket->id);
             return NSAPI_ERROR_WOULD_BLOCK;
         }
     }
 
     if (size > UDP_PACKET_SIZE) {
-        tr_debug("Socket recvfrom size %d > %d", size, UDP_PACKET_SIZE);
         size = UDP_PACKET_SIZE;
     }
 
@@ -459,6 +448,7 @@ nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_recvfrom_impl(Cell
     }
     socket->rx_avail = false;
     if (len >= (nsapi_size_or_error_t)size) {
+        len = (nsapi_size_or_error_t)size;
         int remain_len = _at.read_int();
         if (remain_len > 0) {
             socket->rx_avail = true;
@@ -470,7 +460,7 @@ nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_recvfrom_impl(Cell
         char ip_address[NSAPI_IPv6_SIZE + sizeof("[]:12345") - 1 + 1];
         int ip_len = _at.read_string(ip_address, sizeof(ip_address));
         if (ip_len <= 0) {
-            tr_error("Socket %d recvfrom addr!", socket->id);
+            tr_error("Socket %d recvfrom addr (len %d)", socket->id, ip_len);
             return NSAPI_ERROR_DEVICE_ERROR;
         }
         if (address) {

features/cellular/framework/targets/GEMALTO/CINTERION/GEMALTO_CINTERION_CellularStack.h

@@ -48,9 +48,6 @@ class GEMALTO_CINTERION_CellularStack : public AT_CellularStack {
     virtual nsapi_error_t socket_connect(nsapi_socket_t handle, const SocketAddress &address);
 
 private:
-    // find the socket handle based on socket identifier
-    CellularSocket *find_socket(int sock_id);
-
     // socket URC handlers as per Cinterion AT manuals
     void urc_sis();
     void urc_sisw();