Skip to content

Commit bd21c0e

Browse files
0xc01700xc0170
authored
Merge pull request #10639 from AriParkkila/sock_id_debacle
Cellular: Preventing Socket ID assignment until actual socket creation at the modem
2 parents b735d73 + 9e72fa2 commit bd21c0e

File tree

8 files changed

+98
-44
lines changed

8 files changed

+98
-44
lines changed

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ int AT_CellularStack::find_socket_index(nsapi_socket_t handle)
5252
return -1;
5353
}
5454

55-
5655
/** NetworkStack
5756
*/
5857

@@ -131,10 +130,12 @@ nsapi_error_t AT_CellularStack::socket_open(nsapi_socket_t *handle, nsapi_protoc
131130

132131
tr_info("Socket %d open", index);
133132
// create local socket structure, socket on modem is created when app calls sendto/recvfrom
133+
// Do not assign a socket ID yet. Socket is not created at the Modem yet.
134+
// create_socket_impl(handle) will assign the correct socket ID.
134135
_socket[index] = new CellularSocket;
135136
CellularSocket *psock = _socket[index];
136137
SocketAddress addr(0, get_dynamic_ip_port());
137-
psock->id = index;
138+
138139
psock->localAddress = addr;
139140
psock->proto = proto;
140141
*handle = psock;
@@ -153,7 +154,6 @@ nsapi_error_t AT_CellularStack::socket_close(nsapi_socket_t handle)
153154
return err;
154155
}
155156
int sock_id = socket->id;
156-
bool sock_created = socket->created;
157157

158158
int index = find_socket_index(handle);
159159
if (index == -1) {
@@ -165,14 +165,14 @@ nsapi_error_t AT_CellularStack::socket_close(nsapi_socket_t handle)
165165

166166
// Close the socket on the modem if it was created
167167
_at.lock();
168-
if (sock_created) {
168+
if (sock_id > -1) {
169169
err = socket_close_impl(sock_id);
170170
}
171171

172172
if (!err) {
173173
tr_info("Socket %d closed", index);
174174
} else {
175-
tr_info("Socket %d close (id %d, created %d, started %d, error %d)", index, sock_id, socket->created, socket->started, err);
175+
tr_info("Socket %d close (id %d, started %d, error %d)", index, sock_id, socket->started, err);
176176
}
177177

178178
_socket[index] = NULL;
@@ -206,7 +206,7 @@ nsapi_error_t AT_CellularStack::socket_bind(nsapi_socket_t handle, const SocketA
206206
}
207207
}
208208

209-
if (!socket->created) {
209+
if (socket->id == -1) {
210210
create_socket_impl(socket);
211211
}
212212

@@ -266,7 +266,7 @@ nsapi_size_or_error_t AT_CellularStack::socket_sendto(nsapi_socket_t handle, con
266266

267267
nsapi_size_or_error_t ret_val = NSAPI_ERROR_OK;
268268

269-
if (!socket->created) {
269+
if (socket->id == -1) {
270270
_at.lock();
271271

272272
ret_val = create_socket_impl(socket);
@@ -317,7 +317,7 @@ nsapi_size_or_error_t AT_CellularStack::socket_recvfrom(nsapi_socket_t handle, S
317317

318318
nsapi_size_or_error_t ret_val = NSAPI_ERROR_OK;
319319

320-
if (!socket->created) {
320+
if (socket->id == -1) {
321321
_at.lock();
322322

323323
ret_val = create_socket_impl(socket);

‎features/cellular/framework/AT/AT_CellularStack.h‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ class AT_CellularStack : public NetworkStack, public AT_CellularBase {
9797
localAddress("", 0),
9898
_cb(NULL),
9999
_data(NULL),
100-
created(false),
101100
closed(false),
102101
started(false),
103102
tx_ready(false),
@@ -115,7 +114,6 @@ class AT_CellularStack : public NetworkStack, public AT_CellularBase {
115114
SocketAddress localAddress;
116115
void (*_cb)(void *);
117116
void *_data;
118-
bool created; // socket has been created on modem stack
119117
bool closed; // socket has been closed by a peer
120118
bool started; // socket has been opened on modem stack
121119
bool tx_ready; // socket is ready for sending on modem stack
@@ -176,12 +174,22 @@ class AT_CellularStack : public NetworkStack, public AT_CellularBase {
176174
void *buffer, nsapi_size_t size) = 0;
177175

178176
/**
179-
* Find the socket handle based on socket identifier
177+
* Find the socket handle based on the index of the socket construct
178+
* in the socket container. Please note that this index may or may not be
179+
* the socket id. The actual meaning of this index depends upon the modem
180+
* being used.
180181
*
181-
* @param sock_id Socket identifier
182+
* @param index Index of the socket construct in the container
182183
* @return Socket handle, NULL on error
183184
*/
184-
CellularSocket *find_socket(int sock_id);
185+
CellularSocket *find_socket(int index);
186+
187+
/**
188+
* Find the index of the given CellularSocket handle. This index may or may
189+
* not be the socket id. The actual meaning of this index depends upon the modem
190+
* being used.
191+
*/
192+
int find_socket_index(nsapi_socket_t handle);
185193

186194
// socket container
187195
CellularSocket **_socket;
@@ -199,7 +207,6 @@ class AT_CellularStack : public NetworkStack, public AT_CellularBase {
199207
nsapi_ip_stack_t _stack_type;
200208

201209
private:
202-
int find_socket_index(nsapi_socket_t handle);
203210

204211
int get_socket_index_by_port(uint16_t port);
205212

‎features/cellular/framework/targets/GEMALTO/CINTERION/GEMALTO_CINTERION_CellularStack.cpp‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ nsapi_error_t GEMALTO_CINTERION_CellularStack::socket_open_defer(CellularSocket
167167
int retry_open = 1;
168168
retry_open:
169169
// setup internet session profile
170-
int internet_service_id = socket->id;
170+
int internet_service_id = find_socket_index(socket);
171171
bool foundSrvType = false;
172172
bool foundConIdType = false;
173173
_at.cmd_start("AT^SISS?");
@@ -244,26 +244,26 @@ nsapi_error_t GEMALTO_CINTERION_CellularStack::socket_open_defer(CellularSocket
244244
}
245245

246246
_at.cmd_start("AT^SISS=");
247-
_at.write_int(socket->id);
247+
_at.write_int(internet_service_id);
248248
_at.write_string("address", false);
249249
_at.write_string(sock_addr);
250250
_at.cmd_stop_read_resp();
251251

252252
_at.cmd_start("AT^SISO=");
253-
_at.write_int(socket->id);
253+
_at.write_int(internet_service_id);
254254
_at.cmd_stop_read_resp();
255255

256256
if (_at.get_last_error()) {
257257
tr_error("Socket %d open failed!", socket->id);
258258
_at.clear_error();
259-
socket_close_impl(socket->id); // socket may already be open on modem if app and modem are not in sync, as a recovery, try to close the socket so open succeeds the next time
259+
socket_close_impl(internet_service_id); // socket may already be open on modem if app and modem are not in sync, as a recovery, try to close the socket so open succeeds the next time
260260
if (retry_open--) {
261261
goto retry_open;
262262
}
263263
return NSAPI_ERROR_NO_SOCKET;
264264
}
265265

266-
socket->created = true;
266+
socket->id = internet_service_id;
267267
tr_debug("Cinterion open %d (err %d)", socket->id, _at.get_last_error());
268268

269269
return _at.get_last_error();
@@ -316,11 +316,11 @@ nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_sendto_impl(Cellul
316316
socket->remoteAddress = address;
317317
_at.resp_start("^SISW:");
318318
int sock_id = _at.read_int();
319+
MBED_ASSERT(sock_id == socket->id);
319320
int urc_code = _at.read_int();
320321
tr_debug("TX ready: socket=%d, urc=%d (err=%d)", sock_id, urc_code, _at.get_last_error());
321322
(void)sock_id;
322323
(void)urc_code;
323-
socket->created = true;
324324
socket->started = true;
325325
socket->tx_ready = true;
326326
}
@@ -399,6 +399,10 @@ nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_sendto_impl(Cellul
399399
nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_recvfrom_impl(CellularSocket *socket, SocketAddress *address,
400400
void *buffer, nsapi_size_t size)
401401
{
402+
// AT_CellularStack::recvfrom(...) will make sure that we do have a socket
403+
// open on the modem, assert here to catch a programming error
404+
MBED_ASSERT(socket->id != -1);
405+
402406
// we must use this flag, otherwise ^SISR URC can come while we are reading response and there is
403407
// no way to detect if that is really an URC or response
404408
if (!socket->rx_avail) {

‎features/cellular/framework/targets/QUECTEL/BC95/QUECTEL_BC95_CellularStack.cpp‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ nsapi_error_t QUECTEL_BC95_CellularStack::socket_connect(nsapi_socket_t handle,
5151
CellularSocket *socket = (CellularSocket *)handle;
5252

5353
_at.lock();
54-
if (!socket->created) {
54+
if (socket->id == -1) {
5555
const nsapi_error_t error_create = create_socket_impl(socket);
5656
if (error_create != NSAPI_ERROR_OK) {
5757
return error_create;
@@ -187,23 +187,26 @@ nsapi_error_t QUECTEL_BC95_CellularStack::create_socket_impl(CellularSocket *soc
187187
// Check for duplicate socket id delivered by modem
188188
for (int i = 0; i < BC95_SOCKET_MAX; i++) {
189189
CellularSocket *sock = _socket[i];
190-
if (sock && sock->created && sock->id == sock_id) {
191-
tr_error("Duplicate socket index: %d created:%d, sock_id: %d", i, sock->created, sock_id);
190+
if (sock && sock->id != -1 && sock->id == sock_id) {
191+
tr_error("Duplicate socket index: %d, sock_id: %d", i, sock_id);
192192
return NSAPI_ERROR_NO_SOCKET;
193193
}
194194
}
195195

196196
tr_info("Socket create id: %d", sock_id);
197197

198198
socket->id = sock_id;
199-
socket->created = true;
200199

201200
return NSAPI_ERROR_OK;
202201
}
203202

204203
nsapi_size_or_error_t QUECTEL_BC95_CellularStack::socket_sendto_impl(CellularSocket *socket, const SocketAddress &address,
205204
const void *data, nsapi_size_t size)
206205
{
206+
//AT_CellularStack::socket_sendto(...) will create a socket on modem if it wasn't
207+
// open already.
208+
MBED_ASSERT(socket->id != -1);
209+
207210
int sent_len = 0;
208211

209212
if (size > PACKET_SIZE_MAX) {
@@ -250,6 +253,10 @@ nsapi_size_or_error_t QUECTEL_BC95_CellularStack::socket_sendto_impl(CellularSoc
250253
nsapi_size_or_error_t QUECTEL_BC95_CellularStack::socket_recvfrom_impl(CellularSocket *socket, SocketAddress *address,
251254
void *buffer, nsapi_size_t size)
252255
{
256+
//AT_CellularStack::socket_recvfrom(...) will create a socket on modem if it wasn't
257+
// open already.
258+
MBED_ASSERT(socket->id != -1);
259+
253260
nsapi_size_or_error_t recv_len = 0;
254261
int port;
255262
char ip_address[NSAPI_IP_SIZE];

‎features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_CellularStack.cpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ nsapi_error_t QUECTEL_BG96_CellularStack::socket_connect(nsapi_socket_t handle,
6969

7070
if ((_at.get_last_error() == NSAPI_ERROR_OK) && err) {
7171
if (err == BG96_SOCKET_BIND_FAIL) {
72-
socket->created = false;
72+
socket->id = -1;
7373
_at.unlock();
7474
return NSAPI_ERROR_PARAMETER;
7575
}
@@ -106,7 +106,7 @@ nsapi_error_t QUECTEL_BG96_CellularStack::socket_connect(nsapi_socket_t handle,
106106
_at.unlock();
107107

108108
if ((ret_val == NSAPI_ERROR_OK) && (modem_connect_id == request_connect_id)) {
109-
socket->created = true;
109+
socket->id = request_connect_id;
110110
socket->remoteAddress = address;
111111
socket->connected = true;
112112
return NSAPI_ERROR_OK;
@@ -182,10 +182,14 @@ void QUECTEL_BG96_CellularStack::handle_open_socket_response(int &modem_connect_
182182
nsapi_error_t QUECTEL_BG96_CellularStack::create_socket_impl(CellularSocket *socket)
183183
{
184184
int modem_connect_id = -1;
185-
int request_connect_id = socket->id;
186185
int remote_port = 0;
187186
int err = -1;
188187

188+
int request_connect_id = find_socket_index(socket);
189+
// assert here as its a programming error if the socket container doesn't contain
190+
// specified handle
191+
MBED_ASSERT(request_connect_id != -1);
192+
189193
if (socket->proto == NSAPI_UDP && !socket->connected) {
190194
_at.cmd_start("AT+QIOPEN=");
191195
_at.write_int(_cid);
@@ -201,7 +205,7 @@ nsapi_error_t QUECTEL_BG96_CellularStack::create_socket_impl(CellularSocket *soc
201205

202206
if ((_at.get_last_error() == NSAPI_ERROR_OK) && err) {
203207
if (err == BG96_SOCKET_BIND_FAIL) {
204-
socket->created = false;
208+
socket->id = -1;
205209
return NSAPI_ERROR_PARAMETER;
206210
}
207211
_at.cmd_start("AT+QICLOSE=");
@@ -233,7 +237,7 @@ nsapi_error_t QUECTEL_BG96_CellularStack::create_socket_impl(CellularSocket *soc
233237

234238
if ((_at.get_last_error() == NSAPI_ERROR_OK) && err) {
235239
if (err == BG96_SOCKET_BIND_FAIL) {
236-
socket->created = false;
240+
socket->id = -1;
237241
return NSAPI_ERROR_PARAMETER;
238242
}
239243
_at.cmd_start("AT+QICLOSE=");
@@ -261,7 +265,9 @@ nsapi_error_t QUECTEL_BG96_CellularStack::create_socket_impl(CellularSocket *soc
261265

262266
nsapi_error_t ret_val = _at.get_last_error();
263267

264-
socket->created = ((ret_val == NSAPI_ERROR_OK) && (modem_connect_id == request_connect_id));
268+
if (ret_val == NSAPI_ERROR_OK && (modem_connect_id == request_connect_id)) {
269+
socket->id = request_connect_id;
270+
}
265271

266272
return ret_val;
267273
}

‎features/cellular/framework/targets/QUECTEL/M26/QUECTEL_M26_CellularStack.cpp‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,8 @@ nsapi_error_t QUECTEL_M26_CellularStack::socket_connect(nsapi_socket_t handle, c
302302
{
303303
CellularSocket *socket = (CellularSocket *)handle;
304304

305+
MBED_ASSERT(socket->id != -1);
306+
305307
int modem_connect_id = -1;
306308
int request_connect_id = socket->id;
307309
int err = -1;
@@ -348,7 +350,6 @@ nsapi_error_t QUECTEL_M26_CellularStack::socket_connect(nsapi_socket_t handle, c
348350
_at.unlock();
349351

350352
if ((ret_val == NSAPI_ERROR_OK) && (modem_connect_id == request_connect_id)) {
351-
socket->created = true;
352353
socket->remoteAddress = address;
353354
socket->connected = true;
354355
return NSAPI_ERROR_OK;
@@ -359,7 +360,29 @@ nsapi_error_t QUECTEL_M26_CellularStack::socket_connect(nsapi_socket_t handle, c
359360

360361
nsapi_error_t QUECTEL_M26_CellularStack::create_socket_impl(CellularSocket *socket)
361362
{
362-
int request_connect_id = socket->id;
363+
// This modem is a special case. It takes in the socket ID rather than spitting
364+
// it out. So we will first try to use the index of the socket construct as the id
365+
// but if another opened socket is already opened with that id we will pick the next
366+
// id which is not in use
367+
bool duplicate = false;
368+
int potential_sid = -1;
369+
int index = find_socket_index(socket);
370+
371+
for (int i = 0; i < get_max_socket_count(); i++) {
372+
CellularSocket *sock = _socket[i];
373+
if (sock && sock != socket && sock->id == index) {
374+
duplicate = true;
375+
} else if (duplicate && !sock) {
376+
potential_sid = i;
377+
break;
378+
}
379+
}
380+
381+
if (duplicate) {
382+
index = potential_sid;
383+
}
384+
385+
int request_connect_id = index;
363386
int modem_connect_id = request_connect_id;
364387
int err = -1;
365388
nsapi_error_t ret_val;
@@ -403,13 +426,15 @@ nsapi_error_t QUECTEL_M26_CellularStack::create_socket_impl(CellularSocket *sock
403426
}
404427

405428
ret_val = _at.get_last_error();
406-
socket->created = ((ret_val == NSAPI_ERROR_OK) && (modem_connect_id == request_connect_id));
429+
if ((ret_val == NSAPI_ERROR_OK) && (modem_connect_id == request_connect_id)) {
430+
socket->id = request_connect_id;
431+
}
407432
return ret_val;
408433
} else {
409434
ret_val = NSAPI_ERROR_OK;
410435
}
411436

412-
tr_debug("QUECTEL_M26_CellularStack:%s:%u: END [%d,%d]", __FUNCTION__, __LINE__, socket->created, ret_val);
437+
tr_debug("QUECTEL_M26_CellularStack:%s:%u: END [%d]", __FUNCTION__, __LINE__, ret_val);
413438
return ret_val;
414439
}
415440

@@ -431,11 +456,11 @@ nsapi_size_or_error_t QUECTEL_M26_CellularStack::socket_sendto_impl(CellularSock
431456
return NSAPI_ERROR_PARAMETER;
432457
}
433458

434-
if (!socket->created) {
459+
if (socket->id == -1) {
435460
socket->remoteAddress = address;
436461
socket->connected = true;
437462
nsapi_error_t ret_val = create_socket_impl(socket);
438-
if ((ret_val != NSAPI_ERROR_OK) || (!socket->created)) {
463+
if ((ret_val != NSAPI_ERROR_OK) || (socket->id == -1)) {
439464
tr_error("QUECTEL_M26_CellularStack:%s:%u:[NSAPI_ERROR_NO_SOCKET]", __FUNCTION__, __LINE__);
440465
return NSAPI_ERROR_NO_SOCKET;
441466
}

0 commit comments

Comments
 (0)