ClientContext (tcp) updates (#5089)

* +sync, get/set default nodelay, sync

* default nodelay=1

* update flush()

* fix return value

* ClientContext: put things together

* ClientContext: fix debugging messages

* WiFiClient: move static members out of the class, add comments

* remove circular dependency

* parameter and return value for Client::flush&stop, flush timeout raised to 300ms

* tcp flush: restart timer on ack receive

* OTA protocol needs setNoDelay(true)

* fix Ethernet with Client changes

* 1 line unredable -> 5 lines readable code

* doc

* Update client-class.rst

* Added details for getters

Author: d-a-v

cores/esp8266/Client.h

@@ -34,8 +34,8 @@ class Client: public Stream {
         virtual int read() = 0;
         virtual int read(uint8_t *buf, size_t size) = 0;
         virtual int peek() = 0;
-        virtual void flush() = 0;
-        virtual void stop() = 0;
+        virtual bool flush(unsigned int maxWaitMs = 0) = 0;
+        virtual bool stop(unsigned int maxWaitMs = 0) = 0;
         virtual uint8_t connected() = 0;
         virtual operator bool() = 0;
     protected:

doc/esp8266wifi/client-class.rst

@@ -18,6 +18,17 @@ Methods documented for `Client <https://www.arduino.cc/en/Reference/WiFiClientCo
 
 Methods and properties described further down are specific to ESP8266. They are not covered in `Arduino WiFi library <https://www.arduino.cc/en/Reference/WiFi>`__ documentation. Before they are fully documented please refer to information below.
 
+flush and stop
+~~~~~~~~~~~~~~
+
+``flush(timeoutMs)`` and ``stop(timeoutMs)`` both have now an optional argument: ``timeout`` in millisecond, and both return a boolean.
+
+Default input value 0 means that effective value is left at the discretion of the implementer.
+
+``flush()`` returning ``true`` indicates that output data have effectively been sent, and ``false`` that a timeout has occurred.
+
+``stop()`` returns ``false`` in case of an issue when closing the client (for instance a timed-out ``flush``). Depending on implementation, its parameter can be passed to ``flush()``.
+
 setNoDelay
 ~~~~~~~~~~
 
@@ -35,6 +46,47 @@ This algorithm is intended to reduce TCP/IP traffic of small packets sent over t
 
     client.setNoDelay(true);
 
+getNoDelay
+~~~~~~~~~~
+
+Returns whether NoDelay is enabled or not for the current connection.
+
+setSync
+~~~~~~~
+
+This is an experimental API that will set the client in synchronized mode.
+In this mode, every ``write()`` is flushed.  It means that after a call to
+``write()``, data are ensured to be received where they went sent to (that is
+``flush`` semantic).
+
+When set to ``true`` in ``WiFiClient`` implementation,
+
+- It slows down transfers, and implicitely disable the Nagle algorithm.
+
+- It also allows to avoid a temporary copy of data that otherwise consumes
+  at most ``TCP_SND_BUF`` = (2 * ``MSS``) bytes per connection,
+
+getSync
+~~~~~~~
+
+Returns whether Sync is enabled or not for the current connection.
+
+setDefaultNoDelay and setDefaultSync
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+These set the default value for both ``setSync`` and ``setNoDelay`` for
+every future instance of ``WiFiClient`` (including those coming from
+``WiFiServer.available()`` by default).
+
+Default values are false for both ``NoDelay`` and ``Sync``.
+
+This means that Nagle is enabled by default *for all new connections*.
+
+getDefaultNoDelay and getDefaultSync
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Return the values to be used as default for NoDelay and Sync for all future connections.
+
 Other Function Calls
 ~~~~~~~~~~~~~~~~~~~~
 
@@ -54,7 +106,6 @@ Other Function Calls
     uint16_t  remotePort () 
     IPAddress  localIP () 
     uint16_t  localPort () 
-    bool  getNoDelay () 
 
 Documentation for the above functions is not yet prepared.
 

doc/esp8266wifi/server-class.rst

@@ -32,6 +32,10 @@ This algorithm is intended to reduce TCP/IP traffic of small packets sent over t
     server.begin();
     server.setNoDelay(true);
 
+By default, ``nodelay`` value will depends on global ``WiFiClient::getDefaultNoDelay()`` (currently false by default).
+
+However, a call to ``wiFiServer.setNoDelay()`` will override ``NoDelay`` for all new ``WiFiClient`` provided by the calling instance (``wiFiServer``).
+
 Other Function Calls
 ~~~~~~~~~~~~~~~~~~~~
 

libraries/ArduinoOTA/ArduinoOTA.cpp

@@ -290,6 +290,8 @@ void ArduinoOTAClass::_runUpdate() {
     }
     _state = OTA_IDLE;
   }
+  // OTA sends little packets
+  client.setNoDelay(true);
 
   uint32_t written, total = 0;
   while (!Update.isFinished() && client.connected()) {

libraries/ESP8266WiFi/src/WiFiClient.cpp

@@ -43,6 +43,34 @@ extern "C"
 
 uint16_t WiFiClient::_localPort = 0;
 
+static bool defaultNoDelay = false; // false == Nagle enabled by default
+static bool defaultSync = false;
+
+bool getDefaultPrivateGlobalSyncValue ()
+{
+    return defaultSync;
+}
+
+void WiFiClient::setDefaultNoDelay (bool noDelay)
+{
+    defaultNoDelay = noDelay;
+}
+
+void WiFiClient::setDefaultSync (bool sync)
+{
+    defaultSync = sync;
+}
+
+bool WiFiClient::getDefaultNoDelay ()
+{
+    return defaultNoDelay;
+}
+
+bool WiFiClient::getDefaultSync ()
+{
+    return defaultSync;
+}
+
 template<>
 WiFiClient* SList<WiFiClient>::_s_first = 0;
 
@@ -60,6 +88,9 @@ WiFiClient::WiFiClient(ClientContext* client)
     _timeout = 5000;
     _client->ref();
     WiFiClient::_add(this);
+
+    setSync(defaultSync);
+    setNoDelay(defaultNoDelay);
 }
 
 WiFiClient::~WiFiClient()
@@ -91,7 +122,6 @@ WiFiClient& WiFiClient::operator=(const WiFiClient& other)
     return *this;
 }
 
-
 int WiFiClient::connect(const char* host, uint16_t port)
 {
     IPAddress remote_addr;
@@ -147,6 +177,9 @@ int WiFiClient::connect(IPAddress ip, uint16_t port)
         return 0;
     }
 
+    setSync(defaultSync);
+    setNoDelay(defaultNoDelay);
+
     return 1;
 }
 
@@ -156,12 +189,26 @@ void WiFiClient::setNoDelay(bool nodelay) {
     _client->setNoDelay(nodelay);
 }
 
-bool WiFiClient::getNoDelay() {
+bool WiFiClient::getNoDelay() const {
     if (!_client)
         return false;
     return _client->getNoDelay();
 }
 
+void WiFiClient::setSync(bool sync)
+{
+    if (!_client)
+        return;
+    _client->setSync(sync);
+}
+
+bool WiFiClient::getSync() const
+{
+    if (!_client)
+        return false;
+    return _client->getSync();
+}
+
 size_t WiFiClient::availableForWrite ()
 {
     return _client? _client->availableForWrite(): 0;
@@ -264,19 +311,25 @@ size_t WiFiClient::peekBytes(uint8_t *buffer, size_t length) {
     return _client->peekBytes((char *)buffer, count);
 }
 
-void WiFiClient::flush()
+bool WiFiClient::flush(unsigned int maxWaitMs)
 {
-    if (_client)
-        _client->wait_until_sent();
+    if (!_client)
+        return true;
+
+    if (maxWaitMs == 0)
+        maxWaitMs = WIFICLIENT_MAX_FLUSH_WAIT_MS;
+    return _client->wait_until_sent(maxWaitMs);
 }
 
-void WiFiClient::stop()
+bool WiFiClient::stop(unsigned int maxWaitMs)
 {
     if (!_client)
-        return;
+        return true;
 
-    flush();
-    _client->close();
+    bool ret = flush(maxWaitMs); // virtual, may be ssl's
+    if (_client->close() != ERR_OK)
+        ret = false;
+    return ret;
 }
 
 uint8_t WiFiClient::connected()

libraries/ESP8266WiFi/src/WiFiClient.h

@@ -28,7 +28,12 @@
 #include "IPAddress.h"
 #include "include/slist.h"
 
-#define WIFICLIENT_MAX_PACKET_SIZE 1460
+#ifndef TCP_MSS
+#define TCP_MSS 1460 // lwip1.4
+#endif
+
+#define WIFICLIENT_MAX_PACKET_SIZE TCP_MSS
+#define WIFICLIENT_MAX_FLUSH_WAIT_MS 300
 
 #define TCP_DEFAULT_KEEPALIVE_IDLE_SEC          7200 // 2 hours
 #define TCP_DEFAULT_KEEPALIVE_INTERVAL_SEC      75   // 75 sec
@@ -67,17 +72,16 @@ class WiFiClient : public Client, public SList<WiFiClient> {
   size_t peekBytes(char *buffer, size_t length) {
     return peekBytes((uint8_t *) buffer, length);
   }
-  virtual void flush();
-  virtual void stop();
+  virtual bool flush(unsigned int maxWaitMs = 0);
+  virtual bool stop(unsigned int maxWaitMs = 0);
   virtual uint8_t connected();
   virtual operator bool();
 
   IPAddress remoteIP();
   uint16_t  remotePort();
   IPAddress localIP();
   uint16_t  localPort();
-  bool getNoDelay();
-  void setNoDelay(bool nodelay);
+
   static void setLocalPortStart(uint16_t port) { _localPort = port; }
 
   size_t availableForWrite();
@@ -96,6 +100,24 @@ class WiFiClient : public Client, public SList<WiFiClient> {
   uint8_t  getKeepAliveCount () const;
   void     disableKeepAlive () { keepAlive(0, 0, 0); }
 
+  // default NoDelay=False (Nagle=True=!NoDelay)
+  // Nagle is for shortly delaying outgoing data, to send less/bigger packets
+  // Nagle should be disabled for telnet-like/interactive streams
+  // Nagle is meaningless/ignored when Sync=true
+  static void setDefaultNoDelay (bool noDelay);
+  static bool getDefaultNoDelay ();
+  bool getNoDelay() const;
+  void setNoDelay(bool nodelay);
+
+  // default Sync=false
+  // When sync is true, all writes are automatically flushed.
+  // This is slower but also does not allocate
+  // temporary memory for sending data
+  static void setDefaultSync (bool sync);
+  static bool getDefaultSync ();
+  bool getSync() const;
+  void setSync(bool sync);
+
 protected:
 
   static int8_t _s_connected(void* arg, void* tpcb, int8_t err);

libraries/ESP8266WiFi/src/WiFiClientSecureAxTLS.cpp

@@ -271,12 +271,12 @@ uint8_t WiFiClientSecure::connected()
     return false;
 }
 
-void WiFiClientSecure::stop()
+bool WiFiClientSecure::stop(unsigned int maxWaitMs)
 {
     if (_ssl) {
         _ssl->stop();
     }
-    WiFiClient::stop();
+    return WiFiClient::stop(maxWaitMs);
 }
 
 static bool parseHexNibble(char pb, uint8_t* res)

libraries/ESP8266WiFi/src/WiFiClientSecureAxTLS.h

@@ -51,7 +51,7 @@ class WiFiClientSecure : public WiFiClient {
   int read() override;
   int peek() override;
   size_t peekBytes(uint8_t *buffer, size_t length) override;
-  void stop() override;
+  bool stop(unsigned int maxWaitMs = 0) override;
 
   bool setCACert(const uint8_t* pk, size_t size);
   bool setCertificate(const uint8_t* pk, size_t size);

libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp

@@ -175,23 +175,19 @@ void WiFiClientSecure::setBufferSizes(int recv, int xmit) {
   _iobuf_out_size = xmit;
 }
 
-void WiFiClientSecure::stop() {
-  flush();
-  if (_client) {
-    _client->wait_until_sent();
-    _client->abort();
-  }
-  WiFiClient::stop();
+bool WiFiClientSecure::stop(unsigned int maxWaitMs) {
+  bool ret = WiFiClient::stop(maxWaitMs); // calls our virtual flush()
   // Only if we've already connected, clear the connection options
   if (_handshake_done) {
     _clearAuthenticationSettings();
   }
   _freeSSL();
+  return ret;
 }
 
-void WiFiClientSecure::flush() {
+bool WiFiClientSecure::flush(unsigned int maxWaitMs) {
   (void) _run_until(BR_SSL_SENDAPP);
-  WiFiClient::flush();
+  return WiFiClient::flush(maxWaitMs);
 }
 
 int WiFiClientSecure::connect(IPAddress ip, uint16_t port) {

libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h

@@ -55,8 +55,8 @@ class WiFiClientSecure : public WiFiClient {
     int read() override;
     int peek() override;
     size_t peekBytes(uint8_t *buffer, size_t length) override;
-    void stop() override;
-    void flush() override;
+    bool flush(unsigned int maxWaitMs = 0) override;
+    bool stop(unsigned int maxWaitMs = 0) override;
 
     // Don't validate the chain, just accept whatever is given.  VERY INSECURE!
     void setInsecure() {

libraries/ESP8266WiFi/src/WiFiServer.cpp

@@ -88,11 +88,16 @@ void WiFiServer::begin(uint16_t port) {
 }
 
 void WiFiServer::setNoDelay(bool nodelay) {
-    _noDelay = nodelay;
+    _noDelay = nodelay? _ndTrue: _ndFalse;
 }
 
 bool WiFiServer::getNoDelay() {
-    return _noDelay;
+    switch (_noDelay)
+    {
+    case _ndFalse: return false;
+    case _ndTrue: return true;
+    default: return WiFiClient::getDefaultNoDelay();
+    }
 }
 
 bool WiFiServer::hasClient() {
@@ -106,7 +111,7 @@ WiFiClient WiFiServer::available(byte* status) {
     if (_unclaimed) {
         WiFiClient result(_unclaimed);
         _unclaimed = _unclaimed->next();
-        result.setNoDelay(_noDelay);
+        result.setNoDelay(getNoDelay());
         DEBUGV("WS:av\r\n");
         return result;
     }

libraries/ESP8266WiFi/src/WiFiServer.h

@@ -43,7 +43,7 @@ class WiFiServer : public Server {
 
   ClientContext* _unclaimed;
   ClientContext* _discarded;
-  bool _noDelay = false;
+  enum { _ndDefault, _ndFalse, _ndTrue } _noDelay = _ndDefault;
 
 public:
   WiFiServer(IPAddress addr, uint16_t port);