Skip to content

ONME-3433 ESP8266 driver support for UDP get - modified ESP8266 drive… #12320

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
Feb 6, 2020
Merged

ONME-3433 ESP8266 driver support for UDP get - modified ESP8266 drive… #12320

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
41 changes: 35 additions & 6 deletions components/wifi/esp8266-driver/ESP8266/ESP8266.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,17 @@ bool ESP8266::disconnect(void)
return done;
}

bool ESP8266::ip_info_print(int enable)
{
_smutex.lock();
_disconnect = true;
bool done = _parser.send("AT+CIPDINFO=%d", enable) && _parser.recv("OK\n");
_smutex.unlock();

return done;
}


const char *ESP8266::ip_addr(void)
{
_smutex.lock();
Expand Down Expand Up @@ -508,11 +519,13 @@ int ESP8266::scan(WiFiAccessPoint *res, unsigned limit, scan_mode mode, unsigned
return cnt;
}

nsapi_error_t ESP8266::open_udp(int id, const char *addr, int port, int local_port)
nsapi_error_t ESP8266::open_udp(int id, const char *addr, int port, int local_port, int udp_mode)
{
static const char *type = "UDP";
bool done = false;

ip_info_print(1);

_smutex.lock();

// process OOB so that _sock_i reflects the correct state of the socket
Expand All @@ -533,7 +546,7 @@ nsapi_error_t ESP8266::open_udp(int id, const char *addr, int port, int local_po

for (int i = 0; i < 2; i++) {
if (local_port) {
done = _parser.send("AT+CIPSTART=%d,\"%s\",\"%s\",%d,%d", id, type, addr, port, local_port);
done = _parser.send("AT+CIPSTART=%d,\"%s\",\"%s\",%d,%d,%d", id, type, addr, port, local_port, udp_mode);
} else {
done = _parser.send("AT+CIPSTART=%d,\"%s\",\"%s\",%d", id, type, addr, port);
}
Expand Down Expand Up @@ -572,6 +585,8 @@ nsapi_error_t ESP8266::open_tcp(int id, const char *addr, int port, int keepaliv
static const char *type = "TCP";
bool done = false;

ip_info_print(1);

if (!addr) {
return NSAPI_ERROR_PARAMETER;
}
Expand Down Expand Up @@ -754,6 +769,7 @@ nsapi_size_or_error_t ESP8266::send(int id, const void *data, uint32_t amount)
void ESP8266::_oob_packet_hdlr()
{
int id;
int port;
int amount;
int pdu_len;

Expand All @@ -763,6 +779,8 @@ void ESP8266::_oob_packet_hdlr()
}

if (_tcp_passive && _sock_i[id].open == true && _sock_i[id].proto == NSAPI_TCP) {
//For TCP +IPD return only id and amount and it is independent on AT+CIPDINFO settings
//Unfortunately no information about that in ESP manual but it has sense.
if (_parser.recv("%d\n", &amount)) {
_sock_i[id].tcp_data_avbl = amount;

Expand All @@ -772,8 +790,12 @@ void ESP8266::_oob_packet_hdlr()
}
}
return;
} else if (!_parser.scanf("%d:", &amount)) {
return;
} else {
if (!(_parser.scanf("%d,", &amount)
&& _parser.scanf("%15[^,],", _ip_buffer)
&& _parser.scanf("%d:", &port))) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that _parser.scanf("%d,", &amount) is done whether cip_info is 1 or 0, so let's just do it before the check?
What comes next could be merged so it takes less space and matches the style of how we normally do it in this file. So something like this:

        } else {
            if (!_parser.scanf("%d,", &amount)) {
                 return;
            }
            if (cip_info == 1) {
                if (!(_parser.scanf("%15[^,],", _ip_buffer) 
                    && _parser.scanf("%d:", &port)) {
                    return;
                }
           }

It takes less space, is equally readable and matches the way we normally do this in this file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately you're wrong.
Delimiter for "amount" is different in version with showing ip, port against to version without showing ip, port. In first case it is "," in second case is ":"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, I missed that.
so let's just merge the scanfs together:

if (!(_parser.scanf("%d,", &amount)
    && _parser.scanf("%15[^,],", _ip_buffer) 
    && _parser.scanf("%d:", &port)) {
    return;
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the current structure if (!sth && !sthelse && !sthelse2) will return if ALL conditions fail, so it will have to check all of them, which makes no sense if the first one failed.
I think it is better to do if (!(sth && sthelse && sthelse2)), because then if one condition fails (for example the %d, match fails), the other conditions will not be checked (short-circuit evaluation).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Yes you're right. I've broken logic operation.

}

pdu_len = sizeof(struct packet) + amount;
Expand All @@ -791,6 +813,10 @@ void ESP8266::_oob_packet_hdlr()
_heap_usage += pdu_len;

packet->id = id;
if (_sock_i[id].proto == NSAPI_UDP) {
packet->remote_port = port;
memcpy(packet->remote_ip, _ip_buffer, 16);
}
packet->len = amount;
packet->alloc_len = amount;
packet->next = 0;
Expand Down Expand Up @@ -943,7 +969,7 @@ int32_t ESP8266::recv_tcp(int id, void *data, uint32_t amount, uint32_t timeout)
return NSAPI_ERROR_WOULD_BLOCK;
}

int32_t ESP8266::recv_udp(int id, void *data, uint32_t amount, uint32_t timeout)
int32_t ESP8266::recv_udp(struct esp8266_socket *socket, void *data, uint32_t amount, uint32_t timeout)
{
_smutex.lock();
set_timeout(timeout);
Expand All @@ -956,9 +982,12 @@ int32_t ESP8266::recv_udp(int id, void *data, uint32_t amount, uint32_t timeout)

// check if any packets are ready for us
for (struct packet **p = &_packets; *p; p = &(*p)->next) {
if ((*p)->id == id) {
if ((*p)->id == socket->id) {
struct packet *q = *p;

socket->addr.set_ip_address((*p)->remote_ip);
socket->addr.set_port((*p)->remote_port);

// Return and remove packet (truncated if necessary)
uint32_t len = q->len < amount ? q->len : amount;
memcpy(data, q + 1, len);
Expand Down
25 changes: 23 additions & 2 deletions components/wifi/esp8266-driver/ESP8266/ESP8266.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "platform/mbed_error.h"
#include "rtos/Mutex.h"
#include "rtos/ThisThread.h"
#include "features/netsocket/SocketAddress.h"

// Various timeouts for different ESP8266 operations
#ifndef ESP8266_CONNECT_TIMEOUT
Expand Down Expand Up @@ -64,6 +65,15 @@
#define FW_AT_LEAST_VERSION(MAJOR,MINOR,PATCH,NUSED/*Not used*/,REF) \
(((MAJOR)*1000000+(MINOR)*10000+(PATCH)*100) >= REF ? true : false)

struct esp8266_socket {
int id;
nsapi_protocol_t proto;
bool connected;
bool bound;
SocketAddress addr;
int keepalive; // TCP
};

/** ESP8266Interface class.
This is an interface to a ESP8266 radio.
*/
Expand Down Expand Up @@ -167,6 +177,14 @@ class ESP8266 {
*/
bool disconnect(void);

/**
* Enable or disable Remote IP and Port printing with +IPD
*
* @param enable, 1 on, 0 off
* @return true only if ESP8266 is disconnected successfully
*/
bool ip_info_print(int enable);

/**
* Get the IP address of ESP8266
*
Expand Down Expand Up @@ -236,9 +254,10 @@ class ESP8266 {
* @param addr the IP address of the destination
* @param port the port on the destination
* @param local_port UDP socket's local port, zero means any
* @param udp_mode UDP socket's mode, zero means can't change remote, 1 can change once, 2 can change multiple times
* @return NSAPI_ERROR_OK in success, negative error code in failure
*/
nsapi_error_t open_udp(int id, const char *addr, int port, int local_port = 0);
nsapi_error_t open_udp(int id, const char *addr, int port, int local_port = 0, int udp_mode = 0);

/**
* Open a socketed connection
Expand Down Expand Up @@ -271,7 +290,7 @@ class ESP8266 {
* @param amount number of bytes to be received
* @return the number of bytes received
*/
int32_t recv_udp(int id, void *data, uint32_t amount, uint32_t timeout = ESP8266_RECV_TIMEOUT);
int32_t recv_udp(struct esp8266_socket *socket, void *data, uint32_t amount, uint32_t timeout = ESP8266_RECV_TIMEOUT);

/**
* Receives stream data from an open TCP socket
Expand Down Expand Up @@ -442,6 +461,8 @@ class ESP8266 {
struct packet {
struct packet *next;
int id;
char remote_ip[16];
int remote_port;
uint32_t len; // Remaining length
uint32_t alloc_len; // Original length
// data follows
Expand Down
35 changes: 22 additions & 13 deletions components/wifi/esp8266-driver/ESP8266Interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@

#define ESP8266_WIFI_IF_NAME "es0"

#define LOCAL_ADDR "127.0.0.1"

using namespace mbed;
using namespace rtos;

Expand Down Expand Up @@ -750,14 +752,6 @@ nsapi_error_t ESP8266Interface::_reset()
return _esp.at_available() ? NSAPI_ERROR_OK : NSAPI_ERROR_DEVICE_ERROR;
}

struct esp8266_socket {
int id;
nsapi_protocol_t proto;
bool connected;
SocketAddress addr;
int keepalive; // TCP
};

int ESP8266Interface::socket_open(void **handle, nsapi_protocol_t proto)
{
// Look for an unused socket
Expand All @@ -783,6 +777,7 @@ int ESP8266Interface::socket_open(void **handle, nsapi_protocol_t proto)
socket->id = id;
socket->proto = proto;
socket->connected = false;
socket->bound = false;
socket->keepalive = 0;
*handle = socket;
return 0;
Expand All @@ -801,11 +796,16 @@ int ESP8266Interface::socket_close(void *handle)
err = NSAPI_ERROR_DEVICE_ERROR;
}

if (socket->bound && !_esp.close(socket->id)) {
err = NSAPI_ERROR_DEVICE_ERROR;
}

_cbs[socket->id].callback = NULL;
_cbs[socket->id].data = NULL;
core_util_atomic_store_u8(&_cbs[socket->id].deferred, false);

socket->connected = false;
socket->bound = false;
_sock_i[socket->id].open = false;
_sock_i[socket->id].sport = 0;
delete socket;
Expand All @@ -828,12 +828,17 @@ int ESP8266Interface::socket_bind(void *handle, const SocketAddress &address)
for (int id = 0; id < ESP8266_SOCKET_COUNT; id++) {
if (_sock_i[id].sport == address.get_port() && id != socket->id) { // Port already reserved by another socket
return NSAPI_ERROR_PARAMETER;
} else if (id == socket->id && socket->connected) {
} else if (id == socket->id && (socket->connected || socket->bound)) {
return NSAPI_ERROR_PARAMETER;
}
}
_sock_i[socket->id].sport = address.get_port();
return 0;

int ret = _esp.open_udp(socket->id, LOCAL_ADDR, address.get_port(), _sock_i[socket->id].sport, 2);

socket->bound = (ret == NSAPI_ERROR_OK) ? true : false;

return ret;
}

return NSAPI_ERROR_UNSUPPORTED;
Expand All @@ -854,7 +859,7 @@ int ESP8266Interface::socket_connect(void *handle, const SocketAddress &addr)
}

if (socket->proto == NSAPI_UDP) {
ret = _esp.open_udp(socket->id, addr.get_ip_address(), addr.get_port(), _sock_i[socket->id].sport);
ret = _esp.open_udp(socket->id, addr.get_ip_address(), addr.get_port(), _sock_i[socket->id].sport, 0);
} else {
ret = _esp.open_tcp(socket->id, addr.get_ip_address(), addr.get_port(), socket->keepalive);
}
Expand Down Expand Up @@ -925,7 +930,7 @@ int ESP8266Interface::socket_recv(void *handle, void *data, unsigned size)
socket->connected = false;
}
} else {
recv = _esp.recv_udp(socket->id, data, size);
recv = _esp.recv_udp(socket, data, size);
}

return recv;
Expand All @@ -950,14 +955,18 @@ int ESP8266Interface::socket_sendto(void *handle, const SocketAddress &addr, con
socket->connected = false;
}

if (!socket->connected) {
if (!socket->connected && !socket->bound) {
int err = socket_connect(socket, addr);
if (err < 0) {
return err;
}
socket->addr = addr;
}

if (socket->bound) {
socket->addr = addr;
}

return socket_send(socket, data, size);
}

Expand Down