XY Idle VE Table (#3781)

* config & ui

* implement

* test idle VE switching behavior

* use the interface where we can

* s

* re-bump flash version

firmware/controllers/actuators/idle_thread.h

@@ -29,6 +29,8 @@ struct IIdleController {
 	virtual float getOpenLoop(Phase phase, float clt, SensorResult tps, float crankingTaperFraction) = 0;
 	virtual float getClosedLoop(Phase phase, float tps, int rpm, int target) = 0;
 	virtual float getCrankingTaperFraction() const = 0;
+	virtual bool isIdlingOrTaper() const = 0;
+	virtual float getIdleTimingAdjustment(int rpm) = 0;
 };
 
 class IdleController : public IIdleController, public EngineModule, public idle_state_s {
@@ -51,7 +53,7 @@ public:
 	percent_t getRunningOpenLoop(float clt, SensorResult tps) const override;
 	percent_t getOpenLoop(Phase phase, float clt, SensorResult tps, float crankingTaperFraction) override;
 
-	float getIdleTimingAdjustment(int rpm);
+	float getIdleTimingAdjustment(int rpm) override;
 	float getIdleTimingAdjustment(int rpm, int targetRpm, Phase phase);
 
 	// CLOSED LOOP CORRECTION
@@ -61,7 +63,7 @@ public:
 	void onSlowCallback() final;
 
 	// Allow querying state from outside
-	bool isIdlingOrTaper() {
+	bool isIdlingOrTaper() const override {
 		return m_lastPhase == Phase::Idling || (engineConfiguration->useSeparateIdleTablesForCrankingTaper && m_lastPhase == Phase::CrankToIdleTaper);
 	}
 

firmware/controllers/algo/advance_map.cpp

@@ -52,7 +52,7 @@ static angle_t getRunningAdvance(int rpm, float engineLoad) {
 
 	// get advance from the separate table for Idle
 	if (engineConfiguration->useSeparateAdvanceForIdle &&
-	    engine->module<IdleController>().unmock().isIdlingOrTaper()) {
+	    engine->module<IdleController>()->isIdlingOrTaper()) {
 		float idleAdvance = interpolate2d(rpm, config->idleAdvanceBins, config->idleAdvance);
 
 		auto [valid, tps] = Sensor::get(SensorType::DriverThrottleIntent);
@@ -94,7 +94,7 @@ angle_t getAdvanceCorrections(int rpm) {
 		);
 	}
 
-	float pidTimingCorrection = engine->module<IdleController>().unmock().getIdleTimingAdjustment(rpm);
+	float pidTimingCorrection = engine->module<IdleController>()->getIdleTimingAdjustment(rpm);
 
 #if EFI_TUNER_STUDIO
 		engine->outputChannels.timingIatCorrection = iatCorrection;

firmware/controllers/algo/airmass/airmass.cpp

@@ -24,9 +24,13 @@ float AirmassVeModelBase::getVe(int rpm, float load) const {
 
 	auto tps = Sensor::get(SensorType::Tps1);
 	// get VE from the separate table for Idle if idling
-	if (engine->module<IdleController>().unmock().isIdlingOrTaper() &&
+	if (engine->module<IdleController>()->isIdlingOrTaper() &&
 	    tps && engineConfiguration->useSeparateVeForIdle) {
-		percent_t idleVe = interpolate2d(rpm, config->idleVeBins, config->idleVe);
+		percent_t idleVe = interpolate3d(
+			config->idleVeTable,
+			config->idleVeLoadBins, load,
+			config->idleVeRpmBins, rpm
+		);
 		// interpolate between idle table and normal (running) table using TPS threshold
 		ve = interpolateClamped(0.0f, idleVe, engineConfiguration->idlePidDeactivationTpsThreshold, ve, tps.Value);
 	}

firmware/controllers/algo/airmass/airmass.h

@@ -16,7 +16,6 @@ class AirmassVeModelBase : public AirmassModelBase {
 public:
 	explicit AirmassVeModelBase(const ValueProvider3D& veTable);
 
-protected:
 	// Retrieve the user-calibrated volumetric efficiency from the table
 	float getVe(int rpm, percent_t load) const;
 

firmware/controllers/algo/engine.h

@@ -168,7 +168,7 @@ public:
 	type_list<
 		Mockable<InjectorModel>,
 #if EFI_IDLE_CONTROL
-		IdleController,
+		Mockable<IdleController>,
 #endif // EFI_IDLE_CONTROL
 		TriggerScheduler,
 #if EFI_HPFP && EFI_ENGINE_CONTROL

firmware/controllers/engine_controller.cpp

@@ -618,8 +618,11 @@ bool validateConfig() {
 	if (engineConfiguration->iacCoastingBins[1] != 0) { // only validate map if not all zeroes default
 		ensureArrayIsAscending("Idle coasting position", engineConfiguration->iacCoastingBins);
 	}
-	if (config->idleVeBins[1] != 0) { // only validate map if not all zeroes default
-		ensureArrayIsAscending("Idle VE", config->idleVeBins);
+	if (config->idleVeRpmBins[1] != 0) { // only validate map if not all zeroes default
+		ensureArrayIsAscending("Idle VE RPM", config->idleVeRpmBins);
+	}
+	if (config->idleVeLoadBins[1] != 0) { // only validate map if not all zeroes default
+		ensureArrayIsAscending("Idle VE Load", config->idleVeLoadBins);
 	}
 	if (config->idleAdvanceBins[1] != 0) { // only validate map if not all zeroes default
 		ensureArrayIsAscending("Idle timing", config->idleAdvanceBins);

firmware/integration/rusefi_config.txt

@@ -70,7 +70,7 @@
 ! Any time an incompatible change is made to the configuration format stored in flash,
 ! update this string to the current date! It is required to also update TS_SIGNATURE above
 ! when this happens.
-#define FLASH_DATA_VERSION 10007
+#define FLASH_DATA_VERSION 10008
 
 #define LOG_DELIMITER "`"
 
@@ -142,7 +142,7 @@ struct_no_prefix engine_configuration_s
 
 #define ENGINE_NOISE_CURVE_SIZE 8
 #define CLT_TIMING_CURVE_SIZE 8
-#define IDLE_VE_CURVE_SIZE 8
+#define IDLE_VE_SIZE 4
 
 #define TCU_SOLENOID_COUNT 6
 #define TCU_GEAR_COUNT 10
@@ -1528,8 +1528,9 @@ float[CLT_CRANKING_CURVE_SIZE] cltCrankingCorr    ;CLT-based cranking position m
 
 uint8_t[IDLE_ADVANCE_CURVE_SIZE] autoscale idleAdvanceBins;Optional timing advance table for Idle (see useSeparateAdvanceForIdle);"RPM", @@RPM_1_BYTE_PACKING_MULT@@, 0, 0, 18000, 0
 float[IDLE_ADVANCE_CURVE_SIZE] idleAdvance    ;Optional timing advance table for Idle (see useSeparateAdvanceForIdle);"deg", 1, 0, -20, 90, 1
-uint8_t[IDLE_VE_CURVE_SIZE] autoscale idleVeBins;Optional VE table for Idle (see useSeparateVEForIdle);"RPM", @@RPM_1_BYTE_PACKING_MULT@@, 0, 0, 18000, 0
-float[IDLE_VE_CURVE_SIZE] idleVe;    Optional VE table for Idle (see useSeparateVEForIdle);"%", 1, 0, 0, 999, 1
+uint8_t[IDLE_VE_SIZE] autoscale idleVeRpmBins;;"RPM", 10, 0, 0, 2500, 0
+uint8_t[IDLE_VE_SIZE] autoscale idleVeLoadBins;;"load", 1, 0, 0, 100, 0
+uint16_t[IDLE_VE_SIZE x IDLE_VE_SIZE] autoscale idleVeTable;;"%", 0.1, 0, 0, 999, 1
 
 #define LUA_SCRIPT_SIZE 8000
 custom lua_script_t @@LUA_SCRIPT_SIZE@@ string, ASCII, @OFFSET@, @@LUA_SCRIPT_SIZE@@

firmware/tunerstudio/rusefi.input

@@ -562,18 +562,6 @@ enable2ndByteCanID = false
       yBins       = idleAdvance
       gauge       = RPMGauge
 
-	curve = idleVeCurve, "Idle VE"
-      columnLabel = "RPM", "%"
-      xAxis       = 0, 2400, 13
-      yAxis       = 0, 250, 11
-      xBins       = idleVeBins, RPMValue
-      yBins       = idleVe
-#if LAMBDA
-      gauge       = lambda1Gauge
-#else
-      gauge       = afr1Gauge
-#endif
-    
 	curve = crankingAdvanceCurve, "Cranking Advance Angle"
       columnLabel = "RPM", "degrees"
       xAxis       = 0, 1200, 13
@@ -711,6 +699,13 @@ enable2ndByteCanID = false
       gridOrient  = 250,   0, 340 ; Space 123 rotation of grid in degrees.
       upDownLabel = "(RICHER)", "(LEANER)"
 
+   table = idleVeTableTbl, idleVeTable, "Idle VE"
+      xBins       = idleVeRpmBins,  RPMValue
+      yBins       = idleVeLoadBins, veTableYAxis
+      zBins       = idleVeTable
+      gridOrient  = 250,   0, 340 ; Space 123 rotation of grid in degrees.
+      upDownLabel = "(RICHER)", "(LEANER)"
+
    table = fuelTrimTbl1,  fuelTrimMap1,  "Fuel trim cyl 1",   1
       xBins = fuelTrimRpmBins,  RPMValue
       yBins = fuelTrimLoadBins, veTableYAxis
@@ -1406,7 +1401,7 @@ menuDialog = main
 		subMenu = iacPidMultTbl,			"IAC PID Multiplier", 0, {idleMode == 0 && useIacPidMultTable == 1}
 		subMenu = iacCoastingCurve,			"Coasting IAC Position for Auto-Idle", 0, {useIacTableForCoasting == 1}
 		subMenu = std_separator
-		subMenu = idleVeCurve,              "VE", 0, {useSeparateVeForIdle == 1}
+		subMenu = idleVeTableTbl,              "Idle VE", 0, {useSeparateVeForIdle == 1}
 		subMenu = idleAdvanceCurve,			"Ignition advance", 0, {useSeparateAdvanceForIdle == 1}
 
 	menu = "&Advanced"

unit_tests/mocks.h

@@ -8,6 +8,7 @@
 #include "injector_model.h"
 #include "stepper.h"
 #include "tunerstudio_io.h"
+#include "idle_thread.h"
 
 #include "gmock/gmock.h"
 
@@ -116,3 +117,15 @@ public:
 	MOCK_METHOD(void, write, (const uint8_t* buffer, size_t size, bool isEndOfPacket), (override));
 	MOCK_METHOD(size_t, readTimeout, (uint8_t* buffer, size_t size, int timeout), (override));
 };
+
+class MockIdleController : public IIdleController {
+	MOCK_METHOD(IIdleController::Phase, determinePhase, (int rpm, int targetRpm, SensorResult tps, float vss, float crankingTaperFraction), (override));
+	MOCK_METHOD(int, getTargetRpm, (float clt), (override));
+	MOCK_METHOD(float, getCrankingOpenLoop, (float clt), (const, override));
+	MOCK_METHOD(float, getRunningOpenLoop, (float clt, SensorResult tps), (const, override));
+	MOCK_METHOD(float, getOpenLoop, (IIdleController::Phase phase, float clt, SensorResult tps, float crankingTaperFraction), (override));
+	MOCK_METHOD(float, getClosedLoop, (IIdleController::Phase phase, float tps, int rpm, int target), (override));
+	MOCK_METHOD(float, getCrankingTaperFraction, (), (const, override));
+	MOCK_METHOD(bool, isIdlingOrTaper, (), (const, override));
+	MOCK_METHOD(float, getIdleTimingAdjustment, (int rpm), (override));
+};

unit_tests/tests/ignition_injection/test_fuel_math.cpp

@@ -200,3 +200,50 @@ TEST(FuelMath, CylinderFuelTrim) {
 	EXPECT_NEAR(engine->injectionMass[2], unadjusted * 1.02, EPS4D);
 	EXPECT_NEAR(engine->injectionMass[3], unadjusted * 1.04, EPS4D);
 }
+
+struct MockIdle : public MockIdleController {
+	bool isIdling = false;
+
+	bool isIdlingOrTaper() const override {
+		return isIdling;
+	}
+};
+
+TEST(FuelMath, IdleVeTable) {
+	EngineTestHelper eth(TEST_ENGINE);
+
+	MockAirmass dut;
+
+	// Install mock idle controller
+	MockIdle idler;
+	engine->engineModules.get<IdleController>().set(&idler);
+
+	// Main VE table returns 50
+	EXPECT_CALL(dut.veTable, getValue(_, _)).WillRepeatedly(Return(50));
+
+	// Idle VE table returns 40
+	setTable(config->idleVeTable, 40);
+
+	// Enable separate idle VE table
+	engineConfiguration->useSeparateVeForIdle = true;
+	engineConfiguration->idlePidDeactivationTpsThreshold = 10;
+
+	// Set TPS so this works
+	Sensor::setMockValue(SensorType::Tps1, 0);
+
+	// Gets normal VE table
+	idler.isIdling = false;
+	EXPECT_FLOAT_EQ(dut.getVe(1000, 50), 0.5f);
+
+	// Gets idle VE table
+	idler.isIdling = true;
+	EXPECT_FLOAT_EQ(dut.getVe(1000, 50), 0.4f);
+
+	// As TPS approaches idle threshold, phase-out the idle VE table
+	Sensor::setMockValue(SensorType::Tps1, 2.5f);
+	EXPECT_FLOAT_EQ(dut.getVe(1000, 50), 0.425f);
+	Sensor::setMockValue(SensorType::Tps1, 5.0f);
+	EXPECT_FLOAT_EQ(dut.getVe(1000, 50), 0.45f);
+	Sensor::setMockValue(SensorType::Tps1, 7.5f);
+	EXPECT_FLOAT_EQ(dut.getVe(1000, 50), 0.475f);
+}