diff --git a/firmware/config/boards/prometheus/efifeatures.h b/firmware/config/boards/prometheus/efifeatures.h index 86b3d1a0b1..9f1da164ef 100644 --- a/firmware/config/boards/prometheus/efifeatures.h +++ b/firmware/config/boards/prometheus/efifeatures.h @@ -114,7 +114,5 @@ #define RPM_LOW_THRESHOLD 8 // RPM=8 is an empirical lower sensitivity threshold of MAX9926 for 60-2 #define NO_RPM_EVENTS_TIMEOUT_SECS 5 // (RPM < 12) -#define EFI_PRINT_ERRORS_AS_WARNINGS TRUE - #define EFI_NARROW_EGO_AVERAGING TRUE diff --git a/firmware/config/boards/skeleton/efifeatures.h b/firmware/config/boards/skeleton/efifeatures.h index 8e956f1e93..e248c8cc69 100644 --- a/firmware/config/boards/skeleton/efifeatures.h +++ b/firmware/config/boards/skeleton/efifeatures.h @@ -329,7 +329,6 @@ // Enable file logging (like SD card) logic #define EFI_FILE_LOGGING FALSE -#define EFI_PRINT_ERRORS_AS_WARNINGS TRUE #define EFI_USB_SERIAL TRUE diff --git a/firmware/config/boards/subaru_eg33/efifeatures.h b/firmware/config/boards/subaru_eg33/efifeatures.h index 3d89b09b46..b967d93c93 100644 --- a/firmware/config/boards/subaru_eg33/efifeatures.h +++ b/firmware/config/boards/subaru_eg33/efifeatures.h @@ -141,8 +141,6 @@ #define RPM_LOW_THRESHOLD 8 // RPM=8 is an empirical lower sensitivity threshold of MAX9926 for 60-2 #define NO_RPM_EVENTS_TIMEOUT_SECS 5 // (RPM < 12) -#define EFI_PRINT_ERRORS_AS_WARNINGS TRUE - #define EFI_NARROW_EGO_AVERAGING TRUE #endif /* EFIFEATURES_SUBARUEG33_H_ */ diff --git a/firmware/console/binary/tunerstudio.cpp b/firmware/console/binary/tunerstudio.cpp index 65905afc83..ab6fadcca7 100644 --- a/firmware/console/binary/tunerstudio.cpp +++ b/firmware/console/binary/tunerstudio.cpp @@ -868,7 +868,7 @@ int TunerStudioBase::handleCrcCommand(TsChannelBase* tsChannel, char *data, int break; #endif /* ENABLE_PERF_TRACE */ case TS_GET_CONFIG_ERROR: { - char * configError = getFirmwareError(); + const char* configError = getFirmwareError(); #if HW_CHECK_MODE // analog input errors are returned as firmware error in QC mode if (!hasFirmwareError()) { diff --git a/firmware/controllers/core/error_handling.cpp b/firmware/controllers/core/error_handling.cpp index 92052ec180..2f248aa011 100644 --- a/firmware/controllers/core/error_handling.cpp +++ b/firmware/controllers/core/error_handling.cpp @@ -9,34 +9,9 @@ #include "os_access.h" #include "perf_trace.h" -static char warningBuffer[ERROR_BUFFER_SIZE]; +static critical_msg_t warningBuffer; static critical_msg_t criticalErrorMessageBuffer; -#if EFI_SIMULATOR || EFI_PROD_CODE -//todo: move into simulator global -#include "memstreams.h" -class ErrorState { -public: - /** - * Class constructors are a great way to have simple initialization sequence - */ - ErrorState(); - MemoryStream warningStream; - MemoryStream firmwareErrorMessageStream; -}; - -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" #endif /* EFI_HD44780_LCD */ @@ -57,8 +32,8 @@ int dbg_panic_line; extern persistent_config_s configWorkingCopy; #endif -char *getFirmwareError(void) { - return (char*) criticalErrorMessageBuffer; +const char* getFirmwareError(void) { + return criticalErrorMessageBuffer; } #if EFI_PROD_CODE @@ -106,33 +81,6 @@ void chDbgPanic3(const char *msg, const char * file, int line) { } } -// 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) { - stream->eos = 0; // reset - chvprintf((BaseSequentialStream *) stream, fmt, ap); - - // Terminate, but don't write past the end of the buffer - int terminatorLocation = minI(stream->eos, stream->size - 1); - stream->buffer[terminatorLocation] = '\0'; -} - -static void printWarning(const char *fmt, va_list ap) { - printToStream(&errorState.warningStream, fmt, ap); - - if (CONFIG(showHumanReadableWarning)) { -#if EFI_TUNER_STUDIO - #if defined(EFI_NO_CONFIG_WORKING_COPY) - memcpy(persistentState.persistentConfiguration.warning_message, warningBuffer, sizeof(warningBuffer)); - #else /* defined(EFI_NO_CONFIG_WORKING_COPY) */ - memcpy(configWorkingCopy.warning_message, warningBuffer, sizeof(warningBuffer)); - #endif /* defined(EFI_NO_CONFIG_WORKING_COPY) */ -#endif /* EFI_TUNER_STUDIO */ - } - - scheduleMsg(&logger, "WARNING: %s", warningBuffer); -} - #else WarningCodeState unitTestWarningCodeState; @@ -162,8 +110,20 @@ bool warning(obd_code_e code, const char *fmt, ...) { va_list ap; va_start(ap, fmt); - printWarning(fmt, ap); + chvsnprintf(warningBuffer, sizeof(warningBuffer), fmt, ap); va_end(ap); + + if (CONFIG(showHumanReadableWarning)) { +#if EFI_TUNER_STUDIO + #if defined(EFI_NO_CONFIG_WORKING_COPY) + memcpy(persistentState.persistentConfiguration.warning_message, warningBuffer, sizeof(warningBuffer)); + #else /* defined(EFI_NO_CONFIG_WORKING_COPY) */ + memcpy(configWorkingCopy.warning_message, warningBuffer, sizeof(warningBuffer)); + #endif /* defined(EFI_NO_CONFIG_WORKING_COPY) */ +#endif /* EFI_TUNER_STUDIO */ + } + + scheduleMsg(&logger, "WARNING: %s", warningBuffer); #else // todo: we need access to 'engine' here so that we can migrate to real 'engine->engineState.warnings' unitTestWarningCodeState.addWarningCode(code); @@ -178,7 +138,7 @@ bool warning(obd_code_e code, const char *fmt, ...) { return false; } -char *getWarningMessage(void) { +const char* getWarningMessage(void) { return warningBuffer; } @@ -241,7 +201,7 @@ void firmwareError(obd_code_e code, const char *fmt, ...) { #ifdef EFI_PRINT_ERRORS_AS_WARNINGS va_list ap; va_start(ap, fmt); - printWarning(fmt, ap); + chvsnprintf(warningBuffer, sizeof(warningBuffer), fmt, ap); va_end(ap); #endif ON_CRITICAL_ERROR() @@ -250,20 +210,17 @@ void firmwareError(obd_code_e code, const char *fmt, ...) { if (indexOf(fmt, '%') == -1) { /** * in case of simple error message let's reduce stack usage - * because chvprintf might be causing an error + * chvsnprintf could cause an overflow if we're already low */ strncpy((char*) criticalErrorMessageBuffer, fmt, sizeof(criticalErrorMessageBuffer) - 1); criticalErrorMessageBuffer[sizeof(criticalErrorMessageBuffer) - 1] = 0; // just to be sure } else { - // todo: look into chsnprintf once on Chibios 3 - errorState.firmwareErrorMessageStream.eos = 0; // reset va_list ap; va_start(ap, fmt); - chvprintf((BaseSequentialStream *) &errorState.firmwareErrorMessageStream, fmt, ap); + chvsnprintf(criticalErrorMessageBuffer, sizeof(criticalErrorMessageBuffer), fmt, ap); va_end(ap); - // todo: reuse warning buffer helper method - errorState.firmwareErrorMessageStream.buffer[errorState.firmwareErrorMessageStream.eos] = 0; // need to terminate explicitly } + int size = strlen((char*)criticalErrorMessageBuffer); static char versionBuffer[32]; chsnprintf(versionBuffer, sizeof(versionBuffer), " %d@%s", getRusEfiVersion(), FIRMWARE_ID); diff --git a/firmware/controllers/core/error_handling.h b/firmware/controllers/core/error_handling.h index 3775cf85c6..47bc63f9fe 100644 --- a/firmware/controllers/core/error_handling.h +++ b/firmware/controllers/core/error_handling.h @@ -23,7 +23,8 @@ extern "C" */ bool warning(obd_code_e code, const char *fmt, ...); -typedef uint8_t critical_msg_t[ERROR_BUFFER_SIZE]; +typedef char critical_msg_t[ERROR_BUFFER_SIZE]; + /** * Something really bad had happened - firmware cannot function, we cannot run the engine * We definitely use this critical error approach in case of invalid configuration. If user sets a self-contradicting @@ -38,10 +39,8 @@ extern bool hasFirmwareErrorFlag; #define hasFirmwareError() hasFirmwareErrorFlag // todo: rename to getCriticalErrorMessage -char *getFirmwareError(void); - -// todo: rename to getWarningMessage? -char *getWarningMessage(void); +const char* getFirmwareError(void); +const char* getWarningMessage(void); // todo: better place for this shared declaration? int getRusEfiVersion(void); diff --git a/firmware/init/sensor/init_tps.cpp b/firmware/init/sensor/init_tps.cpp index 36e51aa821..f96b3056f9 100644 --- a/firmware/init/sensor/init_tps.cpp +++ b/firmware/init/sensor/init_tps.cpp @@ -53,7 +53,7 @@ static bool configureTps(LinearFunc& func, adc_channel_e channel, float closed, // If the voltage for closed vs. open is very near, something is wrong with your calibration if (split < 0.5f) { - firmwareError(OBD_Throttle_Position_Sensor_Circuit_Malfunction, "Sensor \"%s\" problem: open %f/closed %f calibration values are too close together. Please check your wiring!", msg, + firmwareError(OBD_Throttle_Position_Sensor_Circuit_Malfunction, "\"%s\" problem: open %.2f/closed %.2f cal values are too close together. Check your wiring!", msg, open, closed); return false;