start making tunerstudio more sane: part 2 (#2381)

* ts cleanup part 1

* move more stuff in to the class

Co-authored-by: Matthew Kennedy <makenne@microsoft.com>
This commit is contained in:
Matthew Kennedy 2021-02-19 17:48:21 -08:00 committed by GitHub
parent 3af1eb69b4
commit a50d1d5ee3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 54 additions and 54 deletions

View File

@ -204,11 +204,11 @@ static constexpr size_t getTunerStudioPageSize() {
} }
void sendOkResponse(ts_channel_s *tsChannel, ts_response_format_e mode) { void sendOkResponse(ts_channel_s *tsChannel, ts_response_format_e mode) {
sr5SendResponse(tsChannel, mode, NULL, 0); tsChannel->sendResponse(mode, NULL, 0);
} }
static void sendErrorCode(ts_channel_s *tsChannel, uint8_t code) { static void sendErrorCode(ts_channel_s *tsChannel, uint8_t code) {
sr5WriteCrcPacket(tsChannel, code, nullptr, 0); tsChannel->writeCrcPacket(code, nullptr, 0);
} }
void TunerStudio::sendErrorCode(ts_channel_s* tsChannel, uint8_t code) { void TunerStudio::sendErrorCode(ts_channel_s* tsChannel, uint8_t code) {
@ -293,7 +293,7 @@ static void handleGetStructContent(ts_channel_s *tsChannel, int structId, int si
// todo: add warning code - unexpected structId // todo: add warning code - unexpected structId
return; return;
} }
sr5SendResponse(tsChannel, TS_CRC, (const uint8_t *)addr, size); tsChannel->sendResponse(TS_CRC, (const uint8_t *)addr, size);
} }
// Validate whether the specified offset and count would cause an overrun in the tune. // Validate whether the specified offset and count would cause an overrun in the tune.
@ -349,7 +349,7 @@ static void handleCrc32Check(ts_channel_s *tsChannel, ts_response_format_e mode,
const char* start = getWorkingPageAddr() + offset; const char* start = getWorkingPageAddr() + offset;
uint32_t crc = SWAP_UINT32(crc32(start, count)); uint32_t crc = SWAP_UINT32(crc32(start, count));
sr5SendResponse(tsChannel, mode, (const uint8_t *) &crc, 4); tsChannel->sendResponse(mode, (const uint8_t *) &crc, 4);
} }
/** /**
@ -403,7 +403,7 @@ static void handlePageReadCommand(ts_channel_s *tsChannel, ts_response_format_e
} }
const uint8_t *addr = (const uint8_t *) (getWorkingPageAddr() + offset); const uint8_t *addr = (const uint8_t *) (getWorkingPageAddr() + offset);
sr5SendResponse(tsChannel, mode, addr, count); tsChannel->sendResponse(mode, addr, count);
#if EFI_TUNER_STUDIO_VERBOSE #if EFI_TUNER_STUDIO_VERBOSE
// scheduleMsg(&tsLogger, "Sending %d done", count); // scheduleMsg(&tsLogger, "Sending %d done", count);
#endif #endif
@ -419,7 +419,7 @@ void requestBurn(void) {
static void sendResponseCode(ts_response_format_e mode, ts_channel_s *tsChannel, const uint8_t responseCode) { static void sendResponseCode(ts_response_format_e mode, ts_channel_s *tsChannel, const uint8_t responseCode) {
if (mode == TS_CRC) { if (mode == TS_CRC) {
sr5WriteCrcPacket(tsChannel, responseCode, nullptr, 0); tsChannel->writeCrcPacket(responseCode, nullptr, 0);
} }
} }
@ -617,7 +617,7 @@ void handleQueryCommand(ts_channel_s *tsChannel, ts_response_format_e mode) {
printTsStats(); printTsStats();
#endif #endif
const char *signature = getTsSignature(); const char *signature = getTsSignature();
sr5SendResponse(tsChannel, mode, (const uint8_t *)signature, strlen(signature) + 1); tsChannel->sendResponse(mode, (const uint8_t *)signature, strlen(signature) + 1);
} }
/** /**
@ -648,7 +648,7 @@ extern CommandHandler console_line_callback;
static void handleGetVersion(ts_channel_s *tsChannel) { static void handleGetVersion(ts_channel_s *tsChannel) {
static char versionBuffer[32]; static char versionBuffer[32];
chsnprintf(versionBuffer, sizeof(versionBuffer), "rusEFI v%d@%s", getRusEfiVersion(), VCS_VERSION); chsnprintf(versionBuffer, sizeof(versionBuffer), "rusEFI v%d@%s", getRusEfiVersion(), VCS_VERSION);
sr5SendResponse(tsChannel, TS_CRC, (const uint8_t *) versionBuffer, strlen(versionBuffer) + 1); tsChannel->sendResponse(TS_CRC, (const uint8_t *) versionBuffer, strlen(versionBuffer) + 1);
} }
static void handleGetText(ts_channel_s *tsChannel) { static void handleGetText(ts_channel_s *tsChannel) {
@ -662,7 +662,7 @@ static void handleGetText(ts_channel_s *tsChannel) {
logMsg("get test sending [%d]\r\n", outputSize); logMsg("get test sending [%d]\r\n", outputSize);
#endif #endif
sr5WriteCrcPacket(tsChannel, TS_RESPONSE_COMMAND_OK, reinterpret_cast<uint8_t*>(output), outputSize); tsChannel->writeCrcPacket(TS_RESPONSE_COMMAND_OK, reinterpret_cast<uint8_t*>(output), outputSize);
#if EFI_SIMULATOR #if EFI_SIMULATOR
logMsg("sent [%d]\r\n", outputSize); logMsg("sent [%d]\r\n", outputSize);
#endif #endif
@ -676,7 +676,7 @@ static void handleExecuteCommand(ts_channel_s *tsChannel, char *data, int incomi
#endif #endif
(console_line_callback)(trimmed); (console_line_callback)(trimmed);
sr5WriteCrcPacket(tsChannel, TS_RESPONSE_COMMAND_OK, nullptr, 0); tsChannel->writeCrcPacket(TS_RESPONSE_COMMAND_OK, nullptr, 0);
} }
/** /**
@ -835,14 +835,14 @@ int TunerStudioBase::handleCrcCommand(ts_channel_s *tsChannel, char *data, int i
if (currentEnd > transmitted) { if (currentEnd > transmitted) {
// more normal case - tail after head // more normal case - tail after head
sr5SendResponse(tsChannel, TS_CRC, start, COMPOSITE_PACKET_SIZE * (currentEnd - transmitted)); tsChannel->sendResponse(TS_CRC, start, COMPOSITE_PACKET_SIZE * (currentEnd - transmitted));
transmitted = currentEnd; transmitted = currentEnd;
} else if (currentEnd == transmitted) { } else if (currentEnd == transmitted) {
sr5SendResponse(tsChannel, TS_CRC, start, 0); tsChannel->sendResponse(TS_CRC, start, 0);
} else { } else {
// we are here if tail of buffer has reached the end of buffer and re-started from the start of buffer // we are here if tail of buffer has reached the end of buffer and re-started from the start of buffer
// sending end of the buffer, next transmission would take care of the rest // sending end of the buffer, next transmission would take care of the rest
sr5SendResponse(tsChannel, TS_CRC, start, COMPOSITE_PACKET_SIZE * (COMPOSITE_PACKET_COUNT - transmitted)); tsChannel->sendResponse(TS_CRC, start, COMPOSITE_PACKET_SIZE * (COMPOSITE_PACKET_COUNT - transmitted));
transmitted = 0; transmitted = 0;
} }
} }
@ -850,7 +850,7 @@ int TunerStudioBase::handleCrcCommand(ts_channel_s *tsChannel, char *data, int i
case TS_GET_LOGGER_GET_BUFFER: case TS_GET_LOGGER_GET_BUFFER:
{ {
auto toothBuffer = GetToothLoggerBuffer(); auto toothBuffer = GetToothLoggerBuffer();
sr5SendResponse(tsChannel, TS_CRC, toothBuffer.Buffer, toothBuffer.Length); tsChannel->sendResponse(TS_CRC, toothBuffer.Buffer, toothBuffer.Length);
} }
break; break;
@ -863,7 +863,7 @@ int TunerStudioBase::handleCrcCommand(ts_channel_s *tsChannel, char *data, int i
case TS_PERF_TRACE_GET_BUFFER: case TS_PERF_TRACE_GET_BUFFER:
{ {
auto trace = perfTraceGetBuffer(); auto trace = perfTraceGetBuffer();
sr5SendResponse(tsChannel, TS_CRC, trace.Buffer, trace.Size); tsChannel->sendResponse(TS_CRC, trace.Buffer, trace.Size);
} }
break; break;
@ -876,7 +876,7 @@ int TunerStudioBase::handleCrcCommand(ts_channel_s *tsChannel, char *data, int i
strcpy(configError, "FACTORY_MODE_PLEASE_CONTACT_SUPPORT"); strcpy(configError, "FACTORY_MODE_PLEASE_CONTACT_SUPPORT");
} }
#endif // HW_CHECK_MODE #endif // HW_CHECK_MODE
sr5SendResponse(tsChannel, TS_CRC, reinterpret_cast<const uint8_t*>(configError), strlen(configError)); tsChannel->sendResponse(TS_CRC, reinterpret_cast<const uint8_t*>(configError), strlen(configError));
break; break;
} }
default: default:

View File

@ -23,7 +23,7 @@ void TunerStudio::cmdOutputChannels(ts_channel_s *tsChannel, uint16_t offset, ui
tsState.outputChannelsCommandCounter++; tsState.outputChannelsCommandCounter++;
prepareTunerStudioOutputs(); prepareTunerStudioOutputs();
// this method is invoked too often to print any debug information // this method is invoked too often to print any debug information
sr5SendResponse(tsChannel, TS_CRC, ((const uint8_t *) &tsOutputChannels) + offset, count); tsChannel->sendResponse(TS_CRC, reinterpret_cast<const uint8_t*>(&tsOutputChannels) + offset, count);
} }
#endif // EFI_TUNER_STUDIO #endif // EFI_TUNER_STUDIO

View File

@ -2,7 +2,7 @@
#include <cstdint> #include <cstdint>
class ts_channel_s; struct ts_channel_s;
class TunerStudioBase { class TunerStudioBase {
public: public:

View File

@ -237,11 +237,11 @@ size_t ts_channel_s::read(uint8_t* buffer, size_t size) {
} }
#endif // EFI_PROD_CODE || EFI_SIMULATOR #endif // EFI_PROD_CODE || EFI_SIMULATOR
void sr5WriteCrcPacketSmall(ts_channel_s* tsChannel, uint8_t responseCode, const uint8_t* buf, size_t size) { void ts_channel_s::writeCrcPacketSmall(uint8_t responseCode, const uint8_t* buf, size_t size) {
auto scratchBuffer = tsChannel->scratchBuffer; auto scratchBuffer = this->scratchBuffer;
// don't transmit too large a buffer // don't transmit too large a buffer
efiAssertVoid(OBD_PCM_Processor_Fault, size <= BLOCKING_FACTOR + 7, "sr5WriteCrcPacket tried to transmit too large a packet") efiAssertVoid(OBD_PCM_Processor_Fault, size <= BLOCKING_FACTOR + 7, "writeCrcPacketSmall tried to transmit too large a packet")
// If transmitting data, copy it in to place in the scratch buffer // If transmitting data, copy it in to place in the scratch buffer
// We want to prevent the data changing itself (higher priority threads could write // We want to prevent the data changing itself (higher priority threads could write
@ -263,10 +263,10 @@ void sr5WriteCrcPacketSmall(ts_channel_s* tsChannel, uint8_t responseCode, const
*reinterpret_cast<uint32_t*>(&scratchBuffer[size + 3]) = SWAP_UINT32(crc); *reinterpret_cast<uint32_t*>(&scratchBuffer[size + 3]) = SWAP_UINT32(crc);
// Write to the underlying stream // Write to the underlying stream
tsChannel->write(reinterpret_cast<uint8_t*>(scratchBuffer), size + 7); write(reinterpret_cast<uint8_t*>(scratchBuffer), size + 7);
} }
void sr5WriteCrcPacketLarge(ts_channel_s* tsChannel, uint8_t responseCode, const uint8_t* buf, size_t size) { void ts_channel_s::writeCrcPacketLarge(uint8_t responseCode, const uint8_t* buf, size_t size) {
uint8_t headerBuffer[3]; uint8_t headerBuffer[3];
uint8_t crcBuffer[4]; uint8_t crcBuffer[4];
@ -280,21 +280,21 @@ void sr5WriteCrcPacketLarge(ts_channel_s* tsChannel, uint8_t responseCode, const
*(uint32_t*)crcBuffer = SWAP_UINT32(crc); *(uint32_t*)crcBuffer = SWAP_UINT32(crc);
// Write header // Write header
tsChannel->write(headerBuffer, sizeof(headerBuffer)); write(headerBuffer, sizeof(headerBuffer));
// If data, write that // If data, write that
if (size) { if (size) {
tsChannel->write(buf, size); write(buf, size);
} }
// Lastly the CRC footer // Lastly the CRC footer
tsChannel->write(crcBuffer, sizeof(crcBuffer)); write(crcBuffer, sizeof(crcBuffer));
} }
/** /**
* Adds size to the beginning of a packet and a crc32 at the end. Then send the packet. * Adds size to the beginning of a packet and a crc32 at the end. Then send the packet.
*/ */
void sr5WriteCrcPacket(ts_channel_s *tsChannel, uint8_t responseCode, const uint8_t* buf, size_t size) { void ts_channel_s::writeCrcPacket(uint8_t responseCode, const uint8_t* buf, size_t size) {
// don't transmit a null buffer... // don't transmit a null buffer...
if (!buf) { if (!buf) {
size = 0; size = 0;
@ -304,31 +304,31 @@ 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, // 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. // because the CAN protocol is already protected by its own checksum.
if ((size + 1) <= 7) { if ((size + 1) <= 7) {
tsChannel->write(&responseCode, 1); // header without size write(&responseCode, 1); // header without size
if (size > 0) { if (size > 0) {
tsChannel->write(buf, size); // body write(buf, size); // body
} }
sr5FlushData(tsChannel); flush();
return; return;
} }
#endif /* TS_CAN_DEVICE */ #endif /* TS_CAN_DEVICE */
if (size <= BLOCKING_FACTOR + 7) { if (size <= BLOCKING_FACTOR + 7) {
// small packets use small packet optimization // small packets use small packet optimization
sr5WriteCrcPacketSmall(tsChannel, responseCode, buf, size); writeCrcPacketSmall(responseCode, buf, size);
} else { } else {
sr5WriteCrcPacketLarge(tsChannel, responseCode, buf, size); writeCrcPacketLarge(responseCode, buf, size);
} }
sr5FlushData(tsChannel); flush();
} }
void sr5SendResponse(ts_channel_s *tsChannel, ts_response_format_e mode, const uint8_t * buffer, int size) { void ts_channel_s::sendResponse(ts_response_format_e mode, const uint8_t * buffer, int size) {
if (mode == TS_CRC) { if (mode == TS_CRC) {
sr5WriteCrcPacket(tsChannel, TS_RESPONSE_OK, buffer, size); writeCrcPacket(TS_RESPONSE_OK, buffer, size);
} else { } else {
if (size > 0) { if (size > 0) {
tsChannel->write(buffer, size); write(buffer, size);
sr5FlushData(tsChannel); flush();
} }
} }
} }
@ -343,12 +343,8 @@ bool sr5IsReady(ts_channel_s *tsChannel) {
return true; return true;
} }
void sr5FlushData(ts_channel_s *tsChannel) { void ts_channel_s::flush() {
#if defined(TS_CAN_DEVICE) #if defined(TS_CAN_DEVICE)
UNUSED(tsChannel);
canFlushTxStream(&TS_CAN_DEVICE); canFlushTxStream(&TS_CAN_DEVICE);
#else
UNUSED(tsChannel);
#endif /* TS_CAN_DEVICE */ #endif /* TS_CAN_DEVICE */
} }

View File

@ -26,6 +26,11 @@ struct ts_channel_s {
void write(const uint8_t* buffer, size_t size); void write(const uint8_t* buffer, size_t size);
size_t readTimeout(uint8_t* buffer, size_t size, int timeout); size_t readTimeout(uint8_t* buffer, size_t size, int timeout);
size_t read(uint8_t* buffer, size_t size); 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 #if ! EFI_UNIT_TEST
BaseChannel * channel = nullptr; BaseChannel * channel = nullptr;
@ -40,6 +45,10 @@ struct ts_channel_s {
#endif // TS_UART_DMA_MODE #endif // TS_UART_DMA_MODE
bool wasReady = false; bool wasReady = false;
private:
void writeCrcPacketSmall(uint8_t responseCode, const uint8_t* buf, size_t size);
void writeCrcPacketLarge(uint8_t responseCode, const uint8_t* buf, size_t size);
}; };
#define CRC_VALUE_SIZE 4 #define CRC_VALUE_SIZE 4
@ -55,11 +64,6 @@ bool stopTsPort(ts_channel_s *tsChannel);
// that's 1 second // that's 1 second
#define SR5_READ_TIMEOUT TIME_MS2I(1000) #define SR5_READ_TIMEOUT TIME_MS2I(1000)
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); void sendOkResponse(ts_channel_s *tsChannel, ts_response_format_e mode);
bool sr5IsReady(ts_channel_s *tsChannel); bool sr5IsReady(ts_channel_s *tsChannel);
void sr5FlushData(ts_channel_s *tsChannel); void sr5FlushData(ts_channel_s *tsChannel);

View File

@ -103,7 +103,7 @@ void handleTsR(ts_channel_s *tsChannel, char *input) {
if (input[0] == 0 && input[1] == TS_SD_PROTOCOL_RTC) { if (input[0] == 0 && input[1] == TS_SD_PROTOCOL_RTC) {
scheduleMsg(&sharedLogger, "TS_SD: RTC read command"); scheduleMsg(&sharedLogger, "TS_SD: RTC read command");
memset(buffer, 0, 9); memset(buffer, 0, 9);
sr5SendResponse(tsChannel, TS_CRC, buffer, 9); tsChannel->sendResponse(TS_CRC, buffer, 9);
} else if (input[0] == 0 && input[1] == TS_SD_PROTOCOL_FETCH_INFO) { } else if (input[0] == 0 && input[1] == TS_SD_PROTOCOL_FETCH_INFO) {
uint16_t length = SWAP_UINT16(data16[2]); uint16_t length = SWAP_UINT16(data16[2]);
@ -125,7 +125,7 @@ void handleTsR(ts_channel_s *tsChannel, char *input) {
buffer[8] = 0; buffer[8] = 0;
buffer[9] = 1; // number of files buffer[9] = 1; // number of files
sr5SendResponse(tsChannel, TS_CRC, buffer, 16); tsChannel->sendResponse(TS_CRC, buffer, 16);
} else if (length == DIR_RESPONSE_BUFFER_SIZE) { } else if (length == DIR_RESPONSE_BUFFER_SIZE) {
// SD read directory command // SD read directory command
memset(buffer, 0, DIR_RESPONSE_BUFFER_SIZE); memset(buffer, 0, DIR_RESPONSE_BUFFER_SIZE);
@ -208,7 +208,7 @@ void handleTsR(ts_channel_s *tsChannel, char *input) {
#endif // EFI_FILE_LOGGING #endif // EFI_FILE_LOGGING
sr5SendResponse(tsChannel, TS_CRC, buffer, tsChannel->sendResponse(TS_CRC, buffer,
DIR_RESPONSE_BUFFER_SIZE); DIR_RESPONSE_BUFFER_SIZE);
} }
} else if (input[0] == 0 && input[1] == TS_SD_PROTOCOL_FETCH_DATA) { } else if (input[0] == 0 && input[1] == TS_SD_PROTOCOL_FETCH_DATA) {
@ -233,7 +233,7 @@ void handleTsR(ts_channel_s *tsChannel, char *input) {
UNLOCK_SD_SPI; UNLOCK_SD_SPI;
#endif // EFI_FILE_LOGGING #endif // EFI_FILE_LOGGING
sr5SendResponse(tsChannel, TS_CRC, buffer, 2 + got); tsChannel->sendResponse(TS_CRC, buffer, 2 + got);
} else { } else {
scheduleMsg(&sharedLogger, "TS_SD: unexpected r"); scheduleMsg(&sharedLogger, "TS_SD: unexpected r");
} }

View File

@ -32,16 +32,16 @@ TEST(binary, testWriteCrc) {
// Let it pick which impl (small vs large) to use // Let it pick which impl (small vs large) to use
sr5TestWriteDataIndex = 0; sr5TestWriteDataIndex = 0;
sr5WriteCrcPacket(&test, CODE, (const uint8_t * )PAYLOAD, SIZE); test.writeCrcPacket(CODE, (const uint8_t*)PAYLOAD, SIZE);
assertCrcPacket(); assertCrcPacket();
// Force the large impl // Force the large impl
sr5TestWriteDataIndex = 0; sr5TestWriteDataIndex = 0;
sr5WriteCrcPacketLarge(&test, CODE, (const uint8_t * )PAYLOAD, SIZE); test.writeCrcPacket(CODE, (const uint8_t*)PAYLOAD, SIZE);
assertCrcPacket(); assertCrcPacket();
// Force the small impl // Force the small impl
sr5TestWriteDataIndex = 0; sr5TestWriteDataIndex = 0;
sr5WriteCrcPacketSmall(&test, CODE, (const uint8_t * )PAYLOAD, SIZE); test.writeCrcPacket(CODE, (const uint8_t*)PAYLOAD, SIZE);
assertCrcPacket(); assertCrcPacket();
} }