diff --git a/UNITTESTS/features/cellular/framework/AT/at_cellularnetwork/at_cellularnetworktest.cpp b/UNITTESTS/features/cellular/framework/AT/at_cellularnetwork/at_cellularnetworktest.cpp index d79bf9e5e96..f755e557caa 100644 --- a/UNITTESTS/features/cellular/framework/AT/at_cellularnetwork/at_cellularnetworktest.cpp +++ b/UNITTESTS/features/cellular/framework/AT/at_cellularnetwork/at_cellularnetworktest.cpp @@ -653,20 +653,6 @@ TEST_F(TestAT_CellularNetwork, test_AT_CellularNetwork_attach) cn.attach(&network_cb); } -TEST_F(TestAT_CellularNetwork, test_get_connection_status) -{ - EventQueue que; - FileHandle_stub fh1; - ATHandler at(&fh1, que, 0, ","); - - AT_CellularNetwork cn(at); - - ATHandler_stub::nsapi_error_value = NSAPI_ERROR_OK; - network_cb_count = 0; - cn.attach(&network_cb); - EXPECT_TRUE(NSAPI_STATUS_DISCONNECTED == cn.get_connection_status()); -} - TEST_F(TestAT_CellularNetwork, test_AT_CellularNetwork_set_receive_period) { EventQueue que; diff --git a/UNITTESTS/stubs/AT_CellularNetwork_stub.cpp b/UNITTESTS/stubs/AT_CellularNetwork_stub.cpp index 899be2fb4c0..136539d8309 100644 --- a/UNITTESTS/stubs/AT_CellularNetwork_stub.cpp +++ b/UNITTESTS/stubs/AT_CellularNetwork_stub.cpp @@ -44,11 +44,6 @@ void AT_CellularNetwork::attach(Callback status_c { } -nsapi_connection_status_t AT_CellularNetwork::get_connection_status() const -{ - return NSAPI_STATUS_LOCAL_UP; -} - nsapi_error_t AT_CellularNetwork::set_registration_urc(RegistrationType type, bool urc_on) { if (AT_CellularNetwork_stub::set_registration_urc_fail_counter) { diff --git a/UNITTESTS/stubs/CellularDevice_stub.cpp b/UNITTESTS/stubs/CellularDevice_stub.cpp index 7309837f068..e53e0d5873a 100644 --- a/UNITTESTS/stubs/CellularDevice_stub.cpp +++ b/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) { } diff --git a/features/cellular/framework/API/CellularContext.h b/features/cellular/framework/API/CellularContext.h index 6779e7fdee0..590b9aada78 100644 --- a/features/cellular/framework/API/CellularContext.h +++ b/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. */ diff --git a/features/cellular/framework/API/CellularDevice.h b/features/cellular/framework/API/CellularDevice.h index a67d226cee8..dd5d7e82066 100644 --- a/features/cellular/framework/API/CellularDevice.h +++ b/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; diff --git a/features/cellular/framework/API/CellularNetwork.h b/features/cellular/framework/API/CellularNetwork.h index 0eb87366f4f..31a9ea8e0b8 100644 --- a/features/cellular/framework/API/CellularNetwork.h +++ b/features/cellular/framework/API/CellularNetwork.h @@ -319,16 +319,14 @@ 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 status_cb) = 0; - /** Get the connection status - * - * @return The connection status according to ConnectionStatusType - */ - virtual nsapi_connection_status_t get_connection_status() const = 0; - /** Read operator names * * @param op_names on successful return contains linked list of operator names. diff --git a/features/cellular/framework/AT/ATHandler.cpp b/features/cellular/framework/AT/ATHandler.cpp index 7ea09476556..7282eb19d33 100644 --- a/features/cellular/framework/AT/ATHandler.cpp +++ b/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; } diff --git a/features/cellular/framework/AT/AT_CellularContext.cpp b/features/cellular/framework/AT/AT_CellularContext.cpp index 1188979c1ce..46b637925ec 100644 --- a/features/cellular/framework/AT/AT_CellularContext.cpp +++ b/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,20 +940,24 @@ 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(); } } @@ -949,6 +965,7 @@ void AT_CellularContext::cellular_callback(nsapi_event_t ev, intptr_t ptr) // 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); } diff --git a/features/cellular/framework/AT/AT_CellularNetwork.cpp b/features/cellular/framework/AT/AT_CellularNetwork.cpp index 538fb27f81f..23b88ab9920 100644 --- a/features/cellular/framework/AT/AT_CellularNetwork.cpp +++ b/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); } } @@ -213,11 +210,6 @@ void AT_CellularNetwork::attach(Callback status_c _connection_status_cb = status_cb; } -nsapi_connection_status_t AT_CellularNetwork::get_connection_status() const -{ - return _connect_status; -} - nsapi_error_t AT_CellularNetwork::set_registration_urc(RegistrationType type, bool urc_on) { int index = (int)type; diff --git a/features/cellular/framework/AT/AT_CellularNetwork.h b/features/cellular/framework/AT/AT_CellularNetwork.h index 044bc50c0de..0bc7566e217 100644 --- a/features/cellular/framework/AT/AT_CellularNetwork.h +++ b/features/cellular/framework/AT/AT_CellularNetwork.h @@ -63,8 +63,6 @@ class AT_CellularNetwork : public CellularNetwork, public AT_CellularBase { virtual void attach(Callback status_cb); - virtual nsapi_connection_status_t get_connection_status() const; - virtual nsapi_error_t set_access_technology(RadioAccessTechnology rat); virtual nsapi_error_t scan_plmn(operList_t &operators, int &ops_count); diff --git a/features/cellular/framework/device/CellularDevice.cpp b/features/cellular/framework/device/CellularDevice.cpp index 8cc63e544c6..751c040b60c 100644 --- a/features/cellular/framework/device/CellularDevice.cpp +++ b/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 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,28 +178,23 @@ 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(); + } } } } @@ -195,11 +202,18 @@ void CellularDevice::cellular_callback(nsapi_event_t ev, intptr_t ptr) // 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); } diff --git a/features/cellular/framework/device/CellularStateMachine.cpp b/features/cellular/framework/device/CellularStateMachine.cpp index 58deb6c1442..f4ad2bef65e 100644 --- a/features/cellular/framework/device/CellularStateMachine.cpp +++ b/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;