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

🚧 WIP: Add HEATER_LOWER_TEMP 50 #4815

Draft
wants to merge 2 commits into
base: MK3
Choose a base branch
from

Conversation

3d-gussner
Copy link
Collaborator

to lower instead of turining off the hotend temperature during a pause. Thanks for the request #4716 by @alexiri

to lower instead of turining off the hotend temperature during a pause.
Thanks for the request prusa3d#4716 by @alexiri
Copy link

github-actions bot commented Nov 22, 2024

All values in bytes. Δ Delta to base

Target ΔFlash ΔSRAM Used Flash Used SRAM Free Flash Free SRAM
MK3S_MULTILANG 10 0 248312 5654 5640 2538
MK3_MULTILANG 10 0 247682 5663 6270 2529

@@ -184,6 +184,9 @@
#endif
#define BED_MAXTEMP 125

//Lowertemp
#define HEATER_LOWER_TEMP 50 //lower hotend temperature by 50°C
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this new setting should be off by default so that we do not change the default behavior. Users who know what they are doing can then change this setting on their own in my opinion.


I feel this description could be more detailed. This doesn't really tell me what the setting does, and I would have look at the source code to understand. They way I see it, the comment/description here is basically our documentation.

Suggested description:

Uncomment to change the maximum allowed temperature drop while a print is paused. The default behavior sets the target temperature to 0°C.
Example: HEATER_LOWER_TEMP = 10 and target temperature is 215°C when the print was paused. The target temperature will then be set to 205°C instead of 0°C.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also agree with off by default. Was a bit confused about the feature until I read the source code for the PR (recall discussion from last meeting). I like the description provided by @gudnimg 👍

@@ -9877,7 +9877,11 @@ void long_pause() //long pause print

// Stop heaters
heating_status = HeatingStatus::NO_HEATING;
#ifdef HEATER_LOWER_TEMP
setTargetHotend(target_temperature[active_extruder]- HEATER_LOWER_TEMP);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need some protection here to cover edges cases. This can produce negative values: HEATER_LOWER_TEMP > target_temperature[active_extruder]

What happens if we are coming from a temperature error? ThermalStop() can sometimes pause prints.

Copy link
Collaborator Author

@3d-gussner 3d-gussner Nov 23, 2024

Choose a reason for hiding this comment

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

Very good point, thanks. Fan check, uvlo and ThermalStop call lcd_pause_print --> LcdCommands::LongPause -> long_pause`

Copy link
Collaborator

Choose a reason for hiding this comment

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

Might want to subtract only if the target temperature is >= HEATER_LOWER_TEMP. Otherwise you might underflow the target temperature in even more scenarios where the target is a small number

@3d-gussner 3d-gussner changed the title Added HEATER_LOWER_TEMP 50 🚧 WIP: Add HEATER_LOWER_TEMP 50 Nov 23, 2024
@3d-gussner 3d-gussner marked this pull request as draft November 23, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants