From 8bcf5fdc1487560df1d41352045e830685cf523a Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Wed, 2 Oct 2019 04:49:53 +0300 Subject: [PATCH 1/9] telnet: buffered output for async server --- code/espurna/config/prototypes.h | 6 + code/espurna/telnet.ino | 219 ++++++++++++++++++++++++------- 2 files changed, 178 insertions(+), 47 deletions(-) diff --git a/code/espurna/config/prototypes.h b/code/espurna/config/prototypes.h index 265a14f6c6..4b1318aa0c 100644 --- a/code/espurna/config/prototypes.h +++ b/code/espurna/config/prototypes.h @@ -32,6 +32,12 @@ extern "C" { #include // LWIP_VERSION_MAJOR } +// ref: https://github.com/me-no-dev/ESPAsyncTCP/pull/115/files#diff-e2e636049095cc1ff920c1bfabf6dcacR8 +// This is missing with Core 2.3.0 and is sometimes missing from the build flags. Assume HIGH_BANDWIDTH version. +#ifndef TCP_MSS +#define TCP_MSS (1460) +#endif + uint32_t systemResetReason(); uint8_t systemStabilityCounter(); void systemStabilityCounter(uint8_t); diff --git a/code/espurna/telnet.ino b/code/espurna/telnet.ino index 3ab088d0c4..b1bd342f17 100644 --- a/code/espurna/telnet.ino +++ b/code/espurna/telnet.ino @@ -14,15 +14,50 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer #define TELNET_XEOF 0xEC #if TELNET_SERVER == TELNET_SERVER_WIFISERVER - #include - WiFiServer _telnetServer = WiFiServer(TELNET_PORT); - std::unique_ptr _telnetClients[TELNET_MAX_CLIENTS]; + using TServer = WiFiServer; + using TClient = WiFiClient; #else #include - AsyncServer _telnetServer = AsyncServer(TELNET_PORT); - std::unique_ptr _telnetClients[TELNET_MAX_CLIENTS]; + #include + #include + using TServer = AsyncServer; + + struct AsyncTelnetClient { + using container_t = std::vector; + + AsyncTelnetClient(unsigned char id, AsyncClient* client) : + _id(id), + _client(client) + { + _client->onAck(_s_onAck, this); + _client->onPoll(_s_onPoll, this); + } + + static void _trySend(AsyncTelnetClient* client); + static void _s_onAck(void* client_ptr, AsyncClient*, size_t, uint32_t); + static void _s_onPoll(void* client_ptr, AsyncClient* client); + + size_t write(char c); + size_t write(const char* data, size_t size=0); + + void flush(); + size_t available(); + + void close(bool now = false); + bool connected(); + + unsigned char _id; + AsyncClient* _client; + + container_t _current; + std::deque _queue; + }; + using TClient = AsyncTelnetClient; #endif +TServer _telnetServer(TELNET_PORT); +std::unique_ptr _telnetClients[TELNET_MAX_CLIENTS]; + bool _telnetAuth = TELNET_AUTHENTICATION; bool _telnetClientsAuth[TELNET_MAX_CLIENTS]; @@ -43,24 +78,122 @@ void _telnetWebSocketOnConnected(JsonObject& root) { #endif +#if TELNET_SERVER == TELNET_SERVER_WIFISERVER + void _telnetDisconnect(unsigned char clientId) { - // ref: we are called from onDisconnect, async is already stopped - #if TELNET_SERVER == TELNET_SERVER_WIFISERVER - _telnetClients[clientId]->stop(); - #endif + _telnetClients[clientId]->stop(); _telnetClients[clientId] = nullptr; wifiReconnectCheck(); DEBUG_MSG_P(PSTR("[TELNET] Client #%d disconnected\n"), clientId); } -bool _telnetWrite(unsigned char clientId, const char *data, size_t len) { +#elif TELNET_SERVER == TELNET_SERVER_ASYNC + +void _telnetCleanUp() { + schedule_function([] () { + for (unsigned char clientId=0; clientId < TELNET_MAX_CLIENTS; ++clientId) { + if (!_telnetClients[clientId]->connected()) { + _telnetClients[clientId] = nullptr; + wifiReconnectCheck(); + DEBUG_MSG_P(PSTR("[TELNET] Client #%d disconnected\n"), clientId); + } + } + }); +} + +// just mark as closed, clean-up method above will destroy the object later +void _telnetDisconnect(unsigned char clientId) { + _telnetClients[clientId]->close(); +} + +void AsyncTelnetClient::_trySend(AsyncTelnetClient* client) { + if (!client->_queue.empty()) { + auto& chunk = client->_queue.back(); + if (client->_client->space() >= chunk.size()) { + client->_client->write((const char*)chunk.data(), chunk.size()); + client->_queue.pop_back(); + } + return; + } + + const auto current_size = client->_current.size(); + if (current_size) { + if (client->_client->space() >= current_size) { + client->_client->write((const char*)client->_current.data(), client->_current.size()); + client->_current.clear(); + } + return; + } +} + +void AsyncTelnetClient::_s_onAck(void* client_ptr, AsyncClient*, size_t, uint32_t) { + _trySend(reinterpret_cast(client_ptr)); +} + +void AsyncTelnetClient::_s_onPoll(void* client_ptr, AsyncClient* client) { + _trySend(reinterpret_cast(client_ptr)); +} + +size_t AsyncTelnetClient::write(const char* data, size_t size) { + + const size_t written = _client->add(data, size); + if (written == size) return size; + + const size_t full_size = size; + char* data_ptr = const_cast(data + written); + size -= written; + + _current.reserve(TCP_MSS); + + while (size) { + const auto have = _current.capacity() - _current.size(); + if (have >= size) { + _current.insert(_current.end(), data_ptr, data_ptr + size); + size = 0; + } else { + _current.insert(_current.end(), data_ptr, data_ptr + have); + _queue.push_front(_current); + _current.clear(); + data_ptr += have; + size -= have; + } + } + + return full_size; + +} + +size_t AsyncTelnetClient::write(char c) { + char _c[1] {c}; + return write(_c, 1); +} + +void AsyncTelnetClient::flush() { + _client->send(); +} + +size_t AsyncTelnetClient::available() { + return _client->space(); +} + +void AsyncTelnetClient::close(bool now) { + _client->close(now); +} + +bool AsyncTelnetClient::connected() { + return _client->connected(); +} + +#endif // TELNET_SERVER == TELNET_SERVER_WIFISERVER + +size_t _telnetWrite(unsigned char clientId, const char *data, size_t len) { if (_telnetClients[clientId] && _telnetClients[clientId]->connected()) { - return (_telnetClients[clientId]->write(data, len) > 0); + return _telnetClients[clientId]->write(data, len); } - return false; + return 0; } -unsigned char _telnetWrite(const char *data, size_t len) { +size_t _telnetWrite(const char *data, size_t len) { unsigned char count = 0; for (unsigned char i = 0; i < TELNET_MAX_CLIENTS; i++) { // Do not send broadcast messages to unauthenticated clients @@ -73,28 +206,30 @@ unsigned char _telnetWrite(const char *data, size_t len) { return count; } -unsigned char _telnetWrite(const char *data) { +size_t _telnetWrite(const char *data) { return _telnetWrite(data, strlen(data)); } -bool _telnetWrite(unsigned char clientId, const char * message) { +size_t _telnetWrite(unsigned char clientId, const char * message) { return _telnetWrite(clientId, message, strlen(message)); } -void _telnetData(unsigned char clientId, void *data, size_t len) { - // Capture close connection - char * p = (char *) data; +void _telnetData(unsigned char clientId, char * data, size_t len) { - if ((len >= 2) && (p[0] == TELNET_IAC)) { + if ((len >= 2) && (data[0] == TELNET_IAC)) { // C-d is sent as two bytes (sometimes repeating) - if (p[1] == TELNET_XEOF) { + if (data[1] == TELNET_XEOF) { _telnetDisconnect(clientId); } return; // Ignore telnet negotiation } - if ((strncmp(p, "close", 5) == 0) || (strncmp(p, "quit", 4) == 0)) { - _telnetDisconnect(clientId); + if ((strncmp(data, "close", 5) == 0) || (strncmp(data, "quit", 4) == 0)) { + #if TELNET_SERVER == TELNET_SERVER_WIFISERVER + _telnetDisconnect(clientId); + #else + _telnetClients[clientId]->close(); + #endif return; } @@ -107,7 +242,7 @@ void _telnetData(unsigned char clientId, void *data, size_t len) { if (_telnetAuth && !authenticated) { String password = getAdminPass(); - if (strncmp(p, password.c_str(), password.length()) == 0) { + if (strncmp(data, password.c_str(), password.length()) == 0) { DEBUG_MSG_P(PSTR("[TELNET] Client #%d authenticated\n"), clientId); _telnetWrite(clientId, "Password correct, welcome!\n"); _telnetClientsAuth[clientId] = true; @@ -119,7 +254,7 @@ void _telnetData(unsigned char clientId, void *data, size_t len) { // Inject command #if TERMINAL_SUPPORT - terminalInject(data, len); + terminalInject((void*)data, len); #endif } @@ -202,7 +337,7 @@ void _telnetLoop() { while (_telnetClients[i] && _telnetClients[i]->available()) { char data[TERMINAL_BUFFER_SIZE]; size_t len = _telnetClients[i]->available(); - unsigned int r = _telnetClients[i]->readBytes(data, min(sizeof(data), len)); + unsigned int r = _telnetClients[i]->readBytes(data, std::min(sizeof(data), len)); _telnetData(i, data, r); } @@ -211,7 +346,7 @@ void _telnetLoop() { } } -#else // TELNET_SERVER_ASYNC +#elif TELNET_SERVER == TELNET_SERVER_ASYNC void _telnetNewClient(AsyncClient* client) { if (client->localIP() != WiFi.softAPIP()) { @@ -236,29 +371,19 @@ void _telnetNewClient(AsyncClient* client) { if (!_telnetClients[i] || !_telnetClients[i]->connected()) { - _telnetClients[i] = std::unique_ptr(client); - - _telnetClients[i]->onAck([i](void *s, AsyncClient *c, size_t len, uint32_t time) { - }, 0); - - _telnetClients[i]->onData([i](void *s, AsyncClient *c, void *data, size_t len) { - _telnetData(i, data, len); - }, 0); - - _telnetClients[i]->onDisconnect([i](void *s, AsyncClient *c) { - _telnetDisconnect(i); - }, 0); - - _telnetClients[i]->onError([i](void *s, AsyncClient *c, int8_t error) { - DEBUG_MSG_P(PSTR("[TELNET] Error %s (%d) on client #%u\n"), c->errorToString(error), error, i); - }, 0); - - _telnetClients[i]->onTimeout([i](void *s, AsyncClient *c, uint32_t time) { - DEBUG_MSG_P(PSTR("[TELNET] Timeout on client #%u at %lu\n"), i, time); - c->close(); - }, 0); + client->onError([i](void *s, AsyncClient *client, int8_t error) { + DEBUG_MSG_P(PSTR("[TELNET] Error %s (%d) on client #%u\n"), client->errorToString(error), error, i); + }); + client->onData([i](void*, AsyncClient*, void *data, size_t len){ + _telnetData(i, reinterpret_cast(data), len); + }); + client->onDisconnect([i](void*, AsyncClient*) { + _telnetCleanUp(); + }); + _telnetClients[i] = std::make_unique(i, client); _telnetNotifyConnected(i); + return; } From 342efc9805d048695be3693c02679c748e52730d Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Wed, 2 Oct 2019 05:35:17 +0300 Subject: [PATCH 2/9] telnet: make async buffer an option --- code/espurna/config/all.h | 2 ++ code/espurna/config/dependencies.h | 5 ++++ code/espurna/config/general.h | 5 ++++ code/espurna/config/prototypes.h | 1 - code/espurna/telnet.ino | 44 +++++++++++++++++++++--------- 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/code/espurna/config/all.h b/code/espurna/config/all.h index 1eef6bd5b0..35906b69db 100644 --- a/code/espurna/config/all.h +++ b/code/espurna/config/all.h @@ -19,6 +19,8 @@ */ +#include + #ifdef USE_CUSTOM_H #include "custom.h" #endif diff --git a/code/espurna/config/dependencies.h b/code/espurna/config/dependencies.h index cd678b7f04..c9b205fc2d 100644 --- a/code/espurna/config/dependencies.h +++ b/code/espurna/config/dependencies.h @@ -86,3 +86,8 @@ #undef MDNS_CLIENT_SUPPORT #define MDNS_CLIENT_SUPPORT 0 // default resolver already handles this #endif + +#if not defined(ARDUINO_ESP8266_RELEASE_2_3_0) +#undef TELNET_SERVER_ASYNC_BUFFERED +#define TELNET_SERVER_ASYNC_BUFFERED 1 // enable buffered telnet by default on latest Cores +#endif diff --git a/code/espurna/config/general.h b/code/espurna/config/general.h index ef8742f830..58046260fd 100644 --- a/code/espurna/config/general.h +++ b/code/espurna/config/general.h @@ -130,6 +130,11 @@ #define TELNET_SERVER TELNET_SERVER_ASYNC // Can be either TELNET_SERVER_ASYNC (using ESPAsyncTCP) or TELNET_SERVER_WIFISERVER (using WiFiServer) #endif +#ifndef TELNET_SERVER_ASYNC_BUFFERED +#define TELNET_SERVER_ASYNC_BUFFERED 0 // Enable buffered output for telnet server (+1.4Kb) + // Helps to avoid lost data with lwip2 TCP_MSS=536 option +#endif + //------------------------------------------------------------------------------ // TERMINAL //------------------------------------------------------------------------------ diff --git a/code/espurna/config/prototypes.h b/code/espurna/config/prototypes.h index 4b1318aa0c..f6434e478d 100644 --- a/code/espurna/config/prototypes.h +++ b/code/espurna/config/prototypes.h @@ -3,7 +3,6 @@ #include #include #include -#include extern "C" { #include "user_interface.h" diff --git a/code/espurna/telnet.ino b/code/espurna/telnet.ino index b1bd342f17..aa39ba9685 100644 --- a/code/espurna/telnet.ino +++ b/code/espurna/telnet.ino @@ -14,19 +14,21 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer #define TELNET_XEOF 0xEC #if TELNET_SERVER == TELNET_SERVER_WIFISERVER - using TServer = WiFiServer; - using TClient = WiFiClient; -#else + using TTelnetServer = WiFiServer; + using TTelnetClient = WiFiClient; + +#elif TELNET_SERVER == TELNET_SERVER_ASYNC #include #include + using TTelnetServer = AsyncServer; + +#if TELNET_SERVER_ASYNC_BUFFERED #include - using TServer = AsyncServer; struct AsyncTelnetClient { using container_t = std::vector; - AsyncTelnetClient(unsigned char id, AsyncClient* client) : - _id(id), + AsyncTelnetClient(AsyncClient* client) : _client(client) { _client->onAck(_s_onAck, this); @@ -46,17 +48,23 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer void close(bool now = false); bool connected(); - unsigned char _id; AsyncClient* _client; container_t _current; std::deque _queue; }; - using TClient = AsyncTelnetClient; -#endif -TServer _telnetServer(TELNET_PORT); -std::unique_ptr _telnetClients[TELNET_MAX_CLIENTS]; + using TTelnetClient = AsyncTelnetClient; + +#else + using TTelnetClient = AsyncClient; + +#endif // TELNET_SERVER_ASYNC_BUFFERED + +#endif // TELNET_SERVER == TELNET_SERVER_WIFISERVER + +TTelnetServer _telnetServer(TELNET_PORT); +std::unique_ptr _telnetClients[TELNET_MAX_CLIENTS]; bool _telnetAuth = TELNET_AUTHENTICATION; bool _telnetClientsAuth[TELNET_MAX_CLIENTS]; @@ -106,6 +114,8 @@ void _telnetDisconnect(unsigned char clientId) { _telnetClients[clientId]->close(); } +#if TELNET_SERVER_ASYNC_BUFFERED + void AsyncTelnetClient::_trySend(AsyncTelnetClient* client) { if (!client->_queue.empty()) { auto& chunk = client->_queue.back(); @@ -184,6 +194,8 @@ bool AsyncTelnetClient::connected() { return _client->connected(); } +#endif // TELNET_SERVER_ASYNC_BUFFERED + #endif // TELNET_SERVER == TELNET_SERVER_WIFISERVER size_t _telnetWrite(unsigned char clientId, const char *data, size_t len) { @@ -296,7 +308,7 @@ void _telnetLoop() { for (i = 0; i < TELNET_MAX_CLIENTS; i++) { if (!_telnetClients[i] || !_telnetClients[i]->connected()) { - _telnetClients[i] = std::unique_ptr(new WiFiClient(_telnetServer.available())); + _telnetClients[i] = std::make_unique(_telnetServer.available()); if (_telnetClients[i]->localIP() != WiFi.softAPIP()) { // Telnet is always available for the ESPurna Core image @@ -381,7 +393,13 @@ void _telnetNewClient(AsyncClient* client) { _telnetCleanUp(); }); - _telnetClients[i] = std::make_unique(i, client); + // XXX: AsyncClient does not have copy ctor + #if TELNET_SERVER_ASYNC_BUFFERED + _telnetClients[i] = std::make_unique(client); + #else + _telnetClients[i] = std::unique_ptr(client); + #endif // TELNET_SERVER_ASYNC_BUFFERED + _telnetNotifyConnected(i); return; From 74d489448312ffc91f7f1e6cd1b3c7238110902a Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Wed, 2 Oct 2019 08:17:10 +0300 Subject: [PATCH 3/9] just use the queue containers directly --- code/espurna/telnet.ino | 43 +++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/code/espurna/telnet.ino b/code/espurna/telnet.ino index aa39ba9685..368580936a 100644 --- a/code/espurna/telnet.ino +++ b/code/espurna/telnet.ino @@ -26,6 +26,7 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer #include struct AsyncTelnetClient { + constexpr static const size_t QUEUE_MAX_SIZE = 4; using container_t = std::vector; AsyncTelnetClient(AsyncClient* client) : @@ -35,6 +36,7 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer _client->onPoll(_s_onPoll, this); } + void _addQueueElem(); static void _trySend(AsyncTelnetClient* client); static void _s_onAck(void* client_ptr, AsyncClient*, size_t, uint32_t); static void _s_onPoll(void* client_ptr, AsyncClient* client); @@ -50,7 +52,6 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer AsyncClient* _client; - container_t _current; std::deque _queue; }; @@ -118,19 +119,10 @@ void _telnetDisconnect(unsigned char clientId) { void AsyncTelnetClient::_trySend(AsyncTelnetClient* client) { if (!client->_queue.empty()) { - auto& chunk = client->_queue.back(); + auto& chunk = client->_queue.front(); if (client->_client->space() >= chunk.size()) { client->_client->write((const char*)chunk.data(), chunk.size()); - client->_queue.pop_back(); - } - return; - } - - const auto current_size = client->_current.size(); - if (current_size) { - if (client->_client->space() >= current_size) { - client->_client->write((const char*)client->_current.data(), client->_current.size()); - client->_current.clear(); + client->_queue.pop_front(); } return; } @@ -144,26 +136,35 @@ void AsyncTelnetClient::_s_onPoll(void* client_ptr, AsyncClient* client) { _trySend(reinterpret_cast(client_ptr)); } +void AsyncTelnetClient::_addQueueElem() { + _queue.emplace_back(); + _queue.back().reserve(TCP_MSS); +} + size_t AsyncTelnetClient::write(const char* data, size_t size) { - const size_t written = _client->add(data, size); - if (written == size) return size; + if (_queue.size() > AsyncTelnetClient::QUEUE_MAX_SIZE) return 0; + + size_t written = 0; + if (_queue.empty()) { + written = _client->add(data, size); + if (written == size) return size; + } const size_t full_size = size; char* data_ptr = const_cast(data + written); size -= written; - _current.reserve(TCP_MSS); - while (size) { - const auto have = _current.capacity() - _current.size(); + if (_queue.empty()) _addQueueElem(); + auto& back = _queue.back(); + const auto have = back.capacity() - back.size(); if (have >= size) { - _current.insert(_current.end(), data_ptr, data_ptr + size); + back.insert(back.end(), data_ptr, data_ptr + size); size = 0; } else { - _current.insert(_current.end(), data_ptr, data_ptr + have); - _queue.push_front(_current); - _current.clear(); + back.insert(back.end(), data_ptr, data_ptr + have); + _addQueueElem(); data_ptr += have; size -= have; } From df8db013cc9071b11cd0847e0b7ce21cbfaa5c91 Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Thu, 3 Oct 2019 09:17:20 +0300 Subject: [PATCH 4/9] try with simpler list --- code/espurna/telnet.ino | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/code/espurna/telnet.ino b/code/espurna/telnet.ino index 368580936a..e89bf60675 100644 --- a/code/espurna/telnet.ino +++ b/code/espurna/telnet.ino @@ -23,11 +23,11 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer using TTelnetServer = AsyncServer; #if TELNET_SERVER_ASYNC_BUFFERED - #include + #include struct AsyncTelnetClient { - constexpr static const size_t QUEUE_MAX_SIZE = 4; - using container_t = std::vector; + constexpr static const size_t BUFFERS_MAX = 5; + using buffer_t = std::vector; AsyncTelnetClient(AsyncClient* client) : _client(client) @@ -36,7 +36,7 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer _client->onPoll(_s_onPoll, this); } - void _addQueueElem(); + void _addBuffer(); static void _trySend(AsyncTelnetClient* client); static void _s_onAck(void* client_ptr, AsyncClient*, size_t, uint32_t); static void _s_onPoll(void* client_ptr, AsyncClient* client); @@ -52,7 +52,7 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer AsyncClient* _client; - std::deque _queue; + std::list _buffers; }; using TTelnetClient = AsyncTelnetClient; @@ -118,11 +118,11 @@ void _telnetDisconnect(unsigned char clientId) { #if TELNET_SERVER_ASYNC_BUFFERED void AsyncTelnetClient::_trySend(AsyncTelnetClient* client) { - if (!client->_queue.empty()) { - auto& chunk = client->_queue.front(); + if (!client->_buffers.empty()) { + auto& chunk = client->_buffers.front(); if (client->_client->space() >= chunk.size()) { client->_client->write((const char*)chunk.data(), chunk.size()); - client->_queue.pop_front(); + client->_buffers.pop_front(); } return; } @@ -136,17 +136,18 @@ void AsyncTelnetClient::_s_onPoll(void* client_ptr, AsyncClient* client) { _trySend(reinterpret_cast(client_ptr)); } -void AsyncTelnetClient::_addQueueElem() { - _queue.emplace_back(); - _queue.back().reserve(TCP_MSS); +void AsyncTelnetClient::_addBuffer() { + // Note: c++17 emplace returns created object reference + _buffers.emplace_back(); + _buffers.back().reserve(TCP_MSS); } size_t AsyncTelnetClient::write(const char* data, size_t size) { - if (_queue.size() > AsyncTelnetClient::QUEUE_MAX_SIZE) return 0; + if (_buffers.size() > AsyncTelnetClient::BUFFERS_MAX) return 0; size_t written = 0; - if (_queue.empty()) { + if (_buffers.empty()) { written = _client->add(data, size); if (written == size) return size; } @@ -156,15 +157,15 @@ size_t AsyncTelnetClient::write(const char* data, size_t size) { size -= written; while (size) { - if (_queue.empty()) _addQueueElem(); - auto& back = _queue.back(); - const auto have = back.capacity() - back.size(); + if (_buffers.empty()) _addBuffer(); + auto& current = _buffers.back(); + const auto have = current.capacity() - current.size(); if (have >= size) { - back.insert(back.end(), data_ptr, data_ptr + size); + current.insert(current.end(), data_ptr, data_ptr + size); size = 0; } else { - back.insert(back.end(), data_ptr, data_ptr + have); - _addQueueElem(); + current.insert(current.end(), data_ptr, data_ptr + have); + _addBuffer(); data_ptr += have; size -= have; } From 8e1b0796041799950f2bc8d2b8a33977ab825627 Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Thu, 3 Oct 2019 09:17:50 +0300 Subject: [PATCH 5/9] exhaust buffers as much as possible in a single try --- code/espurna/telnet.ino | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code/espurna/telnet.ino b/code/espurna/telnet.ino index e89bf60675..f07f15949d 100644 --- a/code/espurna/telnet.ino +++ b/code/espurna/telnet.ino @@ -118,11 +118,12 @@ void _telnetDisconnect(unsigned char clientId) { #if TELNET_SERVER_ASYNC_BUFFERED void AsyncTelnetClient::_trySend(AsyncTelnetClient* client) { - if (!client->_buffers.empty()) { + while (!client->_buffers.empty()) { auto& chunk = client->_buffers.front(); if (client->_client->space() >= chunk.size()) { client->_client->write((const char*)chunk.data(), chunk.size()); client->_buffers.pop_front(); + continue; } return; } From f7fef302507fb1452d6e4c65df4d08bb56024257 Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Fri, 4 Oct 2019 18:03:09 +0300 Subject: [PATCH 6/9] don't forget to destroy the client object --- code/espurna/telnet.ino | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/espurna/telnet.ino b/code/espurna/telnet.ino index f07f15949d..d80cd8ad41 100644 --- a/code/espurna/telnet.ino +++ b/code/espurna/telnet.ino @@ -50,7 +50,7 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer void close(bool now = false); bool connected(); - AsyncClient* _client; + std::unique_ptr _client; std::list _buffers; }; From 0db3b652696e2feb3eda5932b8bc7da1268cc2be Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Fri, 4 Oct 2019 18:26:38 +0300 Subject: [PATCH 7/9] naming --- code/espurna/telnet.ino | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/code/espurna/telnet.ino b/code/espurna/telnet.ino index d80cd8ad41..0648b1a259 100644 --- a/code/espurna/telnet.ino +++ b/code/espurna/telnet.ino @@ -3,9 +3,13 @@ TELNET MODULE Copyright (C) 2017-2019 by Xose Pérez + Parts of the code have been borrowed from Thomas Sarlandie's NetServer (https://github.com/sarfata/kbox-firmware/tree/master/src/esp) +AsyncBufferedClient based on ESPAsyncTCPbuffer, distributed with the ESPAsyncTCP +(https://github.com/me-no-dev/ESPAsyncTCP/blob/master/src/ESPAsyncTCPbuffer.cpp) + */ #if TELNET_SUPPORT @@ -25,11 +29,11 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer #if TELNET_SERVER_ASYNC_BUFFERED #include - struct AsyncTelnetClient { + struct AsyncBufferedClient { constexpr static const size_t BUFFERS_MAX = 5; using buffer_t = std::vector; - AsyncTelnetClient(AsyncClient* client) : + AsyncBufferedClient(AsyncClient* client) : _client(client) { _client->onAck(_s_onAck, this); @@ -37,7 +41,7 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer } void _addBuffer(); - static void _trySend(AsyncTelnetClient* client); + static void _trySend(AsyncBufferedClient* client); static void _s_onAck(void* client_ptr, AsyncClient*, size_t, uint32_t); static void _s_onPoll(void* client_ptr, AsyncClient* client); @@ -55,7 +59,7 @@ Parts of the code have been borrowed from Thomas Sarlandie's NetServer std::list _buffers; }; - using TTelnetClient = AsyncTelnetClient; + using TTelnetClient = AsyncBufferedClient; #else using TTelnetClient = AsyncClient; @@ -117,7 +121,7 @@ void _telnetDisconnect(unsigned char clientId) { #if TELNET_SERVER_ASYNC_BUFFERED -void AsyncTelnetClient::_trySend(AsyncTelnetClient* client) { +void AsyncBufferedClient::_trySend(AsyncBufferedClient* client) { while (!client->_buffers.empty()) { auto& chunk = client->_buffers.front(); if (client->_client->space() >= chunk.size()) { @@ -129,23 +133,23 @@ void AsyncTelnetClient::_trySend(AsyncTelnetClient* client) { } } -void AsyncTelnetClient::_s_onAck(void* client_ptr, AsyncClient*, size_t, uint32_t) { - _trySend(reinterpret_cast(client_ptr)); +void AsyncBufferedClient::_s_onAck(void* client_ptr, AsyncClient*, size_t, uint32_t) { + _trySend(reinterpret_cast(client_ptr)); } -void AsyncTelnetClient::_s_onPoll(void* client_ptr, AsyncClient* client) { - _trySend(reinterpret_cast(client_ptr)); +void AsyncBufferedClient::_s_onPoll(void* client_ptr, AsyncClient* client) { + _trySend(reinterpret_cast(client_ptr)); } -void AsyncTelnetClient::_addBuffer() { +void AsyncBufferedClient::_addBuffer() { // Note: c++17 emplace returns created object reference _buffers.emplace_back(); _buffers.back().reserve(TCP_MSS); } -size_t AsyncTelnetClient::write(const char* data, size_t size) { +size_t AsyncBufferedClient::write(const char* data, size_t size) { - if (_buffers.size() > AsyncTelnetClient::BUFFERS_MAX) return 0; + if (_buffers.size() > AsyncBufferedClient::BUFFERS_MAX) return 0; size_t written = 0; if (_buffers.empty()) { @@ -176,24 +180,24 @@ size_t AsyncTelnetClient::write(const char* data, size_t size) { } -size_t AsyncTelnetClient::write(char c) { +size_t AsyncBufferedClient::write(char c) { char _c[1] {c}; return write(_c, 1); } -void AsyncTelnetClient::flush() { +void AsyncBufferedClient::flush() { _client->send(); } -size_t AsyncTelnetClient::available() { +size_t AsyncBufferedClient::available() { return _client->space(); } -void AsyncTelnetClient::close(bool now) { +void AsyncBufferedClient::close(bool now) { _client->close(now); } -bool AsyncTelnetClient::connected() { +bool AsyncBufferedClient::connected() { return _client->connected(); } From 15aad877a53a6c9877095122ad39e92be52b2830 Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Fri, 4 Oct 2019 18:27:08 +0300 Subject: [PATCH 8/9] kill the connection earlier --- code/espurna/telnet.ino | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/espurna/telnet.ino b/code/espurna/telnet.ino index 0648b1a259..2f3b75bea0 100644 --- a/code/espurna/telnet.ino +++ b/code/espurna/telnet.ino @@ -114,9 +114,9 @@ void _telnetCleanUp() { }); } -// just mark as closed, clean-up method above will destroy the object later +// just close, clean-up method above will destroy the object later void _telnetDisconnect(unsigned char clientId) { - _telnetClients[clientId]->close(); + _telnetClients[clientId]->close(true); } #if TELNET_SERVER_ASYNC_BUFFERED From 20cec4a17cc42a2f2d2e3f84d0933cbe7555e833 Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Wed, 9 Oct 2019 23:24:47 +0300 Subject: [PATCH 9/9] fix merge issues --- code/espurna/telnet.ino | 78 +++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 45 deletions(-) diff --git a/code/espurna/telnet.ino b/code/espurna/telnet.ino index ef55b6341e..e7850f09ab 100644 --- a/code/espurna/telnet.ino +++ b/code/espurna/telnet.ino @@ -51,6 +51,7 @@ AsyncBufferedClient based on ESPAsyncTCPbuffer, distributed with the ESPAsyncTCP void flush(); size_t available(); + bool connect(const char *host, uint16_t port); void close(bool now = false); bool connected(); @@ -100,16 +101,18 @@ void _telnetReverse(const char * host, uint16_t port) { for (i = 0; i < TELNET_MAX_CLIENTS; i++) { if (!_telnetClients[i] || !_telnetClients[i]->connected()) { #if TELNET_SERVER == TELNET_SERVER_WIFISERVER - _telnetClients[i] = std::make_unique(); + _telnetClients[i] = std::make_unique(); #else // TELNET_SERVER == TELNET_SERVER_ASYNC - _telnetClients[i] = std::make_unique(new AsyncClient()); + _telnetSetupClient(i, new AsyncClient()); #endif if (_telnetClients[i]->connect(host, port)) { - return _telnetNotifyConnected(i); + _telnetNotifyConnected(i); + return; } else { DEBUG_MSG_P(PSTR("[TELNET] Error connecting reverse telnet\n")); - return _telnetDisconnect(i); + _telnetDisconnect(i); + return; } } } @@ -249,6 +252,10 @@ size_t AsyncBufferedClient::available() { return _client->space(); } +bool AsyncBufferedClient::connect(const char *host, uint16_t port) { + return _client->connect(host, port); +} + void AsyncBufferedClient::close(bool now) { _client->close(now); } @@ -337,28 +344,6 @@ void _telnetNotifyConnected(unsigned char i) { DEBUG_MSG_P(PSTR("[TELNET] Client #%u connected\n"), i); - #if TELNET_SERVER == TELNET_SERVER_ASYNC - _telnetClients[i]->onAck([i](void *s, AsyncClient *c, size_t len, uint32_t time) { - }, 0); - - _telnetClients[i]->onData([i](void *s, AsyncClient *c, void *data, size_t len) { - _telnetData(i, data, len); - }, 0); - - _telnetClients[i]->onDisconnect([i](void *s, AsyncClient *c) { - _telnetDisconnect(i); - }, 0); - - _telnetClients[i]->onError([i](void *s, AsyncClient *c, int8_t error) { - DEBUG_MSG_P(PSTR("[TELNET] Error %s (%d) on client #%u\n"), c->errorToString(error), error, i); - }, 0); - - _telnetClients[i]->onTimeout([i](void *s, AsyncClient *c, uint32_t time) { - DEBUG_MSG_P(PSTR("[TELNET] Timeout on client #%u at %lu\n"), i, time); - c->close(); - }, 0); - #endif - // If there is no terminal support automatically dump info and crash data #if TERMINAL_SUPPORT == 0 info(); @@ -445,6 +430,27 @@ void _telnetLoop() { #elif TELNET_SERVER == TELNET_SERVER_ASYNC +void _telnetSetupClient(unsigned char i, AsyncClient *client) { + + client->onError([i](void *s, AsyncClient *client, int8_t error) { + DEBUG_MSG_P(PSTR("[TELNET] Error %s (%d) on client #%u\n"), client->errorToString(error), error, i); + }); + client->onData([i](void*, AsyncClient*, void *data, size_t len){ + _telnetData(i, reinterpret_cast(data), len); + }); + client->onDisconnect([i](void*, AsyncClient*) { + _telnetCleanUp(); + }); + + // XXX: AsyncClient does not have copy ctor + #if TELNET_SERVER_ASYNC_BUFFERED + _telnetClients[i] = std::make_unique(client); + #else + _telnetClients[i] = std::unique_ptr(client); + #endif // TELNET_SERVER_ASYNC_BUFFERED + +} + void _telnetNewClient(AsyncClient* client) { if (client->localIP() != WiFi.softAPIP()) { // Telnet is always available for the ESPurna Core image @@ -467,26 +473,8 @@ void _telnetNewClient(AsyncClient* client) { for (unsigned char i = 0; i < TELNET_MAX_CLIENTS; i++) { if (!_telnetClients[i] || !_telnetClients[i]->connected()) { - - client->onError([i](void *s, AsyncClient *client, int8_t error) { - DEBUG_MSG_P(PSTR("[TELNET] Error %s (%d) on client #%u\n"), client->errorToString(error), error, i); - }); - client->onData([i](void*, AsyncClient*, void *data, size_t len){ - _telnetData(i, reinterpret_cast(data), len); - }); - client->onDisconnect([i](void*, AsyncClient*) { - _telnetCleanUp(); - }); - - // XXX: AsyncClient does not have copy ctor - #if TELNET_SERVER_ASYNC_BUFFERED - _telnetClients[i] = std::make_unique(client); - #else - _telnetClients[i] = std::unique_ptr(client); - #endif // TELNET_SERVER_ASYNC_BUFFERED - + _telnetSetupClient(i, client); _telnetNotifyConnected(i); - return; }