Load settings after showing bootscreen

[dbuezas]

Description

In my I2C LCD, the screen shows noise for a couple of seconds when powered on. This is because the LCD is turned on but nothing is drawn until the eeprom settings are loaded (which takes a couple of seconds).

Showing the bootscreen before loading eeprom solves this glitch and it also reduces total load time, since the boot animation can run concurrently with the eeprom read.

Requirements

Benefits

Configurations

Related Issues

[ellensp]

What about lcds that have a settable contrast and brightness in eeprom?

  //
  // HAS_LCD_CONTRAST
  //
  uint8_t lcd_contrast;                                 // M250 C

  //
  // HAS_LCD_BRIGHTNESS
  //
  uint8_t lcd_brightness;                               // M256 B

[dbuezas]

At least in my screen it gets corrected right after. It is still a lot better than before, let me make a video

[ellensp]

Im concerned about everyone screen, not just yours.

For some this will result in a non visible or overly bright boot screen

[dbuezas]

If you turn on the sound, you can also hear the probe doing it startup routine (automatic when power is turned on), and one can also see that the screen finishes loading quicker.

Before

before.mp4

After

after.mp4

[dbuezas]

Im concerned about everyone screen, not just yours.

Good point, I didn't see any reasons for concern myself, but I also only know my machine so happy to hear feedback.
I was hoping the improvement may generalise to others, but understand that may not be the case.

[thisiskeithb]

What about lcds that have a settable contrast and brightness in eeprom?

That's the exact thing I was going to mention as well. If the user's preferred values are stored in EEPROM, then this PR is a step backward for them.

[dbuezas]

Do other LCDs also show noise at startup?
Maybe it would be best to add a call to paint a blank screen right before after initing the LCD to clear the ram.

[thisiskeithb]

Do other LCDs also show noise at startup?

Some do, some don't.

[thinkyhead]

Maybe it would be best to add a call to paint a blank screen right before after initing the LCD to clear the ram.

See if adding a call to clear_lcd() at the end of MarlinUI::init_lcd eliminates the noise.

Some LCDs have a shared SPI with the SD card, and this can sometimes generate extra noise, but it could also be something like a floating pin. Earlier initialization of all pin modes could fix it, but in the meantime a quick clear after init might be enough.

[ellensp]

An option might be to read just the lcd settings form the eeprom before the boot screen when required, then the full eeprom read later

[dbuezas]

See if adding a call to clear_lcd() at the end of MarlinUI::init_lcd eliminates the noise.

I tried that and unfortunately it had no effect, looking closer, DOGM implementation is a noop void MarlinUI::clear_lcd() { } // Automatically cleared by Picture Loop

[dbuezas]

An option might be to read just the lcd settings form the eeprom before the boot screen when required, then the full eeprom read later

Since the eeprom is not indexed, we'd need to run all the load routines to increment the address to the right one.

I think clearing the screen first would be a good compromise: eeprom read won't happen in parallel to the bootscreen animation, but at least no noise. The issue is that not all LCDs implement the clear_lcd function.

I do wonder if having the default brightness/contrast for a couple of seconds is worse than spending the same time with a garbled screen. The latter looks very ugly/broken (and also runs with default brightness/contrast).

[dbuezas]

Another easy fix could be to immediately send the lcd to sleep while the eeprom is read, and wake it up right after, although I don't know if all lcds will take configurations while asleep. Anybody knows?

[thisiskeithb]

I do wonder if having the default brightness/contrast for a couple of seconds is worse than spending the same time with a garbled screen. The latter looks very ugly/broken (and also runs with default brightness/contrast).

While both are bugs and only visible briefly during the boot process, potential garbage on the LCD (with proper brightness & contrast) is better than a screen without a visible bootscreen/screens.

Another easy fix could be to immediately send the lcd to sleep while the eeprom is read, and wake it up right after, although I don't know if all lcds will take configurations while asleep. Anybody knows?

Most displays don't support the sleep feature and display sleep is disabled by default.

[dbuezas]

I see.

potential garbage on the LCD (with proper brightness & contrast)

Actually in my display the garbage is shown at whatever default contrast/brightness the lcd has.

[thinkyhead]

How about the commits I just added: do these help?

[dbuezas]

How about the commits I just added: do these help?

No more noise, but the boot animation is also gone

[dbuezas]

Note: I have a zero bootscreen timeout so eeprom loading ends almost at the same time as the animation and I get the snappiest boot.

    #define BOOTSCREEN_TIMEOUT 0
    #define BOOT_MARLIN_LOGO_ANIMATED

Trying with 2000 now to check if only the animation is gone or also the bootscreen

[dbuezas]

Disabling boot animation doesn't bring back the boot screen either

[thinkyhead]

Any difference noted by moving the clear_lcd() call to different points within MarlinUI::init_lcd ?

[thinkyhead]

I moved the call to MarlinUI::init after the call to init_lcd(), so all LCDs will get cleared immediately after device init.

[dbuezas]

What happens when MarlinSettings::load_contrast is called for an incompatible or uninitialized EEPROM?

If the version check doesn't pass then nothing happens.

I guess the worse that can happen is that the eeprom is corrupted and the lcd contrast is initialized to zero.

[ellensp]

If eeprom version doesn't match we could default to LCD_CONTRAST_DEFAULT and LCD_BRIGHTNESS_DEFAULT

Same as in settings.cpp

  //
  // LCD Contrast
  //
  TERN_(HAS_LCD_CONTRAST, ui.contrast = LCD_CONTRAST_DEFAULT);

  //
  // LCD Brightness
  //
  TERN_(HAS_LCD_BRIGHTNESS, ui.brightness = LCD_BRIGHTNESS_DEFAULT);

[dbuezas]

I think it is already initialized to that, but we could reset them to default if the crc doesn't match later in the full eeprom read. Then the only side effect a corrupted eeprom would have is a couple of seconds with random contrast

[thinkyhead]

In adapting the suggested changes I did apply the defaults for the case where the EEPROM version is incorrect, and I made the new code block conditional on having early-settable brightness or contrast.

For the moment I removed the clear_lcd refactor so the early init of brightness and contrast can be tested in isolation, but there is no harm in having some extra clear at boot time. The only drawback to all our extra startup details is that the baseline build size for common peripherals continues to expand….

[dbuezas]

Thanks @thinkyhead, it's a very tricky balance indeed.

[thinkyhead]

Thanks everyone for bringing your best stuff. Every day in every way we're getting better and better.

[ellensp]

Looks great, I have no issue with this anymore, this should keep everyone happy.

[thisiskeithb]

Using current bugfix-2.1.x (9240ec8) / after recent LCD changes, part of the custom bootscreen is now missing:

Adding changes from this PR doesn't fix it.

If I revert 1f9fc66, then the bootscreen looks normal again.

edit: While working on another issue with an MKS Eagle with REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER, there are no bootscreens and the LCD doesn't display anything until the main status screen is ready.

[thinkyhead]

If I revert … then the bootscreen looks normal again.

Sounds like the clearing of the screen is leaving the color set to 0 and this is unexpected. Perhaps U8G always sets the color to 1 at initialization and that was taken for granted by boot screen drawing. If that be the case, the solution might just be to put u8g.setColorIndex(1) at the end of MarlinUI::clear_lcd for DOGM.

[thijstriemstra]

thanks for fixing this annoying glitch, can confirm it's fixed with mini1284 display.

Before picture:

After: properly shows black screen followed by boot image and menu.

[thisiskeithb]

This PR created a new bug:

• [BUG] M501 logic with version mismatch #27236

...which can lead to some extremely unsafe behaviors:

• [BUG] Wrong MPC settings can cause thermal runaway #27253

[dbuezas]

I think the bug is here https://github.com/MarlinFirmware/Marlin/pull/27199/files#diff-3b2c0da2300fa99c611618bb2d1a9f7e81edb092aac5d49c83a6d9068561838cR1826
Probably part of of this commit Use EEPROM_Error

It looks like it shouldn't return the error in this case, it should only set the eeprom_error variable and continue (I assume the settings are reset later in that fn)

It should only return early if ERR_EEPROM_NOPROM, in which case it used to return ERR_EEPROM_NOERR

I believe the fix is to change,

    if (check == ERR_EEPROM_VERSION) return eeprom_error;

to

   if (check == ERR_EEPROM_NOPROM) return ERR_EEPROM_NOERR;

to match behaviour before that commit

[ellensp]

similar conclusion I just came to #27236 (comment) only to find this saying the same thing

[thisiskeithb]

Both #27199 (comment) & #27236 (comment) work.