From bc85adc74e5d6e06a1c2a53b4cf1ef9a3b68685b Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Fri, 19 Feb 2021 04:40:59 -0800 Subject: [PATCH] start making tunerstudio more sane: part 1 (#2378) * ts cleanup part 1 * unused unused Co-authored-by: Matthew Kennedy --- firmware/bootloader/src/dfu.cpp | 12 ++-- firmware/console/binary/bluetooth.cpp | 6 +- firmware/console/binary/tunerstudio.cpp | 18 +++--- firmware/console/binary/tunerstudio_io.cpp | 65 ++++++++++------------ firmware/console/binary/tunerstudio_io.h | 7 ++- 5 files changed, 51 insertions(+), 57 deletions(-) diff --git a/firmware/bootloader/src/dfu.cpp b/firmware/bootloader/src/dfu.cpp index 9f54b4e3d6..31b75a17c8 100644 --- a/firmware/bootloader/src/dfu.cpp +++ b/firmware/bootloader/src/dfu.cpp @@ -25,11 +25,11 @@ static uint32_t getMcuRevision() { } static bool getByte(uint8_t *b) { - return sr5ReadDataTimeout(&blTsChannel, b, 1, sr5Timeout) == 1; + return blTsChannel.readTimeout(b, 1, sr5Timeout) == 1; } static void sendByte(uint8_t b) { - sr5WriteData(&blTsChannel, &b, 1); + blTsChannel.write(&b, 1); } static uint8_t dfuCalcChecksum(const uint8_t *buf, uint8_t size) { @@ -54,7 +54,7 @@ static bool isInVirtualPageBuffer(uint32_t addr) { // Returns true if all 5 bytes are received and checksum is correct, and false otherwise. static bool readAddress(uint32_t *addr) { uint8_t buf[5]; // 4 bytes+checksum - if (sr5ReadDataTimeout(&blTsChannel, buf, 5, sr5Timeout) != 5) + if (blTsChannel.readTimeout(buf, 5, sr5Timeout) != 5) return false; if (dfuCalcChecksum(buf, 4) != buf[4]) return false; @@ -171,7 +171,7 @@ static void dfuHandleRead(void) { intFlashRead(addr, (char *)buffer, numBytes); // transmit data - sr5WriteData(&blTsChannel, (uint8_t *)buffer, numBytes); + blTsChannel.write(buffer, numBytes); } static void dfuHandleWrite(void) { @@ -188,7 +188,7 @@ static void dfuHandleWrite(void) { int numBytes = buffer[0] + 1; int numBytesAndChecksum = numBytes + 1; // +1 byte of checkSum // receive data - if (sr5ReadDataTimeout(&blTsChannel, buffer + 1, numBytesAndChecksum, sr5Timeout) != numBytesAndChecksum) + if (blTsChannel.readTimeout(buffer + 1, numBytesAndChecksum, sr5Timeout) != numBytesAndChecksum) return; // don't write corrupted data! if (dfuCalcChecksum(buffer, numBytesAndChecksum) != buffer[numBytesAndChecksum]) { @@ -220,7 +220,7 @@ static void dfuHandleErase(void) { numSectorData = (numSectors + 1) * 2 + 1; uint8_t *sectorList = buffer + 2; // read sector data & checksum - if (sr5ReadDataTimeout(&blTsChannel, sectorList, numSectorData, sr5Timeout) != numSectorData) + if (blTsChannel.readTimeout(sectorList, numSectorData, sr5Timeout) != numSectorData) return; // verify checksum if (dfuCalcChecksum(buffer, 2 + numSectorData - 1) != buffer[2 + numSectorData - 1]) { diff --git a/firmware/console/binary/bluetooth.cpp b/firmware/console/binary/bluetooth.cpp index 484001859e..cd1b7344e1 100644 --- a/firmware/console/binary/bluetooth.cpp +++ b/firmware/console/binary/bluetooth.cpp @@ -124,18 +124,18 @@ static void runCommands() { break; // send current command - sr5WriteData(tsChannel, (uint8_t *)commands[cmdIdx], strlen(commands[cmdIdx])); + tsChannel->write((uint8_t*)commands[cmdIdx], strlen(commands[cmdIdx])); // waiting for an answer bool wasAnswer = false; - if (sr5ReadDataTimeout(tsChannel, buffer, 2, btModuleTimeout) == 2) { + if (tsChannel->readTimeout(buffer, 2, btModuleTimeout) == 2) { wasAnswer = (buffer[0] == 'O' && buffer[1] == 'K') || (buffer[0] == '+' && (buffer[1] >= 'A' && buffer[1] <= 'Z')); } // wait 1 second and skip all remaining response bytes from the bluetooth module while (true) { - if (sr5ReadDataTimeout(tsChannel, buffer, 1, btModuleTimeout) < 1) + if (tsChannel->readTimeout(buffer, 1, btModuleTimeout) < 1) break; } diff --git a/firmware/console/binary/tunerstudio.cpp b/firmware/console/binary/tunerstudio.cpp index 1b02c4c3c0..8eca20cf37 100644 --- a/firmware/console/binary/tunerstudio.cpp +++ b/firmware/console/binary/tunerstudio.cpp @@ -484,7 +484,7 @@ static void tsProcessOne(ts_channel_s* tsChannel) { tsState.totalCounter++; uint8_t firstByte; - int received = sr5ReadData(tsChannel, &firstByte, 1); + int received = tsChannel->read(&firstByte, 1); #if EFI_SIMULATOR logMsg("received %d\r\n", received); #endif @@ -504,7 +504,7 @@ static void tsProcessOne(ts_channel_s* tsChannel) { return; uint8_t secondByte; - received = sr5ReadData(tsChannel, &secondByte, 1); + received = tsChannel->read(&secondByte, 1); if (received != 1) { tunerStudioError("TS: ERROR: no second byte"); return; @@ -520,7 +520,7 @@ static void tsProcessOne(ts_channel_s* tsChannel) { return; } - received = sr5ReadData(tsChannel, (uint8_t* )tsChannel->scratchBuffer, 1); + received = tsChannel->read((uint8_t* )tsChannel->scratchBuffer, 1); if (received != 1) { tunerStudioError("ERROR: did not receive command"); sendErrorCode(tsChannel, TS_RESPONSE_UNDERRUN); @@ -538,7 +538,7 @@ static void tsProcessOne(ts_channel_s* tsChannel) { logMsg("command %c\r\n", command); #endif - received = sr5ReadData(tsChannel, (uint8_t * ) (tsChannel->scratchBuffer + 1), + received = tsChannel->read((uint8_t*)(tsChannel->scratchBuffer + 1), incomingPacketSize + CRC_VALUE_SIZE - 1); int expectedSize = incomingPacketSize + CRC_VALUE_SIZE - 1; if (received != expectedSize) { @@ -631,16 +631,16 @@ static void handleTestCommand(ts_channel_s *tsChannel) { * extension of the protocol to simplify troubleshooting */ tunerStudioDebug("got T (Test)"); - sr5WriteData(tsChannel, (const uint8_t *) VCS_VERSION, sizeof(VCS_VERSION)); + tsChannel->write((const uint8_t*)VCS_VERSION, sizeof(VCS_VERSION)); chsnprintf(testOutputBuffer, sizeof(testOutputBuffer), " %d %d", engine->engineState.warnings.lastErrorCode, tsState.testCommandCounter); - sr5WriteData(tsChannel, (const uint8_t *) testOutputBuffer, strlen(testOutputBuffer)); + tsChannel->write((const uint8_t*)testOutputBuffer, strlen(testOutputBuffer)); chsnprintf(testOutputBuffer, sizeof(testOutputBuffer), " uptime=%ds", getTimeNowSeconds()); - sr5WriteData(tsChannel, (const uint8_t *) testOutputBuffer, strlen(testOutputBuffer)); + tsChannel->write((const uint8_t*)testOutputBuffer, strlen(testOutputBuffer)); chsnprintf(testOutputBuffer, sizeof(testOutputBuffer), " %s\r\n", PROTOCOL_TEST_RESPONSE_TAG); - sr5WriteData(tsChannel, (const uint8_t *) testOutputBuffer, strlen(testOutputBuffer)); + tsChannel->write((const uint8_t*)testOutputBuffer, strlen(testOutputBuffer)); } extern CommandHandler console_line_callback; @@ -705,7 +705,7 @@ bool handlePlainCommand(ts_channel_s *tsChannel, uint8_t command) { */ tunerStudioDebug("not ignoring F"); - sr5WriteData(tsChannel, (const uint8_t *) TS_PROTOCOL, strlen(TS_PROTOCOL)); + tsChannel->write((const uint8_t *)TS_PROTOCOL, strlen(TS_PROTOCOL)); return true; } else { // This wasn't a valid command diff --git a/firmware/console/binary/tunerstudio_io.cpp b/firmware/console/binary/tunerstudio_io.cpp index 3aa3a567d3..1c93176db2 100644 --- a/firmware/console/binary/tunerstudio_io.cpp +++ b/firmware/console/binary/tunerstudio_io.cpp @@ -160,46 +160,43 @@ bool stopTsPort(ts_channel_s *tsChannel) { int sr5TestWriteDataIndex = 0; uint8_t st5TestBuffer[16000]; -void sr5WriteData(ts_channel_s *tsChannel, const uint8_t * buffer, int size) { +void ts_channel_s::write(const uint8_t* buffer, size_t size) { memcpy(&st5TestBuffer[sr5TestWriteDataIndex], buffer, size); sr5TestWriteDataIndex += size; } #endif // EFI_UNIT_TEST #if EFI_PROD_CODE || EFI_SIMULATOR -void sr5WriteData(ts_channel_s *tsChannel, const uint8_t * buffer, int size) { +void ts_channel_s::write(const uint8_t* buffer, size_t size) { efiAssertVoid(CUSTOM_ERR_6570, getCurrentRemainingStack() > 64, "tunerStudioWriteData"); #if EFI_SIMULATOR logMsg("chSequentialStreamWrite [%d]\r\n", size); #endif #if (PRIMARY_UART_DMA_MODE || TS_UART_DMA_MODE || TS_UART_MODE) && EFI_PROD_CODE - if (tsChannel->uartp != nullptr) { - int transferred = size; - uartSendTimeout(tsChannel->uartp, (size_t *)&transferred, buffer, BINARY_IO_TIMEOUT); - return; + if (uartp) { + uartSendTimeout(uartp, &size, buffer, BINARY_IO_TIMEOUT); + return; } #elif defined(TS_CAN_DEVICE) - UNUSED(tsChannel); - int transferred = size; - canAddToTxStreamTimeout(&TS_CAN_DEVICE, (size_t *)&transferred, buffer, BINARY_IO_TIMEOUT); + canAddToTxStreamTimeout(&TS_CAN_DEVICE, &size, buffer, BINARY_IO_TIMEOUT); #endif - if (tsChannel->channel == nullptr) + if (!channel) { return; + } // int transferred = chnWriteTimeout(tsChannel->channel, buffer, size, BINARY_IO_TIMEOUT); // temporary attempt to work around #553 // instead of one huge packet let's try sending a few smaller packets - int transferred = 0; - int stillToTransfer = size; + size_t transferred = 0; + size_t stillToTransfer = size; while (stillToTransfer > 0) { int thisTransferSize = minI(stillToTransfer, 768); - transferred += chnWriteTimeout(tsChannel->channel, buffer, thisTransferSize, BINARY_IO_TIMEOUT); + transferred += chnWriteTimeout(channel, buffer, thisTransferSize, BINARY_IO_TIMEOUT); buffer += thisTransferSize; stillToTransfer -= thisTransferSize; } - #if EFI_SIMULATOR logMsg("transferred [%d]\r\n", transferred); #endif @@ -211,36 +208,32 @@ void sr5WriteData(ts_channel_s *tsChannel, const uint8_t * buffer, int size) { } } -int sr5ReadDataTimeout(ts_channel_s *tsChannel, uint8_t * buffer, int size, int timeout) { +size_t ts_channel_s::readTimeout(uint8_t* buffer, size_t size, int timeout) { #if TS_UART_DMA_MODE || PRIMARY_UART_DMA_MODE - if (tsChannel->uartp!= NULL) { + if (uartp) { extern uart_dma_s tsUartDma; - return (int)iqReadTimeout(&tsUartDma.fifoRxQueue, (uint8_t * )buffer, (size_t)size, timeout); + return iqReadTimeout(&tsUartDma.fifoRxQueue, buffer, size, timeout); } #endif #if TS_UART_DMA_MODE #elif TS_UART_MODE - UNUSED(tsChannel); - size_t received = (size_t)size; - uartReceiveTimeout(TS_UART_DEVICE, &received, buffer, timeout); - return (int)received; + uartReceiveTimeout(TS_UART_DEVICE, &size, buffer, timeout); + return size; #elif defined(TS_CAN_DEVICE) - UNUSED(tsChannel); - size_t received = (size_t)size; - canStreamReceiveTimeout(&TS_CAN_DEVICE, &received, buffer, timeout); - return (int)received; + canStreamReceiveTimeout(&TS_CAN_DEVICE, &size, buffer, timeout); + return size; #else /* TS_UART_DMA_MODE */ - if (tsChannel->channel == nullptr) + if (channel == nullptr) return 0; - return chnReadTimeout(tsChannel->channel, (uint8_t * )buffer, size, timeout); + return chnReadTimeout(channel, buffer, size, timeout); #endif /* TS_UART_DMA_MODE */ firmwareError(CUSTOM_ERR_6126, "Unexpected channel situation"); return 0; } -int sr5ReadData(ts_channel_s *tsChannel, uint8_t * buffer, int size) { - return sr5ReadDataTimeout(tsChannel, buffer, size, SR5_READ_TIMEOUT); +size_t ts_channel_s::read(uint8_t* buffer, size_t size) { + return readTimeout(buffer, size, SR5_READ_TIMEOUT); } #endif // EFI_PROD_CODE || EFI_SIMULATOR @@ -270,7 +263,7 @@ void sr5WriteCrcPacketSmall(ts_channel_s* tsChannel, uint8_t responseCode, const *reinterpret_cast(&scratchBuffer[size + 3]) = SWAP_UINT32(crc); // Write to the underlying stream - sr5WriteData(tsChannel, reinterpret_cast(scratchBuffer), size + 7); + tsChannel->write(reinterpret_cast(scratchBuffer), size + 7); } void sr5WriteCrcPacketLarge(ts_channel_s* tsChannel, uint8_t responseCode, const uint8_t* buf, size_t size) { @@ -287,15 +280,15 @@ void sr5WriteCrcPacketLarge(ts_channel_s* tsChannel, uint8_t responseCode, const *(uint32_t*)crcBuffer = SWAP_UINT32(crc); // Write header - sr5WriteData(tsChannel, headerBuffer, sizeof(headerBuffer)); + tsChannel->write(headerBuffer, sizeof(headerBuffer)); // If data, write that if (size) { - sr5WriteData(tsChannel, buf, size); + tsChannel->write(buf, size); } // Lastly the CRC footer - sr5WriteData(tsChannel, crcBuffer, sizeof(crcBuffer)); + tsChannel->write(crcBuffer, sizeof(crcBuffer)); } /** @@ -311,9 +304,9 @@ void sr5WriteCrcPacket(ts_channel_s *tsChannel, uint8_t responseCode, const uint // a special case for short packets: we can sent them in 1 frame, without CRC & size, // because the CAN protocol is already protected by its own checksum. if ((size + 1) <= 7) { - sr5WriteData(tsChannel, &responseCode, 1); // header without size + tsChannel->write(&responseCode, 1); // header without size if (size > 0) { - sr5WriteData(tsChannel, (const uint8_t*)buf, size); // body + tsChannel->write(buf, size); // body } sr5FlushData(tsChannel); return; @@ -334,7 +327,7 @@ void sr5SendResponse(ts_channel_s *tsChannel, ts_response_format_e mode, const u sr5WriteCrcPacket(tsChannel, TS_RESPONSE_OK, buffer, size); } else { if (size > 0) { - sr5WriteData(tsChannel, buffer, size); + tsChannel->write(buffer, size); sr5FlushData(tsChannel); } } diff --git a/firmware/console/binary/tunerstudio_io.h b/firmware/console/binary/tunerstudio_io.h index 9dcfb09fab..d952491dee 100644 --- a/firmware/console/binary/tunerstudio_io.h +++ b/firmware/console/binary/tunerstudio_io.h @@ -23,6 +23,10 @@ typedef enum { } ts_response_format_e; struct ts_channel_s { + void write(const uint8_t* buffer, size_t size); + size_t readTimeout(uint8_t* buffer, size_t size, int timeout); + size_t read(uint8_t* buffer, size_t size); + #if ! EFI_UNIT_TEST BaseChannel * channel = nullptr; #endif @@ -51,14 +55,11 @@ bool stopTsPort(ts_channel_s *tsChannel); // that's 1 second #define SR5_READ_TIMEOUT TIME_MS2I(1000) -void sr5WriteData(ts_channel_s *tsChannel, const uint8_t * buffer, int size); void sr5WriteCrcPacketSmall(ts_channel_s* tsChannel, uint8_t responseCode, const uint8_t* buf, size_t size); void sr5WriteCrcPacketLarge(ts_channel_s* tsChannel, uint8_t responseCode, const uint8_t* buf, size_t size); void sr5WriteCrcPacket(ts_channel_s *tsChannel, uint8_t responseCode, const uint8_t* buf, size_t size); void sr5SendResponse(ts_channel_s *tsChannel, ts_response_format_e mode, const uint8_t * buffer, int size); void sendOkResponse(ts_channel_s *tsChannel, ts_response_format_e mode); -int sr5ReadData(ts_channel_s *tsChannel, uint8_t * buffer, int size); -int sr5ReadDataTimeout(ts_channel_s *tsChannel, uint8_t * buffer, int size, int timeout); bool sr5IsReady(ts_channel_s *tsChannel); void sr5FlushData(ts_channel_s *tsChannel);