-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
base: MK3
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -184,6 +184,9 @@ | |
#endif | ||
#define BED_MAXTEMP 125 | ||
|
||
//Lowertemp | ||
#define HEATER_LOWER_TEMP 50 //lower hotend temperature by 50°C | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 👍 |
||
|
||
#if defined(E3D_PT100_EXTRUDER_WITH_AMP) || defined(E3D_PT100_EXTRUDER_NO_AMP) | ||
// Define PID constants for extruder with PT100 | ||
#define DEFAULT_Kp 21.70 | ||
|
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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`There was a problem hiding this comment.
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