apply open loop idle when in automatic mode (#2745)

* always apply open loop in closed loop

* simplify flags

* comment

* move

* comment

* missed one

* config for fan bump

* adjust miata defaults

* test fan/ac open loop

* idle CL tests

* test coasting

* test integration case

* fix merge

* put back reset logic for now

* s

* don't need that part

* I guess use OL as baseIdlePosition

* reset condition
This commit is contained in:
Matthew Kennedy 2021-06-12 11:21:11 -07:00 committed by GitHub
parent e94bf1fc6e
commit 15d172ab08
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 131 additions and 90 deletions

View File

@ -391,11 +391,10 @@ void setMiataNA6_MAP_MRE(DECLARE_CONFIG_PARAMETER_SIGNATURE) {
engineConfiguration->idle_antiwindupFreq = 0.1;
engineConfiguration->idle_derivativeFilterLoss = 0.1;
engineConfiguration->idleRpmPid.dFactor = 0.002;
engineConfiguration->idleRpmPid.offset = 37;
engineConfiguration->idleRpmPid.offset = 0;
engineConfiguration->acIdleExtraOffset = 14;
engineConfiguration->idleRpmPid.minValue = 30;
engineConfiguration->acIdleExtraMin = 14;
engineConfiguration->idleRpmPid.maxValue = 70;
engineConfiguration->idleRpmPid.minValue = -7;
engineConfiguration->idleRpmPid.maxValue = 35;
engineConfiguration->idleRpmPid.periodMs = 40;
engineConfiguration->idlerpmpid_iTermMin = -6;
engineConfiguration->idlerpmpid_iTermMax = 30;
@ -403,14 +402,6 @@ void setMiataNA6_MAP_MRE(DECLARE_CONFIG_PARAMETER_SIGNATURE) {
engineConfiguration->idlePidRpmDeadZone = 25;
engineConfiguration->idlePidRpmUpperLimit = 1000;
engineConfiguration->useFSIO12ForIdleOffset = true;
setFsioExpression(QUOTE(MAGIC_OFFSET_FOR_IDLE_OFFSET), "ac_on_switch 0 cfg_acIdleExtraOffset if" PASS_CONFIG_PARAMETER_SUFFIX);
engineConfiguration->useFSIO13ForIdleMinValue = true;
setFsioExpression(QUOTE(MAGIC_OFFSET_FOR_IDLE_MIN_VALUE), "ac_on_switch 0 cfg_acIdleExtraMin if" PASS_CONFIG_PARAMETER_SUFFIX);
engineConfiguration->useIdleTimingPidControl = true;
engineConfiguration->idleTimingPid.pFactor = 0.05;
engineConfiguration->idleTimingPid.iFactor = 0.0;

View File

@ -312,13 +312,6 @@ float IdleController::getIdleTimingAdjustment(int rpm, int targetRpm, Phase phas
return m_timingPid.getOutput(targetRpm, rpm, FAST_CALLBACK_PERIOD_MS / 1000.0f);
}
static percent_t manualIdleController(float cltCorrection DECLARE_ENGINE_PARAMETER_SUFFIX) {
percent_t correctedPosition = cltCorrection * CONFIG(manIdlePosition);
return correctedPosition;
}
/**
* idle blip is a development tool: alternator PID research for instance have benefited from a repetitive change of RPM
*/
@ -380,8 +373,10 @@ float IdleController::getClosedLoop(IIdleController::Phase phase, float tpsPos,
}
engine->engineState.idle.idleState = TPS_THRESHOLD;
// just leave IAC position as is (but don't return currentIdlePosition - it may already contain additionalAir)
return engine->engineState.idle.baseIdlePosition;
// We aren't idling, so don't apply any correction. A positive correction could inhibit a return to idle.
m_lastAutomaticPosition = 0;
return 0;
}
// #1553 we need to give FSIO variable offset or minValue a chance
@ -390,15 +385,16 @@ float IdleController::getClosedLoop(IIdleController::Phase phase, float tpsPos,
if (!acToggleJustTouched && absI(rpm - targetRpm) <= CONFIG(idlePidRpmDeadZone)) {
engine->engineState.idle.idleState = RPM_DEAD_ZONE;
// current RPM is close enough, no need to change anything
return engine->engineState.idle.baseIdlePosition;
return m_lastAutomaticPosition;
}
// When rpm < targetRpm, there's a risk of dropping RPM too low - and the engine dies out.
// So PID reaction should be increased by adding extra percent to PID-error:
percent_t errorAmpCoef = 1.0f;
if (rpm < targetRpm)
if (rpm < targetRpm) {
errorAmpCoef += (float)CONFIG(pidExtraForLowRpm) / PERCENT_MULT;
}
// if PID was previously reset, we store the time when it turned on back (see errorAmpCoef correction below)
if (wasResetPid) {
restoreAfterPidResetTimeUs = nowUs;
@ -423,31 +419,16 @@ float IdleController::getClosedLoop(IIdleController::Phase phase, float tpsPos,
float engineLoad = getFuelingLoad(PASS_ENGINE_PARAMETER_SIGNATURE);
float multCoef = iacPidMultMap.getValue(rpm / RPM_1_BYTE_PACKING_MULT, engineLoad);
// PID can be completely disabled of multCoef==0, or it just works as usual if multCoef==1
newValue = interpolateClamped(0, engine->engineState.idle.baseIdlePosition, 1.0f, newValue, multCoef);
newValue = interpolateClamped(0, 0, 1, newValue, multCoef);
}
// Apply PID Deactivation Threshold as a smooth taper for TPS transients.
// if tps==0 then PID just works as usual, or we completely disable it if tps>=threshold
newValue = interpolateClamped(0.0f, newValue, CONFIG(idlePidDeactivationTpsThreshold), engine->engineState.idle.baseIdlePosition, tpsPos);
// Interpolate to the manual position when RPM is close to the upper RPM limit (if idlePidRpmUpperLimit is set).
// If RPM increases and the throttle is closed, then we're in coasting mode, and we should smoothly disable auto-pid.
// If we just leave IAC at baseIdlePosition (as in case of TPS deactivation threshold), RPM would get stuck.
// That's why there's 'useIacTableForCoasting' setting which involves a separate IAC position table for coasting (iacCoasting).
// Currently it's user-defined. But eventually we'll use a real calculated and stored IAC position instead.
int idlePidLowerRpm = targetRpm + CONFIG(idlePidRpmDeadZone);
if (CONFIG(idlePidRpmUpperLimit) > 0) {
engine->engineState.idle.idleState = PID_UPPER;
const auto [cltValid, clt] = Sensor::get(SensorType::Clt);
if (CONFIG(useIacTableForCoasting) && cltValid) {
percent_t iacPosForCoasting = interpolate2d(clt, CONFIG(iacCoastingBins), CONFIG(iacCoasting));
newValue = interpolateClamped(idlePidLowerRpm, newValue, idlePidLowerRpm + CONFIG(idlePidRpmUpperLimit), iacPosForCoasting, rpm);
} else {
// Well, just leave it as is, without PID regulation...
newValue = engine->engineState.idle.baseIdlePosition;
}
}
// TODO: should we just remove this? It reduces the gain if your zero throttle stop isn't perfect,
// which could give unstable results.
newValue = interpolateClamped(0, newValue, CONFIG(idlePidDeactivationTpsThreshold), 0, tpsPos);
m_lastAutomaticPosition = newValue;
return newValue;
}
@ -497,56 +478,25 @@ float IdleController::getClosedLoop(IIdleController::Phase phase, float tpsPos,
finishIdleTestIfNeeded();
undoIdleBlipIfNeeded();
// cltCorrection is used only for cranking or running in manual mode
float cltCorrection;
// Use separate CLT correction table for cranking
if (engineConfiguration->overrideCrankingIacSetting && phase == IIdleController::Phase::Cranking) {
cltCorrection = interpolate2d(clt, config->cltCrankingCorrBins, config->cltCrankingCorr);
} else {
// this value would be ignored if running in AUTO mode
// but we need it while cranking in AUTO mode
cltCorrection = interpolate2d(clt, config->cltIdleCorrBins, config->cltIdleCorr);
}
percent_t iacPosition;
if (timeToStopBlip != 0) {
iacPosition = blipIdlePosition;
engine->engineState.idle.baseIdlePosition = iacPosition;
engine->engineState.idle.idleState = BLIP;
} else if (phase == IIdleController::Phase::Cranking) {
// during cranking it's always manual mode, PID would make no sense during cranking
iacPosition = clampPercentValue(cltCorrection * engineConfiguration->crankingIACposition);
// save cranking position & cycles counter for taper transition
lastCrankingIacPosition = iacPosition;
lastCrankingCyclesCounter = engine->rpmCalculator.getRevolutionCounterSinceStart();
engine->engineState.idle.baseIdlePosition = iacPosition;
} else {
if (!tps.Valid || engineConfiguration->idleMode == IM_MANUAL) {
// let's re-apply CLT correction
iacPosition = manualIdleController(cltCorrection PASS_ENGINE_PARAMETER_SUFFIX);
} else {
iacPosition = getClosedLoop(phase, tps.Value, rpm, targetRpm);
}
iacPosition = clampPercentValue(iacPosition);
// store 'base' iacPosition without adjustments
// Always apply closed loop correction
iacPosition = getOpenLoop(phase, clt, tps);
engine->engineState.idle.baseIdlePosition = iacPosition;
float additionalAir = (float)engineConfiguration->iacByTpsTaper;
if (tps.Valid) {
iacPosition += interpolateClamped(0.0f, 0.0f, CONFIG(idlePidDeactivationTpsThreshold), additionalAir, tps.Value);
// If TPS is working and automatic mode enabled, add any automatic correction
if (tps.Valid && engineConfiguration->idleMode == IM_AUTO) {
iacPosition += getClosedLoop(phase, tps.Value, rpm, targetRpm);
}
// taper transition from cranking to running (uint32_t to float conversion is safe here)
if (engineConfiguration->afterCrankingIACtaperDuration > 0)
iacPosition = interpolateClamped(lastCrankingCyclesCounter, lastCrankingIacPosition,
lastCrankingCyclesCounter + engineConfiguration->afterCrankingIACtaperDuration, iacPosition,
engine->rpmCalculator.getRevolutionCounterSinceStart());
iacPosition = clampPercentValue(iacPosition);
}
#if EFI_TUNER_STUDIO
tsOutputChannels.isIdleClosedLoop = phase == Phase::Idling;
tsOutputChannels.isIdleCoasting = phase == Phase::Coasting;

View File

@ -65,6 +65,9 @@ private:
Phase m_lastPhase = Phase::Cranking;
int m_lastTargetRpm = 0;
// This is stored by getClosedLoop and used in case we want to "do nothing"
float m_lastAutomaticPosition = 0;
Pid m_timingPid;
};

View File

@ -347,12 +347,10 @@ TEST(idle_v2, closedLoopBasic) {
timeNowUs += 5'000'000;
// Test above target, should return negative
// TODO: this fails, returns 55, why?
// EXPECT_FLOAT_EQ(-25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 950, /*tgt*/ 900));
EXPECT_FLOAT_EQ(-25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 950, /*tgt*/ 900));
// Below target, should return positive
// TODO: this fails, returns 55, why?
// EXPECT_FLOAT_EQ(25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 850, /*tgt*/ 900));
EXPECT_FLOAT_EQ(25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 850, /*tgt*/ 900));
}
TEST(idle_v2, closedLoopDeadzone) {
@ -377,10 +375,109 @@ TEST(idle_v2, closedLoopDeadzone) {
timeNowUs += 5'000'000;
// Test above target, should return negative
// TODO: this fails, returns 55, why?
// EXPECT_FLOAT_EQ(-25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 950, /*tgt*/ 900));
EXPECT_FLOAT_EQ(-25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 950, /*tgt*/ 900));
// Inside deadzone, should return same as last time
// TODO: this fails, returns 55, why?
// EXPECT_FLOAT_EQ(-25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 900, /*tgt*/ 900));
EXPECT_FLOAT_EQ(-25, dut.getClosedLoop(ICP::Idling, 0, /*rpm*/ 900, /*tgt*/ 900));
}
struct IntegrationIdleMock : public IdleController {
MOCK_METHOD(int, getTargetRpm, (float clt), (const, override));
MOCK_METHOD(ICP, determinePhase, (int rpm, int targetRpm, SensorResult tps), (const, override));
MOCK_METHOD(float, getOpenLoop, (ICP phase, float clt, SensorResult tps), (const, override));
MOCK_METHOD(float, getClosedLoop, (ICP phase, float tps, int rpm, int target), (override));
};
TEST(idle_v2, IntegrationManual) {
WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
StrictMock<IntegrationIdleMock> dut;
INJECT_ENGINE_REFERENCE(&dut);
SensorResult expectedTps = 1;
float expectedClt = 37;
Sensor::setMockValue(SensorType::DriverThrottleIntent, expectedTps.Value);
Sensor::setMockValue(SensorType::Clt, expectedClt);
ENGINE(rpmCalculator.mockRpm) = 950;
// Target of 1000 rpm
EXPECT_CALL(dut, getTargetRpm(expectedClt))
.WillOnce(Return(1000));
// Determine phase will claim we're idling
EXPECT_CALL(dut, determinePhase(950, 1000, expectedTps))
.WillOnce(Return(ICP::Idling));
// Open loop should be asked for an open loop position
EXPECT_CALL(dut, getOpenLoop(ICP::Idling, expectedClt, expectedTps))
.WillOnce(Return(13));
// getClosedLoop() should not be called!
EXPECT_EQ(13, dut.getIdlePosition());
}
TEST(idle_v2, IntegrationAutomatic) {
WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
StrictMock<IntegrationIdleMock> dut;
INJECT_ENGINE_REFERENCE(&dut);
CONFIG(idleMode) = IM_AUTO;
SensorResult expectedTps = 1;
float expectedClt = 37;
Sensor::setMockValue(SensorType::DriverThrottleIntent, expectedTps.Value);
Sensor::setMockValue(SensorType::Clt, expectedClt);
ENGINE(rpmCalculator.mockRpm) = 950;
// Target of 1000 rpm
EXPECT_CALL(dut, getTargetRpm(expectedClt))
.WillOnce(Return(1000));
// Determine phase will claim we're idling
EXPECT_CALL(dut, determinePhase(950, 1000, expectedTps))
.WillOnce(Return(ICP::Idling));
// Open loop should be asked for an open loop position
EXPECT_CALL(dut, getOpenLoop(ICP::Idling, expectedClt, expectedTps))
.WillOnce(Return(13));
// Closed loop should get called
EXPECT_CALL(dut, getClosedLoop(ICP::Idling, expectedTps.Value, 950, 1000))
.WillOnce(Return(7));
// Result should be open + closed
EXPECT_EQ(13 + 7, dut.getIdlePosition());
}
TEST(idle_v2, IntegrationClamping) {
WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
StrictMock<IntegrationIdleMock> dut;
INJECT_ENGINE_REFERENCE(&dut);
CONFIG(idleMode) = IM_AUTO;
SensorResult expectedTps = 1;
float expectedClt = 37;
Sensor::setMockValue(SensorType::DriverThrottleIntent, expectedTps.Value);
Sensor::setMockValue(SensorType::Clt, expectedClt);
ENGINE(rpmCalculator.mockRpm) = 950;
// Target of 1000 rpm
EXPECT_CALL(dut, getTargetRpm(expectedClt))
.WillOnce(Return(1000));
// Determine phase will claim we're idling
EXPECT_CALL(dut, determinePhase(950, 1000, expectedTps))
.WillOnce(Return(ICP::Idling));
// Open loop should be asked for an open loop position
EXPECT_CALL(dut, getOpenLoop(ICP::Idling, expectedClt, expectedTps))
.WillOnce(Return(75));
// Closed loop should get called
EXPECT_CALL(dut, getClosedLoop(ICP::Idling, expectedTps.Value, 950, 1000))
.WillOnce(Return(75));
// Result would be 75 + 75 = 150, but it should clamp to 100
EXPECT_EQ(100, dut.getIdlePosition());
}