Skip to content

Additions to WiFiClient and WiFiServer #404

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
Jun 14, 2017
Merged

Additions to WiFiClient and WiFiServer #404

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
33 changes: 33 additions & 0 deletions libraries/WiFi/src/WiFiClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ size_t WiFiClient::write(const uint8_t *buf, size_t size)
return res;
}

size_t WiFiClient::write_P(PGM_P buf, size_t size)
{
return write(buf, size);
}

Copy link
Member

Choose a reason for hiding this comment

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

this is not official Arduino API is it? We do not really have _P anything ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ESP8266WebServer has *_P methods so I added support for them in the ESP32 version.

void WebServer::send_P(int code, PGM_P content_type, PGM_P content) {
    size_t contentLength = 0;

    if (content != NULL) {
        contentLength = strlen_P(content);
    }

    String header;
    char type[64];
    memccpy_P((void*)type, (PGM_VOID_P)content_type, 0, sizeof(type));
    _prepareHeader(header, code, (const char* )type, contentLength);
    _currentClient.write(header.c_str(), header.length());
    sendContent_P(content);
}

void WebServer::send_P(int code, PGM_P content_type, PGM_P content, size_t contentLength) {
    String header;
    char type[64];
    memccpy_P((void*)type, (PGM_VOID_P)content_type, 0, sizeof(type));
    _prepareHeader(header, code, (const char* )type, contentLength);
    sendContent(header);
    sendContent_P(content, contentLength);
}

void WebServer::send(int code, char* content_type, const String& content) {
  send(code, (const char*)content_type, content);
}

void WebServer::send(int code, const String& content_type, const String& content) {
  send(code, (const char*)content_type.c_str(), content);
}

void WebServer::sendContent(const String& content) {
  const char * footer = "\r\n";
  size_t len = content.length();
  if(_chunked) {
    char * chunkSize = (char *)malloc(11);
    if(chunkSize){
      sprintf(chunkSize, "%x%s", len, footer);
      _currentClient.write(chunkSize, strlen(chunkSize));
      free(chunkSize);
    }
  }
  _currentClient.write(content.c_str(), len);
  if(_chunked){
    _currentClient.write(footer, 2);
  }
}

void WebServer::sendContent_P(PGM_P content) {
  sendContent_P(content, strlen_P(content));
}

void WebServer::sendContent_P(PGM_P content, size_t size) {
  const char * footer = "\r\n";
  if(_chunked) {
    char * chunkSize = (char *)malloc(11);
    if(chunkSize){
      sprintf(chunkSize, "%x%s", size, footer);
      _currentClient.write(chunkSize, strlen(chunkSize));
      free(chunkSize);
    }
  }
#ifdef ESP8266
  _currentClient.write_P(content, size);
#else
  // TODO add ESP32 WiFiClient::write_P
  _currentClient.write(content, size);
#endif
  if(_chunked){
    _currentClient.write(footer, 2);
  }
}

Copy link
Member

Choose a reason for hiding this comment

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

_P methods imply PROGMEM an we do not have PROGMEM on esp32

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@copercini Compare pgmspace.h on ESP32 vs. ESP8266. There is no support for PROGMEM on ESP32 so pgmspace.h neuters the _P functions to their non _P equivalents.

But to avoid breaking lots of apps and libraries is would be good to translate _P functions to something that works on ESP32. Instead of making this change, libs and apps would have to do this.

#ifdef ESP8266
_currentClient.write_P(content, size);
#else
_currentClient.write(content, size);
#endif

If there are many occurrences, perhaps #define write_P write would do the job. But it would be less work and fewer breakage issues if write_P() is added even though it does the same thing as write().

Copy link
Member

Choose a reason for hiding this comment

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

I can not agree to bloat the classes unnecessary just for the sake of compatibility with everything. We have to add all of the _P methods then to Print, Stream and whatever else has them on ESP8266... bad idea!

Copy link
Member

Choose a reason for hiding this comment

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

I can agree to mark them deprecated and remove at later point.

What approach would you suggest library authors take when it comes to writing code which supports both the ESP8266 and the ESP32?

Copy link
Member

Choose a reason for hiding this comment

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

What would happen if we separate code like it's with WiFi? will all of the source files be included even if not used?

Copy link
Member

Choose a reason for hiding this comment

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

At link time, only the functions which were referenced will be left in the program, if -ffunction-section and -gc-sections options are used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of adding write_P, how about adding #define write_P write to pgmspace.h?

Copy link
Member

Choose a reason for hiding this comment

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

#define write_P write will not lead to proper client.write_P. @igrr correct me if I'm wrong, but that is what I believe the result will be

int WiFiClient::read(uint8_t *buf, size_t size)
{
if(!available()) {
Expand Down Expand Up @@ -346,6 +351,34 @@ uint16_t WiFiClient::remotePort() const
return remotePort(fd());
}

IPAddress WiFiClient::localIP(int fd) const
{
struct sockaddr_storage addr;
socklen_t len = sizeof addr;
getsockname(fd, (struct sockaddr*)&addr, &len);
struct sockaddr_in *s = (struct sockaddr_in *)&addr;
return IPAddress((uint32_t)(s->sin_addr.s_addr));
}

uint16_t WiFiClient::localPort(int fd) const
{
struct sockaddr_storage addr;
socklen_t len = sizeof addr;
getsockname(fd, (struct sockaddr*)&addr, &len);
struct sockaddr_in *s = (struct sockaddr_in *)&addr;
return ntohs(s->sin_port);
}

IPAddress WiFiClient::localIP() const
{
return localIP(fd());
}

uint16_t WiFiClient::localPort() const
{
return localPort(fd());
}

bool WiFiClient::operator==(const WiFiClient& rhs)
{
return clientSocketHandle == rhs.clientSocketHandle && remotePort() == rhs.remotePort() && remoteIP() == rhs.remoteIP();
Expand Down
5 changes: 5 additions & 0 deletions libraries/WiFi/src/WiFiClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class WiFiClient : public Client
int connect(const char *host, uint16_t port);
size_t write(uint8_t data);
size_t write(const uint8_t *buf, size_t size);
size_t write_P(PGM_P buf, size_t size);
int available();
int read();
int read(uint8_t *buf, size_t size);
Expand Down Expand Up @@ -84,6 +85,10 @@ class WiFiClient : public Client
IPAddress remoteIP(int fd) const;
uint16_t remotePort() const;
uint16_t remotePort(int fd) const;
IPAddress localIP() const;
IPAddress localIP(int fd) const;
uint16_t localPort() const;
uint16_t localPort(int fd) const;

//friend class WiFiServer;
using Print::write;
Expand Down
11 changes: 10 additions & 1 deletion libraries/WiFi/src/WiFiServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <lwip/netdb.h>

#undef write
#undef close

int WiFiServer::setTimeout(uint32_t seconds){
struct timeval tv;
Expand Down Expand Up @@ -103,8 +104,16 @@ bool WiFiServer::hasClient() {
}

void WiFiServer::end(){
close(sockfd);
lwip_close_r(sockfd);
Copy link
Member

Choose a reason for hiding this comment

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

why?

Copy link
Member

Choose a reason for hiding this comment

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

oh breaks with the other close...

sockfd = -1;
_listening = false;
}

void WiFiServer::close(){
Copy link
Member

Choose a reason for hiding this comment

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

I think we should go with one or the other ;) What's the official API for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ESP8266WiFiServer has close() and stop(). ESP32 WiFiServer has end(). ESP8266WebServer calls close. I chose to put all three for maximum backwards compatibility.

void WebServer::close() {
#ifdef ESP8266
  _server.close();
#else
  // TODO add ESP32 WiFiServer::close()
  _server.end();
#endif
}

Copy link
Member

Choose a reason for hiding this comment

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

ESP32 should actually always use stop() as that is the official Arduino API. close and end should e deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I can remove close and end.

end();
}

void WiFiServer::stop(){
end();
}

2 changes: 2 additions & 0 deletions libraries/WiFi/src/WiFiServer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class WiFiServer : public Server {
using Print::write;

void end();
void close();
void stop();
operator bool(){return _listening;}
int setTimeout(uint32_t seconds);
void stopAll();
Expand Down