Skip to content

Cellular: Preventing Socket ID assignment until actual socket creation at the modem #10431

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

Closed
wants to merge 6 commits into from
Closed

Cellular: Preventing Socket ID assignment until actual socket creation at the modem #10431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a7ef088
Prevent assigning socket id until opened at modem
Apr 17, 2019
511c734
Updating BG96 to accomodate socket id assignment
Apr 17, 2019
cd9e3b1
Updating Gemalto Cinterion to accomodate socked id assignment
Apr 17, 2019
231df01
Updating BG95 to accomodate socket id assignment
Apr 17, 2019
ff74cec
Updating M26 to accomodate socket id assignment
Apr 17, 2019
204e993
Updating Ublox to accomodate socket id assignment
Apr 17, 2019
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
16 changes: 8 additions & 8 deletions features/cellular/framework/AT/AT_CellularStack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ int AT_CellularStack::find_socket_index(nsapi_socket_t handle)
return -1;
}


/** NetworkStack
*/

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

tr_info("Socket %d open", index);
// create local socket structure, socket on modem is created when app calls sendto/recvfrom
// Do not assign a socket ID yet. Socket is not created at the Modem yet.
// create_socket_impl(handle) will assign the correct socket ID.
_socket[index] = new CellularSocket;
CellularSocket *psock = _socket[index];
SocketAddress addr(0, get_dynamic_ip_port());
psock->id = index;

psock->localAddress = addr;
psock->proto = proto;
*handle = psock;
Expand All @@ -153,7 +154,6 @@ nsapi_error_t AT_CellularStack::socket_close(nsapi_socket_t handle)
return err;
}
int sock_id = socket->id;
bool sock_created = socket->created;

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

// Close the socket on the modem if it was created
_at.lock();
if (sock_created) {
if (sock_id > -1) {
err = socket_close_impl(sock_id);
}

if (!err) {
tr_info("Socket %d closed", index);
} else {
tr_info("Socket %d close (id %d, created %d, started %d, error %d)", index, sock_id, socket->created, socket->started, err);
tr_info("Socket %d close (id %d, started %d, error %d)", index, sock_id, socket->started, err);
}

_socket[index] = NULL;
Expand Down Expand Up @@ -206,7 +206,7 @@ nsapi_error_t AT_CellularStack::socket_bind(nsapi_socket_t handle, const SocketA
}
}

if (!socket->created) {
if (socket->id == -1) {
create_socket_impl(socket);
}

Expand Down Expand Up @@ -266,7 +266,7 @@ nsapi_size_or_error_t AT_CellularStack::socket_sendto(nsapi_socket_t handle, con

nsapi_size_or_error_t ret_val = NSAPI_ERROR_OK;

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

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

nsapi_size_or_error_t ret_val = NSAPI_ERROR_OK;

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

ret_val = create_socket_impl(socket);
Expand Down
19 changes: 13 additions & 6 deletions features/cellular/framework/AT/AT_CellularStack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ class AT_CellularStack : public NetworkStack, public AT_CellularBase {
localAddress("", 0),
_cb(NULL),
_data(NULL),
created(false),
closed(false),
started(false),
tx_ready(false),
Expand All @@ -114,7 +113,6 @@ class AT_CellularStack : public NetworkStack, public AT_CellularBase {
SocketAddress localAddress;
void (*_cb)(void *);
void *_data;
bool created; // socket has been created on modem stack
bool closed; // socket has been closed by a peer
bool started; // socket has been opened on modem stack
bool tx_ready; // socket is ready for sending on modem stack
Expand Down Expand Up @@ -175,12 +173,22 @@ class AT_CellularStack : public NetworkStack, public AT_CellularBase {
void *buffer, nsapi_size_t size) = 0;

/**
* Find the socket handle based on socket identifier
* Find the socket handle based on the index of the socket construct
* in the socket container. Please note that this index may or may not be
* the socket id. The actual meaning of this index depends upon the modem
* being used.
*
* @param sock_id Socket identifier
* @param index Index of the socket construct in the container
* @return Socket handle, NULL on error
*/
CellularSocket *find_socket(int sock_id);
CellularSocket *find_socket(int index);

/**
* Find the index of the given CellularSocket handle. This index may or may
* not be the socket id. The actual meaning of this index depends upon the modem
* being used.
*/
int find_socket_index(nsapi_socket_t handle);

// socket container
CellularSocket **_socket;
Expand All @@ -198,7 +206,6 @@ class AT_CellularStack : public NetworkStack, public AT_CellularBase {
nsapi_ip_stack_t _stack_type;

private:
int find_socket_index(nsapi_socket_t handle);

int get_socket_index_by_port(uint16_t port);

Expand Down
16 changes: 10 additions & 6 deletions features/cellular/framework/targets/GEMALTO/CINTERION/GEMALTO_CINTERION_CellularStack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ nsapi_error_t GEMALTO_CINTERION_CellularStack::socket_open_defer(CellularSocket
int retry_open = 1;
retry_open:
// setup internet session profile
int internet_service_id = socket->id;
int internet_service_id = find_socket_index(socket);
bool foundSrvType = false;
bool foundConIdType = false;
_at.cmd_start("AT^SISS?");
Expand Down Expand Up @@ -244,26 +244,26 @@ nsapi_error_t GEMALTO_CINTERION_CellularStack::socket_open_defer(CellularSocket
}

_at.cmd_start("AT^SISS=");
_at.write_int(socket->id);
_at.write_int(internet_service_id);
_at.write_string("address", false);
_at.write_string(sock_addr);
_at.cmd_stop_read_resp();

_at.cmd_start("AT^SISO=");
_at.write_int(socket->id);
_at.write_int(internet_service_id);
_at.cmd_stop_read_resp();

if (_at.get_last_error()) {
tr_error("Socket %d open failed!", socket->id);
_at.clear_error();
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
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
if (retry_open--) {
goto retry_open;
}
return NSAPI_ERROR_NO_SOCKET;
}

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

return _at.get_last_error();
Expand Down Expand Up @@ -316,11 +316,11 @@ nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_sendto_impl(Cellul
socket->remoteAddress = address;
_at.resp_start("^SISW:");
int sock_id = _at.read_int();
MBED_ASSERT(sock_id == socket->id);
int urc_code = _at.read_int();
tr_debug("TX ready: socket=%d, urc=%d (err=%d)", sock_id, urc_code, _at.get_last_error());
(void)sock_id;
(void)urc_code;
socket->created = true;
socket->started = true;
socket->tx_ready = true;
}
Expand Down Expand Up @@ -399,6 +399,10 @@ nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_sendto_impl(Cellul
nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_recvfrom_impl(CellularSocket *socket, SocketAddress *address,
void *buffer, nsapi_size_t size)
{
// AT_CellularStack::recvfrom(...) will make sure that we do have a socket
// open on the modem, assert here to catch a programming error
MBED_ASSERT(socket->id != -1);

// we must use this flag, otherwise ^SISR URC can come while we are reading response and there is
// no way to detect if that is really an URC or response
if (!socket->rx_avail) {
Expand Down
15 changes: 11 additions & 4 deletions features/cellular/framework/targets/QUECTEL/BC95/QUECTEL_BC95_CellularStack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ nsapi_error_t QUECTEL_BC95_CellularStack::socket_connect(nsapi_socket_t handle,
CellularSocket *socket = (CellularSocket *)handle;

_at.lock();
if (!socket->created) {
if (socket->id == -1) {
const nsapi_error_t error_create = create_socket_impl(socket);
if (error_create != NSAPI_ERROR_OK) {
return error_create;
Expand Down Expand Up @@ -157,23 +157,26 @@ nsapi_error_t QUECTEL_BC95_CellularStack::create_socket_impl(CellularSocket *soc
// Check for duplicate socket id delivered by modem
for (int i = 0; i < BC95_SOCKET_MAX; i++) {
CellularSocket *sock = _socket[i];
if (sock && sock->created && sock->id == sock_id) {
tr_error("Duplicate socket index: %d created:%d, sock_id: %d", i, sock->created, sock_id);
if (sock && sock->id != -1 && sock->id == sock_id) {
tr_error("Duplicate socket index: %d, sock_id: %d", i, sock_id);
return NSAPI_ERROR_NO_SOCKET;
}
}

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

socket->id = sock_id;
socket->created = true;

return NSAPI_ERROR_OK;
}

nsapi_size_or_error_t QUECTEL_BC95_CellularStack::socket_sendto_impl(CellularSocket *socket, const SocketAddress &address,
const void *data, nsapi_size_t size)
{
//AT_CellularStack::socket_sendto(...) will create a socket on modem if it wasn't
// open already.
MBED_ASSERT(socket->id != -1);

int sent_len = 0;

if (size > PACKET_SIZE_MAX) {
Expand Down Expand Up @@ -220,6 +223,10 @@ nsapi_size_or_error_t QUECTEL_BC95_CellularStack::socket_sendto_impl(CellularSoc
nsapi_size_or_error_t QUECTEL_BC95_CellularStack::socket_recvfrom_impl(CellularSocket *socket, SocketAddress *address,
void *buffer, nsapi_size_t size)
{
//AT_CellularStack::socket_recvfrom(...) will create a socket on modem if it wasn't
// open already.
MBED_ASSERT(socket->id != -1);

nsapi_size_or_error_t recv_len = 0;
int port;
char ip_address[NSAPI_IP_SIZE];
Expand Down
18 changes: 12 additions & 6 deletions features/cellular/framework/targets/QUECTEL/BG96/QUECTEL_BG96_CellularStack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ nsapi_error_t QUECTEL_BG96_CellularStack::socket_connect(nsapi_socket_t handle,

if ((_at.get_last_error() == NSAPI_ERROR_OK) && err) {
if (err == BG96_SOCKET_BIND_FAIL) {
socket->created = false;
socket->id = -1;
return NSAPI_ERROR_PARAMETER;
}
_at.cmd_start("AT+QICLOSE=");
Expand Down Expand Up @@ -99,7 +99,7 @@ nsapi_error_t QUECTEL_BG96_CellularStack::socket_connect(nsapi_socket_t handle,
_at.unlock();

if ((ret_val == NSAPI_ERROR_OK) && (modem_connect_id == request_connect_id)) {
socket->created = true;
socket->id = request_connect_id;
socket->remoteAddress = address;
socket->connected = true;
return NSAPI_ERROR_OK;
Expand Down Expand Up @@ -164,10 +164,14 @@ void QUECTEL_BG96_CellularStack::handle_open_socket_response(int &modem_connect_
nsapi_error_t QUECTEL_BG96_CellularStack::create_socket_impl(CellularSocket *socket)
{
int modem_connect_id = -1;
int request_connect_id = socket->id;
int remote_port = 0;
int err = -1;

int request_connect_id = find_socket_index(socket);
// assert here as its a programming error if the socket container doesn't contain
// specified handle
MBED_ASSERT(request_connect_id != -1);

if (socket->proto == NSAPI_UDP && !socket->connected) {
_at.cmd_start("AT+QIOPEN=");
_at.write_int(_cid);
Expand All @@ -183,7 +187,7 @@ nsapi_error_t QUECTEL_BG96_CellularStack::create_socket_impl(CellularSocket *soc

if ((_at.get_last_error() == NSAPI_ERROR_OK) && err) {
if (err == BG96_SOCKET_BIND_FAIL) {
socket->created = false;
socket->id = -1;
return NSAPI_ERROR_PARAMETER;
}
_at.cmd_start("AT+QICLOSE=");
Expand Down Expand Up @@ -215,7 +219,7 @@ nsapi_error_t QUECTEL_BG96_CellularStack::create_socket_impl(CellularSocket *soc

if ((_at.get_last_error() == NSAPI_ERROR_OK) && err) {
if (err == BG96_SOCKET_BIND_FAIL) {
socket->created = false;
socket->id = -1;
return NSAPI_ERROR_PARAMETER;
}
_at.cmd_start("AT+QICLOSE=");
Expand Down Expand Up @@ -243,7 +247,9 @@ nsapi_error_t QUECTEL_BG96_CellularStack::create_socket_impl(CellularSocket *soc

nsapi_error_t ret_val = _at.get_last_error();

socket->created = ((ret_val == NSAPI_ERROR_OK) && (modem_connect_id == request_connect_id));
if (ret_val == NSAPI_ERROR_OK && (modem_connect_id == request_connect_id)) {
socket->id = request_connect_id;
}

return ret_val;
}
Expand Down
37 changes: 31 additions & 6 deletions features/cellular/framework/targets/QUECTEL/M26/QUECTEL_M26_CellularStack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ nsapi_error_t QUECTEL_M26_CellularStack::socket_connect(nsapi_socket_t handle, c
{
CellularSocket *socket = (CellularSocket *)handle;

MBED_ASSERT(socket->id != -1);

int modem_connect_id = -1;
int request_connect_id = socket->id;
int err = -1;
Expand Down Expand Up @@ -348,7 +350,6 @@ nsapi_error_t QUECTEL_M26_CellularStack::socket_connect(nsapi_socket_t handle, c
_at.unlock();

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

nsapi_error_t QUECTEL_M26_CellularStack::create_socket_impl(CellularSocket *socket)
{
int request_connect_id = socket->id;
// This modem is a special case. It takes in the socket ID rather than spitting
// it out. So we will first try to use the index of the socket construct as the id
// but if another opened socket is already opened with that id we will pick the next
// id which is not in use
bool duplicate = false;
int potential_sid = -1;
int index = find_socket_index(socket);

for (int i = 0; i < get_max_socket_count(); i++) {
CellularSocket *sock = _socket[i];
if (sock && sock != socket && sock->id == index) {
duplicate = true;
} else if (duplicate && !sock) {
potential_sid = i;
break;
}
}

if (duplicate) {
index = potential_sid;
}

int request_connect_id = index;
int modem_connect_id = request_connect_id;
int err = -1;
nsapi_error_t ret_val;
Expand Down Expand Up @@ -403,13 +426,15 @@ nsapi_error_t QUECTEL_M26_CellularStack::create_socket_impl(CellularSocket *sock
}

ret_val = _at.get_last_error();
socket->created = ((ret_val == NSAPI_ERROR_OK) && (modem_connect_id == request_connect_id));
if ((ret_val == NSAPI_ERROR_OK) && (modem_connect_id == request_connect_id)) {
socket->id = request_connect_id;
}
return ret_val;
} else {
ret_val = NSAPI_ERROR_OK;
}

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

Expand All @@ -431,11 +456,11 @@ nsapi_size_or_error_t QUECTEL_M26_CellularStack::socket_sendto_impl(CellularSock
return NSAPI_ERROR_PARAMETER;
}

if (!socket->created) {
if (socket->id == -1) {
socket->remoteAddress = address;
socket->connected = true;
nsapi_error_t ret_val = create_socket_impl(socket);
if ((ret_val != NSAPI_ERROR_OK) || (!socket->created)) {
if ((ret_val != NSAPI_ERROR_OK) || (socket->id == -1)) {
tr_error("QUECTEL_M26_CellularStack:%s:%u:[NSAPI_ERROR_NO_SOCKET]", __FUNCTION__, __LINE__);
return NSAPI_ERROR_NO_SOCKET;
}
Expand Down
Loading