From 11a02b639587314c800224d314206a6a4879102b Mon Sep 17 00:00:00 2001 From: rusefi Date: Sun, 3 May 2020 10:58:52 -0400 Subject: [PATCH] class constructors are a great way to have simple initialization sequence --- firmware/controllers/core/error_handling.cpp | 55 ++++++++++---------- firmware/controllers/core/error_handling.h | 1 - firmware/hw_layer/pin_repository.h | 3 ++ firmware/rusefi.cpp | 1 - simulator/simulator/rusEfiFunctionalTest.cpp | 1 - 5 files changed, 30 insertions(+), 31 deletions(-) diff --git a/firmware/controllers/core/error_handling.cpp b/firmware/controllers/core/error_handling.cpp index 1ab32d445b..fdf40bde7a 100644 --- a/firmware/controllers/core/error_handling.cpp +++ b/firmware/controllers/core/error_handling.cpp @@ -8,15 +8,33 @@ #include "engine.h" #include "os_access.h" +static char warningBuffer[ERROR_BUFFER_SIZE]; +static critical_msg_t criticalErrorMessageBuffer; + #if EFI_SIMULATOR || EFI_PROD_CODE //todo: move into simulator global #include "memstreams.h" -static MemoryStream warningStream; -static MemoryStream firmwareErrorMessageStream; -#endif /* EFI_SIMULATOR || EFI_PROD_CODE */ +class ErrorState { +public: + /** + * Class constructors are a great way to have simple initialization sequence + */ + ErrorState(); + MemoryStream warningStream; + MemoryStream firmwareErrorMessageStream; +}; -static char warningBuffer[ERROR_BUFFER_SIZE]; -static volatile bool isWarningStreamInitialized = false; +ErrorState::ErrorState() { + /** + * these methods only change RAM state of data structures without any HAL access thus safe in contructor + */ + msObjectInit(&warningStream, (uint8_t *) warningBuffer, ERROR_BUFFER_SIZE, 0); + msObjectInit(&firmwareErrorMessageStream, criticalErrorMessageBuffer, sizeof(criticalErrorMessageBuffer), 0); +} + +static ErrorState errorState; + +#endif /* EFI_SIMULATOR || EFI_PROD_CODE */ #if EFI_HD44780_LCD #include "lcd_HD44780.h" @@ -31,7 +49,6 @@ EXTERN_ENGINE; extern int warningEnabled; extern bool main_loop_started; -static critical_msg_t criticalErrorMessageBuffer; bool hasFirmwareErrorFlag = false; const char *dbg_panic_file; @@ -105,7 +122,7 @@ static void printWarning(const char *fmt, va_list ap) { logger.append(WARNING_PREFIX); - printToStream(&warningStream, fmt, ap); + printToStream(&errorState.warningStream, fmt, ap); if (CONFIG(showHumanReadableWarning)) { #if EFI_TUNER_STUDIO @@ -141,10 +158,6 @@ bool warning(obd_code_e code, const char *fmt, ...) { #endif /* EFI_SIMULATOR */ #if EFI_SIMULATOR || EFI_PROD_CODE - if (!isWarningStreamInitialized) { - firmwareError(CUSTOM_ERR_ASSERT, "warn stream not initialized for %d", code); - return false; - } engine->engineState.warnings.addWarningCode(code); // todo: move this logic into WarningCodeState? @@ -209,20 +222,6 @@ void onUnlockHook(void) { #endif /* EFI_CLOCK_LOCKS */ -/** - * This method should be invoked really early in firmware initialization cycle. - * - * Implementation can only do trivial things like changing memory state. No hardware or OS access allowed - * within this method. - */ -void initErrorHandlingDataStructures(void) { -#if EFI_SIMULATOR || EFI_PROD_CODE - msObjectInit(&warningStream, (uint8_t *) warningBuffer, ERROR_BUFFER_SIZE, 0); - msObjectInit(&firmwareErrorMessageStream, criticalErrorMessageBuffer, sizeof(criticalErrorMessageBuffer), 0); -#endif - isWarningStreamInitialized = true; -} - void firmwareError(obd_code_e code, const char *fmt, ...) { #if EFI_PROD_CODE if (hasFirmwareErrorFlag) @@ -246,13 +245,13 @@ void firmwareError(obd_code_e code, const char *fmt, ...) { criticalErrorMessageBuffer[sizeof(criticalErrorMessageBuffer) - 1] = 0; // just to be sure } else { // todo: look into chsnprintf once on Chibios 3 - firmwareErrorMessageStream.eos = 0; // reset + errorState.firmwareErrorMessageStream.eos = 0; // reset va_list ap; va_start(ap, fmt); - chvprintf((BaseSequentialStream *) &firmwareErrorMessageStream, fmt, ap); + chvprintf((BaseSequentialStream *) &errorState.firmwareErrorMessageStream, fmt, ap); va_end(ap); // todo: reuse warning buffer helper method - firmwareErrorMessageStream.buffer[firmwareErrorMessageStream.eos] = 0; // need to terminate explicitly + errorState.firmwareErrorMessageStream.buffer[errorState.firmwareErrorMessageStream.eos] = 0; // need to terminate explicitly } #else diff --git a/firmware/controllers/core/error_handling.h b/firmware/controllers/core/error_handling.h index bca7435b04..3775cf85c6 100644 --- a/firmware/controllers/core/error_handling.h +++ b/firmware/controllers/core/error_handling.h @@ -40,7 +40,6 @@ extern bool hasFirmwareErrorFlag; // todo: rename to getCriticalErrorMessage char *getFirmwareError(void); -void initErrorHandlingDataStructures(void); // todo: rename to getWarningMessage? char *getWarningMessage(void); diff --git a/firmware/hw_layer/pin_repository.h b/firmware/hw_layer/pin_repository.h index d6124572fe..697c9d304c 100644 --- a/firmware/hw_layer/pin_repository.h +++ b/firmware/hw_layer/pin_repository.h @@ -18,6 +18,9 @@ class PinRepository { public: + /** + * Class constructors are a great way to have simple initialization sequence + */ PinRepository(); int totalPinsUsed = 0; }; diff --git a/firmware/rusefi.cpp b/firmware/rusefi.cpp index 599a746a7f..08837224ed 100644 --- a/firmware/rusefi.cpp +++ b/firmware/rusefi.cpp @@ -158,7 +158,6 @@ static void scheduleReboot(void) { } void runRusEfi(void) { - initErrorHandlingDataStructures(); efiAssertVoid(CUSTOM_RM_STACK_1, getCurrentRemainingStack() > 512, "init s"); assertEngineReference(); engine->setConfig(config); diff --git a/simulator/simulator/rusEfiFunctionalTest.cpp b/simulator/simulator/rusEfiFunctionalTest.cpp index b0946d9553..0d2e9254fe 100644 --- a/simulator/simulator/rusEfiFunctionalTest.cpp +++ b/simulator/simulator/rusEfiFunctionalTest.cpp @@ -90,7 +90,6 @@ static void runChprintfTest() { void rusEfiFunctionalTest(void) { printToConsole("Running rusEfi simulator version:"); - initErrorHandlingDataStructures(); static char versionBuffer[20]; itoa10(versionBuffer, (int)getRusEfiVersion()); printToConsole(versionBuffer);