From e4c34e016a114fc52d2e1ff66352f6c6ca4ac5ea Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 18 Oct 2021 16:59:08 -0700 Subject: [PATCH] last packet optimization (#3363) * last packet optimization * comment * test * missed one * set tcp mss --- firmware/console/binary/tunerstudio_io.cpp | 10 +++++----- firmware/console/binary/tunerstudio_io.h | 6 +++--- firmware/console/binary/tunerstudio_io_serial.cpp | 4 ++-- firmware/console/ethernet_console.cpp | 7 +++++-- firmware/console/lwipopts.h | 5 +++++ firmware/console/usb_console.cpp | 2 +- unit_tests/tests/test_tunerstudio.cpp | 2 +- 7 files changed, 22 insertions(+), 14 deletions(-) diff --git a/firmware/console/binary/tunerstudio_io.cpp b/firmware/console/binary/tunerstudio_io.cpp index a1bce6c007..6ff986741a 100644 --- a/firmware/console/binary/tunerstudio_io.cpp +++ b/firmware/console/binary/tunerstudio_io.cpp @@ -45,7 +45,7 @@ void TsChannelBase::writeCrcPacketSmall(uint8_t responseCode, const uint8_t* buf *reinterpret_cast(&scratchBuffer[size + 3]) = SWAP_UINT32(crc); // Write to the underlying stream - write(reinterpret_cast(scratchBuffer), size + 7); + write(reinterpret_cast(scratchBuffer), size + 7, true); } void TsChannelBase::writeCrcPacketLarge(uint8_t responseCode, const uint8_t* buf, size_t size) { @@ -62,15 +62,15 @@ void TsChannelBase::writeCrcPacketLarge(uint8_t responseCode, const uint8_t* buf *(uint32_t*)crcBuffer = SWAP_UINT32(crc); // Write header - write(headerBuffer, sizeof(headerBuffer)); + write(headerBuffer, sizeof(headerBuffer), false); // If data, write that if (size) { - write(buf, size); + write(buf, size, false); } // Lastly the CRC footer - write(crcBuffer, sizeof(crcBuffer)); + write(crcBuffer, sizeof(crcBuffer), true); } TsChannelBase::TsChannelBase(const char *name) { @@ -101,7 +101,7 @@ void TsChannelBase::sendResponse(ts_response_format_e mode, const uint8_t * buff writeCrcPacket(TS_RESPONSE_OK, buffer, size); } else { if (size > 0) { - write(buffer, size); + write(buffer, size, true); flush(); } } diff --git a/firmware/console/binary/tunerstudio_io.h b/firmware/console/binary/tunerstudio_io.h index 2b37cd401d..6d10847242 100644 --- a/firmware/console/binary/tunerstudio_io.h +++ b/firmware/console/binary/tunerstudio_io.h @@ -38,7 +38,7 @@ class TsChannelBase { public: TsChannelBase(const char *name); // Virtual functions - implement these for your underlying transport - virtual void write(const uint8_t* buffer, size_t size) = 0; + virtual void write(const uint8_t* buffer, size_t size, bool isEndOfPacket = false) = 0; virtual size_t readTimeout(uint8_t* buffer, size_t size, int timeout) = 0; // These functions are optional to implement, not all channels need them @@ -84,7 +84,7 @@ public: void start(uint32_t baud) override; void stop() override; - void write(const uint8_t* buffer, size_t size) override; + void write(const uint8_t* buffer, size_t size, bool isEndOfPacket) override; size_t readTimeout(uint8_t* buffer, size_t size, int timeout) override; private: @@ -101,7 +101,7 @@ public: void start(uint32_t baud) override; void stop() override; - void write(const uint8_t* buffer, size_t size) override; + void write(const uint8_t* buffer, size_t size, bool isEndOfPacket) override; size_t readTimeout(uint8_t* buffer, size_t size, int timeout) override; protected: diff --git a/firmware/console/binary/tunerstudio_io_serial.cpp b/firmware/console/binary/tunerstudio_io_serial.cpp index d90e52aace..1cd432a5e2 100644 --- a/firmware/console/binary/tunerstudio_io_serial.cpp +++ b/firmware/console/binary/tunerstudio_io_serial.cpp @@ -23,7 +23,7 @@ void SerialTsChannel::stop() { sdStop(m_driver); } -void SerialTsChannel::write(const uint8_t* buffer, size_t size) { +void SerialTsChannel::write(const uint8_t* buffer, size_t size, bool) { chnWriteTimeout(m_driver, buffer, size, BINARY_IO_TIMEOUT); } @@ -56,7 +56,7 @@ void UartTsChannel::stop() { uartStop(m_driver); } -void UartTsChannel::write(const uint8_t* buffer, size_t size) { +void UartTsChannel::write(const uint8_t* buffer, size_t size, bool) { uartSendTimeout(m_driver, &size, buffer, BINARY_IO_TIMEOUT); } diff --git a/firmware/console/ethernet_console.cpp b/firmware/console/ethernet_console.cpp index ff8283b215..ff826c34c8 100644 --- a/firmware/console/ethernet_console.cpp +++ b/firmware/console/ethernet_console.cpp @@ -36,8 +36,11 @@ public: return connectionSocket != -1; } - void write(const uint8_t* buffer, size_t size) override { - lwip_send(connectionSocket, buffer, size, /*flags =*/ 0); + void write(const uint8_t* buffer, size_t size, bool isEndOfPacket) override { + // If not the end of a packet, set the MSG_MORE flag to indicate to the transport + // that we have more to add to the buffer before queuing a flush. + auto flags = isEndOfPacket ? 0 : MSG_MORE; + lwip_send(connectionSocket, buffer, size, flags); } size_t readTimeout(uint8_t* buffer, size_t size, int /*timeout*/) override { diff --git a/firmware/console/lwipopts.h b/firmware/console/lwipopts.h index a64714b924..ff6865faed 100644 --- a/firmware/console/lwipopts.h +++ b/firmware/console/lwipopts.h @@ -76,4 +76,9 @@ #define LWIP_ETHADDR_4 0x34 #define LWIP_ETHADDR_5 0x56 +#include "rusefi_generated.h" + +// Ensure that one TCP segment can always fit an entire response to TS - we never need to split a TS packet across multiple frames. +#define TCP_MSS (BLOCKING_FACTOR + 10) + #endif /* LWIP_HDR_LWIPOPTS_H__ */ diff --git a/firmware/console/usb_console.cpp b/firmware/console/usb_console.cpp index 1d8022c145..7b6fa32d4e 100644 --- a/firmware/console/usb_console.cpp +++ b/firmware/console/usb_console.cpp @@ -23,7 +23,7 @@ public: return is_usb_serial_ready(); } - void write(const uint8_t* buffer, size_t size) override { + void write(const uint8_t* buffer, size_t size, bool) override { chnWriteTimeout(m_channel, buffer, size, BINARY_IO_TIMEOUT); } diff --git a/unit_tests/tests/test_tunerstudio.cpp b/unit_tests/tests/test_tunerstudio.cpp index 9620ebb200..d54ba916ef 100644 --- a/unit_tests/tests/test_tunerstudio.cpp +++ b/unit_tests/tests/test_tunerstudio.cpp @@ -7,7 +7,7 @@ class MockTsChannel : public TsChannelBase { public: MockTsChannel() : TsChannelBase("Test") { } - void write(const uint8_t* buffer, size_t size) override { + void write(const uint8_t* buffer, size_t size, bool /*isLastWriteInTransaction*/) override { memcpy(&st5TestBuffer[writeIdx], buffer, size); writeIdx += size; }