Merge pull request #12281 from AriParkkila/cell-stack-deinit

Cellular: Refactor socket_stack_init() from generic to modem specific

Author: adbridge

UNITTESTS/features/cellular/framework/AT/at_cellularstack/at_cellularstacktest.cpp

@@ -197,21 +197,18 @@ TEST_F(TestAT_CellularStack, test_AT_CellularStack_socket_open)
 
     MyStack st(at, 0, IPV6_STACK, *_dev);
     AT_CellularDevice_stub::supported_bool = 0;
-    AT_CellularDevice_stub::max_sock_value = 1;
     EXPECT_EQ(st.socket_open(NULL, NSAPI_TCP), NSAPI_ERROR_UNSUPPORTED);
 
     AT_CellularDevice_stub::supported_bool = 1;
     nsapi_socket_t sock = &st.socket;
     AT_CellularDevice_stub::max_sock_value = 0;
     EXPECT_EQ(st.socket_open(&sock, NSAPI_TCP), NSAPI_ERROR_NO_SOCKET);
+    AT_CellularDevice_stub::max_sock_value = 1;
 
     MyStack st2(at, 0, IPV6_STACK, *_dev);
     st2.bool_value = true;
-    AT_CellularDevice_stub::max_sock_value = 1;
     sock = &st2.socket;
     EXPECT_EQ(st2.socket_open(&sock, NSAPI_TCP), NSAPI_ERROR_OK);
-
-    AT_CellularDevice_stub::max_sock_value = 1; // value must be the same as before the first open
 }
 
 TEST_F(TestAT_CellularStack, test_AT_CellularStack_socket_close)
@@ -225,13 +222,12 @@ TEST_F(TestAT_CellularStack, test_AT_CellularStack_socket_close)
 
     nsapi_socket_t sock = &st.socket;
     st.bool_value = true;
-    AT_CellularDevice_stub::max_sock_value = 1;
     EXPECT_EQ(st.socket_open(&sock, NSAPI_TCP), NSAPI_ERROR_OK);
     AT_CellularDevice_stub::max_sock_value = 0;
     EXPECT_EQ(st.socket_close(sock), NSAPI_ERROR_DEVICE_ERROR);
+    AT_CellularDevice_stub::max_sock_value = 1;
 
     MyStack st2(at, 0, IPV6_STACK, *_dev);
-    AT_CellularDevice_stub::max_sock_value = 1;
     st2.bool_value = true;
     sock = &st2.socket;
     EXPECT_EQ(st2.socket_open(&sock, NSAPI_TCP), NSAPI_ERROR_OK);
@@ -298,7 +294,6 @@ TEST_F(TestAT_CellularStack, test_AT_CellularStack_socket_send)
     EXPECT_EQ(st.socket_send(&st.socket, "addr", 4), NSAPI_ERROR_NO_CONNECTION);
 
     SocketAddress addr("fc00::", 123);
-    AT_CellularDevice_stub::max_sock_value = 1;
     st.bool_value = true;
     nsapi_socket_t sock = &st.socket;
     st.socket_open(&sock, NSAPI_TCP);
@@ -317,7 +312,6 @@ TEST_F(TestAT_CellularStack, test_AT_CellularStack_socket_sendto)
     SocketAddress addr("fc00::", 123);
     EXPECT_EQ(st.socket_sendto(NULL, addr, "addr", 4), NSAPI_ERROR_NO_SOCKET);
 
-    AT_CellularDevice_stub::max_sock_value = 1;
     st.bool_value = true;
     nsapi_socket_t sock = &st.socket;
     st.socket_open(&sock, NSAPI_TCP);
@@ -351,7 +345,6 @@ TEST_F(TestAT_CellularStack, test_AT_CellularStack_socket_recvfrom)
     EXPECT_EQ(st.socket_recvfrom(NULL, NULL, table, 4), NSAPI_ERROR_NO_SOCKET);
 
     SocketAddress addr;
-    AT_CellularDevice_stub::max_sock_value = 1;
     st.bool_value = true;
     nsapi_socket_t sock = &st.socket;
     st.socket_open(&sock, NSAPI_TCP);
@@ -372,7 +365,6 @@ TEST_F(TestAT_CellularStack, test_AT_CellularStack_socket_attach)
     MyStack st(at, 0, IPV6_STACK, *_dev);
 
     st.socket_attach(NULL, NULL, NULL);
-    AT_CellularDevice_stub::max_sock_value = 1;
     st.bool_value = true;
     nsapi_socket_t sock = &st.socket;
     st.socket_open(&sock, NSAPI_TCP);

UNITTESTS/stubs/AT_CellularDevice_stub.cpp

@@ -31,7 +31,7 @@ int AT_CellularDevice_stub::set_pin_failure_count = 0;
 int AT_CellularDevice_stub::get_sim_failure_count = 0;
 bool AT_CellularDevice_stub::pin_needed = false;
 bool AT_CellularDevice_stub::supported_bool = false;
-int AT_CellularDevice_stub::max_sock_value = 0;
+int AT_CellularDevice_stub::max_sock_value = 1;
 
 AT_CellularDevice::AT_CellularDevice(FileHandle *fh) : CellularDevice(fh),
 #if MBED_CONF_CELLULAR_USE_SMS

UNITTESTS/stubs/AT_CellularStack_stub.cpp

@@ -31,11 +31,6 @@ AT_CellularStack::~AT_CellularStack()
 {
 }
 
-nsapi_error_t AT_CellularStack::socket_stack_init()
-{
-    return NSAPI_ERROR_OK;
-}
-
 nsapi_error_t AT_CellularStack::get_ip_address(SocketAddress* address)
 {
     return NSAPI_ERROR_UNSUPPORTED;

features/cellular/framework/AT/AT_CellularStack.cpp

@@ -29,20 +29,19 @@ AT_CellularStack::AT_CellularStack(ATHandler &at, int cid, nsapi_ip_stack_t stac
     _stack_type(stack_type), _ip_ver_sendto(NSAPI_UNSPEC), _at(at), _device(device)
 {
     memset(_ip, 0, PDP_IPV6_SIZE);
+
+    MBED_ASSERT(_device.get_property(AT_CellularDevice::PROPERTY_SOCKET_COUNT) > 0);
+    _socket = new CellularSocket *[_device.get_property(AT_CellularDevice::PROPERTY_SOCKET_COUNT)]();
 }
 
 AT_CellularStack::~AT_CellularStack()
 {
-    if (_socket) {
-        for (int i = 0; i < _device.get_property(AT_CellularDevice::PROPERTY_SOCKET_COUNT); i++) {
-            if (_socket[i]) {
-                delete _socket[i];
-                _socket[i] = NULL;
-            }
+    for (int i = 0; i < _device.get_property(AT_CellularDevice::PROPERTY_SOCKET_COUNT); i++) {
+        if (_socket[i]) {
+            delete _socket[i];
         }
-        delete [] _socket;
-        _socket = NULL;
     }
+    delete [] _socket;
 }
 
 int AT_CellularStack::find_socket_index(nsapi_socket_t handle)
@@ -153,11 +152,6 @@ void AT_CellularStack::set_cid(int cid)
     _cid = cid;
 }
 
-nsapi_error_t AT_CellularStack::socket_stack_init()
-{
-    return NSAPI_ERROR_OK;
-}
-
 nsapi_error_t AT_CellularStack::socket_open(nsapi_socket_t *handle, nsapi_protocol_t proto)
 {
     if (!handle) {
@@ -178,22 +172,6 @@ nsapi_error_t AT_CellularStack::socket_open(nsapi_socket_t *handle, nsapi_protoc
 
     _socket_mutex.lock();
 
-    if (!_socket) {
-        if (_device.get_property(AT_CellularDevice::PROPERTY_SOCKET_COUNT) == 0) {
-            _socket_mutex.unlock();
-            return NSAPI_ERROR_NO_SOCKET;
-        }
-        if (socket_stack_init() != NSAPI_ERROR_OK) {
-            _socket_mutex.unlock();
-            return NSAPI_ERROR_NO_SOCKET;
-        }
-
-        _socket = new CellularSocket*[_device.get_property(AT_CellularDevice::PROPERTY_SOCKET_COUNT)];
-        for (int i = 0; i < _device.get_property(AT_CellularDevice::PROPERTY_SOCKET_COUNT); i++) {
-            _socket[i] = 0;
-        }
-    }
-
     int index = find_socket_index(0);
     if (index == -1) {
         tr_warn("No free sockets!");

features/cellular/framework/AT/AT_CellularStack.h

@@ -58,13 +58,6 @@ class AT_CellularStack : public NetworkStack {
 
 protected: // NetworkStack
 
-    /**
-     * Modem specific socket stack initialization
-     *
-     *  @return 0 on success
-     */
-    virtual nsapi_error_t socket_stack_init();
-
     /**
       * Note: Socket_open does not actually open socket on all drivers, but that's deferred until calling `sendto`.
       * The reason is that IP stack implementations are very much modem specific and it's quite common that when a

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

@@ -40,6 +40,10 @@ NetworkStack *GEMALTO_CINTERION_CellularContext::get_stack()
 
     if (!_stack) {
         _stack = new GEMALTO_CINTERION_CellularStack(get_at_handler(), _apn, _uname, _pwd, _cid, (nsapi_ip_stack_t)_pdp_type, *get_device());
+        if (static_cast<GEMALTO_CINTERION_CellularStack *>(_stack)->socket_stack_init() != NSAPI_ERROR_OK) {
+            delete _stack;
+            _stack = NULL;
+        }
     }
     return _stack;
 }

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

@@ -28,9 +28,13 @@ class GEMALTO_CINTERION_CellularStack : public AT_CellularStack {
                                     const char *password, int cid, nsapi_ip_stack_t stack_type, AT_CellularDevice &device);
     virtual ~GEMALTO_CINTERION_CellularStack();
 
-protected:
+    /** Initialize
+     *  Must be called immediately after constructor to initialize IP stack on the modem.
+     *  @return NSAPI_ERROR_OK on success
+     */
+    nsapi_error_t socket_stack_init();
 
-    virtual nsapi_error_t socket_stack_init();
+protected:
 
     virtual nsapi_error_t socket_close_impl(int sock_id);
 

features/cellular/framework/targets/QUECTEL/M26/QUECTEL_M26_CellularContext.cpp

@@ -29,6 +29,10 @@ NetworkStack *QUECTEL_M26_CellularContext::get_stack()
 {
     if (!_stack) {
         _stack = new QUECTEL_M26_CellularStack(_at, _cid, (nsapi_ip_stack_t)_pdp_type, *get_device());
+        if (static_cast<QUECTEL_M26_CellularStack *>(_stack)->socket_stack_init() != NSAPI_ERROR_OK) {
+            delete _stack;
+            _stack = NULL;
+        }
     }
     return _stack;
 }

features/cellular/framework/targets/QUECTEL/M26/QUECTEL_M26_CellularStack.h

@@ -31,6 +31,12 @@ class QUECTEL_M26_CellularStack : public AT_CellularStack {
     QUECTEL_M26_CellularStack(ATHandler &atHandler, int cid, nsapi_ip_stack_t stack_type, AT_CellularDevice &device);
     virtual ~QUECTEL_M26_CellularStack();
 
+    /** Initialize
+     *  Must be called immediately after constructor to initialize IP stack on the modem.
+     *  @return NSAPI_ERROR_OK on success
+     */
+    nsapi_error_t socket_stack_init();
+
 protected: // NetworkStack
 
     virtual nsapi_error_t socket_listen(nsapi_socket_t handle, int backlog);
@@ -44,8 +50,6 @@ class QUECTEL_M26_CellularStack : public AT_CellularStack {
 
 protected: // AT_CellularStack
 
-    virtual nsapi_error_t socket_stack_init();
-
     virtual nsapi_error_t socket_close_impl(int sock_id);
 
     virtual nsapi_error_t create_socket_impl(CellularSocket *socket);