From a285ba67435cae30a8afca01512baa20bd65c3e7 Mon Sep 17 00:00:00 2001 From: Andrey G Date: Sun, 22 Nov 2020 20:23:27 +0300 Subject: [PATCH] ADC: fixes: (#1979) * ADC: fixes: -use enums -internalAdcIndexByHardwareIndex array should be adc_channel_e size -add sanity checks -save few bytes of RAM * ADC: use logical OR instead of math add --- .../boards/hellen/cypress/rusefi_hw_enums.h | 3 ++ .../config/boards/kinetis/rusefi_hw_enums.h | 3 ++ firmware/controllers/algo/rusefi_hw_enums.h | 3 ++ firmware/hw_layer/adc/AdcConfiguration.h | 10 ++++--- firmware/hw_layer/adc/adc_inputs.cpp | 30 ++++++++++++------- 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/firmware/config/boards/hellen/cypress/rusefi_hw_enums.h b/firmware/config/boards/hellen/cypress/rusefi_hw_enums.h index cb0c0d5cf8..eb0e62fb3b 100644 --- a/firmware/config/boards/hellen/cypress/rusefi_hw_enums.h +++ b/firmware/config/boards/hellen/cypress/rusefi_hw_enums.h @@ -289,3 +289,6 @@ typedef enum __attribute__ ((__packed__)) { // todo: bad choice of value since now we have ADC_CHANNEL_SENSOR and could end up with 17 and 18 also EFI_ADC_ERROR = 33, } adc_channel_e; + +/* Plase keep updating this define */ +#define EFI_ADC_LAST EFI_ADC_31 diff --git a/firmware/config/boards/kinetis/rusefi_hw_enums.h b/firmware/config/boards/kinetis/rusefi_hw_enums.h index c64fa15e07..5406c184f2 100644 --- a/firmware/config/boards/kinetis/rusefi_hw_enums.h +++ b/firmware/config/boards/kinetis/rusefi_hw_enums.h @@ -180,3 +180,6 @@ typedef enum __attribute__ ((__packed__)) { // todo: bad choice of value since now we have ADC_CHANNEL_SENSOR and could end up with 17 and 18 also EFI_ADC_ERROR = 17, } adc_channel_e; + +/* Plase keep updating this define */ +#define EFI_ADC_LAST EFI_ADC_15 diff --git a/firmware/controllers/algo/rusefi_hw_enums.h b/firmware/controllers/algo/rusefi_hw_enums.h index f00ffd8692..e27917c1ff 100644 --- a/firmware/controllers/algo/rusefi_hw_enums.h +++ b/firmware/controllers/algo/rusefi_hw_enums.h @@ -272,3 +272,6 @@ typedef enum __attribute__ ((__packed__)) { EFI_ADC_ERROR = 50, } adc_channel_e; + +/* Plase keep updating this define */ +#define EFI_ADC_LAST EFI_ADC_TEMP_SENSOR diff --git a/firmware/hw_layer/adc/AdcConfiguration.h b/firmware/hw_layer/adc/AdcConfiguration.h index 921d1c3bd2..d3dbba01f5 100644 --- a/firmware/hw_layer/adc/AdcConfiguration.h +++ b/firmware/hw_layer/adc/AdcConfiguration.h @@ -24,7 +24,8 @@ public: void enableChannel(adc_channel_e hwChannelIndex); void enableChannelAndPin(const char *msg, adc_channel_e hwChannelIndex); adc_channel_e getAdcHardwareIndexByInternalIndex(int index) const; - int internalAdcIndexByHardwareIndex[ADC_MAX_CHANNELS_COUNT + 4]; + /* We use adc_channel_e as index for following array */ + char internalAdcIndexByHardwareIndex[EFI_ADC_LAST + 1]; bool isHwUsed(adc_channel_e hwChannel) const; int size() const; void init(void); @@ -35,16 +36,17 @@ public: adcsample_t *samples; - int getAdcValueByHwChannel(int hwChannel) const; + int getAdcValueByHwChannel(adc_channel_e hwChannel) const; adc_state values; - int channelCount; private: ADCConversionGroup* hwConfig; /** * Number of ADC channels in use */ - + unsigned char channelCount; + + /* STM32 has up-to 4 additional channels routed to internal voltage sources */ adc_channel_e hardwareIndexByIndernalAdcIndex[ADC_MAX_CHANNELS_COUNT + 4]; }; diff --git a/firmware/hw_layer/adc/adc_inputs.cpp b/firmware/hw_layer/adc/adc_inputs.cpp index 9e8a827b63..00aa05c14c 100644 --- a/firmware/hw_layer/adc/adc_inputs.cpp +++ b/firmware/hw_layer/adc/adc_inputs.cpp @@ -76,7 +76,7 @@ AdcDevice::AdcDevice(ADCConversionGroup* hwConfig, adcsample_t *buf) { hwConfig->sqr5 = 0; #endif /* ADC_MAX_CHANNELS_COUNT */ memset(hardwareIndexByIndernalAdcIndex, EFI_ADC_NONE, sizeof(hardwareIndexByIndernalAdcIndex)); - memset(internalAdcIndexByHardwareIndex, 0xFFFFFFFF, sizeof(internalAdcIndexByHardwareIndex)); + memset(internalAdcIndexByHardwareIndex, EFI_ADC_ERROR, sizeof(internalAdcIndexByHardwareIndex)); } #if !defined(GPT_FREQ_FAST) || !defined(GPT_PERIOD_FAST) @@ -315,13 +315,21 @@ int AdcDevice::size() const { return channelCount; } -int AdcDevice::getAdcValueByHwChannel(int hwChannel) const { - int internalIndex = internalAdcIndexByHardwareIndex[hwChannel]; +int AdcDevice::getAdcValueByIndex(int internalIndex) const { + if (internalIndex >= size()) { + firmwareError(OBD_PCM_Processor_Fault, "ADC channel index out of range"); + return 0; + } return values.adc_data[internalIndex]; } -int AdcDevice::getAdcValueByIndex(int internalIndex) const { - return values.adc_data[internalIndex]; +int AdcDevice::getAdcValueByHwChannel(adc_channel_e hwChannel) const { + if (hwChannel >= ARRAY_SIZE(internalAdcIndexByHardwareIndex)) { + firmwareError(OBD_PCM_Processor_Fault, "ADC hwChannel out of range"); + return 0; + } + int internalIndex = internalAdcIndexByHardwareIndex[hwChannel]; + return getAdcValueByIndex(internalIndex); } void AdcDevice::invalidateSamplesCache() { @@ -351,7 +359,7 @@ bool AdcDevice::isHwUsed(adc_channel_e hwChannelIndex) const { } void AdcDevice::enableChannel(adc_channel_e hwChannel) { - if (channelCount >= efi::size(values.adc_data)) { + if (channelCount >= ADC_MAX_CHANNELS_COUNT) { firmwareError(OBD_PCM_Processor_Fault, "Too many ADC channels configured"); return; } @@ -363,18 +371,18 @@ void AdcDevice::enableChannel(adc_channel_e hwChannel) { internalAdcIndexByHardwareIndex[hwChannel] = logicChannel; hardwareIndexByIndernalAdcIndex[logicChannel] = hwChannel; if (logicChannel < 6) { - hwConfig->sqr3 += (channelAdcIndex) << (5 * logicChannel); + hwConfig->sqr3 |= channelAdcIndex << (5 * logicChannel); } else if (logicChannel < 12) { - hwConfig->sqr2 += (channelAdcIndex) << (5 * (logicChannel - 6)); + hwConfig->sqr2 |= channelAdcIndex << (5 * (logicChannel - 6)); } else if (logicChannel < 18) { - hwConfig->sqr1 += (channelAdcIndex) << (5 * (logicChannel - 12)); + hwConfig->sqr1 |= channelAdcIndex << (5 * (logicChannel - 12)); } #if ADC_MAX_CHANNELS_COUNT > 16 else if (logicChannel < 24) { - hwConfig->sqr4 += (channelAdcIndex) << (5 * (logicChannel - 18)); + hwConfig->sqr4 |= channelAdcIndex << (5 * (logicChannel - 18)); } else if (logicChannel < 30) { - hwConfig->sqr5 += (channelAdcIndex) << (5 * (logicChannel - 24)); + hwConfig->sqr5 |= channelAdcIndex << (5 * (logicChannel - 24)); } #endif /* ADC_MAX_CHANNELS_COUNT */ }