From 6a060e5cae1b92b5ca67d94fdc65d9ab177835e0 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 10 May 2021 05:01:24 -0700 Subject: [PATCH] remove intermediate buffer (#2668) * remove define * goodbye intermediate logging buffer * free ram! woo! --- .../boards/hellen/cypress/efifeatures.h | 1 - firmware/config/boards/kinetis/efifeatures.h | 1 - firmware/config/boards/skeleton/efifeatures.h | 6 -- firmware/config/stm32f4ems/efifeatures.h | 5 -- firmware/controllers/engine_controller.cpp | 2 +- firmware/util/datalogging.cpp | 58 ++++--------------- firmware/util/datalogging.h | 1 - simulator/simulator/efifeatures.h | 5 -- unit_tests/global.h | 1 - 9 files changed, 12 insertions(+), 68 deletions(-) diff --git a/firmware/config/boards/hellen/cypress/efifeatures.h b/firmware/config/boards/hellen/cypress/efifeatures.h index 4eaf5ed837..8c965cfd09 100644 --- a/firmware/config/boards/hellen/cypress/efifeatures.h +++ b/firmware/config/boards/hellen/cypress/efifeatures.h @@ -337,7 +337,6 @@ /** * This is the size of the MemoryStream used by chvprintf */ -#define INTERMEDIATE_LOGGING_BUFFER_SIZE 2000 #define STATUS_LOGGING_BUFFER_SIZE 1800 #define SETTINGS_LOGGING_BUFFER_SIZE 1000 #define DL_OUTPUT_BUFFER 6500 diff --git a/firmware/config/boards/kinetis/efifeatures.h b/firmware/config/boards/kinetis/efifeatures.h index 2fb45faf54..7e19cc8ba2 100644 --- a/firmware/config/boards/kinetis/efifeatures.h +++ b/firmware/config/boards/kinetis/efifeatures.h @@ -303,7 +303,6 @@ /** * This is the size of the MemoryStream used by chvprintf */ -#define INTERMEDIATE_LOGGING_BUFFER_SIZE 200 /*2000*/ #define STATUS_LOGGING_BUFFER_SIZE 120 /*1800*/ #define SETTINGS_LOGGING_BUFFER_SIZE 100 /*1000*/ #define DL_OUTPUT_BUFFER 10 /*6500*/ diff --git a/firmware/config/boards/skeleton/efifeatures.h b/firmware/config/boards/skeleton/efifeatures.h index acaec3fd70..fa88f4efb2 100644 --- a/firmware/config/boards/skeleton/efifeatures.h +++ b/firmware/config/boards/skeleton/efifeatures.h @@ -321,12 +321,6 @@ #define CONFIG_RESET_SWITCH_PIN 6 #endif -/** - * This is the size of the MemoryStream used by chvprintf - */ -#define INTERMEDIATE_LOGGING_BUFFER_SIZE 2000 - - // Enable file logging (like SD card) logic #define EFI_FILE_LOGGING FALSE diff --git a/firmware/config/stm32f4ems/efifeatures.h b/firmware/config/stm32f4ems/efifeatures.h index f2fa5ae112..f02e2e9fe5 100644 --- a/firmware/config/stm32f4ems/efifeatures.h +++ b/firmware/config/stm32f4ems/efifeatures.h @@ -399,9 +399,4 @@ #define CONFIG_RESET_SWITCH_PIN 6 #endif -/** - * This is the size of the MemoryStream used by chvprintf - */ -#define INTERMEDIATE_LOGGING_BUFFER_SIZE 2000 - #define EFI_JOYSTICK TRUE diff --git a/firmware/controllers/engine_controller.cpp b/firmware/controllers/engine_controller.cpp index 2cb09d2eaa..701dfec136 100644 --- a/firmware/controllers/engine_controller.cpp +++ b/firmware/controllers/engine_controller.cpp @@ -702,7 +702,7 @@ void initEngineContoller(DECLARE_ENGINE_PARAMETER_SUFFIX) { * UNUSED_SIZE constants. */ #ifndef RAM_UNUSED_SIZE -#define RAM_UNUSED_SIZE 3900 +#define RAM_UNUSED_SIZE 5900 #endif #ifndef CCM_UNUSED_SIZE #define CCM_UNUSED_SIZE 300 diff --git a/firmware/util/datalogging.cpp b/firmware/util/datalogging.cpp index 978659b938..2a4ee78607 100644 --- a/firmware/util/datalogging.cpp +++ b/firmware/util/datalogging.cpp @@ -40,36 +40,6 @@ #include "os_util.h" #endif // EFI_UNIT_TEST -static uint8_t intermediateLoggingBufferData[INTERMEDIATE_LOGGING_BUFFER_SIZE] CCM_OPTIONAL; - -class IntermediateLogging { -public: - /** - * Class constructors are a great way to have simple initialization sequence - */ - IntermediateLogging() { -#if ! EFI_UNIT_TEST - msObjectInit(&intermediateLoggingBuffer, intermediateLoggingBufferData, INTERMEDIATE_LOGGING_BUFFER_SIZE, 0); -#endif // EFI_UNIT_TEST - } -#if ! EFI_UNIT_TEST - MemoryStream intermediateLoggingBuffer; -#endif // EFI_UNIT_TEST - - // todo: look into chsnprintf once on Chibios 3 - void vappendPrintfI(Logging *logging, const char *fmt, va_list arg) { -#if ! EFI_UNIT_TEST - intermediateLoggingBuffer.eos = 0; // reset - efiAssertVoid(CUSTOM_ERR_6603, getCurrentRemainingStack() > 128, "lowstck#1b"); - chvprintf((BaseSequentialStream *) &intermediateLoggingBuffer, fmt, arg); - intermediateLoggingBuffer.buffer[intermediateLoggingBuffer.eos] = 0; // need to terminate explicitly - logging->append((char *)intermediateLoggingBuffer.buffer); -#endif // EFI_UNIT_TEST - } -}; - -static IntermediateLogging intermediateLogging; - /** * @returns true if data does not fit into this buffer */ @@ -108,22 +78,6 @@ void Logging::appendFast(const char *text) { linePointer = s - 1; } -/** - * this method acquires system lock to guard the shared intermediateLoggingBuffer memory stream - */ -void Logging::vappendPrintf(const char *fmt, va_list arg) { -#if ! EFI_UNIT_TEST -#if EFI_ENABLE_ASSERTS - // todo: Kinetis needs real getCurrentRemainingStack or mock - if (getCurrentRemainingStack() < 128) { - firmwareError(CUSTOM_ERR_6604, "lowstck#5b %s", chThdGetSelfX()->name); - } -#endif // EFI_ENABLE_ASSERTS - chibios_rt::CriticalSectionLocker csl; - intermediateLogging.vappendPrintfI(this, fmt, arg); -#endif // EFI_UNIT_TEST -} - void Logging::appendPrintf(const char *fmt, ...) { #if EFI_UNIT_TEST va_list ap; @@ -132,10 +86,20 @@ void Logging::appendPrintf(const char *fmt, ...) { va_end(ap); #else efiAssertVoid(CUSTOM_APPEND_STACK, getCurrentRemainingStack() > 128, "lowstck#4"); + + size_t available = remainingSize(); + va_list ap; va_start(ap, fmt); - vappendPrintf(fmt, ap); + size_t written = chvsnprintf(linePointer, available, fmt, ap); va_end(ap); + + // chvnsprintf returns how many bytes WOULD HAVE been written if it fit, + // so clip it to the available space if necessary + linePointer += (written > available) ? available : written; + // ensure buffer is always null terminated + buffer[bufferSize - 1] = '\0'; + #endif // EFI_UNIT_TEST } diff --git a/firmware/util/datalogging.h b/firmware/util/datalogging.h index d3ba876156..bc4b7a608f 100644 --- a/firmware/util/datalogging.h +++ b/firmware/util/datalogging.h @@ -21,7 +21,6 @@ public: void reset(); - void vappendPrintf(const char *fmt, va_list arg); void append(const char *text); void appendFast(const char *text); void appendPrintf(const char *fmt, ...); diff --git a/simulator/simulator/efifeatures.h b/simulator/simulator/efifeatures.h index 9155128b0c..0295667cfa 100644 --- a/simulator/simulator/efifeatures.h +++ b/simulator/simulator/efifeatures.h @@ -145,10 +145,5 @@ #define EFI_TUNER_STUDIO TRUE -/** - * This is the size of the MemoryStream used by chvprintf - */ -#define INTERMEDIATE_LOGGING_BUFFER_SIZE 2000 - #define EFI_BOARD_TEST FALSE #define EFI_JOYSTICK FALSE diff --git a/unit_tests/global.h b/unit_tests/global.h index 36e4e8a880..e69aa690f9 100644 --- a/unit_tests/global.h +++ b/unit_tests/global.h @@ -19,7 +19,6 @@ typedef uint32_t ioportid_t; typedef uint32_t ioportmask_t; #define DL_OUTPUT_BUFFER 200 -#define INTERMEDIATE_LOGGING_BUFFER_SIZE 100 // just a stub implementation for unit tests #define EXPECTED_REMAINING_STACK 1