[FR] Remove or Rename "Change Media" Confusing Menu

[qwewer0]

So, in fact it's not a bug. Maybe a confusing menu ... I think we can even remove it, or maybe rename to "Refresh Media", or "Restart Media" ...

#20309 (comment)

Maybe removing it at all would be better.

#20309 (comment)

Bug Description

After the PR #20176, there is no longer a "Release Media", instead there is a "Change Media" menu item, that doesn't appear to do anything.

SKR Mini E3 v1.2, non USB STM32F103RC_btt / _512k environment.

I opened this issue not to get back the "Release Media", but instead because there is now a menu item that does nothing.
If on a non USB environment (e.g. STM32F103RC_btt_512K) the SD card can be safely removed without releasing the media, then I think there is no reason to have "Release Media" or "Change Media" menu item (I prefer this option).
But if this isn't the case, then we should have either "Release Media" or "Change Media" menu item that is working.

Configuration Files

Configuration.zip

Steps to Reproduce

0. Flash the latest bugfix-2.0.x (after Fix NO_SD_HOST_DRIVE when shared media is not supported #20176)
1. Insert SD card
2. Select "Change Media" menu item
3. Observe that nothing happens

Expected behavior:

Not having either a "Release Media" or "Change Media" menu item.

Actual behavior:

There is a "Change Media" menu item, that does nothing.

[rhapsodyv]

In theory, all we did was remove the section that did “undef SD_DETECT_PIN”. But seems it have a colateral effect.

I will take a look.

[qwewer0]

@rhapsodyv Yes, I remember, I tried to look at the changes, but couldn't find anything that could affect this.
Thank you very much for the check.

[rhapsodyv]

Well ... it's confusing ... but there's a reason ...

Point 1) Before that PR, all boards with SD_DETECT were being ignored. So, even if you have a SD_DETECT_PIN, it wasn't being used by Marlin.

Point 2) Marlin have different behavior when you have SD_DETECT and you when don't have it.

a) When you ** don't ** have SD_DETECT: you need manually release the media, because Marlin will not be able to detect when you removed it. So, here we have the Release Media menu, that just does a M22.

b) When you have SD_DETECT: Marlin will automatically detect when the SD card was pulled off. So, it automatically call its internal release routines. So Marlin don't need that you manually release it in the menus. Because of that, it change the menu to "Change Media", that just does a "M21" to re-init it. It even don't need to be called, just to use force a re-init if it gave any error on automatic marlin code.

The "Change Media" is a new thing to a lot of us, as it was almost always disabled because of the bug with SD_DETECT that was fixed by # 20176 ....

So, in fact it's not a bug. Maybe a confusing menu ... I think we can even remove it, or maybe rename to "Refresh Media", or "Restart Media" ...

But the behavior seems correct, although confusing ...

[qwewer0]

Refresh Media sounds better, more telling.
While I would love to see and option to disable that menu item, but I did not thought it through to what disadvantages would the lack of it mean.

@rhapsodyv What do you think, close this and open a FR to rename Change Media to Refresh Media, or change this issue into the FR?

[rhapsodyv]

Maybe removing it at all would be better.
I think we already have a "Refresh Media" inside the file browser, or not?
I will take a look in all options.

Good point. I will change this to FR.

[qwewer0]

There is a "Refresh", but that is only that:

Marlin/Marlin/src/lcd/language/language_en.h

Line 346 in e817773

PROGMEM Language_Str MSG_REFRESH = LCD_STR_REFRESH _UxGT("Refresh");

If removing it wouldn't mean any negative side effects and that is the easiest way to handle it, than so be it.

[qwewer0]

@rhapsodyv I have no idea how to describe this as a FR, so I just quoted and linked two of your comment.

[rhapsodyv]

It's ok this way. I think I will work on this soon.

[mathiasvr]

This reminds me of #17272, where I also wanted to remove all these types of menu items but it wasn't so simple to support all types of configurations. Still hoping this can be simplified somehow. 🙂