From 68b007c759407e2946431a7569cf800eefb751e9 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Sat, 14 May 2022 15:39:43 +0200 Subject: [PATCH] Fix compile-time conversion MMU2 ErrorCode -> Prusa-Error-Code and hide some of the implementation details into mmu2_error_converter.cpp which makes the code in mmu_reporting.cpp much easier to read. --- Firmware/mmu2.cpp | 16 +-------- Firmware/mmu2/buttons.h | 22 ++++++++++++ Firmware/mmu2/errors_list.h | 25 ++++--------- Firmware/mmu2_error_converter.cpp | 58 +++++++++++++++++++------------ Firmware/mmu2_error_converter.h | 32 +++++++++++++++-- Firmware/mmu2_reporting.cpp | 28 ++++++--------- Firmware/ultralcd.cpp | 4 +-- Firmware/ultralcd.h | 4 +-- 8 files changed, 110 insertions(+), 79 deletions(-) create mode 100644 Firmware/mmu2/buttons.h diff --git a/Firmware/mmu2.cpp b/Firmware/mmu2.cpp index f490f9cd6..3feda5bd6 100644 --- a/Firmware/mmu2.cpp +++ b/Firmware/mmu2.cpp @@ -607,22 +607,8 @@ void MMU2::ReportError(ErrorCode ec) { if( ec != lastErrorCode ){ // deduplicate: only report changes in error codes into the log lastErrorCode = ec; - - // Log error format: MMU2:E=32766 ErrorTitle TextDescription - - // The longest error description in errors_list.h is 144 bytes. - // and the longest error title is 20 bytes. msg buffer needs - // to have enough space to fit both. - char msg[192]; - int len = snprintf(msg, sizeof(msg), "MMU2:E=%hu ", (uint16_t)ec); - // Append a human readable form of the error code(s) - TranslateErr((uint16_t)ec, &msg[len], 192 - len); - - // beware - the prefix in the message ("MMU2") will get stripped by the logging subsystem - // and a correct MMU2 component will be assigned accordingly - see appmain.cpp - // Therefore I'm not calling MMU2_ERROR_MSG or MMU2_ECHO_MSG here SERIAL_ECHO_START; - SERIAL_ECHOLN(msg); + SERIAL_ECHOLNRPGM( PrusaErrorTitle(PrusaErrorCodeIndex((uint16_t)ec)) ); } static_assert(mmu2Magic[0] == 'M' diff --git a/Firmware/mmu2/buttons.h b/Firmware/mmu2/buttons.h new file mode 100644 index 000000000..68673971b --- /dev/null +++ b/Firmware/mmu2/buttons.h @@ -0,0 +1,22 @@ +#pragma once +#include + +// Helper macros to parse the operations from Btns() +#define BUTTON_OP_HI_NIBBLE(X) ( ( X & 0xF0 ) >> 4 ) +#define BUTTON_OP_LO_NIBBLE(X) ( X & 0x0F ) + +namespace MMU2 { + +/// Will be mapped onto dialog button responses in the FW +/// Those responses have their unique+translated texts as well +enum class ButtonOperations : uint8_t { + NoOperation = 0, + Retry = 1, + Continue = 2, + RestartMMU = 3, + Unload = 4, + StopPrint = 5, + DisableMMU = 6, +}; + +} // namespace MMU2 diff --git a/Firmware/mmu2/errors_list.h b/Firmware/mmu2/errors_list.h index e68652dba..081ddc017 100644 --- a/Firmware/mmu2/errors_list.h +++ b/Firmware/mmu2/errors_list.h @@ -1,9 +1,11 @@ // Extracted from Prusa-Error-Codes repo // Subject to automation and optimization +// BEWARE - this file shall be included only into mmu2_error_converter.cpp, not anywhere else! #pragma once #include "inttypes.h" #include "../language.h" #include +#include "buttons.h" namespace MMU2 { @@ -72,7 +74,7 @@ typedef enum : uint16_t { // and inadvertedly falls back to copying the whole structure into RAM (which is obviously unwanted). // But since this file ought to be generated in the future from yaml prescription, // it really makes no difference if there are "nice" data structures or plain arrays. -static const uint16_t errorCodes[] PROGMEM = { +static const constexpr uint16_t errorCodes[] PROGMEM = { ERR_MECHANICAL_FINDA_DIDNT_TRIGGER, ERR_MECHANICAL_FINDA_DIDNT_GO_OFF, ERR_MECHANICAL_FSENSOR_DIDNT_TRIGGER, @@ -260,19 +262,6 @@ static const char * const errorDescs[] PROGMEM = { descUNLOAD_MANUALLY }; - -/// Will be mapped onto dialog button responses in the FW -/// Those responses have their unique+translated texts as well -enum class ButtonOperations : uint8_t { - NoOperation = 0, - Retry = 1, - Continue = 2, - RestartMMU = 3, - Unload = 4, - StopPrint = 5, - DisableMMU = 6, -}; - // we have max 3 buttons/operations to select from // one of them is "More" to show the explanation text normally hidden in the next screens. // 01234567890123456789 @@ -297,10 +286,6 @@ static const char * const btnOperation[] PROGMEM = { btnDisableMMU }; -// Helper macros to parse the operations from Btns() -#define BUTTON_OP_HI_NIBBLE(X) ( ( X & 0xF0 ) >> 4 ) -#define BUTTON_OP_LO_NIBBLE(X) ( X & 0x0F ) - // We have 8 different operations/buttons at this time, so we need at least 4 bits to encode each. // Since one of the buttons is always "More", we can skip that one. // Therefore we need just 1 byte to describe the necessary buttons for each screen. @@ -350,4 +335,8 @@ static const uint8_t errorButtons[] PROGMEM = { Btns(ButtonOperations::Retry, ButtonOperations::NoOperation), }; +static_assert( sizeof(errorCodes) / sizeof(errorCodes[0]) == sizeof(errorDescs) / sizeof (errorDescs[0])); +static_assert( sizeof(errorCodes) / sizeof(errorCodes[0]) == sizeof(errorTitles) / sizeof (errorTitles[0])); +static_assert( sizeof(errorCodes) / sizeof(errorCodes[0]) == sizeof(errorButtons) / sizeof (errorButtons[0])); + } // namespace MMU2 diff --git a/Firmware/mmu2_error_converter.cpp b/Firmware/mmu2_error_converter.cpp index ee98765c0..6514430ad 100644 --- a/Firmware/mmu2_error_converter.cpp +++ b/Firmware/mmu2_error_converter.cpp @@ -20,16 +20,17 @@ constexpr InputIt find_if_cx(InputIt first, InputIt last, UnaryPredicate p) { // Making a constexpr FindError should instruct the compiler to optimize the ConvertMMUErrorCode // in such a way that no searching will ever be done at runtime. // A call to FindError then compiles to a single instruction even on the AVR. -static constexpr uint16_t FindErrorIndex(uint32_t pec) { - constexpr uint32_t errorCodesSize = sizeof(errorCodes) / sizeof(errorCodes[0]); - constexpr auto errorCodesEnd = errorCodes + errorCodesSize; - auto i = find_if_cx(errorCodes, errorCodesEnd, [pec](uint16_t ed) -> bool { - return ed == pec; - }); - return i != errorCodesEnd ? *i : errorCodes[errorCodesSize - 1]; +static constexpr uint8_t FindErrorIndex(uint16_t pec) { + constexpr uint16_t errorCodesSize = sizeof(errorCodes) / sizeof(errorCodes[0]); + constexpr const auto *errorCodesEnd = errorCodes + errorCodesSize; + const auto *i = find_if_cx(errorCodes, errorCodesEnd, [pec](uint16_t ed){ return ed == pec; }); + return (i != errorCodesEnd) ? (i-errorCodes) : (errorCodesSize - 1); } -const uint16_t MMUErrorCodeIndex(uint16_t ec) { +// check that the searching algoritm works +static_assert( FindErrorIndex(ERR_MECHANICAL_FINDA_DIDNT_TRIGGER) == 0); + +uint8_t PrusaErrorCodeIndex(uint16_t ec) { switch (ec) { case (uint16_t)ErrorCode::FINDA_DIDNT_SWITCH_ON: return FindErrorIndex(ERR_MECHANICAL_FINDA_DIDNT_TRIGGER); @@ -66,9 +67,9 @@ const uint16_t MMUErrorCodeIndex(uint16_t ec) { return FindErrorIndex(ERR_SYSTEM_UNLOAD_MANUALLY); } -// // TMC-related errors - multiple of these can occur at once -// // - in such a case we report the first which gets found/converted into Prusa-Error-Codes (usually the fact, that one TMC has an issue is serious enough) -// // By carefully ordering the checks here we can prioritize the errors being reported to the user. + // TMC-related errors - multiple of these can occur at once + // - in such a case we report the first which gets found/converted into Prusa-Error-Codes (usually the fact, that one TMC has an issue is serious enough) + // By carefully ordering the checks here we can prioritize the errors being reported to the user. if (ec & (uint16_t)ErrorCode::TMC_PULLEY_BIT) { if (ec & (uint16_t)ErrorCode::TMC_IOIN_MISMATCH) return FindErrorIndex(ERR_ELECTRICAL_PULLEY_TMC_DRIVER_ERROR); @@ -110,19 +111,32 @@ const uint16_t MMUErrorCodeIndex(uint16_t ec) { return FindErrorIndex(ERR_TEMPERATURE_IDLER_TMC_OVERHEAT_ERROR); } -// // if nothing got caught, return a generic error -// return FindError(ERR_OTHER); + // if nothing got caught, return a generic runtime error + return FindErrorIndex(ERR_SYSTEM_FW_RUNTIME_ERROR); } -void TranslateErr(uint16_t ec, char *dst, size_t dstSize) { - uint16_t ei = MMUErrorCodeIndex(ec); - // just to prevent the compiler from stripping the data structures from the final binary for now - *dst = errorButtons[ei]; - snprintf( - dst, dstSize, "%S %S", - static_cast(pgm_read_ptr(&errorTitles[ei])), - static_cast(pgm_read_ptr(&errorDescs[ei])) - ); +uint16_t PrusaErrorCode(uint8_t i){ + return pgm_read_word(errorCodes + i); +} + +const char * const PrusaErrorTitle(uint8_t i){ + return (const char * const)pgm_read_ptr(errorTitles + i); +} + +const char * const PrusaErrorDesc(uint8_t i){ + return (const char * const)pgm_read_ptr(errorDescs + i); +} + +uint8_t PrusaErrorButtons(uint8_t i){ + return pgm_read_byte(errorButtons + i); +} + +const char * const PrusaErrorButtonTitle(uint8_t bi){ + return (const char * const)pgm_read_ptr(btnOperation + bi); +} + +const char * const PrusaErrorButtonMore(){ + return btnMore; } } // namespace MMU2 diff --git a/Firmware/mmu2_error_converter.h b/Firmware/mmu2_error_converter.h index 891fe0848..257b3e5d3 100644 --- a/Firmware/mmu2_error_converter.h +++ b/Firmware/mmu2_error_converter.h @@ -3,6 +3,32 @@ #include namespace MMU2 { -const uint16_t MMUErrorCodeIndex(uint16_t ec); -void TranslateErr(uint16_t ec, char *dst, size_t dstSize); -} + +/// Translates MMU2::ErrorCode into an index of Prusa-Error-Codes +/// Basically this is the way to obtain an index into all other functions in this API +uint8_t PrusaErrorCodeIndex(uint16_t ec); + +/// @returns pointer to a PROGMEM string representing the Title of the Prusa-Error-Codes error +/// @param i index of the error - obtained by calling ErrorCodeIndex +const char * const PrusaErrorTitle(uint8_t i); + +/// @returns pointer to a PROGMEM string representing the multi-page Description of the Prusa-Error-Codes error +/// @param i index of the error - obtained by calling ErrorCodeIndex +const char * const PrusaErrorDesc(uint8_t i); + +/// @returns the actual numerical value of the Prusa-Error-Codes error +/// @param i index of the error - obtained by calling ErrorCodeIndex +uint16_t PrusaErrorCode(uint8_t i); + +/// @returns Btns pair of buttons for a particular Prusa-Error-Codes error +/// @param i index of the error - obtained by calling ErrorCodeIndex +uint8_t PrusaErrorButtons(uint8_t i); + +/// @returns pointer to a PROGMEM string representing the Title of a button +/// @param i index of the error - obtained by calling PrusaErrorButtons + extracting low or high nibble from the Btns pair +const char * const PrusaErrorButtonTitle(uint8_t bi); + +/// @returns pointer to a PROGMEM string representing the "More" button +const char * const PrusaErrorButtonMore(); + +} // namespace MMU2 diff --git a/Firmware/mmu2_reporting.cpp b/Firmware/mmu2_reporting.cpp index d3b8523c6..119e0cb61 100644 --- a/Firmware/mmu2_reporting.cpp +++ b/Firmware/mmu2_reporting.cpp @@ -1,8 +1,9 @@ #include "mmu2_reporting.h" #include "mmu2_error_converter.h" #include "mmu2/error_codes.h" -#include "mmu2/errors_list.h" +#include "mmu2/buttons.h" #include "ultralcd.h" +#include "language.h" namespace MMU2 { @@ -26,13 +27,13 @@ void ReportErrorHook(CommandInProgress cip, uint16_t ec) { //! |prusa3d.com/ERR04504| <- URL 20 characters //! | | <- empty line //! |>Retry >Done >MoreW| <- buttons - const uint16_t ei = MMUErrorCodeIndex((uint16_t)ec); + const uint8_t ei = PrusaErrorCodeIndex(ec); uint8_t choice_selected = 0; bool two_choices = false; // Read and determine what operations should be shown on the menu // Note: uint16_t is used here to avoid compiler warning. uint8_t is only half the size of void* - const uint8_t button_operation = reinterpret_cast(const_cast(pgm_read_ptr(&errorButtons[ei]))); + const uint8_t button_operation = PrusaErrorButtons(ei); const uint8_t button_high_nibble = BUTTON_OP_HI_NIBBLE(button_operation); const uint8_t button_low_nibble = BUTTON_OP_LO_NIBBLE(button_operation); @@ -46,23 +47,18 @@ void ReportErrorHook(CommandInProgress cip, uint16_t ec) { back_to_choices: lcd_clear(); lcd_update_enable(false); - + // Print title and header - lcd_printf_P(PSTR("%S\nprusa3d.com/ERR04%hu"), - static_cast(pgm_read_ptr(&errorTitles[ei])), - reinterpret_cast(const_cast(pgm_read_ptr(&errorCodes[ei]))) - ); + lcd_printf_P(PSTR("%S\nprusa3d.com/ERR04%hu"), _T(PrusaErrorTitle(ei)), PrusaErrorCode(ei) ); // Render the choices and store selection in 'choice_selected' choice_selected = lcd_show_multiscreen_message_with_choices_and_wait_P( NULL, // NULL, since title screen is not in PROGMEM false, two_choices ? LEFT_BUTTON_CHOICE : MIDDLE_BUTTON_CHOICE, - static_cast(pgm_read_ptr(&btnOperation[button_low_nibble - 1])), - two_choices ? - btnMore - : static_cast(pgm_read_ptr(&btnOperation[button_high_nibble - 1])), - two_choices ? nullptr : btnMore, + _T(PrusaErrorButtonTitle(button_low_nibble - 1)), + _T(two_choices ? PrusaErrorButtonMore() : PrusaErrorButtonTitle(button_high_nibble - 1)), + two_choices ? nullptr : _T(PrusaErrorButtonMore()), two_choices ? 10 // If two choices, allow the first choice to have more characters : 7 @@ -72,9 +68,7 @@ back_to_choices: || (!two_choices && choice_selected == RIGHT_BUTTON_CHOICE)) // Three choices and right most button selected { // 'More' show error description - lcd_show_fullscreen_message_and_wait_P( - static_cast(pgm_read_ptr(&errorDescs[ei])) - ); + lcd_show_fullscreen_message_and_wait_P(_T(PrusaErrorDesc(ei))); // Return back to the choice menu goto back_to_choices; @@ -115,7 +109,7 @@ back_to_choices: void ReportProgressHook(CommandInProgress cip, uint16_t ec) { custom_message_type = CustomMsg::MMUProgress; - lcd_setstatuspgm( ProgressCodeToText(ec) ); + lcd_setstatuspgm( _T(ProgressCodeToText(ec)) ); } Buttons ButtonPressed(uint16_t ec) { diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index f46fee31a..80df94763 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -3233,8 +3233,8 @@ void lcd_show_choices_prompt_P(uint8_t selected, const char *first_choice, const //! @retval 1 first choice selected by user //! @retval 2 third choice selected by user //! @retval -1 screen timed out (only possible if allow_timeouting is true) -int8_t lcd_show_multiscreen_message_with_choices_and_wait_P(const char *msg, bool allow_timeouting, uint8_t default_selection, - const char *first_choice, const char *second_choice, const char *third_choice, uint8_t second_col) +int8_t lcd_show_multiscreen_message_with_choices_and_wait_P(const char * const msg, bool allow_timeouting, uint8_t default_selection, + const char * const first_choice, const char * const second_choice, const char * const third_choice, uint8_t second_col) { const char *msg_next = msg ? lcd_display_message_fullscreen_P(msg) : NULL; bool multi_screen = msg_next != NULL; diff --git a/Firmware/ultralcd.h b/Firmware/ultralcd.h index 59819bfd9..86d785217 100755 --- a/Firmware/ultralcd.h +++ b/Firmware/ultralcd.h @@ -82,8 +82,8 @@ extern void lcd_show_fullscreen_message_and_wait_P(const char *msg); extern int8_t lcd_show_yes_no_and_wait(bool allow_timeouting = true, uint8_t default_selection = MIDDLE_BUTTON_CHOICE); // 1: no, 0: yes, -1: timeouted extern int8_t lcd_show_fullscreen_message_yes_no_and_wait_P(const char *msg, bool allow_timeouting = true, uint8_t default_selection = MIDDLE_BUTTON_CHOICE); -extern int8_t lcd_show_multiscreen_message_with_choices_and_wait_P(const char *msg, bool allow_timeouting, uint8_t default_selection, - const char *first_choice, const char *second_choice, const char *third_choice = nullptr, uint8_t second_col = 7); +extern int8_t lcd_show_multiscreen_message_with_choices_and_wait_P(const char * const msg, bool allow_timeouting, uint8_t default_selection, + const char * const first_choice, const char * const second_choice, const char * const third_choice = nullptr, uint8_t second_col = 7); extern int8_t lcd_show_multiscreen_message_yes_no_and_wait_P(const char *msg, bool allow_timeouting = true, uint8_t default_selection = MIDDLE_BUTTON_CHOICE); // Ask the user to move the Z axis up to the end stoppers and let // the user confirm that it has been done.