Skip to content

Commit a59ba20

Browse files
0xc01700xc0170
authored
Merge pull request #10338 from jarvte/fix_deleting_of_stm
Cellular: Fix deleting of state machine to correct class
2 parents 47838ea + fe5f978 commit a59ba20

File tree

6 files changed

+39
-54
lines changed

6 files changed

+39
-54
lines changed

‎UNITTESTS/features/cellular/framework/device/cellularstatemachine/cellularstatemachinetest.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,9 @@ class UT_CellularStateMachine {
7373
_state_machine = NULL;
7474
}
7575

76-
CellularStateMachine *create_state_machine(CellularDevice &device, events::EventQueue &queue)
76+
CellularStateMachine *create_state_machine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw)
7777
{
78-
_state_machine = new CellularStateMachine(device, queue);
78+
_state_machine = new CellularStateMachine(device, queue, nw);
7979
return _state_machine;
8080
}
8181

@@ -171,7 +171,7 @@ TEST_F(TestCellularStateMachine, test_create_delete)
171171
CellularDevice *dev = new myCellularDevice(&fh1);
172172
EXPECT_TRUE(dev);
173173

174-
CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
174+
CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
175175
EXPECT_TRUE(stm);
176176
ut.delete_state_machine();
177177

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

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

@@ -215,14 +215,14 @@ TEST_F(TestCellularStateMachine, test_start_dispatch)
215215
CellularDevice *dev = new myCellularDevice(&fh1);
216216
EXPECT_TRUE(dev);
217217

218-
CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue());
218+
CellularStateMachine *stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
219219
EXPECT_TRUE(stm);
220220
nsapi_error_t err = ut.start_dispatch();
221221
ASSERT_EQ(NSAPI_ERROR_OK, err);
222222
ut.delete_state_machine();
223223

224224
Thread_stub::osStatus_value = osErrorNoMemory;
225-
stm = ut.create_state_machine(*dev, *dev->get_queue());
225+
stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
226226
EXPECT_TRUE(stm);
227227
err = ut.start_dispatch();
228228
ASSERT_EQ(NSAPI_ERROR_NO_MEMORY, err);
@@ -240,21 +240,21 @@ TEST_F(TestCellularStateMachine, test_stop)
240240
CellularDevice *dev = new AT_CellularDevice(&fh1);
241241
EXPECT_TRUE(dev);
242242

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

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

249-
stm = ut.create_state_machine(*dev, *dev->get_queue());
249+
stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
250250
EXPECT_TRUE(stm);
251251
nsapi_error_t err = ut.start_dispatch();
252252
ASSERT_EQ(NSAPI_ERROR_OK, err);
253253

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

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

273-
stm = ut.create_state_machine(*dev, *dev->get_queue());
273+
stm = ut.create_state_machine(*dev, *dev->get_queue(), *dev->open_network());
274274
EXPECT_TRUE(stm);
275275
err = ut.start_dispatch();
276276
ASSERT_EQ(NSAPI_ERROR_OK, err);
@@ -294,7 +294,7 @@ TEST_F(TestCellularStateMachine, test_run_to_state)
294294
CellularDevice *dev = new AT_CellularDevice(&fh1);
295295
EXPECT_TRUE(dev);
296296

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

300300
nsapi_error_t err = ut.start_dispatch();

‎UNITTESTS/stubs/CellularStateMachine_stub.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ CellularStubState CellularStateMachine_stub::get_current_target_state = STATE_IN
2525
CellularStubState CellularStateMachine_stub::get_current_current_state = STATE_INIT;
2626
bool CellularStateMachine_stub::bool_value = false;
2727

28-
CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue) :
29-
_cellularDevice(device), _queue(queue)
28+
CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw) :
29+
_cellularDevice(device), _network(nw), _queue(queue)
3030
{
3131
}
3232

‎features/cellular/framework/AT/AT_CellularDevice.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ AT_CellularDevice::AT_CellularDevice(FileHandle *fh) : CellularDevice(fh), _netw
4747

4848
AT_CellularDevice::~AT_CellularDevice()
4949
{
50-
delete _state_machine;
51-
5250
// make sure that all is deleted even if somewhere close was not called and reference counting is messed up.
5351
_network_ref_count = 1;
5452
_sms_ref_count = 1;

‎features/cellular/framework/device/CellularDevice.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ CellularDevice::CellularDevice(FileHandle *fh) : _network_ref_count(0), _sms_ref
4444
CellularDevice::~CellularDevice()
4545
{
4646
tr_debug("CellularDevice destruct");
47+
delete _state_machine;
4748
}
4849

4950
void CellularDevice::stop()
@@ -118,7 +119,10 @@ nsapi_error_t CellularDevice::create_state_machine()
118119
{
119120
nsapi_error_t err = NSAPI_ERROR_OK;
120121
if (!_state_machine) {
121-
_state_machine = new CellularStateMachine(*this, *get_queue());
122+
_nw = open_network(_fh);
123+
// Attach to network so we can get update status from the network
124+
_nw->attach(callback(this, &CellularDevice::stm_callback));
125+
_state_machine = new CellularStateMachine(*this, *get_queue(), *_nw);
122126
_state_machine->set_cellular_callback(callback(this, &CellularDevice::stm_callback));
123127
err = _state_machine->start_dispatch();
124128
if (err) {
@@ -184,13 +188,6 @@ void CellularDevice::cellular_callback(nsapi_event_t ev, intptr_t ptr, CellularC
184188
// broadcast only network registration changes to state machine
185189
_state_machine->cellular_event_changed(ev, ptr);
186190
}
187-
if (cell_ev == CellularDeviceReady && ptr_data->error == NSAPI_ERROR_OK) {
188-
// Here we can create mux and give new filehandles as mux reserves the one what was in use.
189-
// if mux we would need to set new filehandle:_state_machine->set_filehandle( get fh from mux);
190-
_nw = open_network(_fh);
191-
// Attach to network so we can get update status from the network
192-
_nw->attach(callback(this, &CellularDevice::stm_callback));
193-
}
194191
} else {
195192
tr_debug("callback: %d, ptr: %d", ev, ptr);
196193
if (ev == NSAPI_EVENT_CONNECTION_STATUS_CHANGE && ptr == NSAPI_STATUS_DISCONNECTED) {

‎features/cellular/framework/device/CellularStateMachine.cpp

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ const int DEVICE_READY = 0x04;
4343

4444
namespace mbed {
4545

46-
CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue) :
46+
CellularStateMachine::CellularStateMachine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw) :
4747
_cellularDevice(device), _state(STATE_INIT), _next_state(_state), _target_state(_state),
48-
_event_status_cb(0), _network(0), _queue(queue), _queue_thread(0), _sim_pin(0),
48+
_event_status_cb(0), _network(nw), _queue(queue), _queue_thread(0), _sim_pin(0),
4949
_retry_count(0), _event_timeout(-1), _event_id(-1), _plmn(0), _command_success(false),
5050
_is_retry(false), _cb_data(), _current_event(NSAPI_EVENT_CONNECTION_STATUS_CHANGE), _status(0)
5151
{
@@ -99,11 +99,6 @@ void CellularStateMachine::stop()
9999

100100
reset();
101101
_event_id = STM_STOPPED;
102-
103-
if (_network) {
104-
_cellularDevice.close_network();
105-
_network = NULL;
106-
}
107102
}
108103

109104
bool CellularStateMachine::power_on()
@@ -163,7 +158,7 @@ bool CellularStateMachine::open_sim()
163158
if (sim_ready) {
164159
// If plmn is set, we should it right after sim is opened so that registration is forced to correct network.
165160
if (_plmn && strlen(_plmn)) {
166-
_cb_data.error = _network->set_registration(_plmn);
161+
_cb_data.error = _network.set_registration(_plmn);
167162
tr_debug("STM: manual set_registration: %d, plmn: %s", _cb_data.error, _plmn);
168163
if (_cb_data.error) {
169164
return false;
@@ -204,7 +199,7 @@ bool CellularStateMachine::get_network_registration(CellularNetwork::Registratio
204199
is_registered = false;
205200
bool is_roaming = false;
206201
CellularNetwork::registration_params_t reg_params;
207-
_cb_data.error = _network->get_registration_params(type, reg_params);
202+
_cb_data.error = _network.get_registration_params(type, reg_params);
208203

209204
if (_cb_data.error != NSAPI_ERROR_OK) {
210205
if (_cb_data.error != NSAPI_ERROR_UNSUPPORTED) {
@@ -329,14 +324,10 @@ bool CellularStateMachine::device_ready()
329324
{
330325
tr_info("Modem ready");
331326

332-
if (!_network) {
333-
_network = _cellularDevice.open_network();
334-
}
335-
336327
#ifdef MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY
337328
MBED_ASSERT(MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY >= CellularNetwork::RAT_GSM &&
338329
MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY < CellularNetwork::RAT_UNKNOWN);
339-
nsapi_error_t err = _network->set_access_technology((CellularNetwork::RadioAccessTechnology)MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY);
330+
nsapi_error_t err = _network.set_access_technology((CellularNetwork::RadioAccessTechnology)MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY);
340331
if (err != NSAPI_ERROR_OK && err != NSAPI_ERROR_UNSUPPORTED) {
341332
tr_warning("Failed to set access technology to %d", MBED_CONF_CELLULAR_RADIO_ACCESS_TECHNOLOGY);
342333
return false;
@@ -382,7 +373,7 @@ void CellularStateMachine::state_sim_pin()
382373
if (open_sim()) {
383374
bool success = false;
384375
for (int type = 0; type < CellularNetwork::C_MAX; type++) {
385-
_cb_data.error = _network->set_registration_urc((CellularNetwork::RegistrationType)type, true);
376+
_cb_data.error = _network.set_registration_urc((CellularNetwork::RegistrationType)type, true);
386377
if (!_cb_data.error && (type == CellularNetwork::C_EREG || type == CellularNetwork::C_GREG)) {
387378
success = true;
388379
}
@@ -393,19 +384,19 @@ void CellularStateMachine::state_sim_pin()
393384
return;
394385
}
395386

396-
if (_network->is_active_context()) { // check if context was already activated
387+
if (_network.is_active_context()) { // check if context was already activated
397388
tr_debug("Active context found.");
398389
_status |= ACTIVE_PDP_CONTEXT;
399390
}
400391
CellularNetwork::AttachStatus status = CellularNetwork::Detached; // check if modem is already attached to a network
401-
if (_network->get_attach(status) == NSAPI_ERROR_OK && status == CellularNetwork::Attached) {
392+
if (_network.get_attach(status) == NSAPI_ERROR_OK && status == CellularNetwork::Attached) {
402393
_status |= ATTACHED_TO_NETWORK;
403394
tr_debug("Cellular already attached.");
404395
}
405396

406397
// if packet domain event reporting is not set it's not a stopper. We might lack some events when we are
407398
// dropped from the network.
408-
_cb_data.error = _network->set_packet_domain_event_reporting(true);
399+
_cb_data.error = _network.set_packet_domain_event_reporting(true);
409400
if (_cb_data.error == NSAPI_STATUS_ERROR_UNSUPPORTED) {
410401
tr_warning("Packet domain event reporting not supported!");
411402
} else if (_cb_data.error) {
@@ -434,7 +425,7 @@ void CellularStateMachine::state_registering()
434425
} else {
435426
_cellularDevice.set_timeout(TIMEOUT_REGISTRATION);
436427
if (!_command_success && !_plmn) { // don't call set_registration twice for manual registration
437-
_cb_data.error = _network->set_registration(_plmn);
428+
_cb_data.error = _network.set_registration(_plmn);
438429
_command_success = (_cb_data.error == NSAPI_ERROR_OK);
439430
}
440431
retry_state_or_fail();
@@ -446,7 +437,7 @@ void CellularStateMachine::state_attaching()
446437
_cellularDevice.set_timeout(TIMEOUT_CONNECT);
447438
tr_info("Attaching network (timeout %d s)", TIMEOUT_CONNECT / 1000);
448439
if (_status != ATTACHED_TO_NETWORK) {
449-
_cb_data.error = _network->set_attach();
440+
_cb_data.error = _network.set_attach();
450441
}
451442
if (_cb_data.error == NSAPI_ERROR_OK) {
452443
if (_event_status_cb) {
@@ -534,14 +525,12 @@ bool CellularStateMachine::get_current_status(CellularStateMachine::CellularStat
534525
void CellularStateMachine::event()
535526
{
536527
#if MBED_CONF_MBED_TRACE_ENABLE
537-
if (_network) {
538-
int rssi;
539-
if (_network->get_signal_quality(rssi) == NSAPI_ERROR_OK) {
540-
if (rssi == CellularNetwork::SignalQualityUnknown) {
541-
tr_info("RSSI unknown");
542-
} else {
543-
tr_info("RSSI %d dBm", rssi);
544-
}
528+
int rssi;
529+
if (_network.get_signal_quality(rssi) == NSAPI_ERROR_OK) {
530+
if (rssi == CellularNetwork::SignalQualityUnknown) {
531+
tr_info("RSSI unknown");
532+
} else {
533+
tr_info("RSSI %d dBm", rssi);
545534
}
546535
}
547536
#endif
@@ -643,7 +632,7 @@ void CellularStateMachine::cellular_event_changed(nsapi_event_t ev, intptr_t ptr
643632
if ((cellular_connection_status_t)ev == CellularRegistrationStatusChanged && _state == STATE_REGISTERING_NETWORK) {
644633
// expect packet data so only these states are valid
645634
CellularNetwork::registration_params_t reg_params;
646-
nsapi_error_t err = _network->get_registration_params(reg_params);
635+
nsapi_error_t err = _network.get_registration_params(reg_params);
647636

648637
if (err == NSAPI_ERROR_OK && (reg_params._type == CellularNetwork::C_EREG || reg_params._type == CellularNetwork::C_GREG)) {
649638
if ((data->status_data == CellularNetwork::RegisteredHomeNetwork ||

‎features/cellular/framework/device/CellularStateMachine.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ class CellularStateMachine {
4444
*
4545
* @param device reference to CellularDevice
4646
* @param queue reference to queue used in state transitions
47+
* @param nw reference to CellularNetwork
4748
*/
48-
CellularStateMachine(CellularDevice &device, events::EventQueue &queue);
49+
CellularStateMachine(CellularDevice &device, events::EventQueue &queue, CellularNetwork &nw);
4950
~CellularStateMachine();
5051

5152
/** Cellular connection states
@@ -161,7 +162,7 @@ class CellularStateMachine {
161162

162163
Callback<void(nsapi_event_t, intptr_t)> _event_status_cb;
163164

164-
CellularNetwork *_network;
165+
CellularNetwork &_network;
165166
events::EventQueue &_queue;
166167
rtos::Thread *_queue_thread;
167168

0 commit comments

Comments
 (0)