Skip to content

Rework WiFiClient #238

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 4 commits into from
Mar 1, 2017
Merged

Rework WiFiClient #238

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ec46157
Rework WiFiClient
schrod Feb 26, 2017
19e6b43
Rework WiFiClient to use shared_ptr
DSchroederPresco Feb 27, 2017
c15b79a
Merge branch 'master'
schrod Mar 1, 2017
12c2c1d
Incorporate comments from review
schrod Mar 1, 2017
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
91 changes: 13 additions & 78 deletions libraries/WiFi/src/WiFiClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#undef write
#undef read

#define WIFI_CLIENT_SELECT_TIMEOUT_US (100000)

WiFiClientSocketHandle::~WiFiClientSocketHandle()
{
Expand All @@ -35,12 +34,11 @@ WiFiClientSocketHandle::~WiFiClientSocketHandle()

WiFiClient::WiFiClient():_connected(false),next(NULL)
{
clientSocketHandle = NULL;
}

WiFiClient::WiFiClient(int fd):_connected(true),next(NULL)
{
clientSocketHandle = new WiFiClientSocketHandle(fd);
clientSocketHandle.reset(new WiFiClientSocketHandle(fd));
}

WiFiClient::~WiFiClient()
Expand All @@ -52,28 +50,12 @@ WiFiClient & WiFiClient::operator=(const WiFiClient &other)
{
stop();
clientSocketHandle = other.clientSocketHandle;
if (clientSocketHandle != NULL) {
clientSocketHandle->ref();
}
_connected = other._connected;
return *this;
}

WiFiClient::WiFiClient(const WiFiClient &other)
{
clientSocketHandle = other.clientSocketHandle;
if (clientSocketHandle != NULL) {
clientSocketHandle->ref();
}
_connected = other._connected;
next = other.next;
}

void WiFiClient::stop()
{
if (clientSocketHandle && clientSocketHandle->unref() == 0) {
delete clientSocketHandle;
}
clientSocketHandle = NULL;
_connected = false;
}
Expand All @@ -86,12 +68,6 @@ int WiFiClient::connect(IPAddress ip, uint16_t port)
return 0;
}

//If there is an exisiting socket, unreference it
if (clientSocketHandle && clientSocketHandle->unref() == 0) {
delete clientSocketHandle;
}
clientSocketHandle = new WiFiClientSocketHandle(sockfd);

uint32_t ip_addr = ip;
struct sockaddr_in serveraddr;
bzero((char *) &serveraddr, sizeof(serveraddr));
Expand All @@ -101,10 +77,10 @@ int WiFiClient::connect(IPAddress ip, uint16_t port)
int res = lwip_connect_r(sockfd, (struct sockaddr*)&serveraddr, sizeof(serveraddr));
if (res < 0) {
log_e("lwip_connect_r: %d", errno);
delete clientSocketHandle;
clientSocketHandle = NULL;
close(sockfd);
return 0;
}
clientSocketHandle.reset(new WiFiClientSocketHandle(sockfd));
_connected = true;
return 1;
}
Expand All @@ -122,9 +98,6 @@ int WiFiClient::connect(const char *host, uint16_t port)

int WiFiClient::setSocketOption(int option, char* value, size_t len)
{
if (clientSocketHandle == NULL) {
return -1;
}
int res = setsockopt(fd(), SOL_SOCKET, option, value, len);
if(res < 0) {
log_e("%d", errno);
Expand All @@ -145,22 +118,15 @@ int WiFiClient::setTimeout(uint32_t seconds)

int WiFiClient::setOption(int option, int *value)
{
if (clientSocketHandle == NULL) {
return -1;
}
int res = setsockopt(fd(), IPPROTO_TCP, option, (char *) value,
sizeof(int));
if (res < 0) {
int res = setsockopt(fd(), IPPROTO_TCP, option, (char *) value, sizeof(int));
if(res < 0) {
log_e("%d", errno);
}
return res;
}

int WiFiClient::getOption(int option, int *value)
{
if (clientSocketHandle == NULL) {
return -1;
}
size_t size = sizeof(int);
int res = getsockopt(fd(), IPPROTO_TCP, option, (char *)value, &size);
if(res < 0) {
Expand Down Expand Up @@ -199,52 +165,21 @@ int WiFiClient::read()

size_t WiFiClient::write(const uint8_t *buf, size_t size)
Copy link
Member

Choose a reason for hiding this comment

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

You need to sync this code now with the one that I already merged :)

{
int res =0;
bool retry = true;

if (!_connected || clientSocketHandle == NULL) {
if(!_connected) {
return 0;
}

int sockfd = fd();

while (retry) {
//use select to make sure the socket is ready for writing
fd_set set;
struct timeval tv;
FD_ZERO(&set); /* empties the set */
FD_SET(sockfd, &set); /* adds FD to the set */
tv.tv_sec = 0;
tv.tv_usec = WIFI_CLIENT_SELECT_TIMEOUT_US;

if (select(sockfd + 1, NULL, &set, NULL, &tv) < 0) {
return 0;
}

if (FD_ISSET(sockfd, &set)) {
res = send(sockfd, (void*) buf, size, MSG_DONTWAIT);
if (res < 0) {
log_e("%d", errno);
Serial.print("Write Error: ");
Serial.println(errno);
if (errno != EAGAIN) {
//if resource was busy, can try again, otherwise give up
stop();
res = 0;
retry = false;
}
} else {
//completed successfully
retry = false;
}
}
int res = send(fd(), (void*)buf, size, MSG_DONTWAIT);
if(res < 0) {
log_e("%d", errno);
stop();
res = 0;
}
return res;
}

int WiFiClient::read(uint8_t *buf, size_t size)
{
if (!available() || clientSocketHandle == NULL) {
if(!available()) {
return -1;
}
int res = recv(fd(), buf, size, MSG_DONTWAIT);
Expand All @@ -257,7 +192,7 @@ int WiFiClient::read(uint8_t *buf, size_t size)

int WiFiClient::available()
{
if (!_connected || clientSocketHandle == NULL) {
if(!_connected) {
return 0;
}
int count;
Expand Down
20 changes: 5 additions & 15 deletions libraries/WiFi/src/WiFiClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,21 @@
#ifndef _WIFICLIENT_H_
#define _WIFICLIENT_H_


#include "Arduino.h"
#include "Client.h"
#include <memory>

Copy link
Member

Choose a reason for hiding this comment

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

You can move the whole class in the CPP and only forward declare it here. It's private really and users do not need to bother with it.

class WiFiClientSocketHandle {
private:
int sockfd;
int refCount;

public:
WiFiClientSocketHandle(int fd):sockfd(fd), refCount(1)
{
}
;
~WiFiClientSocketHandle();

int ref()
WiFiClientSocketHandle(int fd):sockfd(fd)
{
return ++refCount;
}

int unref()
{
return --refCount;
}
~WiFiClientSocketHandle();

int fd()
{
Expand All @@ -54,14 +45,13 @@ class WiFiClientSocketHandle {
class WiFiClient : public Client
{
protected:
WiFiClientSocketHandle *clientSocketHandle;
std::shared_ptr<WiFiClientSocketHandle> clientSocketHandle;
bool _connected;

public:
WiFiClient *next;
WiFiClient();
WiFiClient(int fd);
WiFiClient(const WiFiClient &other);
~WiFiClient();
int connect(IPAddress ip, uint16_t port);
int connect(const char *host, uint16_t port);
Expand Down