From b0410167de88fa904bdc74b01eb246d2053f37e6 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Sat, 5 Dec 2020 09:16:52 -0600 Subject: [PATCH] encapsulate entry logic in to entry (#2018) Co-authored-by: Matthew Kennedy --- firmware/controllers/sensors/sensor.cpp | 214 ++++++++++++------------ firmware/controllers/sensors/sensor.h | 18 +- firmware/init/sensor/init_sensors.cpp | 4 +- 3 files changed, 120 insertions(+), 116 deletions(-) diff --git a/firmware/controllers/sensors/sensor.cpp b/firmware/controllers/sensors/sensor.cpp index 66fd00deca..ed6cdf4eb9 100644 --- a/firmware/controllers/sensors/sensor.cpp +++ b/firmware/controllers/sensors/sensor.cpp @@ -3,51 +3,6 @@ #include "efilib.h" #include "loggingcentral.h" -// This struct represents one sensor in the registry. -// It stores whether the sensor should use a mock value, -// the value to use, and if not a pointer to the sensor that -// can provide a real value. -class SensorRegistryEntry { -public: - Sensor *getSensor() { - return sensor; - } - - void setSensor(Sensor * sensor) { - this->sensor = sensor; - } - - bool isMock() { - return useMock; - } - - void setMockValue(float value) { - mockValue = value; - useMock = true; - } - - float getMockValue() { - return mockValue; - } - - void resetMock() { - useMock = false; - mockValue = 0.0f; - } - - void reset() { - sensor = nullptr; - resetMock(); - } - -private: - bool useMock = false; - float mockValue; - Sensor *sensor = nullptr; -}; - -static SensorRegistryEntry s_sensorRegistry[static_cast(SensorType::PlaceholderLast)] = {}; - static const char* s_sensorNames[] = { "Invalid", "CLT", @@ -85,24 +40,104 @@ static const char* s_sensorNames[] = { "Idle Valve Position", }; +// This struct represents one sensor in the registry. +// It stores whether the sensor should use a mock value, +// the value to use, and if not a pointer to the sensor that +// can provide a real value. +class SensorRegistryEntry { +public: + const Sensor* getSensor() { + return m_sensor; + } + + void setMockValue(float value) { + m_mockValue = value; + m_useMock = true; + } + + void resetMock() { + m_useMock = false; + m_mockValue = 0.0f; + } + + void reset() { + m_sensor = nullptr; + resetMock(); + } + + bool Register(Sensor* sensor) { + // If there's somebody already here - a consumer tried to double-register a sensor + if (m_sensor) { + // This sensor has already been registered. Don't re-register it. + #if ! EFI_UNIT_TEST + firmwareError(CUSTOM_OBD_26, "Duplicate registration for %s sensor", sensor->getSensorName()); + #endif + return false; + } else { + // Put the sensor in the registry + m_sensor = sensor; + return true; + } + } + + SensorResult get() const { + // Check if mock + if (m_useMock) { + return m_mockValue; + } + + // Get the sensor out of the entry + const Sensor *s = m_sensor; + if (s) { + // If we found the sensor, ask it for a result. + return s->get(); + } + + // We've exhausted all valid ways to return something - sensor not found. + return unexpected; + } + + void showInfo(Logging* logger, const char* sensorName) const { + if (m_useMock) { + scheduleMsg(logger, "Sensor \"%s\" mocked with value %.2f", sensorName, m_mockValue); + } else { + const auto sensor = m_sensor; + + if (sensor) { + sensor->showInfo(logger, sensorName); + } else { + scheduleMsg(logger, "Sensor \"%s\" is not configured.", sensorName); + } + } + } + + bool hasSensor() const { + return m_useMock || m_sensor; + } + + float getRaw() const { + const auto sensor = m_sensor; + + if (m_sensor) { + return m_sensor->getRaw(); + } + + // We've exhausted all valid ways to return something - sensor not found. + return 0; + } + +private: + bool m_useMock = false; + float m_mockValue; + Sensor* m_sensor = nullptr; +}; + +static SensorRegistryEntry s_sensorRegistry[static_cast(SensorType::PlaceholderLast)] = {}; + static_assert(efi::size(s_sensorNames) == efi::size(s_sensorRegistry)); bool Sensor::Register() { - // Get a ref to where we should be - auto &entry = s_sensorRegistry[getIndex()]; - - // If there's somebody already here - a consumer tried to double-register a sensor - if (entry.getSensor()) { - // This sensor has already been registered. Don't re-register it. -#if ! EFI_UNIT_TEST - firmwareError(CUSTOM_OBD_26, "Duplicate registration for %s sensor", s_sensorNames[getIndex()]); -#endif - return false; - } else { - // put ourselves in the registry - entry.setSensor(this); - return true; - } + return s_sensorRegistry[getIndex()].Register(this); } /*static*/ void Sensor::resetRegistry() { @@ -137,47 +172,19 @@ bool Sensor::Register() { return unexpected; } - // Next check for mock - if (entry->isMock()) { - return entry->getMockValue(); - } - - // Get the sensor out of the entry - const Sensor *s = entry->getSensor(); - if (s) { - // If we found the sensor, ask it for a result. - return s->get(); - } - - // We've exhausted all valid ways to return something - sensor not found. - return unexpected; + return entry->get(); } /*static*/ float Sensor::getRaw(SensorType type) { const auto entry = getEntryForType(type); - // Check if this is a valid sensor entry - if (!entry) { - return 0; - } - - const auto s = entry->getSensor(); - if (s) { - return s->getRaw(); - } - - // We've exhausted all valid ways to return something - sensor not found. - return 0; + return entry ? entry->getRaw() : 0; } /*static*/ bool Sensor::hasSensor(SensorType type) { const auto entry = getEntryForType(type); - if (!entry) { - return false; - } - - return entry->isMock() || entry->getSensor(); + return entry ? entry->hasSensor() : false; } /*static*/ void Sensor::setMockValue(SensorType type, float value) { @@ -208,9 +215,7 @@ bool Sensor::Register() { /*static*/ void Sensor::resetAllMocks() { // Reset all mocks for (size_t i = 0; i < efi::size(s_sensorRegistry); i++) { - auto &entry = s_sensorRegistry[i]; - - entry.resetMock(); + s_sensorRegistry[i].resetMock(); } } @@ -224,16 +229,15 @@ bool Sensor::Register() { auto& entry = s_sensorRegistry[i]; const char* name = s_sensorNames[i]; - if (entry.isMock()) { - scheduleMsg(logger, "Sensor \"%s\" mocked with value %.2f", name, entry.getMockValue()); - } else { - const auto sensor = entry.getSensor(); - - if (sensor) { - sensor->showInfo(logger, name); - } else { - scheduleMsg(logger, "Sensor \"%s\" is not configured.", name); - } - } + entry.showInfo(logger, name); + } +} + +// Print information about a particular sensor +/*static*/ void Sensor::showInfo(Logging* logger, SensorType type) { + auto entry = getEntryForType(type); + + if (entry) { + entry->showInfo(logger, getSensorName(type)); } } diff --git a/firmware/controllers/sensors/sensor.h b/firmware/controllers/sensors/sensor.h index b647e73d9c..19e10b8504 100644 --- a/firmware/controllers/sensors/sensor.h +++ b/firmware/controllers/sensors/sensor.h @@ -71,6 +71,9 @@ public: // Print information about all sensors static void showAllSensorInfo(Logging* logger); + // Print information about a particular sensor + static void showInfo(Logging* logger, SensorType type); + // Remove all sensors from the sensor registry - tread carefully if you use this outside of a unit test static void resetRegistry(); @@ -127,6 +130,13 @@ public: // this should be field lookup and simple math. virtual SensorResult get() const = 0; + /* + * Get an unconverted value from the sensor, if available. + */ + virtual float getRaw() const { + return 0; + } + protected: // Protected constructor - only subclasses call this explicit Sensor(SensorType type) @@ -135,14 +145,6 @@ protected: static const char* getSensorName(SensorType type); private: - - /* - * Get an unconverted value from the sensor, if available. - */ - virtual float getRaw() const { - return 0; - } - const SensorType m_type; // Get this sensor's index in the list diff --git a/firmware/init/sensor/init_sensors.cpp b/firmware/init/sensor/init_sensors.cpp index 3e74aad9be..ffbc273337 100644 --- a/firmware/init/sensor/init_sensors.cpp +++ b/firmware/init/sensor/init_sensors.cpp @@ -39,8 +39,6 @@ static void initSensorCli(Logging* logger) { addConsoleAction("show_sensors", []() { Sensor::showAllSensorInfo(s_logger); }); addConsoleActionI("show_sensor", [](int idx) { - if (auto s = Sensor::getSensorOfType(static_cast(idx))) { - s->showAllSensorInfo(s_logger); - } + Sensor::showInfo(s_logger, static_cast(idx)); }); }