Sensors: composition >> inheritance (#953)

* functional sensors

* format

* tests

* sim makefile

* eol

* format tests

firmware/Makefile

@@ -306,6 +306,7 @@ INCDIR = $(CHIBIOS)/os/license \
 	development/test \
 	$(CONTROLLERS_INC) \
 	controllers/sensors \
+	controllers/sensors/converters \
 	controllers/system \
 	controllers/algo \
 	controllers/core \

firmware/controllers/sensors/converter_sensor.h

@@ -1,60 +0,0 @@
-/**
- * @file    converter_sensor.h
- *
- * @date September 12, 2019
- * @author Matthew Kennedy, (c) 2019
- */
-
-#pragma once
-
-#include "stored_value_sensor.h"
-
-/**
- * @brief Base class for sensors that convert from some raw floating point
- * value (ex: voltage, frequency, pulse width) to a sensor reading.
- *
- * To use this base class, inherit it and implement ConvertFromInputValue(float input).
- * Perform any conversion work necessary to convert from the raw value to a sensor
- * reading, and return it.  Register an instance of the new class with an interface
- * that provides and posts raw values so the sensor can update.
- */
-class ConvertedSensor : public StoredValueSensor {
-public:
-	void postRawValue(float inputValue) {
-		// Report the raw value
-		float *rawReportLocation = m_rawReportingLocation;
-		if (rawReportLocation) {
-			*rawReportLocation = inputValue;
-		}
-
-		auto r = convertFromInputValue(inputValue);
-
-		// This has to happen so that we set the valid bit after
-		// the value is stored, to prevent the data race of reading
-		// an old invalid value
-		if (r.Valid) {
-			setValidValue(r.Value);
-		} else {
-			invalidate();
-		}
-	}
-
-	void setRawReportingLocation(float *rawReportingLocation) {
-		m_rawReportingLocation = rawReportingLocation;
-	}
-
-protected:
-	explicit ConvertedSensor(SensorType type)
-		: StoredValueSensor(type) {}
-
-	/**
-	 * @brief Convert from the "raw" input value to a sensor reading (or invalid).
-	 *
-	 * For example, this function might convert from a voltage to the pressure
-	 * represented by that voltage.
-	 */
-	virtual SensorResult convertFromInputValue(float inputValue) = 0;
-
-private:
-	float *m_rawReportingLocation = nullptr;
-};

firmware/controllers/sensors/converters/linear_func.cpp

@@ -1,8 +1,8 @@
-#include "linear_sensor.h"
+#include "linear_func.h"
 
 #include "interpolation.h"
 
-void LinearSensor::configure(float in1, float out1, float in2, float out2, float minOutput, float maxOutput) {
+void LinearFunc::configure(float in1, float out1, float in2, float out2, float minOutput, float maxOutput) {
 	m_minOutput = minOutput;
 	m_maxOutput = maxOutput;
 
@@ -10,7 +10,7 @@ void LinearSensor::configure(float in1, float out1, float in2, float out2, float
 	m_b = out1 - m_a * in1;
 }
 
-SensorResult LinearSensor::convertFromInputValue(float inputValue) {
+SensorResult LinearFunc::convert(float inputValue) const {
 	float result = m_a * inputValue + m_b;
 
 	// Bounds check

firmware/controllers/sensors/converters/linear_func.h

@@ -1,16 +1,14 @@
 #pragma once
 
-#include "converter_sensor.h"
+#include "sensor_converter_func.h"
 
-class LinearSensor final : public ConvertedSensor {
+class LinearFunc final : public SensorConverter {
 public:
-	explicit LinearSensor(SensorType type)
-		: ConvertedSensor(type) {}
+	LinearFunc() = default;
 
 	void configure(float in1, float out1, float in2, float out2, float minOutput, float maxOutput);
 
-protected:
-	SensorResult convertFromInputValue(float inputValue) override;
+	SensorResult convert(float inputValue) const override;
 
 private:
 	// Linear equation parameters for equation of form

firmware/controllers/sensors/converters/sensor_converter_func.h

@@ -0,0 +1,7 @@
+#pragma once
+
+#include "sensor.h"
+
+struct SensorConverter {
+	virtual SensorResult convert(float raw) const = 0;
+};

firmware/controllers/sensors/functional_sensor.h

@@ -0,0 +1,75 @@
+/**
+ * @file    converter_sensor.h
+ *
+ * @date September 12, 2019
+ * @author Matthew Kennedy, (c) 2019
+ */
+
+#pragma once
+
+#include "converters/sensor_converter_func.h"
+#include "stored_value_sensor.h"
+
+#include <type_traits>
+
+struct FunctionalSensorBase : public StoredValueSensor {
+	FunctionalSensorBase(SensorType type)
+		: StoredValueSensor(type) {}
+
+	virtual void postRawValue(float value) = 0;
+};
+
+/**
+ * @brief Class for sensors that convert from some raw floating point
+ * value (ex: voltage, frequency, pulse width) to a sensor reading.
+ *
+ * To use this class, implement the conversion operation for your sensor
+ * as a class that inherits from SensorConverter, and implement convert
+ * to convert a raw reading from the sensor to a usable value (and valid bit).
+ *
+ * Register an instance of the new class with an interface
+ * that provides and posts raw values so the sensor can update.
+ */
+template <typename TFunc>
+class FunctionalSensor final : public FunctionalSensorBase {
+	static_assert(std::is_base_of_v<SensorConverter, TFunc>, "TFunc must inherit from SensorConverter");
+	static_assert(std::is_default_constructible_v<TFunc>, "TFunc must be default constructible");
+
+public:
+	explicit FunctionalSensor(SensorType type)
+		: FunctionalSensorBase(type) {}
+
+	void postRawValue(float inputValue) {
+		// Report the raw value
+		float *rawReportLocation = m_rawReportingLocation;
+		if (rawReportLocation) {
+			*rawReportLocation = inputValue;
+		}
+
+		auto r = m_function.convert(inputValue);
+
+		// This has to happen so that we set the valid bit after
+		// the value is stored, to prevent the data race of reading
+		// an old invalid value
+		if (r.Valid) {
+			setValidValue(r.Value);
+		} else {
+			invalidate();
+		}
+	}
+
+	void setRawReportingLocation(float *rawReportingLocation) {
+		m_rawReportingLocation = rawReportingLocation;
+	}
+
+	// Allow access to the underlying function
+	TFunc &f() {
+		return m_function;
+	}
+
+private:
+	float *m_rawReportingLocation = nullptr;
+
+	// Conversion function for this sensor
+	TFunc m_function;
+};

firmware/controllers/sensors/sensors.mk

@@ -11,4 +11,4 @@ CONTROLLERS_SENSORS_SRC_CPP = 	$(PROJECT_DIR)/controllers/sensors/thermistors.cp
 	$(PROJECT_DIR)/controllers/sensors/maf2map.cpp \
 	$(PROJECT_DIR)/controllers/sensors/hip9011_lookup.cpp \
 	$(PROJECT_DIR)/controllers/sensors/sensor.cpp \
-	$(PROJECT_DIR)/controllers/sensors/linear_sensor.cpp
+	$(PROJECT_DIR)/controllers/sensors/converters/linear_func.cpp

firmware/hw_layer/adc_subscription.cpp

@@ -1,4 +1,5 @@
 #include "adc_subscription.h"
+
 #include "adc_inputs.h"
 #include "engine.h"
 
@@ -9,7 +10,7 @@ EXTERN_ENGINE;
 #if !EFI_UNIT_TEST
 
 struct AdcSubscriptionEntry {
-	ConvertedSensor* Sensor;
+	FunctionalSensorBase *Sensor;
 	float VoltsPerAdcVolt;
 	adc_channel_e Channel;
 };
@@ -17,12 +18,14 @@ struct AdcSubscriptionEntry {
 static size_t s_nextEntry = 0;
 static AdcSubscriptionEntry s_entries[8];
 
-void AdcSubscription::SubscribeSensor(ConvertedSensor& sensor, adc_channel_e channel, float voltsPerAdcVolt /*= 0.0f*/) {
+void AdcSubscription::SubscribeSensor(FunctionalSensorBase &sensor,
+									  adc_channel_e channel,
+									  float voltsPerAdcVolt /*= 0.0f*/) {
 	// Don't subscribe null channels
 	if (channel == EFI_ADC_NONE) {
 		return;
 	}
-	
+
 	// bounds check
 	if (s_nextEntry >= std::size(s_entries)) {
 		return;
@@ -34,7 +37,7 @@ void AdcSubscription::SubscribeSensor(ConvertedSensor& sensor, adc_channel_e cha
 	}
 
 	// Populate the entry
-	auto& entry = s_entries[s_nextEntry];
+	auto &entry = s_entries[s_nextEntry];
 	entry.Sensor = &sensor;
 	entry.VoltsPerAdcVolt = voltsPerAdcVolt;
 	entry.Channel = channel;
@@ -44,7 +47,7 @@ void AdcSubscription::SubscribeSensor(ConvertedSensor& sensor, adc_channel_e cha
 
 void AdcSubscription::UpdateSubscribers() {
 	for (size_t i = 0; i < s_nextEntry; i++) {
-		auto& entry = s_entries[i];
+		auto &entry = s_entries[i];
 
 		float mcuVolts = getVoltage("sensor", entry.Channel);
 		float sensorVolts = mcuVolts * entry.VoltsPerAdcVolt;
@@ -53,4 +56,4 @@ void AdcSubscription::UpdateSubscribers() {
 	}
 }
 
-#endif// !EFI_UNIT_TEST
+#endif // !EFI_UNIT_TEST

firmware/hw_layer/adc_subscription.h

@@ -2,11 +2,13 @@
  * @file adc_subscription.h
  */
 
+#pragma once
+
+#include "functional_sensor.h"
 #include "global.h"
-#include "converter_sensor.h"
 
 class AdcSubscription {
 public:
-	static void SubscribeSensor(ConvertedSensor& sensor, adc_channel_e channel, float voltsPerAdcVolt = 0.0f);
+	static void SubscribeSensor(FunctionalSensorBase &sensor, adc_channel_e channel, float voltsPerAdcVolt = 0.0f);
 	static void UpdateSubscribers();
 };

firmware/init/sensor/init_oil_pressure.cpp

@@ -2,14 +2,15 @@
 #include "engine.h"
 #include "error_handling.h"
 #include "global.h"
-#include "linear_sensor.h"
+#include "functional_sensor.h"
+#include "linear_func.h"
 #include "tunerstudio_configuration.h"
 
 EXTERN_ENGINE;
 
 extern TunerStudioOutputChannels tsOutputChannels;
 
-LinearSensor oilpSensor(SensorType::OilPressure);
+FunctionalSensor<LinearFunc> oilpSensor(SensorType::OilPressure);
 
 void initOilPressure() {
 	// Only register if we have a sensor
@@ -28,7 +29,7 @@ void initOilPressure() {
 	float greaterOutput = val1 > val2 ? val1 : val2;
 
 	// Allow slightly negative output (-5kpa) so as to not fail the sensor when engine is off
-	oilpSensor.configure(sensorCfg->v1, val1, sensorCfg->v2, val2, /*minOutput*/ -5, greaterOutput);
+	oilpSensor.f().configure(sensorCfg->v1, val1, sensorCfg->v2, val2, /*minOutput*/ -5, greaterOutput);
 
 	// Tell it to report to its output channel
 	oilpSensor.setReportingLocation(&tsOutputChannels.oilPressure);

simulator/Makefile

@@ -208,6 +208,7 @@ INCDIR = . \
           $(PROJECT_DIR)/controllers/core \
           $(PROJECT_DIR)/controllers/math \
           $(PROJECT_DIR)/controllers/sensors \
+          $(PROJECT_DIR)/controllers/sensors/converters \
           $(PROJECT_DIR)/controllers/system \
           $(PROJECT_DIR)/controllers/trigger \
           $(PROJECT_DIR)/controllers/trigger/decoders \

unit_tests/Makefile

@@ -170,6 +170,7 @@ INCDIR = . \
 	$(CONTROLLERS_INC) \
 	$(PROJECT_DIR)/console \
 	$(PROJECT_DIR)/controllers/sensors \
+	$(PROJECT_DIR)/controllers/sensors/converters \
 	$(PROJECT_DIR)/controllers/algo \
 	$(PROJECT_DIR)/controllers/core \
 	$(PROJECT_DIR)/controllers/math \

unit_tests/tests/sensor/functional_sensor.cpp

@@ -1,25 +1,9 @@
-#include "converter_sensor.h"
+#include "functional_sensor.h"
 
 #include <gtest/gtest.h>
 
-class SensorConverted : public ::testing::Test {
-protected:
-	void SetUp() override {
-		Sensor::resetRegistry();
-	}
-
-	void TearDown() override {
-		Sensor::resetRegistry();
-	}
-};
-
-class DoublerConverterSensor final : public ConvertedSensor {
-public:
-	DoublerConverterSensor()
-		: ConvertedSensor(SensorType::Clt) {}
-
-protected:
-	SensorResult convertFromInputValue(float input) {
+struct DoublerFunc final : public SensorConverter {
+	SensorResult convert(float input) const {
 		bool valid = input > 0;
 		float value = input * 2;
 
@@ -27,8 +11,23 @@ protected:
 	}
 };
 
+class SensorConverted : public ::testing::Test {
+protected:
+	SensorConverted()
+		: dut(SensorType::Clt) {}
+
+	void SetUp() override {
+		Sensor::resetRegistry();
+	}
+
+	void TearDown() override {
+		Sensor::resetRegistry();
+	}
+
+	FunctionalSensor<DoublerFunc> dut;
+};
+
 TEST_F(SensorConverted, TestValid) {
-	DoublerConverterSensor dut;
 	ASSERT_TRUE(dut.Register());
 
 	// Should be invalid - not set yet
@@ -48,7 +47,6 @@ TEST_F(SensorConverted, TestValid) {
 }
 
 TEST_F(SensorConverted, TestInvalid) {
-	DoublerConverterSensor dut;
 	ASSERT_TRUE(dut.Register());
 
 	// Should be invalid - not set yet
@@ -66,3 +64,8 @@ TEST_F(SensorConverted, TestInvalid) {
 		EXPECT_FLOAT_EQ(s.Value, 0);
 	}
 }
+
+TEST_F(SensorConverted, TestGet) {
+	// we're only checking that this compiles
+	DoublerFunc &f = dut.f();
+}

unit_tests/tests/sensor/lin_func.cpp

@@ -1,14 +1,10 @@
-#include "linear_sensor.h"
-
+#include "linear_func.h"
 #include "unit_test_framework.h"
 
-class SensorLinear : public ::testing::Test {
+class LinearFuncTest : public ::testing::Test {
 protected:
 	// Maps (1, 4) -> (100, -100)
-	LinearSensor dut;
-
-	SensorLinear()
-		: dut(SensorType::Clt) {}
+	LinearFunc dut;
 
 	void SetUp() override {
 		dut.configure(1, 100, 4, -100, -110, 110);
@@ -17,26 +13,22 @@ protected:
 
 #define test_point(in, out)                                                                                            \
 	{                                                                                                                  \
-		dut.postRawValue(in);                                                                                          \
-		auto result = dut.get();                                                                                       \
+		auto result = dut.convert(in);                                                                                 \
                                                                                                                        \
 		EXPECT_TRUE(result.Valid);                                                                                     \
 		ASSERT_NEAR(result.Value, (out), EPS4D) << "Not " << out << " for " << in;                                     \
 	}
 
 #define test_point_invalid(in)                                                                                         \
-	{                                                                                                                  \
-		dut.postRawValue(in);                                                                                          \
-		EXPECT_FALSE(dut.get().Valid);                                                                                 \
-	}
+	{ EXPECT_FALSE(dut.convert(in).Valid); }
 
-TEST_F(SensorLinear, TestInRange) {
+TEST_F(LinearFuncTest, TestInRange) {
 	test_point(2.5, 0);
 	test_point(1, 100);
 	test_point(4, -100);
 }
 
-TEST_F(SensorLinear, TestOutOfRange) {
+TEST_F(LinearFuncTest, TestOutOfRange) {
 	test_point(1, 100);
 	test_point_invalid(0.5);
 

unit_tests/tests/tests.mk

@@ -29,8 +29,8 @@ TESTS_SRC_CPP = \
 	tests/test_gpiochip.cpp \
 \
 	tests/sensor/basic_sensor.cpp \
-	tests/sensor/converter_sensor.cpp \
+	tests/sensor/functional_sensor.cpp \
 	tests/sensor/function_pointer_sensor.cpp \
 	tests/sensor/mock_sensor.cpp \
 	tests/sensor/sensor_reader.cpp \
-	tests/sensor/lin_sensor.cpp
+	tests/sensor/lin_func.cpp