fixup! windows/serialInterface: Implemented read buffering to reduce …

…the number of ReadFile() syscalls being made

src/include/windows/serialInterface.hxx

@@ -14,12 +14,10 @@ struct serialInterface_t
 {
 private:
 	HANDLE device{INVALID_HANDLE_VALUE};
-	mutable std::array<uint8_t, 4096U> readBuffer{};
-	mutable size_t readBufferFullness{0U};
-	mutable size_t readBufferOffset{0U};
 
 	void handleDeviceError(std::string_view operation) noexcept;
 	void refillBuffer() const;
+	[[nodiscard]] char nextByte() const;
 
 public:
 	serialInterface_t() noexcept = default;

src/windows/serialInterface.cxx

@@ -18,6 +18,10 @@ using substrate::console;
 
 constexpr static auto uncDeviceSuffix{"\\\\.\\"sv};
 
+static std::array<char, 4096U> readBuffer{};
+static size_t readBufferFullness{0U};
+static size_t readBufferOffset{0U};
+
 [[nodiscard]] std::string serialForDevice(const usbDevice_t &device)
 {
 	// Grab the serial number string descriptor index
@@ -280,46 +284,31 @@ void serialInterface_t::refillBuffer() const
 		console.error("Read from device failed ("sv, GetLastError(), ")"sv);
 		throw bmpCommsError_t{};
 	}
-	/* We now have more data, so update the read buffer counters */
+	// We now have more data, so update the read buffer counters
 	readBufferFullness = bytesReceived;
 	readBufferOffset = 0U;
 }
 
+// NOLINTNEXTLINE(readability-convert-member-functions-to-static)
+[[gnu::noinline]] char serialInterface_t::nextByte() const
+{
+	// Check if we need more data or should use what's in the buffer already
+	while (readBufferOffset == readBufferFullness)
+		refillBuffer();
+	return readBuffer[readBufferOffset++];
+}
+
 // NOLINTNEXTLINE(readability-convert-member-functions-to-static)
 std::string serialInterface_t::readPacket() const
 {
 	std::array<char, bmp_t::maxPacketSize> packet{};
-	size_t length{0};
-	// Try gathering a '#' terminated response
-	while (length < packet.size())
+	size_t length{0U};
+	for (; length < packet.size(); ++length)
 	{
-		// Check if we need more data or should use what's in the buffer already
-		if (readBufferOffset == readBufferFullness)
-			refillBuffer();
-
-		// Look for an end of message marker
-		const auto responseLength
-		{
-			[&]()
-			{
-				for (const auto offset : substrate::indexSequence_t{readBufferOffset, readBufferFullness})
-				{
-					if (readBuffer[offset] == '#')
-						return offset;
-				}
-				return readBufferFullness;
-			}()
-		};
-		// We now either have a remote end of message marker, or need all the data from the buffer
-		std::memcpy(packet.data() + length, readBuffer.data() + readBufferOffset, responseLength);
-		readBufferOffset += responseLength;
-		length += responseLength;
-		// If it's a remote end of message marker, break out the loop
-		if (responseLength != readBufferFullness)
-		{
-			++readBufferOffset;
+		const auto byte{nextByte()};
+		if (byte == '#')
 			break;
-		}
+		packet[length] = byte;
 	}
 
 	// Make a new std::string of an appropriate length, copying the data in to return it