From a8057cfc163e8410de91fa6966f08eb11f4134a6 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Thu, 15 Oct 2020 13:00:13 -0700 Subject: [PATCH] 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 --- .../console/binary_log/binary_logging.cpp | 30 ++++++++----------- firmware/console/binary_log/binary_logging.h | 3 +- firmware/console/binary_log/log_field.cpp | 7 +++-- firmware/console/binary_log/log_field.h | 4 +-- firmware/console/status_loop.cpp | 16 ++++------ firmware/controllers/engine_controller.cpp | 2 +- unit_tests/tests/test_binary_log.cpp | 25 +++++++++++----- 7 files changed, 46 insertions(+), 41 deletions(-) diff --git a/firmware/console/binary_log/binary_logging.cpp b/firmware/console/binary_log/binary_logging.cpp index f555029201..77c21eee2f 100644 --- a/firmware/console/binary_log/binary_logging.cpp +++ b/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); - entryHeaders += sz; - headerSize += sz; + fields[i].writeHeader(outBuffer); } - - // 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; diff --git a/firmware/console/binary_log/binary_logging.h b/firmware/console/binary_log/binary_logging.h index dd0d6200f3..70cd7b7c78 100644 --- a/firmware/console/binary_log/binary_logging.h +++ b/firmware/console/binary_log/binary_logging.h @@ -1,4 +1,5 @@ #include -size_t writeHeader(char* buffer); +struct Writer; +void writeHeader(Writer& buffer); size_t writeBlock(char* buffer); diff --git a/firmware/console/binary_log/log_field.cpp b/firmware/console/binary_log/log_field.cpp index ae5a63ae6c..58e2c0bdbc 100644 --- a/firmware/console/binary_log/log_field.cpp +++ b/firmware/console/binary_log/log_field.cpp @@ -1,4 +1,5 @@ #include "log_field.h" +#include "buffered_writer.h" #include @@ -16,7 +17,9 @@ static void copyFloat(char* buffer, float value) { memcpy_swapend(buffer, reinterpret_cast(&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(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 { diff --git a/firmware/console/binary_log/log_field.h b/firmware/console/binary_log/log_field.h index 0ab289fa9f..eb7a3c987d 100644 --- a/firmware/console/binary_log/log_field.h +++ b/firmware/console/binary_log/log_field.h @@ -4,6 +4,7 @@ #include #include +struct Writer; class LogField { public: template @@ -34,8 +35,7 @@ public: } // Write the header data describing this field. - // Returns the number of bytes written. - size_t writeHeader(char* buffer) const; + void writeHeader(Writer& outBuffer) const; // Write the field's data to the buffer. // Returns the number of bytes written. diff --git a/firmware/console/status_loop.cpp b/firmware/console/status_loop.cpp index 35f85ff214..ebf5c9a4fc 100644 --- a/firmware/console/status_loop.cpp +++ b/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; diff --git a/firmware/controllers/engine_controller.cpp b/firmware/controllers/engine_controller.cpp index 88d4b2ae86..d790c9acff 100644 --- a/firmware/controllers/engine_controller.cpp +++ b/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 diff --git a/unit_tests/tests/test_binary_log.cpp b/unit_tests/tests/test_binary_log.cpp index 7a42f10f3b..7a44eb18a2 100644 --- a/unit_tests/tests/test_binary_log.cpp +++ b/unit_tests/tests/test_binary_log.cpp @@ -1,18 +1,32 @@ #include "log_field.h" +#include "buffered_writer.h" #include +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 channel; LogField field(channel, "name", "units", 2); - char buffer[56]; - memset(buffer, 0xAA, sizeof(buffer)); + char buffer[55]; + StrictMock 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, - - // After end should be 0xAA, not touched - 0xAA + 2 )); }