Add extra max-temp safety checks #13756

marcio-ao · 2019-04-18T20:00:14Z

I fixed a few errors that rendered the temperature error messages incomplete.

I implemented and tested the BED_MAXTEMP suggestion in #13711, but decided to make the checks optional and called them HEATER_KILL_TEMP and BED_KILL_TEMP.

I also changed the MSG_COOLING and MSG_HEATING to use the same macro (now renamed TEMP_PSTR). This allows those messages to use the MSG_E##n strings for consistency with everything else.

- Would just show "BED" for bed msg, rather than "BED <msg>" - Would use MSG_E1 for both E0 and E1 - Renamed TEMP_ERR_PSTR to TEMP_PSTR - MSG_COOLING and MSG_HEATING now use the TEMP_PSTR macro for consistency

marcio-ao · 2019-04-18T21:45:12Z

It really isn't clear whether MAXTEMP is supposed to be the highest temperature the printer is supposed to reach, or a dangerously high temperature that can only be triggered during a fault.

The config file says "above this temperature the heater will be switched off" which is less urgent than "OMG! My hair is on fire!". Traditionally Lulzbot has used MAXTEMP for the latter, but perhaps this was a misunderstanding on our part or maybe the intent has changed in Marlin over time?

I have decided to add two new configuration options, HEATER_KILL_TEMP and BED_KILL_TEMP. We can use this on our printers to get the behavior we are accustomed to, while others can choose to enable it or not. These new configuration options also have an advantage over more sophisticated checks in that they are easily testable by using fake thermistors (i.e. resistors), which is good for quick testing in a production line.

- Readings above these temperatures will cause the printer to immediately halt.

ejtagle · 2019-04-19T06:11:34Z

Makes more than sense

robbycandra · 2019-04-19T08:29:08Z

@marcio-ao .. Thats better...

marcio-ao · 2019-04-19T14:08:02Z

Alternatives might be to have the HEATER_KILL_TEMP be some fixed delta above the MAX of the the individual HEATER_MAXTEMP, so choosing a value would not be necessary.

robbycandra · 2019-04-19T17:59:28Z

Delta can be done in configuration.h.

marcio-ao · 2019-04-19T18:14:51Z

Delta can be done in configuration.h.

Hum? I mean a ΔT, not a delta printer.

ManuelMcLure · 2019-04-19T20:56:32Z

Because you can only set the temperature using the LCD to <whatever>_MAXTEMP - 15C, I always considered <whatever>_MAXTEMP to be the "something's gone horribly wrong and I need to shut down the printer`` temperature.

marcio-ao · 2019-04-19T21:41:29Z

@ManuelMcLure: Yes, I think this is what the current implementation does, but it isn't particularly intuitive. This is what I think it looks like now:

Temp Greater Than	But Less Than	Current Behavior
0	MAX_TEMP - 15	User can tune temps withing this range
MAX_TEMP - 15	MAX_TEMP	Not sure what happens here?
MAX_TEMP	Inf	Heaters off

To me, the most important parameter from a user's perspective is the maximum temperature they intend to print at. They should be able to set that temperature directly and the LCD and GCODE should allow the temp to be set up to that value. If the thermistor detected a temperature above that value, the heaters should be turned off; but the printer would remain operational, to account for possible overshoot in PID. At some even higher temperature, the printer would kill itself. It could look like this:

Temp Greater Than	But Less Than	Ideal Behavior
0	MAX_TEMP	User can tune temps withing this range
MAX_TEMP	KILL_TEMP	Heaters off
KILL_TEMP	Inf	Printer Killed

Alas, this doing this change to the meaning of MAX_TEMP would mess everybody's config, so this PR leaves MAX_TEMP as is and adds KILL_TEMP. We end up with something like this:

Temp Greater Than	But Less Than	After This PR
0	MAX_TEMP - 15	User can tune temps withing this range
MAX_TEMP - 15	MAX_TEMP	Not sure what happens here?
MAX_TEMP	KILL_TEMP	Heaters off
KILL_TEMP	Inf	Printer Killed

ManuelMcLure · 2019-04-19T22:31:42Z

I think currently nothing special happens between MAX_TEMP - 15 and MAX_TEMP - my guess is that it is a "grey area" to handle the fact that you might be running with less-than-perfect PIDs. You want to avoid getting an error if the temperature is oscillating just a few degrees around the set point. You don't want to turn off the heaters right when you hit the maximum settable temperature because that will break PID - you need at least some leeway above the maximum settable temperature.
IMHO, MAX_TEMP should act the way you're suggesting KILL_TEMP works - an absolute "something's wrong - kill the print" temperature, not just a "turn off the heaters" temperature. That seems to be the intent of the code that thinkyhead described - _temp_error() will call kill() as soon as the temperature reaches MAX_TEMP.
Given that, I don't really see the utility in splitting MAX_TEMP and KILL_TEMP.

robbycandra · 2019-04-20T00:01:28Z

Hum? I mean a ΔT, not a delta printer.

Yes....
#define BED_KILL_TEMP BED_MAXTEMP+10

The differences:
use absolute value = 1 same Kill Temp for all Hot End.
use delta value = different Kill Temp for different Hot End.

robbycandra · 2019-04-20T00:15:27Z

Given that, I don't really see the utility in splitting MAX_TEMP and KILL_TEMP.

MAX TEMP-15 -- MAX TEMP : Space for Temp OverShoot
. > MAX TEMP -> Heater Off : The Overshoot is too much - Turn Off Heater
. >KILL TEMP -> Printer Off : SomeThing Wrong, Maybe Mosfet failure. Turn off printer. (hopefully it will turn off the power supply)

I think its better to have KILL TEMP to turn printer off.

ManuelMcLure · 2019-04-20T00:25:02Z

If you turn off the heater after a 15C overshoot it means your PIDs are very bad and it's pretty much assured that your print is ruined anyway, so I don't see why you wouldn't take the safer option of killing the printer.

ManuelMcLure · 2019-04-20T00:30:59Z

I do agree that the naming is confusing. I'd much prefer MAXTEMP to be "maximum settable/printing temp" and KILL_TEMP be "something's gone wrong - kill the printer" defaulting to MAXTEMP + 15. But I don't see the reason for an intermediate "turn off the heaters but don't kill the printer" setting.

robbycandra · 2019-04-20T00:40:01Z

@ManuelMcLure you are correct too....
The problem @marcio-ao want to fix is the printer still turn on even the temp is very high.

Maybe still need more work, but it is better and safer to turn off printer at some point.

ManuelMcLure · 2019-04-20T01:17:25Z

_temp_error() should always kill the printer (unless BOGUS_TEMPERATURE_FAILSAFE_OVERRIDE is set), so if _temp_error() is being called without killing the printer there's something seriously weird going on.

thinkyhead · 2019-04-20T01:23:25Z

Try setting a hotend or the bed to use one of the dummy thermistors, then set the dummy thermistor temperature to be higher than the max temperature for the hotend or the bed. This can be used to test what happens when the machine boots or is running in the idle loop with an out-of-range temperature reading.

thinkyhead · 2019-04-20T01:26:11Z

As detailed elsewhere, the MINTEMP and MAXTEMP errors are very specific, and indicate either a shorted thermistor (zero resistance) or a broken thermistor wire (infinite resistance). These types of errors are always checked and cannot be disabled. They are tested by looking directly at the ADC reading, not the celsius temperature.

thinkyhead · 2019-04-20T01:35:50Z

Marlin/src/module/temperature.cpp

-      #else
-        ui.set_status_P(heating ? PSTR("E " MSG_HEATING) : PSTR("E " MSG_COOLING));
-      #endif
+      ui.set_status_P(heating ? TEMP_PSTR(MSG_HEATING, e) : TEMP_PSTR(MSG_COOLING, e));


I would expect this to bloat the size of the binary more than the original code because it produces up to 12 PROGMEM strings where the previous code only generates 2 no matter how many hotends are present.

thinkyhead · 2019-04-20T01:50:54Z

Marlin/src/module/temperature.cpp

@@ -86,17 +86,17 @@ Temperature thermalManager;
 */

 #if HAS_HEATED_BED
-  #define _BED_PSTR(E) (E) == -1 ? PSTR(MSG ## _BED) :
+  #define _BED_PSTR(M,E) (E) == -1 ? PSTR(MSG_BED " " M) :


We tend not to do this kind of appending of strings because in some languages the order of subject-object-modifier is completely different, and we don't want to impose our English-centric word ordering on everyone. That is why the intent here was to concatenate to get the right translation string name and use the translated string which has the correct word-ordering.

thinkyhead · 2019-04-20T02:00:56Z

Marlin/src/module/temperature.cpp

 #endif
 #define _E_PSTR(M,E,N) (HOTENDS >= (N) && (E) == (N)-1) ? PSTR(MSG_E##N " " M) :
-#define TEMP_ERR_PSTR(M,E) _BED_PSTR(E) _CHAMBER_PSTR(E) _E_PSTR(M,E,2) _E_PSTR(M,E,3) _E_PSTR(M,E,4) _E_PSTR(M,E,5) _E_PSTR(M,E,6) PSTR(MSG_E1 " " M)
+#define TEMP_PSTR(M,E) _BED_PSTR(M,E) _CHAMBER_PSTR(M,E) _E_PSTR(M,E,2) _E_PSTR(M,E,3) _E_PSTR(M,E,4) _E_PSTR(M,E,5) _E_PSTR(M,E,6) PSTR(MSG_E0 " " M)


It is important to note that "E1" is the first extruder when displayed to the user, not "E0" as it is named in the code.

Followup to #13756

robbycandra · 2019-04-20T05:37:55Z

@thinkyhead , I cannot compile this PR.
error: unable to find string literal operator 'operator""_BED'

Followup to MarlinFirmware#13756

Fixed temperature error messages

c5fb98a

- Would just show "BED" for bed msg, rather than "BED <msg>" - Would use MSG_E1 for both E0 and E1 - Renamed TEMP_ERR_PSTR to TEMP_PSTR - MSG_COOLING and MSG_HEATING now use the TEMP_PSTR macro for consistency

marcio-ao mentioned this pull request Apr 18, 2019

[BUG] [Possible Safety Issue] MAXTEMP_BED not triggering #13711

Closed

Added optional BED_KILL_TEMP and HEATER_KILL_TEMP

3dd0fb7

- Readings above these temperatures will cause the printer to immediately halt.

marcio-ao force-pushed the pr-temp-message-fixes branch from 5ac24dd to 3dd0fb7 Compare April 18, 2019 21:50

marcio-ao changed the title ~~Fixed garbled temperature messages; added check for BED_MAXTEMP~~ Fixed garbled temperature messages; added HEATER_KILL_TEMP and BED_KILL_TEMP Apr 18, 2019

thinkyhead changed the title ~~Fixed garbled temperature messages; added HEATER_KILL_TEMP and BED_KILL_TEMP~~ Fix garbled temperature messages; add HEATER_KILL_TEMP, BED_KILL_TEMP Apr 20, 2019

MSG_E0 is being deleted

229a6d0

thinkyhead reviewed Apr 20, 2019

View reviewed changes

thinkyhead added 2 commits April 19, 2019 20:38

Update temperature.cpp

ba8d14b

No need for extra kill temp

a791023

thinkyhead reviewed Apr 20, 2019

View reviewed changes

ibid

8fe0293

thinkyhead reviewed Apr 20, 2019

View reviewed changes

thinkyhead added 2 commits April 19, 2019 21:29

Correction to TEMP_ERR_PSTR repair

068eaf5

Fix bed/chamber error messages

143f58d

thinkyhead force-pushed the pr-temp-message-fixes branch from ef21c31 to 143f58d Compare April 20, 2019 02:29

thinkyhead changed the title ~~Fix garbled temperature messages; add HEATER_KILL_TEMP, BED_KILL_TEMP~~ Add extra max-temp safety checks Apr 20, 2019

thinkyhead added C: Safety PR: Improvement labels Apr 20, 2019

thinkyhead merged commit d0c1eee into MarlinFirmware:bugfix-2.0.x Apr 20, 2019

thinkyhead added a commit that referenced this pull request Apr 20, 2019

Add chamber max-temp check

9fd00f9

Followup to #13756

thinkyhead added a commit that referenced this pull request Apr 20, 2019

Add chamber max-temp check

bddb776

Followup to #13756

Idorobots mentioned this pull request Apr 20, 2019

Fix _temp_error-related compile error #13768

Merged

Tymek pushed a commit to Tymek/Ender-3-Ghost that referenced this pull request Apr 23, 2019

Add extra max-temp safety checks (MarlinFirmware#13756)

3e8d63e

Tymek pushed a commit to Tymek/Ender-3-Ghost that referenced this pull request Apr 23, 2019

Add chamber max-temp check

d30003b

Followup to MarlinFirmware#13756

marcio-ao deleted the pr-temp-message-fixes branch May 13, 2019 22:14

robbycandra added a commit to robbycandra/Marlin that referenced this pull request Sep 12, 2019

correction for MarlinFirmware#13756

e7dc7b3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add extra max-temp safety checks #13756

Add extra max-temp safety checks #13756

marcio-ao commented Apr 18, 2019 •

edited

Loading

marcio-ao commented Apr 18, 2019 •

edited

Loading

ejtagle commented Apr 19, 2019

robbycandra commented Apr 19, 2019

marcio-ao commented Apr 19, 2019

robbycandra commented Apr 19, 2019

marcio-ao commented Apr 19, 2019

ManuelMcLure commented Apr 19, 2019

marcio-ao commented Apr 19, 2019

ManuelMcLure commented Apr 19, 2019

robbycandra commented Apr 20, 2019 •

edited

Loading

robbycandra commented Apr 20, 2019 •

edited

Loading

ManuelMcLure commented Apr 20, 2019

ManuelMcLure commented Apr 20, 2019

robbycandra commented Apr 20, 2019 •

edited by thinkyhead

Loading

ManuelMcLure commented Apr 20, 2019

thinkyhead commented Apr 20, 2019 •

edited

Loading

thinkyhead commented Apr 20, 2019

thinkyhead Apr 20, 2019 •

edited

Loading

thinkyhead Apr 20, 2019 •

edited

Loading

thinkyhead Apr 20, 2019

robbycandra commented Apr 20, 2019 •

edited

Loading

Add extra max-temp safety checks #13756

Add extra max-temp safety checks #13756

Conversation

marcio-ao commented Apr 18, 2019 • edited Loading

marcio-ao commented Apr 18, 2019 • edited Loading

ejtagle commented Apr 19, 2019

robbycandra commented Apr 19, 2019

marcio-ao commented Apr 19, 2019

robbycandra commented Apr 19, 2019

marcio-ao commented Apr 19, 2019

ManuelMcLure commented Apr 19, 2019

marcio-ao commented Apr 19, 2019

ManuelMcLure commented Apr 19, 2019

robbycandra commented Apr 20, 2019 • edited Loading

robbycandra commented Apr 20, 2019 • edited Loading

ManuelMcLure commented Apr 20, 2019

ManuelMcLure commented Apr 20, 2019

robbycandra commented Apr 20, 2019 • edited by thinkyhead Loading

ManuelMcLure commented Apr 20, 2019

thinkyhead commented Apr 20, 2019 • edited Loading

thinkyhead commented Apr 20, 2019

thinkyhead Apr 20, 2019 • edited Loading

Choose a reason for hiding this comment

thinkyhead Apr 20, 2019 • edited Loading

Choose a reason for hiding this comment

thinkyhead Apr 20, 2019

Choose a reason for hiding this comment

robbycandra commented Apr 20, 2019 • edited Loading

marcio-ao commented Apr 18, 2019 •

edited

Loading

marcio-ao commented Apr 18, 2019 •

edited

Loading

robbycandra commented Apr 20, 2019 •

edited

Loading

robbycandra commented Apr 20, 2019 •

edited

Loading

robbycandra commented Apr 20, 2019 •

edited by thinkyhead

Loading

thinkyhead commented Apr 20, 2019 •

edited

Loading

thinkyhead Apr 20, 2019 •

edited

Loading

thinkyhead Apr 20, 2019 •

edited

Loading

robbycandra commented Apr 20, 2019 •

edited

Loading