Skip to content

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.

3 participants