From 83a8076db87b77de6a7b500c18b85057e38ed117 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Tue, 25 Sep 2018 15:47:27 +0200 Subject: [PATCH] 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 --- cores/esp8266/Client.h | 4 +- doc/esp8266wifi/client-class.rst | 53 +++++- doc/esp8266wifi/server-class.rst | 4 + libraries/ArduinoOTA/ArduinoOTA.cpp | 2 + libraries/ESP8266WiFi/src/WiFiClient.cpp | 71 ++++++-- libraries/ESP8266WiFi/src/WiFiClient.h | 32 +++- .../ESP8266WiFi/src/WiFiClientSecureAxTLS.cpp | 4 +- .../ESP8266WiFi/src/WiFiClientSecureAxTLS.h | 2 +- .../src/WiFiClientSecureBearSSL.cpp | 14 +- .../ESP8266WiFi/src/WiFiClientSecureBearSSL.h | 4 +- libraries/ESP8266WiFi/src/WiFiServer.cpp | 11 +- libraries/ESP8266WiFi/src/WiFiServer.h | 2 +- .../ESP8266WiFi/src/include/ClientContext.h | 153 ++++++++++++------ libraries/Ethernet/src/EthernetClient.cpp | 20 ++- libraries/Ethernet/src/EthernetClient.h | 4 +- 15 files changed, 287 insertions(+), 93 deletions(-) diff --git a/cores/esp8266/Client.h b/cores/esp8266/Client.h index d776a2e16d..3cab9f2d69 100644 --- a/cores/esp8266/Client.h +++ b/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: diff --git a/doc/esp8266wifi/client-class.rst b/doc/esp8266wifi/client-class.rst index 9ddd46b458..2f153b254d 100644 --- a/doc/esp8266wifi/client-class.rst +++ b/doc/esp8266wifi/client-class.rst @@ -18,6 +18,17 @@ Methods documented for `Client `__ 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. diff --git a/doc/esp8266wifi/server-class.rst b/doc/esp8266wifi/server-class.rst index 6b3512959c..8855083ab9 100644 --- a/doc/esp8266wifi/server-class.rst +++ b/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 ~~~~~~~~~~~~~~~~~~~~ diff --git a/libraries/ArduinoOTA/ArduinoOTA.cpp b/libraries/ArduinoOTA/ArduinoOTA.cpp index 24be99d374..b6d1a915e6 100644 --- a/libraries/ArduinoOTA/ArduinoOTA.cpp +++ b/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()) { diff --git a/libraries/ESP8266WiFi/src/WiFiClient.cpp b/libraries/ESP8266WiFi/src/WiFiClient.cpp index f266597a28..6bb79a4a9b 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.cpp +++ b/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::_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() diff --git a/libraries/ESP8266WiFi/src/WiFiClient.h b/libraries/ESP8266WiFi/src/WiFiClient.h index ac37b181ee..7f9d02ee51 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.h +++ b/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,8 +72,8 @@ class WiFiClient : public Client, public SList { 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(); @@ -76,8 +81,7 @@ class WiFiClient : public Client, public SList { 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 { 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); diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureAxTLS.cpp b/libraries/ESP8266WiFi/src/WiFiClientSecureAxTLS.cpp index d200d51e59..d275f2aacf 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureAxTLS.cpp +++ b/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) diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureAxTLS.h b/libraries/ESP8266WiFi/src/WiFiClientSecureAxTLS.h index e836d7bd5f..fabfb5e4ef 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureAxTLS.h +++ b/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); diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp index 44471d640c..c2567b7303 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp +++ b/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) { diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h index aff2935c80..7d8d657ec7 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h +++ b/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() { diff --git a/libraries/ESP8266WiFi/src/WiFiServer.cpp b/libraries/ESP8266WiFi/src/WiFiServer.cpp index 16f5bcc754..d1178d52d9 100644 --- a/libraries/ESP8266WiFi/src/WiFiServer.cpp +++ b/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; } diff --git a/libraries/ESP8266WiFi/src/WiFiServer.h b/libraries/ESP8266WiFi/src/WiFiServer.h index ecf251be5a..95e7e5a904 100644 --- a/libraries/ESP8266WiFi/src/WiFiServer.h +++ b/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); diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h index eb8a286d6d..86327dc403 100644 --- a/libraries/ESP8266WiFi/src/include/ClientContext.h +++ b/libraries/ESP8266WiFi/src/include/ClientContext.h @@ -31,11 +31,14 @@ extern "C" void esp_schedule(); #include "DataSource.h" +bool getDefaultPrivateGlobalSyncValue (); + class ClientContext { public: ClientContext(tcp_pcb* pcb, discard_cb_t discard_cb, void* discard_cb_arg) : - _pcb(pcb), _rx_buf(0), _rx_buf_offset(0), _discard_cb(discard_cb), _discard_cb_arg(discard_cb_arg), _refcnt(0), _next(0) + _pcb(pcb), _rx_buf(0), _rx_buf_offset(0), _discard_cb(discard_cb), _discard_cb_arg(discard_cb_arg), _refcnt(0), _next(0), + _sync(::getDefaultPrivateGlobalSyncValue()) { tcp_setprio(pcb, TCP_PRIO_MIN); tcp_arg(pcb, this); @@ -44,7 +47,7 @@ class ClientContext tcp_err(pcb, &_s_error); tcp_poll(pcb, &_s_poll, 1); - // not enabled by default for 2.4.0 + // keep-alive not enabled by default //keepAlive(); } @@ -159,7 +162,7 @@ class ClientContext } } - bool getNoDelay() + bool getNoDelay() const { if(!_pcb) { return false; @@ -167,17 +170,17 @@ class ClientContext return tcp_nagle_disabled(_pcb); } - void setTimeout(int timeout_ms) + void setTimeout(int timeout_ms) { _timeout_ms = timeout_ms; } - int getTimeout() + int getTimeout() const { return _timeout_ms; } - uint32_t getRemoteAddress() + uint32_t getRemoteAddress() const { if(!_pcb) { return 0; @@ -186,7 +189,7 @@ class ClientContext return _pcb->remote_ip.addr; } - uint16_t getRemotePort() + uint16_t getRemotePort() const { if(!_pcb) { return 0; @@ -195,7 +198,7 @@ class ClientContext return _pcb->remote_port; } - uint32_t getLocalAddress() + uint32_t getLocalAddress() const { if(!_pcb) { return 0; @@ -204,7 +207,7 @@ class ClientContext return _pcb->local_ip.addr; } - uint16_t getLocalPort() + uint16_t getLocalPort() const { if(!_pcb) { return 0; @@ -257,7 +260,7 @@ class ClientContext return size_read; } - char peek() + char peek() const { if(!_rx_buf) { return 0; @@ -266,7 +269,7 @@ class ClientContext return reinterpret_cast(_rx_buf->payload)[_rx_buf_offset]; } - size_t peekBytes(char *dst, size_t size) + size_t peekBytes(char *dst, size_t size) const { if(!_rx_buf) { return 0; @@ -296,20 +299,48 @@ class ClientContext _rx_buf_offset = 0; } - void wait_until_sent() + bool wait_until_sent(int max_wait_ms = WIFICLIENT_MAX_FLUSH_WAIT_MS) { - // fix option 1 in // https://github.com/esp8266/Arduino/pull/3967#pullrequestreview-83451496 - // TODO: option 2 + // option 1 done + // option 2 / _write_some() not necessary since _datasource is always nullptr here + + if (!_pcb) + return true; + + int loop = -1; + int prevsndbuf = -1; + max_wait_ms++; + + // wait for peer's acks to flush lwIP's output buffer + + while (1) { + + // force lwIP to send what can be sent + tcp_output(_pcb); - #define WAIT_TRIES_MS 10 // at most 10ms + int sndbuf = tcp_sndbuf(_pcb); + if (sndbuf != prevsndbuf) { + // send buffer has changed (or first iteration) + // we received an ack: restart the loop counter + prevsndbuf = sndbuf; + loop = max_wait_ms; + } + + if (state() != ESTABLISHED || sndbuf == TCP_SND_BUF || --loop <= 0) + break; - int tries = 1+ WAIT_TRIES_MS; + delay(1); + } - while (state() == ESTABLISHED && tcp_sndbuf(_pcb) != TCP_SND_BUF && --tries) { - _write_some(); - delay(1); // esp_ schedule+yield + #ifdef DEBUGV + if (loop <= 0) { + // wait until sent: timeout + DEBUGV(":wustmo\n"); } + #endif + + return max_wait_ms > 0; } uint8_t state() const @@ -321,7 +352,6 @@ class ClientContext return _pcb->state; } - size_t write(const uint8_t* data, size_t size) { if (!_pcb) { @@ -379,6 +409,16 @@ class ClientContext return isKeepAliveEnabled()? _pcb->keep_cnt: 0; } + bool getSync () const + { + return _sync; + } + + void setSync (bool sync) + { + _sync = sync; + } + protected: bool _is_timeout() @@ -418,6 +458,10 @@ class ClientContext esp_yield(); } while(true); _send_waiting = 0; + + if (_sync) + wait_until_sent(); + return _written; } @@ -427,40 +471,50 @@ class ClientContext return false; } - size_t left = _datasource->available(); - size_t can_send = tcp_sndbuf(_pcb); - if (_pcb->snd_queuelen >= TCP_SND_QUEUELEN) { - can_send = 0; - } - size_t will_send = (can_send < left) ? can_send : left; - DEBUGV(":wr %d %d %d\r\n", will_send, left, _written); - bool need_output = false; - while( will_send && _datasource) { - size_t next_chunk = - will_send > _write_chunk_size ? _write_chunk_size : will_send; - const uint8_t* buf = _datasource->get_buffer(next_chunk); - if (state() == CLOSED) { - need_output = false; + DEBUGV(":wr %d %d\r\n", _datasource->available(), _written); + + bool has_written = false; + + while (_datasource) { + if (state() == CLOSED) + return false; + size_t next_chunk_size = std::min((size_t)tcp_sndbuf(_pcb), _datasource->available()); + if (!next_chunk_size) break; - } - err_t err = tcp_write(_pcb, buf, next_chunk, TCP_WRITE_FLAG_COPY); - DEBUGV(":wrc %d %d %d\r\n", next_chunk, will_send, (int) err); + const uint8_t* buf = _datasource->get_buffer(next_chunk_size); + // use TCP_WRITE_FLAG_MORE to remove PUSH flag from packet (lwIP's doc), + // because PUSH code implicitely disables Nagle code (see lwIP's tcp_out.c) + // Notes: + // 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 is quite unclear in its application + // Nagle is for shortly delaying outgoing data, to send less/bigger packets + uint8_t flags = TCP_WRITE_FLAG_MORE; // do not tcp-PuSH + if (!_sync) + // user data must be copied when data are sent but not yet acknowledged + // (with sync, we wait for acknowledgment before returning to user) + flags |= TCP_WRITE_FLAG_COPY; + err_t err = tcp_write(_pcb, buf, next_chunk_size, flags); + DEBUGV(":wrc %d %d %d\r\n", next_chunk_size, _datasource->available(), (int)err); if (err == ERR_OK) { - _datasource->release_buffer(buf, next_chunk); - _written += next_chunk; - need_output = true; + _datasource->release_buffer(buf, next_chunk_size); + _written += next_chunk_size; + has_written = true; } else { // ERR_MEM(-1) is a valid error meaning // "come back later". It leaves state() opened break; } - will_send -= next_chunk; } - if( need_output ) { + + if (has_written && (_sync || tcp_nagle_disabled(_pcb))) + { + // handle no-Nagle manually because of TCP_WRITE_FLAG_MORE + // lwIP's tcp_output doc: "Find out what we can send and send it" tcp_output(_pcb); - return true; } - return false; + + return has_written; } void _write_some_from_cb() @@ -482,14 +536,13 @@ class ClientContext void _consume(size_t size) { + if(_pcb) + tcp_recved(_pcb, size); ptrdiff_t left = _rx_buf->len - _rx_buf_offset - size; if(left > 0) { _rx_buf_offset += size; } else if(!_rx_buf->next) { DEBUGV(":c0 %d, %d\r\n", size, _rx_buf->tot_len); - if(_pcb) { - tcp_recved(_pcb, _rx_buf->len); - } pbuf_free(_rx_buf); _rx_buf = 0; _rx_buf_offset = 0; @@ -499,9 +552,6 @@ class ClientContext _rx_buf = _rx_buf->next; _rx_buf_offset = 0; pbuf_ref(_rx_buf); - if(_pcb) { - tcp_recved(_pcb, head->len); - } pbuf_free(head); } } @@ -592,7 +642,6 @@ class ClientContext DataSource* _datasource = nullptr; size_t _written = 0; - size_t _write_chunk_size = 256; uint32_t _timeout_ms = 5000; uint32_t _op_start_time = 0; uint8_t _send_waiting = 0; @@ -600,6 +649,8 @@ class ClientContext int8_t _refcnt; ClientContext* _next; + + bool _sync; }; #endif//CLIENTCONTEXT_H diff --git a/libraries/Ethernet/src/EthernetClient.cpp b/libraries/Ethernet/src/EthernetClient.cpp index 1feed4c424..7e9dc9abbd 100644 --- a/libraries/Ethernet/src/EthernetClient.cpp +++ b/libraries/Ethernet/src/EthernetClient.cpp @@ -119,13 +119,15 @@ int EthernetClient::peek() { return b; } -void EthernetClient::flush() { +bool EthernetClient::flush(unsigned int maxWaitMs) { + (void)maxWaitMs; ::flush(_sock); + return true; } -void EthernetClient::stop() { +bool EthernetClient::stop(unsigned int maxWaitMs) { if (_sock == MAX_SOCK_NUM) - return; + return true; // attempt to close the connection gracefully (send a FIN to other side) disconnect(_sock); @@ -133,19 +135,27 @@ void EthernetClient::stop() { // wait up to a second for the connection to close uint8_t s; + if (maxWaitMs == 0) + maxWaitMs = 1000; do { s = status(); if (s == SnSR::CLOSED) break; // exit the loop delay(1); - } while (millis() - start < 1000); + } while (millis() - start < maxWaitMs); + + bool ret = true; // if it hasn't closed, close it forcefully - if (s != SnSR::CLOSED) + if (s != SnSR::CLOSED) { + ret = false; close(_sock); + } EthernetClass::_server_port[_sock] = 0; _sock = MAX_SOCK_NUM; + + return ret; } uint8_t EthernetClient::connected() { diff --git a/libraries/Ethernet/src/EthernetClient.h b/libraries/Ethernet/src/EthernetClient.h index 16e2500bc3..ff4198ff17 100644 --- a/libraries/Ethernet/src/EthernetClient.h +++ b/libraries/Ethernet/src/EthernetClient.h @@ -20,8 +20,8 @@ class EthernetClient : public Client { virtual int read(); virtual int read(uint8_t *buf, size_t size); virtual int peek(); - 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(); virtual bool operator==(const bool value) { return bool() == value; }