Skip to content

Commit

Permalink
MISRA fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@adbancroft
adbancroft committed Dec 14, 2024
1 parent b89b3e9 commit 719ca0b
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 27 deletions.
32 changes: 16 additions & 16 deletions speeduino/corrections.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,27 @@ There are 2 top level functions that call more detailed corrections for Fuel and

#define IGN_IDLE_THRESHOLD 200 //RPM threshold (below CL idle target) for when ign based idle control will engage

long PID_O2, PID_output, PID_AFRTarget;
static long PID_O2, PID_output, PID_AFRTarget;
/** Instance of the PID object in case that algorithm is used (Always instantiated).
* Needs to be global as it maintains state outside of each function call.
* Comes from Arduino (?) PID library.
*/
PID egoPID(&PID_O2, &PID_output, &PID_AFRTarget, configPage6.egoKP, configPage6.egoKI, configPage6.egoKD, REVERSE);
static PID egoPID(&PID_O2, &PID_output, &PID_AFRTarget, configPage6.egoKP, configPage6.egoKI, configPage6.egoKD, REVERSE);

byte activateMAPDOT; //The mapDOT value seen when the MAE was activated.
byte activateTPSDOT; //The tpsDOT value seen when the MAE was activated.
static byte activateMAPDOT; //The mapDOT value seen when the MAE was activated.
static byte activateTPSDOT; //The tpsDOT value seen when the MAE was activated.

bool idleAdvActive = false;
static bool idleAdvActive = false;
uint16_t AFRnextCycle;
unsigned long knockStartTime;
uint8_t knockLastRecoveryStep;
static unsigned long knockStartTime;
static uint8_t knockLastRecoveryStep;
//int16_t knockWindowMin; //The current minimum crank angle for a knock pulse to be valid
//int16_t knockWindowMax;//The current maximum crank angle for a knock pulse to be valid
uint8_t aseTaper;
uint8_t dfcoDelay;
uint8_t idleAdvTaper;
uint8_t crankingEnrichTaper;
uint8_t dfcoTaper;
static uint8_t aseTaper;
uint8_t dfcoDelay; // Non-static for unit testing
static uint8_t idleAdvTaper;
static uint8_t crankingEnrichTaper;
uint8_t dfcoTaper; // Non-static for unit testing

/** Initialise instances and vars related to corrections (at ECU boot-up).
*/
Expand Down Expand Up @@ -369,15 +369,15 @@ uint16_t correctionAccel(void)
//If CLT is less than taper min temp, apply full modifier on top of accelValue
if ( currentStatus.coolant <= (int)(configPage2.aeColdTaperMin - CALIBRATION_TEMPERATURE_OFFSET) )
{
uint16_t accelValue_uint = percentage(configPage2.aeColdPct, (uint16_t)accelValue);
uint16_t accelValue_uint = (uint16_t)percentage(configPage2.aeColdPct, (uint16_t)accelValue);
accelValue = (int16_t) accelValue_uint;
}
//If CLT is between taper min and max, taper the modifier value and apply it on top of accelValue
else
{
int16_t taperRange = (int16_t) configPage2.aeColdTaperMax - configPage2.aeColdTaperMin;
int16_t taperPercent = (int)((currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET - configPage2.aeColdTaperMin) * 100) / taperRange;
int16_t coldPct = (int16_t) 100 + percentage((uint8_t) (100U-taperPercent), (configPage2.aeColdPct-100U) );
int16_t coldPct = (int16_t) 100 + (int16_t)percentage((uint8_t) (100U-taperPercent), (configPage2.aeColdPct-100U) );
uint16_t accelValue_uint = (uint16_t) accelValue * coldPct / 100; //Potential overflow (if AE is large) without using uint16_t (percentage() may overflow)
accelValue = (int16_t) accelValue_uint;
}
Expand Down Expand Up @@ -441,8 +441,8 @@ uint16_t correctionAccel(void)
else
{
int16_t taperRange = (int16_t) configPage2.aeColdTaperMax - configPage2.aeColdTaperMin;
int16_t taperPercent = (int)((currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET - configPage2.aeColdTaperMin) * 100) / taperRange;
int16_t coldPct = (int16_t)100 + percentage((uint8_t) (100 - taperPercent), (uint16_t)(configPage2.aeColdPct-100) );
uint16_t taperPercent = (int)((currentStatus.coolant + CALIBRATION_TEMPERATURE_OFFSET - configPage2.aeColdTaperMin) * 100) / taperRange;
uint16_t coldPct = 100U + (uint16_t)percentage((uint8_t) (100U - taperPercent), configPage2.aeColdPct-100U);
uint16_t accelValue_uint = (uint16_t) accelValue * coldPct / 100; //Potential overflow (if AE is large) without using uint16_t
accelValue = (int16_t) accelValue_uint;
}
Expand Down
1 change: 0 additions & 1 deletion speeduino/corrections.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,5 @@ uint16_t correctionsDwell(uint16_t dwell);
bool isFixedTimingOn(void);

extern uint16_t AFRnextCycle;
extern uint8_t aseTaper;

#endif // CORRECTIONS_H
4 changes: 4 additions & 0 deletions speeduino/globals.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ struct config2 {
byte crkngAddCLTAdv : 1;
byte includeAFR : 1; //< Enable AFR compensation ? (See also @ref config2.incorporateAFR)
byte hardCutType : 1;
// cppcheck-suppress misra-c2012-6.1
LoadSource ignAlgorithm : 3;
byte indInjAng : 1;
byte injOpen; ///< Injector opening time (ms * 10)
Expand All @@ -533,6 +534,7 @@ struct config2 {
byte nCylinders : 4; ///< Number of cylinders

//config2 in ini
// cppcheck-suppress misra-c2012-6.1
LoadSource fuelAlgorithm : 3;///< Fuel algorithm - 0=Manifold pressure/MAP (LOAD_SOURCE_MAP, default, proven), 1=Throttle/TPS (LOAD_SOURCE_TPS), 2=IMAP/EMAP (LOAD_SOURCE_IMAPEMAP)
byte fixAngEnable : 1; ///< Whether fixed/locked timing is enabled (0=disable, 1=enable, See @ref configPage4.FixAng)
byte nInjectors : 4; ///< Number of injectors
Expand Down Expand Up @@ -1031,6 +1033,7 @@ struct config10 {
byte knock_recoveryStep; //Byte 121

//Byte 122
// cppcheck-suppress misra-c2012-6.1
LoadSource fuel2Algorithm : 3;
byte fuel2Mode : 3;
byte fuel2SwitchVariable : 2;
Expand Down Expand Up @@ -1104,6 +1107,7 @@ struct config10 {
byte fuelTempValues[6]; //180

//Byte 186
// cppcheck-suppress misra-c2012-6.1
LoadSource spark2Algorithm : 3;
byte spark2Mode : 3;
byte spark2SwitchVariable : 2;
Expand Down
20 changes: 10 additions & 10 deletions speeduino/secondaryTables.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
static inline uint8_t getVE2(void)
{
currentStatus.fuelLoad2 = getLoad(configPage10.fuel2Algorithm, currentStatus);
return get3DTableValue(&fuelTable2, currentStatus.fuelLoad2, currentStatus.RPM); //Perform lookup into fuel map for RPM vs MAP value
return get3DTableValue(&fuelTable2, currentStatus.fuelLoad2, (table3d_axis_t)currentStatus.RPM); //Perform lookup into fuel map for RPM vs MAP value
}

static inline bool fuelModeCondSwitchRpmActive(void) {
Expand Down Expand Up @@ -59,16 +59,16 @@ void calculateSecondaryFuel(void)
currentStatus.VE2 = getVE2();
BIT_SET(currentStatus.status3, BIT_STATUS3_FUEL2_ACTIVE); //Set the bit indicating that the 2nd fuel table is in use.
//Fuel 2 table is treated as a % value. Table 1 and 2 are multiplied together and divided by 100
uint16_t combinedVE = percentage(currentStatus.VE2, (uint16_t)currentStatus.VE1);
currentStatus.VE = min(MAX_VE, combinedVE);
uint16_t combinedVE = div100((uint16_t)currentStatus.VE2 * (uint16_t)currentStatus.VE1);
currentStatus.VE = (uint8_t)min(MAX_VE, combinedVE);
}
else if(configPage10.fuel2Mode == FUEL2_MODE_ADD)
{
currentStatus.VE2 = getVE2();
BIT_SET(currentStatus.status3, BIT_STATUS3_FUEL2_ACTIVE); //Set the bit indicating that the 2nd fuel table is in use.
//Fuel tables are added together, but a check is made to make sure this won't overflow the 8-bit VE value
uint16_t combinedVE = (uint16_t)currentStatus.VE1 + (uint16_t)currentStatus.VE2;
currentStatus.VE = min(MAX_VE, combinedVE);
currentStatus.VE = (uint8_t)min(MAX_VE, combinedVE);
}
else if(fuelModeCondSwitchActive() || fuelModeInputSwitchActive())
{
Expand All @@ -90,7 +90,7 @@ static constexpr int16_t MIN_ADVANCE = INT8_MIN; // out-of-line constant require

static inline uint8_t lookupSpark2(void) {
currentStatus.ignLoad2 = getLoad(configPage10.spark2Algorithm, currentStatus);
return get3DTableValue(&ignitionTable2, currentStatus.ignLoad2, currentStatus.RPM);
return get3DTableValue(&ignitionTable2, currentStatus.ignLoad2, (table3d_axis_t)currentStatus.RPM);
}

/**
Expand All @@ -100,9 +100,9 @@ static inline uint8_t lookupSpark2(void) {
*/
static inline int8_t getAdvance2(void)
{
int16_t advance2 = (int16_t)lookupSpark2() - INT16_C(OFFSET_IGNITION);
int16_t lookUpResult = (int16_t)lookupSpark2() - INT16_C(OFFSET_IGNITION);
// Clamp to return type range.
advance2 = constrain(advance2, MIN_ADVANCE, MAX_ADVANCE);
int8_t advance2 = (int8_t)constrain(lookUpResult, MIN_ADVANCE, MAX_ADVANCE);
#if !defined(UNIT_TEST)
//Perform the corrections calculation on the secondary advance value, only if it uses a switched mode
if( (configPage10.spark2SwitchVariable == SPARK2_MODE_CONDITIONAL_SWITCH) || (configPage10.spark2SwitchVariable == SPARK2_MODE_INPUT_SWITCH) ) {
Expand Down Expand Up @@ -156,9 +156,9 @@ void calculateSecondarySpark(void)
BIT_SET(currentStatus.status5, BIT_STATUS5_SPARK2_ACTIVE);
uint8_t spark2Percent = lookupSpark2();
//Spark 2 table is treated as a % value. Table 1 and 2 are multiplied together and divided by 100
int16_t combinedAdvance = div100((int16_t)((int16_t)spark2Percent * (int16_t)currentStatus.advance1));
int16_t combinedAdvance = div100((int16_t)spark2Percent * (int16_t)currentStatus.advance1);
//make sure we don't overflow and accidentally set negative timing: currentStatus.advance can only hold a signed 8 bit value
currentStatus.advance = (int8_t)min((int16_t)MAX_ADVANCE, combinedAdvance);
currentStatus.advance = (int8_t)min(MAX_ADVANCE, combinedAdvance);

// This is informational only, but the value needs corrected into the int8_t range
currentStatus.advance2 = (int8_t)max(MIN_ADVANCE, (int16_t)((int16_t)spark2Percent-INT16_C(OFFSET_IGNITION)));
Expand All @@ -169,7 +169,7 @@ void calculateSecondarySpark(void)
currentStatus.advance2 = getAdvance2();
//Spark tables are added together, but a check is made to make sure this won't overflow the 8-bit VE value
int16_t combinedAdvance = (int16_t)currentStatus.advance1 + (int16_t)currentStatus.advance2;
currentStatus.advance = min(MAX_ADVANCE, combinedAdvance);
currentStatus.advance = (int8_t)min(MAX_ADVANCE, combinedAdvance);
}
else if(sparkModeCondSwitchActive() || sparkModeInputSwitchActive())
{
Expand Down
8 changes: 8 additions & 0 deletions speeduino/statuses.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,22 @@ using byte = uint8_t;
* unit based values in similar variable(s) without ADC part in name (see sensors.ino for reading of sensors).
*/
struct statuses {
// cppcheck-suppress misra-c2012-6.1 ; False positive - MISRA C:2012 Rule (R 6.1) permits the use of boolean for bit fields.
volatile bool hasSync : 1; /**< Flag for crank/cam position being known by decoders (See decoders.ino).
This is used for sanity checking e.g. before logging tooth history or reading some sensors and computing readings. */
// cppcheck-suppress misra-c2012-6.1
bool initialisationComplete : 1; //Tracks whether the setup() function has run completely
// cppcheck-suppress misra-c2012-6.1
bool clutchTrigger : 1;
// cppcheck-suppress misra-c2012-6.1
bool previousClutchTrigger : 1;
// cppcheck-suppress misra-c2012-6.1
volatile bool fpPrimed : 1; //Tracks whether or not the fuel pump priming has been completed yet
// cppcheck-suppress misra-c2012-6.1
volatile bool injPrimed : 1; //Tracks whether or not the injector priming has been completed yet
// cppcheck-suppress misra-c2012-6.1
volatile bool tachoSweepEnabled : 1;
// cppcheck-suppress misra-c2012-6.1
volatile bool tachoAlt : 1;

uint16_t RPM; ///< RPM - Current Revs per minute
Expand Down

0 comments on commit 719ca0b

Please sign in to comment.