Add redundant sensor (#1249)

* add redundant sensor

* add tests

* add configure, info printing

* tests

* tests

* casing

Co-authored-by: Matthew Kennedy <matthew@mck-mbp-15.local>
Co-authored-by: Matthew Kennedy <makenne@microsoft.com>

firmware/controllers/sensors/redundant_sensor.cpp

@@ -0,0 +1,42 @@
+#include "redundant_sensor.h"
+
+#include "efilib.h"
+
+RedundantSensor::RedundantSensor(SensorType outputType, SensorType first, SensorType second)
+	: Sensor(outputType)
+	, m_first(first)
+	, m_second(second)
+{
+}
+
+void RedundantSensor::configure(float maxDifference, bool ignoreSecondSensor) {
+	m_maxDifference = maxDifference;
+	m_ignoreSecond = ignoreSecondSensor;
+}
+
+SensorResult RedundantSensor::get() const {
+	auto result1 = Sensor::get(m_first);
+
+	// If we're set to disable redundancy, just pass thru the first sensor
+	if (m_ignoreSecond) {
+		return result1;
+	}
+
+	auto result2 = Sensor::get(m_second);
+
+	// If either result is invalid, return invalid.
+	if (!result1.Valid || !result2.Valid) {
+		return { false, 0 };
+	}
+
+	// If both are valid, check that they're near one another
+	float delta = absF(result1.Value - result2.Value);
+	if (delta > m_maxDifference) {
+		return { false, 0 };
+	}
+
+	// Both sensors are valid, and their readings are close. All is well.
+	// Return the average
+	float avg = (result1.Value + result2.Value) / 2;
+	return { true, avg };
+}

firmware/controllers/sensors/redundant_sensor.h

@@ -0,0 +1,30 @@
+#pragma once
+
+#include "sensor.h"
+
+class RedundantSensor final : public Sensor
+{
+public:
+	RedundantSensor(
+		SensorType outputType,
+		SensorType firstSensor,
+		SensorType secondSensor
+	);
+
+	void configure(float maxDifference, bool ignoreSecondSensor);
+
+	SensorResult get() const override;
+
+	void showInfo(Logging* logger, const char* sensorName) const override;
+
+private:
+	// The two sensors we interpret to form one redundant sensor
+	const SensorType m_first;
+	const SensorType m_second;
+
+	// How far apart do we allow the sensors to be before reporting an issue?
+	float m_maxDifference = 0;
+
+	// Should we ignore the second sensor? (disable redundancy)
+	bool m_ignoreSecond = false;
+};

firmware/controllers/sensors/sensor_info_printing.cpp

@@ -1,6 +1,7 @@
 #include "global.h"
 #include "proxy_sensor.h"
 #include "functional_sensor.h"
+#include "redundant_sensor.h"
 #include "efilib.h"
 #include "loggingcentral.h"
 
@@ -26,3 +27,7 @@ void CanSensorBase::showInfo(Logging* logger, const char* sensorName) const {
 	scheduleMsg(logger, "CAN Sensor \"%s\": valid: %d value: %.2f", sensorName, valid, value);
 }
 #endif // EFI_CAN_SUPPORT
+
+void RedundantSensor::showInfo(Logging* logger, const char* sensorName) const {
+	scheduleMsg(logger, "Redundant sensor \"%s\" combining \"%s\" and \"%s\"", sensorName, getSensorName(m_first), getSensorName(m_second));
+}

firmware/controllers/sensors/sensors.mk

@@ -13,6 +13,7 @@ CONTROLLERS_SENSORS_SRC_CPP = 	$(PROJECT_DIR)/controllers/sensors/thermistors.cp
 	$(PROJECT_DIR)/controllers/sensors/sensor.cpp \
 	$(PROJECT_DIR)/controllers/sensors/sensor_info_printing.cpp \
 	$(PROJECT_DIR)/controllers/sensors/functional_sensor.cpp \
+	$(PROJECT_DIR)/controllers/sensors/redundant_sensor.cpp \
 	$(PROJECT_DIR)/controllers/sensors/converters/linear_func.cpp \
 	$(PROJECT_DIR)/controllers/sensors/converters/resistance_func.cpp \
 	$(PROJECT_DIR)/controllers/sensors/converters/thermistor_func.cpp

unit_tests/tests/sensor/redundant.cpp

@@ -0,0 +1,218 @@
+#include "redundant_sensor.h"
+
+#include <gtest/gtest.h>
+
+#include "mock/mock_sensor.h"
+
+class SensorRedundant : public ::testing::Test
+{
+protected:
+	RedundantSensor dut;
+	MockSensor m1, m2;
+
+	SensorRedundant()
+		: dut(SensorType::Tps1, SensorType::Tps1Primary, SensorType::Tps1Secondary)
+		, m1(SensorType::Tps1Primary)
+		, m2(SensorType::Tps1Secondary)
+	{
+	}
+
+	void SetUp() override
+	{
+		Sensor::resetRegistry();
+
+		// Other tests verify registry function - don't re-test it here
+		ASSERT_TRUE(dut.Register());
+		ASSERT_TRUE(m1.Register());
+		ASSERT_TRUE(m2.Register());
+
+		dut.configure(5.0f, false);
+	}
+
+	void TearDown() override
+	{
+		Sensor::resetRegistry();
+	}
+};
+
+TEST_F(SensorRedundant, SetOnlyOneSensor)
+{
+	// Don't set any sensors - expect invalid
+	{
+		auto result = dut.get();
+		EXPECT_FALSE(result.Valid);
+	}
+
+	// Set one sensor
+	m1.set(24.0f);
+
+	// Should still be invalid - only one is set!
+	{
+		auto result = dut.get();
+		EXPECT_FALSE(result.Valid);
+	}
+}
+
+TEST_F(SensorRedundant, SetTwoSensors)
+{
+	// Don't set any sensors - expect invalid
+	{
+		auto result = dut.get();
+		EXPECT_FALSE(result.Valid);
+	}
+
+	// Set one sensor
+	m1.set(24.0f);
+	// Set the other sensor
+	m2.set(26.0f);
+
+	// Should now be valid - and the average of the two input
+	{
+		auto result = dut.get();
+		EXPECT_TRUE(result.Valid);
+		EXPECT_FLOAT_EQ(result.Value, 25.0f);
+	}
+}
+
+TEST_F(SensorRedundant, DifferenceNone)
+{
+	// Set both sensors to the same value
+	m1.set(10);
+	m2.set(10);
+
+	// Expect valid, and 10 output
+	{
+		auto result = dut.get();
+		EXPECT_TRUE(result.Valid);
+		EXPECT_FLOAT_EQ(result.Value, 10.0f);
+	}
+}
+
+TEST_F(SensorRedundant, DifferenceNearLimit)
+{
+	// Set both sensors to nearly the limit (4.998 apart)
+	m1.set(7.501f);
+	m2.set(12.499f);
+
+	// Expect valid, and 10 output
+	{
+		auto result = dut.get();
+		EXPECT_TRUE(result.Valid);
+		EXPECT_FLOAT_EQ(result.Value, 10.0f);
+	}
+}
+
+TEST_F(SensorRedundant, DifferenceOverLimit)
+{
+	// Set both sensors barely over the limit (5.002 apart)
+	m1.set(7.499f);
+	m2.set(12.501f);
+
+	// Expect invalid
+	{
+		auto result = dut.get();
+		EXPECT_FALSE(result.Valid);
+	}
+}
+
+TEST_F(SensorRedundant, DifferenceOverLimitSwapped)
+{
+	// Now try it the other way (m1 > m2)
+	m1.set(12.501f);
+	m2.set(7.499f);
+
+	// Expect invalid
+	{
+		auto result = dut.get();
+		EXPECT_FALSE(result.Valid);
+	}
+}
+
+
+class SensorRedundantIgnoreSecond : public ::testing::Test
+{
+protected:
+	RedundantSensor dut;
+	MockSensor m1, m2;
+
+	SensorRedundantIgnoreSecond()
+		: dut(SensorType::Tps1, SensorType::Tps1Primary, SensorType::Tps1Secondary)
+		, m1(SensorType::Tps1Primary)
+		, m2(SensorType::Tps1Secondary)
+	{
+	}
+
+	void SetUp() override
+	{
+		Sensor::resetRegistry();
+
+		// Other tests verify registry function - don't re-test it here
+		ASSERT_TRUE(dut.Register());
+		ASSERT_TRUE(m1.Register());
+		ASSERT_TRUE(m2.Register());
+
+		dut.configure(5.0f, true);
+	}
+
+	void TearDown() override
+	{
+		Sensor::resetRegistry();
+	}
+};
+
+TEST_F(SensorRedundantIgnoreSecond, OnlyFirst)
+{
+	// Don't set any sensors - expect invalid
+	{
+		auto result = dut.get();
+		EXPECT_FALSE(result.Valid);
+	}
+
+	// Set one sensor
+	m1.set(44.0f);
+
+	// Should be valid - we don't care about second sensor
+	{
+		auto result = dut.get();
+		EXPECT_TRUE(result.Valid);
+		EXPECT_FLOAT_EQ(result.Value, 44.0f);
+	}
+}
+
+TEST_F(SensorRedundantIgnoreSecond, OnlySecond)
+{
+	// Don't set any sensors - expect invalid
+	{
+		auto result = dut.get();
+		EXPECT_FALSE(result.Valid);
+	}
+
+	// Set second sensor only
+	m2.set(66.0f);
+
+	// Should be invalid - should ignore second sensor
+	{
+		auto result = dut.get();
+		EXPECT_FALSE(result.Valid);
+	}
+}
+
+TEST_F(SensorRedundantIgnoreSecond, SetBothIgnoreSecond)
+{
+	// Don't set any sensors - expect invalid
+	{
+		auto result = dut.get();
+		EXPECT_FALSE(result.Valid);
+	}
+
+	// Set both sensors
+	m1.set(74.0f);
+	m2.set(76.0f);
+
+	// Should be valid, but only get the value from m1
+	{
+		auto result = dut.get();
+		EXPECT_TRUE(result.Valid);
+		EXPECT_FLOAT_EQ(result.Value, 74.0f);
+	}
+}

unit_tests/tests/tests.mk

@@ -43,4 +43,5 @@ TESTS_SRC_CPP = \
 	tests/sensor/lin_func.cpp \
 	tests/sensor/resist_func.cpp \
 	tests/sensor/therm_func.cpp \
-	tests/sensor/func_chain.cpp
+	tests/sensor/func_chain.cpp \
+	tests/sensor/redundant.cpp