Introduce temp watch protection for the bed #3554

scalez · 2016-04-18T17:07:04Z

When setting the bed temp via M140/M190, if the thermistor does not read an increase of WATCH_BED_TEMP_INCREASE degrees within WATCH_BED_TEMP_PERIOD seconds then it will throw "Error:Heating failed, system stopped! Heater_ID: bed" and call the kill() function.

When setting the bed temp via M140/M190 if the thermistor does not read an increase of WATCH_BED_TEMP_INCREASE degrees by WATCH_BED_TEMP_PERIOD seconds then it will throw "Error:Heating failed, system stopped! Heater_ID: bed" and call the kill() function. Conflicts: Marlin/Configuration_adv.h

…configuration headers and not WATCH_TEMP_PERIOD and WATCH_TEMP_INCREASE

AnHardt · 2016-04-18T19:38:47Z

This looks like a well made reinvention of my #2310 and it's followers.
If we don't have a maximum test temperature here, we will get a lot of false positives
(#2310 (comment))

To find a consensus about the activation of this feature will be much easier these days.

Most of the beds out there do not need this feature at all, because they are self limiting and there is no risk of fire. But some (me) desperately need it for their monster-bed-heaters:

If merged in the RC-phase i recommend a deactivated by default version.

scalez · 2016-04-18T20:15:53Z

@AnHardt Not sure how I missed the fact that millis_t is typedef'ed as an unsigned long but now WATCH_BED_TEMP_PERIOD should be able to hold a value up to 4294967 seconds (49.71 days)! I set WATCH_BED_TEMP_PERIOD to 60 seconds, I'd say that there should be a point where we can claim that if a bed does not reach its target temp within an extreme WATCH_BED_TEMP_PERIOD then the design is inherently flawed and the designer/user should be aware that what they're doing is potentially dangerous.

jbrazio · 2016-04-18T20:30:39Z

@AnHardt will you validate this PR with your ~1KW setup ? :-)

AnHardt · 2016-04-18T21:00:25Z

@jbrazio
Sorry. No. No in the near future (days - weeks). Since Easter I'm still barely healthy enough to keep my business going and to drop a few lines now and then.

But as long the algorithm is the same as in the nozzle heater version, i have unlimited trust in it.
@scalez
Alone finding reasonable default values for the case the target temperature is above the self-limiting-temperature, is an impossible task. If the bed will not reach the target temperature the user will realize that - it does not need a error message and a safety shutdown then and more important: "There is no risk of fire" . For that a maximum test temperature is needed and that has to be well below the self-limiting-temperature - if there is a self-limiting-temperature. If the is no self-limiting before self destruction - the maximum test temperature can be MAX_TEMP_BED.).

scalez · 2016-04-18T21:08:57Z

@AnHardt sincerely hope you get better.

The logic is identical to the nozzle.
If the designer/user is aware of the self-limiting-temperature then the case when the target is unreachable should never happen. If the designer/user is setting the target temp using M140 and printing prior to the target temperature being reach (i.e. using M190) then that can also be considered a design/user error.

AnHardt · 2016-04-18T22:31:03Z

In the days when i begun to experiment with, heat activated epoxy's, on high temperature beds, before i got the power-beds, i used to say "give me all you have", give me 150°, even if the bed barely could reach 225° maybe 130° with closed printer doors.
If i remember correct, for an original K8200, "give me all" is the standard for printing ABS.

Of course this is not what you want, but the best you can get, and more common than you might think.

thinkyhead · 2016-04-19T01:15:17Z

This feature and the Bed Temp Residency checking have both been hovering on the periphery as features "we never wanted to add" because heated beds operate in a lower temp range and are –as pointed out– pretty well self-limiting. Still, for symmetry I don't think it's bad to have these two features added now. But I'm sure we can leave them disabled by default.

We ought to be leaving these extra bed temperature features (and a couple of other enhancements disabled by default) anyway, to accommodate the smaller (64K) boards that are now getting crowded out by the ballooning size of the Marlin binary.

With that in mind, keep an eye out for any chance to reduce the size of the "standard" or "default" install of Marlin. Aim for a target binary as small as version 1.0.2-1, or even smaller if possible.

jbrazio · 2016-04-19T10:16:34Z

There is no way to only disable this feature, it is enabled by THERMAL_PROTECTION_BED which currently controls THERMAL_PROTECTION_BED_HYSTERESIS.. nothing against, just a reminder.

Blue-Marlin · 2016-04-19T11:25:58Z

There are several ways to check that separately.
On of them is to check for

#if WATCH_BED_TEMP_PERIOD > 0

instead of

#if ENABLED(THERMAL_PROTECTION_BED)

Conflicts: Marlin/example_configurations/delta/kossel_xl/Configuration_adv.h

scalez · 2016-04-19T15:34:58Z

@thinkyhead @Blue-Marlin I can wrap the heating sanity check code for both the nozzle and the bed with #if WATCH_TEMP_PERIOD > 0 and #if WATCH_BED_TEMP_PERIOD > 0, and set #define WATCH_BED_TEMP_PERIOD 0 in the PR if we get a consensus. Let me know what should be done and I can commit it. Ultimately, I believe a new macro should be created for > 0 purposes.

jbrazio · 2016-04-19T17:17:33Z

NVM that will [in the long run] be worst. Let's keep the config flags for the bed as close as possible as the ones for the nozzle.

As this is a safety feature, do we have any idea what would be a good fits-the-most default value ? Let's do some testing if nothing wrong comes then let's ship this enabled by default as we do for nozzle.

thinkyhead · 2016-04-20T00:35:08Z

For "completeness" I suppose it makes sense to wrap the appropriate code in conditionals like the following, so we can disable these "sub-features" by either non-defining or setting the option to zero:

#if ENABLED(THERMAL_PROTECTION_HOTENDS) && WATCH_TEMP_PERIOD > 0

We already have some precedent for this, as it allows us to avoid needing to add another "enable" flag.

thinkyhead · 2016-04-22T00:54:01Z

Marlin/example_configurations/tvrrug/Round2/Configuration_adv.h

+   * WATCH_TEMP_INCREASE should not be below 2.
+   */
+  #define WATCH_TEMP_PERIOD 20                // Seconds
+  #define WATCH_TEMP_INCREASE 2               // Degrees Celsius


These lines should be in the block above, and the WATCH_BED_TEMP_* items in the code above should be here.

…ocessor conditional involving a check for whether or not they are enabled. start_watching_bed() is now also called in ultralcd.cpp Also correct SanityCheck error for THERMAL_PROTECTION_BED.

…d in tvrrug Round2's example Configuration_adv.h

scalez · 2016-04-25T16:20:50Z

@thinkyhead @jbrazio check out my last two commits, I think it should be good to go now.

jbrazio · 2016-04-25T18:09:54Z

@scalez thanks, it's looking good !

…t_ms is now defined as millis_t and millis_t is typedef'ed as an unsigned long not int (WATCH_TEMP_PERIOD's limit is therefore 4294967s).

thinkyhead · 2016-04-26T03:35:11Z

Marlin/SanityCheck.h

@@ -439,9 +439,9 @@
 */
 #if WATCH_TEMP_PERIOD > 500
  #error WATCH_TEMP_PERIOD now uses seconds instead of milliseconds.
-#elif DISABLED(THERMAL_PROTECTION_HOTENDS) && (defined(WATCH_TEMP_PERIOD) || defined(THERMAL_PROTECTION_PERIOD))
+#elif (DISABLED(THERMAL_PROTECTION_HOTENDS) || WATCH_TEMP_PERIOD <= 0) && (defined(WATCH_TEMP_PERIOD) || (defined(THERMAL_PROTECTION_PERIOD) && WATCH_TEMP_PERIOD > 0))


The original reason for these tests was to deal with older configurations that didn't have THERMAL_PROTECTION_HOTENDS or THERMAL_PROTECTION_BED options, so the other options could be enabled without them. This isn't a problem with your new options, which are … new. So you can drop all your changes to SanityCheck.h here.

thinkyhead · 2016-04-26T03:39:36Z

Marlin/temperature.cpp

  enum TRState { TRReset, TRInactive, TRFirstHeating, TRStable, TRRunaway };
  void thermal_runaway_protection(TRState* state, millis_t* timer, float temperature, float target_temperature, int heater_id, int period_seconds, int hysteresis_degc);
-  #if ENABLED(THERMAL_PROTECTION_HOTENDS)
+  #if ENABLED(THERMAL_PROTECTION_HOTENDS) && WATCH_TEMP_PERIOD > 0


Same here and below. This condition will break compilation if WATCH_TEMP_PERIOD is zero.

thinkyhead · 2016-04-26T03:43:57Z

Other than the needed changes I just listed, it all looks good!

scalez · 2016-04-26T15:39:33Z

@thinkyhead abc3bc1 made the needed changes, let me know what you think.

thinkyhead · 2016-05-01T20:14:48Z

@scalez I've squashed all the commits into a single commit and will merge #3653 shortly.

scalez added 2 commits April 18, 2016 11:04

Use WATCH_BED_TEMP_PERIOD and WATCH_BED_TEMP_INCREASE in the example …

70dc811

…configuration headers and not WATCH_TEMP_PERIOD and WATCH_TEMP_INCREASE

Properly type cast watch_bed_next_ms's assignment

e7250b7

jbrazio added Needs: Testing Testing is needed for this change PR: New Feature labels Apr 18, 2016

jbrazio added the PR: Improvement label Apr 18, 2016

Merge remote-tracking branch 'upstream/RCBugFix' into BedTempWatch

b6783be

Conflicts: Marlin/example_configurations/delta/kossel_xl/Configuration_adv.h

scalez mentioned this pull request Apr 19, 2016

About time for RC6 #3506

Closed

thinkyhead reviewed Apr 22, 2016
View reviewed changes

jbrazio modified the milestone: 1.1.0-RC6 Apr 23, 2016

scalez added 2 commits April 25, 2016 08:18

Use WATCH_TEMP_PERIOD > 0 and WATCH_BED_TEMP_PERIOD > 0 in each prepr…

904c970

…ocessor conditional involving a check for whether or not they are enabled. start_watching_bed() is now also called in ultralcd.cpp Also correct SanityCheck error for THERMAL_PROTECTION_BED.

Correct the locations of watch temp definitions for the hotend and be…

0b957ac

…d in tvrrug Round2's example Configuration_adv.h

It's okay for WATCH_TEMP_PERIOD to be above 32 since watch_heater_nex…

1120eca

…t_ms is now defined as millis_t and millis_t is typedef'ed as an unsigned long not int (WATCH_TEMP_PERIOD's limit is therefore 4294967s).

thinkyhead reviewed Apr 26, 2016
View reviewed changes

Only use WATCH_*TEMP_PERIOD where needed

abc3bc1

thinkyhead mentioned this pull request May 1, 2016

Introduce temp watch protection for the bed #3653

Merged

thinkyhead closed this May 1, 2016

alephobjects-sync-bot deleted the BedTempWatch branch July 15, 2016 15:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce temp watch protection for the bed #3554

Introduce temp watch protection for the bed #3554

scalez commented Apr 18, 2016 •

edited by thinkyhead

Loading

AnHardt commented Apr 18, 2016 •

edited

Loading

scalez commented Apr 18, 2016 •

edited by thinkyhead

Loading

jbrazio commented Apr 18, 2016

AnHardt commented Apr 18, 2016 •

edited

Loading

scalez commented Apr 18, 2016

AnHardt commented Apr 18, 2016 •

edited by thinkyhead

Loading

thinkyhead commented Apr 19, 2016

jbrazio commented Apr 19, 2016

Blue-Marlin commented Apr 19, 2016

scalez commented Apr 19, 2016 •

edited

Loading

jbrazio commented Apr 19, 2016

thinkyhead commented Apr 20, 2016

thinkyhead Apr 22, 2016

scalez commented Apr 25, 2016 •

edited by thinkyhead

Loading

jbrazio commented Apr 25, 2016

thinkyhead Apr 26, 2016 •

edited

Loading

thinkyhead Apr 26, 2016

thinkyhead commented Apr 26, 2016 •

edited

Loading

scalez commented Apr 26, 2016 •

edited

Loading

thinkyhead commented May 1, 2016

Introduce temp watch protection for the bed #3554

Introduce temp watch protection for the bed #3554

Conversation

scalez commented Apr 18, 2016 • edited by thinkyhead Loading

AnHardt commented Apr 18, 2016 • edited Loading

scalez commented Apr 18, 2016 • edited by thinkyhead Loading

jbrazio commented Apr 18, 2016

AnHardt commented Apr 18, 2016 • edited Loading

scalez commented Apr 18, 2016

AnHardt commented Apr 18, 2016 • edited by thinkyhead Loading

thinkyhead commented Apr 19, 2016

jbrazio commented Apr 19, 2016

Blue-Marlin commented Apr 19, 2016

scalez commented Apr 19, 2016 • edited Loading

jbrazio commented Apr 19, 2016

thinkyhead commented Apr 20, 2016

thinkyhead Apr 22, 2016

Choose a reason for hiding this comment

scalez commented Apr 25, 2016 • edited by thinkyhead Loading

jbrazio commented Apr 25, 2016

thinkyhead Apr 26, 2016 • edited Loading

Choose a reason for hiding this comment

thinkyhead Apr 26, 2016

Choose a reason for hiding this comment

thinkyhead commented Apr 26, 2016 • edited Loading

scalez commented Apr 26, 2016 • edited Loading

thinkyhead commented May 1, 2016

scalez commented Apr 18, 2016 •

edited by thinkyhead

Loading

AnHardt commented Apr 18, 2016 •

edited

Loading

scalez commented Apr 18, 2016 •

edited by thinkyhead

Loading

AnHardt commented Apr 18, 2016 •

edited

Loading

AnHardt commented Apr 18, 2016 •

edited by thinkyhead

Loading

scalez commented Apr 19, 2016 •

edited

Loading

scalez commented Apr 25, 2016 •

edited by thinkyhead

Loading

thinkyhead Apr 26, 2016 •

edited

Loading

thinkyhead commented Apr 26, 2016 •

edited

Loading

scalez commented Apr 26, 2016 •

edited

Loading