Skip to content

WiFiClient::write refactoring #1570

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.

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

WiFiClient::write refactoring #1570

wants to merge 6 commits into from

Conversation

igrr
Copy link
Member

@igrr igrr commented Feb 4, 2016

  • Moved write behaviour (splitting payload into chunks, waiting for ACKs to arrive) from ClientContext into strategies.
  • Implemented BufferStrategy for payloads already present in RAM, and ChunkedStrategy for payloads which need to be copied to RAM on the fly.
  • WiFiClient::write(Stream&) will now send as much data as possible, as soon as possible. Previously it would send 1460 byte chunks, and would wait for each chunk to be ACKed before sending new one. Depending on network latency, this gives transfer speed increase within 25% — 100%.
  • WiFiClient will now get write timeout from _timeout member (1 second by default). Previously timeout was hard-coded to 5 seconds
  • ESP8266WebServer sets client timeout of 5 seconds before writing data. This is done to preserve old behaviour of WebServer.
  • WiFiClientSecure still has 5 seconds timeout hardcoded, some refactoring is necessary to expose _timeout member at the point when write function is called by axTLS.
  • ESP8266WebServer sendContent (_P) will now rely on WiFiClient::write (_P) to deliver the data

@igrr igrr force-pushed the wificlient-strategies branch from f08bb26 to b61c33d Compare February 4, 2016 14:47
@madsci1016
Copy link

Having WiFiClient::write(buf, length) require a stream object breaks compatibility with the Generic Arduino WiFiClient function and is not noted in the ESP Arduino documentation. (Caused me to loose some time today tracking this issue down). It also means Client.write() will behave differently than other stream-like function including serial.write().

I was casting some structures as a byte array and passing the pointer to .write functions. Since they aren't objects with available and source members, complier throws an issue.

@madsci1016
Copy link

Nevermind, please ignore. Not sure why It was defaulting to the stream function when I was passing in a byte array the exact same way I passed it into Serial.write() but I correctly cast it and the compiler took it anyway.

@igrr
Copy link
Member Author

igrr commented Feb 6, 2016

WiFiClient has the int write (const uint8_t*, size_t) overloaded member function, both with and without these changes: https://github.com/esp8266/Arduino/blob/wificlient-strategies/libraries/ESP8266WiFi/src/WiFiClient.h#L50

Before these changes, we had an additional overloaded template member function template <typename T> int write(T& file, size_t unitSize). It was a bad design decision. When someone intended to call write(const uint8_t*, size_t) and passed a const char* for the first argument, compiler selected this template overload, which obviously failed due to lack of .size() member function for a POD type.

Edit: this template<typename T> int write(T&, size_t) was replaced with int write(Stream&, size_t), which was marked deprecated, because the second argument (unitSize) is not used now. The new function, int write(Stream&) is indeed not present in the standard Arduino stream classes, but it is a nice extension, and it shouldn't result in collisions similar to the ones you have experienced.

@madsci1016
Copy link

Yep, that's what's biting my butt right now. Thanks for maintaining this codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants