require redundant TPS for ETB (#2041)

* Require redundant tps for ETB (#2037)

* mod sensor api

* require redundancy

* fix tests

* check that it fails

Co-authored-by: Matthew Kennedy <makenne@microsoft.com>

* add a test that fails but should not

* fix the bug

Co-authored-by: Matthew Kennedy <makenne@microsoft.com>
This commit is contained in:
Matthew Kennedy 2020-12-08 05:24:20 -06:00 committed by GitHub
parent 7bdd047955
commit bad0e6b949
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 132 additions and 53 deletions

View File

@ -161,7 +161,7 @@ static percent_t currentEtbDuty;
// this macro clamps both positive and negative percentages from about -100% to 100%
#define ETB_PERCENT_TO_DUTY(x) (clampF(-ETB_DUTY_LIMIT, 0.01f * (x), ETB_DUTY_LIMIT))
bool EtbController::init(etb_function_e function, DcMotor *motor, pid_s *pidParameters, const ValueProvider3D* pedalMap) {
bool EtbController::init(etb_function_e function, DcMotor *motor, pid_s *pidParameters, const ValueProvider3D* pedalMap, bool initializeThrottles) {
if (function == ETB_None) {
// if not configured, don't init.
return false;
@ -169,6 +169,25 @@ bool EtbController::init(etb_function_e function, DcMotor *motor, pid_s *pidPara
m_function = function;
m_positionSensor = functionToPositionSensor(function);
// If we are a throttle, require redundant TPS sensor
if (function == ETB_Throttle1 || function == ETB_Throttle2) {
// We don't need to init throttles, so nothing to do here.
if (!initializeThrottles) {
return false;
}
if (!Sensor::isRedundant(m_positionSensor)) {
firmwareError(
OBD_Throttle_Position_Sensor_Circuit_Malfunction,
"Use of electronic throttle requires %s to be redundant.",
Sensor::getSensorName(m_positionSensor)
);
return false;
}
}
m_motor = motor;
m_pid.initPidClass(pidParameters);
m_pedalMap = pedalMap;
@ -846,7 +865,7 @@ void doInitElectronicThrottle(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
pedal2tpsMap.init(config->pedalToTpsTable, config->pedalToTpsPedalBins, config->pedalToTpsRpmBins);
bool mustHaveEtbConfigured = Sensor::hasSensor(SensorType::AcceleratorPedalPrimary);
bool shouldInitThrottles = Sensor::hasSensor(SensorType::AcceleratorPedalPrimary);
bool anyEtbConfigured = false;
for (int i = 0 ; i < ETB_COUNT; i++) {
@ -863,14 +882,14 @@ void doInitElectronicThrottle(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
auto func = CONFIG(etbFunctions[i]);
auto pid = getEtbPidForFunction(func PASS_ENGINE_PARAMETER_SUFFIX);
anyEtbConfigured |= controller->init(func, motor, pid, &pedal2tpsMap);
anyEtbConfigured |= controller->init(func, motor, pid, &pedal2tpsMap, shouldInitThrottles);
INJECT_ENGINE_REFERENCE(engine->etbControllers[i]);
}
}
if (!anyEtbConfigured) {
// It's not valid to have a PPS without any ETBs - check that at least one ETB was enabled along with the pedal
if (mustHaveEtbConfigured) {
if (shouldInitThrottles) {
firmwareError(OBD_PCM_Processor_Fault, "A pedal position sensor was configured, but no electronic throttles are configured.");
}

View File

@ -41,7 +41,7 @@ public:
// Initialize the throttle.
// returns true if the throttle was initialized, false otherwise.
virtual bool init(etb_function_e function, DcMotor *motor, pid_s *pidParameters, const ValueProvider3D* pedalMap) = 0;
virtual bool init(etb_function_e function, DcMotor *motor, pid_s *pidParameters, const ValueProvider3D* pedalMap, bool initializeThrottles = true) = 0;
virtual void reset() = 0;
virtual void setIdlePosition(percent_t pos) = 0;
virtual void setWastegatePosition(percent_t pos) = 0;

View File

@ -26,7 +26,7 @@ class Logging;
class EtbController : public IEtbController {
public:
bool init(etb_function_e function, DcMotor *motor, pid_s *pidParameters, const ValueProvider3D* pedalMap) override;
bool init(etb_function_e function, DcMotor *motor, pid_s *pidParameters, const ValueProvider3D* pedalMap, bool initializeThrottles) override;
void setIdlePosition(percent_t pos) override;
void setWastegatePosition(percent_t pos) override;
void reset() override;

View File

@ -50,9 +50,10 @@ public:
return m_sensor;
}
void setMockValue(float value) {
void setMockValue(float value, bool mockRedundant) {
m_mockValue = value;
m_useMock = true;
m_mockRedundant = mockRedundant;
}
void resetMock() {
@ -131,11 +132,16 @@ public:
return sensor->isRedundant();
}
if (m_useMock) {
return m_mockRedundant;
}
return false;
}
private:
bool m_useMock = false;
bool m_mockRedundant = false;
float m_mockValue;
Sensor* m_sensor = nullptr;
};
@ -201,11 +207,11 @@ bool Sensor::Register() {
return entry ? entry->hasSensor() : false;
}
/*static*/ void Sensor::setMockValue(SensorType type, float value) {
/*static*/ void Sensor::setMockValue(SensorType type, float value, bool mockRedundant) {
auto entry = getEntryForType(type);
if (entry) {
entry->setMockValue(value);
entry->setMockValue(value, mockRedundant);
}
}

View File

@ -105,7 +105,7 @@ public:
/*
* Mock a value for a particular sensor.
*/
static void setMockValue(SensorType type, float value);
static void setMockValue(SensorType type, float value, bool mockRedundant = false);
/*
* Mock a value for a particular sensor.
@ -127,6 +127,7 @@ public:
* For example, CLT, IAT, Throttle Position 2, etc.
*/
const char* getSensorName() { return getSensorName(m_type); }
static const char* getSensorName(SensorType type);
// Retrieve the current reading from the sensor.
//
@ -155,8 +156,6 @@ protected:
explicit Sensor(SensorType type)
: m_type(type) {}
static const char* getSensorName(SensorType type);
private:
const SensorType m_type;

View File

@ -13,7 +13,7 @@ public:
// IEtbController mocks
MOCK_METHOD(void, reset, (), ());
MOCK_METHOD(void, update, (), (override));
MOCK_METHOD(bool, init, (etb_function_e function, DcMotor* motor, pid_s* pidParameters, const ValueProvider3D* pedalMap), (override));
MOCK_METHOD(bool, init, (etb_function_e function, DcMotor* motor, pid_s* pidParameters, const ValueProvider3D* pedalMap, bool initializeThrottles), (override));
MOCK_METHOD(void, setIdlePosition, (percent_t pos), (override));
MOCK_METHOD(void, setWastegatePosition, (percent_t pos), (override));
MOCK_METHOD(void, autoCalibrateTps, (), (override));

View File

@ -27,8 +27,8 @@ TEST(etb, initializationNoPedal) {
engine->etbControllers[i] = &mocks[i];
}
EXPECT_CALL(mocks[0], init(ETB_Throttle1, _, _, _)).WillOnce(Return(false));
EXPECT_CALL(mocks[1], init(ETB_Throttle2, _, _, _)).WillOnce(Return(false));
EXPECT_CALL(mocks[0], init(ETB_Throttle1, _, _, _, false)).WillOnce(Return(false));
EXPECT_CALL(mocks[1], init(ETB_Throttle2, _, _, _, false)).WillOnce(Return(false));
// This shouldn't throw, since no throttles are configured, but no pedal is configured either
EXPECT_NO_FATAL_ERROR(doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE));
@ -46,8 +46,8 @@ TEST(etb, initializationMissingThrottle) {
engineConfiguration->etbFunctions[0] = ETB_None;
engineConfiguration->etbFunctions[1] = ETB_None;
EXPECT_CALL(mocks[0], init(ETB_None, _, _, _)).WillOnce(Return(false));
EXPECT_CALL(mocks[1], init(ETB_None, _, _, _)).WillOnce(Return(false));
EXPECT_CALL(mocks[0], init(ETB_None, _, _, _, true)).WillOnce(Return(false));
EXPECT_CALL(mocks[1], init(ETB_None, _, _, _, true)).WillOnce(Return(false));
// Must have a sensor configured before init
Sensor::setMockValue(SensorType::AcceleratorPedal, 0);
@ -74,10 +74,10 @@ TEST(etb, initializationSingleThrottle) {
engineConfiguration->etbFunctions[1] = ETB_None;
// Expect mock0 to be init as throttle 1, and PID params
EXPECT_CALL(mocks[0], init(ETB_Throttle1, _, &engineConfiguration->etb, Ne(nullptr))).WillOnce(Return(true));
EXPECT_CALL(mocks[0], init(ETB_Throttle1, _, &engineConfiguration->etb, Ne(nullptr), true)).WillOnce(Return(true));
// Expect mock1 to be init as none
EXPECT_CALL(mocks[1], init(ETB_None, _, _, _)).WillOnce(Return(true));
EXPECT_CALL(mocks[1], init(ETB_None, _, _, _, true)).WillOnce(Return(true));
doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE);
}
@ -99,10 +99,10 @@ TEST(etb, initializationSingleThrottleInSecondSlot) {
engineConfiguration->etbFunctions[1] = ETB_Throttle1;
// Expect mock0 to be init as none
EXPECT_CALL(mocks[0], init(ETB_None, _, _, _)).WillOnce(Return(true));
EXPECT_CALL(mocks[0], init(ETB_None, _, _, _, true)).WillOnce(Return(true));
// Expect mock1 to be init as throttle 1, and PID params
EXPECT_CALL(mocks[1], init(ETB_Throttle1, _, &engineConfiguration->etb, Ne(nullptr))).WillOnce(Return(true));
EXPECT_CALL(mocks[1], init(ETB_Throttle1, _, &engineConfiguration->etb, Ne(nullptr), true)).WillOnce(Return(true));
doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE);
}
@ -120,17 +120,17 @@ TEST(etb, initializationDualThrottle) {
Sensor::setMockValue(SensorType::AcceleratorPedal, 0);
Sensor::setMockValue(SensorType::AcceleratorPedalPrimary, 0);
// The presence of a second TPS indicates dual throttle
Sensor::setMockValue(SensorType::Tps2, 25.0f);
Sensor::setMockValue(SensorType::Tps1, 25.0f, true);
Sensor::setMockValue(SensorType::Tps2, 25.0f, true);
engineConfiguration->etbFunctions[0] = ETB_Throttle1;
engineConfiguration->etbFunctions[1] = ETB_Throttle2;
// Expect mock0 to be init as throttle 1, and PID params
EXPECT_CALL(mocks[0], init(ETB_Throttle1, _, &engineConfiguration->etb, Ne(nullptr))).WillOnce(Return(true));
EXPECT_CALL(mocks[0], init(ETB_Throttle1, _, &engineConfiguration->etb, Ne(nullptr), true)).WillOnce(Return(true));
// Expect mock1 to be init as throttle 2, and PID params
EXPECT_CALL(mocks[1], init(ETB_Throttle2, _, &engineConfiguration->etb, Ne(nullptr))).WillOnce(Return(true));
EXPECT_CALL(mocks[1], init(ETB_Throttle2, _, &engineConfiguration->etb, Ne(nullptr), true)).WillOnce(Return(true));
doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE);
}
@ -144,18 +144,14 @@ TEST(etb, initializationWastegate) {
engine->etbControllers[i] = &mocks[i];
}
// Must have a sensor configured before init
Sensor::setMockValue(SensorType::AcceleratorPedal, 0);
Sensor::setMockValue(SensorType::AcceleratorPedalPrimary, 0);
engineConfiguration->etbFunctions[0] = ETB_Wastegate;
engineConfiguration->etbFunctions[1] = ETB_None;
// Expect mock0 to be init as throttle 1, and PID wastegate params
EXPECT_CALL(mocks[0], init(ETB_Wastegate, _, &engineConfiguration->etbWastegatePid, Ne(nullptr))).WillOnce(Return(true));
EXPECT_CALL(mocks[0], init(ETB_Wastegate, _, &engineConfiguration->etbWastegatePid, Ne(nullptr), false)).WillOnce(Return(true));
// Expect mock1 to be init as none
EXPECT_CALL(mocks[1], init(ETB_None, _, _, _)).WillOnce(Return(true));
EXPECT_CALL(mocks[1], init(ETB_None, _, _, _, false)).WillOnce(Return(true));
doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE);
}
@ -166,12 +162,47 @@ TEST(etb, initializationNoFunction) {
EtbController dut;
// When init called with ETB_None, should ignore the provided params and return false
EXPECT_FALSE(dut.init(ETB_None, &motor, nullptr, nullptr));
EXPECT_FALSE(dut.init(ETB_None, &motor, nullptr, nullptr, false));
// This should no-op, it shouldn't call motor.set(float duty)
dut.setOutput(0.5f);
}
TEST(etb, initializationNotRedundantTps) {
EtbController dut;
// Needs pedal for init
Sensor::setMockValue(SensorType::AcceleratorPedal, 0.0f);
// Not redundant, should fail upon init
Sensor::setMockValue(SensorType::Tps1, 0.0f, false);
EXPECT_FATAL_ERROR(dut.init(ETB_Throttle1, nullptr, nullptr, nullptr, true));
}
TEST(etb, initializationNoThrottles) {
// This tests the case where you don't want an ETB, and expect everything to go fine
EtbController duts[2];
WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
for (int i = 0; i < ETB_COUNT; i++) {
engine->etbControllers[i] = &duts[i];
}
// Configure ETB functions, but no pedal
engineConfiguration->etbFunctions[0] = ETB_Throttle1;
engineConfiguration->etbFunctions[1] = ETB_Throttle2;
// No pedal - don't init ETBs
Sensor::resetMockValue(SensorType::AcceleratorPedal);
// Not redundant TPS (aka cable throttle)
Sensor::setMockValue(SensorType::Tps1, 0.0f, false);
EXPECT_NO_FATAL_ERROR(doInitElectronicThrottle(PASS_ENGINE_PARAMETER_SIGNATURE));
}
TEST(etb, idlePlumbing) {
StrictMock<MockEtb> mocks[ETB_COUNT];
@ -208,7 +239,10 @@ TEST(etb, testSetpointOnlyPedal) {
// Uninitialized ETB must return unexpected (and not deference a null pointer)
EXPECT_EQ(etb.getSetpoint(), unexpected);
etb.init(ETB_Throttle1, nullptr, nullptr, &pedalMap);
// Must have TPS initialized for ETB setup
Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
etb.init(ETB_Throttle1, nullptr, nullptr, &pedalMap, true);
// Check endpoints and midpoint
Sensor::setMockValue(SensorType::AcceleratorPedal, 0.0f);
@ -249,6 +283,9 @@ TEST(etb, setpointIdle) {
engineConfiguration->useETBforIdleControl = true;
engineConfiguration->etbIdleThrottleRange = 0;
// Must have TPS initialized for ETB setup
Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
EtbController etb;
INJECT_ENGINE_REFERENCE(&etb);
@ -258,7 +295,7 @@ TEST(etb, setpointIdle) {
.WillRepeatedly([](float xRpm, float y) {
return y;
});
etb.init(ETB_Throttle1, nullptr, nullptr, &pedalMap);
etb.init(ETB_Throttle1, nullptr, nullptr, &pedalMap, true);
// No idle range, should just pass pedal
Sensor::setMockValue(SensorType::AcceleratorPedal, 0.0f);
@ -299,10 +336,10 @@ TEST(etb, setpointNoPedalMap) {
EtbController etb;
// Must have TPS initialized for ETB setup
// Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
// Don't pass a pedal map
etb.init(ETB_Throttle1, nullptr, nullptr, nullptr);
etb.init(ETB_Throttle1, nullptr, nullptr, nullptr, true);
EXPECT_EQ(etb.getSetpoint(), unexpected);
}
@ -310,7 +347,7 @@ TEST(etb, setpointNoPedalMap) {
TEST(etb, setpointIdleValveController) {
EtbController etb;
etb.init(ETB_IdleValve, nullptr, nullptr, nullptr);
etb.init(ETB_IdleValve, nullptr, nullptr, nullptr, false);
etb.setIdlePosition(0);
EXPECT_FLOAT_EQ(0, etb.getSetpoint().value_or(-1));
@ -329,7 +366,7 @@ TEST(etb, setpointIdleValveController) {
TEST(etb, setpointWastegateController) {
EtbController etb;
etb.init(ETB_Wastegate, nullptr, nullptr, nullptr);
etb.init(ETB_Wastegate, nullptr, nullptr, nullptr, false);
etb.setWastegatePosition(0);
EXPECT_FLOAT_EQ(0, etb.getSetpoint().value_or(-1));
@ -347,36 +384,36 @@ TEST(etb, setpointWastegateController) {
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);
Sensor::setMockValue(SensorType::Tps2, 75.0f);
Sensor::setMockValue(SensorType::Tps1, 25.0f, true);
Sensor::setMockValue(SensorType::Tps2, 75.0f, true);
Sensor::setMockValue(SensorType::WastegatePosition, 33.0f);
Sensor::setMockValue(SensorType::IdlePosition, 66.0f);
// Test first throttle
{
EtbController etb;
etb.init(ETB_Throttle1, nullptr, nullptr, nullptr);
etb.init(ETB_Throttle1, nullptr, nullptr, nullptr, true);
EXPECT_EQ(etb.observePlant().Value, 25.0f);
}
// Test second throttle
{
EtbController etb;
etb.init(ETB_Throttle2, nullptr, nullptr, nullptr);
etb.init(ETB_Throttle2, nullptr, nullptr, nullptr, true);
EXPECT_EQ(etb.observePlant().Value, 75.0f);
}
// Test wastegate control
{
EtbController etb;
etb.init(ETB_Wastegate, nullptr, nullptr, nullptr);
etb.init(ETB_Wastegate, nullptr, nullptr, nullptr, true);
EXPECT_EQ(etb.observePlant().Value, 33.0f);
}
// Test idle valve control
{
EtbController etb;
etb.init(ETB_IdleValve, nullptr, nullptr, nullptr);
etb.init(ETB_IdleValve, nullptr, nullptr, nullptr, true);
EXPECT_EQ(etb.observePlant().Value, 66.0f);
}
}
@ -388,7 +425,7 @@ TEST(etb, setOutputInvalid) {
EtbController etb;
INJECT_ENGINE_REFERENCE(&etb);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr, true);
// Should be disabled in case of unexpected
EXPECT_CALL(motor, disable());
@ -400,9 +437,12 @@ TEST(etb, setOutputValid) {
WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
StrictMock<MockMotor> motor;
// Must have TPS initialized for ETB setup
Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
EtbController etb;
INJECT_ENGINE_REFERENCE(&etb);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr, true);
// Should be enabled and value set
EXPECT_CALL(motor, enable());
@ -416,9 +456,12 @@ TEST(etb, setOutputValid2) {
WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
StrictMock<MockMotor> motor;
// Must have TPS initialized for ETB setup
Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
EtbController etb;
INJECT_ENGINE_REFERENCE(&etb);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr, true);
// Should be enabled and value set
EXPECT_CALL(motor, enable());
@ -432,9 +475,12 @@ TEST(etb, setOutputOutOfRangeHigh) {
WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
StrictMock<MockMotor> motor;
// Must have TPS initialized for ETB setup
Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
EtbController etb;
INJECT_ENGINE_REFERENCE(&etb);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr, true);
// Should be enabled and value set
EXPECT_CALL(motor, enable());
@ -448,9 +494,12 @@ TEST(etb, setOutputOutOfRangeLow) {
WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
StrictMock<MockMotor> motor;
// Must have TPS initialized for ETB setup
Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
EtbController etb;
INJECT_ENGINE_REFERENCE(&etb);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr, true);
// Should be enabled and value set
EXPECT_CALL(motor, enable());
@ -464,9 +513,12 @@ TEST(etb, setOutputPauseControl) {
WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
StrictMock<MockMotor> motor;
// Must have TPS initialized for ETB setup
Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
EtbController etb;
INJECT_ENGINE_REFERENCE(&etb);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr);
etb.init(ETB_Throttle1, &motor, nullptr, nullptr, true);
// Pause control - should get no output
engineConfiguration->pauseEtbControl = true;
@ -483,8 +535,11 @@ TEST(etb, closedLoopPid) {
pid.maxValue = 75;
pid.minValue = -60;
// Must have TPS initialized for ETB setup
Sensor::setMockValue(SensorType::Tps1, 0.0f, true);
EtbController etb;
etb.init(ETB_Throttle1, nullptr, &pid, nullptr);
etb.init(ETB_Throttle1, nullptr, &pid, nullptr, true);
// Disable autotune for now
Engine e;
@ -506,7 +561,7 @@ TEST(etb, openLoopThrottle) {
EtbController etb;
INJECT_ENGINE_REFERENCE(&etb);
etb.init(ETB_Throttle1, nullptr, nullptr, nullptr);
etb.init(ETB_Throttle1, nullptr, nullptr, nullptr, true);
// Map [0, 100] -> [-50, 50]
setLinearCurve(engineConfiguration->etbBiasBins, 0, 100);
@ -524,7 +579,7 @@ TEST(etb, openLoopNonThrottle) {
EtbController etb;
INJECT_ENGINE_REFERENCE(&etb);
etb.init(ETB_Wastegate, nullptr, nullptr, nullptr);
etb.init(ETB_Wastegate, nullptr, nullptr, nullptr, false);
// Map [0, 100] -> [-50, 50]
setLinearCurve(engineConfiguration->etbBiasBins, 0, 100);