Add a timeout for Lua ETB adjustment (#4331)

* lua etb timeout

* mocks

* test

firmware/controllers/actuators/electronic_throttle.cpp

@@ -318,13 +318,16 @@ expected<percent_t> EtbController::getSetpointEtb() {
 	// 100% target from table -> 100% target position
 	idlePosition = interpolateClamped(0, etbIdleAddition, 100, 100, etbCurrentTarget);
 
-	percent_t targetPosition = idlePosition + luaAdjustment;
+	percent_t targetPosition = idlePosition + getLuaAdjustment();
 
 	// Apply any adjustment that this throttle alone needs
 	// Clamped to +-10 to prevent anything too wild
 	trim = clampF(-10, getThrottleTrim(rpm, targetPosition), 10);
 	targetPosition += trim;
 
+	// Clamp before rev limiter to avoid ineffective rev limit due to crazy out of range position target
+	targetPosition = clampF(0, targetPosition, 100);
+
 	// Lastly, apply ETB rev limiter
 	auto etbRpmLimit = engineConfiguration->etbRevLimitStart;
 	if (etbRpmLimit != 0) {
@@ -361,6 +364,21 @@ expected<percent_t> EtbController::getSetpointEtb() {
 	return targetPosition;
 }
 
+void EtbController::setLuaAdjustment(float adjustment) {
+	luaAdjustment = adjustment;
+	m_luaAdjustmentTimer.reset();
+}
+
+float EtbController::getLuaAdjustment() const {
+	// If the lua position hasn't been set in 0.2 second, don't adjust!
+	// This avoids a stuck throttle due to hung/rogue/etc Lua script
+	if (m_luaAdjustmentTimer.getElapsedSeconds() > 0.2f) {
+		return 0;
+	} else {
+		return luaAdjustment;
+	}
+}
+
 percent_t EtbController2::getThrottleTrim(float /*rpm*/, percent_t /*targetPosition*/) const {
 	// TODO: implement me #3680
 	return 0;
@@ -668,7 +686,7 @@ static EtbImpl<EtbController1> etb1;
 static EtbImpl<EtbController2> etb2;
 
 static_assert(ETB_COUNT == 2);
-EtbController* etbControllers[] = { &etb1, &etb2 };
+static EtbController* etbControllers[] = { &etb1, &etb2 };
 
 struct EtbThread final : public PeriodicController<512> {
 	EtbThread() : PeriodicController("ETB", PRIO_ETB, ETB_LOOP_FREQUENCY) {}
@@ -1029,6 +1047,14 @@ void setEtbWastegatePosition(percent_t pos) {
 	}
 }
 
+void setEtbLuaAdjustment(percent_t pos) {
+	for (int i = 0; i < ETB_COUNT; i++) {
+		if (auto etb = engine->etbControllers[i]) {
+			etb->setLuaAdjustment(pos);
+		}
+	}
+}
+
 void set18919_AM810_pedal_position_sensor() {
 	engineConfiguration->throttlePedalUpVoltage = 0.1;
 	engineConfiguration->throttlePedalWOTVoltage = 4.5;

firmware/controllers/actuators/electronic_throttle.h

@@ -15,6 +15,7 @@ void doInitElectronicThrottle();
 
 void setEtbIdlePosition(percent_t pos);
 void setEtbWastegatePosition(percent_t pos);
+void setEtbLuaAdjustment(percent_t adjustment);
 void setHitachiEtbCalibration();
 
 // these two sensors use same plug but have different calibrations and even rotate in different directions
@@ -54,4 +55,6 @@ public:
 	virtual void autoCalibrateTps() = 0;
 
 	virtual const pid_state_s* getPidState() const = 0;
+
+	virtual void setLuaAdjustment(percent_t adjustment) = 0;
 };

firmware/controllers/actuators/electronic_throttle_impl.h

@@ -66,6 +66,10 @@ public:
 		return 0;
 	}
 
+	// Lua throttle adjustment
+	void setLuaAdjustment(percent_t adjustment) override;
+	float getLuaAdjustment() const;
+
 protected:
 	// This is set if an automatic TPS calibration should be run
 	bool m_isAutocal = false;
@@ -103,6 +107,8 @@ private:
 
 	uint8_t m_autotuneCounter = 0;
 	uint8_t m_autotuneCurrentParam = 0;
+
+	Timer m_luaAdjustmentTimer;
 };
 
 class EtbController1 : public EtbController { };

firmware/controllers/lua/lua_hooks.cpp

@@ -543,10 +543,9 @@ void configureRusefiLuaHooks(lua_State* l) {
 #if EFI_PROD_CODE
 	lua_register(l, "setEtbAdd", [](lua_State* l) {
 		auto luaAdjustment = luaL_checknumber(l, 1);
-		for (int i = 0 ; i < ETB_COUNT; i++) {
-			extern EtbController* etbControllers[];
-			etbControllers[i]->luaAdjustment = luaAdjustment;
-		}
+
+		setEtbLuaAdjustment(luaAdjustment);
+
 		return 0;
 	});
 #endif // EFI_PROD_CODE

unit_tests/mocks.h

@@ -25,6 +25,8 @@ public:
 	MOCK_METHOD(void, setWastegatePosition, (percent_t pos), (override));
 	MOCK_METHOD(void, autoCalibrateTps, (), (override));
 	MOCK_METHOD(const pid_state_s*, getPidState, (), (const, override));
+	MOCK_METHOD(void, setLuaAdjustment, (percent_t adjustment), (override));
+
 
 	// ClosedLoopController mocks
 	MOCK_METHOD(expected<percent_t>, getSetpoint, (), (override));

unit_tests/tests/actuators/test_etb.cpp

@@ -474,6 +474,57 @@ TEST(etb, setpointWastegateController) {
 	EXPECT_FLOAT_EQ(100, etb.getSetpoint().value_or(-1));
 }
 
+TEST(etb, setpointLuaAdder) {
+	EngineTestHelper eth(TEST_ENGINE);
+
+	// Must have TPS & PPS initialized for ETB setup
+	Sensor::setMockValue(SensorType::Tps1Primary, 0);
+	Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
+	Sensor::setMockValue(SensorType::AcceleratorPedal, 0.0f, true);
+
+	EtbController etb;
+
+	// Mock pedal map to just return 50%
+	StrictMock<MockVp3d> pedalMap;
+	EXPECT_CALL(pedalMap, getValue(_, _))
+		.WillRepeatedly([](float, float) {
+			return 50;
+		});
+	etb.init(ETB_Throttle1, nullptr, nullptr, &pedalMap, true);
+
+	// No adjustment, should be unadjusted
+	etb.setLuaAdjustment(0);
+	EXPECT_EQ(50, etb.getSetpoint().value_or(-1));
+
+	// Normal adjustments should do as expected
+	etb.setLuaAdjustment(10);
+	EXPECT_EQ(60, etb.getSetpoint().value_or(-1));
+	etb.setLuaAdjustment(-10);
+	EXPECT_EQ(40, etb.getSetpoint().value_or(-1));
+
+	// Crazy adjustments don't cause unreasonable target
+	etb.setLuaAdjustment(1000);
+	EXPECT_EQ(100, etb.getSetpoint().value_or(-1));
+	etb.setLuaAdjustment(-1000);
+	EXPECT_EQ(1, etb.getSetpoint().value_or(-1));
+
+	extern int timeNowUs;
+	int startTime = 1e6;
+	timeNowUs = startTime;
+
+	// Adjustment works immediately after setting
+	etb.setLuaAdjustment(10);
+	EXPECT_EQ(60, etb.getSetpoint().value_or(-1));
+
+	// Adjustment works 0.19 second after setting
+	timeNowUs = startTime + 0.19 * 1e6;
+	EXPECT_EQ(60, etb.getSetpoint().value_or(-1));
+
+	// Adjustment resets to 0 after 0.21 second
+	timeNowUs = startTime + 0.21 * 1e6;
+	EXPECT_EQ(50, etb.getSetpoint().value_or(-1));
+}
+
 TEST(etb, etbTpsSensor) {
 	// Throw some distinct values on the TPS sensors so we can identify that we're getting the correct one
 	Sensor::setMockValue(SensorType::Tps1, 25.0f, true);