write SD header using new Writer class (#1882)

* write header

* hooray, free memory!

* fix test

* is there really this much free space...?

* no, there is not

firmware/console/binary_log/binary_logging.cpp

@@ -3,13 +3,13 @@
  * See also mlq_file_format.txt
  */
 
-
+#include "binary_logging.h"
 #include "tunerstudio_outputs.h"
 #include "log_field.h"
 #include "efilib.h"
 #include "efitime.h"
 #include "crc.h"
-
+#include "buffered_writer.h"
 
 #define TIME_PRECISION 1000
 
@@ -59,7 +59,8 @@ static const LogField fields[] = {
 	{tsOutputChannels.massAirFlow, GAUGE_NAME_AIR_FLOW, "kg/h", 1},
 };
 
-size_t writeHeader(char* buffer) {
+void writeHeader(Writer& outBuffer) {
+	char buffer[MLQ_HEADER_SIZE];
 	// File format: MLVLG\0
 	strncpy(buffer, "MLVLG", 6);
 
@@ -77,7 +78,13 @@ size_t writeHeader(char* buffer) {
 	buffer[12] = 0;
 	buffer[13] = 0;
 
-	// Index 14-17 are written at the end - header end offset
+	size_t headerSize = MLQ_HEADER_SIZE + efi::size(fields) * 55;
+
+	// Data begin index: begins immediately after the header
+	buffer[14] = 0;
+	buffer[15] = 0;
+	buffer[16] = (headerSize >> 8) & 0xFF;
+	buffer[17] = headerSize & 0xFF;
 
 	// Record length - length of a single data record: sum size of all fields
 	uint16_t recLength = 0;
@@ -92,23 +99,12 @@ size_t writeHeader(char* buffer) {
 	buffer[20] = 0;
 	buffer[21] = efi::size(fields);
 
-	size_t headerSize = MLQ_HEADER_SIZE;
+	outBuffer.write(buffer, MLQ_HEADER_SIZE);
 
 	// Write the actual logger fields, offset 22
-	char* entryHeaders = buffer + MLQ_HEADER_SIZE;
 	for (size_t i = 0; i < efi::size(fields); i++) {
-		size_t sz = fields[i].writeHeader(entryHeaders);
+		fields[i].writeHeader(outBuffer);
-		entryHeaders += sz;
-		headerSize += sz;
 	}
-
-	// Data begin index: begins immediately after the header
-	buffer[14] = 0;
-	buffer[15] = 0;
-	buffer[16] = (headerSize >> 8) & 0xFF;
-	buffer[17] = headerSize & 0xFF;
-
-	return headerSize;
 }
 
 static uint8_t blockRollCounter = 0;

firmware/console/binary_log/binary_logging.h

@@ -1,4 +1,5 @@
 #include <cstddef>
 
-size_t writeHeader(char* buffer);
+struct Writer;
+void writeHeader(Writer& buffer);
 size_t writeBlock(char* buffer);

firmware/console/binary_log/log_field.cpp

@@ -1,4 +1,5 @@
 #include "log_field.h"
+#include "buffered_writer.h"
 
 #include <cstring>
 
@@ -16,7 +17,9 @@ static void copyFloat(char* buffer, float value) {
 	memcpy_swapend(buffer, reinterpret_cast<char*>(&value), sizeof(float));
 }
 
-size_t LogField::writeHeader(char* buffer) const {
+void LogField::writeHeader(Writer& outBuffer) const {
+	char buffer[MLQ_FIELD_HEADER_SIZE];
+
 	// Offset 0, length 1 = type
 	buffer[0] = static_cast<char>(m_type);
 
@@ -40,7 +43,7 @@ size_t LogField::writeHeader(char* buffer) const {
 	buffer[54] = m_digits;
 
 	// Total size = 55
-	return MLQ_FIELD_HEADER_SIZE;
+	outBuffer.write(buffer, MLQ_FIELD_HEADER_SIZE);
 }
 
 size_t LogField::writeData(char* buffer) const {

firmware/console/binary_log/log_field.h

@@ -4,6 +4,7 @@
 #include <cstdint>
 #include <cstddef>
 
+struct Writer;
 class LogField {
 public:
 	template <typename TValue, int TMult = 1>
@@ -34,8 +35,7 @@ public:
 	}
 
 	// Write the header data describing this field.
-	// Returns the number of bytes written.
+	void writeHeader(Writer& outBuffer) const;
-	size_t writeHeader(char* buffer) const;
 
 	// Write the field's data to the buffer.
 	// Returns the number of bytes written.

firmware/console/status_loop.cpp

@@ -120,15 +120,13 @@ static void setWarningEnabled(int value) {
 
 #if EFI_FILE_LOGGING
 // this one needs to be in main ram so that SD card SPI DMA works fine
-static char sdLogBuffer[2200] MAIN_RAM;
+static char sdLogBuffer[100] MAIN_RAM;
 static uint64_t binaryLogCount = 0;
 
 #endif /* EFI_FILE_LOGGING */
 
 EXTERN_ENGINE;
 
-static char buf[6];
-
 /**
  * This is useful if we are changing engine mode dynamically
  * For example http://rusefi.com/forum/viewtopic.php?f=5&t=1085
@@ -148,18 +146,15 @@ void writeLogLine(Writer& buffer) {
 	if (!main_loop_started)
 		return;
 
-	size_t length;
 	if (binaryLogCount == 0) {
-		length = writeHeader(sdLogBuffer);
+		writeHeader(buffer);
 	} else {
 		updateTunerStudioState(&tsOutputChannels);
-		length = writeBlock(sdLogBuffer);
+		size_t length = writeBlock(sdLogBuffer);
+		efiAssertVoid(OBD_PCM_Processor_Fault, length <= efi::size(sdLogBuffer), "SD log buffer overflow");
+		buffer.write(sdLogBuffer, length);
 	}
 
-	efiAssertVoid(OBD_PCM_Processor_Fault, length <= efi::size(sdLogBuffer), "SD log buffer overflow");
-
-	buffer.write(sdLogBuffer, length);
-
 	binaryLogCount++;
 #endif /* EFI_FILE_LOGGING */
 }
@@ -773,7 +768,6 @@ void updateTunerStudioState(TunerStudioOutputChannels *tsOutputChannels DECLARE_
 	case DBG_FSIO_ADC:
 		// todo: implement a proper loop
 		if (engineConfiguration->fsioAdc[0] != EFI_ADC_NONE) {
-			strcpy(buf, "adcX");
 			tsOutputChannels->debugFloatField1 = getVoltage("fsio", engineConfiguration->fsioAdc[0] PASS_ENGINE_PARAMETER_SUFFIX);
 		}
 		break;

firmware/controllers/engine_controller.cpp

@@ -704,7 +704,7 @@ void initEngineContoller(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX)
  * UNUSED_SIZE constants.
  */
 #ifndef RAM_UNUSED_SIZE
-#define RAM_UNUSED_SIZE 2350
+#define RAM_UNUSED_SIZE 4000
 #endif
 #ifndef CCM_UNUSED_SIZE
 #define CCM_UNUSED_SIZE 2900

unit_tests/tests/test_binary_log.cpp

@@ -1,18 +1,32 @@
 #include "log_field.h"
+#include "buffered_writer.h"
 
 #include <gmock/gmock.h>
 
+using ::testing::_;
 using ::testing::ElementsAre;
+using ::testing::StrictMock;
+
+class MockWriter : public Writer {
+public:
+	MOCK_METHOD(size_t, write, (const char* buffer, size_t count), (override));
+	MOCK_METHOD(size_t, flush, (), (override));
+};
 
 TEST(BinaryLogField, FieldHeader) {
 	scaled_channel<int8_t, 10> channel;
 	LogField field(channel, "name", "units", 2);
 
-	char buffer[56];
+	char buffer[55];
-	memset(buffer, 0xAA, sizeof(buffer));
+	StrictMock<MockWriter> bufWriter;
+	EXPECT_CALL(bufWriter, write(_, 55))
+		.WillOnce([&] (const char* buf, size_t count) {
+			memcpy(buffer, buf, count);
+			return 0;
+		});
 
 	// Should write 55 bytes
-	EXPECT_EQ(55, field.writeHeader(buffer));
+	field.writeHeader(bufWriter);
 
 	// Expect correctly written header
 	EXPECT_THAT(buffer, ElementsAre(
@@ -28,10 +42,7 @@ TEST(BinaryLogField, FieldHeader) {
 		// Transform - we always use 0
 		0, 0, 0, 0,
 		// Digits - 2, as configured
-		2,
+		2
-
-		// After end should be 0xAA, not touched
-		0xAA
 	));
 }