Merge branch 'plmn_stm_fix2' of https://github.com/jarvte/mbed-os into rollup

0xc0170 · 0xc0170 · commit 759627ff1279 · 2019-03-15T08:45:14.000Z
diff --git a/UNITTESTS/features/cellular/framework/device/cellularstatemachine/cellularstatemachinetest.cpp b/UNITTESTS/features/cellular/framework/device/cellularstatemachine/cellularstatemachinetest.cpp
@@ -34,7 +34,6 @@ enum UT_CellularState {
     UT_STATE_DEVICE_READY,
     UT_STATE_SIM_PIN,
     UT_STATE_REGISTERING_NETWORK,
-    UT_STATE_MANUAL_REGISTERING_NETWORK,
     UT_STATE_ATTACHING_NETWORK,
     UT_STATE_MAX_FSM_STATE
 };
@@ -392,8 +391,8 @@ TEST_F(TestCellularStateMachine, test_run_to_state)
     ut.set_plmn("12345");
     ASSERT_EQ(NSAPI_ERROR_OK, ut.run_to_device_registered());
     (void)ut.get_current_status(current_state, target_state);
-    ASSERT_EQ(UT_STATE_MANUAL_REGISTERING_NETWORK, current_state);
-    ASSERT_EQ(UT_STATE_MANUAL_REGISTERING_NETWORK, target_state);
+    ASSERT_EQ(UT_STATE_REGISTERING_NETWORK, current_state);
+    ASSERT_EQ(UT_STATE_REGISTERING_NETWORK, target_state);
     ut.cellular_event_changed((nsapi_event_t)CellularRegistrationStatusChanged, (intptr_t)&data);
     ut.reset();
 
diff --git a/features/cellular/framework/AT/AT_CellularNetwork.cpp b/features/cellular/framework/AT/AT_CellularNetwork.cpp
@@ -171,7 +171,9 @@ void AT_CellularNetwork::read_reg_params_and_compare(RegistrationType type)
                     reg_params._status == RegisteredRoaming)) {
                 if (previous_registration_status == RegisteredHomeNetwork ||
                         previous_registration_status == RegisteredRoaming) {
-                    call_network_cb(NSAPI_STATUS_DISCONNECTED);
+                    if (type != C_REG) {// we are interested only if we drop from packet network
+                        _connection_status_cb(NSAPI_EVENT_CONNECTION_STATUS_CHANGE, NSAPI_STATUS_DISCONNECTED);
+                    }
                 }
             }
         }
@@ -268,6 +270,9 @@ nsapi_error_t AT_CellularNetwork::set_registration(const char *plmn)
         tr_debug("Manual network registration to %s", plmn);
         _at.cmd_start("AT+COPS=1,2,");
         _at.write_string(plmn);
+        if (_op_act != RAT_UNKNOWN) {
+            _at.write_int(_op_act);
+        }
         _at.cmd_stop_read_resp();
     }
 
diff --git a/features/cellular/framework/device/CellularStateMachine.cpp b/features/cellular/framework/device/CellularStateMachine.cpp
@@ -48,8 +48,7 @@ CellularStateMachine::CellularStateMachine(CellularDevice &device, events::Event
     _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),
     _retry_count(0), _event_timeout(-1), _event_id(-1), _plmn(0), _command_success(false),
-    _plmn_network_found(false), _is_retry(false), _cb_data(), _current_event(NSAPI_EVENT_CONNECTION_STATUS_CHANGE),
-    _status(0)
+    _is_retry(false), _cb_data(), _current_event(NSAPI_EVENT_CONNECTION_STATUS_CHANGE), _status(0)
 {
 #if MBED_CONF_CELLULAR_RANDOM_MAX_START_DELAY == 0
     _start_time = 0;
@@ -83,7 +82,6 @@ void CellularStateMachine::reset()
     _state = STATE_INIT;
     _event_timeout = -1;
     _event_id = -1;
-    _plmn_network_found = false;
     _is_retry = false;
     _status = 0;
     _target_state = STATE_INIT;
@@ -161,15 +159,30 @@ bool CellularStateMachine::open_sim()
         }
     }
 
-    return state == CellularDevice::SimStateReady;
+    bool sim_ready = state == CellularDevice::SimStateReady;
+
+    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);
+            tr_debug("STM: manual set_registration: %d, plmn: %s", _cb_data.error, _plmn);
+            if (_cb_data.error) {
+                return false;
+            }
+        }
+    }
+
+    return sim_ready;
 }
 
 bool CellularStateMachine::is_registered()
 {
     CellularNetwork::RegistrationStatus status;
     bool is_registered = false;
 
-    for (int type = 0; type < CellularNetwork::C_MAX; type++) {
+    // accept only CGREG/CEREG. CREG is for circuit switch network changed. If we accept CREG attach will fail if also
+    // CGREG/CEREG is not registered.
+    for (int type = 0; type < CellularNetwork::C_REG; type++) {
         if (get_network_registration((CellularNetwork::RegistrationType) type, status, is_registered)) {
             if (is_registered) {
                 break;
@@ -178,6 +191,11 @@ bool CellularStateMachine::is_registered()
     }
 
     _cb_data.status_data = status;
+    // in manual registering we are forcing registration to certain network so we don't accept active context or attached
+    // as indication that device is registered to correct network.
+    if (_plmn && strlen(_plmn)) {
+        return is_registered;
+    }
     return is_registered || _status;
 }
 
@@ -249,61 +267,13 @@ void CellularStateMachine::report_failure(const char *msg)
 const char *CellularStateMachine::get_state_string(CellularState state) const
 {
 #if MBED_CONF_MBED_TRACE_ENABLE
-    static const char *strings[STATE_MAX_FSM_STATE] = { "Init", "Power", "Device ready", "SIM pin", "Registering network", "Manual registering", "Attaching network"};
+    static const char *strings[STATE_MAX_FSM_STATE] = { "Init", "Power", "Device ready", "SIM pin", "Registering network", "Attaching network"};
     return strings[state];
 #else
     return NULL;
 #endif // #if MBED_CONF_MBED_TRACE_ENABLE
 }
 
-bool CellularStateMachine::is_registered_to_plmn()
-{
-    int format;
-    CellularNetwork::operator_t op;
-
-    _cb_data.error = _network->get_operator_params(format, op);
-    if (_cb_data.error == NSAPI_ERROR_OK) {
-        if (format == 2) {
-            // great, numeric format we can do comparison for that
-            if (strcmp(op.op_num, _plmn) == 0) {
-                return true;
-            }
-            return false;
-        }
-
-        // format was alpha, get operator names to do the comparing
-        CellularNetwork::operator_names_list names_list;
-        _cb_data.error = _network->get_operator_names(names_list);
-        if (_cb_data.error == NSAPI_ERROR_OK) {
-            CellularNetwork::operator_names_t *op_names = names_list.get_head();
-            bool found_match = false;
-            while (op_names) {
-                if (format == 0) {
-                    if (strcmp(op.op_long, op_names->alpha) == 0) {
-                        found_match = true;
-                    }
-                } else if (format == 1) {
-                    if (strcmp(op.op_short, op_names->alpha) == 0) {
-                        found_match = true;
-                    }
-                }
-
-                if (found_match) {
-                    if (strcmp(_plmn, op_names->numeric)) {
-                        names_list.delete_all();
-                        return true;
-                    }
-                    names_list.delete_all();
-                    return false;
-                }
-            }
-        }
-        names_list.delete_all();
-    }
-
-    return false;
-}
-
 void CellularStateMachine::enter_to_state(CellularState state)
 {
     _next_state = state;
@@ -378,6 +348,7 @@ bool CellularStateMachine::device_ready()
         _event_status_cb((nsapi_event_t)CellularDeviceReady, (intptr_t)&_cb_data);
     }
     _cellularDevice.set_ready_cb(0);
+
     return true;
 }
 
@@ -410,16 +381,15 @@ void CellularStateMachine::state_sim_pin()
     _cellularDevice.set_timeout(TIMEOUT_SIM_PIN);
     tr_info("Setup SIM (timeout %d s)", TIMEOUT_SIM_PIN / 1000);
     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);
-            if (!_cb_data.error) {
+            if (!_cb_data.error && (type == CellularNetwork::C_EREG || type == CellularNetwork::C_GREG)) {
                 success = true;
             }
         }
         if (!success) {
-            tr_warn("Failed to set any URC's for registration");
+            tr_error("Failed to set CEREG/CGREG URC's for registration");
             retry_state_or_fail();
             return;
         }
@@ -428,16 +398,13 @@ void CellularStateMachine::state_sim_pin()
             tr_debug("Active context found.");
             _status |= ACTIVE_PDP_CONTEXT;
         }
-        CellularNetwork::AttachStatus status; // check if modem is already attached to a network
+        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) {
             _status |= ATTACHED_TO_NETWORK;
             tr_debug("Cellular already attached.");
         }
-        if (_plmn) {
-            enter_to_state(STATE_MANUAL_REGISTERING_NETWORK);
-        } else {
-            enter_to_state(STATE_REGISTERING_NETWORK);
-        }
+
+        enter_to_state(STATE_REGISTERING_NETWORK);
     } else {
         retry_state_or_fail();
     }
@@ -448,44 +415,25 @@ void CellularStateMachine::state_registering()
     _cellularDevice.set_timeout(TIMEOUT_NETWORK);
     tr_info("Network registration (timeout %d s)", TIMEOUT_REGISTRATION / 1000);
     if (is_registered()) {
-        _cb_data.status_data = CellularNetwork::AlreadyRegistered;
+        if (_cb_data.status_data != CellularNetwork::RegisteredHomeNetwork &&
+                _cb_data.status_data != CellularNetwork::RegisteredRoaming && _status) {
+            // there was already activated context or attached to network, and registration status is not registered, set to already registered.
+            _cb_data.status_data = CellularNetwork::AlreadyRegistered;
+        }
         _cb_data.error = NSAPI_ERROR_OK;
         _event_status_cb(_current_event, (intptr_t)&_cb_data);
         // we are already registered, go to attach
         enter_to_state(STATE_ATTACHING_NETWORK);
     } else {
         _cellularDevice.set_timeout(TIMEOUT_REGISTRATION);
-        if (!_command_success) {
-            _cb_data.error = _network->set_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);
         }
         retry_state_or_fail();
     }
 }
 
-// only used when _plmn is set
-void CellularStateMachine::state_manual_registering_network()
-{
-    _cellularDevice.set_timeout(TIMEOUT_REGISTRATION);
-    tr_info("Manual registration %s (timeout %d s)", _plmn, TIMEOUT_REGISTRATION / 1000);
-    if (!_plmn_network_found) {
-        if (is_registered() && is_registered_to_plmn()) {
-            // we have to send registration changed event as network thinks that we are not registered even we have active PDP context
-            _cb_data.status_data = CellularNetwork::AlreadyRegistered;
-            _cb_data.error = NSAPI_ERROR_OK;
-            _event_status_cb(_current_event, (intptr_t)&_cb_data);
-            _plmn_network_found = true;
-            enter_to_state(STATE_ATTACHING_NETWORK);
-        } else {
-            if (!_command_success) {
-                _cb_data.error = _network->set_registration(_plmn);
-                _command_success = (_cb_data.error == NSAPI_ERROR_OK);
-            }
-            retry_state_or_fail();
-        }
-    }
-}
-
 void CellularStateMachine::state_attaching()
 {
     _cellularDevice.set_timeout(TIMEOUT_CONNECT);
@@ -523,13 +471,8 @@ void CellularStateMachine::continue_from_state(CellularState state)
 nsapi_error_t CellularStateMachine::run_to_state(CellularStateMachine::CellularState state)
 {
     _mutex.lock();
-
-    CellularState tmp_state = state;
-    if (_plmn && tmp_state == STATE_REGISTERING_NETWORK) {
-        tmp_state = STATE_MANUAL_REGISTERING_NETWORK;
-    }
     // call pre_event via queue so that it's in same thread and it's safe to decisions
-    int id = _queue.call_in(0, this, &CellularStateMachine::pre_event, tmp_state);
+    int id = _queue.call_in(0, this, &CellularStateMachine::pre_event, state);
     if (!id) {
         report_failure("Failed to call queue.");
         stop();
@@ -620,10 +563,6 @@ void CellularStateMachine::event()
             _current_event = (nsapi_event_t)CellularRegistrationStatusChanged;
             state_registering();
             break;
-        case STATE_MANUAL_REGISTERING_NETWORK:
-            _current_event = (nsapi_event_t)CellularRegistrationStatusChanged;
-            state_manual_registering_network();
-            break;
         case STATE_ATTACHING_NETWORK:
             _current_event = (nsapi_event_t)CellularAttachNetwork;
             state_attaching();
@@ -694,30 +633,23 @@ bool CellularStateMachine::check_is_target_reached()
 void CellularStateMachine::cellular_event_changed(nsapi_event_t ev, intptr_t ptr)
 {
     cell_callback_data_t *data = (cell_callback_data_t *)ptr;
-    if ((cellular_connection_status_t)ev == CellularRegistrationStatusChanged &&
-            (_state == STATE_REGISTERING_NETWORK || _state == STATE_MANUAL_REGISTERING_NETWORK)) {
+    if ((cellular_connection_status_t)ev == CellularRegistrationStatusChanged && _state == STATE_REGISTERING_NETWORK) {
         // expect packet data so only these states are valid
-        if ((data->status_data == CellularNetwork::RegisteredHomeNetwork || data->status_data == CellularNetwork::RegisteredRoaming) && data->error == NSAPI_ERROR_OK) {
-            if (_plmn) {
-                if (is_registered_to_plmn()) {
-                    if (!_plmn_network_found) {
-                        _plmn_network_found = true;
-                        _queue.cancel(_event_id);
-                        _is_retry = false;
-                        _event_id = -1;
-                        if (!check_is_target_reached()) {
-                            continue_from_state(STATE_ATTACHING_NETWORK);
-                        }
-                    }
-                }
-            } else {
+        CellularNetwork::registration_params_t 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 ||
+                    data->status_data == CellularNetwork::RegisteredRoaming) && data->error == NSAPI_ERROR_OK) {
                 _queue.cancel(_event_id);
                 _is_retry = false;
                 _event_id = -1;
                 if (!check_is_target_reached()) {
                     continue_from_state(STATE_ATTACHING_NETWORK);
                 }
             }
+        } else {
+            tr_debug("creg event, discard...");
         }
     }
 }
diff --git a/features/cellular/framework/device/CellularStateMachine.h b/features/cellular/framework/device/CellularStateMachine.h
@@ -58,7 +58,6 @@ class CellularStateMachine {
         STATE_DEVICE_READY,
         STATE_SIM_PIN,
         STATE_REGISTERING_NETWORK,
-        STATE_MANUAL_REGISTERING_NETWORK,
         STATE_ATTACHING_NETWORK,
         STATE_MAX_FSM_STATE
     };
@@ -146,12 +145,10 @@ class CellularStateMachine {
     void state_device_ready();
     void state_sim_pin();
     void state_registering();
-    void state_manual_registering_network();
     void state_attaching();
     void enter_to_state(CellularState state);
     void retry_state_or_fail();
     void continue_from_state(CellularState state);
-    bool is_registered_to_plmn();
     void report_failure(const char *msg);
     void event();
     void device_ready_cb();
@@ -179,7 +176,6 @@ class CellularStateMachine {
     int _event_id;
     const char *_plmn;
     bool _command_success;
-    bool _plmn_network_found;
     bool _is_retry;
     cell_callback_data_t _cb_data;
     nsapi_event_t _current_event;

Original file line number	Diff line number	Diff line change
`@@ -171,7 +171,9 @@ void AT_CellularNetwork::read_reg_params_and_compare(RegistrationType type)`
`171`	`171`	`reg_params._status == RegisteredRoaming)) {`
`172`	`172`	`if (previous_registration_status == RegisteredHomeNetwork \|\|`
`173`	`173`	`previous_registration_status == RegisteredRoaming) {`
`174`		`- call_network_cb(NSAPI_STATUS_DISCONNECTED);`
	`174`	`+ if (type != C_REG) {// we are interested only if we drop from packet network`
	`175`	`+ _connection_status_cb(NSAPI_EVENT_CONNECTION_STATUS_CHANGE, NSAPI_STATUS_DISCONNECTED);`
	`176`	`+ }`
`175`	`177`	`}`
`176`	`178`	`}`
`177`	`179`	`}`
`@@ -268,6 +270,9 @@ nsapi_error_t AT_CellularNetwork::set_registration(const char *plmn)`
`268`	`270`	`tr_debug("Manual network registration to %s", plmn);`
`269`	`271`	`_at.cmd_start("AT+COPS=1,2,");`
`270`	`272`	`_at.write_string(plmn);`
	`273`	`+ if (_op_act != RAT_UNKNOWN) {`
	`274`	`+ _at.write_int(_op_act);`
	`275`	`+ }`
`271`	`276`	`_at.cmd_stop_read_resp();`
`272`	`277`	`}`
`273`	`278`