fixes for WiFiClient::write(Stream) (#7987)

fixes for WiFiClient::write(Stream) and Stream transfers
- remove deprecated WiFiClient::write(Stream,size)
- fix and deprecate WiFiClient::write(Stream) to use Stream::sendAll instead of ::sendAvailable
- update ESP8266WebServer::streamFile to use file.sendAll(client) instead of client.write(file)
- remove stream dependence in ClientContext
- Stream::send(): honor timeout in all case, avoid short transfer when output is temporarily full
- example WiFiEcho: show sendAll and sendAvailable

Author: d-a-v

cores/esp8266/StreamSend.cpp

@@ -104,10 +104,7 @@ size_t Stream::SendGenericPeekBuffer(Print* to, const ssize_t len, const int rea
             {
                 peekConsume(w);
                 written += w;
-                if (maxLen)
-                {
-                    timedOut.reset();
-                }
+                timedOut.reset(); // something has been written
             }
             if (foundChar)
             {
@@ -116,12 +113,6 @@ size_t Stream::SendGenericPeekBuffer(Print* to, const ssize_t len, const int rea
             }
         }
 
-        if (!w && !maxLen && readUntilChar < 0)
-        {
-            // nothing has been transferred and no specific condition is requested
-            break;
-        }
-
         if (timedOut)
         {
             // either (maxLen>0) nothing has been transferred for too long
@@ -195,16 +186,7 @@ size_t Stream::SendGenericRegularUntil(Print* to, const ssize_t len, const int r
                 break;
             }
             written += 1;
-            if (maxLen)
-            {
-                timedOut.reset();
-            }
-        }
-
-        if (!w && !maxLen && readUntilChar < 0)
-        {
-            // nothing has been transferred and no specific condition is requested
-            break;
+            timedOut.reset(); // something has been written
         }
 
         if (timedOut)
@@ -288,16 +270,7 @@ size_t Stream::SendGenericRegular(Print* to, const ssize_t len, const esp8266::p
                 setReport(Report::WriteError);
                 break;
             }
-            if (maxLen && w)
-            {
-                timedOut.reset();
-            }
-        }
-
-        if (!w && !maxLen)
-        {
-            // nothing has been transferred and no specific condition is requested
-            break;
+            timedOut.reset(); // something has been written
         }
 
         if (timedOut)

libraries/ESP8266WebServer/src/ESP8266WebServer.h

@@ -226,7 +226,7 @@ class ESP8266WebServerTemplate
     size_t contentLength = 0;
     _streamFileCore(file.size(), file.name(), contentType);
     if (requestMethod == HTTP_GET) {
-      contentLength = _currentClient.write(file);
+      contentLength = file.sendAll(_currentClient);
     }
     return contentLength;
   }

libraries/ESP8266WiFi/examples/WiFiEcho/WiFiEcho.ino

@@ -84,9 +84,10 @@ void loop() {
   if (Serial.available()) {
     s = (s + 1) % (sizeof(sizes) / sizeof(sizes[0]));
     switch (Serial.read()) {
-      case '1': if (t != 1) s = 0; t = 1; Serial.println("byte-by-byte (watch then press 2 or 3)"); break;
-      case '2': if (t != 2) s = 1; t = 2; Serial.printf("through buffer (watch then press 2 again, or 1 or 3)\n"); break;
-      case '3': if (t != 3) s = 0; t = 3; Serial.printf("direct access (watch then press 3 again, or 1 or 2)\n"); break;
+      case '1': if (t != 1) s = 0; t = 1; Serial.println("byte-by-byte (watch then press 2, 3 or 4)"); break;
+      case '2': if (t != 2) s = 1; t = 2; Serial.printf("through buffer (watch then press 2 again, or 1, 3 or 4)\n"); break;
+      case '3': if (t != 3) s = 0; t = 3; Serial.printf("direct access (sendAvailable - watch then press 3 again, or 1, 2 or 4)\n"); break;
+      case '4':                    t = 4; Serial.printf("direct access (sendAll - close peer to stop, then press 1, 2 or 3 before restarting peer)\n"); break;
     }
     tot = cnt = 0;
     ESP.resetFreeContStack();
@@ -125,7 +126,7 @@ void loop() {
     if (sizes[s]) {
       tot += client.sendSize(&client, sizes[s]);
     } else {
-      tot += client.sendAll(&client);
+      tot += client.sendAvailable(&client);
     }
     cnt++;
 
@@ -138,4 +139,18 @@ void loop() {
     }
   }
 
+  else if (t == 4) {
+    // stream to print, possibly with only one copy
+    tot += client.sendAll(&client); // this one might not exit until peer close
+    cnt++;
+
+    switch (client.getLastSendReport()) {
+      case Stream::Report::Success: break;
+      case Stream::Report::TimedOut: Serial.println("Stream::send: timeout"); break;
+      case Stream::Report::ReadError: Serial.println("Stream::send: read error"); break;
+      case Stream::Report::WriteError: Serial.println("Stream::send: write error"); break;
+      case Stream::Report::ShortOperation: Serial.println("Stream::send: short transfer"); break;
+    }
+  }
+
 }

libraries/ESP8266WiFi/src/WiFiClient.cpp

@@ -213,28 +213,19 @@ size_t WiFiClient::write(const uint8_t *buf, size_t size)
         return 0;
     }
     _client->setTimeout(_timeout);
-    StreamConstPtr ptr(buf, size);
-    return _client->write(ptr);
-}
-
-size_t WiFiClient::write(Stream& stream, size_t unused)
-{
-    (void) unused;
-    return WiFiClient::write(stream);
+    return _client->write((const char*)buf, size);
 }
 
 size_t WiFiClient::write(Stream& stream)
 {
+    // (this method is deprecated)
+
     if (!_client || !stream.available())
     {
         return 0;
     }
-    if (stream.hasPeekBufferAPI())
-    {
-        _client->setTimeout(_timeout);
-        return _client->write(stream);
-    }
-    return stream.sendAvailable(this);
+    // core up to 2.7.4 was equivalent to this
+    return stream.sendAll(this);
 }
 
 size_t WiFiClient::write_P(PGM_P buf, size_t size)

libraries/ESP8266WiFi/src/WiFiClient.h

@@ -59,10 +59,7 @@ class WiFiClient : public Client, public SList<WiFiClient> {
   virtual size_t write(uint8_t) override;
   virtual size_t write(const uint8_t *buf, size_t size) override;
   virtual size_t write_P(PGM_P buf, size_t size);
-  size_t write(Stream& stream);
-
-  // This one is deprecated, use write(Stream& instead)
-  size_t write(Stream& stream, size_t unitSize) __attribute__ ((deprecated));
+  size_t write(Stream& stream) [[ deprecated("use stream.sendHow(client...)") ]];
 
   virtual int available() override;
   virtual int read() override;

libraries/ESP8266WiFi/src/include/ClientContext.h

@@ -30,7 +30,6 @@ extern "C" void esp_yield();
 extern "C" void esp_schedule();
 
 #include <assert.h>
-#include <StreamDev.h>
 #include <esp_priv.h>
 
 bool getDefaultPrivateGlobalSyncValue ();
@@ -376,13 +375,12 @@ class ClientContext
         return _pcb->state;
     }
 
-    size_t write(Stream& stream)
+    size_t write(const char* ds, const size_t dl)
     {
         if (!_pcb) {
             return 0;
         }
-        assert(stream.hasPeekBufferAPI());
-        return _write_from_source(&stream);
+        return _write_from_source(ds, dl);
     }
 
     void keepAlive (uint16_t idle_sec = TCP_DEFAULT_KEEPALIVE_IDLE_SEC, uint16_t intv_sec = TCP_DEFAULT_KEEPALIVE_INTERVAL_SEC, uint8_t count = TCP_DEFAULT_KEEPALIVE_COUNT)
@@ -466,23 +464,25 @@ class ClientContext
         }
     }
 
-    size_t _write_from_source(Stream* ds)
+    size_t _write_from_source(const char* ds, const size_t dl)
     {
         assert(_datasource == nullptr);
         assert(!_send_waiting);
         _datasource = ds;
+        _datalen = dl;
         _written = 0;
         _op_start_time = millis();
         do {
             if (_write_some()) {
                 _op_start_time = millis();
             }
 
-            if (!_datasource->available() || _is_timeout() || state() == CLOSED) {
+            if (_written == _datalen || _is_timeout() || state() == CLOSED) {
                 if (_is_timeout()) {
                     DEBUGV(":wtmo\r\n");
                 }
                 _datasource = nullptr;
+                _datalen = 0;
                 break;
             }
 
@@ -507,20 +507,21 @@ class ClientContext
             return false;
         }
 
-        DEBUGV(":wr %d %d\r\n", _datasource->peekAvailable(), _written);
+        DEBUGV(":wr %d %d\r\n", _datalen - _written, _written);
 
         bool has_written = false;
 
-        while (_datasource) {
+        while (_written < _datalen) {
             if (state() == CLOSED)
                 return false;
-            size_t next_chunk_size = std::min((size_t)tcp_sndbuf(_pcb), _datasource->peekAvailable());
+            const auto remaining = _datalen - _written;
+            size_t next_chunk_size = std::min((size_t)tcp_sndbuf(_pcb), remaining);
             if (!next_chunk_size)
                 break;
-            const char* buf = _datasource->peekBuffer();
+            const char* buf = _datasource + _written;
 
             uint8_t flags = 0;
-            if (next_chunk_size < _datasource->peekAvailable())
+            if (next_chunk_size < remaining)
                 //   PUSH is meant for peer, telling to give data to user app as soon as received
                 //   PUSH "may be set" when sender has finished sending a "meaningful" data block
                 //   PUSH does not break Nagle
@@ -534,10 +535,9 @@ class ClientContext
 
             err_t err = tcp_write(_pcb, buf, next_chunk_size, flags);
 
-            DEBUGV(":wrc %d %d %d\r\n", next_chunk_size, _datasource->peekAvailable(), (int)err);
+            DEBUGV(":wrc %d %d %d\r\n", next_chunk_size, remaining, (int)err);
 
             if (err == ERR_OK) {
-                _datasource->peekConsume(next_chunk_size);
                 _written += next_chunk_size;
                 has_written = true;
             } else {
@@ -695,7 +695,8 @@ class ClientContext
     discard_cb_t _discard_cb;
     void* _discard_cb_arg;
 
-    Stream* _datasource = nullptr;
+    const char* _datasource = nullptr;
+    size_t _datalen = 0;
     size_t _written = 0;
     uint32_t _timeout_ms = 5000;
     uint32_t _op_start_time = 0;

tests/host/common/include/ClientContext.h

@@ -223,9 +223,9 @@ class ClientContext
         return _sock >= 0? ESTABLISHED: CLOSED;
     }
 
-    size_t write(const uint8_t* data, size_t size)
+    size_t write(const char* data, size_t size)
     {
-	ssize_t ret = mockWrite(_sock, data, size, _timeout_ms);
+	ssize_t ret = mockWrite(_sock, (const uint8_t*)data, size, _timeout_ms);
 	if (ret < 0)
 	{
 	    abort();
@@ -234,28 +234,6 @@ class ClientContext
 	return ret;
     }
 
-    size_t write(Stream& stream)
-    {
-        size_t avail = stream.available();
-        uint8_t buf [avail];
-        avail = stream.readBytes(buf, avail);
-        size_t totwrote = 0;
-        uint8_t* w = buf;
-        while (avail && _sock >= 0)
-        {
-            size_t wrote = write(w, avail);
-            w += wrote;
-            avail -= wrote;
-            totwrote += wrote;
-    	}
-        return totwrote;
-    }
-
-    size_t write_P(PGM_P buf, size_t size)
-    {
-        return write((const uint8_t*)buf, size);
-    }
-
     void keepAlive (uint16_t idle_sec = TCP_DEFAULT_KEEPALIVE_IDLE_SEC, uint16_t intv_sec = TCP_DEFAULT_KEEPALIVE_INTERVAL_SEC, uint8_t count = TCP_DEFAULT_KEEPALIVE_COUNT)
     {
         (void) idle_sec;