From 63775dfabb97608c63ec2470ff5ae2edac9125c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Mon, 13 Mar 2023 20:33:29 +0000 Subject: [PATCH 1/5] PFW-1504 fix conflicting UI issues Proposal to fix some of the issues with the initial implementation it is safer to use the status line code to print the message so there aren't any conflicts in the LCD cursor position. Allow inserting a byte into any position in the LCD status message Also, add a variable to control from which index in the array should the message start printing. This is very useful for progress bars and messages which continually update. I think we can save some memory by applying this to Mesh Bed Leveling later. Change in memory: Flash: +106 bytes SRAM: +1 byte --- Firmware/lcd.cpp | 4 ++-- Firmware/lcd.h | 2 +- Firmware/mmu2.cpp | 1 + Firmware/mmu2_reporting.cpp | 17 ++++++++------- Firmware/mmu2_reporting.h | 3 +++ Firmware/ultralcd.cpp | 42 ++++++++++++++++++++++++++++++------- Firmware/ultralcd.h | 3 +++ 7 files changed, 55 insertions(+), 17 deletions(-) diff --git a/Firmware/lcd.cpp b/Firmware/lcd.cpp index 5715ca97d..204ba73e9 100644 --- a/Firmware/lcd.cpp +++ b/Firmware/lcd.cpp @@ -546,14 +546,14 @@ void lcd_print(const char* s) while (*s) lcd_write(*(s++)); } -char lcd_print_pad(const char* s, uint8_t len) +uint8_t lcd_print_pad(const char* s, uint8_t len) { while (len && *s) { lcd_write(*(s++)); --len; } lcd_space(len); - return *s; + return len; } uint8_t lcd_print_pad_P(const char* s, uint8_t len) diff --git a/Firmware/lcd.h b/Firmware/lcd.h index 2a4682739..da509de07 100644 --- a/Firmware/lcd.h +++ b/Firmware/lcd.h @@ -56,7 +56,7 @@ extern void lcd_space(uint8_t n); extern void lcd_printNumber(unsigned long n, uint8_t base); extern void lcd_print(const char*); -extern char lcd_print_pad(const char* s, uint8_t len); +extern uint8_t lcd_print_pad(const char* s, uint8_t len); /// @brief print a string from PROGMEM with left-adjusted padding /// @param s string from PROGMEM. diff --git a/Firmware/mmu2.cpp b/Firmware/mmu2.cpp index 6969bb5c5..b55b18080 100644 --- a/Firmware/mmu2.cpp +++ b/Firmware/mmu2.cpp @@ -305,6 +305,7 @@ bool MMU2::VerifyFilamentEnteredPTFE() { } Disable_E0(); + TryLoadUnloadProgressbarDeinit(); if (fsensorState) { IncrementLoadFails(); diff --git a/Firmware/mmu2_reporting.cpp b/Firmware/mmu2_reporting.cpp index 09c62f4a8..1959133ef 100644 --- a/Firmware/mmu2_reporting.cpp +++ b/Firmware/mmu2_reporting.cpp @@ -283,16 +283,19 @@ void ReportProgressHook(CommandInProgress cip, uint16_t ec) { } void TryLoadUnloadProgressbarInit() { - // Clear the status line - lcd_set_cursor(0, 3); - lcd_space(LCD_WIDTH); + lcd_clearstatus(); +} + +void TryLoadUnloadProgressbarDeinit() { + // Delay the next status message just so + // the user can see the results clearly + lcd_reset_status_message_timeout(); + lcd_clearstatus(); } void TryLoadUnloadProgressbar(uint8_t col, bool sensorState) { - // Set the cursor position each time in case some other - // part of the firmware changes the cursor position - lcd_putc_at(col, 3, sensorState ? '-' : LCD_STR_SOLID_BLOCK[0]); - lcd_reset_status_message_timeout(); + lcd_insert_char_into_status(col, sensorState ? '-' : LCD_STR_SOLID_BLOCK[0]); + if (!lcd_update_enabled) lcdui_print_status_line(); } void IncrementLoadFails(){ diff --git a/Firmware/mmu2_reporting.h b/Firmware/mmu2_reporting.h index e03b15ddd..8fedc19d4 100644 --- a/Firmware/mmu2_reporting.h +++ b/Firmware/mmu2_reporting.h @@ -38,6 +38,9 @@ void ReportProgressHook(CommandInProgress cip, uint16_t ec); /// @brief Clear the status line and setup the LCD cursor void TryLoadUnloadProgressbarInit(); +/// @brief Clear the status line and setup the LCD cursor +void TryLoadUnloadProgressbarDeinit(); + /// @brief Add one block to the progress bar /// @param col pixel position on the LCD status line, should range from 0 to (LCD_WIDTH - 1) /// @param sensorState if true, filament is not present, else filament is present. This controls which character to render diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index dbcd2edb9..d8ee515a3 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -78,6 +78,7 @@ static float manual_feedrate[] = MANUAL_FEEDRATE; /* LCD message status */ static LongTimer lcd_status_message_timeout; static uint8_t lcd_status_message_level; +static uint8_t lcd_status_message_idx = 0; static char lcd_status_message[LCD_WIDTH + 1]; /* !Configuration settings */ @@ -542,7 +543,7 @@ void lcdui_print_status_line(void) { { // If printing from SD, show what we are printing const char* longFilenameOLD = (card.longFilename[0] ? card.longFilename : card.filename); - if( lcd_print_pad(&longFilenameOLD[scrollstuff], LCD_WIDTH) ) + if( lcd_print_pad(&longFilenameOLD[scrollstuff], LCD_WIDTH) == 0) { scrollstuff++; } else { @@ -556,13 +557,18 @@ void lcdui_print_status_line(void) { return; // Nothing to do, waiting for delay to expire } + lcd_set_cursor(lcd_status_message_idx, 3); + switch (custom_message_type) { case CustomMsg::M117: // M117 Set the status line message on the LCD case CustomMsg::Status: // Nothing special, print status message normally case CustomMsg::M0Wait: // M0/M1 Wait command working even from SD case CustomMsg::FilamentLoading: // If loading filament, print status case CustomMsg::MMUProgress: // MMU Progress Codes - lcd_print_pad(lcd_status_message, LCD_WIDTH); + { + const uint8_t padding = lcd_print_pad(&lcd_status_message[lcd_status_message_idx], LCD_WIDTH - lcd_status_message_idx); + lcd_status_message_idx = LCD_WIDTH - padding; + } break; case CustomMsg::MeshBedLeveling: // If mesh bed leveling in progress, show the status if (custom_message_state > 10) { @@ -684,6 +690,12 @@ void lcdui_print_status_screen(void) } +static void lcdui_refresh(uint8_t clear = true) +{ + clear ? lcd_refresh() : lcd_refresh_noclear(); + lcd_status_message_idx = 0; // Re-draw message from beginning +} + // Main status screen. It's up to the implementation specific part to show what is needed. As this is very display dependent void lcd_status_screen() // NOT static due to using inside "Marlin_main" module ("manage_inactivity()") { @@ -727,13 +739,15 @@ void lcd_status_screen() // NOT static due to using ins ReInitLCD++; if (ReInitLCD == 30) { - lcd_refresh(); // to maybe revive the LCD if static electricity killed it. ReInitLCD = 0 ; + lcdui_refresh(); } else { if ((ReInitLCD % 10) == 0) - lcd_refresh_noclear(); //to maybe revive the LCD if static electricity killed it. + { + lcdui_refresh(false); //to maybe revive the LCD if static electricity killed it. + } } lcdui_print_status_screen(); @@ -988,7 +1002,7 @@ void lcd_commands() void lcd_return_to_status() { - lcd_refresh(); // to maybe revive the LCD if static electricity killed it. + lcdui_refresh(false); // to maybe revive the LCD if static electricity killed it. menu_goto(lcd_status_screen, 0, true); menu_depth = 0; eFilamentAction = FilamentAction::None; // i.e. non-autoLoad @@ -5759,7 +5773,7 @@ void lcd_sdcard_menu() if (_md->isDir) lcd_print(LCD_STR_FOLDER[0]); - if( lcd_print_pad(&_md->scrollPointer[_md->offset], len) ) + if( lcd_print_pad(&_md->scrollPointer[_md->offset], len) == 0) { _md->lcd_scrollTimer.start(); _md->offset++; @@ -7039,6 +7053,7 @@ static void lcd_updatestatus(const char *message, bool progmem = false) strncpy(lcd_status_message, message, LCD_WIDTH); lcd_status_message[LCD_WIDTH] = 0; + lcd_status_message_idx = 0; // Print message from beginning SERIAL_PROTOCOLLNRPGM(MSG_LCD_STATUS_CHANGED); @@ -7052,6 +7067,19 @@ void lcd_setstatus(const char* message) lcd_updatestatus(message); } +void lcd_insert_char_into_status(uint8_t position, const char message) +{ + if (position > LCD_WIDTH - 1) return; + lcd_status_message[position] = message; + lcd_draw_update = 1; // force redraw +} + +void lcd_clearstatus() +{ + memset(lcd_status_message, 0, sizeof(lcd_status_message)); + lcd_status_message_idx = 0; +} + void lcd_setstatuspgm(const char* message) { if (lcd_message_check(LCD_STATUS_NONE)) @@ -7243,7 +7271,7 @@ void menu_lcd_lcdupdate_func(void) lcd_return_to_status(); lcd_draw_update = 2; } - if (lcd_draw_update == 2) lcd_clear(); + if (lcd_draw_update == 2) lcdui_refresh(); if (lcd_draw_update) lcd_draw_update--; lcd_next_update_millis = _millis() + LCD_UPDATE_INTERVAL; } diff --git a/Firmware/ultralcd.h b/Firmware/ultralcd.h index 4c1b92729..ca917e96c 100755 --- a/Firmware/ultralcd.h +++ b/Firmware/ultralcd.h @@ -20,6 +20,9 @@ void ultralcd_init(); #define LCD_STATUS_DELAYED_TIMEOUT 4000 // Set the current status message (equivalent to LCD_STATUS_NONE) +void lcdui_print_status_line(void); +void lcd_clearstatus(); +void lcd_insert_char_into_status(uint8_t position, const char message); void lcd_setstatus(const char* message); void lcd_setstatuspgm(const char* message); void lcd_setstatus_serial(const char* message); From 28f6cebfd2bcbce4cd2c787af2b129ed8f2a6bd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Tue, 14 Mar 2023 21:35:52 +0000 Subject: [PATCH 2/5] PFW-1504 Cleanup Only set cursor with lcd_status_message_idx where the variable is used. No change in memory --- Firmware/ultralcd.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index d8ee515a3..10f1187b8 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -557,8 +557,6 @@ void lcdui_print_status_line(void) { return; // Nothing to do, waiting for delay to expire } - lcd_set_cursor(lcd_status_message_idx, 3); - switch (custom_message_type) { case CustomMsg::M117: // M117 Set the status line message on the LCD case CustomMsg::Status: // Nothing special, print status message normally @@ -566,6 +564,7 @@ void lcdui_print_status_line(void) { case CustomMsg::FilamentLoading: // If loading filament, print status case CustomMsg::MMUProgress: // MMU Progress Codes { + lcd_set_cursor(lcd_status_message_idx, 3); const uint8_t padding = lcd_print_pad(&lcd_status_message[lcd_status_message_idx], LCD_WIDTH - lcd_status_message_idx); lcd_status_message_idx = LCD_WIDTH - padding; } From 940e626f3ad6172037acbf6d0530dc2c658ce7a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Tue, 14 Mar 2023 22:09:15 +0000 Subject: [PATCH 3/5] Previously we called cleared the LCD, no need to change that Change in memory: Flash: -6 bytes SRAM: 0 bytes --- Firmware/ultralcd.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 10f1187b8..7e3c68757 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -1001,7 +1001,7 @@ void lcd_commands() void lcd_return_to_status() { - lcdui_refresh(false); // to maybe revive the LCD if static electricity killed it. + lcdui_refresh(); // to maybe revive the LCD if static electricity killed it. menu_goto(lcd_status_screen, 0, true); menu_depth = 0; eFilamentAction = FilamentAction::None; // i.e. non-autoLoad From 20c6a448fad0fded4c3b0d6bf34d14686662d5d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Sun, 2 Apr 2023 11:13:39 +0000 Subject: [PATCH 4/5] PFW-1504 Don't clear Status line on Deinit Status line code should decide what to render next Change in memory: Flash: -12 bytes SRAM: 0 bytes --- Firmware/mmu2_reporting.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Firmware/mmu2_reporting.cpp b/Firmware/mmu2_reporting.cpp index 1959133ef..31ded00e3 100644 --- a/Firmware/mmu2_reporting.cpp +++ b/Firmware/mmu2_reporting.cpp @@ -290,7 +290,6 @@ void TryLoadUnloadProgressbarDeinit() { // Delay the next status message just so // the user can see the results clearly lcd_reset_status_message_timeout(); - lcd_clearstatus(); } void TryLoadUnloadProgressbar(uint8_t col, bool sensorState) { From 7e025894d1a1bcec846e407fea700640f857e5a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0ni=20M=C3=A1r=20Gilbert?= Date: Sat, 29 Apr 2023 17:10:33 +0000 Subject: [PATCH 5/5] Echo the result onto serial Example: MMU2:1111111111111110011 1 means filament present (solid block) 0 means otherwise (dash) Change in memory: Flash: +94 bytes SRAM: 0 bytes --- Firmware/mmu2.cpp | 1 + Firmware/mmu2_reporting.cpp | 12 ++++++++++++ Firmware/mmu2_reporting.h | 3 +++ Firmware/ultralcd.cpp | 4 ++++ Firmware/ultralcd.h | 4 ++++ 5 files changed, 24 insertions(+) diff --git a/Firmware/mmu2.cpp b/Firmware/mmu2.cpp index b55b18080..37bb72d6a 100644 --- a/Firmware/mmu2.cpp +++ b/Firmware/mmu2.cpp @@ -305,6 +305,7 @@ bool MMU2::VerifyFilamentEnteredPTFE() { } Disable_E0(); + TryLoadUnloadProgressbarEcho(); TryLoadUnloadProgressbarDeinit(); if (fsensorState) { diff --git a/Firmware/mmu2_reporting.cpp b/Firmware/mmu2_reporting.cpp index 31ded00e3..ea14fdbea 100644 --- a/Firmware/mmu2_reporting.cpp +++ b/Firmware/mmu2_reporting.cpp @@ -1,4 +1,5 @@ #include "mmu2.h" +#include "mmu2_log.h" #include "mmu2_reporting.h" #include "mmu2_error_converter.h" #include "mmu2/error_codes.h" @@ -292,6 +293,17 @@ void TryLoadUnloadProgressbarDeinit() { lcd_reset_status_message_timeout(); } +void TryLoadUnloadProgressbarEcho() { + char buf[LCD_WIDTH]; + lcd_getstatus(buf); + for (uint8_t i = 0; i < sizeof(buf); i++) { + // 0xFF is -1 when converting from unsigned to signed char + // If the number is negative, that means filament is present + buf[i] = (buf[i] < 0) ? '1' : '0'; + } + MMU2_ECHO_MSGLN(buf); +} + void TryLoadUnloadProgressbar(uint8_t col, bool sensorState) { lcd_insert_char_into_status(col, sensorState ? '-' : LCD_STR_SOLID_BLOCK[0]); if (!lcd_update_enabled) lcdui_print_status_line(); diff --git a/Firmware/mmu2_reporting.h b/Firmware/mmu2_reporting.h index 8fedc19d4..071dd545e 100644 --- a/Firmware/mmu2_reporting.h +++ b/Firmware/mmu2_reporting.h @@ -41,6 +41,9 @@ void TryLoadUnloadProgressbarInit(); /// @brief Clear the status line and setup the LCD cursor void TryLoadUnloadProgressbarDeinit(); +/// @brief Report the results to serial +void TryLoadUnloadProgressbarEcho(); + /// @brief Add one block to the progress bar /// @param col pixel position on the LCD status line, should range from 0 to (LCD_WIDTH - 1) /// @param sensorState if true, filament is not present, else filament is present. This controls which character to render diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 7e3c68757..c06196f09 100644 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -7079,6 +7079,10 @@ void lcd_clearstatus() lcd_status_message_idx = 0; } +void lcd_getstatus(char buf[LCD_WIDTH]) { + strncpy(buf, lcd_status_message, LCD_WIDTH); +} + void lcd_setstatuspgm(const char* message) { if (lcd_message_check(LCD_STATUS_NONE)) diff --git a/Firmware/ultralcd.h b/Firmware/ultralcd.h index ca917e96c..c340fef95 100755 --- a/Firmware/ultralcd.h +++ b/Firmware/ultralcd.h @@ -22,6 +22,10 @@ void ultralcd_init(); // Set the current status message (equivalent to LCD_STATUS_NONE) void lcdui_print_status_line(void); void lcd_clearstatus(); + +/// @brief Copy the contents of lcd_status_message +/// @param buf destination buffer +void lcd_getstatus(char buf[LCD_WIDTH]); void lcd_insert_char_into_status(uint8_t position, const char message); void lcd_setstatus(const char* message); void lcd_setstatuspgm(const char* message);