Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scrolling status message (option) #6877

Merged

Conversation

thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented May 28, 2017

Handle status messages as UTF strings.

Add an option STATUS_MESSAGE_SCROLLING to allow an excess length status message to be set. The status message will be scrolled on each screen update if it's wider than the display.

@Tannoo
Copy link
Contributor

Tannoo commented May 28, 2017

Can you also set this to handle some long machine names on the status screen?

@Roxy-3D
Copy link
Member

Roxy-3D commented May 28, 2017

Can you also set this to handle some long machine names on the status screen?

And more importantly... Long SD-Card file names!!!!! It really wouldn't be hard to do the whole display. The big problem is all the extra RAM memory so the whole display can scroll.

@Tannoo
Copy link
Contributor

Tannoo commented May 28, 2017

Or maybe just the selected file do the scrolling.

@@ -5882,7 +5881,7 @@ inline void gcode_M17() {
KEEPALIVE_STATE(IN_HANDLER);
}

static void resume_print(const float& load_length = 0, const float& initial_extrude_length = 0, int max_beep_count = 0) {
static void resume_print(const float& load_length = 0, const float& initial_extrude_length = 0, int8_t max_beep_count = 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the &s be moved to be next to the variable name since you made that change to pause_print?

static bool pause_print(const float& retract, const float& z_lift, const float& x_pos, const float& y_pos,
const float& unload_length = 0 , int max_beep_count = 0, bool show_lcd = false) {
static bool pause_print(const float &retract, const float &z_lift, const float &x_pos, const float &y_pos,
const float &unload_length = 0 , int8_t max_beep_count = 0, bool show_lcd = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the &s to next to the variable names doesn't seem to match the code style for the rest of the file

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Search turns up 181 matches for float &, only 30 for float&. So float & wins.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this same standard also be applied to pointers?

@thinkyhead
Copy link
Member Author

thinkyhead commented May 28, 2017

Anything set on the status line that is too long to fit will scroll. Long filenames are another thing. The menu system is optimized to prevent interference with printing, and in order to add a scrolling filename there would be more interruption for screen painting. On character displays this isn't too much, but would be best handled as a special side function so it doesn't require a full screen repaint. On graphical displays, you must do a full screen repaint to scroll even a single line of text. In either case, a single line drawing function would need to be added as an augmentation the SD file listing code. It's a bit more work than this feature.

@Roxy-3D
Copy link
Member

Roxy-3D commented May 28, 2017

Maybe the file selection menu logic could also copy the currently selected file name to the status line also? Because that would provide a place to see what the file name really was as it scrolled?

@thinkyhead
Copy link
Member Author

The message line only appears on the info screen.

@thinkyhead thinkyhead merged commit 5855699 into MarlinFirmware:bugfix-1.1.x May 28, 2017
@thinkyhead thinkyhead deleted the bf_scrolling_status branch May 28, 2017 19:12
@tcm0116
Copy link
Contributor

tcm0116 commented May 31, 2017

I'm having a few issues with this option on my 20x4 LCD:

  1. When a short, non-scrolling message is displayed after a scrolling message, sometimes the tail end of the longer message is still displayed.
  2. Rotating the encoder changes the rate at which the text is scrolled.

@Roxy-3D
Copy link
Member

Roxy-3D commented May 31, 2017

When a short, non-scrolling message is displayed after a scrolling message, sometimes the tail end of the longer message is still displayed.

I have seen this. This is real. This shouldn't be too hard to find and fix. It may be we should clear out the tail end of the string with spaces.

I'll put a 'Bug' label on this thread.

void lcd_print(const char c) { charset_mapper(c); }
void lcd_printPGM_utf(const char* str, const uint8_t maxLength=LCD_WIDTH) {
char c;
for (uint8_t i = 0, n = maxLength; n && (c = str[i]); ++i)
Copy link
Contributor

@tcm0116 tcm0116 May 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be:

for (uint8_t n = maxLength; n && (c = pgm_read_byte(str)); ++str)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Patching now…


void lcd_finishstatus(bool persist=false) {
set_utf_strlen(lcd_status_message, LCD_WIDTH);
#if DISABLED(STATUS_MESSAGE_SCROLLING)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be why we're seeing the end of the last message being displayed when a shorter message is displayed. Prior to this change, set_utf_strlen always filled the lcd_status_message with spaces to ensure it was at least LCD_WIDTH characters long. However, now that set_utf_strlen is only being called if STATUS_MESSAGE_SCROLLING is disabled, messages shorter than LCD_WIDTH are not lengthened with spaces.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made a patch that fills up the line with spaces, but doesn't chop off the string as with the non-scrolling version. Will merge soon!

@thinkyhead
Copy link
Member Author

#7030 makes the scrolling circular, which is much better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants