Adc isAdcChannelValid helper (#2188)

* Guard define argument

* ADC: isAdcChannelValid

Check for both <= EFI_ADC_NONE and >= EFI_ADC_LAST_CHANNEL
Also check for value out of enum range (corrupted settings)

* Fix unit tests
This commit is contained in:
Andrey G 2021-01-06 00:02:20 +03:00 committed by GitHub
parent 0abfc7c9c1
commit b92e3086d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 77 additions and 47 deletions

View File

@ -772,7 +772,7 @@ void updateTunerStudioState(TunerStudioOutputChannels *tsOutputChannels DECLARE_
break;
case DBG_FSIO_ADC:
// todo: implement a proper loop
if (engineConfiguration->fsioAdc[0] != EFI_ADC_NONE) {
if (isAdcChannelValid(engineConfiguration->fsioAdc[0])) {
tsOutputChannels->debugFloatField1 = getVoltage("fsio", engineConfiguration->fsioAdc[0] PASS_ENGINE_PARAMETER_SUFFIX);
}
break;
@ -829,13 +829,13 @@ void updateTunerStudioState(TunerStudioOutputChannels *tsOutputChannels DECLARE_
break;
#endif /* EFI_CAN_SUPPORT */
case DBG_ANALOG_INPUTS:
tsOutputChannels->debugFloatField1 = (engineConfiguration->vbattAdcChannel != EFI_ADC_NONE) ? getVoltageDivided("vbatt", engineConfiguration->vbattAdcChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField1 = isAdcChannelValid(engineConfiguration->vbattAdcChannel) ? getVoltageDivided("vbatt", engineConfiguration->vbattAdcChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField2 = Sensor::getRaw(SensorType::Tps1);
tsOutputChannels->debugFloatField3 = (engineConfiguration->mafAdcChannel != EFI_ADC_NONE) ? getVoltageDivided("maf", engineConfiguration->mafAdcChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField4 = (engineConfiguration->map.sensor.hwChannel != EFI_ADC_NONE) ? getVoltageDivided("map", engineConfiguration->map.sensor.hwChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField5 = (engineConfiguration->clt.adcChannel != EFI_ADC_NONE) ? getVoltageDivided("clt", engineConfiguration->clt.adcChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField6 = (engineConfiguration->iat.adcChannel != EFI_ADC_NONE) ? getVoltageDivided("iat", engineConfiguration->iat.adcChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField7 = (engineConfiguration->afr.hwChannel != EFI_ADC_NONE) ? getVoltageDivided("ego", engineConfiguration->afr.hwChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField3 = isAdcChannelValid(engineConfiguration->mafAdcChannel) ? getVoltageDivided("maf", engineConfiguration->mafAdcChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField4 = isAdcChannelValid(engineConfiguration->map.sensor.hwChannel) ? getVoltageDivided("map", engineConfiguration->map.sensor.hwChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField5 = isAdcChannelValid(engineConfiguration->clt.adcChannel) ? getVoltageDivided("clt", engineConfiguration->clt.adcChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField6 = isAdcChannelValid(engineConfiguration->iat.adcChannel) ? getVoltageDivided("iat", engineConfiguration->iat.adcChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
tsOutputChannels->debugFloatField7 = isAdcChannelValid(engineConfiguration->afr.hwChannel) ? getVoltageDivided("ego", engineConfiguration->afr.hwChannel PASS_ENGINE_PARAMETER_SUFFIX) : 0.0f;
break;
case DBG_ANALOG_INPUTS2:
// TPS 1 pri/sec split

View File

@ -217,7 +217,7 @@ void Engine::periodicSlowCallback(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
slowCallBackWasInvoked = true;
#if ANALOG_HW_CHECK_MODE
efiAssertVoid(OBD_PCM_Processor_Fault, CONFIG(clt).adcChannel != EFI_ADC_NONE, "No CLT setting");
efiAssertVoid(OBD_PCM_Processor_Fault, isAdcChannelValid(CONFIG(clt).adcChannel), "No CLT setting");
efitimesec_t secondsNow = getTimeNowSeconds();
if (secondsNow > 2 && secondsNow < 180) {
assertCloseTo("RPM", Sensor::get(SensorType::Rpm).Value, HW_CHECK_RPM);
@ -256,7 +256,7 @@ void Engine::updateSlowSensors(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
engineState.updateSlowSensors(PASS_ENGINE_PARAMETER_SIGNATURE);
// todo: move this logic somewhere to sensors folder?
if (CONFIG(fuelLevelSensor) != EFI_ADC_NONE) {
if (isAdcChannelValid(CONFIG(fuelLevelSensor))) {
float fuelLevelVoltage = getVoltageDivided("fuel", engineConfiguration->fuelLevelSensor PASS_ENGINE_PARAMETER_SUFFIX);
sensors.fuelTankLevel = interpolateMsg("fgauge", CONFIG(fuelLevelEmptyTankVoltage), 0,
CONFIG(fuelLevelFullTankVoltage), 100,

View File

@ -280,7 +280,7 @@ void initPeriodicEvents(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
char * getPinNameByAdcChannel(const char *msg, adc_channel_e hwChannel, char *buffer) {
#if HAL_USE_ADC
if (hwChannel == EFI_ADC_NONE) {
if (!isAdcChannelValid(hwChannel)) {
strcpy(buffer, "NONE");
} else {
strcpy(buffer, portname(getAdcChannelPort(msg, hwChannel)));
@ -301,7 +301,7 @@ extern AdcDevice fastAdc;
static void printAnalogChannelInfoExt(const char *name, adc_channel_e hwChannel, float adcVoltage,
float dividerCoeff) {
#if HAL_USE_ADC
if (hwChannel == EFI_ADC_NONE) {
if (!isAdcChannelValid(hwChannel)) {
scheduleMsg(&logger, "ADC is not assigned for %s", name);
return;
}
@ -687,7 +687,7 @@ void initEngineContoller(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX)
initEgoAveraging(PASS_ENGINE_PARAMETER_SIGNATURE);
if (engineConfiguration->externalKnockSenseAdc != EFI_ADC_NONE) {
if (isAdcChannelValid(engineConfiguration->externalKnockSenseAdc)) {
addConsoleAction("knockinfo", getKnockInfo);
}

View File

@ -450,7 +450,7 @@ void mainTriggerCallback(uint32_t trgEventIndex, efitick_t edgeTimestamp DECLARE
}
if (trgEventIndex == (uint32_t)CONFIG(ignMathCalculateAtIndex)) {
if (CONFIG(externalKnockSenseAdc) != EFI_ADC_NONE) {
if (isAdcChannelValid(CONFIG(externalKnockSenseAdc))) {
float externalKnockValue = getVoltageDivided("knock", engineConfiguration->externalKnockSenseAdc PASS_ENGINE_PARAMETER_SUFFIX);
engine->knockLogic(externalKnockValue PASS_ENGINE_PARAMETER_SUFFIX);
}

View File

@ -105,7 +105,7 @@ bool hasAfrSensor(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
return cjHasAfrSensor(PASS_ENGINE_PARAMETER_SIGNATURE);
}
#endif /* EFI_CJ125 && HAL_USE_SPI */
return engineConfiguration->afr.hwChannel != EFI_ADC_NONE;
return isAdcChannelValid(engineConfiguration->afr.hwChannel);
}
extern float InnovateLC2AFR;
@ -123,7 +123,7 @@ float getAfr(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
#endif /* EFI_CJ125 && HAL_USE_SPI */
afr_sensor_s * sensor = &CONFIG(afr);
if (engineConfiguration->afr.hwChannel == EFI_ADC_NONE) {
if (!isAdcChannelValid(engineConfiguration->afr.hwChannel)) {
return 0;
}

View File

@ -13,7 +13,7 @@ float getMafVoltage(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
}
bool hasMafSensor(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
return engineConfiguration->mafAdcChannel != EFI_ADC_NONE;
return isAdcChannelValid(engineConfiguration->mafAdcChannel);
}
/**

View File

@ -172,11 +172,11 @@ float getRawMap(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
* Also if 'useFixedBaroCorrFromMap' option is enabled, and we have the initial pressure value stored and passed validation.
*/
bool hasBaroSensor(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
return engineConfiguration->baroSensor.hwChannel != EFI_ADC_NONE || !cisnan(storedInitialBaroPressure);
return isAdcChannelValid(engineConfiguration->baroSensor.hwChannel) || !cisnan(storedInitialBaroPressure);
}
bool hasMapSensor(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
return engineConfiguration->map.sensor.hwChannel != EFI_ADC_NONE;
return isAdcChannelValid(engineConfiguration->map.sensor.hwChannel);
}
float getBaroPressure(DECLARE_ENGINE_PARAMETER_SIGNATURE) {

View File

@ -15,7 +15,7 @@
EXTERN_ENGINE;
bool hasVBatt(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
return engineConfiguration->vbattAdcChannel != EFI_ADC_NONE;
return isAdcChannelValid(engineConfiguration->vbattAdcChannel);
}
float getVBatt(DECLARE_ENGINE_PARAMETER_SIGNATURE) {

View File

@ -12,6 +12,5 @@
#include "mcp3208.h"
#define getAdcValue(channel) getMcp3208adc(channel)
#define adcToVoltsDivided(adc) (5.0f / 4095 * adc)
#define getVoltageDivided(msg, channel) (channel == EFI_ADC_NONE ? 66.66 : adcToVoltsDivided(getAdcValue(msg, channel)))
#define adcToVoltsDivided(adc) (5.0f / 4095 * (adc))
#define getVoltageDivided(msg, channel) (isAdcChannelValid(channel) ? adcToVoltsDivided(getAdcValue(msg, channel)) : 66.66)

View File

@ -268,7 +268,7 @@ float getMCUInternalTemperature() {
}
int getInternalAdcValue(const char *msg, adc_channel_e hwChannel) {
if (hwChannel == EFI_ADC_NONE) {
if (!isAdcChannelValid(hwChannel)) {
warning(CUSTOM_OBD_ANALOG_INPUT_NOT_CONFIGURED, "ADC: %s input is not configured", msg);
return -1;
}
@ -410,7 +410,7 @@ static void printFullAdcReport(Logging *logger) {
adc_channel_e hwIndex = fastAdc.getAdcHardwareIndexByInternalIndex(index);
if (hwIndex != EFI_ADC_NONE && hwIndex != EFI_ADC_ERROR) {
if (isAdcChannelValid(hwIndex)) {
ioportid_t port = getAdcChannelPort("print", hwIndex);
int pin = getAdcChannelPin(hwIndex);
@ -430,7 +430,7 @@ static void printFullAdcReport(Logging *logger) {
adc_channel_e hwIndex = slowAdc.getAdcHardwareIndexByInternalIndex(index);
if (hwIndex != EFI_ADC_NONE && hwIndex != EFI_ADC_ERROR) {
if (isAdcChannelValid(hwIndex)) {
ioportid_t port = getAdcChannelPort("print", hwIndex);
int pin = getAdcChannelPin(hwIndex);
@ -516,7 +516,7 @@ public:
};
void addChannel(const char *name, adc_channel_e setting, adc_channel_mode_e mode) {
if (setting == EFI_ADC_NONE) {
if (!isAdcChannelValid(setting)) {
return;
}
if (/*type-limited (int)setting < 0 || */(int)setting>=HW_MAX_ADC_INDEX) {
@ -532,7 +532,7 @@ void addChannel(const char *name, adc_channel_e setting, adc_channel_mode_e mode
void removeChannel(const char *name, adc_channel_e setting) {
(void)name;
if (setting == EFI_ADC_NONE) {
if (!isAdcChannelValid(setting)) {
return;
}
adcHwChannelEnabled[setting] = ADC_OFF;

View File

@ -13,6 +13,21 @@
#define SLOW_ADC_RATE 500
static inline bool isAdcChannelValid(adc_channel_e hwChannel) {
if (hwChannel <= EFI_ADC_NONE) {
return false;
} else if (hwChannel >= EFI_ADC_LAST_CHANNEL) {
/* this should not happen!
* if we have enum out of range somewhere in settings
* that means something goes terribly wrong
* TODO: should we say something?
*/
return false;
} else {
return true;
}
}
#if HAL_USE_ADC
adc_channel_mode_e getAdcMode(adc_channel_e hwChannel);

View File

@ -36,7 +36,7 @@ void AdcSubscription::SubscribeSensor(FunctionalSensor &sensor,
float lowpassCutoff,
float voltsPerAdcVolt /*= 0.0f*/) {
// Don't subscribe null channels
if (channel == EFI_ADC_NONE) {
if (!isAdcChannelValid(channel)) {
return;
}

View File

@ -250,17 +250,15 @@ static void calcFastAdcIndexes(void) {
#if HAL_USE_ADC
fastMapSampleIndex = fastAdc.internalAdcIndexByHardwareIndex[engineConfiguration->map.sensor.hwChannel];
hipSampleIndex =
engineConfiguration->hipOutputChannel == EFI_ADC_NONE ?
-1 : fastAdc.internalAdcIndexByHardwareIndex[engineConfiguration->hipOutputChannel];
if (engineConfiguration->tps1_1AdcChannel != EFI_ADC_NONE) {
tpsSampleIndex = fastAdc.internalAdcIndexByHardwareIndex[engineConfiguration->tps1_1AdcChannel];
} else {
tpsSampleIndex = TPS_IS_SLOW;
}
isAdcChannelValid(engineConfiguration->hipOutputChannel) ?
fastAdc.internalAdcIndexByHardwareIndex[engineConfiguration->hipOutputChannel] : -1;
tpsSampleIndex =
isAdcChannelValid(engineConfiguration->tps1_1AdcChannel) ?
fastAdc.internalAdcIndexByHardwareIndex[engineConfiguration->tps1_1AdcChannel] : TPS_IS_SLOW;
#if HAL_TRIGGER_USE_ADC
adc_channel_e triggerChannel = getAdcChannelForTrigger();
triggerSampleIndex = (triggerChannel == EFI_ADC_NONE) ?
-1 : fastAdc.internalAdcIndexByHardwareIndex[triggerChannel];
triggerSampleIndex = isAdcChannelValid(triggerChannel) ?
fastAdc.internalAdcIndexByHardwareIndex[triggerChannel] : -1;
#endif /* HAL_TRIGGER_USE_ADC */
#endif/* HAL_USE_ADC */

View File

@ -127,7 +127,7 @@ static void cjWriteRegister(uint8_t regAddr, uint8_t regValue) {
static float getUr() {
#if ! EFI_UNIT_TEST
if (CONFIG(cj125ur) != EFI_ADC_NONE) {
if (isAdcChannelValid(CONFIG(cj125ur))) {
#if EFI_PROD_CODE
if (!engineConfiguration->cj125isUrDivided) {
// in case of directly connected Ur signal from CJ125 to the ADC pin of MCU
@ -146,7 +146,7 @@ static float getUr() {
static float getUa() {
#if ! EFI_UNIT_TEST
if (CONFIG(cj125ua) != EFI_ADC_NONE) {
if (isAdcChannelValid(CONFIG(cj125ua))) {
#if EFI_PROD_CODE
if (engineConfiguration->cj125isUaDivided) {
return getVoltageDivided("cj125ua", CONFIG(cj125ua) PASS_ENGINE_PARAMETER_SUFFIX);
@ -618,7 +618,7 @@ void initCJ125(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) {
return;
}
if (CONFIG(cj125ur) == EFI_ADC_NONE || CONFIG(cj125ua) == EFI_ADC_NONE) {
if (!isAdcChannelValid(CONFIG(cj125ur)) || !isAdcChannelValid(CONFIG(cj125ua))) {
scheduleMsg(logger, "cj125 init error! cj125ur and cj125ua are required.");
warning(CUSTOM_CJ125_0, "cj ur ua");
globalInstance.errorCode = CJ125_ERROR_DISABLED;

View File

@ -299,7 +299,7 @@ adc_channel_e getAdcChannelForTrigger(void) {
void addAdcChannelForTrigger(void) {
adc_channel_e ch = getAdcChannelForTrigger();
if (ch != EFI_ADC_NONE) {
if (isAdcChannelValid(ch)) {
addChannel("TRIG", ch, ADC_FAST);
}
}

View File

@ -1,4 +1,5 @@
#include "init.h"
#include "adc_inputs.h"
#include "adc_subscription.h"
#include "engine.h"
#include "error_handling.h"
@ -39,7 +40,7 @@ static void initFluidPressure(LinearFunc& func, FunctionalSensor& sensor, const
auto channel = cfg.hwChannel;
// Only register if we have a sensor
if (channel == EFI_ADC_NONE) {
if (!isAdcChannelValid(channel)) {
return;
}

View File

@ -1,7 +1,8 @@
#include "global.h"
#include "adc_inputs.h"
#include "adc_subscription.h"
#include "engine.h"
#include "error_handling.h"
#include "global.h"
#include "functional_sensor.h"
#include "func_chain.h"
#include "linear_func.h"
@ -54,7 +55,7 @@ static void configureTempSensor(FunctionalSensor &sensor,
auto channel = config.adcChannel;
// Only register if we have a sensor
if (channel == EFI_ADC_NONE) {
if (!isAdcChannelValid(channel)) {
return;
}

View File

@ -1,7 +1,8 @@
#include "global.h"
#include "adc_inputs.h"
#include "adc_subscription.h"
#include "engine.h"
#include "error_handling.h"
#include "global.h"
#include "functional_sensor.h"
#include "redundant_sensor.h"
#include "proxy_sensor.h"
@ -41,7 +42,7 @@ FunctionalSensor idlePosSens(SensorType::IdlePosition, MS2NT(10));
static bool configureTps(LinearFunc& func, adc_channel_e channel, float closed, float open, float min, float max, const char* msg) {
// Only configure if we have a channel
if (channel == EFI_ADC_NONE || channel >= EFI_ADC_LAST_CHANNEL) {
if (!isAdcChannelValid(channel)) {
return false;
}
@ -106,7 +107,7 @@ void initTps(DECLARE_CONFIG_PARAMETER_SIGNATURE) {
}
// Route the pedal or TPS to driverIntent as appropriate
if (CONFIG(throttlePedalPositionAdcChannel) != EFI_ADC_NONE) {
if (isAdcChannelValid(CONFIG(throttlePedalPositionAdcChannel))) {
driverIntent.setProxiedSensor(SensorType::AcceleratorPedal);
} else {
driverIntent.setProxiedSensor(SensorType::Tps1);

View File

@ -7,4 +7,19 @@
#pragma once
static inline bool isAdcChannelValid(adc_channel_e hwChannel) {
if (hwChannel <= EFI_ADC_NONE) {
return false;
} else if (hwChannel >= EFI_ADC_LAST_CHANNEL) {
/* this should not happen!
* if we have enum out of range somewhere in settings
* that means something goes terribly wrong
* TODO: should we say something?
*/
return false;
} else {
return true;
}
}
#include "boards.h"