diff --git a/firmware/console/binary/tunerstudio.cpp b/firmware/console/binary/tunerstudio.cpp index 4cf75407bd..a06d0ffce5 100644 --- a/firmware/console/binary/tunerstudio.cpp +++ b/firmware/console/binary/tunerstudio.cpp @@ -89,13 +89,6 @@ #include "rusEfiFunctionalTest.h" #endif /* EFI_SIMULATOR */ -#if EFI_TUNER_STUDIO - -/* 1S */ -#define TS_COMMUNICATION_TIMEOUT TIME_MS2I(1000) - -extern persistent_config_container_s persistentState; - #if !defined(EFI_NO_CONFIG_WORKING_COPY) /** * this is a local copy of the configuration. Any changes to this copy @@ -105,12 +98,6 @@ persistent_config_s configWorkingCopy; #endif /* EFI_NO_CONFIG_WORKING_COPY */ -static efitimems_t previousWriteReportMs = 0; - -static void resetTs(void) { - memset(&tsState, 0, sizeof(tsState)); -} - static void printErrorCounters(void) { efiPrintf("TunerStudio size=%d / total=%d / errors=%d / H=%d / O=%d / P=%d / B=%d", sizeof(tsOutputChannels), tsState.totalCounter, tsState.errorCounter, tsState.queryCommandCounter, @@ -119,6 +106,19 @@ static void printErrorCounters(void) { tsState.writeChunkCommandCounter, tsState.pageCommandCounter); } +#if EFI_TUNER_STUDIO + +/* 1S */ +#define TS_COMMUNICATION_TIMEOUT TIME_MS2I(1000) + +extern persistent_config_container_s persistentState; + +static efitimems_t previousWriteReportMs = 0; + +static void resetTs(void) { + memset(&tsState, 0, sizeof(tsState)); +} + void printTsStats(void) { #if EFI_PROD_CODE #ifdef EFI_CONSOLE_RX_BRAIN_PIN @@ -161,6 +161,8 @@ static void bluetoothSPP(const char *baudRate, const char *name, const char *pin } #endif /* EFI_BLUETOOTH_SETUP */ +#endif // EFI_TUNER_STUDIO + void tunerStudioDebug(TsChannelBase* tsChannel, const char *msg) { #if EFI_TUNER_STUDIO_VERBOSE efiPrintf("%s: %s", tsChannel->name, msg); @@ -209,7 +211,7 @@ static void handlePageSelectCommand(TsChannelBase *tsChannel, ts_response_format * On the contrary, 'hard parameters' are waiting for the Burn button to be clicked and configuration version * would be increased and much more complicated logic would be executed. */ -static void onlineApplyWorkingCopyBytes(uint32_t offset, int count) { +static void onlineApplyWorkingCopyBytes(uint32_t offset, int count DECLARE_ENGINE_PARAMETER_SUFFIX) { if (offset >= sizeof(engine_configuration_s)) { int maxSize = sizeof(persistent_config_s) - offset; if (count > maxSize) { @@ -218,7 +220,7 @@ static void onlineApplyWorkingCopyBytes(uint32_t offset, int count) { } efiPrintf("applying soft change from %d length %d", offset, count); #if !defined(EFI_NO_CONFIG_WORKING_COPY) - memcpy(((char*) &persistentState.persistentConfiguration) + offset, ((char*) &configWorkingCopy) + offset, + memcpy(((char*)config) + offset, ((char*) &configWorkingCopy) + offset, count); #endif /* EFI_NO_CONFIG_WORKING_COPY */ @@ -230,6 +232,8 @@ static void onlineApplyWorkingCopyBytes(uint32_t offset, int count) { // is negligable comparing with the IO costs? } +#if EFI_TUNER_STUDIO + static const void * getStructAddr(live_data_e structId) { switch (structId) { case LDS_ENGINE: @@ -277,6 +281,8 @@ static void handleGetStructContent(TsChannelBase* tsChannel, int structId, int s tsChannel->sendResponse(TS_CRC, (const uint8_t *)addr, size); } +#endif // EFI_TUNER_STUDIO + // 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, TsChannelBase* tsChannel) { @@ -299,8 +305,8 @@ bool rebootForPresetPending = false; * This command is needed to make the whole transfer a bit faster * @note See also handleWriteValueCommand */ -static void handleWriteChunkCommand(TsChannelBase* tsChannel, ts_response_format_e mode, uint16_t offset, uint16_t count, - void *content) { +void handleWriteChunkCommand(TsChannelBase* tsChannel, ts_response_format_e mode, uint16_t offset, uint16_t count, + void *content DECLARE_ENGINE_PARAMETER_SUFFIX) { tsState.writeChunkCommandCounter++; efiPrintf("WRITE CHUNK mode=%d o=%d s=%d", mode, offset, count); @@ -313,12 +319,14 @@ static void handleWriteChunkCommand(TsChannelBase* tsChannel, ts_response_format if (!rebootForPresetPending) { uint8_t * addr = (uint8_t *) (getWorkingPageAddr() + offset); memcpy(addr, content, count); - onlineApplyWorkingCopyBytes(offset, count); + onlineApplyWorkingCopyBytes(offset, count PASS_ENGINE_PARAMETER_SUFFIX); } sendOkResponse(tsChannel, mode); } +#if EFI_TUNER_STUDIO + static void handleCrc32Check(TsChannelBase *tsChannel, ts_response_format_e mode, uint16_t offset, uint16_t count) { tsState.crc32CheckCommandCounter++; @@ -386,12 +394,16 @@ static void handlePageReadCommand(TsChannelBase* tsChannel, ts_response_format_e #endif } +#endif // EFI_TUNER_STUDIO + void requestBurn(void) { +#if !EFI_UNIT_TEST onBurnRequest(PASS_ENGINE_PARAMETER_SIGNATURE); #if EFI_INTERNAL_FLASH setNeedToWriteConfiguration(); #endif +#endif // !EFI_UNIT_TEST } static void sendResponseCode(ts_response_format_e mode, TsChannelBase *tsChannel, const uint8_t responseCode) { @@ -403,7 +415,7 @@ static void sendResponseCode(ts_response_format_e mode, TsChannelBase *tsChannel /** * 'Burn' command is a command to commit the changes */ -static void handleBurnCommand(TsChannelBase* tsChannel, ts_response_format_e mode) { +void handleBurnCommand(TsChannelBase* tsChannel, ts_response_format_e mode DECLARE_ENGINE_PARAMETER_SUFFIX) { efitimems_t nowMs = currentTimeMillis(); tsState.burnCommandCounter++; @@ -412,7 +424,7 @@ static void handleBurnCommand(TsChannelBase* tsChannel, ts_response_format_e mod // Skip the burn if a preset was just loaded - we don't want to overwrite it if (!rebootForPresetPending) { #if !defined(EFI_NO_CONFIG_WORKING_COPY) - memcpy(&persistentState.persistentConfiguration, &configWorkingCopy, sizeof(persistent_config_s)); + memcpy(config, &configWorkingCopy, sizeof(persistent_config_s)); #endif /* EFI_NO_CONFIG_WORKING_COPY */ requestBurn(); @@ -422,6 +434,8 @@ static void handleBurnCommand(TsChannelBase* tsChannel, ts_response_format_e mod efiPrintf("BURN in %dms", currentTimeMillis() - nowMs); } +#if EFI_TUNER_STUDIO + static bool isKnownCommand(char command) { return command == TS_HELLO_COMMAND || command == TS_READ_COMMAND || command == TS_OUTPUT_COMMAND || command == TS_PAGE_COMMAND || command == TS_BURN_COMMAND || command == TS_SINGLE_WRITE_COMMAND @@ -567,6 +581,8 @@ void syncTunerStudioCopy(void) { #endif /* EFI_NO_CONFIG_WORKING_COPY */ } +#endif // EFI_TUNER_STUDIO + tunerstudio_counters_s tsState; TunerStudioOutputChannels tsOutputChannels; @@ -576,6 +592,8 @@ void tunerStudioError(TsChannelBase* tsChannel, const char *msg) { tsState.errorCounter++; } +#if EFI_TUNER_STUDIO + /** * Query with CRC takes place while re-establishing connection * Query without CRC takes place on TunerStudio startup diff --git a/firmware/console/binary/tunerstudio.h b/firmware/console/binary/tunerstudio.h index dc6f9a743d..3c1eda3a5e 100644 --- a/firmware/console/binary/tunerstudio.h +++ b/firmware/console/binary/tunerstudio.h @@ -9,10 +9,6 @@ #include "global.h" #include "tunerstudio_io.h" -#if EFI_TUNER_STUDIO -#include "thread_controller.h" -#include "thread_priority.h" - typedef struct { int queryCommandCounter; int outputChannelsCommandCounter; @@ -30,6 +26,16 @@ typedef struct { extern tunerstudio_counters_s tsState; +void tunerStudioDebug(TsChannelBase* tsChannel, const char *msg); +void tunerStudioError(TsChannelBase* tsChannel, const char *msg); + +char *getWorkingPageAddr(); + +#if EFI_TUNER_STUDIO +#include "thread_controller.h" +#include "thread_priority.h" + + #define CONNECTIVITY_THREAD_STACK (2 * UTILITY_THREAD_STACK_SIZE) /** @@ -42,11 +48,6 @@ bool handlePlainCommand(TsChannelBase* tsChannel, uint8_t command); */ void handleQueryCommand(TsChannelBase* tsChannel, ts_response_format_e mode); -char *getWorkingPageAddr(); - -void tunerStudioDebug(TsChannelBase* tsChannel, const char *msg); -void tunerStudioError(TsChannelBase* tsChannel, const char *msg); - void updateTunerStudioState(TunerStudioOutputChannels *tsOutputChannels DECLARE_ENGINE_PARAMETER_SUFFIX); void printTsStats(void); void requestBurn(void); @@ -84,3 +85,6 @@ public: }; #endif /* EFI_TUNER_STUDIO */ + +void handleWriteChunkCommand(TsChannelBase* tsChannel, ts_response_format_e mode, uint16_t offset, uint16_t count, void *content DECLARE_ENGINE_PARAMETER_SUFFIX); +void handleBurnCommand(TsChannelBase* tsChannel, ts_response_format_e mode DECLARE_ENGINE_PARAMETER_SUFFIX); diff --git a/unit_tests/mocks.cpp b/unit_tests/mocks.cpp index 98e56d99ea..6b6aab0f24 100644 --- a/unit_tests/mocks.cpp +++ b/unit_tests/mocks.cpp @@ -32,3 +32,6 @@ MockInjectorModel2::~MockInjectorModel2() { } MockStepperHardware::MockStepperHardware() { } MockStepperHardware::~MockStepperHardware() { } + +MockTsChannel::MockTsChannel() : TsChannelBase("mock") { } +MockTsChannel::~MockTsChannel() { } diff --git a/unit_tests/mocks.h b/unit_tests/mocks.h index db399b93e8..a6fdd134d9 100644 --- a/unit_tests/mocks.h +++ b/unit_tests/mocks.h @@ -7,6 +7,7 @@ #include "airmass.h" #include "injector_model.h" #include "stepper.h" +#include "tunerstudio_io.h" #include "gmock/gmock.h" @@ -106,3 +107,12 @@ public: MOCK_METHOD(bool, step, (bool positive), (override)); }; + +class MockTsChannel : public TsChannelBase { +public: + MockTsChannel(); + virtual ~MockTsChannel(); + + MOCK_METHOD(void, write, (const uint8_t* buffer, size_t size, bool isEndOfPacket), (override)); + MOCK_METHOD(size_t, readTimeout, (uint8_t* buffer, size_t size, int timeout), (override)); +}; diff --git a/unit_tests/tests/test_tunerstudio.cpp b/unit_tests/tests/test_tunerstudio.cpp index d54ba916ef..1814efd0bc 100644 --- a/unit_tests/tests/test_tunerstudio.cpp +++ b/unit_tests/tests/test_tunerstudio.cpp @@ -1,11 +1,12 @@ #include "pch.h" +#include "tunerstudio.h" #include "tunerstudio_io.h" static uint8_t st5TestBuffer[16000]; -class MockTsChannel : public TsChannelBase { +class BufferTsChannel : public TsChannelBase { public: - MockTsChannel() : TsChannelBase("Test") { } + BufferTsChannel() : TsChannelBase("Test") { } void write(const uint8_t* buffer, size_t size, bool /*isLastWriteInTransaction*/) override { memcpy(&st5TestBuffer[writeIdx], buffer, size); @@ -28,7 +29,7 @@ public: #define PAYLOAD "123" #define SIZE strlen(PAYLOAD) -static void assertCrcPacket(MockTsChannel& dut) { +static void assertCrcPacket(BufferTsChannel& dut) { ASSERT_EQ(dut.writeIdx, SIZE + 7); // todo: proper uint16 comparison @@ -48,7 +49,7 @@ static void assertCrcPacket(MockTsChannel& dut) { } TEST(binary, testWriteCrc) { - MockTsChannel test; + BufferTsChannel test; // Let it pick which impl (small vs large) to use test.reset(); @@ -65,3 +66,44 @@ TEST(binary, testWriteCrc) { test.writeCrcPacket(CODE, (const uint8_t*)PAYLOAD, SIZE); assertCrcPacket(test); } + +TEST(TunerstudioCommands, writeChunkEngineConfig) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + MockTsChannel channel; + + uint8_t* configBytes = reinterpret_cast(config); + + // Contains zero before the write + configBytes[100] = 0; + EXPECT_EQ(configBytes[100], 0); + + // two step - writes to the engineConfiguration section require a burn + uint8_t val = 50; + handleWriteChunkCommand(&channel, TS_CRC, 100, 1, &val PASS_ENGINE_PARAMETER_SUFFIX); + + // hasn't changed yet + EXPECT_EQ(configBytes[100], 0); + + handleBurnCommand(&channel, TS_CRC PASS_ENGINE_PARAMETER_SUFFIX); + + EXPECT_EQ(configBytes[100], 50); +} + +TEST(TunerstudioCommands, writeChunkOutsideEngineConfig) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + MockTsChannel channel; + + uint8_t* configBytes = reinterpret_cast(config); + + size_t offset = sizeof(engine_configuration_s) + 100; + + // Contains zero before the write + configBytes[offset] = 0; + EXPECT_EQ(configBytes[offset], 0); + + // one step - writes past engineConfiguration don't need a burn + uint8_t val = 50; + handleWriteChunkCommand(&channel, TS_CRC, offset, 1, &val PASS_ENGINE_PARAMETER_SUFFIX); + + EXPECT_EQ(configBytes[offset], 50); +}