Fuel pressure sensor checks (#2449)

* test for missing sensor

* test negative pressure delta checking logic

* test new hasSensor logic

firmware/controllers/algo/fuel/injector_model.cpp

@@ -23,6 +23,11 @@ expected<float> 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);
 

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;
 };

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 {

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.
 	 */

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
 	
 

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<TesterGetRailPressure> 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<TesterGetRailPressure> 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());
+}

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));
 

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;
 };