Cellular: fix connect-disconnect sequence called many times

Fix syncing back to at mode after ppp disconnect.
Fix AT_CellularContext flags and states to allow new connect after disconnect.
Fix that state machine is not reseted in disconnect is it's running (might be
running because of another context or new connect already started).

Author: Teppo Järvelin

UNITTESTS/stubs/CellularDevice_stub.cpp

@@ -103,6 +103,6 @@ nsapi_error_t CellularDevice::shutdown()
     return NSAPI_ERROR_OK;
 }
 
-void CellularDevice::cellular_callback(nsapi_event_t ev, intptr_t ptr)
+void CellularDevice::cellular_callback(nsapi_event_t ev, intptr_t ptr, CellularContext *ctx)
 {
 }

features/cellular/framework/API/CellularContext.h

@@ -133,6 +133,8 @@ class CellularContext : public CellularInterface {
      *  The parameters on the callback are the event type and event type dependent reason parameter.
      *
      *  @remark  deleting CellularDevice/CellularContext in callback is not allowed.
+     *  @remark  Allocating/adding lots of traces not recommended as callback is called mostly from State machines thread which
+     *           is now 2048. You can change to main thread for example via EventQueue.
      *
      *  @param status_cb The callback for status changes.
      */

features/cellular/framework/API/CellularDevice.h

@@ -283,8 +283,8 @@ class CellularDevice {
      *  The parameters on the callback are the event type and event-type dependent reason parameter.
      *
      *  @remark  deleting CellularDevice/CellularContext in callback not allowed.
-     *  @remark  application should not attach to this function if using CellularContext::attach as it will contain the
-     *           same information.
+     *  @remark  Allocating/adding lots of traces not recommended as callback is called mostly from State machines thread which
+     *           is now 2048. You can change to main thread for example via EventQueue.
      *
      *  @param status_cb The callback for status changes.
      */
@@ -420,8 +420,8 @@ class CellularDevice {
      *  This method will broadcast to every interested classes:
      *  CellularContext (might be many) and CellularStateMachine if available.
      */
-    void cellular_callback(nsapi_event_t ev, intptr_t ptr);
-
+    void cellular_callback(nsapi_event_t ev, intptr_t ptr, CellularContext *ctx = NULL);
+    void stm_callback(nsapi_event_t ev, intptr_t ptr);
     int _network_ref_count;
     int _sms_ref_count;
     int _info_ref_count;

features/cellular/framework/API/CellularNetwork.h

@@ -319,6 +319,10 @@ class CellularNetwork {
      *  on the network. The parameters on the callback are the event type and
      *  event-type dependent reason parameter.
      *
+     *  @remark Application should not call attach if using CellularContext class. Call instead CellularContext::attach
+     *          as CellularDevice is dependent of this attach if CellularContext/CellularDevice is used to get
+     *          device/sim ready, registered, attached, connected.
+     *
      *  @param status_cb The callback for status changes
      */
     virtual void attach(mbed::Callback<void(nsapi_event_t, intptr_t)> status_cb) = 0;

features/cellular/framework/AT/ATHandler.cpp

@@ -1256,22 +1256,27 @@ void ATHandler::debug_print(const char *p, int len)
 bool ATHandler::sync(int timeout_ms)
 {
     tr_debug("AT sync");
+    lock();
+    uint32_t timeout = _at_timeout;
+    _at_timeout = timeout_ms;
     // poll for 10 seconds
     for (int i = 0; i < 10; i++) {
-        lock();
-        set_at_timeout(timeout_ms, false);
         // For sync use an AT command that is supported by all modems and likely not used frequently,
         // especially a common response like OK could be response to previous request.
+        clear_error();
+        _start_time = rtos::Kernel::get_ms_count();
         cmd_start("AT+CMEE?");
         cmd_stop();
         resp_start("+CMEE:");
         resp_stop();
-        restore_at_timeout();
-        unlock();
         if (!_last_err) {
+            _at_timeout = timeout;
+            unlock();
             return true;
         }
     }
     tr_error("AT sync failed");
+    _at_timeout = timeout;
+    unlock();
     return false;
 }

features/cellular/framework/AT/AT_CellularContext.cpp

@@ -498,7 +498,6 @@ nsapi_error_t AT_CellularContext::activate_context()
     if (err != NSAPI_ERROR_OK) {
         _at.unlock();
         tr_error("Failed to activate network context! (%d)", err);
-        call_network_cb(NSAPI_STATUS_DISCONNECTED);
         return err;
     }
 
@@ -551,16 +550,18 @@ void AT_CellularContext::do_connect()
 {
     if (!_is_context_active) {
         _cb_data.error = do_activate_context();
+    } else {
+        _cb_data.error = NSAPI_ERROR_OK;
+    }
+
 #if !NSAPI_PPP_AVAILABLE
-        // in PPP mode we did not activate any context, just searched the correct _cid
-        if (_status_cb) {
-            _status_cb((nsapi_event_t)CellularActivatePDPContext, (intptr_t)&_cb_data);
-        }
-#endif // !NSAPI_PPP_AVAILABLE
+    // in PPP mode we did not activate any context, just searched the correct _cid
+    if (_status_cb) {
+        _status_cb((nsapi_event_t)CellularActivatePDPContext, (intptr_t)&_cb_data);
     }
+#endif // !NSAPI_PPP_AVAILABLE
 
     if (_cb_data.error != NSAPI_ERROR_OK) {
-        call_network_cb(NSAPI_STATUS_DISCONNECTED);
         _is_connected = false;
         return;
     }
@@ -630,16 +631,23 @@ void AT_CellularContext::ppp_status_cb(nsapi_event_t ev, intptr_t ptr)
     tr_debug("ppp_status_cb: event %d, ptr %d", ev, ptr);
     if (ev == NSAPI_EVENT_CONNECTION_STATUS_CHANGE && ptr == NSAPI_STATUS_GLOBAL_UP) {
         _is_connected = true;
-    } else if (ev == NSAPI_EVENT_CONNECTION_STATUS_CHANGE && ptr == NSAPI_STATUS_DISCONNECTED) {
-        ppp_disconnected();
     } else {
         _is_connected = false;
     }
 
     _connect_status = (nsapi_connection_status_t)ptr;
 
+    // catch all NSAPI_STATUS_DISCONNECTED events but send to device only when we did not ask for disconnect.
+    if (ev == NSAPI_EVENT_CONNECTION_STATUS_CHANGE && ptr == NSAPI_STATUS_DISCONNECTED) {
+        if (_is_connected) {
+            ppp_disconnected();
+            _device->cellular_callback(ev, ptr, this);
+        }
+        return;
+    }
+
     // call device's callback, it will broadcast this to here (cellular_callback)
-    _device->cellular_callback(ev, ptr);
+    _device->cellular_callback(ev, ptr, this);
 }
 
 void AT_CellularContext::ppp_disconnected()
@@ -660,10 +668,13 @@ void AT_CellularContext::ppp_disconnected()
 
 nsapi_error_t AT_CellularContext::disconnect()
 {
-    tr_info("CellularContext disconnect");
+    tr_info("CellularContext disconnect()");
     if (!_nw || !_is_connected) {
         return NSAPI_ERROR_NO_CONNECTION;
     }
+
+    // set false here so callbacks know that we are not connected and so should not send DISCONNECTED
+    _is_connected = false;
 #if NSAPI_PPP_AVAILABLE
     nsapi_error_t err = nsapi_ppp_disconnect(_at.get_file_handle());
     if (err != NSAPI_ERROR_OK) {
@@ -681,11 +692,12 @@ nsapi_error_t AT_CellularContext::disconnect()
         } else {
             deactivate_ip_context();
         }
-    } else {
-        call_network_cb(NSAPI_STATUS_DISCONNECTED);
     }
+    _is_context_active = false;
+    _connect_status = NSAPI_STATUS_DISCONNECTED;
 
-    _is_connected = false;
+    // call device's callback, it will broadcast this to here (cellular_callback)
+    _device->cellular_callback(NSAPI_EVENT_CONNECTION_STATUS_CHANGE, NSAPI_STATUS_DISCONNECTED, this);
 
     return _at.unlock_return_error();
 }
@@ -928,27 +940,32 @@ void AT_CellularContext::cellular_callback(nsapi_event_t ev, intptr_t ptr)
         if (_is_blocking) {
             if (data->error != NSAPI_ERROR_OK) {
                 // operation failed, release semaphore
+                _current_op = OP_INVALID;
                 _semaphore.release();
             } else {
                 if ((st == CellularDeviceReady && _current_op == OP_DEVICE_READY) ||
                         (st == CellularSIMStatusChanged && _current_op == OP_SIM_READY &&
                          data->status_data == CellularDevice::SimStateReady)) {
                     // target reached, release semaphore
+                    _current_op = OP_INVALID;
                     _semaphore.release();
                 } else if (st == CellularRegistrationStatusChanged && (data->status_data == CellularNetwork::RegisteredHomeNetwork ||
                                                                        data->status_data == CellularNetwork::RegisteredRoaming || data->status_data == CellularNetwork::AlreadyRegistered) && _current_op == OP_REGISTER) {
                     // target reached, release semaphore
+                    _current_op = OP_INVALID;
                     _semaphore.release();
                 } else if (st == CellularAttachNetwork && (_current_op == OP_ATTACH || _current_op == OP_CONNECT) &&
                            data->status_data == CellularNetwork::Attached) {
                     // target reached, release semaphore
+                    _current_op = OP_INVALID;
                     _semaphore.release();
                 }
             }
         } else {
             // non blocking
             if (st == CellularAttachNetwork && _current_op == OP_CONNECT && data->error == NSAPI_ERROR_OK &&
                     data->status_data == CellularNetwork::Attached) {
+                _current_op = OP_INVALID;
                 // forward to application
                 if (_status_cb) {
                     _status_cb(ev, ptr);
@@ -963,14 +980,18 @@ void AT_CellularContext::cellular_callback(nsapi_event_t ev, intptr_t ptr)
             if (ev == NSAPI_EVENT_CONNECTION_STATUS_CHANGE && ptr == NSAPI_STATUS_GLOBAL_UP) {
                 tr_info("CellularContext IP %s", get_ip_address());
                 _cb_data.error = NSAPI_ERROR_OK;
-                _semaphore.release();
             } else if (ev == NSAPI_EVENT_CONNECTION_STATUS_CHANGE && ptr == NSAPI_STATUS_DISCONNECTED) {
                 tr_info("PPP disconnected");
                 _cb_data.error = NSAPI_ERROR_NO_CONNECTION;
-                _semaphore.release();
             }
         }
-#endif
+#else
+#if MBED_CONF_MBED_TRACE_ENABLE
+        if (ev == NSAPI_EVENT_CONNECTION_STATUS_CHANGE && ptr == NSAPI_STATUS_DISCONNECTED) {
+            tr_info("cb: CellularContext disconnected");
+        }
+#endif // MBED_CONF_MBED_TRACE_ENABLE
+#endif // NSAPI_PPP_AVAILABLE
     }
 
     // forward to application
@@ -1044,8 +1065,7 @@ void AT_CellularContext::ciot_opt_cb(mbed::CellularNetwork::CIoT_Supported_Opt
 
 void AT_CellularContext::set_disconnect()
 {
+    tr_debug("AT_CellularContext::set_disconnect()");
     _is_connected = false;
-    cell_callback_data_t data;
-    data.error = NSAPI_STATUS_DISCONNECTED;
-    _device->cellular_callback(NSAPI_EVENT_CONNECTION_STATUS_CHANGE, (intptr_t)&data);
+    _device->cellular_callback(NSAPI_EVENT_CONNECTION_STATUS_CHANGE, NSAPI_STATUS_DISCONNECTED, this);
 }

features/cellular/framework/AT/AT_CellularNetwork.cpp

@@ -171,7 +171,7 @@ void AT_CellularNetwork::read_reg_params_and_compare(RegistrationType type)
                     reg_params._status == RegisteredRoaming)) {
                 if (previous_registration_status == RegisteredHomeNetwork ||
                         previous_registration_status == RegisteredRoaming) {
-                    _connection_status_cb(NSAPI_EVENT_CONNECTION_STATUS_CHANGE, NSAPI_STATUS_DISCONNECTED);
+                    call_network_cb(NSAPI_STATUS_DISCONNECTED);
                 }
             }
         }
@@ -200,11 +200,8 @@ void AT_CellularNetwork::urc_cgreg()
 
 void AT_CellularNetwork::call_network_cb(nsapi_connection_status_t status)
 {
-    if (_connect_status != status) {
-        _connect_status = status;
-        if (_connection_status_cb) {
-            _connection_status_cb(NSAPI_EVENT_CONNECTION_STATUS_CHANGE, _connect_status);
-        }
+    if (_connection_status_cb) {
+        _connection_status_cb(NSAPI_EVENT_CONNECTION_STATUS_CHANGE, _connect_status);
     }
 }
 

features/cellular/framework/device/CellularDevice.cpp

@@ -113,13 +113,20 @@ nsapi_error_t CellularDevice::create_state_machine()
     nsapi_error_t err = NSAPI_ERROR_OK;
     if (!_state_machine) {
         _state_machine = new CellularStateMachine(*this, *get_queue());
-        _state_machine->set_cellular_callback(callback(this, &CellularDevice::cellular_callback));
+        _state_machine->set_cellular_callback(callback(this, &CellularDevice::stm_callback));
         err = _state_machine->start_dispatch();
         if (err) {
             tr_error("Start state machine failed.");
             delete _state_machine;
             _state_machine = NULL;
         }
+
+        if (strlen(_plmn)) {
+            _state_machine->set_plmn(_plmn);
+        }
+        if (strlen(_sim_pin)) {
+            _state_machine->set_sim_pin(_sim_pin);
+        }
     }
     return err;
 }
@@ -156,7 +163,12 @@ void CellularDevice::attach(Callback<void(nsapi_event_t, intptr_t)> status_cb)
     _status_cb = status_cb;
 }
 
-void CellularDevice::cellular_callback(nsapi_event_t ev, intptr_t ptr)
+void CellularDevice::stm_callback(nsapi_event_t ev, intptr_t ptr)
+{
+    cellular_callback(ev, ptr);
+}
+
+void CellularDevice::cellular_callback(nsapi_event_t ev, intptr_t ptr, CellularContext *ctx)
 {
     if (ev >= NSAPI_EVENT_CELLULAR_STATUS_BASE && ev <= NSAPI_EVENT_CELLULAR_STATUS_END) {
         cell_callback_data_t *ptr_data = (cell_callback_data_t *)ptr;
@@ -166,40 +178,42 @@ void CellularDevice::cellular_callback(nsapi_event_t ev, intptr_t ptr)
             // 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::cellular_callback));
-            if (strlen(_plmn)) {
-                _state_machine->set_plmn(_plmn);
-            }
-        } else if (cell_ev == CellularSIMStatusChanged && ptr_data->error == NSAPI_ERROR_OK &&
-                   ptr_data->status_data == SimStatePinNeeded) {
-            if (strlen(_sim_pin)) {
-                _state_machine->set_sim_pin(_sim_pin);
-            }
+            _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) {
             // we have been disconnected, reset state machine so that application can start connect sequence again
             if (_state_machine) {
-                _state_machine->reset();
+                CellularStateMachine::CellularState current_state, targeted_state;
+                bool is_running = _state_machine->get_current_status(current_state, targeted_state);
+                if (!is_running) {
+                    _state_machine->reset();
+                }
             }
         }
     }
 
     // broadcast network and cellular changes to state machine and CellularContext.
     CellularContext *curr = get_context_list();
     while (curr) {
-        curr->cellular_callback(ev, ptr);
+        if (ctx) {
+            if (ctx == curr) {
+                curr->cellular_callback(ev, ptr);
+                break;
+            }
+        } else {
+            curr->cellular_callback(ev, ptr);
+        }
         curr = curr->_next;
     }
 
-    // forward to callback function if set by attach(...)
+    // forward to callback function if set by attach(...).
     if (_status_cb) {
         _status_cb(ev, ptr);
     }

features/cellular/framework/device/CellularStateMachine.cpp

@@ -320,6 +320,7 @@ void CellularStateMachine::retry_state_or_fail()
         tr_debug("%s: retry %d/%d", get_state_string(_state), _retry_count, RETRY_ARRAY_SIZE);
         _event_timeout = _retry_timeout_array[_retry_count];
         _is_retry = true;
+        _cb_data.error = NSAPI_ERROR_OK;
     } else {
         report_failure(get_state_string(_state));
         return;