Skip to content

Cellular: Fix deleting of state machine to correct class #10338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2019
Merged

Cellular: Fix deleting of state machine to correct class #10338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions ...ESTS/features/cellular/framework/device/cellularstatemachine/cellularstatemachinetest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ class UT_CellularStateMachine {
_state_machine = NULL;
}

CellularStateMachine *create_state_machine(CellularDevice &device, events::EventQueue &queue)
CellularStateMachine *create_state_machine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw)
{
_state_machine = new CellularStateMachine(device, queue);
_state_machine = new CellularStateMachine(device, queue, nw);
return _state_machine;
}

Expand Down Expand Up @@ -171,7 +171,7 @@ TEST_F(TestCellularStateMachine, test_create_delete)
CellularDevice *dev = new myCellularDevice(&fh1);
EXPECT_TRUE(dev);

CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
EXPECT_TRUE(stm);
ut.delete_state_machine();

Expand All @@ -187,7 +187,7 @@ TEST_F(TestCellularStateMachine, test_setters)
CellularDevice *dev = new myCellularDevice(&fh1);
EXPECT_TRUE(dev);

CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
EXPECT_TRUE(stm);
ut.set_cellular_callback(&cellular_callback);

Expand Down Expand Up @@ -215,14 +215,14 @@ TEST_F(TestCellularStateMachine, test_start_dispatch)
CellularDevice *dev = new myCellularDevice(&fh1);
EXPECT_TRUE(dev);

CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
EXPECT_TRUE(stm);
nsapi_error_t err = ut.start_dispatch();
ASSERT_EQ(NSAPI_ERROR_OK, err);
ut.delete_state_machine();

Thread_stub::osStatus_value = osErrorNoMemory;
stm = ut.create_state_machine(*dev, *dev->get_queue());
stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
EXPECT_TRUE(stm);
err = ut.start_dispatch();
ASSERT_EQ(NSAPI_ERROR_NO_MEMORY, err);
Expand All @@ -240,21 +240,21 @@ TEST_F(TestCellularStateMachine, test_stop)
CellularDevice *dev = new AT_CellularDevice(&fh1);
EXPECT_TRUE(dev);

CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
EXPECT_TRUE(stm);

ut.stop(); // nothing created, run through
ut.delete_state_machine();

stm = ut.create_state_machine(*dev, *dev->get_queue());
stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
EXPECT_TRUE(stm);
nsapi_error_t err = ut.start_dispatch();
ASSERT_EQ(NSAPI_ERROR_OK, err);

ut.stop(); // thread is created, now stop will delete it
ut.delete_state_machine();

stm = ut.create_state_machine(*dev, *dev->get_queue());
stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
EXPECT_TRUE(stm);
err = ut.start_dispatch();
ASSERT_EQ(NSAPI_ERROR_OK, err);
Expand All @@ -270,7 +270,7 @@ TEST_F(TestCellularStateMachine, test_stop)
ut.stop(); // thread and power are created, now stop will delete them
ut.delete_state_machine();

stm = ut.create_state_machine(*dev, *dev->get_queue());
stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
EXPECT_TRUE(stm);
err = ut.start_dispatch();
ASSERT_EQ(NSAPI_ERROR_OK, err);
Expand All @@ -294,7 +294,7 @@ TEST_F(TestCellularStateMachine, test_run_to_state)
CellularDevice *dev = new AT_CellularDevice(&fh1);
EXPECT_TRUE(dev);

CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
EXPECT_TRUE(stm);

nsapi_error_t err = ut.start_dispatch();
Expand Down
4 changes: 2 additions & 2 deletions UNITTESTS/stubs/CellularStateMachine_stub.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ CellularStubState CellularStateMachine_stub::get_current_target_state = STATE_IN
CellularStubState CellularStateMachine_stub::get_current_current_state = STATE_INIT;
bool CellularStateMachine_stub::bool_value = false;

CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue) :
_cellularDevice(device), _queue(queue)
CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw) :
_cellularDevice(device), _network(nw), _queue(queue)
{
}

Expand Down
2 changes: 0 additions & 2 deletions features/cellular/framework/AT/AT_CellularDevice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ AT_CellularDevice::AT_CellularDevice(FileHandle *fh) : CellularDevice(fh), _netw

AT_CellularDevice::~AT_CellularDevice()
{
delete _state_machine;

// make sure that all is deleted even if somewhere close was not called and reference counting is messed up.
_network_ref_count = 1;
_sms_ref_count = 1;
Expand Down
13 changes: 5 additions & 8 deletions features/cellular/framework/device/CellularDevice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ CellularDevice::CellularDevice(FileHandle *fh) : _network_ref_count(0), _sms_ref
CellularDevice::~CellularDevice()
{
tr_debug("CellularDevice destruct");
delete _state_machine;
}

void CellularDevice::stop()
Expand Down Expand Up @@ -118,7 +119,10 @@ nsapi_error_t CellularDevice::create_state_machine()
{
nsapi_error_t err = NSAPI_ERROR_OK;
if (!_state_machine) {
_state_machine = new CellularStateMachine(*this, *get_queue());
_nw = open_network(_fh);
// Attach to network so we can get update status from the network
_nw->attach(callback(this, &CellularDevice::stm_callback));
_state_machine = new CellularStateMachine(*this, *get_queue(), *_nw);
_state_machine->set_cellular_callback(callback(this, &CellularDevice::stm_callback));
err = _state_machine->start_dispatch();
if (err) {
Expand Down Expand Up @@ -184,13 +188,6 @@ void CellularDevice::cellular_callback(nsapi_event_t ev, intptr_t ptr, CellularC
// 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::stm_callback));
}
} else {
tr_debug("callback: %d, ptr: %d", ev, ptr);
if (ev == NSAPI_EVENT_CONNECTION_STATUS_CHANGE && ptr == NSAPI_STATUS_DISCONNECTED) {
Expand Down
47 changes: 18 additions & 29 deletions features/cellular/framework/device/CellularStateMachine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ const int DEVICE_READY = 0x04;

namespace mbed {

CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue) :
CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw) :
_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),
_event_status_cb(0), _network(nw), _queue(queue), _queue_thread(0), _sim_pin(0),
_retry_count(0), _event_timeout(-1), _event_id(-1), _plmn(0), _command_success(false),
_is_retry(false), _cb_data(), _current_event(NSAPI_EVENT_CONNECTION_STATUS_CHANGE), _status(0)
{
Expand Down Expand Up @@ -99,11 +99,6 @@ void CellularStateMachine::stop()

reset();
_event_id = STM_STOPPED;

if (_network) {
_cellularDevice.close_network();
_network = NULL;
}
}

bool CellularStateMachine::power_on()
Expand Down Expand Up @@ -163,7 +158,7 @@ bool CellularStateMachine::open_sim()
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);
_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;
Expand Down Expand Up @@ -204,7 +199,7 @@ bool CellularStateMachine::get_network_registration(CellularNetwork::Registratio
is_registered = false;
bool is_roaming = false;
CellularNetwork::registration_params_t reg_params;
_cb_data.error = _network->get_registration_params(type, reg_params);
_cb_data.error = _network.get_registration_params(type, reg_params);

if (_cb_data.error != NSAPI_ERROR_OK) {
if (_cb_data.error != NSAPI_ERROR_UNSUPPORTED) {
Expand Down Expand Up @@ -329,14 +324,10 @@ bool CellularStateMachine::device_ready()
{
tr_info("Modem ready");

if (!_network) {
_network = _cellularDevice.open_network();
}

#ifdef MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY
MBED_ASSERT(MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY >= CellularNetwork::RAT_GSM &&
MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY < CellularNetwork::RAT_UNKNOWN);
nsapi_error_t err = _network->set_access_technology((CellularNetwork::RadioAccessTechnology)MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY);
nsapi_error_t err = _network.set_access_technology((CellularNetwork::RadioAccessTechnology)MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY);
if (err != NSAPI_ERROR_OK && err != NSAPI_ERROR_UNSUPPORTED) {
tr_warning("Failed to set access technology to %d", MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY);
return false;
Expand Down Expand Up @@ -382,7 +373,7 @@ void CellularStateMachine::state_sim_pin()
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);
_cb_data.error = _network.set_registration_urc((CellularNetwork::RegistrationType)type, true);
if (!_cb_data.error && (type == CellularNetwork::C_EREG || type == CellularNetwork::C_GREG)) {
success = true;
}
Expand All @@ -393,19 +384,19 @@ void CellularStateMachine::state_sim_pin()
return;
}

if (_network->is_active_context()) { // check if context was already activated
if (_network.is_active_context()) { // check if context was already activated
tr_debug("Active context found.");
_status |= ACTIVE_PDP_CONTEXT;
}
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) {
if (_network.get_attach(status) == NSAPI_ERROR_OK && status == CellularNetwork::Attached) {
_status |= ATTACHED_TO_NETWORK;
tr_debug("Cellular already attached.");
}

// if packet domain event reporting is not set it's not a stopper. We might lack some events when we are
// dropped from the network.
_cb_data.error = _network->set_packet_domain_event_reporting(true);
_cb_data.error = _network.set_packet_domain_event_reporting(true);
if (_cb_data.error == NSAPI_STATUS_ERROR_UNSUPPORTED) {
tr_warning("Packet domain event reporting not supported!");
} else if (_cb_data.error) {
Expand Down Expand Up @@ -434,7 +425,7 @@ void CellularStateMachine::state_registering()
} else {
_cellularDevice.set_timeout(TIMEOUT_REGISTRATION);
if (!_command_success && !_plmn) { // don't call set_registration twice for manual registration
_cb_data.error = _network->set_registration(_plmn);
_cb_data.error = _network.set_registration(_plmn);
_command_success = (_cb_data.error == NSAPI_ERROR_OK);
}
retry_state_or_fail();
Expand All @@ -446,7 +437,7 @@ void CellularStateMachine::state_attaching()
_cellularDevice.set_timeout(TIMEOUT_CONNECT);
tr_info("Attaching network (timeout %d s)", TIMEOUT_CONNECT / 1000);
if (_status != ATTACHED_TO_NETWORK) {
_cb_data.error = _network->set_attach();
_cb_data.error = _network.set_attach();
}
if (_cb_data.error == NSAPI_ERROR_OK) {
if (_event_status_cb) {
Expand Down Expand Up @@ -534,14 +525,12 @@ bool CellularStateMachine::get_current_status(CellularStateMachine::CellularStat
void CellularStateMachine::event()
{
#if MBED_CONF_MBED_TRACE_ENABLE
if (_network) {
int rssi;
if (_network->get_signal_quality(rssi) == NSAPI_ERROR_OK) {
if (rssi == CellularNetwork::SignalQualityUnknown) {
tr_info("RSSI unknown");
} else {
tr_info("RSSI %d dBm", rssi);
}
int rssi;
if (_network.get_signal_quality(rssi) == NSAPI_ERROR_OK) {
if (rssi == CellularNetwork::SignalQualityUnknown) {
tr_info("RSSI unknown");
} else {
tr_info("RSSI %d dBm", rssi);
}
}
#endif
Expand Down Expand Up @@ -643,7 +632,7 @@ void CellularStateMachine::cellular_event_changed(nsapi_event_t ev, intptr_t ptr
if ((cellular_connection_status_t)ev == CellularRegistrationStatusChanged && _state == STATE_REGISTERING_NETWORK) {
// expect packet data so only these states are valid
CellularNetwork::registration_params_t reg_params;
nsapi_error_t err = _network->get_registration_params(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 ||
Expand Down
5 changes: 3 additions & 2 deletions features/cellular/framework/device/CellularStateMachine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ class CellularStateMachine {
*
* @param device reference to CellularDevice
* @param queue reference to queue used in state transitions
* @param nw reference to CellularNetwork
*/
CellularStateMachine(CellularDevice &device, events::EventQueue &queue);
CellularStateMachine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw);
~CellularStateMachine();

/** Cellular connection states
Expand Down Expand Up @@ -161,7 +162,7 @@ class CellularStateMachine {

Callback<void(nsapi_event_t, intptr_t)> _event_status_cb;

CellularNetwork *_network;
CellularNetwork &_network;
events::EventQueue &_queue;
rtos::Thread *_queue_thread;

Expand Down