diff --git a/firmware/controllers/algo/fuel/injector_model.cpp b/firmware/controllers/algo/fuel/injector_model.cpp index d45865e65d..cdb7145781 100644 --- a/firmware/controllers/algo/fuel/injector_model.cpp +++ b/firmware/controllers/algo/fuel/injector_model.cpp @@ -23,6 +23,11 @@ expected InjectorModel::getAbsoluteRailPressure() const { // TODO: should this add baro pressure instead of 1atm? return (CONFIG(fuelReferencePressure) + 101.325f); case ICM_SensedRailPressure: + if (!Sensor::hasSensor(SensorType::FuelPressureInjector)) { + firmwareError(OBD_PCM_Processor_Fault, "Fuel pressure compensation is set to use a pressure sensor, but none is configured."); + return unexpected; + } + // TODO: what happens when the sensor fails? return Sensor::get(SensorType::FuelPressureInjector); default: return unexpected; @@ -50,8 +55,13 @@ float InjectorModel::getInjectorFlowRatio() const { return 1.0f; } - // TODO: what to do when pressureDelta is less than 0? float pressureDelta = absRailPressure.Value - map.Value; + + // Somehow pressure delta is less than 0, assume failed sensor and return default flow + if (pressureDelta <= 0) { + return 1.0f; + } + float pressureRatio = pressureDelta / referencePressure; float flowRatio = sqrtf(pressureRatio); diff --git a/firmware/controllers/sensors/proxy_sensor.h b/firmware/controllers/sensors/proxy_sensor.h index 257b4068b1..67251c4d91 100644 --- a/firmware/controllers/sensors/proxy_sensor.h +++ b/firmware/controllers/sensors/proxy_sensor.h @@ -28,5 +28,10 @@ private: return Sensor::get(m_proxiedSensor); } + bool hasSensor() const override { + // query if the underlying sensor exists + return Sensor::hasSensor(m_proxiedSensor); + } + SensorType m_proxiedSensor = SensorType::Invalid; }; diff --git a/firmware/controllers/sensors/sensor.cpp b/firmware/controllers/sensors/sensor.cpp index 9944bff9c1..415901068b 100644 --- a/firmware/controllers/sensors/sensor.cpp +++ b/firmware/controllers/sensors/sensor.cpp @@ -95,6 +95,11 @@ public: // Get the sensor out of the entry const Sensor *s = m_sensor; if (s) { + // If this sensor says it doesn't exist, return unexpected + if (!s->hasSensor()) { + return unexpected; + } + // If we found the sensor, ask it for a result. return s->get(); } @@ -118,7 +123,7 @@ public: } bool hasSensor() const { - return m_useMock || m_sensor; + return m_useMock || (m_sensor && m_sensor->hasSensor()); } float getRaw() const { diff --git a/firmware/controllers/sensors/sensor.h b/firmware/controllers/sensors/sensor.h index 5ee1d88bb6..396b58b0fd 100644 --- a/firmware/controllers/sensors/sensor.h +++ b/firmware/controllers/sensors/sensor.h @@ -136,6 +136,11 @@ public: // this should be field lookup and simple math. virtual SensorResult get() const = 0; + // Retrieve whether the sensor is present. Some sensors may be registered but not present, ie if inintialization failed. + virtual bool hasSensor() const { + return true; + } + /* * Get an unconverted value from the sensor, if available. */ diff --git a/unit_tests/Makefile b/unit_tests/Makefile index d141371fc7..60756519e6 100644 --- a/unit_tests/Makefile +++ b/unit_tests/Makefile @@ -189,6 +189,7 @@ INCDIR = . \ googletest/googletest \ googletest/googletest/include \ $(PROJECT_DIR)/../unit_tests/tests \ + $(PROJECT_DIR)/../unit_tests/tests/sensor \ $(PROJECT_DIR)/../unit_tests/test_basic_math diff --git a/unit_tests/tests/ignition_injection/test_injector_model.cpp b/unit_tests/tests/ignition_injection/test_injector_model.cpp index ed96a973d3..32a4496833 100644 --- a/unit_tests/tests/ignition_injection/test_injector_model.cpp +++ b/unit_tests/tests/ignition_injection/test_injector_model.cpp @@ -1,6 +1,7 @@ #include "engine_test_helper.h" #include "injector_model.h" #include "mocks.h" +#include "mock/mock_sensor.h" #include "gtest/gtest.h" @@ -115,6 +116,26 @@ TEST_P(FlowRateFixture, PressureRatio) { EXPECT_FLOAT_EQ(expectedFlowRatio, dut.getInjectorFlowRatio()); } +TEST(InjectorModel, NegativePressureDelta) { + StrictMock dut; + + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + INJECT_ENGINE_REFERENCE(&dut); + + // Use injector compensation + engineConfiguration->injectorCompensationMode = ICM_SensedRailPressure; + + // Reference pressure is 400kPa + engineConfiguration->fuelReferencePressure = 400.0f; + + EXPECT_CALL(dut, getAbsoluteRailPressure()).WillOnce(Return(50)); + // MAP sensor reads more pressure than fuel rail + Sensor::setMockValue(SensorType::Map, 100); + + // Flow ratio defaults to 1.0 in this case + EXPECT_FLOAT_EQ(1.0f, dut.getInjectorFlowRatio()); +} + TEST(InjectorModel, VariableInjectorFlowModeNone) { StrictMock dut; @@ -169,7 +190,27 @@ TEST(InjectorModel, FailedPressureSensor) { engineConfiguration->injectorCompensationMode = ICM_SensedRailPressure; // Sensor is broken! - Sensor::resetMockValue(SensorType::FuelPressureInjector); + // We have to register a broken sensor because the fuel pressure comp system + // has different logic for missing sensor + MockSensor ms(SensorType::FuelPressureInjector); + ms.invalidate(); + ms.Register(); EXPECT_EQ(1.0f, dut.getInjectorFlowRatio()); } + +TEST(InjectorModel, MissingPressureSensor) { + InjectorModel dut; + + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + INJECT_ENGINE_REFERENCE(&dut); + + // Reference pressure is 350kpa + engineConfiguration->injectorCompensationMode = ICM_SensedRailPressure; + + // Sensor is missing! + Sensor::resetMockValue(SensorType::FuelPressureInjector); + + // Missing sensor should trigger a fatal as it's a misconfiguration + EXPECT_FATAL_ERROR(dut.getInjectorFlowRatio()); +} diff --git a/unit_tests/tests/sensor/basic_sensor.cpp b/unit_tests/tests/sensor/basic_sensor.cpp index f55ccbe66f..08575b2bed 100644 --- a/unit_tests/tests/sensor/basic_sensor.cpp +++ b/unit_tests/tests/sensor/basic_sensor.cpp @@ -92,11 +92,13 @@ TEST_F(SensorBasic, HasSensor) { // Now we should! ASSERT_TRUE(Sensor::hasSensor(SensorType::Clt)); + + // Check that we can have the sensor report that it's missing + dut.setHasSensor(false); + ASSERT_FALSE(Sensor::hasSensor(SensorType::Clt)); } TEST_F(SensorBasic, HasSensorMock) { - MockSensor dut(SensorType::Clt); - // Check that we don't have the sensor ASSERT_FALSE(Sensor::hasSensor(SensorType::Clt)); diff --git a/unit_tests/tests/sensor/mock/mock_sensor.h b/unit_tests/tests/sensor/mock/mock_sensor.h index 287758046b..feff272acd 100644 --- a/unit_tests/tests/sensor/mock/mock_sensor.h +++ b/unit_tests/tests/sensor/mock/mock_sensor.h @@ -17,5 +17,16 @@ struct MockSensor final : public StoredValueSensor StoredValueSensor::invalidate(); } + bool hasSensor() const override { + return m_hasSensor; + } + + void setHasSensor(bool h) { + m_hasSensor = h; + } + void showInfo(Logging* logger, const char* name) const override {} + +private: + bool m_hasSensor = true; };