rewrite etb tests with gmock (#1330)

firmware/controllers/actuators/electronic_throttle.h

@@ -19,14 +19,14 @@
 class DcMotor;
 class Logging;
 
-class IEtbController : public PeriodicTimerController {
+class IEtbController : public PeriodicTimerController, public ClosedLoopController<percent_t, percent_t> {
 public:
 	DECLARE_ENGINE_PTR;
 	virtual void init(DcMotor *motor, int ownIndex, pid_s *pidParameters) = 0;
 	virtual void reset() = 0;
 };
 
-class EtbController final : public IEtbController, protected ClosedLoopController<percent_t, percent_t> {
+class EtbController final : public IEtbController {
 public:
 	void init(DcMotor *motor, int ownIndex, pid_s *pidParameters) override;
 

unit_tests/tests/test_etb.cpp

@@ -11,38 +11,46 @@
 #include "engine_controller.h"
 #include "sensor.h"
 
+using ::testing::_;
+using ::testing::StrictMock;
+
 class MockEtb : public IEtbController {
 public:
-	// todo: somehow I am failinig to figure out GMOCK syntax here?
-	// todo: convert to GMOCK
-	int resetCount = 0;
-	int startCount = 0;
-	int initCount = 0;
+	// PeriodicTimerController mocks
+	MOCK_METHOD(void, PeriodicTask, (), (override));
+	MOCK_METHOD(int, getPeriodMs, (), (override));
 
-	void reset() {
-		resetCount++;
-	}
+	// IEtbController mocks
+	MOCK_METHOD(void, reset, (), ());
+	MOCK_METHOD(void, Start, (), (override));
+	MOCK_METHOD(void, init, (DcMotor* motor, int ownIndex, pid_s* pidParameters));
 
-	void Start() override {
-		startCount++;
-	}
-
-	void init(DcMotor *motor, int ownIndex, pid_s *pidParameters) {
-		initCount++;
-	};
-
-	void PeriodicTask() {
-	};
-
-	int getPeriodMs() {
-		return 1;
-	};
+	// ClosedLoopController mocks
+	MOCK_METHOD(expected<percent_t>, getSetpoint, (), (const, override));
+	MOCK_METHOD(expected<percent_t>, observePlant, (), (const, override));
+	MOCK_METHOD(expected<percent_t>, getOpenLoop, (percent_t setpoint), (const, override));
+	MOCK_METHOD(expected<percent_t>, getClosedLoop, (percent_t setpoint, percent_t observation), (override));
+	MOCK_METHOD(void, setOutput, (expected<percent_t> outputValue), (override));
 };
 
 
-TEST(etb, singleEtbInitialization) {
+TEST(etb, initializationNoPedal) {
+	StrictMock<MockEtb> mocks[ETB_COUNT];
 
-	MockEtb mocks[ETB_COUNT];
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
+
+	for (int i = 0; i < ETB_COUNT; i++) {
+		engine->etbControllers[i] = &mocks[i];
+	}
+
+	// We expect no throttle init stuff to be called - lack of pedal should disable ETB
+
+	doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE);
+}
+
+TEST(etb, initializationSingleThrottle) {
+
+	StrictMock<MockEtb> mocks[ETB_COUNT];
 
 	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
 
@@ -53,42 +61,75 @@ TEST(etb, singleEtbInitialization) {
 	// Must have a sensor configured before init
 	Sensor::setMockValue(SensorType::AcceleratorPedal, 0);
 
+	// Expect mock0 to be init with index 0, and PID params
+	EXPECT_CALL(mocks[0], init(_, 0, &engineConfiguration->etb));
+	EXPECT_CALL(mocks[0], reset);
+	EXPECT_CALL(mocks[0], Start);
+
+	// We do not expect throttle #2 to be initialized
+
 	doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE);
-
-	// assert that 1st ETB is initialized and started
-	ASSERT_EQ(1, mocks[0].initCount) << "1st init";
-	ASSERT_EQ(1, mocks[0].startCount);
-
-	// assert that 2nd ETB is neither initialized nor started
-	ASSERT_EQ(0, mocks[1].initCount) << "2nd init";
-	ASSERT_EQ(0, mocks[1].startCount) << "2nd start";
-
-
-	// todo: set mock pedal position
-	// todo: set mock ETB throttle position
-	// todo: invoke EtbController#PeriodicTask a few times and assert that duty cycle changes
 }
 
-TEST(etb, testTargetTpsIsFloatBug945) {
+TEST(etb, initializationDualThrottle) {
+	StrictMock<MockEtb> mocks[ETB_COUNT];
+
 	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
 
+	for (int i = 0; i < ETB_COUNT; i++) {
+		engine->etbControllers[i] = &mocks[i];
+	}
+
+	// Must have a sensor configured before init
+	Sensor::setMockValue(SensorType::AcceleratorPedal, 0);
+
+	// The presence of a second TPS indicates dual throttle
+	Sensor::setMockValue(SensorType::Tps2, 25.0f);
+
+	// Expect mock0 to be init with index 0, and PID params
+	EXPECT_CALL(mocks[0], init(_, 0, &engineConfiguration->etb));
+	EXPECT_CALL(mocks[0], reset);
+	EXPECT_CALL(mocks[0], Start);
+
+	// Expect mock1 to be init with index 2, and PID params
+	EXPECT_CALL(mocks[1], init(_, 1, &engineConfiguration->etb));
+	EXPECT_CALL(mocks[1], reset);
+	EXPECT_CALL(mocks[1], Start);
+
+	doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE);
+}
+
+TEST(etb, testSetpointOnlyPedal) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
+
+	// Don't use ETB for idle, we aren't testing that yet - just pedal table for now
+	engineConfiguration->useETBforIdleControl = false;
+
 	// Must have a sensor configured before init
 	Sensor::setMockValue(SensorType::AcceleratorPedal, 0);
 	Sensor::setMockValue(SensorType::Tps1, 0);
 
+	EtbController etb;
+	engine->etbControllers[0] = &etb;
 	doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE);
 
+	// Check endpoints and midpoint
+	Sensor::setMockValue(SensorType::AcceleratorPedal, 0.0f);
+	EXPECT_EQ(0, etb.getSetpoint().value_or(-1));
 	Sensor::setMockValue(SensorType::AcceleratorPedal, 50.0f);
-	engine->etbControllers[0]->PeriodicTask();
-	ASSERT_NEAR(50, engine->engineState.targetFromTable, EPS4D);
+	EXPECT_EQ(50, etb.getSetpoint().value_or(-1));
+	Sensor::setMockValue(SensorType::AcceleratorPedal, 100.0f);
+	EXPECT_EQ(100, etb.getSetpoint().value_or(-1));
 
+	// Test some floating point pedal/output values
 	Sensor::setMockValue(SensorType::AcceleratorPedal, 50.8302f);
-	engine->etbControllers[0]->PeriodicTask();
-	ASSERT_NEAR(50.8302, engine->engineState.targetFromTable, EPS4D);
-
+	EXPECT_NEAR(50.8302, etb.getSetpoint().value_or(-1), EPS4D);
 	Sensor::setMockValue(SensorType::AcceleratorPedal, 51.6605f);
-	engine->etbControllers[0]->PeriodicTask();
-	ASSERT_NEAR(51.6605, engine->engineState.targetFromTable, EPS4D);
+	EXPECT_NEAR(51.6605, etb.getSetpoint().value_or(-1), EPS4D);
+
+	// Test invalid pedal position - should give unexpected
+	Sensor::resetMockValue(SensorType::AcceleratorPedal);
+	EXPECT_EQ(etb.getSetpoint(), unexpected);
 }
 
 TEST(etb, etbTpsSensor) {