Skip to content

color_ui: refresh the menu on media detach #20958

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.

Sign up for GitHub

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

Merged
thinkyhead merged 1 commit into from
Feb 1, 2021
Merged

color_ui: refresh the menu on media detach #20958

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Marlin/src/gcode/sd/M21_M22.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include "../gcode.h"
#include "../../sd/cardreader.h"
#include "../../lcd/marlinui.h"

/**
* M21: Init SD Card
Expand All @@ -36,9 +37,8 @@ void GcodeSuite::M21() { card.mount(); }
* M22: Release SD Card
*/
void GcodeSuite::M22() {

if (!IS_SD_PRINTING()) card.release();

IF_ENABLED(TFT_COLOR_UI, ui.refresh(LCDVIEW_CALL_REDRAW_NEXT));
Copy link
Member

@rhapsodyv rhapsodyv Jan 31, 2021

Choose a reason for hiding this comment

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

Is here the right place for that?

I prefer that UI code should be handled on UI code, not on core/gcode.

I think the right place for that, is right after the menu call, isn't?

Copy link
Contributor Author

@tpruvot tpruvot Jan 31, 2021

Choose a reason for hiding this comment

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

i seen the lcd/marlinui.h in most others M2x commands... and menu code is messy

Copy link
Member

@rhapsodyv rhapsodyv Jan 31, 2021

Choose a reason for hiding this comment

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

The point is: only when you have that menu opened on screen that you need a refresh, not every time a M22 is called. That is the main reason I would prefer the UI handling its own needs.

Copy link
Contributor Author

@tpruvot tpruvot Jan 31, 2021

Choose a reason for hiding this comment

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

you only detach the SD from the menu no ? to remove it...

Copy link
Member

@rhapsodyv rhapsodyv Jan 31, 2021

Choose a reason for hiding this comment

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

you can do it manually calling M22... We should try our best to separated core than UI

Copy link
Contributor Author

@tpruvot tpruvot Jan 31, 2021

Choose a reason for hiding this comment

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

Also, there are other reasons to refresh even on the status page... to show/hide the sd icon

Copy link
Member

@rhapsodyv rhapsodyv Jan 31, 2021

Choose a reason for hiding this comment

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

Now you got a really good point. So, maybe it's a hook for card.release.... I don't know if card.release is called only on M22... This issue happens when we have SD_DETECT and pull out the SD?

Copy link
Contributor Author

@tpruvot tpruvot Feb 1, 2021

Choose a reason for hiding this comment

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

a refresh doesnt hurt, and you dont do mount/umount during prints

Copy link
Member

@rhapsodyv rhapsodyv Feb 1, 2021

Choose a reason for hiding this comment

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

I mean: if I pull out the SD card while on that menu screen, it will refresh?

Copy link
Contributor Author

@tpruvot tpruvot Feb 1, 2021

Choose a reason for hiding this comment

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

for the others, this issue was without SD DETECT PIN, with COLOR UI

}

#endif // SDSUPPORT