From 21c2695c93dbe3a4dff4fcfe59de92fd39264f47 Mon Sep 17 00:00:00 2001 From: rusefi Date: Mon, 7 Jan 2019 23:55:56 -0500 Subject: [PATCH] #35 refactoring towards making class testable --- firmware/console/status_loop.cpp | 11 ++--- firmware/hw_layer/HIP9011.cpp | 64 +++++++++++++++-------------- firmware/hw_layer/HIP9011_logic.cpp | 10 ++++- firmware/hw_layer/HIP9011_logic.h | 13 ++++-- unit_tests/tests/test_hip9011.cpp | 2 +- 5 files changed, 59 insertions(+), 41 deletions(-) diff --git a/firmware/console/status_loop.cpp b/firmware/console/status_loop.cpp index fff1d30dff..741a541fd7 100644 --- a/firmware/console/status_loop.cpp +++ b/firmware/console/status_loop.cpp @@ -26,6 +26,7 @@ #include "global.h" #include "status_loop.h" +#include "HIP9011_logic.h" #include "adc_inputs.h" #if EFI_WAVE_ANALYZER || defined(__DOXYGEN__) @@ -659,8 +660,7 @@ static void lcdThread(void *arg) { } #if EFI_HIP_9011 || defined(__DOXYGEN__) -extern int correctResponsesCount; -extern int invalidHip9011ResponsesCount; +extern HIP9011 instance; #endif /* EFI_HIP_9011 */ #if EFI_TUNER_STUDIO || defined(__DOXYGEN__) @@ -745,7 +745,7 @@ void updateTunerStudioState(TunerStudioOutputChannels *tsOutputChannels DECLARE_ tsOutputChannels->isWarnNow = isWarningNow(timeSeconds, true); tsOutputChannels->isCltBroken = engine->isCltBroken; #if EFI_HIP_9011 || defined(__DOXYGEN__) - tsOutputChannels->isKnockChipOk = (invalidHip9011ResponsesCount == 0); + tsOutputChannels->isKnockChipOk = (instance.invalidHip9011ResponsesCount == 0); #endif /* EFI_HIP_9011 */ switch (engineConfiguration->debugMode) { @@ -812,8 +812,9 @@ void updateTunerStudioState(TunerStudioOutputChannels *tsOutputChannels DECLARE_ break; #if EFI_HIP_9011 || defined(__DOXYGEN__) case DBG_KNOCK: - tsOutputChannels->debugIntField1 = correctResponsesCount; - tsOutputChannels->debugIntField2 = invalidHip9011ResponsesCount; + // todo: maybe extract hipPostState(tsOutputChannels); + tsOutputChannels->debugIntField1 = instance.correctResponsesCount; + tsOutputChannels->debugIntField2 = instance.invalidHip9011ResponsesCount; break; #endif /* EFI_HIP_9011 */ #if EFI_CJ125 || defined(__DOXYGEN__) diff --git a/firmware/hw_layer/HIP9011.cpp b/firmware/hw_layer/HIP9011.cpp index ced6f68cab..14e59bfe88 100644 --- a/firmware/hw_layer/HIP9011.cpp +++ b/firmware/hw_layer/HIP9011.cpp @@ -59,14 +59,19 @@ extern EnginePins enginePins; uint32_t hipLastExecutionCount; +class Hip9011Hardware: public Hip9011HardwareInterface { + void sendSyncCommand(unsigned char command); + void sendCommand(unsigned char command); +}; + +static Hip9011Hardware hardware; + static float hipValueMax = 0; -static HIP9011 instance; +HIP9011 instance(&hardware); static unsigned char tx_buff[1]; static unsigned char rx_buff[1]; -int correctResponsesCount = 0; -int invalidHip9011ResponsesCount = 0; static char pinNameBuffer[16]; static float currentAngleWindowWidth; @@ -90,9 +95,9 @@ SPI_CR1_MSTR | static void checkResponse(void) { if (tx_buff[0] == rx_buff[0]) { - correctResponsesCount++; + instance.correctResponsesCount++; } else { - invalidHip9011ResponsesCount++; + instance.invalidHip9011ResponsesCount++; } } @@ -104,8 +109,21 @@ static void checkResponse(void) { spiUnselect(driver); \ checkResponse(); + static SPIDriver *driver; +void Hip9011Hardware::sendSyncCommand(unsigned char command) { + SPI_SYNCHRONOUS(command); + chThdSleepMilliseconds(10); +} + +void Hip9011Hardware::sendCommand(unsigned char command) { + tx_buff[0] = command; + + spiSelectI(driver); + spiStartExchangeI(driver, 1, tx_buff, rx_buff); +} + EXTERN_ENGINE ; @@ -133,12 +151,12 @@ static void showHipInfo(void) { instance.currentIntergratorIndex, engineConfiguration->knockVThreshold, engine->knockCount, engineConfiguration->maxKnockSubDeg); - const char * msg = invalidHip9011ResponsesCount > 0 ? "NOT GOOD" : "ok"; + const char * msg = instance.invalidHip9011ResponsesCount > 0 ? "NOT GOOD" : "ok"; scheduleMsg(logger, "spi=%s IntHold@%s/%d response count=%d incorrect response=%d %s", getSpi_device_e(engineConfiguration->hip9011SpiDevice), hwPortname(boardConfiguration->hip9011IntHoldPin), boardConfiguration->hip9011IntHoldPinMode, - correctResponsesCount, invalidHip9011ResponsesCount, + instance.correctResponsesCount, instance.invalidHip9011ResponsesCount, msg); scheduleMsg(logger, "CS@%s updateCount=%d", hwPortname(boardConfiguration->hip9011CsPin), instance.settingUpdateCount); @@ -274,14 +292,6 @@ static void endOfSpiExchange(SPIDriver *spip) { checkResponse(); } -static void sendCommand(hip_state_e s, unsigned char cmd) { - instance.state = s; - tx_buff[0] = cmd; - - spiSelectI(driver); - spiStartExchangeI(driver, 1, tx_buff, rx_buff); -} - void hipAdcCallback(adcsample_t adcValue) { if (instance.state == WAITING_FOR_ADC_TO_SKIP) { instance.state = WAITING_FOR_RESULT_ADC; @@ -306,17 +316,17 @@ void hipAdcCallback(adcsample_t adcValue) { if (instance.currentGainIndex != gainIndex) { instance.currentGainIndex = gainIndex; - sendCommand(IS_SENDING_SPI_COMMAND, SET_GAIN_CMD + gainIndex); + instance.setStateAndCommand(IS_SENDING_SPI_COMMAND, SET_GAIN_CMD + gainIndex); } else if (instance.currentIntergratorIndex != integratorIndex) { instance.currentIntergratorIndex = integratorIndex; - sendCommand(IS_SENDING_SPI_COMMAND, SET_INTEGRATOR_CMD + integratorIndex); + instance.setStateAndCommand(IS_SENDING_SPI_COMMAND, SET_INTEGRATOR_CMD + integratorIndex); } else if (instance.currentBandIndex != bandIndex) { instance.currentBandIndex = bandIndex; - sendCommand(IS_SENDING_SPI_COMMAND, SET_BAND_PASS_CMD + bandIndex); + instance.setStateAndCommand(IS_SENDING_SPI_COMMAND, SET_BAND_PASS_CMD + bandIndex); } else if (instance.currentPrescaler != prescalerIndex) { instance.currentPrescaler = prescalerIndex; - sendCommand(IS_SENDING_SPI_COMMAND, SET_PRESCALER_CMD + prescalerIndex); + instance.setStateAndCommand(IS_SENDING_SPI_COMMAND, SET_PRESCALER_CMD + prescalerIndex); } else { instance.state = READY_TO_INTEGRATE; @@ -339,29 +349,23 @@ static void hipStartupCode(void) { // 0 for 4MHz // 6 for 8 MHz instance.currentPrescaler = engineConfiguration->hip9011PrescalerAndSDO; - SPI_SYNCHRONOUS(SET_PRESCALER_CMD + instance.currentPrescaler); + instance.hardware->sendSyncCommand(SET_PRESCALER_CMD + instance.currentPrescaler); - chThdSleepMilliseconds(10); // '0' for channel #1 - SPI_SYNCHRONOUS(SET_CHANNEL_CMD + 0); - - chThdSleepMilliseconds(10); + instance.hardware->sendSyncCommand(SET_CHANNEL_CMD + 0); // band index depends on cylinder bore - SPI_SYNCHRONOUS(SET_BAND_PASS_CMD + instance.currentBandIndex); + instance.hardware->sendSyncCommand(SET_BAND_PASS_CMD + instance.currentBandIndex); - chThdSleepMilliseconds(10); - if (correctResponsesCount == 0) { + if (instance.correctResponsesCount == 0) { warning(CUSTOM_OBD_KNOCK_PROCESSOR, "TPIC/HIP does not respond"); } if (boardConfiguration->useTpicAdvancedMode) { // enable advanced mode for digital integrator output - SPI_SYNCHRONOUS(SET_ADVANCED_MODE); - - chThdSleepMilliseconds(10); + instance.hardware->sendSyncCommand(SET_ADVANCED_MODE); } /** diff --git a/firmware/hw_layer/HIP9011_logic.cpp b/firmware/hw_layer/HIP9011_logic.cpp index d39042a760..da344a6277 100644 --- a/firmware/hw_layer/HIP9011_logic.cpp +++ b/firmware/hw_layer/HIP9011_logic.cpp @@ -9,7 +9,7 @@ EXTERN_ENGINE; -HIP9011::HIP9011() { +HIP9011::HIP9011(Hip9011HardwareInterface *hardware) { needToInit = true; state = NOT_READY; /** @@ -21,6 +21,14 @@ HIP9011::HIP9011() { settingUpdateCount = 0; totalKnockEventsCount = 0; currentPrescaler = 0; + correctResponsesCount = 0; + invalidHip9011ResponsesCount = 0; + this->hardware = hardware; +} + +void HIP9011::setStateAndCommand(hip_state_e state, unsigned char cmd) { + this->state = state; + hardware->sendCommand(cmd); } #define BAND(bore) (900 / (PIF * (bore) / 2)) diff --git a/firmware/hw_layer/HIP9011_logic.h b/firmware/hw_layer/HIP9011_logic.h index 8a35528104..0de5b99c21 100644 --- a/firmware/hw_layer/HIP9011_logic.h +++ b/firmware/hw_layer/HIP9011_logic.h @@ -13,29 +13,34 @@ #include "hip9011_lookup.h" /** - * this interface defines SPI communication channel with HIP9011 chip + * this interface defines hardware communication layer for HIP9011 chip */ -class HIP9011SpiChannel { +class Hip9011HardwareInterface { public: - void sendCommand(unsigned char command); + virtual void sendSyncCommand(unsigned char command) = 0; + virtual void sendCommand(unsigned char command) = 0; }; class HIP9011 { public: - HIP9011(); + HIP9011(Hip9011HardwareInterface *hardware); void prepareHip9011RpmLookup(float angleWindowWidth); int getIntegrationIndexByRpm(float rpm); + void setStateAndCommand(hip_state_e state, unsigned char cmd); /** * band index is only send to HIP chip on startup */ int currentBandIndex; int currentGainIndex; + int correctResponsesCount; + int invalidHip9011ResponsesCount; int currentIntergratorIndex; bool needToInit; int settingUpdateCount; int totalKnockEventsCount; int currentPrescaler; + Hip9011HardwareInterface *hardware; /** * Int/Hold pin is controlled from scheduler call-backs which are set according to current RPM * diff --git a/unit_tests/tests/test_hip9011.cpp b/unit_tests/tests/test_hip9011.cpp index 5affb369b0..c346d4573c 100644 --- a/unit_tests/tests/test_hip9011.cpp +++ b/unit_tests/tests/test_hip9011.cpp @@ -25,7 +25,7 @@ TEST(hip9011, lookup) { } TEST(hip9011, rpmLookup) { - HIP9011 instace; + HIP9011 instace(NULL); instace.prepareHip9011RpmLookup(50);