unexpected contains information about why it failed (#4393)

* unexpected can contain information

* info printing

* sensors return appropriate error codes

* remove reliance on undefined behavior

* s

firmware/controllers/actuators/fan_control.cpp

@@ -5,16 +5,16 @@
 #include "bench_test.h"
 
 bool FanController::getState(bool acActive, bool lastState) {
-	auto [cltValid, clt] = Sensor::get(SensorType::Clt);
+	auto clt = Sensor::get(SensorType::Clt);
 
 	cranking = engine->rpmCalculator.isCranking();
 	notRunning = !engine->rpmCalculator.isRunning();
 
 	disabledWhileEngineStopped = notRunning && disableWhenStopped();
-	brokenClt = !cltValid;
+	brokenClt = !clt;
 	enabledForAc = enableWithAc() && acActive;
-	hot = clt > getFanOnTemp();
-	cold = clt < getFanOffTemp();
+	hot = clt.Value > getFanOnTemp();
+	cold = clt.Value < getFanOffTemp();
 
 	if (cranking) {
 		// Inhibit while cranking

firmware/controllers/algo/advance_map.cpp

@@ -55,10 +55,10 @@ static angle_t getRunningAdvance(int rpm, float engineLoad) {
 	    engine->module<IdleController>()->isIdlingOrTaper()) {
 		float idleAdvance = interpolate2d(rpm, config->idleAdvanceBins, config->idleAdvance);
 
-		auto [valid, tps] = Sensor::get(SensorType::DriverThrottleIntent);
-		if (valid) {
+		auto tps = Sensor::get(SensorType::DriverThrottleIntent);
+		if (tps) {
 			// interpolate between idle table and normal (running) table using TPS threshold
-			advanceAngle = interpolateClamped(0.0f, idleAdvance, engineConfiguration->idlePidDeactivationTpsThreshold, advanceAngle, tps);
+			advanceAngle = interpolateClamped(0.0f, idleAdvance, engineConfiguration->idlePidDeactivationTpsThreshold, advanceAngle, tps.Value);
 		}
 	}
 
@@ -80,14 +80,14 @@ static angle_t getRunningAdvance(int rpm, float engineLoad) {
 }
 
 angle_t getAdvanceCorrections(int rpm) {
-	const auto [iatValid, iat] = Sensor::get(SensorType::Iat);
+	auto iat = Sensor::get(SensorType::Iat);
 
-	if (!iatValid) {
+	if (!iat) {
 		engine->engineState.timingIatCorrection = 0;
 	} else {
 		engine->engineState.timingIatCorrection = interpolate3d(
 			config->ignitionIatCorrTable,
-			config->ignitionIatCorrLoadBins, iat,
+			config->ignitionIatCorrLoadBins, iat.Value,
 			config->ignitionIatCorrRpmBins, rpm
 		);
 	}

firmware/controllers/algo/fuel/dfco.cpp

@@ -5,21 +5,21 @@ bool DfcoController::getState() const {
 		return false;
 	}
 
-	const auto [tpsValid, tpsPos] = Sensor::get(SensorType::DriverThrottleIntent);
-	const auto [cltValid, clt] = Sensor::get(SensorType::Clt);
-	const auto [mapValid, map] = Sensor::get(SensorType::Map);
+	const auto tps = Sensor::get(SensorType::DriverThrottleIntent);
+	const auto clt = Sensor::get(SensorType::Clt);
+	const auto map = Sensor::get(SensorType::Map);
 
 	// If some sensor is broken, inhibit DFCO
-	if (!tpsValid || !cltValid || !mapValid) {
+	if (!tps || !clt || !map) {
 		return false;
 	}
 
 	float rpm = Sensor::getOrZero(SensorType::Rpm);
 	float vss = Sensor::getOrZero(SensorType::VehicleSpeed);
 
-	bool mapActivate = map < engineConfiguration->coastingFuelCutMap;
-	bool tpsActivate = tpsPos < engineConfiguration->coastingFuelCutTps;
-	bool cltActivate = clt > engineConfiguration->coastingFuelCutClt;
+	bool mapActivate = map.Value < engineConfiguration->coastingFuelCutMap;
+	bool tpsActivate = tps.Value < engineConfiguration->coastingFuelCutTps;
+	bool cltActivate = clt.Value > engineConfiguration->coastingFuelCutClt;
 	// True if throttle, MAP, and CLT are all acceptable for DFCO to occur
 	bool dfcoAllowed = mapActivate && tpsActivate && cltActivate;
 

firmware/controllers/algo/fuel_math.cpp

@@ -344,30 +344,30 @@ void initFuelMap() {
  * @brief Engine warm-up fuel correction.
  */
 float getCltFuelCorrection() {
-	const auto [valid, clt] = Sensor::get(SensorType::Clt);
+	const auto clt = Sensor::get(SensorType::Clt);
 	
-	if (!valid)
+	if (!clt)
 		return 1; // this error should be already reported somewhere else, let's just handle it
 
-	return interpolate2d(clt, config->cltFuelCorrBins, config->cltFuelCorr);
+	return interpolate2d(clt.Value, config->cltFuelCorrBins, config->cltFuelCorr);
 }
 
 angle_t getCltTimingCorrection() {
-	const auto [valid, clt] = Sensor::get(SensorType::Clt);
+	const auto clt = Sensor::get(SensorType::Clt);
 
-	if (!valid)
+	if (!clt)
 		return 0; // this error should be already reported somewhere else, let's just handle it
 
-	return interpolate2d(clt, config->cltTimingBins, config->cltTimingExtra);
+	return interpolate2d(clt.Value, config->cltTimingBins, config->cltTimingExtra);
 }
 
 float getIatFuelCorrection() {
-	const auto [valid, iat] = Sensor::get(SensorType::Iat);
+	const auto iat = Sensor::get(SensorType::Iat);
 
-	if (!valid)
+	if (!iat)
 		return 1; // this error should be already reported somewhere else, let's just handle it
 
-	return interpolate2d(iat, config->iatFuelCorrBins, config->iatFuelCorr);
+	return interpolate2d(iat.Value, config->iatFuelCorrBins, config->iatFuelCorr);
 }
 
 float getBaroCorrection() {

firmware/controllers/engine_cycle/aux_valves.cpp

@@ -81,17 +81,17 @@ void recalculateAuxValveTiming() {
 		return;
 	}
 
-	auto [valid, tps] = Sensor::get(SensorType::DriverThrottleIntent);
-	if (!valid) {
+	auto tps = Sensor::get(SensorType::DriverThrottleIntent);
+	if (!tps) {
 		// error should be already reported by now
 		return;
 	}
 
-	engine->engineState.auxValveStart = interpolate2d(tps,
+	engine->engineState.auxValveStart = interpolate2d(tps.Value,
 			config->scriptCurve1Bins,
 			config->scriptCurve1);
 
-	engine->engineState.auxValveEnd = interpolate2d(tps,
+	engine->engineState.auxValveEnd = interpolate2d(tps.Value,
 			config->scriptCurve2Bins,
 			config->scriptCurve2);
 

firmware/controllers/engine_cycle/high_pressure_fuel_pump.cpp

@@ -81,11 +81,16 @@ float HpfpQuantity::calcPI(int rpm, float calc_fuel_percent) {
 		m_pressureTarget_kPa - (engineConfiguration->hpfpTargetDecay *
 					(FAST_CALLBACK_PERIOD_MS / 1000.)),
 		interpolate3d(engineConfiguration->hpfpTarget,
-			      engineConfiguration->hpfpTargetLoadBins, Sensor::get(SensorType::Map).Value, // TODO: allow other load axis, like we claim to
+			      engineConfiguration->hpfpTargetLoadBins, Sensor::get(SensorType::Map).value_or(0), // TODO: allow other load axis, like we claim to
 			      engineConfiguration->hpfpTargetRpmBins, rpm));
 
+	auto fuelPressure = Sensor::get(SensorType::FuelPressureHigh);
+	if (!fuelPressure) {
+		return 0;
+	}
+
 	float pressureError_kPa =
-		m_pressureTarget_kPa - Sensor::get(SensorType::FuelPressureHigh).Value;
+		m_pressureTarget_kPa - fuelPressure.Value;
 
 	float p_control_percent = pressureError_kPa * engineConfiguration->hpfpPidP;
 	float i_factor_divisor =

firmware/controllers/sensors/converters/flex_sensor.h

@@ -12,13 +12,17 @@ public:
 	SensorResult convert(float frequency) const override {
 		// Sensor should only report 50-150hz, significantly outside that range indicates a problem
 		// it changes to 200hz+ to indicate methanol "contamination"
-		if (frequency > 45 && frequency < 155) {
-			float flexPct = clampF(0, frequency - 50, 100);
-
-			return m_filter.filter(flexPct);
-		} else {
-			return unexpected;
+		if (frequency < 45) {
+			return UnexpectedCode::Low;
 		}
+
+		if (frequency > 155) {
+			return UnexpectedCode::High;
+		}
+
+		float flexPct = clampF(0, frequency - 50, 100);
+
+		return m_filter.filter(flexPct);
 	}
 
 private:

firmware/controllers/sensors/converters/linear_func.cpp

@@ -16,11 +16,15 @@ void LinearFunc::configure(float in1, float out1, float in2, float out2, float m
 SensorResult LinearFunc::convert(float inputValue) const {
 	float result = m_a * inputValue + m_b;
 
-	// Bounds check
-	bool isValid = result <= m_maxOutput && result >= m_minOutput;
+	// Bounds checks
+	// Flipped error codes in case of m_a < 0 so that they indicate whether the input
+	// voltage is high/low, instead of the output high/low
+	if (result > m_maxOutput) {
+		return m_a > 0 ? UnexpectedCode::High : UnexpectedCode::Low;
+	}
 
-	if (!isValid) {
-		return unexpected;
+	if (result < m_minOutput) {
+		return m_a > 0 ? UnexpectedCode::Low : UnexpectedCode::High;
 	}
 
 	return result;

firmware/controllers/sensors/converters/resistance_func.cpp

@@ -12,12 +12,12 @@ void ResistanceFunc::configure(float supplyVoltage, float pullupResistor) {
 SensorResult ResistanceFunc::convert(float raw) const {
 	// If the voltage is very low, the sensor is a dead short.
 	if (raw < 0.05f) {
-		return unexpected;
+		return UnexpectedCode::Low;
 	}
 
 	// If the voltage is very high (98% VCC), the sensor is open circuit.
 	if (raw > (m_supplyVoltage * 0.98f)) {
-		return unexpected;
+		return UnexpectedCode::High;
 	}
 
 	// Voltage is in a sensible range - convert

firmware/controllers/sensors/converters/thermistor_func.cpp

@@ -12,7 +12,7 @@ SensorResult ThermistorFunc::convert(float ohms) const {
 	// This resistance should have already been validated - only
 	// thing we can check is that it's non-negative
 	if (ohms <= 0) {
-		return unexpected;
+		return UnexpectedCode::Low;
 	}
 
 	float lnR = logf(ohms);
@@ -26,9 +26,14 @@ SensorResult ThermistorFunc::convert(float ohms) const {
 	float celsius = convertKelvinToCelcius(kelvin);
 
 	// bounds check result - please don't try to run rusEfi when colder than -50C
-	// high end limit is required as this could be an oil temp sensor
-	if (celsius < -50 || celsius > 250) {
-		return unexpected;
+	// high end limit is required as this could be an oil temp sensor on an
+	// air cooled engine
+	if (celsius < -50) {
+		return UnexpectedCode::Low;
+	}
+
+	if (celsius > 250) {
+		return UnexpectedCode::High;
 	}
 
 	return celsius;

firmware/controllers/sensors/functional_sensor.cpp

@@ -7,7 +7,7 @@
 void FunctionalSensor::postRawValue(float inputValue, efitick_t timestamp) {
 	// If no function is set, this sensor isn't valid.
 	if (!m_function) {
-		invalidate();
+		invalidate(UnexpectedCode::Configuration);
 		return;
 	}
 
@@ -21,6 +21,6 @@ void FunctionalSensor::postRawValue(float inputValue, efitick_t timestamp) {
 	if (r.Valid) {
 		setValidValue(r.Value, timestamp);
 	} else {
-		invalidate();
+		invalidate(r.Code);
 	}
 }

firmware/controllers/sensors/redundant_ford_tps.cpp

@@ -23,7 +23,7 @@ SensorResult RedundantFordTps::get() const {
 
 	// If either result is invalid, return invalid.
 	if (!tps1 || !tps2) {
-		return unexpected;
+		return UnexpectedCode::Inconsistent;
 	}
 
 	// The "actual" position resolved by the second throttle - this tops out at m_secondaryMaximum instead of 100%
@@ -37,7 +37,7 @@ SensorResult RedundantFordTps::get() const {
 		// Check that the resolved positions are close
 		float delta = absF(tps1.Value - tps2Actual);
 		if (delta > m_maxDifference) {
-			return unexpected;
+			return UnexpectedCode::Inconsistent;
 		}
 
 		return (tps1.Value + tps2Actual) / 2;
@@ -49,5 +49,5 @@ SensorResult RedundantFordTps::get() const {
 	}
 
 	// Any other condition indicates an mismatch, and therefore an error
-	return unexpected;
+	return UnexpectedCode::Inconsistent;
 }

firmware/controllers/sensors/redundant_sensor.cpp

@@ -26,13 +26,13 @@ SensorResult RedundantSensor::get() const {
 
 	// If either result is invalid, return invalid.
 	if (!result1.Valid || !result2.Valid) {
-		return unexpected;
+		return UnexpectedCode::Inconsistent;
 	}
 
 	// If both are valid, check that they're near one another
 	float delta = absF(result1.Value - result2.Value);
 	if (delta > m_maxDifference) {
-		return unexpected;
+		return UnexpectedCode::Inconsistent;
 	}
 
 	// Both sensors are valid, and their readings are close. All is well.

firmware/controllers/sensors/sensor.cpp

@@ -55,7 +55,7 @@ public:
 		if (s) {
 			// If this sensor says it doesn't exist, return unexpected
 			if (!s->hasSensor()) {
-				return unexpected;
+				return UnexpectedCode::Configuration;
 			}
 
 			// If we found the sensor, ask it for a result.
@@ -63,7 +63,7 @@ public:
 		}
 
 		// We've exhausted all valid ways to return something - sensor not found.
-		return unexpected;
+		return UnexpectedCode::Configuration;
 	}
 
 	void showInfo(const char* sensorName) const {
@@ -155,7 +155,7 @@ void Sensor::unregister() {
 
 	// Check if this is a valid sensor entry
 	if (!entry) {
-		return unexpected;
+		return UnexpectedCode::Configuration;
 	}
 
 	return entry->get();

firmware/controllers/sensors/sensor_info_printing.cpp

@@ -15,8 +15,8 @@ void ProxySensor::showInfo(const char* sensorName) const {
 }
 
 void FunctionalSensor::showInfo(const char* sensorName) const {
-	const auto [valid, value] = get();
-	efiPrintf("Sensor \"%s\": Raw value: %.2f Valid: %s Converted value %.2f", sensorName, m_rawValue, boolToString(valid), value);
+	const auto value = get();
+	efiPrintf("Sensor \"%s\": Raw value: %.2f Valid: %s Converted value %.2f", sensorName, m_rawValue, boolToString(value.Valid), value.Value);
 
 	// now print out the underlying function's info
 	if (auto func = m_function) {
@@ -28,8 +28,8 @@ void FunctionalSensor::showInfo(const char* sensorName) const {
 #include "can_sensor.h"
 
 void CanSensorBase::showInfo(const char* sensorName) const {
-	const auto [valid, value] = get();
-	efiPrintf("CAN Sensor \"%s\": valid: %s value: %.2f", sensorName, boolToString(valid), value);
+	const auto value = get();
+	efiPrintf("CAN Sensor \"%s\": valid: %s value: %.2f", sensorName, boolToString(value.Valid), value.Value);
 }
 #endif // EFI_CAN_SUPPORT
 
@@ -63,8 +63,8 @@ void Lps25Sensor::showInfo(const char* sensorName) const {
 
 void LinearFunc::showInfo(float testRawValue) const {
 	efiPrintf("    Linear function slope: %.2f offset: %.2f min: %.1f max: %.1f", m_a, m_b, m_minOutput, m_maxOutput);
-	const auto [valid, value] = convert(testRawValue);
-	efiPrintf("      raw value %.2f converts to %.2f valid: %s", testRawValue, value, boolToString(valid));
+	const auto value = convert(testRawValue);
+	efiPrintf("      raw value %.2f converts to %.2f valid: %s", testRawValue, value.Value, boolToString(value.Valid));
 }
 
 void ResistanceFunc::showInfo(float testInputValue) const {
@@ -73,8 +73,8 @@ void ResistanceFunc::showInfo(float testInputValue) const {
 }
 
 void ThermistorFunc::showInfo(float testInputValue) const {
-	const auto [valid, value] = convert(testInputValue);
-	efiPrintf("    %.1f ohms -> valid: %s. %.1f deg C", testInputValue, boolToString(valid), value);
+	const auto value = convert(testInputValue);
+	efiPrintf("    %.1f ohms -> valid: %s. %.1f deg C", testInputValue, boolToString(value.Valid), value.Value);
 }
 
 void IdentityFunction::showInfo(float /*testInputValue*/) const {

firmware/controllers/sensors/stored_value_sensor.h

@@ -30,51 +30,50 @@
 class StoredValueSensor : public Sensor {
 public:
 	SensorResult get() const final override {
-		bool valid = m_isValid;
-		float value = m_value;
-
-		if (!valid) {
-			return unexpected;
-		}
+		auto result = m_result;
 
 		// Timeouts are disabled, return last value
 		if (Sensor::s_inhibitSensorTimeouts) {
-			return value;
+			return result;
 		}
 
 		if (m_timeoutPeriod != 0) { // zero m_timeoutPeriod means value lasts forever
 			if (getTimeNowNt() - m_timeoutPeriod > m_lastUpdate) {
-				return unexpected;
+				return UnexpectedCode::Timeout;
 			}
 		}
 
-		return value;
+		return result;
 	}
 
 	StoredValueSensor(SensorType type, efitick_t timeoutNt)
 		: Sensor(type)
+		, m_result(unexpected)
 		, m_timeoutPeriod(timeoutNt)
 	{
 	}
 
 	// Invalidate the stored value.
 	void invalidate() {
-		m_isValid = false;
+		m_result = unexpected;
+	}
+
+	// Invalidate the stored value with an error code
+	void invalidate(UnexpectedCode why) {
+		m_result = why;
 	}
 
 	// A new reading is available: set and validate a new value for the sensor.
 	void setValidValue(float value, efitick_t timestamp) {
 		// Set value before valid - so we don't briefly have the valid bit set on an invalid value
-		m_value = value;
-		m_isValid = true;
+		m_result = value;
 		m_lastUpdate = timestamp;
 	}
 
 	void showInfo(const char*) const override { }
 
 private:
-	bool m_isValid = false;
-	float m_value = 0.0f;
+	SensorResult m_result;
 
 	const efitick_t m_timeoutPeriod;
 	efitick_t m_lastUpdate = 0;

firmware/util/expected.h

@@ -17,13 +17,35 @@
 
 struct unexpected_t {};
 
+enum class UnexpectedCode : char {
+	Unknown = 0,
+
+	// Too much time has passed
+	Timeout,
+
+	// The decoded value was impossibly high/low
+	High,
+	Low,
+
+	// An inconsistency was detected using multiple sources of information
+	Inconsistent,
+
+	// A value is unavailable due to configuration
+	Configuration,
+};
 template <class TValue>
 struct expected {
-	const bool Valid;
-	const TValue Value;
+	bool Valid;
+
+	union {
+		TValue Value;
+		UnexpectedCode Code;
+	};
 
 	// Implicit constructor to construct in the invalid state
-	constexpr expected(const unexpected_t&) : Valid(false), Value{} {}
+	constexpr expected(const unexpected_t&) : Valid(false), Code{UnexpectedCode::Unknown} {}
+
+	constexpr expected(UnexpectedCode code) : Valid(false), Code{code} {}
 
 	// Implicit constructor to convert from TValue (for valid values, so an expected<T> behaves like a T)
 	constexpr expected(TValue validValue)

unit_tests/tests/sensor/func_sensor.cpp

@@ -64,6 +64,5 @@ TEST_F(SensorConverted, TestInvalid) {
 	{
 		auto s = Sensor::get(SensorType::Clt);
 		EXPECT_FALSE(s.Valid);
-		EXPECT_FLOAT_EQ(s.Value, 0);
 	}
 }