yet more TS sanity (#2382)

* ts cleanup part 1

* move more stuff in to the class

* extract base class

* switch to TsChannelBase where we can

* dfu

* more to TsChannelBase

* bad merge

* another bad merge

* bootloader

* bluetooth

* format

Co-authored-by: Matthew Kennedy <makenne@microsoft.com>

firmware/console/binary/tunerstudio.cpp

@@ -203,19 +203,19 @@ static constexpr size_t getTunerStudioPageSize() {
 	return TOTAL_CONFIG_SIZE;
 }
 
-void sendOkResponse(ts_channel_s *tsChannel, ts_response_format_e mode) {
+void sendOkResponse(TsChannelBase *tsChannel, ts_response_format_e mode) {
 	tsChannel->sendResponse(mode, NULL, 0);
 }
 
-static void sendErrorCode(ts_channel_s *tsChannel, uint8_t code) {
+static void sendErrorCode(TsChannelBase *tsChannel, uint8_t code) {
 	tsChannel->writeCrcPacket(code, nullptr, 0);
 }
 
-void TunerStudio::sendErrorCode(ts_channel_s* tsChannel, uint8_t code) {
+void TunerStudio::sendErrorCode(TsChannelBase* tsChannel, uint8_t code) {
 	::sendErrorCode(tsChannel, code);
 }
 
-static void handlePageSelectCommand(ts_channel_s *tsChannel, ts_response_format_e mode) {
+static void handlePageSelectCommand(TsChannelBase *tsChannel, ts_response_format_e mode) {
 	tsState.pageCommandCounter++;
 
 	sendOkResponse(tsChannel, mode);
@@ -285,7 +285,7 @@ static const void * getStructAddr(int structId) {
  * This is somewhat similar to read page and somewhat similar to read outputs
  * We can later consider combining this functionality
  */
-static void handleGetStructContent(ts_channel_s *tsChannel, int structId, int size) {
+static void handleGetStructContent(TsChannelBase* tsChannel, int structId, int size) {
 	tsState.readPageCommandsCounter++;
 
 	const void *addr = getStructAddr(structId);
@@ -298,7 +298,7 @@ static void handleGetStructContent(ts_channel_s *tsChannel, int structId, int si
 
 // Validate whether the specified offset and count would cause an overrun in the tune.
 // Returns true if an overrun would occur.
-static bool validateOffsetCount(size_t offset, size_t count, ts_channel_s *tsChannel) {
+static bool validateOffsetCount(size_t offset, size_t count, TsChannelBase* tsChannel) {
 	if (offset + count > getTunerStudioPageSize()) {
 		scheduleMsg(&tsLogger, "TS: Project mismatch? Too much configuration requested %d/%d", offset, count);
 		tunerStudioError("ERROR: out of range");
@@ -318,7 +318,7 @@ bool rebootForPresetPending = false;
  * This command is needed to make the whole transfer a bit faster
  * @note See also handleWriteValueCommand
  */
-static void handleWriteChunkCommand(ts_channel_s *tsChannel, ts_response_format_e mode, uint16_t offset, uint16_t count,
+static void handleWriteChunkCommand(TsChannelBase* tsChannel, ts_response_format_e mode, uint16_t offset, uint16_t count,
 		void *content) {
 	tsState.writeChunkCommandCounter++;
 
@@ -338,7 +338,7 @@ static void handleWriteChunkCommand(ts_channel_s *tsChannel, ts_response_format_
 	sendOkResponse(tsChannel, mode);
 }
 
-static void handleCrc32Check(ts_channel_s *tsChannel, ts_response_format_e mode, uint16_t offset, uint16_t count) {
+static void handleCrc32Check(TsChannelBase *tsChannel, ts_response_format_e mode, uint16_t offset, uint16_t count) {
 	tsState.crc32CheckCommandCounter++;
 
 	// Ensure we are reading from in bounds
@@ -356,7 +356,7 @@ static void handleCrc32Check(ts_channel_s *tsChannel, ts_response_format_e mode,
  * 'Write' command receives a single value at a given offset
  * @note Writing values one by one is pretty slow
  */
-static void handleWriteValueCommand(ts_channel_s *tsChannel, ts_response_format_e mode, uint16_t offset, uint8_t value) {
+static void handleWriteValueCommand(TsChannelBase* tsChannel, ts_response_format_e mode, uint16_t offset, uint8_t value) {
 	UNUSED(tsChannel);
 	UNUSED(mode);
 
@@ -386,7 +386,7 @@ static void handleWriteValueCommand(ts_channel_s *tsChannel, ts_response_format_
 	}
 }
 
-static void handlePageReadCommand(ts_channel_s *tsChannel, ts_response_format_e mode, uint16_t offset, uint16_t count) {
+static void handlePageReadCommand(TsChannelBase* tsChannel, ts_response_format_e mode, uint16_t offset, uint16_t count) {
 	tsState.readPageCommandsCounter++;
 
 	if (rebootForPresetPending) {
@@ -417,7 +417,7 @@ void requestBurn(void) {
 #endif
 }
 
-static void sendResponseCode(ts_response_format_e mode, ts_channel_s *tsChannel, const uint8_t responseCode) {
+static void sendResponseCode(ts_response_format_e mode, TsChannelBase *tsChannel, const uint8_t responseCode) {
 	if (mode == TS_CRC) {
 		tsChannel->writeCrcPacket(responseCode, nullptr, 0);
 	}
@@ -426,7 +426,7 @@ static void sendResponseCode(ts_response_format_e mode, ts_channel_s *tsChannel,
 /**
  * 'Burn' command is a command to commit the changes
  */
-static void handleBurnCommand(ts_channel_s *tsChannel, ts_response_format_e mode) {
+static void handleBurnCommand(TsChannelBase* tsChannel, ts_response_format_e mode) {
 	efitimems_t nowMs = currentTimeMillis();
 	tsState.burnCommandCounter++;
 
@@ -466,11 +466,10 @@ static bool isKnownCommand(char command) {
 
 TunerStudio tsInstance(&tsLogger);
 
-static void tsProcessOne(ts_channel_s* tsChannel) {
+static void tsProcessOne(TsChannelBase* tsChannel) {
 	validateStack("communication", STACK_USAGE_COMMUNICATION, 128);
 
-	int isReady = sr5IsReady(tsChannel);
+	if (!tsChannel->isReady()) {
-	if (!isReady) {
 		chThdSleepMilliseconds(10);
 		tsChannel->wasReady = false;
 		return;
@@ -571,8 +570,7 @@ static void tsProcessOne(ts_channel_s* tsChannel) {
 		print("got unexpected TunerStudio command %x:%c\r\n", command, command);
 }
 
-void runBinaryProtocolLoop(ts_channel_s *tsChannel)
+void runBinaryProtocolLoop(TsChannelBase* tsChannel) {
-{
 	// Until the end of time, process incoming messages.
 	while(true) {
 		tsProcessOne(tsChannel);
@@ -610,7 +608,7 @@ void tunerStudioError(const char *msg) {
  * Query with CRC takes place while re-establishing connection
  * Query without CRC takes place on TunerStudio startup
  */
-void handleQueryCommand(ts_channel_s *tsChannel, ts_response_format_e mode) {
+void handleQueryCommand(TsChannelBase* tsChannel, ts_response_format_e mode) {
 	tsState.queryCommandCounter++;
 #if EFI_TUNER_STUDIO_VERBOSE
 	scheduleMsg(&tsLogger, "got S/H (queryCommand) mode=%d", mode);
@@ -623,7 +621,7 @@ void handleQueryCommand(ts_channel_s *tsChannel, ts_response_format_e mode) {
 /**
  * rusEfi own test command
  */
-static void handleTestCommand(ts_channel_s *tsChannel) {
+static void handleTestCommand(TsChannelBase* tsChannel) {
 	tsState.testCommandCounter++;
 	static char testOutputBuffer[24];
 	/**
@@ -645,13 +643,13 @@ static void handleTestCommand(ts_channel_s *tsChannel) {
 
 extern CommandHandler console_line_callback;
 
-static void handleGetVersion(ts_channel_s *tsChannel) {
+static void handleGetVersion(TsChannelBase* tsChannel) {
 	static char versionBuffer[32];
 	chsnprintf(versionBuffer, sizeof(versionBuffer), "rusEFI v%d@%s", getRusEfiVersion(), VCS_VERSION);
 	tsChannel->sendResponse(TS_CRC, (const uint8_t *) versionBuffer, strlen(versionBuffer) + 1);
 }
 
-static void handleGetText(ts_channel_s *tsChannel) {
+static void handleGetText(TsChannelBase* tsChannel) {
 	tsState.textCommandCounter++;
 
 	printOverallStatus(getTimeNowSeconds());
@@ -668,7 +666,7 @@ static void handleGetText(ts_channel_s *tsChannel) {
 #endif
 }
 
-static void handleExecuteCommand(ts_channel_s *tsChannel, char *data, int incomingPacketSize) {
+static void handleExecuteCommand(TsChannelBase* tsChannel, char *data, int incomingPacketSize) {
 	data[incomingPacketSize] = 0;
 	char *trimmed = efiTrim(data);
 #if EFI_SIMULATOR
@@ -682,7 +680,7 @@ static void handleExecuteCommand(ts_channel_s *tsChannel, char *data, int incomi
 /**
  * @return true if legacy command was processed, false otherwise
  */
-bool handlePlainCommand(ts_channel_s *tsChannel, uint8_t command) {
+bool handlePlainCommand(TsChannelBase* tsChannel, uint8_t command) {
 	// Bail fast if guaranteed not to be a plain command
 	if (command == 0)
 	{
@@ -715,7 +713,7 @@ bool handlePlainCommand(ts_channel_s *tsChannel, uint8_t command) {
 
 static int transmitted = 0;
 
-int TunerStudioBase::handleCrcCommand(ts_channel_s *tsChannel, char *data, int incomingPacketSize) {
+int TunerStudioBase::handleCrcCommand(TsChannelBase* tsChannel, char *data, int incomingPacketSize) {
 	ScopePerf perf(PE::TunerStudioHandleCrcCommand);
 
 	char command = data[0];

firmware/console/binary/tunerstudio.h

@@ -35,12 +35,12 @@ extern tunerstudio_counters_s tsState;
 /**
  * handle non CRC wrapped command
  */
-bool handlePlainCommand(ts_channel_s *tsChannel, uint8_t command);
+bool handlePlainCommand(TsChannelBase* tsChannel, uint8_t command);
 
 /**
  * this command is part of protocol initialization
  */
-void handleQueryCommand(ts_channel_s *tsChannel, ts_response_format_e mode);
+void handleQueryCommand(TsChannelBase* tsChannel, ts_response_format_e mode);
 
 char *getWorkingPageAddr();
 
@@ -53,7 +53,7 @@ void requestBurn(void);
 
 void startTunerStudioConnectivity(void);
 void syncTunerStudioCopy(void);
-void runBinaryProtocolLoop(ts_channel_s *tsChannel);
+void runBinaryProtocolLoop(TsChannelBase* tsChannel);
 
 #if defined __GNUC__
 // GCC

firmware/console/binary/tunerstudio_commands.cpp

@@ -12,7 +12,7 @@
  * @brief 'Output' command sends out a snapshot of current values
  * Gauges refresh
  */
-void TunerStudio::cmdOutputChannels(ts_channel_s *tsChannel, uint16_t offset, uint16_t count) {
+void TunerStudio::cmdOutputChannels(TsChannelBase* tsChannel, uint16_t offset, uint16_t count) {
 	if (offset + count > sizeof(TunerStudioOutputChannels)) {
 		scheduleMsg(tsLogger, "TS: Version Mismatch? Too much outputs requested %d/%d/%d", offset, count,
 				sizeof(TunerStudioOutputChannels));

firmware/console/binary/tunerstudio_impl.h

@@ -2,14 +2,14 @@
 
 #include <cstdint>
 
-struct ts_channel_s;
+struct TsChannelBase;
 
 class TunerStudioBase {
 public:
-	int handleCrcCommand(ts_channel_s *tsChannel, char *data, int incomingPacketSize);
+	int handleCrcCommand(TsChannelBase* tsChannel, char *data, int incomingPacketSize);
 
 protected:
-	virtual void cmdOutputChannels(ts_channel_s *tsChannel, uint16_t offset, uint16_t count) = 0;
+	virtual void cmdOutputChannels(TsChannelBase* tsChannel, uint16_t offset, uint16_t count) = 0;
 };
 
 class TunerStudio : public TunerStudioBase {
@@ -19,10 +19,10 @@ public:
 	{
 	}
 
-	void cmdOutputChannels(ts_channel_s *tsChannel, uint16_t offset, uint16_t count) override;
+	void cmdOutputChannels(TsChannelBase* tsChannel, uint16_t offset, uint16_t count) override;
 
 private:
-	void sendErrorCode(ts_channel_s* tsChannel, uint8_t code);
+	void sendErrorCode(TsChannelBase* tsChannel, uint8_t code);
 
 	Logging* tsLogger;
 };

firmware/console/binary/tunerstudio_io.cpp

@@ -160,6 +160,11 @@ bool stopTsPort(ts_channel_s *tsChannel) {
 int sr5TestWriteDataIndex = 0;
 uint8_t st5TestBuffer[16000];
 
+size_t ts_channel_s::readTimeout(uint8_t* buffer, size_t size, int timeout) {
+	// unit test, nothing to do here
+	return size;
+}
+
 void ts_channel_s::write(const uint8_t* buffer, size_t size) {
 	memcpy(&st5TestBuffer[sr5TestWriteDataIndex], buffer, size);
 	sr5TestWriteDataIndex += size;
@@ -232,12 +237,12 @@ size_t ts_channel_s::readTimeout(uint8_t* buffer, size_t size, int timeout) {
 	return 0;
 }
 
-size_t ts_channel_s::read(uint8_t* buffer, size_t size) {
+size_t TsChannelBase::read(uint8_t* buffer, size_t size) {
 	return readTimeout(buffer, size, SR5_READ_TIMEOUT);
 }
 #endif // EFI_PROD_CODE || EFI_SIMULATOR
 
-void ts_channel_s::writeCrcPacketSmall(uint8_t responseCode, const uint8_t* buf, size_t size) {
+void TsChannelBase::writeCrcPacketSmall(uint8_t responseCode, const uint8_t* buf, size_t size) {
 	auto scratchBuffer = this->scratchBuffer;
 
 	// don't transmit too large a buffer
@@ -266,7 +271,7 @@ void ts_channel_s::writeCrcPacketSmall(uint8_t responseCode, const uint8_t* buf,
 	write(reinterpret_cast<uint8_t*>(scratchBuffer), size + 7);
 }
 
-void ts_channel_s::writeCrcPacketLarge(uint8_t responseCode, const uint8_t* buf, size_t size) {
+void TsChannelBase::writeCrcPacketLarge(uint8_t responseCode, const uint8_t* buf, size_t size) {
 	uint8_t headerBuffer[3];
 	uint8_t crcBuffer[4];
 
@@ -294,7 +299,7 @@ void ts_channel_s::writeCrcPacketLarge(uint8_t responseCode, const uint8_t* buf,
 /**
  * Adds size to the beginning of a packet and a crc32 at the end. Then send the packet.
  */
-void ts_channel_s::writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size) {
+void TsChannelBase::writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size) {
 	// don't transmit a null buffer...
 	if (!buf) {
 		size = 0;
@@ -322,7 +327,7 @@ void ts_channel_s::writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size
 	flush();
 }
 
-void ts_channel_s::sendResponse(ts_response_format_e mode, const uint8_t * buffer, int size) {
+void TsChannelBase::sendResponse(ts_response_format_e mode, const uint8_t * buffer, int size) {
 	if (mode == TS_CRC) {
 		writeCrcPacket(TS_RESPONSE_OK, buffer, size);
 	} else {
@@ -333,9 +338,9 @@ void ts_channel_s::sendResponse(ts_response_format_e mode, const uint8_t * buffe
 	}
 }
 
-bool sr5IsReady(ts_channel_s *tsChannel) {
+bool ts_channel_s::isReady() {
 #if EFI_USB_SERIAL
-	if (isUsbSerial(tsChannel->channel)) {
+	if (isUsbSerial(this->channel)) {
 		// TS uses USB when console uses serial
 		return is_usb_serial_ready();
 	}

firmware/console/binary/tunerstudio_io.h

@@ -22,28 +22,25 @@ typedef enum {
 	TS_CRC = 1
 } ts_response_format_e;
 
-struct ts_channel_s {
+class TsChannelBase {
-	void write(const uint8_t* buffer, size_t size);
+public:
-	size_t readTimeout(uint8_t* buffer, size_t size, int timeout);
+	// Virtual functions - implement these for your underlying transport
+	virtual void write(const uint8_t* buffer, size_t size) = 0;
+	virtual size_t readTimeout(uint8_t* buffer, size_t size, int timeout) = 0;
+	virtual void flush() = 0;
+	virtual bool isReady() = 0;
+
+	// Base functions that use the above virtual implementation
 	size_t read(uint8_t* buffer, size_t size);
-	void flush();
 
 	void writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size);
-
 	void sendResponse(ts_response_format_e mode, const uint8_t * buffer, int size);
 
-#if ! EFI_UNIT_TEST
-	BaseChannel * channel = nullptr;
-#endif
 	/**
 	 * See 'blockingFactor' in rusefi.ini
 	 */
 	char scratchBuffer[BLOCKING_FACTOR + 30];
 
-#if TS_UART_DMA_MODE || PRIMARY_UART_DMA_MODE || TS_UART_MODE
-	UARTDriver *uartp = nullptr;
-#endif // TS_UART_DMA_MODE
-
 	bool wasReady = false;
 
 private:
@@ -51,6 +48,21 @@ private:
 	void writeCrcPacketLarge(uint8_t responseCode, const uint8_t* buf, size_t size);
 };
 
+struct ts_channel_s : public TsChannelBase {
+	void write(const uint8_t* buffer, size_t size) override;
+	size_t readTimeout(uint8_t* buffer, size_t size, int timeout) override;
+	void flush() override;
+	bool isReady() override;
+
+#if !EFI_UNIT_TEST
+	BaseChannel * channel = nullptr;
+#endif
+
+#if TS_UART_DMA_MODE || PRIMARY_UART_DMA_MODE || TS_UART_MODE
+	UARTDriver *uartp = nullptr;
+#endif // TS_UART_DMA_MODE
+};
+
 #define CRC_VALUE_SIZE 4
 // todo: double-check this
 #define CRC_WRAPPING_SIZE (CRC_VALUE_SIZE + 3)
@@ -64,6 +76,4 @@ bool stopTsPort(ts_channel_s *tsChannel);
 // that's 1 second
 #define SR5_READ_TIMEOUT TIME_MS2I(1000)
 
-void sendOkResponse(ts_channel_s *tsChannel, ts_response_format_e mode);
+void sendOkResponse(TsChannelBase *tsChannel, ts_response_format_e mode);
-bool sr5IsReady(ts_channel_s *tsChannel);
-void sr5FlushData(ts_channel_s *tsChannel);

firmware/hw_layer/mmc_card.h

@@ -20,8 +20,8 @@ bool isSdCardAlive(void);
 
 void readLogFileContent(char *buffer, short fileId, short offset, short length);
 
-void handleTsR(ts_channel_s *tsChannel, char *input);
+void handleTsR(TsChannelBase* tsChannel, char *input);
-void handleTsW(ts_channel_s *tsChannel, char *input);
+void handleTsW(TsChannelBase* tsChannel, char *input);
 
 extern spi_device_e mmcSpiDevice;
 #define LOCK_SD_SPI lockSpi(mmcSpiDevice)

firmware/hw_layer/mmc_card_access.cpp

@@ -93,7 +93,7 @@ static void setFileEntry(uint8_t *buffer, int index, const char *fileName,
 	*(uint32_t*) (&buffer[offset + 28]) = fileSize;
 }
 
-void handleTsR(ts_channel_s *tsChannel, char *input) {
+void handleTsR(TsChannelBase* tsChannel, char *input) {
 #if EFI_SIMULATOR
 	printf("TS_SD r %d\n", input[1]);
 #endif // EFI_SIMULATOR
@@ -239,7 +239,7 @@ void handleTsR(ts_channel_s *tsChannel, char *input) {
 	}
 }
 
-void handleTsW(ts_channel_s *tsChannel, char *input) {
+void handleTsW(TsChannelBase* tsChannel, char *input) {
 	const uint16_t *data16 = reinterpret_cast<uint16_t*>(input);
 
 #if EFI_SIMULATOR