From aeea2b95d090c72669f695fe114493b27aab54b7 Mon Sep 17 00:00:00 2001 From: rusefi Date: Fri, 11 Jan 2019 09:58:48 -0500 Subject: [PATCH] warning code refactoring related to Question: something strange trigger errors #662 --- firmware/console/binary/tunerstudio.cpp | 2 +- firmware/console/status_loop.cpp | 10 +++++----- firmware/controllers/algo/engine.h | 16 +++++++++++++--- firmware/controllers/algo/engine2.cpp | 24 +++++++++++++++++++++--- firmware/controllers/error_handling.cpp | 25 ++++++------------------- firmware/controllers/error_handling.h | 2 -- firmware/controllers/obd2.cpp | 4 ++-- 7 files changed, 48 insertions(+), 35 deletions(-) diff --git a/firmware/console/binary/tunerstudio.cpp b/firmware/console/binary/tunerstudio.cpp index 25acc65952..b2c3735b2a 100644 --- a/firmware/console/binary/tunerstudio.cpp +++ b/firmware/console/binary/tunerstudio.cpp @@ -632,7 +632,7 @@ void handleTestCommand(ts_channel_s *tsChannel) { */ tunerStudioDebug("got T (Test)"); sr5WriteData(tsChannel, (const uint8_t *) VCS_VERSION, sizeof(VCS_VERSION)); - chsnprintf(testOutputBuffer, sizeof(testOutputBuffer), " %d %d", engine->engineState.lastErrorCode, tsState.testCommandCounter); + chsnprintf(testOutputBuffer, sizeof(testOutputBuffer), " %d %d", engine->engineState.warnings.lastErrorCode, tsState.testCommandCounter); sr5WriteData(tsChannel, (const uint8_t *) testOutputBuffer, strlen(testOutputBuffer)); /** * Please note that this response is a magic constant used by dev console for protocol detection diff --git a/firmware/console/status_loop.cpp b/firmware/console/status_loop.cpp index 5026d2d9c1..0c2b47d1fa 100644 --- a/firmware/console/status_loop.cpp +++ b/firmware/console/status_loop.cpp @@ -334,8 +334,8 @@ static void printSensors(Logging *log, bool fileFormat) { } } - reportSensorI(log, fileFormat, GAUGE_NAME_WARNING_COUNTER, "count", engine->engineState.warningCounter); - reportSensorI(log, fileFormat, GAUGE_NAME_WARNING_LAST, "code", engine->engineState.lastErrorCode); + reportSensorI(log, fileFormat, GAUGE_NAME_WARNING_COUNTER, "count", engine->engineState.warnings.warningCounter); + reportSensorI(log, fileFormat, GAUGE_NAME_WARNING_LAST, "code", engine->engineState.warnings.lastErrorCode); reportSensorI(log, fileFormat, INDICATOR_NAME_CLUTCH_UP, "bool", engine->clutchUpState); reportSensorI(log, fileFormat, INDICATOR_NAME_CLUTCH_DOWN, "bool", engine->clutchDownState); @@ -742,7 +742,7 @@ void updateTunerStudioState(TunerStudioOutputChannels *tsOutputChannels DECLARE_ tsOutputChannels->timeSeconds = timeSeconds; tsOutputChannels->firmwareVersion = getRusEfiVersion(); - tsOutputChannels->isWarnNow = isWarningNow(timeSeconds, true); + tsOutputChannels->isWarnNow = engine->engineState.warnings.isWarningNow(timeSeconds, true); tsOutputChannels->isCltBroken = engine->isCltBroken; #if EFI_HIP_9011 || defined(__DOXYGEN__) tsOutputChannels->isKnockChipOk = (instance.invalidHip9011ResponsesCount == 0); @@ -914,8 +914,8 @@ void updateTunerStudioState(TunerStudioOutputChannels *tsOutputChannels DECLARE_ tsOutputChannels->fuelConsumptionPerHour = engine->engineState.fuelConsumption.perSecondConsumption; - tsOutputChannels->warningCounter = engine->engineState.warningCounter; - tsOutputChannels->lastErrorCode = engine->engineState.lastErrorCode; + tsOutputChannels->warningCounter = engine->engineState.warnings.warningCounter; + tsOutputChannels->lastErrorCode = engine->engineState.warnings.lastErrorCode; tsOutputChannels->knockNowIndicator = engine->knockCount > 0; tsOutputChannels->knockEverIndicator = engine->knockEver; diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index 18420dc30e..5a34bce855 100644 --- a/firmware/controllers/algo/engine.h +++ b/firmware/controllers/algo/engine.h @@ -135,6 +135,18 @@ public: gear_e gearSelectorPosition; }; +class WarningCodeState { +public: + WarningCodeState(); + void addWarningCode(obd_code_e code); + bool isWarningNow(efitimesec_t now, bool forIndicator DECLARE_ENGINE_PARAMETER_SUFFIX); + void clear(); + int warningCounter; + int lastErrorCode; + efitimesec_t timeOfPreviousWarning; + // todo: we need a way to post multiple recent warnings into TS + cyclic_buffer recentWarninig; +}; class EngineState { public: @@ -148,9 +160,7 @@ public: efitick_t crankingTime; efitick_t timeSinceCranking; - int warningCounter; - int lastErrorCode; - efitimesec_t timeOfPreviousWarning; + WarningCodeState warnings; /** * speed-density logic, calculated air mass in grams diff --git a/firmware/controllers/algo/engine2.cpp b/firmware/controllers/algo/engine2.cpp index 35734124b5..716d45584e 100644 --- a/firmware/controllers/algo/engine2.cpp +++ b/firmware/controllers/algo/engine2.cpp @@ -29,6 +29,27 @@ extern LoggingWithStorage engineLogger; extern TunerStudioOutputChannels tsOutputChannels; #endif /* EFI_TUNER_STUDIO */ +WarningCodeState::WarningCodeState() { + warningCounter = 0; + lastErrorCode = 0; + timeOfPreviousWarning = -10; +} + +void WarningCodeState::addWarningCode(obd_code_e code) { + warningCounter++; + lastErrorCode = code; +//todo: add cyclic_buffer.contains method + // if (!recentWarninig contains code) + recentWarninig.add((int)code); +} + +/** + * @param forIndicator if we want to retrieving value for TS indicator, this case a minimal period is applued + */ +bool WarningCodeState::isWarningNow(efitimesec_t now, bool forIndicator DECLARE_ENGINE_PARAMETER_SUFFIX) { + int period = forIndicator ? maxI(3, engineConfiguration->warningPeriod) : engineConfiguration->warningPeriod; + return absI(now - timeOfPreviousWarning) < period; +} MockAdcState::MockAdcState() { memset(hasMockAdc, 0, sizeof(hasMockAdc)); @@ -80,8 +101,6 @@ EngineState::EngineState() { dwellAngle = NAN; engineNoiseHipLevel = 0; injectorLag = 0; - warningCounter = 0; - lastErrorCode = 0; crankingTime = 0; timeSinceCranking = 0; vssEventCounter = 0; @@ -92,7 +111,6 @@ EngineState::EngineState() { airFlow = 0; cltTimingCorrection = 0; runningFuel = baseFuel = currentVE = 0; - timeOfPreviousWarning = -10; baseTableFuel = iatFuelCorrection = 0; fuelPidCorrection = 0; cltFuelCorrection = postCrankingFuelCorrection = 0; diff --git a/firmware/controllers/error_handling.cpp b/firmware/controllers/error_handling.cpp index 8e8b06aa62..91de2bc7f7 100644 --- a/firmware/controllers/error_handling.cpp +++ b/firmware/controllers/error_handling.cpp @@ -74,20 +74,6 @@ void chDbgPanic3(const char *msg, const char * file, int line) { } } - -/** - * @param forIndicator if we want to retrieving value for TS indicator, this case a minimal period is applued - */ -bool isWarningNow(efitimesec_t now, bool forIndicator) { - int period = forIndicator ? maxI(3, engineConfiguration->warningPeriod) : engineConfiguration->warningPeriod; - return absI(now - engine->engineState.timeOfPreviousWarning) < period; -} - -void addWarningCode(obd_code_e code) { - engine->engineState.warningCounter++; - engine->engineState.lastErrorCode = code; -} - // todo: look into chsnprintf // todo: move to some util file & reuse for 'firmwareError' method static void printToStream(MemoryStream *stream, const char *fmt, va_list ap) { @@ -117,7 +103,7 @@ int unitTestWarningCounter = 0; /** * OBD_PCM_Processor_Fault is the general error code for now * - * @returns TRUE in case there are too many warnings + * @returns TRUE in case there were warnings recently */ bool warning(obd_code_e code, const char *fmt, ...) { if (hasFirmwareErrorFlag) @@ -132,12 +118,13 @@ bool warning(obd_code_e code, const char *fmt, ...) { firmwareError(CUSTOM_ERR_ASSERT, "warn stream not initialized for %d", code); return false; } - addWarningCode(code); + engine->engineState.warnings.addWarningCode(code); + // todo: move this logic into WarningCodeState? efitimesec_t now = getTimeNowSeconds(); - if (isWarningNow(now, false) || !warningEnabled) + if (engine->engineState.warnings.isWarningNow(now, false) || !warningEnabled) return true; // we just had another warning, let's not spam - engine->engineState.timeOfPreviousWarning = now; + engine->engineState.warnings.timeOfPreviousWarning = now; va_list ap; va_start(ap, fmt); @@ -207,7 +194,7 @@ void firmwareError(obd_code_e code, const char *fmt, ...) { #if EFI_PROD_CODE || defined(__DOXYGEN__) if (hasFirmwareErrorFlag) return; - addWarningCode(code); + engine->engineState.warnings.addWarningCode(code); #ifdef EFI_PRINT_ERRORS_AS_WARNINGS va_list ap; va_start(ap, fmt); diff --git a/firmware/controllers/error_handling.h b/firmware/controllers/error_handling.h index bcd7f16da1..d7bcbe9d1d 100644 --- a/firmware/controllers/error_handling.h +++ b/firmware/controllers/error_handling.h @@ -16,7 +16,6 @@ extern "C" #include "global.h" #include "obd_error_codes.h" -void addWarningCode(obd_code_e code); /** * Something is wrong, but we can live with it: some minor sensor is disconnected @@ -36,7 +35,6 @@ void firmwareError(obd_code_e code, const char *fmt, ...); #define hasFirmwareError() hasFirmwareErrorFlag -bool isWarningNow(efitimesec_t now, bool forIndicator); // todo: rename to getFatalErrorMessage char *getFirmwareError(void); diff --git a/firmware/controllers/obd2.cpp b/firmware/controllers/obd2.cpp index f4b0c451dc..7b91450100 100644 --- a/firmware/controllers/obd2.cpp +++ b/firmware/controllers/obd2.cpp @@ -202,11 +202,11 @@ void obdOnCanPacketRx(CANRxFrame *rx) { } else if (rx->data8[0] == 1 && rx->data8[1] == OBD_STORED_DIAGNOSTIC_TROUBLE_CODES) { scheduleMsg(&logger, "Got stored DTC request"); // todo: implement stored/pending difference? - handleDtcRequest(1, &engine->engineState.lastErrorCode); + handleDtcRequest(1, &engine->engineState.warnings.lastErrorCode); } else if (rx->data8[0] == 1 && rx->data8[1] == OBD_PENDING_DIAGNOSTIC_TROUBLE_CODES) { scheduleMsg(&logger, "Got pending DTC request"); // todo: implement stored/pending difference? - handleDtcRequest(1, &engine->engineState.lastErrorCode); + handleDtcRequest(1, &engine->engineState.warnings.lastErrorCode); } else { scheduleMsg(&logger, "Got unhandled OBD message"); }