diff --git a/UNITTESTS/features/cellular/framework/device/cellularstatemachine/cellularstatemachinetest.cpp b/UNITTESTS/features/cellular/framework/device/cellularstatemachine/cellularstatemachinetest.cpp index 5466b72ae6..b96a20f4be 100644 --- 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 index 4d0591e963..b82438ca96 100644 --- 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 index f4ad2bef65..ad64585c20 100644 --- 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,7 +159,20 @@ 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() @@ -169,7 +180,9 @@ 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,23 +633,14 @@ 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; @@ -718,6 +648,8 @@ void CellularStateMachine::cellular_event_changed(nsapi_event_t ev, intptr_t ptr 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 index a2c815a6c5..f0efa16454 100644 --- a/features/cellular/framework/device/CellularStateMachine.h +++ b/features/cellular/framework/device/CellularStateMachine.h @@ -58,7 +58,6 @@ private: STATE_DEVICE_READY, STATE_SIM_PIN, STATE_REGISTERING_NETWORK, - STATE_MANUAL_REGISTERING_NETWORK, STATE_ATTACHING_NETWORK, STATE_MAX_FSM_STATE }; @@ -146,12 +145,10 @@ private: 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 @@ private: 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;