From 7d657106d741964b09b3a184380fb3ad6f1321d1 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Tue, 22 Mar 2022 13:53:24 -0700 Subject: [PATCH] warning cleanup (#4020) --- firmware/controllers/core/error_handling.h | 4 - firmware/controllers/math/engine_math.cpp | 99 ++++++++++--------- firmware/controllers/math/engine_math.h | 4 +- .../trigger/decoders/trigger_nissan.cpp | 2 +- .../controllers/trigger/trigger_decoder.cpp | 10 +- firmware/hw_layer/pin_repository.cpp | 2 +- firmware/util/efilib.h | 4 +- 7 files changed, 61 insertions(+), 64 deletions(-) diff --git a/firmware/controllers/core/error_handling.h b/firmware/controllers/core/error_handling.h index 3c65b19233..7fea3bb1c5 100644 --- a/firmware/controllers/core/error_handling.h +++ b/firmware/controllers/core/error_handling.h @@ -44,10 +44,6 @@ const char* getWarningMessage(void); // todo: better place for this shared declaration? int getRusEfiVersion(void); -/** - * @deprecated Global panic is inconvenient because it's hard to deliver the error message while whole instance - * is stopped. Please use firmwareWarning() instead - */ #if EFI_ENABLE_ASSERTS #define efiAssert(code, condition, message, result) { if (!(condition)) { firmwareError(code, message); return result; } } #define efiAssertVoid(code, condition, message) { if (!(condition)) { firmwareError(code, message); return; } } diff --git a/firmware/controllers/math/engine_math.cpp b/firmware/controllers/math/engine_math.cpp index 4cc2b3f58b..97c08433fb 100644 --- a/firmware/controllers/math/engine_math.cpp +++ b/firmware/controllers/math/engine_math.cpp @@ -107,58 +107,58 @@ floatms_t getSparkDwell(int rpm) { #endif } -static const int order_1[] = {1}; +static const uint8_t order_1[] = {1}; -static const int order_1_2[] = {1, 2}; +static const uint8_t order_1_2[] = {1, 2}; -static const int order_1_2_3[] = {1, 2, 3}; -static const int order_1_3_2[] = {1, 3, 2}; +static const uint8_t order_1_2_3[] = {1, 2, 3}; +static const uint8_t order_1_3_2[] = {1, 3, 2}; // 4 cylinder -static const int order_1_THEN_3_THEN_4_THEN2[] = { 1, 3, 4, 2 }; -static const int order_1_THEN_2_THEN_4_THEN3[] = { 1, 2, 4, 3 }; -static const int order_1_THEN_3_THEN_2_THEN4[] = { 1, 3, 2, 4 }; -static const int order_1_THEN_4_THEN_3_THEN2[] = { 1, 4, 3, 2 }; +static const uint8_t order_1_THEN_3_THEN_4_THEN2[] = { 1, 3, 4, 2 }; +static const uint8_t order_1_THEN_2_THEN_4_THEN3[] = { 1, 2, 4, 3 }; +static const uint8_t order_1_THEN_3_THEN_2_THEN4[] = { 1, 3, 2, 4 }; +static const uint8_t order_1_THEN_4_THEN_3_THEN2[] = { 1, 4, 3, 2 }; // 5 cylinder -static const int order_1_2_4_5_3[] = {1, 2, 4, 5, 3}; +static const uint8_t order_1_2_4_5_3[] = {1, 2, 4, 5, 3}; // 6 cylinder -static const int order_1_THEN_5_THEN_3_THEN_6_THEN_2_THEN_4[] = { 1, 5, 3, 6, 2, 4 }; -static const int order_1_THEN_4_THEN_2_THEN_5_THEN_3_THEN_6[] = { 1, 4, 2, 5, 3, 6 }; -static const int order_1_THEN_2_THEN_3_THEN_4_THEN_5_THEN_6[] = { 1, 2, 3, 4, 5, 6 }; -static const int order_1_6_3_2_5_4[] = {1, 6, 3, 2, 5, 4}; -static const int order_1_4_3_6_2_5[] = {1, 4, 3, 6, 2, 5}; -static const int order_1_6_2_4_3_5[] = {1, 6, 2, 4, 3, 5}; -static const int order_1_6_5_4_3_2[] = {1, 6, 5, 4, 3, 2}; -static const int order_1_4_5_2_3_6[] = {1, 4, 5, 2, 3, 6}; +static const uint8_t order_1_THEN_5_THEN_3_THEN_6_THEN_2_THEN_4[] = { 1, 5, 3, 6, 2, 4 }; +static const uint8_t order_1_THEN_4_THEN_2_THEN_5_THEN_3_THEN_6[] = { 1, 4, 2, 5, 3, 6 }; +static const uint8_t order_1_THEN_2_THEN_3_THEN_4_THEN_5_THEN_6[] = { 1, 2, 3, 4, 5, 6 }; +static const uint8_t order_1_6_3_2_5_4[] = {1, 6, 3, 2, 5, 4}; +static const uint8_t order_1_4_3_6_2_5[] = {1, 4, 3, 6, 2, 5}; +static const uint8_t order_1_6_2_4_3_5[] = {1, 6, 2, 4, 3, 5}; +static const uint8_t order_1_6_5_4_3_2[] = {1, 6, 5, 4, 3, 2}; +static const uint8_t order_1_4_5_2_3_6[] = {1, 4, 5, 2, 3, 6}; // 8 cylinder -static const int order_1_8_4_3_6_5_7_2[] = { 1, 8, 4, 3, 6, 5, 7, 2 }; -static const int order_1_8_7_2_6_5_4_3[] = { 1, 8, 7, 2, 6, 5, 4, 3 }; -static const int order_1_5_4_2_6_3_7_8[] = { 1, 5, 4, 2, 6, 3, 7, 8 }; -static const int order_1_2_7_8_4_5_6_3[] = { 1, 2, 7, 8, 4, 5, 6, 3 }; -static const int order_1_3_7_2_6_5_4_8[] = { 1, 3, 7, 2, 6, 5, 4, 8 }; -static const int order_1_2_3_4_5_6_7_8[] = { 1, 2, 3, 4, 5, 6, 7, 8 }; -static const int order_1_5_4_8_6_3_7_2[] = { 1, 5, 4, 8, 6, 3, 7, 2 }; -static const int order_1_8_7_3_6_5_4_2[] = { 1, 8, 7, 3, 6, 5, 4, 2 }; +static const uint8_t order_1_8_4_3_6_5_7_2[] = { 1, 8, 4, 3, 6, 5, 7, 2 }; +static const uint8_t order_1_8_7_2_6_5_4_3[] = { 1, 8, 7, 2, 6, 5, 4, 3 }; +static const uint8_t order_1_5_4_2_6_3_7_8[] = { 1, 5, 4, 2, 6, 3, 7, 8 }; +static const uint8_t order_1_2_7_8_4_5_6_3[] = { 1, 2, 7, 8, 4, 5, 6, 3 }; +static const uint8_t order_1_3_7_2_6_5_4_8[] = { 1, 3, 7, 2, 6, 5, 4, 8 }; +static const uint8_t order_1_2_3_4_5_6_7_8[] = { 1, 2, 3, 4, 5, 6, 7, 8 }; +static const uint8_t order_1_5_4_8_6_3_7_2[] = { 1, 5, 4, 8, 6, 3, 7, 2 }; +static const uint8_t order_1_8_7_3_6_5_4_2[] = { 1, 8, 7, 3, 6, 5, 4, 2 }; // 9 cylinder -static const int order_1_2_3_4_5_6_7_8_9[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9 }; +static const uint8_t order_1_2_3_4_5_6_7_8_9[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9 }; // 10 cylinder -static const int order_1_10_9_4_3_6_5_8_7_2[] = {1, 10, 9, 4, 3, 6, 5, 8, 7, 2}; +static const uint8_t order_1_10_9_4_3_6_5_8_7_2[] = {1, 10, 9, 4, 3, 6, 5, 8, 7, 2}; // 12 cyliner -static const int order_1_7_5_11_3_9_6_12_2_8_4_10[] = {1, 7, 5, 11, 3, 9, 6, 12, 2, 8, 4, 10}; -static const int order_1_7_4_10_2_8_6_12_3_9_5_11[] = {1, 7, 4, 10, 2, 8, 6, 12, 3, 9, 5, 11}; -static const int order_1_12_5_8_3_10_6_7_2_11_4_9[] = {1, 12, 5, 8, 3, 10, 6, 7, 2, 11, 4, 9}; -static const int order_1_2_3_4_5_6_7_8_9_10_11_12[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; +static const uint8_t order_1_7_5_11_3_9_6_12_2_8_4_10[] = {1, 7, 5, 11, 3, 9, 6, 12, 2, 8, 4, 10}; +static const uint8_t order_1_7_4_10_2_8_6_12_3_9_5_11[] = {1, 7, 4, 10, 2, 8, 6, 12, 3, 9, 5, 11}; +static const uint8_t order_1_12_5_8_3_10_6_7_2_11_4_9[] = {1, 12, 5, 8, 3, 10, 6, 7, 2, 11, 4, 9}; +static const uint8_t order_1_2_3_4_5_6_7_8_9_10_11_12[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; // no comments -static const int order_1_14_9_4_7_12_15_6_13_8_3_16_11_2_5_10[] = {1, 14, 9, 4, 7, 12, 15, 6, 13, 8, 3, 16, 11, 2, 5, 10}; +static const uint8_t order_1_14_9_4_7_12_15_6_13_8_3_16_11_2_5_10[] = {1, 14, 9, 4, 7, 12, 15, 6, 13, 8, 3, 16, 11, 2, 5, 10}; -static int getFiringOrderLength() { +static size_t getFiringOrderLength() { switch (engineConfiguration->specs.firingOrder) { case FO_1: @@ -226,7 +226,7 @@ static int getFiringOrderLength() { return 1; } -static const int *getFiringOrderTable() +static const uint8_t* getFiringOrderTable() { switch (engineConfiguration->specs.firingOrder) { case FO_1: @@ -323,9 +323,8 @@ static const int *getFiringOrderTable() * @param index from zero to cylindersCount - 1 * @return cylinderId from one to cylindersCount */ -int getCylinderId(int index) { - - const int firingOrderLength = getFiringOrderLength(); +size_t getCylinderId(size_t index) { + const size_t firingOrderLength = getFiringOrderLength(); if (firingOrderLength < 1 || firingOrderLength > MAX_CYLINDER_COUNT) { firmwareError(CUSTOM_FIRING_LENGTH, "fol %d", firingOrderLength); @@ -337,34 +336,36 @@ int getCylinderId(int index) { return 1; } - if (index < 0 || index >= firingOrderLength) { + if (index >= firingOrderLength) { // May 2020 this somehow still happens with functional tests, maybe race condition? warning(CUSTOM_ERR_6686, "firing order index %d", index); return 1; } - const int *firingOrderTable = getFiringOrderTable(); - if (firingOrderTable) + if (auto firingOrderTable = getFiringOrderTable()) { return firingOrderTable[index]; - /* else - error already reported */ - - return 1; + } else { + // error already reported + return 1; + } } /** * @param prevCylinderId from one to cylindersCount * @return cylinderId from one to cylindersCount */ -int getNextFiringCylinderId(int prevCylinderId) { - const int firingOrderLength = getFiringOrderLength(); - const int *firingOrderTable = getFiringOrderTable(); +size_t getNextFiringCylinderId(size_t prevCylinderId) { + const size_t firingOrderLength = getFiringOrderLength(); + auto firingOrderTable = getFiringOrderTable(); if (firingOrderTable) { - for (size_t i = 0; i < firingOrderLength; i++) - if (firingOrderTable[i] == prevCylinderId) + for (size_t i = 0; i < firingOrderLength; i++) { + if (firingOrderTable[i] == prevCylinderId) { return firingOrderTable[(i + 1) % firingOrderLength]; + } + } } + return 1; } diff --git a/firmware/controllers/math/engine_math.h b/firmware/controllers/math/engine_math.h index 565a300731..3e2ba4203c 100644 --- a/firmware/controllers/math/engine_math.h +++ b/firmware/controllers/math/engine_math.h @@ -49,8 +49,8 @@ ignition_mode_e getCurrentIgnitionMode(); */ void prepareIgnitionPinIndices(ignition_mode_e ignitionMode); -int getCylinderId(int index); -int getNextFiringCylinderId(int prevCylinderId); +size_t getCylinderId(size_t index); +size_t getNextFiringCylinderId(size_t prevCylinderId); void setTimingRpmBin(float from, float to); void setTimingLoadBin(float from, float to); diff --git a/firmware/controllers/trigger/decoders/trigger_nissan.cpp b/firmware/controllers/trigger/decoders/trigger_nissan.cpp index 1d036bb82c..30dc472999 100644 --- a/firmware/controllers/trigger/decoders/trigger_nissan.cpp +++ b/firmware/controllers/trigger/decoders/trigger_nissan.cpp @@ -138,7 +138,7 @@ void makeNissanPattern(TriggerWaveform* s, size_t halfCylinderCount, size_t tota auto toothCount = patternTeeth - missing; float currentAngle = missing * toothAngle; - for (int i = 0; i < toothCount; i++) { + for (size_t i = 0; i < toothCount; i++) { currentAngle += toothAngle; s->addEventAngle(currentAngle - 5, T_PRIMARY, TV_RISE); s->addEventAngle(currentAngle, T_PRIMARY, TV_FALL); diff --git a/firmware/controllers/trigger/trigger_decoder.cpp b/firmware/controllers/trigger/trigger_decoder.cpp index 53b1a8c301..06c328a2a6 100644 --- a/firmware/controllers/trigger/trigger_decoder.cpp +++ b/firmware/controllers/trigger/trigger_decoder.cpp @@ -163,17 +163,17 @@ void prepareEventAngles(TriggerWaveform *shape, int riseOnlyIndex = 0; - int length = shape->getLength(); + size_t length = shape->getLength(); memset(details->eventAngles, 0, sizeof(details->eventAngles)); // this may be getSize(); + size_t triggerShapeLength = shape->getSize(); assertAngleRange(shape->triggerShapeSynchPointIndex, "triggerShapeSynchPointIndex", CUSTOM_TRIGGER_SYNC_ANGLE2); efiAssertVoid(CUSTOM_TRIGGER_CYCLE, engine->engineCycleEventCount != 0, "zero engineCycleEventCount"); - for (int eventIndex = 0; eventIndex < length; eventIndex++) { + for (size_t eventIndex = 0; eventIndex < length; eventIndex++) { if (eventIndex == 0) { // explicit check for zero to avoid issues where logical zero is not exactly zero due to float nature details->eventAngles[0] = 0; @@ -259,7 +259,7 @@ float TriggerStateWithRunningStatistics::calculateInstantRpm( /** * todo: Martin has this fatal error while feeding external RPM and changing trigger mode from 4 stoke cam to 4 stroke symmetrical */ - assertIsInBoundsWithResult((int)current_index, timeOfLastEvent, "calc timeOfLastEvent", 0); + assertIsInBoundsWithResult(current_index, timeOfLastEvent, "calc timeOfLastEvent", 0); // Record the time of this event so we can calculate RPM from it later timeOfLastEvent[current_index] = nowNt; @@ -294,7 +294,7 @@ float TriggerStateWithRunningStatistics::calculateInstantRpm( return prevInstantRpmValue; float instantRpm = (60000000.0 / 360 * US_TO_NT_MULTIPLIER) * angleDiff / time; - assertIsInBoundsWithResult((int)current_index, instantRpmValue, "instantRpmValue", 0); + assertIsInBoundsWithResult(current_index, instantRpmValue, "instantRpmValue", 0); instantRpmValue[current_index] = instantRpm; // This fixes early RPM instability based on incomplete data diff --git a/firmware/hw_layer/pin_repository.cpp b/firmware/hw_layer/pin_repository.cpp index 6ff2170d87..e5ca8b3864 100644 --- a/firmware/hw_layer/pin_repository.cpp +++ b/firmware/hw_layer/pin_repository.cpp @@ -189,7 +189,7 @@ void printSpiConfig(const char *msg, spi_device_e device) { #endif // HAL_USE_SPI } -__attribute__((weak)) const char * getBoardSpecificPinName(brain_pin_e brainPin) { +__attribute__((weak)) const char * getBoardSpecificPinName(brain_pin_e /*brainPin*/) { return nullptr; } diff --git a/firmware/util/efilib.h b/firmware/util/efilib.h index 0a20571c41..2d9cb72f22 100644 --- a/firmware/util/efilib.h +++ b/firmware/util/efilib.h @@ -133,9 +133,9 @@ constexpr void clear(T& obj) { } } // namespace efi -#define assertIsInBounds(length, array, msg) efiAssertVoid(OBD_PCM_Processor_Fault, (length) >= 0 && (length) < efi::size(array), msg) +#define assertIsInBounds(length, array, msg) efiAssertVoid(OBD_PCM_Processor_Fault, std::is_unsigned_v && (length) < efi::size(array), msg) -#define assertIsInBoundsWithResult(length, array, msg, failedResult) efiAssert(OBD_PCM_Processor_Fault, (length) >= 0 && (length) < efi::size(array), msg, failedResult) +#define assertIsInBoundsWithResult(length, array, msg, failedResult) efiAssert(OBD_PCM_Processor_Fault, std::is_unsigned_v && (length) < efi::size(array), msg, failedResult) /** * Copies an array from src to dest. The lengths of the arrays must match.