Use a shared buffer for "high memory" operations (#4878)

* big buffer

* mostly hooked up big buffer

* bad merge

* s

* sneak preview of trigger scope

* s

* constness

* channel limits

* s

* unnecessary

* efilib

* TcpServerSandbox

* includes fix

* binary format

firmware/console/binary/output_channels.txt

@@ -11,7 +11,7 @@ struct_no_prefix output_channels_s
 
 bit sd_present
 bit sd_logging_internal
-bit unusedb4;
+bit triggerScopeReady
 bit unusedb5;
 bit isFanOn;radiator fan
 bit isO2HeaterOn;

firmware/console/binary/trigger_scope.cpp

@@ -0,0 +1,42 @@
+#include "pch.h"
+
+#include "trigger_scope.h"
+
+static BigBufferHandle buffer;
+
+static bool isRunning = false;
+
+// Enable one buffer's worth of perf tracing, and retrieve the buffer size in bytes
+void triggerScopeEnable() {
+	buffer = getBigBuffer(BigBufferUser::TriggerScope);
+
+	// TODO: trigger ADC
+
+	isRunning = true;
+	engine->outputChannels.triggerScopeReady = true;
+}
+
+void triggerScopeDisable() {
+	// we're done with the buffer - let somebody else have it
+	buffer = {};
+
+	isRunning = false;
+	engine->outputChannels.triggerScopeReady = false;
+}
+
+static int theta = 0;
+
+// Retrieve the trace buffer
+const BigBufferHandle& triggerScopeGetBuffer() {
+	if (buffer) {
+		for (size_t i = 0; i < buffer.size(); i++)
+		{
+			buffer.get<uint8_t>()[i] = 128 + 100 * sin((float)theta / 50);
+			theta++;
+		}
+	}
+
+	// engine->outputChannels.triggerScopeReady = false;
+
+	return buffer;
+}

firmware/console/binary/trigger_scope.h

@@ -0,0 +1,5 @@
+#pragma once
+
+void triggerScopeEnable();
+void triggerScopeDisable();
+const BigBufferHandle& triggerScopeGetBuffer();

firmware/console/binary/tunerstudio.cpp

@@ -74,6 +74,7 @@
 #include "bluetooth.h"
 #include "tunerstudio_io.h"
 #include "tooth_logger.h"
+#include "trigger_scope.h"
 #include "electronic_throttle.h"
 #include "live_data.h"
 
@@ -366,7 +367,6 @@ static bool isKnownCommand(char command) {
 			|| command == TS_IO_TEST_COMMAND
 			|| command == TS_GET_SCATTERED_GET_COMMAND
 			|| command == TS_SET_LOGGER_SWITCH
-			|| command == TS_GET_LOGGER_GET_BUFFER
 			|| command == TS_GET_COMPOSITE_BUFFER_DONE_DIFFERENTLY
 			|| command == TS_GET_TEXT
 			|| command == TS_CRC_CHECK_COMMAND
@@ -747,6 +747,36 @@ int TunerStudio::handleCrcCommand(TsChannelBase* tsChannel, char *data, int inco
 		case TS_COMPOSITE_DISABLE:
 			DisableToothLogger();
 			break;
+		case TS_COMPOSITE_READ:
+			{
+				auto toothBuffer = GetToothLoggerBuffer();
+
+				if (toothBuffer) {
+					tsChannel->sendResponse(TS_CRC, toothBuffer.Value.Buffer, toothBuffer.Value.Length, true);
+				} else {
+					// TS asked for a tooth logger buffer, but we don't have one to give it.
+					sendErrorCode(tsChannel, TS_RESPONSE_OUT_OF_RANGE);
+				}
+			}
+			break;
+		case TS_TRIGGER_SCOPE_ENABLE:
+			triggerScopeEnable();
+			break;
+		case TS_TRIGGER_SCOPE_DISABLE:
+			triggerScopeDisable();
+			break;
+		case TS_TRIGGER_SCOPE_READ:
+			{
+				const auto& buffer = triggerScopeGetBuffer();
+
+				if (buffer) {
+					tsChannel->sendResponse(TS_CRC, buffer.get<uint8_t>(), buffer.size(), true);
+				} else {
+					// TS asked for a tooth logger buffer, but we don't have one to give it.
+					sendErrorCode(tsChannel, TS_RESPONSE_OUT_OF_RANGE);
+				}
+			}
+			break;
 		default:
 			// dunno what that was, send NAK
 			return false;
@@ -756,10 +786,9 @@ int TunerStudio::handleCrcCommand(TsChannelBase* tsChannel, char *data, int inco
 
 		break;
 	case TS_GET_COMPOSITE_BUFFER_DONE_DIFFERENTLY:
-		EnableToothLoggerIfNotEnabled();
-		// falls through
-	case TS_GET_LOGGER_GET_BUFFER:
 		{
+			EnableToothLoggerIfNotEnabled();
+
 			auto toothBuffer = GetToothLoggerBuffer();
 
 			if (toothBuffer) {
@@ -780,7 +809,7 @@ int TunerStudio::handleCrcCommand(TsChannelBase* tsChannel, char *data, int inco
 	case TS_PERF_TRACE_GET_BUFFER:
 		{
 			auto trace = perfTraceGetBuffer();
-			tsChannel->sendResponse(TS_CRC, trace.Buffer, trace.Size, true);
+			tsChannel->sendResponse(TS_CRC, trace.get<uint8_t>(), trace.size(), true);
 		}
 
 		break;

firmware/console/binary/tunerstudio.mk

@@ -8,4 +8,6 @@ TUNERSTUDIO_SRC_CPP = $(PROJECT_DIR)/console/binary/tunerstudio_io.cpp \
 	$(PROJECT_DIR)/console/binary/tunerstudio.cpp \
 	$(PROJECT_DIR)/console/binary/tunerstudio_commands.cpp \
 	$(PROJECT_DIR)/console/binary/bluetooth.cpp \
-	$(PROJECT_DIR)/console/binary/signature.cpp
+	$(PROJECT_DIR)/console/binary/signature.cpp \
+	$(PROJECT_DIR)/console/binary/trigger_scope.cpp \
+

firmware/controllers/core/big_buffer.cpp

@@ -0,0 +1,57 @@
+#include "pch.h"
+
+#include "big_buffer.h"
+
+static BigBufferUser s_currentUser;
+static uint8_t s_bigBuffer[BIG_BUFFER_SIZE];
+
+static void releaseBuffer(void* bufferPtr, BigBufferUser user) {
+	if (bufferPtr != &s_bigBuffer || user != s_currentUser) {
+		// todo: panic!
+	}
+
+	s_currentUser = BigBufferUser::None;
+}
+
+BigBufferHandle::BigBufferHandle(void* buffer, BigBufferUser user)
+	: m_bufferPtr(buffer)
+	, m_user(user)
+{
+}
+
+BigBufferHandle::BigBufferHandle(BigBufferHandle&& other) {
+	// swap contents of the two objects
+	m_bufferPtr = other.m_bufferPtr;
+	other.m_bufferPtr = nullptr;
+
+	m_user = other.m_user;
+	other.m_user = BigBufferUser::None;
+}
+
+BigBufferHandle& BigBufferHandle::operator= (BigBufferHandle&& other) {
+	// swap contents of the two objects
+	m_bufferPtr = other.m_bufferPtr;
+	other.m_bufferPtr = nullptr;
+
+	m_user = other.m_user;
+	other.m_user = BigBufferUser::None;
+
+	return *this;
+}
+
+BigBufferHandle::~BigBufferHandle() {
+	if (m_bufferPtr) {
+		releaseBuffer(m_bufferPtr, m_user);
+	}
+}
+
+BigBufferHandle getBigBuffer(BigBufferUser user) {
+	if (s_currentUser != BigBufferUser::None) {
+		// fatal
+		return {};
+	}
+
+	s_currentUser = user;
+
+	return BigBufferHandle(s_bigBuffer, user);
+}

firmware/controllers/core/big_buffer.h

@@ -0,0 +1,50 @@
+// This file handles the "big buffer" - a shared buffer that can be used by multiple users depending on which function is enabled
+
+#pragma once
+
+#ifndef BIG_BUFFER_SIZE
+#define BIG_BUFFER_SIZE 8192
+#endif
+
+enum class BigBufferUser {
+	None,
+	ToothLogger,
+	PerfTrace,
+	TriggerScope,
+};
+
+class BigBufferHandle {
+public:
+	BigBufferHandle() = default;
+	BigBufferHandle(void* buffer, BigBufferUser user);
+	~BigBufferHandle();
+
+	// But allow moving (passing ownership of the buffer)
+	BigBufferHandle(BigBufferHandle&& other);
+	BigBufferHandle& operator=(BigBufferHandle&&);
+
+	// Implicit conversion operator to bool, so you can do things like if (myBuffer) { ... } as if it was a raw pointer
+	constexpr explicit operator bool() const {
+		return m_bufferPtr != nullptr;
+	}
+
+	template <class TBuffer>
+	const TBuffer* get() const {
+		return reinterpret_cast<TBuffer*>(m_bufferPtr);
+	}
+
+	template <class TBuffer>
+	TBuffer* get() {
+		return reinterpret_cast<TBuffer*>(m_bufferPtr);
+	}
+
+	size_t size() const {
+		return BIG_BUFFER_SIZE;
+	}
+
+private:
+	void* m_bufferPtr = nullptr;
+	BigBufferUser m_user = BigBufferUser::None;
+};
+
+BigBufferHandle getBigBuffer(BigBufferUser user);

firmware/controllers/core/core.mk

@@ -1,4 +1,4 @@
 
 CONTROLLERS_CORE_SRC_CPP = \
 	$(PROJECT_DIR)/controllers/core/state_sequence.cpp \
-	
+	$(PROJECT_DIR)/controllers/core/big_buffer.cpp \

firmware/development/perf_trace.cpp

@@ -6,24 +6,12 @@
  * See JsonOutput.java in rusEfi console
  */
 
-
 #include "pch.h"
 
-
 #ifndef ENABLE_PERF_TRACE
 #error ENABLE_PERF_TRACE must be defined!
 #endif
 
-#ifndef TRACE_BUFFER_LENGTH
-#define TRACE_BUFFER_LENGTH 2048
-#endif /* TRACE_BUFFER_LENGTH */
-
-// Disable the buffer if we're not enabled at all
-#if !ENABLE_PERF_TRACE
-#undef TRACE_BUFFER_LENGTH
-#define TRACE_BUFFER_LENGTH 1
-#endif
-
 enum class EPhase : char
 {
 	Start,
@@ -44,12 +32,19 @@ struct TraceEntry
 // Ensure that the struct is the size we think it is - the binary layout is important
 static_assert(sizeof(TraceEntry) == 8);
 
+#define TRACE_BUFFER_LENGTH (BIG_BUFFER_SIZE / sizeof(TraceEntry))
+
 // This buffer stores a trace - we write the full buffer once, then disable tracing
-static TraceEntry s_traceBuffer[TRACE_BUFFER_LENGTH];
+static BigBufferHandle s_traceBuffer;
 static size_t s_nextIdx = 0;
 
 static bool s_isTracing = false;
 
+static void stopTrace() {
+	s_isTracing = false;
+	s_nextIdx = 0;
+}
+
 static void perfEventImpl(PE event, EPhase phase)
 {
 	// Bail if we aren't allowed to trace
@@ -58,7 +53,7 @@ static void perfEventImpl(PE event, EPhase phase)
 	}
 	
 	// Bail if we aren't tracing
-	if (!s_isTracing) {
+	if (!s_isTracing || !s_traceBuffer) {
 		return;
 	}
 
@@ -79,8 +74,7 @@ static void perfEventImpl(PE event, EPhase phase)
 
 		idx = s_nextIdx++;
 		if (s_nextIdx >= TRACE_BUFFER_LENGTH) {
-			s_nextIdx = 0;
-			s_isTracing = false;
+			stopTrace();
 		}
 
 		// Restore previous interrupt state - don't restore if they weren't enabled
@@ -90,7 +84,7 @@ static void perfEventImpl(PE event, EPhase phase)
 	}
 
 	// We can safely write data out of the lock, our spot is reserved
-	volatile TraceEntry& entry = s_traceBuffer[idx];
+	volatile TraceEntry& entry = s_traceBuffer.get<TraceEntry>()[idx];
 
 	entry.Event = event;
 	entry.Phase = phase;
@@ -122,12 +116,14 @@ void perfEventInstantGlobal(PE event) {
 }
 
 void perfTraceEnable() {
+	s_traceBuffer = getBigBuffer(BigBufferUser::PerfTrace);
 	s_isTracing = true;
 }
 
-const TraceBufferResult perfTraceGetBuffer() {
+const BigBufferHandle perfTraceGetBuffer() {
 	// stop tracing if you try to get the buffer early
-	s_isTracing = false;
+	stopTrace();
 
-	return {reinterpret_cast<const uint8_t*>(s_traceBuffer), sizeof(s_traceBuffer)};
+	// transfer ownership of the buffer to the caller
+	return efi::move(s_traceBuffer);
 }

firmware/development/perf_trace.h

@@ -6,6 +6,8 @@
  */
 #pragma once
 
+#include "big_buffer.h"
+
 #include <cstdint>
 #include <cstddef>
 
@@ -79,14 +81,8 @@ void perfEventInstantGlobal(PE event);
 // Enable one buffer's worth of perf tracing, and retrieve the buffer size in bytes
 void perfTraceEnable();
 
-struct TraceBufferResult
-{
-	const uint8_t* const Buffer;
-	const size_t Size;
-};
-
 // Retrieve the trace buffer
-const TraceBufferResult perfTraceGetBuffer();
+const BigBufferHandle perfTraceGetBuffer();
 
 #if ENABLE_PERF_TRACE
 class ScopePerf

firmware/integration/rusefi_config.txt

@@ -2063,9 +2063,6 @@ end_struct
 #define TS_GET_COMPOSITE_BUFFER_DONE_DIFFERENTLY '8'
 #define TS_GET_SCATTERED_GET_COMMAND '9'
 
-#define TS_COMPOSITE_ENABLE 1
-#define TS_COMPOSITE_DISABLE 2
-
 ! Performance tracing
 #define TS_PERF_TRACE_BEGIN '_'
 #define TS_PERF_TRACE_GET_BUFFER 'b'
@@ -2103,7 +2100,14 @@ end_struct
 
 ! High speed logger commands
 #define TS_SET_LOGGER_SWITCH  'l'
-#define TS_GET_LOGGER_GET_BUFFER 'L'
+
+#define TS_COMPOSITE_ENABLE 1
+#define TS_COMPOSITE_DISABLE 2
+#define TS_COMPOSITE_READ 3
+
+#define TS_TRIGGER_SCOPE_ENABLE 4
+#define TS_TRIGGER_SCOPE_DISABLE 5
+#define TS_TRIGGER_SCOPE_READ 6
 
 #define PROTOCOL_COIL1_SHORT_NAME "c1"
 #define PROTOCOL_INJ1_SHORT_NAME "i1"

firmware/tunerstudio/rusefi.input

@@ -113,7 +113,7 @@ enable2ndByteCanID = false
 	loggerDef = compositeLogger, "Trigger Logger", composite
 		startCommand = "@#TS_SET_LOGGER_SWITCH#@\x01"
 		stopCommand  = "@#TS_SET_LOGGER_SWITCH#@\x02"
-		dataReadCommand = "@#TS_GET_LOGGER_GET_BUFFER#@"
+		dataReadCommand = "@#TS_SET_LOGGER_SWITCH#@\x03"
 		dataReadTimeout = 10000 ; time in ms
 		dataReadyCondition = { toothLogReady }
 		continuousRead = true
@@ -133,6 +133,21 @@ enable2ndByteCanID = false
 		; it seems that TS also needs to know the diff.size of a tooth
 		calcField = toothTime,			"ToothTime",	"ms", { time - pastValue(time, 1) }
 
+	loggerDef = triggerScope, "Trigger Oscilloscope", csv
+		startCommand = "@#TS_SET_LOGGER_SWITCH#@\x04"
+		stopCommand  = "@#TS_SET_LOGGER_SWITCH#@\x05"
+		dataReadCommand = "@#TS_SET_LOGGER_SWITCH#@\x06"
+		dataReadTimeout = 1000
+		dataReadyCondition = { triggerScopeReady }
+		continuousRead = true
+
+		recordDef = 0, 0, 4
+		recordField = lowBits1, "low", 0, 8, 1, "" , hidden
+		recordField = highBits1, "high", 8, 8, 1, "" , hidden
+		calcField = channel1, "Channel 1", "v", { 6.6 / 4096 * ( lowBits1 * 256 + highBits1 ) }
+		recordField = lowBits2, "low2", 16, 8, 1, "" , hidden
+		recordField = highBits2, "high2", 24, 8, 1, "" , hidden
+		calcField = channel2, "Channel 2", "v", { 6.6 / 4096 * ( lowBits2 * 256 + highBits2 ) }
 
 [VeAnalyze]
 

firmware/util/efilib.h

@@ -123,4 +123,31 @@ static constexpr Gpio operator+(size_t a, Gpio b) {
 	return b + a;
 }
 
+namespace efi
+{
+template <class _Ty>
+struct remove_reference {
+    using type = _Ty;
+};
+
+template <class _Ty>
+struct remove_reference<_Ty&> {
+    using type = _Ty;
+};
+
+template <class _Ty>
+struct remove_reference<_Ty&&> {
+    using type = _Ty;
+};
+
+template <class _Ty>
+using remove_reference_t = typename remove_reference<_Ty>::type;
+
+// FUNCTION TEMPLATE move
+template <class _Ty>
+constexpr remove_reference_t<_Ty>&& move(_Ty&& _Arg) noexcept {
+    return static_cast<remove_reference_t<_Ty>&&>(_Arg);
+}
+}
+
 #endif /* __cplusplus */

java_console/io/src/test/java/com/rusefi/binaryprotocol/test/TcpServerSandbox.java

@@ -102,8 +102,7 @@ public class TcpServerSandbox {
         } else if (command == Fields.TS_CRC_CHECK_COMMAND) {
             stream.sendPacket(BinaryProtocolServer.createCrcResponse(TOTALLY_EMPTY_CONFIGURATION));
         } else if (command == Fields.TS_SET_LOGGER_SWITCH) {
-            stream.sendPacket(TS_OK.getBytes());
-        } else if (command == Fields.TS_GET_LOGGER_GET_BUFFER) {
+            if (payload[1] == Fields.TS_COMPOSITE_READ) {
                 ByteArrayOutputStream baos = new ByteArrayOutputStream();
                 baos.write(TS_OK.charAt(0));
                 LittleEndianOutputStream dout = new LittleEndianOutputStream(baos);
@@ -118,6 +117,9 @@ public class TcpServerSandbox {
                     baos.write(100);
                 }
                 stream.sendPacket(baos.toByteArray());
+            } else {
+                stream.sendPacket(TS_OK.getBytes());
+            }
         } else if (command == Fields.TS_OUTPUT_COMMAND) {
             byte[] response = getOutputCommandResponse(payload, ecuState.outputs);
             stream.sendPacket(response);