Do not heat the bed, on load/unload (or autoload)

[Panayiotis-git]

It has been requested a long time ago:

#422
#2295

Since I've already implemented and tested this improvement for months, I thing I can share it.

[wavexx]

I like the behavior. Maybe we can save some space by avoiding the repetition of the entire menu?
Showing a bed temperature of 0 would be fine IMHO.

[Panayiotis-git]

@wavexx After the changes, we gain 22 bytes in program space. Every other method I tried resulted to a larger binary.

[wavexx]

If I could get a dime for every time I needed this before 👍
I essentially always set the hotend temperature manually to avoid heating the bed

[wavexx]

So I gave this a try, it's nice, but if I already have the bed heated it will set it 0 which is something I do not expect. I also think this could cause problem if preheat is called for unload during the paused state where the bed /should/ be kept on.

When bPreheatOnlyNozzle is set, instead of setting the bed temperature to zero in all menus, we should just skip setting the bed temperature in mFilamentItem instead, leaving it as-is.

This avoids all the need for all checks in mFilamentItem_* functions.

Comments?

[wavexx]

So this one area I have some doubts on. Shouldn't "cut filament" also be a good candidate for bPreheatOnlyNozzle = true?

[Panayiotis-git]

Since I've never seen this functionality working, I left it untouched. But I think you are right.

[Panayiotis-git]

@wavexx indeed the load/unload filament selection during a paused print, set the heated bed to 0 degrees. You have an excellent idea about moving the check into the mFilamentItem function! The only drawback is that then we should initialize the bPreheatOnlyNozzle before each call to the mFilamentItem function.

But then, instead of setting the bPreheatOnlyNozzle beforehand to true or false, it is enough to implement only the bPreheatOnlyNozzle = true. After the temperatures are set, we can reset the variable back to bPreheatOnlyNozzle = false. The variable should also be reset if the user returns to the previous menu or the menu timeout is expired.

Also, as you proposed, we now set the bPreheatOnlyNozzle = true to the MMU cutter preheat menu and it is working perfectly.

So, after all these changes, the compiled code size decreased another 108 bytes and now the whole functionality occupies 194 bytes of code.

PS. The load/unload temperatures during a paused print still are not set correctly. For example, if the user cancels the unloading of the filament while waiting for the nozzle to reach the requested temperature, then both the hotend and the heated bed temperatures are set to 0. But this is a preexisting issue, not related to this PR.

[wavexx]

PS. The load/unload temperatures during a paused print still are not set correctly. For example, if the user cancels the unloading of the filament while waiting for the nozzle to reach the requested temperature, then both the hotend and the heated bed temperatures are set to 0. But this is a preexisting issue, not related to this PR.

That sounds bad :/, especially in pause this would detach the print.
What sequence of actions do I actually need to do to get to this state? Print, pause, resume..?

[Panayiotis-git]

Steps to reproduce:

1. Start a print.

2. Pause the print (Nozzle temp targets to 0, Heated bed keeps the target temp)

3. Wait for the nozzle to cool down

4. Load/Unload filament, choose one. The hotend starts warming.

5. Press Cancel. This will set both the nozzle's and the bed's target temperatures to 0.

6. At this state, if Resume is selected, then first the hotend will restore its temperature and after it reaches the target temp, then starts the heating of the bed.

[wavexx]

I don't know if my review does anything, but I tested the changes and it works perfectly.

[wavexx]

I filed a new bug for the unload behavior during a pause to keep track of it. It's a good idea to fix this separately.

[wavexx]

After using this again, I'm afraid I've run into the same problem I had into PR #2386. While heating the extruder, mFilament is called over and over to display the raising temperature. On the first call we clear the bPreheatOnlyNozzle, then on the second the bed is heated anyway :(

[Panayiotis-git]

My bad. The last commit broke the main functionality, due to the continues call of the mFilament. And it seems that I've skipped the most obvious tests.
I'll give it another try.

[Panayiotis-git]

I returned this PR to a working state. I tried to keep the current logic, but the resulted code/logic was too complex.
So, I reverted to the previous logic, the one that checks at each mFilamentItem_xxx for the bPreheatOnlyNozzle value and set the temperatures accordingly. I assigned the temperature value of -1 to the 'keep current set bed temperature'.

Also, I fixed the bug of resetting the bed temperature after a 'Cancel filament loading' during a paused print.

[wavexx]

Really like that is also now shows the bed temperature while heating.
Maybe that should be hidden when heating only the nozzle?

Still.. nTarget*Old is technically lost when calling mFilamentItem multiple times, which I think defeats the purpose of the bFilamentPreheatState flag. I still don't have the complete mental picture of what's the intention of restoring the temperature of a single cycle earlier...

[Panayiotis-git]

Since the eFilamentAction keeps the currently selected FilamentAction, there is no need to define the boolean variable bFilamentPreheatState. The mFilamentItem depending on the value of the eFilamentAction, decides if it is also required to preheat the bed.

Now the source is as clean and compact as it can get.

[Panayiotis-git]

Still.. nTarget*Old is technically lost when calling mFilamentItem multiple times, which I think defeats the purpose of the bFilamentPreheatState flag. I still don't have the complete mental picture of what's the intention of restoring the temperature of a single cycle earlier...

A value of true for the bFilamentPreheatState indicates that a preheating is already in place. This affects the menu history and if after cancelling the load/unload/... action the preheating should continue or not.

The two variables nTargetOld and nTargetBedOld do not have any use at this place. In fact, I removed them and tested all the affected use cases. The system responded as expected: exactly the same.

Since I cannot also find any use for them, I push another commit with these variables removed.

[wavexx]

Since I cannot also find any use for them, I push another commit with these variables removed.

👍

I've been looking at powerpanic issues lately, but I'll get back to this asap :)

[wavexx]

Looking back, is #2385 by chance also addressed?

[Panayiotis-git]

Yes, this PR also fixes the #2385.

[Panayiotis-git]

I've also identified 2 "problems" that have to do with the temperature display, but are not relevant to this PR:

1. The temperature is stored as a float. At some places we round the temperature to the next integer (eg. the status screen) while at other places we just cut the decimal digits. We should follow the same strategy everywhere.
2. There are cases where the custom characters are messed up. The temperature degree symbol gets replaced with the down arrow. We should make sure that whenever we display temperature, we have called the lcd_set_custom_characters_degree beforehand.

@wavexx Do you suggest a new PR?

[DRracer]

@Panayiotis-git after some internal (flame-war :) ) discussion, there is another valid opinion on this issue - it is not about technical solution, but if you are unloading and loading a piece of filament, you may very well also want to start printing - therefore the bed heating (it is more likely).
The correct solution to this mutually distinct behavior is either:

• custom preheat profiles (like we have the sheets)
• some setting to allow/disable bed heating while loading/unloading etc.

[DRracer]

Marking as FW 3.10 - we shall decide about the generally accepted solution here.

[wavexx]

My 2c, I personally really like the behavior since I switch between materials quite often. I also like a lot the cleanup of the preheat flags too.

[Panayiotis-git]

I rarely leave the printer with a filament loaded and when I finish a printing I usually remove the filament and keep it stored into its sealing bag to avoid absorbing humidity.
But most of the times, when I access the printer it is too late and the hot-end has already cooled. So, I have to re-heat the hot-end (and only the hot-end) in order to unload the filament. This was the main reason that I started this implementation almost a year ago and I find it really useful since it solved my problem.

I have also used the filament loading functionality of this PR some times, like to do a cold-pull cleaning to the hot-end.

If I want to start a printing afterwards, I usually go through the preheat menu. Then, after everything reached the target temperatures, I simply select the Load filament function. It is useful if I want to heat uniformly the bed so I preheat it and leave it for a couple of minutes before I start the printing.

In general, I mostly use the Load/unload functionality as it is implemented with this PR rather than the preheat menu.

Regarding the custom preheat profiles, it is indeed a very useful feature especially for the cases when the filament is not included in the printer's list and requires special temperature handling. My opinion is that the custom preheat profiles rather complement than replace this PR. Otherwise, as a user, I would have to re-create the existing list of filaments with custom profiles only to set the bed temperature to 0 degrees.

The solution to use some sort of settings to allow/disable the bed heating while loading/unloading sounds interesting. Although, I would leave it to disabled, I think it gives the user more choices.

[tcurdt]

Given the age of this PR I think it's safe to assume this will never be merged (although it is annoying AF).

Maybe worth setting up a GCODE macro for it then.
Should this be enough?

M109 S210 ; wait for extruder temp
M412 S0 ; disable filament detection
M83 ; relative mode
G1 E-100 F1000.0 ; unload

[slackista]

4 years old annoying bug. Please merge.

[Panayiotis-git]

The feature implementation including the menu setting, now occupies only 280 bytes.
It is really hard to follow the optimizations that happen during the build. 😃

[slackista]

How many mk3's has Prusa sold overall? 200k? 500k units? How many billions watt-hours of energy were wasted for bed heating while user wants to only unload the filament after finished printing? How long this bug is well known?: 4.5 years.

[wavexx]

🔥 @DRracer

[gudnimg]

@Panayiotis-git Hi 👋 thanks for this PR and sorry this is taking so long to merge. I'm testing your PR at the moment and am seeing it doesn't work as I expected when using the Preheat menu

If I select PLA from Preheat menu, the Bed is heated regardless of what the EEPROM setting is. Is this expected behavior?

If I apply the fix below, then your PR also works for the Preheat menu :) This is super useful for me when testing the MMU. Note I am OK with this not being applied to the Preheat menu, just thought I'd ask about this.

I will try to avoid creating conflict with this PR until we have decided whether to merge or not next week. 👍

[gudnimg]

Approving the current state of the PR 👍 Works great when using for example "Load to Nozzle" menu.

PR will need one more rebase due to a recent merge to add PA to the filament menu. 3e976e0

[Panayiotis-git]

@gudnimg, the Preheat action was intentionally left out from this PR. The logic is that from the Preheat menu you can preheat both nozzle and bed for the selected filament. But for the filament handling actions, only the nozzle will be heated.

TY, for your review!

[Panayiotis-git]

Travis build failed, with message:
CMake Error at /home/travis/build/prusa3d/Prusa-Firmware/cmake/Check_lang_size.cmake:7 (message):
Language file MK3S-EINSy10a_fr.bin (13824b) exceeds maximum allowed size of 13568 bytes - Aborting!

Is there something I can do, in the context of this PR, to correct this error?

[gudnimg]

@Panayiotis-git no the French translation is currently too large (byte wise). It's failing all builds at the moment, but it should be resolved hopefully very soon.

For this PR, this error should not prevent a merge.