#35 refactoring towards making class testable

firmware/hw_layer/HIP9011.cpp

@@ -73,7 +73,6 @@ HIP9011 instance(&hardware);
 static unsigned char tx_buff[1];
 static unsigned char rx_buff[1];
 static char pinNameBuffer[16];
-static float currentAngleWindowWidth;
 
 static scheduling_s startTimer[2];
 static scheduling_s endTimer[2];
@@ -303,34 +302,12 @@ void hipAdcCallback(adcsample_t adcValue) {
 		float angleWindowWidth =
 		engineConfiguration->knockDetectionWindowEnd - engineConfiguration->knockDetectionWindowStart;
 
-		if (angleWindowWidth != currentAngleWindowWidth) {
+		instance.setAngleWindowWidth(angleWindowWidth);
-			currentAngleWindowWidth = angleWindowWidth;
-			instance.prepareHip9011RpmLookup(currentAngleWindowWidth);
-		}
 
-		int integratorIndex = instance.getIntegrationIndexByRpm(GET_RPM());
-		int gainIndex = getHip9011GainIndex(PASS_HIP_PARAMS);
-		int bandIndex = getBandIndex(PASS_HIP_PARAMS);
 		int prescalerIndex = engineConfiguration->hip9011PrescalerAndSDO;
 
+		instance.handleValue(GET_RPM(), prescalerIndex DEFINE_PARAM_SUFFIX(PASS_HIP_PARAMS));
 
-		if (instance.currentGainIndex != gainIndex) {
-			instance.currentGainIndex = gainIndex;
-			instance.setStateAndCommand(IS_SENDING_SPI_COMMAND, SET_GAIN_CMD + gainIndex);
-
-		} else if (instance.currentIntergratorIndex != integratorIndex) {
-			instance.currentIntergratorIndex = integratorIndex;
-			instance.setStateAndCommand(IS_SENDING_SPI_COMMAND, SET_INTEGRATOR_CMD + integratorIndex);
-		} else if (instance.currentBandIndex != bandIndex) {
-			instance.currentBandIndex = bandIndex;
-			instance.setStateAndCommand(IS_SENDING_SPI_COMMAND, SET_BAND_PASS_CMD + bandIndex);
-		} else if (instance.currentPrescaler != prescalerIndex) {
-			instance.currentPrescaler = prescalerIndex;
-			instance.setStateAndCommand(IS_SENDING_SPI_COMMAND, SET_PRESCALER_CMD + prescalerIndex);
-
-		} else {
-			instance.state = READY_TO_INTEGRATE;
-		}
 	}
 }
 
@@ -410,10 +387,7 @@ void initHip9011(Logging *sharedLogger) {
 		return;
 
 
-	currentAngleWindowWidth =
+	instance.setAngleWindowWidth(engineConfiguration->knockDetectionWindowEnd - engineConfiguration->knockDetectionWindowStart);
-	engineConfiguration->knockDetectionWindowEnd - engineConfiguration->knockDetectionWindowStart;
-
-	instance.prepareHip9011RpmLookup(currentAngleWindowWidth);
 
 #if EFI_PROD_CODE
 	driver = getSpiDevice(engineConfiguration->hip9011SpiDevice);

firmware/hw_layer/HIP9011.h

@@ -10,21 +10,6 @@
 #define HIP9011_H_
 
 
-// 0b01000000
-#define SET_PRESCALER_CMD 0x40
-
-// 0b11100000
-#define SET_CHANNEL_CMD 0xE0
-
-// 0b11000000
-#define SET_INTEGRATOR_CMD 0xC0
-
-// 0b00000000
-#define SET_BAND_PASS_CMD 0x0
-
-// 0b10000000
-#define SET_GAIN_CMD 0x80
-
 // 0b01110001
 #define SET_ADVANCED_MODE 0x71
 

firmware/hw_layer/HIP9011_logic.cpp

@@ -23,11 +23,13 @@ HIP9011::HIP9011(Hip9011HardwareInterface *hardware) {
 	currentPrescaler = 0;
 	correctResponsesCount = 0;
 	invalidHip9011ResponsesCount = 0;
+	angleWindowWidth = -1;
+
 	this->hardware = hardware;
 }
 
-void HIP9011::setStateAndCommand(hip_state_e state, unsigned char cmd) {
+void HIP9011::setStateAndCommand(unsigned char cmd) {
-	this->state = state;
+	this->state = IS_SENDING_SPI_COMMAND;
 	hardware->sendCommand(cmd);
 }
 
@@ -75,3 +77,37 @@ int HIP9011::getIntegrationIndexByRpm(float rpm) {
 	int i = findIndexMsg("getIbR", rpmLookup, INT_LOOKUP_SIZE, (rpm));
 	return i == -1 ? INT_LOOKUP_SIZE - 1 : INT_LOOKUP_SIZE - i - 1;
 }
+
+void HIP9011::setAngleWindowWidth(float angleWindowWidth) {
+	// float '==' is totally appropriate here
+	if (this->angleWindowWidth == angleWindowWidth)
+		return; // exit if value has not change
+	this->angleWindowWidth = angleWindowWidth;
+	prepareHip9011RpmLookup(angleWindowWidth);
+}
+
+void HIP9011::handleValue(int rpm, int prescalerIndex DEFINE_PARAM_SUFFIX(DEFINE_HIP_PARAMS)) {
+	int integratorIndex = getIntegrationIndexByRpm(rpm);
+	int gainIndex = getHip9011GainIndex(FORWARD_HIP_PARAMS);
+	int bandIndex = getBandIndex(FORWARD_HIP_PARAMS);
+
+
+	if (currentGainIndex != gainIndex) {
+		currentGainIndex = gainIndex;
+		setStateAndCommand(SET_GAIN_CMD + gainIndex);
+
+	} else if (currentIntergratorIndex != integratorIndex) {
+		currentIntergratorIndex = integratorIndex;
+		setStateAndCommand(SET_INTEGRATOR_CMD + integratorIndex);
+	} else if (currentBandIndex != bandIndex) {
+		currentBandIndex = bandIndex;
+		setStateAndCommand(SET_BAND_PASS_CMD + bandIndex);
+	} else if (currentPrescaler != prescalerIndex) {
+		currentPrescaler = prescalerIndex;
+		setStateAndCommand(SET_PRESCALER_CMD + prescalerIndex);
+
+	} else {
+		state = READY_TO_INTEGRATE;
+	}
+
+}

firmware/hw_layer/HIP9011_logic.h

@@ -21,12 +21,38 @@ public:
 	virtual void sendCommand(unsigned char command) = 0;
 };
 
+#if EFI_PROD_CODE || EFI_SIMULATOR
+#define PASS_HIP_PARAMS
+#define DEFINE_HIP_PARAMS
+#define GET_CONFIG_VALUE(x) CONFIG(x)
+#define FORWARD_HIP_PARAMS
+#define DEFINE_PARAM_SUFFIX(x)
+#else
+
+#define PASS_HIP_PARAMS CONFIG(knockBandCustom), \
+		CONFIG(cylinderBore), \
+		CONFIG(hip9011Gain)
+
+#define FORWARD_HIP_PARAMS knockBandCustom, \
+		cylinderBore, \
+		hip9011Gain
+
+#define DEFINE_HIP_PARAMS float knockBandCustom,\
+		float cylinderBore, \
+		float hip9011Gain
+
+#define GET_CONFIG_VALUE(x) x
+#define DEFINE_PARAM_SUFFIX(x) , x
+#endif
+
 class HIP9011 {
 public:
 	HIP9011(Hip9011HardwareInterface *hardware);
 	void prepareHip9011RpmLookup(float angleWindowWidth);
 	int getIntegrationIndexByRpm(float rpm);
-	void setStateAndCommand(hip_state_e state, unsigned char cmd);
+	void setStateAndCommand(unsigned char cmd);
+	void setAngleWindowWidth(float angleWindowWidth);
+	void handleValue(int rpm, int prescalerIndex DEFINE_PARAM_SUFFIX(DEFINE_HIP_PARAMS));
 
 	/**
 	 * band index is only send to HIP chip on startup
@@ -35,6 +61,8 @@ public:
 	int currentGainIndex;
 	int correctResponsesCount;
 	int invalidHip9011ResponsesCount;
+	float angleWindowWidth;
+
 	int currentIntergratorIndex;
 	bool needToInit;
 	int settingUpdateCount;
@@ -56,31 +84,23 @@ public:
 	float rpmLookup[INT_LOOKUP_SIZE];
 };
 
-#if EFI_PROD_CODE || EFI_SIMULATOR
-#define PASS_HIP_PARAMS
-#define DEFINE_HIP_PARAMS
-#define GET_CONFIG_VALUE(x) CONFIG(x)
-#define FORWARD_HIP_PARAMS
-#else
-
-#define PASS_HIP_PARAMS CONFIG(knockBandCustom), \
-		CONFIG(cylinderBore), \
-		CONFIG(hip9011Gain)
-
-#define FORWARD_HIP_PARAMS knockBandCustom, \
-		cylinderBore, \
-		hip9011Gain
-
-#define DEFINE_HIP_PARAMS float knockBandCustom,\
-		float cylinderBore, \
-		float hip9011Gain
-
-#define GET_CONFIG_VALUE(x) x
-#endif
-
-
 float getHIP9011Band(DEFINE_HIP_PARAMS);
 int getBandIndex(DEFINE_HIP_PARAMS);
 int getHip9011GainIndex(DEFINE_HIP_PARAMS);
 
+// 0b01000000
+#define SET_PRESCALER_CMD 0x40
+
+// 0b11100000
+#define SET_CHANNEL_CMD 0xE0
+
+// 0b11000000
+#define SET_INTEGRATOR_CMD 0xC0
+
+// 0b00000000
+#define SET_BAND_PASS_CMD 0x0
+
+// 0b10000000
+#define SET_GAIN_CMD 0x80
+
 #endif /* HW_LAYER_HIP9011_LOGIC_H_ */

unit_tests/tests/test_hip9011.cpp

@@ -8,6 +8,7 @@
 #include "hip9011_lookup.h"
 #include "HIP9011_logic.h"
 #include "gtest/gtest.h"
+#include "gmock/gmock.h"
 
 TEST(hip9011, lookup) {
 	assertEqualsM2("", 3183.1013, getRpmByAngleWindowAndTimeUs(600, 360), 0.1);
@@ -43,3 +44,17 @@ TEST(hip9011, band) {
 	EXPECT_EQ(42, getBandIndex(/* knockBandCustom*/0, /*cylinderBore*/76, /*hip9011Gain*/NAN));
 
 }
+
+class MockHip9011Hardware : public Hip9011HardwareInterface
+{
+public:
+	MockHip9011Hardware() {  }
+
+    MOCK_METHOD1(sendSyncCommand, void(unsigned char));
+    MOCK_METHOD1(sendCommand, void(unsigned char));
+};
+
+TEST(hip9011, takeValue) {
+
+	HIP9011 instace(NULL);
+}