Marlin: Garbage displayed at the beginning of the Mesh leveling routine on LCD

[WheresWaldo]

Symptoms:

When you start the Level Bed sequence using the LCD smart controller or the Full Graphics LCD smart Controller at the initial screen while homing the X, Y and Z axes you get a line of garbage. It is the second line of two displayed. Once homes and you are at the first probe location the screen is normal. The process does appear to function 100% reliably but for the initial display.

Tested with both RC4 and RC4BugFix form Sunday March 27

Changes to the default _Configuration.h_ from the LCD section include:

#define SDSUPPORT
#define LCD_FEEDBACK_FREQUENCY_DURATION_MS 100
#define LCD_FEEDBACK_FREQUENCY_HZ 1000
#define REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER

I have also tested this by changing the following lines from:

  #define DISPLAY_CHARSET_HD44780_JAPAN
  //#define DISPLAY_CHARSET_HD44780_WESTERN
  //#define DISPLAY_CHARSET_HD44780_CYRILLIC

to

 //#define DISPLAY_CHARSET_HD44780_JAPAN
  #define DISPLAY_CHARSET_HD44780_WESTERN
  //#define DISPLAY_CHARSET_HD44780_CYRILLIC

with no difference in the results and also tried it with the following line both enabled and commented out with no changes:

//#define ULTRA_LCD
//#define DOGLCD

I am not 100% sure this is firmware or hardware related, others in the Robo forum have indicated to me the same issue (different LCD controllers).

[jbrazio]

This errant behavior is only found on RC4/BugFix ?

[epatel]

Think it should write "XYZ Homing"
https://github.com/MarlinFirmware/Marlin/blob/RC/Marlin/ultralcd.cpp#L2454

[epatel]

Using different language than English? that has a different string for MSG_LEVEL_BED_HOMING (i.e. longer than Homing)

[WheresWaldo]

English
Both RC4 and RC4BugFix
and the line "XYZ Homing" displays properly, under it is a line of graphical garbage.

[Blue-Marlin]

As far i can see lcd_implementation_drawedit() is not made for PSTR() at the second parameter. (editing a PSTR() is impossible)
Try:

-  if (lcdDrawUpdate) lcd_implementation_drawedit(PSTR("XYZ"), PSTR(MSG_LEVEL_BED_HOMING));
+ if (lcdDrawUpdate) lcd_implementation_drawedit(PSTR("XYZ"), MSG_LEVEL_BED_HOMING);

If that works also correct line 539.

[Roxy-3D]

As far i can see lcd_implementation_drawedit() is not made for PSTR() at the second parameter. (editing a PSTR() is impossible)

Yes. And there is another interesting point here. Even though lcd_implementation_drawedit() should not need to change the strings passed into it, it won't work as a PSTR(). The reason is because the AVR is a 'Harvard' architecture. The generated code needs to do different things to get at addresses in the Flash area compared to RAM address. It is common in 'Havard' architecture processors to have the same address in all of the memory banks. For example, address 0x0000 exists in the EEPROM, the Flash and the RAM area.

lcd_implementation_drawedit() needs to know which area it is getting the data from in order for GCC to generate the correct code.

[epatel]

Not sure where that PSTR came from. I did not add that...AFAIK

[WheresWaldo]

It is in the actual source, I just check it here, line 2480 of RC4BugFix ultralcd.cpp, I can test if that simple change fixes it in a couple of hours, in the middle of a print at the moment.

[epatel]

Added 10 days ago
https://github.com/MarlinFirmware/Marlin/commit/c97c2dcaf736a83ec7a00ec714ec40a51937621a

[Roxy-3D]

If we are going to use:

if (lcdDrawUpdate) lcd_implementation_drawedit(PSTR("XYZ"), PSTR(MSG_LEVEL_BED_HOMING));

I think we need to do two things. First, the 2nd parameter in the lcd_implementation_drawedit() calls needs to be changed to PSTR("???") everywhere. And then the last line of lcd_implementation_drawedit() needs to be changed to something like this:

void lcd_implementation_drawedit(const char* pstr, char* value) {
  lcd.setCursor(1, 1);
  lcd_printPGM(pstr);
  lcd.print(':');
  lcd.setCursor(LCD_WIDTH - lcd_strlen(value), 1);
//  lcd_print(value);
  lcd_printPGM(value);
}

I think there is a limitation on the amount of PSTR() memory we can define. (I don't know that this is true!!!) If so, we may want to go the opposite route and just make lcd_implementation_drawedit() take RAM parameters instead.

[Blue-Marlin]

It would also need to replace lcd_strlen() with lcd_strlen_P().

But this is mainly used to handle strings resulting from number->string-conversions. There is no chance to push them to promgem.
It would need a additional lcd_implementation_drawedit_P() function. But the name would be misleading, because there is noting to edit. Up to now there are only two uses with PSTR(). Up to now it's cheaper to use simple strings than having the new function.

[Roxy-3D]

Thank you for pointing all of that out! Yes, the lcd_strlen() would have to change also.

Do you know if there is a limitation on how much program memory can be used for const char strings? The reason I think there is a limitation is if I add too many lines of debugging code with:

SERIAL_ECHOPGM("Debug Text\n");  

everything breaks. And just removing some of the debug lines I don't need any more lets me continue. And I know I haven't filled up the flash memory with firmware. So that kind of leads me to think there is a limitation on how much program memory can be filled with const char stuff.

[thinkyhead]

Good catch. It got lost in translation that const and PSTR are not intrinsically connected. #3285 should take care of this. Note that the string MSG_LEVEL_BED_HOMING won't be stored in PROGMEM because of this change. But it could be, using the following code, which might make a good general function (make a temp copy of a PROGMEM string that can be passed to functions that expect non-PSTR parameters)…

const char msg_level_bed_homing[] PROGMEM = MSG_LEVEL_BED_HOMING;
if (lcdDrawUpdate) {
  char homing_str[strlen_P(msg_level_bed_homing) + 1];
  strcpy_P(homing_str, (char*)pgm_read_word(&(msg_level_bed_homing)));
  lcd_implementation_drawedit(PSTR("XYZ"), homing_str);
}

[thinkyhead]

This issue is fixed with #3285 – now merged.