Minor ETB improvements (#1381)

* fix pid reset and pedal failure

* pause control later

* test pauseEtbControl

* update tooltip + field name

* update test

firmware/controllers/actuators/electronic_throttle.cpp

@@ -124,7 +124,7 @@ void EtbController::reset() {
 }
 
 void EtbController::onConfigurationChange(pid_s* previousConfiguration) {
-	if (m_motor && m_pid.isSame(previousConfiguration)) {
+	if (m_motor && !m_pid.isSame(previousConfiguration)) {
 		m_shouldResetPid = true;
 	}
 }
@@ -147,21 +147,17 @@ expected<percent_t> EtbController::getSetpoint() const {
 		return unexpected;
 	}
 
-	if (engineConfiguration->pauseEtbControl) {
-		return unexpected;
-	}
-
 	// If the pedal map hasn't been set, we can't provide a setpoint.
 	if (!m_pedalMap) {
 		return unexpected;
 	}
 
 	auto pedalPosition = Sensor::get(SensorType::AcceleratorPedal);
-	if (!pedalPosition.Valid) {
-		return unexpected;
-	}
 
-	float sanitizedPedal = clampF(0, pedalPosition.Value, 100);
+	// If the pedal has failed, just use 0 position.
+	// This is safer than disabling throttle control - we can at least push the throttle closed
+	// and let the engine idle.
+	float sanitizedPedal = clampF(0, pedalPosition.value_or(0), 100);
 	
 	float rpm = GET_RPM();
 	float targetFromTable = m_pedalMap->getValue(rpm / RPM_1_BYTE_PACKING_MULT, sanitizedPedal);
@@ -301,7 +297,8 @@ void EtbController::setOutput(expected<percent_t> outputValue) {
 
 	if (!m_motor) return;
 
-	if (outputValue) {
+	// If output is valid and we aren't paused, output to motor.
+	if (outputValue && !engineConfiguration->pauseEtbControl) {
 		m_motor->enable();
 		m_motor->set(ETB_PERCENT_TO_DUTY(outputValue.Value));
 	} else {

firmware/integration/rusefi_config.txt

@@ -713,7 +713,7 @@ bit is_enabled_spi_2
 	bit useIdleTimingPidControl
 	bit useTPSBasedVeTable
 	bit is_enabled_spi_4
-	bit pauseEtbControl
+	bit pauseEtbControl;+Disable the electronic throttle motor for testing.\nThis mode is for testing ETB position sensors, etc without actually driving the throttle.
 	bit alignEngineSnifferAtTDC
 	bit useETBforIdleControl;+This setting allows the ETB to act as the idle air control valve and move to regulate the airflow at idle.
 	bit idleIncrementalPidCic

firmware/tunerstudio/rusefi.input

@@ -2793,7 +2793,7 @@ cmd_set_engine_type_default					= "w\x00\x31\x00\x00"
 	dialog = etbDialogLeft
 		field = "https://rusefi.com/s/etb"
 		field = "Detailed status in console",			isVerboseETB
-		field = "Pause ETB control",					pauseEtbControl
+		field = "Disable ETB Motor",					pauseEtbControl
 		field = etbCalibrationOnStart,					etbCalibrationOnStart
 		; we need the term about stepper idle in here, because there's a bug in TS that you can't have different visibility
 		; criteria for the same panel when used in multiple places

unit_tests/tests/test_etb.cpp

@@ -144,9 +144,9 @@ TEST(etb, testSetpointOnlyPedal) {
 	Sensor::setMockValue(SensorType::AcceleratorPedal, 20);
 	EXPECT_EQ(20, etb.getSetpoint().value_or(-1));
 
-	// Test invalid pedal position - should give unexpected
+	// Test invalid pedal position - should give 0 position
 	Sensor::resetMockValue(SensorType::AcceleratorPedal);
-	EXPECT_EQ(etb.getSetpoint(), unexpected);
+	EXPECT_EQ(0, etb.getSetpoint().value_or(-1));
 }
 
 TEST(etb, setpointIdle) {
@@ -223,9 +223,12 @@ TEST(etb, etbTpsSensor) {
 }
 
 TEST(etb, setOutputInvalid) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
+
 	StrictMock<MockMotor> motor;
 
 	EtbController etb;
+	INJECT_ENGINE_REFERENCE(&etb);
 	etb.init(&motor, 0, nullptr, nullptr);
 
 	// Should be disabled in case of unexpected
@@ -235,9 +238,11 @@ TEST(etb, setOutputInvalid) {
 }
 
 TEST(etb, setOutputValid) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
 	StrictMock<MockMotor> motor;
 
 	EtbController etb;
+	INJECT_ENGINE_REFERENCE(&etb);
 	etb.init(&motor, 0, nullptr, nullptr);
 
 	// Should be enabled and value set
@@ -249,10 +254,11 @@ TEST(etb, setOutputValid) {
 }
 
 TEST(etb, setOutputValid2) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
 	StrictMock<MockMotor> motor;
 
 	EtbController etb;
-
+	INJECT_ENGINE_REFERENCE(&etb);
 	etb.init(&motor, 0, nullptr, nullptr);
 
 	// Should be enabled and value set
@@ -264,9 +270,11 @@ TEST(etb, setOutputValid2) {
 }
 
 TEST(etb, setOutputOutOfRangeHigh) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
 	StrictMock<MockMotor> motor;
 
 	EtbController etb;
+	INJECT_ENGINE_REFERENCE(&etb);
 	etb.init(&motor, 0, nullptr, nullptr);
 
 	// Should be enabled and value set
@@ -278,9 +286,11 @@ TEST(etb, setOutputOutOfRangeHigh) {
 }
 
 TEST(etb, setOutputOutOfRangeLow) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
 	StrictMock<MockMotor> motor;
 
 	EtbController etb;
+	INJECT_ENGINE_REFERENCE(&etb);
 	etb.init(&motor, 0, nullptr, nullptr);
 
 	// Should be enabled and value set
@@ -291,12 +301,29 @@ TEST(etb, setOutputOutOfRangeLow) {
 	etb.setOutput(-110);
 }
 
+TEST(etb, setOutputPauseControl) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
+	StrictMock<MockMotor> motor;
+
+	EtbController etb;
+	INJECT_ENGINE_REFERENCE(&etb);
+	etb.init(&motor, 0, nullptr, nullptr);
+
+	// Pause control - should get no output
+	engineConfiguration->pauseEtbControl = true;
+
+	// Disable should be called, and set shouldn't be called
+	EXPECT_CALL(motor, disable());
+
+	etb.setOutput(25.0f);
+}
+
 TEST(etb, closedLoopPid) {
 	pid_s pid = {};
 	pid.pFactor = 5;
 	pid.maxValue = 75;
 	pid.minValue = -60;
-	
+
 	EtbController etb;
 	etb.init(nullptr, 0, &pid, nullptr);