Cellular: Fix deleting of state machine to correct class

Author: Teppo Järvelin

UNITTESTS/features/cellular/framework/device/cellularstatemachine/cellularstatemachinetest.cpp

@@ -73,9 +73,9 @@ class UT_CellularStateMachine {
         _state_machine = NULL;
     }
 
-    CellularStateMachine *create_state_machine(CellularDevice &device, events::EventQueue &queue)
+    CellularStateMachine *create_state_machine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw)
     {
-        _state_machine = new CellularStateMachine(device, queue);
+        _state_machine = new CellularStateMachine(device, queue, nw);
         return _state_machine;
     }
 
@@ -171,7 +171,7 @@ TEST_F(TestCellularStateMachine, test_create_delete)
     CellularDevice *dev = new myCellularDevice(&fh1);
     EXPECT_TRUE(dev);
 
-    CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
+    CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
     EXPECT_TRUE(stm);
     ut.delete_state_machine();
 
@@ -187,7 +187,7 @@ TEST_F(TestCellularStateMachine, test_setters)
     CellularDevice *dev = new myCellularDevice(&fh1);
     EXPECT_TRUE(dev);
 
-    CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
+    CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
     EXPECT_TRUE(stm);
     ut.set_cellular_callback(&cellular_callback);
 
@@ -215,14 +215,14 @@ TEST_F(TestCellularStateMachine, test_start_dispatch)
     CellularDevice *dev = new myCellularDevice(&fh1);
     EXPECT_TRUE(dev);
 
-    CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
+    CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
     EXPECT_TRUE(stm);
     nsapi_error_t err = ut.start_dispatch();
     ASSERT_EQ(NSAPI_ERROR_OK, err);
     ut.delete_state_machine();
 
     Thread_stub::osStatus_value = osErrorNoMemory;
-    stm = ut.create_state_machine(*dev, *dev->get_queue());
+    stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
     EXPECT_TRUE(stm);
     err = ut.start_dispatch();
     ASSERT_EQ(NSAPI_ERROR_NO_MEMORY, err);
@@ -240,21 +240,21 @@ TEST_F(TestCellularStateMachine, test_stop)
     CellularDevice *dev = new AT_CellularDevice(&fh1);
     EXPECT_TRUE(dev);
 
-    CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
+    CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
     EXPECT_TRUE(stm);
 
     ut.stop(); // nothing created, run through
     ut.delete_state_machine();
 
-    stm = ut.create_state_machine(*dev, *dev->get_queue());
+    stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
     EXPECT_TRUE(stm);
     nsapi_error_t err = ut.start_dispatch();
     ASSERT_EQ(NSAPI_ERROR_OK, err);
 
     ut.stop(); // thread is created, now stop will delete it
     ut.delete_state_machine();
 
-    stm = ut.create_state_machine(*dev, *dev->get_queue());
+    stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
     EXPECT_TRUE(stm);
     err = ut.start_dispatch();
     ASSERT_EQ(NSAPI_ERROR_OK, err);
@@ -270,7 +270,7 @@ TEST_F(TestCellularStateMachine, test_stop)
     ut.stop(); // thread and power are created, now stop will delete them
     ut.delete_state_machine();
 
-    stm = ut.create_state_machine(*dev, *dev->get_queue());
+    stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
     EXPECT_TRUE(stm);
     err = ut.start_dispatch();
     ASSERT_EQ(NSAPI_ERROR_OK, err);
@@ -294,7 +294,7 @@ TEST_F(TestCellularStateMachine, test_run_to_state)
     CellularDevice *dev = new AT_CellularDevice(&fh1);
     EXPECT_TRUE(dev);
 
-    CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
+    CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
     EXPECT_TRUE(stm);
 
     nsapi_error_t err = ut.start_dispatch();

UNITTESTS/stubs/CellularStateMachine_stub.cpp

@@ -25,8 +25,8 @@ CellularStubState CellularStateMachine_stub::get_current_target_state = STATE_IN
 CellularStubState CellularStateMachine_stub::get_current_current_state = STATE_INIT;
 bool CellularStateMachine_stub::bool_value = false;
 
-CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue) :
-    _cellularDevice(device), _queue(queue)
+CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw) :
+    _cellularDevice(device), _network(nw), _queue(queue)
 {
 }
 

features/cellular/framework/AT/AT_CellularDevice.cpp

@@ -47,8 +47,6 @@ AT_CellularDevice::AT_CellularDevice(FileHandle *fh) : CellularDevice(fh), _netw
 
 AT_CellularDevice::~AT_CellularDevice()
 {
-    delete _state_machine;
-
     // make sure that all is deleted even if somewhere close was not called and reference counting is messed up.
     _network_ref_count = 1;
     _sms_ref_count = 1;

features/cellular/framework/device/CellularDevice.cpp

@@ -44,6 +44,7 @@ CellularDevice::CellularDevice(FileHandle *fh) : _network_ref_count(0), _sms_ref
 CellularDevice::~CellularDevice()
 {
     tr_debug("CellularDevice destruct");
+    delete _state_machine;
 }
 
 void CellularDevice::stop()
@@ -118,7 +119,10 @@ nsapi_error_t CellularDevice::create_state_machine()
 {
     nsapi_error_t err = NSAPI_ERROR_OK;
     if (!_state_machine) {
-        _state_machine = new CellularStateMachine(*this, *get_queue());
+        _nw = open_network(_fh);
+        // Attach to network so we can get update status from the network
+        _nw->attach(callback(this, &CellularDevice::stm_callback));
+        _state_machine = new CellularStateMachine(*this, *get_queue(), *_nw);
         _state_machine->set_cellular_callback(callback(this, &CellularDevice::stm_callback));
         err = _state_machine->start_dispatch();
         if (err) {
@@ -184,13 +188,6 @@ void CellularDevice::cellular_callback(nsapi_event_t ev, intptr_t ptr, CellularC
             // broadcast only network registration changes to state machine
             _state_machine->cellular_event_changed(ev, ptr);
         }
-        if (cell_ev == CellularDeviceReady && ptr_data->error == NSAPI_ERROR_OK) {
-            // Here we can create mux and give new filehandles as mux reserves the one what was in use.
-            // if mux we would need to set new filehandle:_state_machine->set_filehandle( get fh from mux);
-            _nw = open_network(_fh);
-            // Attach to network so we can get update status from the network
-            _nw->attach(callback(this, &CellularDevice::stm_callback));
-        }
     } else {
         tr_debug("callback: %d, ptr: %d", ev, ptr);
         if (ev == NSAPI_EVENT_CONNECTION_STATUS_CHANGE && ptr == NSAPI_STATUS_DISCONNECTED) {

features/cellular/framework/device/CellularStateMachine.cpp

@@ -43,9 +43,9 @@ const int DEVICE_READY = 0x04;
 
 namespace mbed {
 
-CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue) :
+CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw) :
     _cellularDevice(device), _state(STATE_INIT), _next_state(_state), _target_state(_state),
-    _event_status_cb(0), _network(0), _queue(queue), _queue_thread(0), _sim_pin(0),
+    _event_status_cb(0), _network(nw), _queue(queue), _queue_thread(0), _sim_pin(0),
     _retry_count(0), _event_timeout(-1), _event_id(-1), _plmn(0), _command_success(false),
     _is_retry(false), _cb_data(), _current_event(NSAPI_EVENT_CONNECTION_STATUS_CHANGE), _status(0)
 {
@@ -99,11 +99,6 @@ void CellularStateMachine::stop()
 
     reset();
     _event_id = STM_STOPPED;
-
-    if (_network) {
-        _cellularDevice.close_network();
-        _network = NULL;
-    }
 }
 
 bool CellularStateMachine::power_on()
@@ -163,7 +158,7 @@ bool CellularStateMachine::open_sim()
     if (sim_ready) {
         // If plmn is set, we should it right after sim is opened so that registration is forced to correct network.
         if (_plmn && strlen(_plmn)) {
-            _cb_data.error = _network->set_registration(_plmn);
+            _cb_data.error = _network.set_registration(_plmn);
             tr_debug("STM: manual set_registration: %d, plmn: %s", _cb_data.error, _plmn);
             if (_cb_data.error) {
                 return false;
@@ -204,7 +199,7 @@ bool CellularStateMachine::get_network_registration(CellularNetwork::Registratio
     is_registered = false;
     bool is_roaming = false;
     CellularNetwork::registration_params_t reg_params;
-    _cb_data.error = _network->get_registration_params(type, reg_params);
+    _cb_data.error = _network.get_registration_params(type, reg_params);
 
     if (_cb_data.error != NSAPI_ERROR_OK) {
         if (_cb_data.error != NSAPI_ERROR_UNSUPPORTED) {
@@ -329,14 +324,10 @@ bool CellularStateMachine::device_ready()
 {
     tr_info("Modem ready");
 
-    if (!_network) {
-        _network = _cellularDevice.open_network();
-    }
-
 #ifdef MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY
     MBED_ASSERT(MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY >= CellularNetwork::RAT_GSM &&
                 MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY < CellularNetwork::RAT_UNKNOWN);
-    nsapi_error_t err = _network->set_access_technology((CellularNetwork::RadioAccessTechnology)MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY);
+    nsapi_error_t err = _network.set_access_technology((CellularNetwork::RadioAccessTechnology)MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY);
     if (err != NSAPI_ERROR_OK && err != NSAPI_ERROR_UNSUPPORTED) {
         tr_warning("Failed to set access technology to %d", MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY);
         return false;
@@ -382,7 +373,7 @@ void CellularStateMachine::state_sim_pin()
     if (open_sim()) {
         bool success = false;
         for (int type = 0; type < CellularNetwork::C_MAX; type++) {
-            _cb_data.error = _network->set_registration_urc((CellularNetwork::RegistrationType)type, true);
+            _cb_data.error = _network.set_registration_urc((CellularNetwork::RegistrationType)type, true);
             if (!_cb_data.error && (type == CellularNetwork::C_EREG || type == CellularNetwork::C_GREG)) {
                 success = true;
             }
@@ -393,19 +384,19 @@ void CellularStateMachine::state_sim_pin()
             return;
         }
 
-        if (_network->is_active_context()) { // check if context was already activated
+        if (_network.is_active_context()) { // check if context was already activated
             tr_debug("Active context found.");
             _status |= ACTIVE_PDP_CONTEXT;
         }
         CellularNetwork::AttachStatus status = CellularNetwork::Detached; // check if modem is already attached to a network
-        if (_network->get_attach(status) == NSAPI_ERROR_OK && status == CellularNetwork::Attached) {
+        if (_network.get_attach(status) == NSAPI_ERROR_OK && status == CellularNetwork::Attached) {
             _status |= ATTACHED_TO_NETWORK;
             tr_debug("Cellular already attached.");
         }
 
         // if packet domain event reporting is not set it's not a stopper. We might lack some events when we are
         // dropped from the network.
-        _cb_data.error = _network->set_packet_domain_event_reporting(true);
+        _cb_data.error = _network.set_packet_domain_event_reporting(true);
         if (_cb_data.error == NSAPI_STATUS_ERROR_UNSUPPORTED) {
             tr_warning("Packet domain event reporting not supported!");
         } else if (_cb_data.error) {
@@ -434,7 +425,7 @@ void CellularStateMachine::state_registering()
     } else {
         _cellularDevice.set_timeout(TIMEOUT_REGISTRATION);
         if (!_command_success && !_plmn) { // don't call set_registration twice for manual registration
-            _cb_data.error = _network->set_registration(_plmn);
+            _cb_data.error = _network.set_registration(_plmn);
             _command_success = (_cb_data.error == NSAPI_ERROR_OK);
         }
         retry_state_or_fail();
@@ -446,7 +437,7 @@ void CellularStateMachine::state_attaching()
     _cellularDevice.set_timeout(TIMEOUT_CONNECT);
     tr_info("Attaching network (timeout %d s)", TIMEOUT_CONNECT / 1000);
     if (_status != ATTACHED_TO_NETWORK) {
-        _cb_data.error = _network->set_attach();
+        _cb_data.error = _network.set_attach();
     }
     if (_cb_data.error == NSAPI_ERROR_OK) {
         if (_event_status_cb) {
@@ -534,14 +525,12 @@ bool CellularStateMachine::get_current_status(CellularStateMachine::CellularStat
 void CellularStateMachine::event()
 {
 #if MBED_CONF_MBED_TRACE_ENABLE
-    if (_network) {
-        int rssi;
-        if (_network->get_signal_quality(rssi) == NSAPI_ERROR_OK) {
-            if (rssi == CellularNetwork::SignalQualityUnknown) {
-                tr_info("RSSI unknown");
-            } else {
-                tr_info("RSSI %d dBm", rssi);
-            }
+    int rssi;
+    if (_network.get_signal_quality(rssi) == NSAPI_ERROR_OK) {
+        if (rssi == CellularNetwork::SignalQualityUnknown) {
+            tr_info("RSSI unknown");
+        } else {
+            tr_info("RSSI %d dBm", rssi);
         }
     }
 #endif
@@ -643,7 +632,7 @@ void CellularStateMachine::cellular_event_changed(nsapi_event_t ev, intptr_t ptr
     if ((cellular_connection_status_t)ev == CellularRegistrationStatusChanged && _state == STATE_REGISTERING_NETWORK) {
         // expect packet data so only these states are valid
         CellularNetwork::registration_params_t reg_params;
-        nsapi_error_t err = _network->get_registration_params(reg_params);
+        nsapi_error_t err = _network.get_registration_params(reg_params);
 
         if (err == NSAPI_ERROR_OK && (reg_params._type == CellularNetwork::C_EREG || reg_params._type == CellularNetwork::C_GREG)) {
             if ((data->status_data == CellularNetwork::RegisteredHomeNetwork ||

features/cellular/framework/device/CellularStateMachine.h

@@ -44,8 +44,9 @@ class CellularStateMachine {
      *
      * @param device    reference to CellularDevice
      * @param queue     reference to queue used in state transitions
+     * @param nw        reference to CellularNetwork
      */
-    CellularStateMachine(CellularDevice &device, events::EventQueue &queue);
+    CellularStateMachine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw);
     ~CellularStateMachine();
 
     /** Cellular connection states
@@ -161,7 +162,7 @@ class CellularStateMachine {
 
     Callback<void(nsapi_event_t, intptr_t)> _event_status_cb;
 
-    CellularNetwork *_network;
+    CellularNetwork &_network;
     events::EventQueue &_queue;
     rtos::Thread *_queue_thread;