Cellular: Fix CellularStateMachine timeout configurations

Author: Ari Parkkila

UNITTESTS/stubs/CellularStateMachine_stub.cpp

@@ -79,3 +79,11 @@ void CellularStateMachine::get_retry_timeout_array(uint16_t *timeout, int &array
 {
 
 }
+
+void CellularStateMachine::set_retry_timeout_array(const uint16_t timeout[], int array_len)
+{
+}
+
+void CellularStateMachine::set_timeout(int timeout)
+{
+}

features/cellular/framework/API/CellularDevice.h

@@ -331,11 +331,27 @@ class CellularDevice {
     virtual void close_information() = 0;
 
     /** Set the default response timeout.
+     *
+     *  @remark CellularStateMachine timeouts for all states are also changed to `timeout`.
      *
      *  @param timeout    milliseconds to wait response from modem
      */
     virtual void set_timeout(int timeout) = 0;
 
+    /** Set an array of timeouts to wait before CellularStateMachine retries after failure.
+     *  To disable retry behavior completely use `set_retry_timeout_array(NULL, 0)`.
+     *  CellularContext callback event `cell_callback_data_t.final_try` indicates true when all retries have failed.
+     *
+     *  @remark Use `set_retry_timeout_array` for CellularStateMachine to wait before it retries again after failure,
+     *          this is useful to send repetitive requests when don't know exactly when modem is ready to accept requests.
+     *          Use `set_timeout` for timeout how long to wait for a response from modem for each request,
+     *          this is useful if modem can accept requests but processing takes long time before sending response.
+     *
+     *  @param timeout      timeout array using seconds
+     *  @param array_len    length of the array
+     */
+    void set_retry_timeout_array(const uint16_t timeout[], int array_len);
+
     /** Turn modem debug traces on
      *
      *  @param on         set true to enable debug traces

features/cellular/framework/AT/AT_CellularContext.cpp

@@ -903,6 +903,12 @@ void AT_CellularContext::cellular_callback(nsapi_event_t ev, intptr_t ptr)
         cell_callback_data_t *data = (cell_callback_data_t *)ptr;
         cellular_connection_status_t st = (cellular_connection_status_t)ev;
         _cb_data.error = data->error;
+        _cb_data.final_try = data->final_try;
+        if (data->final_try) {
+            if (_current_op != OP_INVALID) {
+                _semaphore.release();
+            }
+        }
 #if USE_APN_LOOKUP
         if (st == CellularSIMStatusChanged && data->status_data == CellularDevice::SimStateReady &&
                 _cb_data.error == NSAPI_ERROR_OK) {
@@ -924,7 +930,9 @@ void AT_CellularContext::cellular_callback(nsapi_event_t ev, intptr_t ptr)
                     _device->stop();
                     if (_is_blocking) {
                         // operation failed, release semaphore
-                        _semaphore.release();
+                        if (_current_op != OP_INVALID) {
+                            _semaphore.release();
+                        }
                     }
                 }
                 _device->close_information();
@@ -948,8 +956,10 @@ void AT_CellularContext::cellular_callback(nsapi_event_t ev, intptr_t ptr)
         if (_is_blocking) {
             if (_cb_data.error != NSAPI_ERROR_OK) {
                 // operation failed, release semaphore
-                _current_op = OP_INVALID;
-                _semaphore.release();
+                if (_current_op != OP_INVALID) {
+                    _current_op = OP_INVALID;
+                    _semaphore.release();
+                }
             } else {
                 if ((st == CellularDeviceReady && _current_op == OP_DEVICE_READY) ||
                         (st == CellularSIMStatusChanged && _current_op == OP_SIM_READY &&

features/cellular/framework/AT/AT_CellularDevice.cpp

@@ -344,6 +344,10 @@ void AT_CellularDevice::set_timeout(int timeout)
     _default_timeout = timeout;
 
     ATHandler::set_at_timeout_list(_default_timeout, true);
+
+    if (_state_machine) {
+        _state_machine->set_timeout(_default_timeout);
+    }
 }
 
 uint16_t AT_CellularDevice::get_send_delay() const

features/cellular/framework/device/CellularContext.cpp

@@ -70,6 +70,12 @@ CellularDevice *CellularContext::get_device() const
 
 void CellularContext::do_connect_with_retry()
 {
+    if (_cb_data.final_try) {
+        _cb_data.final_try = false;
+        _cb_data.error == NSAPI_ERROR_NO_CONNECTION;
+        call_network_cb(NSAPI_STATUS_DISCONNECTED);
+        return;
+    }
     do_connect();
     if (_cb_data.error == NSAPI_ERROR_OK) {
         return;

features/cellular/framework/device/CellularDevice.cpp

@@ -237,4 +237,11 @@ nsapi_error_t CellularDevice::shutdown()
     return NSAPI_ERROR_OK;
 }
 
+void CellularDevice::set_retry_timeout_array(const uint16_t timeout[], int array_len)
+{
+    if (create_state_machine() == NSAPI_ERROR_OK) {
+        _state_machine->set_retry_timeout_array(timeout, array_len);
+    }
+}
+
 } // namespace mbed

features/cellular/framework/device/CellularStateMachine.cpp

@@ -28,6 +28,11 @@
 #define TIMEOUT_POWER_ON     (1*1000)
 #define TIMEOUT_SIM_PIN      (1*1000)
 #define TIMEOUT_NETWORK      (10*1000)
+/** CellularStateMachine does connecting up to packet service attach, and
+ *  after that it's up to CellularContext::connect() to connect to PDN.
+ *  If CellularContext or an application does not set timeout (via `CellularDevice::set_timeout`)
+ *  then TIMEOUT_CONNECT is used also for connecting to PDN and also for socket operations.
+ */
 #define TIMEOUT_CONNECT      (60*1000)
 #define TIMEOUT_REGISTRATION (180*1000)
 
@@ -36,6 +41,7 @@
 
 #define RETRY_COUNT_DEFAULT 3
 
+
 const int STM_STOPPED = -99;
 const int ACTIVE_PDP_CONTEXT = 0x01;
 const int ATTACHED_TO_NETWORK = 0x02;
@@ -68,6 +74,12 @@ CellularStateMachine::CellularStateMachine(CellularDevice &device, events::Event
     _retry_timeout_array[8] = 600;
     _retry_timeout_array[9] = TIMEOUT_NETWORK_MAX;
     _retry_array_length = CELLULAR_RETRY_ARRAY_SIZE;
+
+    _state_timeout_power_on = TIMEOUT_POWER_ON;
+    _state_timeout_sim_pin = TIMEOUT_SIM_PIN;
+    _state_timeout_registration = TIMEOUT_REGISTRATION;
+    _state_timeout_network = TIMEOUT_NETWORK;
+    _state_timeout_connect = TIMEOUT_CONNECT;
 }
 
 CellularStateMachine::~CellularStateMachine()
@@ -273,8 +285,8 @@ void CellularStateMachine::enter_to_state(CellularState state)
 
 void CellularStateMachine::retry_state_or_fail()
 {
-    if (++_retry_count < CELLULAR_RETRY_ARRAY_SIZE) {
-        tr_debug("%s: retry %d/%d", get_state_string(_state), _retry_count, CELLULAR_RETRY_ARRAY_SIZE);
+    if (_retry_count < _retry_array_length) {
+        tr_debug("%s: retry %d/%d", get_state_string(_state), _retry_count, _retry_array_length);
         // send info to application/driver about error logic so it can implement proper error logic
         _cb_data.status_data = _current_event;
         _cb_data.data = &_retry_count;
@@ -284,15 +296,17 @@ void CellularStateMachine::retry_state_or_fail()
         _event_timeout = _retry_timeout_array[_retry_count];
         _is_retry = true;
         _cb_data.error = NSAPI_ERROR_OK;
+        _retry_count++;
     } else {
+        _cb_data.final_try = true;
         report_failure(get_state_string(_state));
     }
 }
 
 void CellularStateMachine::state_init()
 {
-    _cellularDevice.set_timeout(TIMEOUT_POWER_ON);
-    tr_info("Start connecting (timeout %d s)", TIMEOUT_POWER_ON / 1000);
+    _cellularDevice.set_timeout(_state_timeout_power_on);
+    tr_info("Start connecting (timeout %d ms)", _state_timeout_power_on);
     _cb_data.error = _cellularDevice.is_ready();
     _status = _cb_data.error ? 0 : DEVICE_READY;
     if (_cb_data.error != NSAPI_ERROR_OK) {
@@ -308,8 +322,8 @@ void CellularStateMachine::state_init()
 
 void CellularStateMachine::state_power_on()
 {
-    _cellularDevice.set_timeout(TIMEOUT_POWER_ON);
-    tr_info("Modem power ON (timeout %d s)", TIMEOUT_POWER_ON / 1000);
+    _cellularDevice.set_timeout(_state_timeout_power_on);
+    tr_info("Modem power ON (timeout %d ms)", _state_timeout_power_on);
     if (power_on()) {
         enter_to_state(STATE_DEVICE_READY);
     } else {
@@ -340,7 +354,7 @@ bool CellularStateMachine::device_ready()
 
 void CellularStateMachine::state_device_ready()
 {
-    _cellularDevice.set_timeout(TIMEOUT_POWER_ON);
+    _cellularDevice.set_timeout(_state_timeout_power_on);
     if (!(_status & DEVICE_READY)) {
         tr_debug("Device was not ready, calling soft_power_on()");
         _cb_data.error = _cellularDevice.soft_power_on();
@@ -364,8 +378,8 @@ void CellularStateMachine::state_device_ready()
 
 void CellularStateMachine::state_sim_pin()
 {
-    _cellularDevice.set_timeout(TIMEOUT_SIM_PIN);
-    tr_info("Setup SIM (timeout %d s)", TIMEOUT_SIM_PIN / 1000);
+    _cellularDevice.set_timeout(_state_timeout_sim_pin);
+    tr_info("Setup SIM (timeout %d ms)", _state_timeout_sim_pin);
     if (open_sim()) {
         bool success = false;
         for (int type = 0; type < CellularNetwork::C_MAX; type++) {
@@ -419,8 +433,7 @@ void CellularStateMachine::state_signal_quality()
 
 void CellularStateMachine::state_registering()
 {
-    _cellularDevice.set_timeout(TIMEOUT_NETWORK);
-    tr_info("Network registration (timeout %d s)", TIMEOUT_REGISTRATION / 1000);
+    _cellularDevice.set_timeout(_state_timeout_network);
     if (is_registered()) {
         if (_cb_data.status_data != CellularNetwork::RegisteredHomeNetwork &&
                 _cb_data.status_data != CellularNetwork::RegisteredRoaming && _status) {
@@ -432,7 +445,8 @@ void CellularStateMachine::state_registering()
         // we are already registered, go to attach
         enter_to_state(STATE_ATTACHING_NETWORK);
     } else {
-        _cellularDevice.set_timeout(TIMEOUT_REGISTRATION);
+        tr_info("Network registration (timeout %d ms)", _state_timeout_registration);
+        _cellularDevice.set_timeout(_state_timeout_registration);
         if (!_command_success && !_plmn) { // don't call set_registration twice for manual registration
             _cb_data.error = _network.set_registration(_plmn);
             _command_success = (_cb_data.error == NSAPI_ERROR_OK);
@@ -443,9 +457,9 @@ void CellularStateMachine::state_registering()
 
 void CellularStateMachine::state_attaching()
 {
-    _cellularDevice.set_timeout(TIMEOUT_CONNECT);
-    tr_info("Attaching network (timeout %d s)", TIMEOUT_CONNECT / 1000);
     if (_status != ATTACHED_TO_NETWORK) {
+        _cellularDevice.set_timeout(_state_timeout_connect);
+        tr_info("Attaching network (timeout %d ms)", _state_timeout_connect);
         _cb_data.error = _network.set_attach();
     }
     if (_cb_data.error == NSAPI_ERROR_OK) {
@@ -695,11 +709,10 @@ void CellularStateMachine::device_ready_cb()
 void CellularStateMachine::set_retry_timeout_array(const uint16_t timeout[], int array_len)
 {
     if (!timeout || array_len <= 0) {
-        tr_warn("set_retry_timeout_array, timeout array null or invalid length");
+        _retry_array_length = 0;
         return;
     }
     _retry_array_length = array_len > CELLULAR_RETRY_ARRAY_SIZE ? CELLULAR_RETRY_ARRAY_SIZE : array_len;
-
     for (int i = 0; i < _retry_array_length; i++) {
         _retry_timeout_array[i] = timeout[i];
     }
@@ -713,5 +726,14 @@ void CellularStateMachine::get_retry_timeout_array(uint16_t *timeout, int &array
     array_len = _retry_array_length;
 }
 
+void CellularStateMachine::set_timeout(int timeout)
+{
+    _state_timeout_power_on = timeout;
+    _state_timeout_sim_pin = timeout;
+    _state_timeout_registration = timeout;
+    _state_timeout_network = timeout;
+    _state_timeout_connect = timeout;
+}
+
 } // namespace
 

features/cellular/framework/device/CellularStateMachine.h

@@ -184,6 +184,15 @@ class CellularStateMachine {
     cellular_connection_status_t _current_event;
     int _status;
     PlatformMutex _mutex;
+
+    // Cellular state timeouts
+    int _state_timeout_power_on;
+    int _state_timeout_sim_pin;
+    int _state_timeout_registration;
+    int _state_timeout_network;
+    int _state_timeout_connect; // timeout for PS attach, PDN connect and socket operations
+    // Change all cellular state timeouts to `timeout`
+    void set_timeout(int timeout);
     cell_signal_quality_t _signal_quality;
 };