SD card tooth log (#4897)

* adjust tooth logger api

* mmc card writes tooth log

* changelog

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"
 

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<CompositeBuffer*, bufferCount> freeBuffers CCM_OPTIONAL;
@@ -146,35 +127,33 @@ void DisableToothLogger() {
 	setToothLogReady(false);
 }
 
-expected<ToothLoggerBuffer> 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<uint8_t*>(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;
 	}

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<ToothLoggerBuffer> 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"

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<const uint8_t*>(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<const uint8_t*>(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);

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<const char*>(buffer->buffer), buffer->nextIdx * sizeof(composite_logger_s));
+
+		ReturnToothLoggerBuffer(buffer);
+	}
+}
+
 bool isSdCardAlive(void) {
 	return fs_ready;
 }

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

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