From 483d4a220476ab7236c83cab6ac632e7da2de554 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 30 Mar 2020 15:29:42 -0700 Subject: [PATCH] Debuggability in the new sensor world (#1238) * rename to avoid conflict * fix efilib * add sensor printing * makefile * that check was already there * const * const * fix tests * formatting Co-authored-by: Matthew Kennedy --- firmware/controllers/engine_controller.cpp | 6 +- .../sensors/function_pointer_sensor.h | 2 + .../controllers/sensors/functional_sensor.h | 2 + firmware/controllers/sensors/proxy_sensor.h | 2 + firmware/controllers/sensors/sensor.cpp | 58 +++++++++++++++++-- firmware/controllers/sensors/sensor.h | 9 +++ .../sensors/sensor_info_printing.cpp | 13 +++++ firmware/controllers/sensors/sensors.mk | 1 + firmware/init/init.h | 4 +- firmware/init/sensor/init_sensors.cpp | 19 ++++-- firmware/util/efilib.h | 26 ++++----- firmware/util/loggingcentral.h | 2 + unit_tests/tests/sensor/mock/mock_sensor.h | 2 + 13 files changed, 118 insertions(+), 28 deletions(-) create mode 100644 firmware/controllers/sensors/sensor_info_printing.cpp diff --git a/firmware/controllers/engine_controller.cpp b/firmware/controllers/engine_controller.cpp index 3c758a6136..7ba2270953 100644 --- a/firmware/controllers/engine_controller.cpp +++ b/firmware/controllers/engine_controller.cpp @@ -123,7 +123,7 @@ void initDataStructures(DECLARE_ENGINE_PARAMETER_SIGNATURE) { static void mostCommonInitEngineController(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) { #if !EFI_UNIT_TEST - initSensors(); + initNewSensors(sharedLogger); #endif /* EFI_UNIT_TEST */ initSensors(sharedLogger PASS_ENGINE_PARAMETER_SUFFIX); @@ -368,11 +368,9 @@ static void printAnalogChannelInfoExt(const char *name, adc_channel_e hwChannel, } static void printAnalogChannelInfo(const char *name, adc_channel_e hwChannel) { - if (hwChannel != EFI_ADC_NONE) { #if HAL_USE_ADC - printAnalogChannelInfoExt(name, hwChannel, getVoltage("print", hwChannel PASS_ENGINE_PARAMETER_SUFFIX), engineConfiguration->analogInputDividerCoefficient); + printAnalogChannelInfoExt(name, hwChannel, getVoltage("print", hwChannel PASS_ENGINE_PARAMETER_SUFFIX), engineConfiguration->analogInputDividerCoefficient); #endif /* HAL_USE_ADC */ - } } static void printAnalogInfo(void) { diff --git a/firmware/controllers/sensors/function_pointer_sensor.h b/firmware/controllers/sensors/function_pointer_sensor.h index 716fd0ef05..58e29f5b29 100644 --- a/firmware/controllers/sensors/function_pointer_sensor.h +++ b/firmware/controllers/sensors/function_pointer_sensor.h @@ -29,6 +29,8 @@ public: return {valid, result}; } + void showInfo(Logging* logger, const char* sensorName) const override {} + private: float (*m_func)(); }; diff --git a/firmware/controllers/sensors/functional_sensor.h b/firmware/controllers/sensors/functional_sensor.h index a9ff9f530b..0a51374b88 100644 --- a/firmware/controllers/sensors/functional_sensor.h +++ b/firmware/controllers/sensors/functional_sensor.h @@ -38,6 +38,8 @@ public: return m_rawValue; } + void showInfo(Logging* logger, const char* sensorName) const override; + private: // Conversion function for this sensor SensorConverter* m_function = nullptr; diff --git a/firmware/controllers/sensors/proxy_sensor.h b/firmware/controllers/sensors/proxy_sensor.h index aebc62de85..5518d4e108 100644 --- a/firmware/controllers/sensors/proxy_sensor.h +++ b/firmware/controllers/sensors/proxy_sensor.h @@ -21,6 +21,8 @@ public: m_proxiedSensor = proxiedSensor; } + void showInfo(Logging* logger, const char* sensorName) const override; + private: SensorResult get() const { return Sensor::get(m_proxiedSensor); diff --git a/firmware/controllers/sensors/sensor.cpp b/firmware/controllers/sensors/sensor.cpp index 9081e2cbc2..fd4b4ab68f 100644 --- a/firmware/controllers/sensors/sensor.cpp +++ b/firmware/controllers/sensors/sensor.cpp @@ -1,4 +1,6 @@ #include "sensor.h" +#include "efilib.h" +#include "loggingcentral.h" // This struct represents one sensor in the registry. // It stores whether the sensor should use a mock value, @@ -12,6 +14,30 @@ struct SensorRegistryEntry { static SensorRegistryEntry s_sensorRegistry[static_cast(SensorType::PlaceholderLast)] = {}; +static const char* s_sensorNames[] = { + "Invalid", + "CLT", + "IAT", + + "Oil Pressure", + + "TPS 1", + "TPS 1 Primary", + "TPS 1 Secondary", + + "TPS 2", + "TPS 2 Primary", + "TPS 2 Secondary", + + "Acc Pedal", + "Acc Pedal Primary", + "Acc Pedal Secondary", + + "Driver Acc Intent" +}; + +static_assert(efi::size(s_sensorNames) == efi::size(s_sensorNames)); + bool Sensor::Register() { // Get a ref to where we should be auto &entry = s_sensorRegistry[getIndex()]; @@ -28,10 +54,8 @@ bool Sensor::Register() { } /*static*/ void Sensor::resetRegistry() { - constexpr size_t len = sizeof(s_sensorRegistry) / sizeof(s_sensorRegistry[0]); - // Clear all entries - for (size_t i = 0; i < len; i++) { + for (size_t i = 0; i < efi::size(s_sensorRegistry); i++) { auto &entry = s_sensorRegistry[i]; entry.sensor = nullptr; @@ -123,12 +147,34 @@ bool Sensor::Register() { } /*static*/ void Sensor::resetAllMocks() { - constexpr size_t len = sizeof(s_sensorRegistry) / sizeof(s_sensorRegistry[0]); - // Reset all mocks - for (size_t i = 0; i < len; i++) { + for (size_t i = 0; i < efi::size(s_sensorRegistry); i++) { auto &entry = s_sensorRegistry[i]; entry.useMock = false; } } + +/*static*/ const char* Sensor::getSensorName(SensorType type) { + return s_sensorNames[static_cast(type)]; +} + +// Print information about all sensors +/*static*/ void Sensor::showAllSensorInfo(Logging* logger) { + for (size_t i = 1; i < efi::size(s_sensorRegistry); i++) { + auto& entry = s_sensorRegistry[i]; + const char* name = s_sensorNames[i]; + + if (entry.useMock) { + scheduleMsg(logger, "Sensor \"%s\" mocked with value %.2f", name, entry.mockValue); + } else { + const auto sensor = entry.sensor; + + if (sensor) { + sensor->showInfo(logger, name); + } else { + scheduleMsg(logger, "Sensor \"%s\" is not configured.", name); + } + } + } +} diff --git a/firmware/controllers/sensors/sensor.h b/firmware/controllers/sensors/sensor.h index 63c2da2a42..cda198a23f 100644 --- a/firmware/controllers/sensors/sensor.h +++ b/firmware/controllers/sensors/sensor.h @@ -68,6 +68,7 @@ struct SensorResult { // Fwd declare - nobody outside of Sensor.cpp needs to see inside this type struct SensorRegistryEntry; +class Logging; class Sensor { public: @@ -78,6 +79,12 @@ public: // done internally! [[nodiscard]] bool Register(); + // Print information about this sensor + virtual void showInfo(Logging* logger, const char* sensorName) const = 0; + + // Print information about all sensors + static void showAllSensorInfo(Logging* logger); + // Remove all sensors from the sensor registry - tread carefully if you use this outside of a unit test static void resetRegistry(); @@ -121,6 +128,8 @@ protected: explicit Sensor(SensorType type) : m_type(type) {} + static const char* getSensorName(SensorType type); + private: // Retrieve the current reading from the sensor. // diff --git a/firmware/controllers/sensors/sensor_info_printing.cpp b/firmware/controllers/sensors/sensor_info_printing.cpp new file mode 100644 index 0000000000..40e302ad10 --- /dev/null +++ b/firmware/controllers/sensors/sensor_info_printing.cpp @@ -0,0 +1,13 @@ +#include "proxy_sensor.h" +#include "functional_sensor.h" +#include "efilib.h" +#include "loggingcentral.h" + +void ProxySensor::showInfo(Logging* logger, const char* sensorName) const { + scheduleMsg(logger, "Sensor \"%s\" proxied from sensor \"%s\"", sensorName, getSensorName(m_proxiedSensor)); +} + +void FunctionalSensor::showInfo(Logging* logger, const char* sensorName) const { + const auto [valid, value] = get(); + scheduleMsg(logger, "Sensor \"%s\": Raw value: %.2f Valid: %d Converted value %.2f", sensorName, m_rawValue, valid, value); +} diff --git a/firmware/controllers/sensors/sensors.mk b/firmware/controllers/sensors/sensors.mk index bd61f4bc00..4b07c2e123 100644 --- a/firmware/controllers/sensors/sensors.mk +++ b/firmware/controllers/sensors/sensors.mk @@ -11,6 +11,7 @@ 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/sensor_info_printing.cpp \ $(PROJECT_DIR)/controllers/sensors/functional_sensor.cpp \ $(PROJECT_DIR)/controllers/sensors/converters/linear_func.cpp \ $(PROJECT_DIR)/controllers/sensors/converters/resistance_func.cpp \ diff --git a/firmware/init/init.h b/firmware/init/init.h index ec8ae1bdfd..4de1de25bf 100644 --- a/firmware/init/init.h +++ b/firmware/init/init.h @@ -4,8 +4,10 @@ #pragma once +class Logging; + // Call this once at startup to initialize, configure, and subscribe sensors -void initSensors(); +void initNewSensors(Logging* logger); // Call this whenever the configuration may have changed, so any sensors // can be reconfigured with the new settings. diff --git a/firmware/init/sensor/init_sensors.cpp b/firmware/init/sensor/init_sensors.cpp index 10c4e15d89..3b8f91f3fa 100644 --- a/firmware/init/sensor/init_sensors.cpp +++ b/firmware/init/sensor/init_sensors.cpp @@ -6,18 +6,18 @@ #include "init.h" #include "sensor.h" -static void initSensorCli(); +static void initSensorCli(Logging* logger); // Sensor init/config void initTps(); void initOilPressure(); -void initSensors() { +void initNewSensors(Logging* logger) { initTps(); initOilPressure(); // Init CLI functionality for sensors (mocking) - initSensorCli(); + initSensorCli(logger); } // Sensor reconfiguration @@ -29,8 +29,19 @@ void reconfigureSensors() { reconfigureOilPressure(); } +static Logging* s_logger; + // Mocking/testing helpers -static void initSensorCli() { +static void initSensorCli(Logging* logger) { + s_logger = logger; + addConsoleActionIF("set_sensor_mock", Sensor::setMockValue); addConsoleAction("reset_sensor_mocks", Sensor::resetAllMocks); + addConsoleAction("show_sensors", []() { Sensor::showAllSensorInfo(s_logger); }); + addConsoleActionI("show_sensor", + [](int idx) { + if (auto s = Sensor::getSensorOfType(static_cast(idx))) { + s->showAllSensorInfo(s_logger); + } + }); } diff --git a/firmware/util/efilib.h b/firmware/util/efilib.h index fa620373ba..64ec350fb3 100644 --- a/firmware/util/efilib.h +++ b/firmware/util/efilib.h @@ -45,19 +45,6 @@ int indexOf(const char *string, char ch); float atoff(const char *string); int atoi(const char *string); -#if defined(__cplusplus) && defined(__OPTIMIZE__) -#include -// "g++ -O2" version, adds more strict type check and yet no "strict-aliasing" warnings! -#define cisnan(f) ({ \ - static_assert(sizeof(f) == sizeof(int32_t)); \ - union cisnanu_t { std::remove_reference_t __f; int32_t __i; } __cisnan_u = { f }; \ - __cisnan_u.__i == 0x7FC00000; \ -}) -#else -// "g++ -O0" or other C++/C compilers -#define cisnan(f) (*(((int*) (&f))) == 0x7FC00000) -#endif /* __cplusplus && __OPTIMIZE__ */ - #define UNUSED(x) (void)(x) int absI(int32_t value); @@ -124,4 +111,17 @@ constexpr void copyArrayPartial(TElement (&dest)[NDest], const TElement (&src)[N #endif /* __cplusplus */ +#if defined(__cplusplus) && defined(__OPTIMIZE__) +#include +// "g++ -O2" version, adds more strict type check and yet no "strict-aliasing" warnings! +#define cisnan(f) ({ \ + static_assert(sizeof(f) == sizeof(int32_t)); \ + union cisnanu_t { std::remove_reference_t __f; int32_t __i; } __cisnan_u = { f }; \ + __cisnan_u.__i == 0x7FC00000; \ +}) +#else +// "g++ -O0" or other C++/C compilers +#define cisnan(f) (*(((int*) (&f))) == 0x7FC00000) +#endif /* __cplusplus && __OPTIMIZE__ */ + #endif /* EFILIB_H_ */ diff --git a/firmware/util/loggingcentral.h b/firmware/util/loggingcentral.h index 8a99f7c99d..b3797a3acf 100644 --- a/firmware/util/loggingcentral.h +++ b/firmware/util/loggingcentral.h @@ -7,6 +7,8 @@ #ifndef UTIL_LOGGINGCENTRAL_H_ #define UTIL_LOGGINGCENTRAL_H_ +class Logging; + void initLoggingCentral(void); char * swapOutputBuffers(int *actualOutputBufferSize); void scheduleMsg(Logging *logging, const char *fmt, ...); diff --git a/unit_tests/tests/sensor/mock/mock_sensor.h b/unit_tests/tests/sensor/mock/mock_sensor.h index 72177d30c3..c037194b00 100644 --- a/unit_tests/tests/sensor/mock/mock_sensor.h +++ b/unit_tests/tests/sensor/mock/mock_sensor.h @@ -18,4 +18,6 @@ struct MockSensor final : public StoredValueSensor { StoredValueSensor::invalidate(); } + + void showInfo(Logging* logger, const char* name) const override {} };