RE usability: live data for idle controller

static bad, class fields better

firmware/controllers/actuators/idle_thread.cpp

@@ -40,24 +40,6 @@
 #include "stepper.h"
 #endif
 
-static efitimeus_t restoreAfterPidResetTimeUs = 0;
-
-static PidIndustrial industrialWithOverrideIdlePid;
-
-#if EFI_IDLE_PID_CIC
-// Use PID with CIC integrator
-static PidCic idleCicPid;
-#endif //EFI_IDLE_PID_CIC
-
-Pid * getIdlePid() {
-#if EFI_IDLE_PID_CIC
-	if (engineConfiguration->useCicPidForIdle) {
-		return &idleCicPid;
-	}
-#endif /* EFI_IDLE_PID_CIC */
-	return &industrialWithOverrideIdlePid;
-}
-
 #if ! EFI_UNIT_TEST
 
 void idleDebug(const char *msg, percent_t value) {
@@ -134,11 +116,12 @@ void setManualIdleValvePosition(int positionPercent) {
 
 #endif /* EFI_UNIT_TEST */
 
-void IdleController::init(pid_s* idlePidConfig) {
+void IdleController::init() {
 	shouldResetPid = false;
 	mightResetPid = false;
 	wasResetPid = false;
-	m_timingPid.initPidClass(idlePidConfig);
+	m_timingPid.initPidClass(&engineConfiguration->idleTimingPid);
+	getIdlePid()->initPidClass(&engineConfiguration->idleRpmPid);
 }
 
 int IdleController::getTargetRpm(float clt) const {
@@ -357,7 +340,7 @@ float IdleController::getClosedLoop(IIdleController::Phase phase, float tpsPos,
 	}
 	// increase the errorAmpCoef slowly to restore the process correctly after the PID reset
 	// todo: move restoreAfterPidResetTimeUs to idle?
-	efitimeus_t timeSincePidResetUs = nowUs - /*engine->idle.*/restoreAfterPidResetTimeUs;
+	efitimeus_t timeSincePidResetUs = nowUs - restoreAfterPidResetTimeUs;
 	// todo: add 'pidAfterResetDampingPeriodMs' setting
 	errorAmpCoef = interpolateClamped(0, 0, MS2US(/*engineConfiguration->pidAfterResetDampingPeriodMs*/1000), errorAmpCoef, timeSincePidResetUs);
 	// If errorAmpCoef > 1.0, then PID thinks that RPM is lower than it is, and controls IAC more aggressively
@@ -485,7 +468,7 @@ void IdleController::onSlowCallback() {
 }
 
 static void applyPidSettings() {
-	getIdlePid()->updateFactors(engineConfiguration->idleRpmPid.pFactor, engineConfiguration->idleRpmPid.iFactor, engineConfiguration->idleRpmPid.dFactor);
+	engine->module<IdleController>().unmock().getIdlePid()->updateFactors(engineConfiguration->idleRpmPid.pFactor, engineConfiguration->idleRpmPid.iFactor, engineConfiguration->idleRpmPid.dFactor);
 }
 
 void setDefaultIdleParameters() {
@@ -559,9 +542,7 @@ void startIdleBench(void) {
 #endif /* EFI_UNIT_TEST */
 
 void startIdleThread() {
-	engine->module<IdleController>().unmock().init(&engineConfiguration->idleTimingPid);
+	engine->module<IdleController>().unmock().init();
-
-	getIdlePid()->initPidClass(&engineConfiguration->idleRpmPid);
 
 #if ! EFI_UNIT_TEST
 	// todo: we still have to explicitly init all hardware on start in addition to handling configuration change via

firmware/controllers/actuators/idle_thread.h

@@ -35,7 +35,7 @@ class IdleController : public IIdleController, public EngineModule, public idle_
 public:
 	typedef IIdleController interface_t;
 
-	void init(pid_s* idlePidConfig);
+	void init();
 
 	float getIdlePosition();
 
@@ -65,10 +65,29 @@ public:
 		return m_lastPhase == Phase::Idling || (engineConfiguration->useSeparateIdleTablesForCrankingTaper && m_lastPhase == Phase::CrankToIdleTaper);
 	}
 
+	PidIndustrial industrialWithOverrideIdlePid;
+
+	#if EFI_IDLE_PID_CIC
+	// Use PID with CIC integrator
+		PidCic idleCicPid;
+	#endif //EFI_IDLE_PID_CIC
+
+	Pid * getIdlePid() {
+	#if EFI_IDLE_PID_CIC
+		if (engineConfiguration->useCicPidForIdle) {
+			return &idleCicPid;
+		}
+	#endif /* EFI_IDLE_PID_CIC */
+		return &industrialWithOverrideIdlePid;
+	}
+
+
 private:
+
 	// These are stored by getIdlePosition() and used by getIdleTimingAdjustment()
 	Phase m_lastPhase = Phase::Cranking;
 	int m_lastTargetRpm = 0;
+	efitimeus_t restoreAfterPidResetTimeUs = 0;
 
 	// This is stored by getClosedLoop and used in case we want to "do nothing"
 	float m_lastAutomaticPosition = 0;
@@ -90,6 +109,5 @@ void setIdleIFactor(float value);
 void setIdleDFactor(float value);
 void setIdleMode(idle_mode_e value);
 void setTargetIdleRpm(int value);
-Pid * getIdlePid();
 void startPedalPins();
 void stopPedalPins();

unit_tests/tests/test_idle_controller.cpp

@@ -24,11 +24,10 @@ TEST(idle_v2, timingPid) {
 
 	engineConfiguration->useIdleTimingPidControl = true;
 
-	pid_s pidCfg{};
+	engineConfiguration->idleTimingPid.pFactor = 0.1;
-	pidCfg.pFactor = 0.1;
+	engineConfiguration->idleTimingPid.minValue = -10;
-	pidCfg.minValue = -10;
+	engineConfiguration->idleTimingPid.maxValue = 10;
-	pidCfg.maxValue = 10;
+	dut.init();
-	dut.init(&pidCfg);
 
 	// Check that out of idle mode it doesn't do anything
 	EXPECT_EQ(0, dut.getIdleTimingAdjustment(1050, 1000, ICP::Cranking));
@@ -300,6 +299,7 @@ extern int timeNowUs;
 TEST(idle_v2, closedLoopBasic) {
 	EngineTestHelper eth(TEST_ENGINE);
 	IdleController dut;
+	dut.init();
 
 	// Not testing PID here, so we can set very simple PID gains
 	engineConfiguration->idleRpmPid.pFactor = 0.5;	// 0.5 output per 1 RPM error = 50% per 100 rpm
@@ -327,6 +327,8 @@ TEST(idle_v2, closedLoopBasic) {
 TEST(idle_v2, closedLoopDeadzone) {
 	EngineTestHelper eth(TEST_ENGINE);
 	IdleController dut;
+	dut.init();
+
 
 	// Not testing PID here, so we can set very simple PID gains
 	engineConfiguration->idleRpmPid.pFactor = 0.5;	// 0.5 output per 1 RPM error = 50% per 100 rpm