From 2e35013d1d7e5d4355936919c734f01831072421 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Thu, 15 Dec 2022 04:34:10 -0800 Subject: [PATCH] SD card tooth log (#4897) * adjust tooth logger api * mmc card writes tooth log * changelog --- firmware/CHANGELOG.md | 1 + firmware/console/binary/tooth_logger.cpp | 46 +++++++----------------- firmware/console/binary/tooth_logger.h | 30 ++++++++++++---- firmware/console/binary/tunerstudio.cpp | 8 +++-- firmware/hw_layer/mmc_card.cpp | 32 ++++++++++++++++- firmware/integration/rusefi_config.txt | 2 +- firmware/tunerstudio/rusefi.input | 1 + 7 files changed, 77 insertions(+), 43 deletions(-) diff --git a/firmware/CHANGELOG.md b/firmware/CHANGELOG.md index b1d8cca42b..d2b41aace8 100644 --- a/firmware/CHANGELOG.md +++ b/firmware/CHANGELOG.md @@ -33,6 +33,7 @@ Release template (copy/paste this for new release): - VR trigger input oscilloscope for boards with "discrete VR" hardware (AlphaX ECUs, some Hellen) #4885 - Jammed ETB detection #4873 - RPM correction/multiplier for Accel Enrich #4760 + - Tooth logger writes to SD card #4897 ## December 2022 Release - "Day 289" diff --git a/firmware/console/binary/tooth_logger.cpp b/firmware/console/binary/tooth_logger.cpp index 0e55d85125..3608841cc6 100644 --- a/firmware/console/binary/tooth_logger.cpp +++ b/firmware/console/binary/tooth_logger.cpp @@ -11,18 +11,6 @@ #if EFI_TOOTH_LOGGER -typedef struct __attribute__ ((packed)) { - // the whole order of all packet bytes is reversed, not just the 'endian-swap' integers - uint32_t timestamp; - // unfortunately all these fields are required by TS... - bool priLevel : 1; - bool secLevel : 1; - bool trigger : 1; - bool sync : 1; - bool coil : 1; - bool injector : 1; -} composite_logger_s; - /** * Engine idles around 20Hz and revs up to 140Hz, at 60/2 and 8 cylinders we have about 20Khz events * If we can read buffer at 50Hz we want buffer to be about 400 elements. @@ -75,15 +63,8 @@ void DisableToothLogger() { #else // not EFI_UNIT_TEST -static constexpr size_t entriesPerBuffer = 250; static constexpr size_t totalEntryCount = BIG_BUFFER_SIZE / sizeof(composite_logger_s); -static constexpr size_t bufferCount = totalEntryCount / entriesPerBuffer; - -struct CompositeBuffer { - composite_logger_s buffer[entriesPerBuffer]; - size_t nextIdx; - Timer startTime; -}; +static constexpr size_t bufferCount = totalEntryCount / toothLoggerEntriesPerBuffer; static CompositeBuffer* buffers = nullptr; static chibios_rt::Mailbox freeBuffers CCM_OPTIONAL; @@ -146,35 +127,33 @@ void DisableToothLogger() { setToothLogReady(false); } -expected GetToothLoggerBuffer() { - chibios_rt::CriticalSectionLocker csl; - +CompositeBuffer* GetToothLoggerBuffer() { CompositeBuffer* buffer; - msg_t msg = filledBuffers.fetchI(&buffer); + msg_t msg = filledBuffers.fetch(&buffer, TIME_INFINITE); if (msg == MSG_TIMEOUT) { setToothLogReady(false); - return unexpected; + return nullptr; } if (msg != MSG_OK) { // What even happened if we didn't get timeout, but also didn't get OK? - return unexpected; + return nullptr; } - size_t entryCount = buffer->nextIdx; - buffer->nextIdx = 0; + return buffer; +} - // Return this buffer to the free list - msg = freeBuffers.postI(buffer); - efiAssert(OBD_PCM_Processor_Fault, msg == MSG_OK, "Composite logger post to free buffer fail", unexpected); +void ReturnToothLoggerBuffer(CompositeBuffer* buffer) { + chibios_rt::CriticalSectionLocker csl; + + msg_t msg = freeBuffers.postI(buffer); + efiAssertVoid(OBD_PCM_Processor_Fault, msg == MSG_OK, "Composite logger post to free buffer fail"); // If the used list is empty, clear the ready flag if (filledBuffers.getUsedCountI() == 0) { setToothLogReady(false); } - - return ToothLoggerBuffer{ reinterpret_cast(buffer->buffer), entryCount * sizeof(composite_logger_s)}; } static CompositeBuffer* findBuffer(efitick_t timestamp) { @@ -190,6 +169,7 @@ static CompositeBuffer* findBuffer(efitick_t timestamp) { // This ensures the user sees *something* even if they don't have enough trigger events // to fill the buffer. buffer->startTime.reset(timestamp); + buffer->nextIdx = 0; currentBuffer = buffer; } diff --git a/firmware/console/binary/tooth_logger.h b/firmware/console/binary/tooth_logger.h index 73136aad59..1a70210c7e 100644 --- a/firmware/console/binary/tooth_logger.h +++ b/firmware/console/binary/tooth_logger.h @@ -33,12 +33,30 @@ void LogTriggerCoilState(efitick_t timestamp, bool state); void LogTriggerInjectorState(efitick_t timestamp, bool state); -struct ToothLoggerBuffer -{ - const uint8_t* const Buffer; - const size_t Length; +typedef struct __attribute__ ((packed)) { + // the whole order of all packet bytes is reversed, not just the 'endian-swap' integers + uint32_t timestamp; + // unfortunately all these fields are required by TS... + bool priLevel : 1; + bool secLevel : 1; + bool trigger : 1; + bool sync : 1; + bool coil : 1; + bool injector : 1; +} composite_logger_s; + +static constexpr size_t toothLoggerEntriesPerBuffer = 250; + +struct CompositeBuffer { + composite_logger_s buffer[toothLoggerEntriesPerBuffer]; + size_t nextIdx; + Timer startTime; }; // Get a reference to the buffer -// Returns unexpected if no buffer is available -expected GetToothLoggerBuffer(); +// Returns nullptr if no buffer is available +CompositeBuffer* GetToothLoggerBuffer(); +// Return a buffer to the pool once its contents have been read +void ReturnToothLoggerBuffer(CompositeBuffer*); + +#include "big_buffer.h" diff --git a/firmware/console/binary/tunerstudio.cpp b/firmware/console/binary/tunerstudio.cpp index 231adc785d..31fa0147f9 100644 --- a/firmware/console/binary/tunerstudio.cpp +++ b/firmware/console/binary/tunerstudio.cpp @@ -752,7 +752,9 @@ int TunerStudio::handleCrcCommand(TsChannelBase* tsChannel, char *data, int inco auto toothBuffer = GetToothLoggerBuffer(); if (toothBuffer) { - tsChannel->sendResponse(TS_CRC, toothBuffer.Value.Buffer, toothBuffer.Value.Length, true); + tsChannel->sendResponse(TS_CRC, reinterpret_cast(toothBuffer->buffer), toothBuffer->nextIdx * sizeof(composite_logger_s), true); + + ReturnToothLoggerBuffer(toothBuffer); } else { // TS asked for a tooth logger buffer, but we don't have one to give it. sendErrorCode(tsChannel, TS_RESPONSE_OUT_OF_RANGE); @@ -794,7 +796,9 @@ int TunerStudio::handleCrcCommand(TsChannelBase* tsChannel, char *data, int inco auto toothBuffer = GetToothLoggerBuffer(); if (toothBuffer) { - tsChannel->sendResponse(TS_CRC, toothBuffer.Value.Buffer, toothBuffer.Value.Length, true); + tsChannel->sendResponse(TS_CRC, reinterpret_cast(toothBuffer->buffer), toothBuffer->nextIdx * sizeof(composite_logger_s), true); + + ReturnToothLoggerBuffer(toothBuffer); } else { // TS asked for a tooth logger buffer, but we don't have one to give it. sendErrorCode(tsChannel, TS_RESPONSE_OUT_OF_RANGE); diff --git a/firmware/hw_layer/mmc_card.cpp b/firmware/hw_layer/mmc_card.cpp index d3ca4a820d..28fb84fbb0 100644 --- a/firmware/hw_layer/mmc_card.cpp +++ b/firmware/hw_layer/mmc_card.cpp @@ -19,6 +19,7 @@ #include "buffered_writer.h" #include "status_loop.h" #include "binary_logging.h" +#include "tooth_logger.h" static bool fs_ready = false; @@ -182,7 +183,11 @@ static void prepareLogFileName() { ptr = itoa10(&logName[PREFIX_LEN], logFileIndex); } - strcat(ptr, DOT_MLG); + if (engineConfiguration->sdTriggerLog) { + strcat(ptr, ".teeth"); + } else { + strcat(ptr, DOT_MLG); + } } /** @@ -502,6 +507,11 @@ private: static NO_CACHE SdLogBufferWriter logBuffer; +// Log 'regular' ECU log to MLG file +static void mlgLogger(); + +// Log binary trigger log +static void sdTriggerLogger(); static THD_WORKING_AREA(mmcThreadStack, 3 * UTILITY_THREAD_STACK_SIZE); // MMC monitor thread static THD_FUNCTION(MMCmonThread, arg) { @@ -517,6 +527,14 @@ static THD_FUNCTION(MMCmonThread, arg) { engine->outputChannels.sd_logging_internal = true; #endif + if (engineConfiguration->sdTriggerLog) { + sdTriggerLogger(); + } else { + mlgLogger(); + } +} + +void mlgLogger() { while (true) { // if the SPI device got un-picked somehow, cancel SD card // Don't do this check at all if using SDMMC interface instead of SPI @@ -552,6 +570,18 @@ static THD_FUNCTION(MMCmonThread, arg) { } } +static void sdTriggerLogger() { + EnableToothLogger(); + + while (true) { + auto buffer = GetToothLoggerBuffer(); + + logBuffer.write(reinterpret_cast(buffer->buffer), buffer->nextIdx * sizeof(composite_logger_s)); + + ReturnToothLoggerBuffer(buffer); + } +} + bool isSdCardAlive(void) { return fs_ready; } diff --git a/firmware/integration/rusefi_config.txt b/firmware/integration/rusefi_config.txt index 6e7e69a9ff..d6761858f2 100644 --- a/firmware/integration/rusefi_config.txt +++ b/firmware/integration/rusefi_config.txt @@ -1082,7 +1082,7 @@ bit forceO2Heating,"yes","no";If enabled, don't wait for engine start to heat O2 bit invertVvtControlIntake, "retard","advance";If increased VVT duty cycle increases the indicated VVT angle, set this to 'advance'. If it decreases, set this to 'retard'. Most intake cams use 'advance', and most exhaust cams use 'retard'. bit invertVvtControlExhaust,"retard","advance";If increased VVT duty cycle increases the indicated VVT angle, set this to 'advance'. If it decreases, set this to 'retard'. Most intake cams use 'advance', and most exhaust cams use 'retard'. bit useBiQuadOnAuxSpeedSensors -bit unused_1484_bit_38 +bit sdTriggerLog,"trigger","normal";'Trigger' mode will write a high speed log of trigger events (warning: uses lots of space!). 'Normal' mode will write a standard MLG of sensors, engine function, etc. similar to the one captured in TunerStudio. bit unused_1484_bit_29 bit unused_1484_bit_30 bit tempBooleanForVerySpecialLogic diff --git a/firmware/tunerstudio/rusefi.input b/firmware/tunerstudio/rusefi.input index 892893f679..462422c46d 100644 --- a/firmware/tunerstudio/rusefi.input +++ b/firmware/tunerstudio/rusefi.input @@ -3575,6 +3575,7 @@ cmd_set_engine_type_default = "@@TS_IO_TEST_COMMAND_char@@@@ts_command_e_TS_ field = "CS Pin", sdCardCsPin @@if_ts_show_sd_pins field = "SPI", sdCardSpiDevice @@if_ts_show_sd_pins field = "SD logger rate", sdCardLogFrequency + field = "SD logger mode", sdTriggerLog dialog = gpsReceiver, "GPS Receiver" field = "gps RX", gps_rx_pin