Marlin: Printer name not displaying at all on power up

[perkmeister]

The printer name is not being displayed at all.

I think I've tracked this down to the following code segment in ultralcd.cpp (around line 5138):

  #if ENABLED(SDSUPPORT) && PIN_EXISTS(SD_DETECT)

    const bool sd_status = IS_SD_INSERTED;
    if (sd_status != lcd_sd_status && lcd_detected()) {

      bool old_sd_status = lcd_sd_status; // prevent re-entry to this block!
      lcd_sd_status = sd_status;

      if (sd_status) {
        safe_delay(1000); // some boards need a delay or the LCD won't show the new status
        card.initsd();
        if (old_sd_status != 2) LCD_MESSAGEPGM(MSG_SD_INSERTED);
      }
      else {
        card.release();
        if (old_sd_status != 2) LCD_MESSAGEPGM(MSG_SD_REMOVED);
      }

I believe the !=2 comparison was to stop the first iteration from writing a message which would overwrite the printer name, because lcd_sd_status is defined as uint8_t and intiailised to 2. However, old_sd_status is defined as a bool which can never have a value of 2 (in fact it will turn the 2 of lcd_sd_status into a 1), and the comparison is always true. The fix I think is to replace the line:

bool old_sd_status = lcd_sd_status; // prevent re-entry to this block!

with:

uint8_t old_sd_status = lcd_sd_status; // prevent re-entry to this block!

Mark.

[thinkyhead]

Changes applied. Hope it helps!

[perkmeister]

Thanks! I'm unsure why you changed sd_status to uint8_t though, it is a bool (lcd_sd_status && lcd_detected() returns a boolean result, and IS_SD_INSERTED is already boolean). It won't matter, maybe a style thing....

[cjsoong]

@perkmeister Did you not read this description in detail?

"
I believe the !=2 comparison was to stop the first iteration from writing a message which would overwrite the printer name, because lcd_sd_status is defined as uint8_t and intiailised to 2. However, old_sd_status is defined as a bool which can never have a value of 2 (in fact it will turn the 2 of lcd_sd_status into a 1), and the comparison is always true. The fix I think is to replace the line…
"

[thinkyhead]

I'm unsure why you changed sd_status to uint8_t though

A style thing. Just so it's clear that sd_status gets a value of 1 for true, and then the comparison sd_status != lcd_sd_status has no implicit coercion to think about.

[perkmeister]

@cjsoong Yes, I wrote it ;) I'm not sure what you mean, sd_status is (was) a bool, it is assigned a boolean value, IS_SD_INSERTED is also a bool, if (sd_status) is (was) a condition on a boolean. The comparison if (old_sd_status != 2) is now done on a uint8_t correctly.

[thinkyhead]

There's nothing untoward with using a uint8_t for consistency across all these variables. Would this really be any better…?

uint8_t sd_status = IS_SD_INSERTED ? 1 : 0;

[perkmeister]

@thinkyhead The actual assignment in the code is
sd_status != lcd_sd_status && lcd_detected();
That's a logical AND operation, the result returned is a boolean. At no point was sd_status ever anything else.

[thinkyhead]

The actual assignment in the code is
sd_status != lcd_sd_status && lcd_detected()

That's not an assignment. It's a comparison (bool result) AND'ed with a boolean.
Parentheses make this clearer…
if ( (sd_status != lcd_sd_status) && lcd_detected() ) { ... }

[perkmeister]

Ah, OK. I was confused, for some reason I thought the comparison was done after the logical AND, it's done before according to C precedence. So you want consistency in comparison types without promotion, so you defined sd_status as a uint8_t. Sorry! (BTW if the AND was done before I might have had a case for keeping sd_status a bool, but it isn't so I haven't ;) )

[thinkyhead]

Yeah, I'll try to be more liberal with parentheses in future. Sometimes they can help!

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.