Skip to content
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.

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

Filament loading state fixes #3564

Merged
merged 6 commits into from
Jul 26, 2023
Merged

Conversation

wavexx
Copy link
Collaborator

@wavexx wavexx commented Aug 20, 2022

There are some issues in the management of "filament autoloading" when the printer is idle. Because the autoloading is performed asynchronously, we need to ensure incompatible actions are disabled while loading is taking place.

With this PR we make impossible to perform another autoload/unload/filament action when one is already taking place (the autoload/load/unload menus are now hidden while autoloading is taking place).

To do so we check for the loading_flag. This was previously done for loading only, but doing so for unload makes the behavior uniform (#2318 performs more than a single move and is also non-blocking - you shouldn't load while the old unload is still pending).

Using bFilamentState flag would have been a little bit cleaner here (removing the need for loading_flag), but it's currently used both as a flag for autoload and also for the autoload state...

We also now prevent to start an SD print while autoload is taking place (fixes #2651). You can still navigate the SD card when waiting, but clicking on the file will just beep (see my comments on b0561a0)

As a side note, in the first commit, we change PRINTER_ACTIVE to consider any custom command to be non-idle, which is logical. All custom commands are incompatible with each other, and will return the printer to Idle when complete (LongPause included). Checking for Layer1Cal in the main menu is simply incorrect.

Change in memory (MK3S+ Multilang):
Flash: -8 bytes
SRAM: -1 byte

@gudnimg
Copy link
Collaborator

gudnimg commented Sep 12, 2022

Is loading_flag only applicable for autoloading? Should we consider renaming it to auto_loading_flag or something to that effect?

@wavexx
Copy link
Collaborator Author

wavexx commented Sep 19, 2022

Is loading_flag only applicable for autoloading? Should we consider renaming it to auto_loading_flag or something to that effect?

I would be in favor to rename this as auto[_]loading_flag, but we need to this after the MK3_MMU2 branch is merged.

@wavexx
Copy link
Collaborator Author

wavexx commented Dec 13, 2022

@gudnimg rereading the code it seems that now eFilamentAction does what we need, so I removed loading_flag completely.

The old code was resetting the "autoload" flag for some reason. I changed the meaning so that the "AutoLoad" state is the moment when autoload has been triggered, but loading is still not taking place. AutoLoad can be cancelled (if you're still in the preheat menu), while "Load" is already starting to feed the filament and cannot be undone.

IMHO this is a lot saner, and I hope I understood all existing scenarios.

Can you review the MMU part? I added one case in lcd_wizard_load which I think was missing.

@3d-gussner because eFilamentAction is now used everywhere this requires some retesting of #2651 but also retesting all scenarios with loading/unloading, autoloading+cancel, and using the menu load/load filament correctly clears the load flag, otherwise you won't see the "load/unload" menus anymore.

Copy link
Collaborator

@gudnimg gudnimg left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think you're right about eFilamentAction = FilamentAction::MmuLoad being missing before.

@wavexx
Copy link
Collaborator Author

wavexx commented Dec 31, 2022

Should fix #3297

@gudnimg
Copy link
Collaborator

gudnimg commented Jul 16, 2023

@wavexx I rebased the PR to resolve the merge conflicts. Hopefully I didn't break anything. The PR was quite a bit behind MK3 branch :)

wavexx added 6 commits July 25, 2023 14:27
All custom commands are transitory and eventually switch back to Idle
state by themselves.

It doesn't make any sense to explicitly check for Layer1Cal: any
non-idle state is active by design.

Fix this check in the main menu. This is probably incomplete (Layer1Cal
is incorrectly used in several other places).
Do not blidnly clear the loading_flag, check for it!

Just disallowing the SD menu while loading is being performed is not
sufficient, since the menu can be entered also by inserting card while
loading is taking place.

This is also nicer in behavior, as we allow to navigate the SD card
while loading.
Remove loading_flag and check for eFilamentAction instead which already
flags both load/unload (in addition to mmu actions).

Correctly transition from AutoLoad to Load as soon as the operation
cannot be cancelled anymore as opposed to resetting it.
@gudnimg gudnimg force-pushed the fil_loading_state branch from 37f05bc to 3485c20 Compare July 25, 2023 14:31
@gudnimg
Copy link
Collaborator

gudnimg commented Jul 25, 2023

Rebased again. For some reason I had to re-do the entire rebase (~600 commits)

Copy link
Collaborator

@3d-gussner 3d-gussner left a comment

Choose a reason for hiding this comment

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

Approved and fixes issues like #3595
Other tests with MK404 show that this PR is an improvement to previous version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants