Move temp errors calling kill() out of ISR (#21832)

Marlin/src/module/temperature.cpp

@@ -576,8 +576,7 @@ volatile bool Temperature::raw_temps_ready = false;
 
       const millis_t ms = millis();
 
-      if (raw_temps_ready) { // temp sample ready
+      if (updateTemperaturesIfReady()) { // temp sample ready
-        updateTemperaturesFromRawValues();
 
         // Get the current temperature and constrain it
         current_temp = GHV(degChamber(), degBed(), degHotend(heater_id));
@@ -1212,9 +1211,7 @@ void Temperature::manage_heater() {
     }
   #endif
 
-  if (!raw_temps_ready) return;
+  if (!updateTemperaturesIfReady()) return; // Will also reset the watchdog if temperatures are ready
-
-  updateTemperaturesFromRawValues(); // also resets the watchdog
 
   #if DISABLED(IGNORE_THERMOCOUPLE_ERRORS)
     #if TEMP_SENSOR_0_IS_MAX_TC
@@ -1890,29 +1887,88 @@ void Temperature::manage_heater() {
 #endif // HAS_TEMP_PROBE
 
 /**
- * Get the raw values into the actual temperatures.
+ * Convert the raw sensor readings into actual Celsius temperatures and
- * The raw values are created in interrupt context,
+ * validate raw temperatures. Bad readings generate min/maxtemp errors.
- * and this function is called from normal context
+ *
- * as it would block the stepper routine.
+ * The raw values are generated entirely in interrupt context, and this
+ * method is called from normal context once 'raw_temps_ready' has been
+ * set by update_raw_temperatures().
+ *
+ * The watchdog is dependent on this method. If 'raw_temps_ready' stops
+ * being set by the interrupt so that this method is not called for over
+ * 4 seconds then something has gone afoul and the machine will be reset.
  */
 void Temperature::updateTemperaturesFromRawValues() {
+
+  watchdog_refresh(); // Reset because raw_temps_ready was set by the interrupt
+
   TERN_(TEMP_SENSOR_0_IS_MAX_TC, temp_hotend[0].raw = READ_MAX_TC(0));
   TERN_(TEMP_SENSOR_1_IS_MAX_TC, TERN(TEMP_SENSOR_1_AS_REDUNDANT, temp_redundant, temp_hotend[1]).raw = READ_MAX_TC(1));
   #if HAS_HOTEND
     HOTEND_LOOP() temp_hotend[e].celsius = analog_to_celsius_hotend(temp_hotend[e].raw, e);
   #endif
   TERN_(TEMP_SENSOR_1_AS_REDUNDANT, temp_redundant.celsius = analog_to_celsius_hotend(temp_redundant.raw, 1));
-  TERN_(HAS_HEATED_BED, temp_bed.celsius = analog_to_celsius_bed(temp_bed.raw));
+
+  TERN_(HAS_HEATED_BED,   temp_bed.celsius     = analog_to_celsius_bed(temp_bed.raw));
   TERN_(HAS_TEMP_CHAMBER, temp_chamber.celsius = analog_to_celsius_chamber(temp_chamber.raw));
-  TERN_(HAS_TEMP_COOLER, temp_cooler.celsius = analog_to_celsius_cooler(temp_cooler.raw));
+  TERN_(HAS_TEMP_COOLER,  temp_cooler.celsius  = analog_to_celsius_cooler(temp_cooler.raw));
-  TERN_(HAS_TEMP_PROBE, temp_probe.celsius = analog_to_celsius_probe(temp_probe.raw));
+  TERN_(HAS_TEMP_PROBE,   temp_probe.celsius   = analog_to_celsius_probe(temp_probe.raw));
+
   TERN_(FILAMENT_WIDTH_SENSOR, filwidth.update_measured_mm());
-  TERN_(HAS_POWER_MONITOR, power_monitor.capture_values());
+  TERN_(HAS_POWER_MONITOR,     power_monitor.capture_values());
 
-  // Reset the watchdog on good temperature measurement
+  #if HAS_HOTEND
-  watchdog_refresh();
+
+    static constexpr int8_t temp_dir[] = {
+      TERN(TEMP_SENSOR_0_IS_MAX_TC, 0, TEMPDIR(0))
+      #if HAS_MULTI_HOTEND
+        , TERN(TEMP_SENSOR_1_IS_MAX_TC, 0, TEMPDIR(1))
+        #if HOTENDS > 2
+          #define _TEMPDIR(N) , TEMPDIR(N)
+          REPEAT_S(2, HOTENDS, _TEMPDIR)
+        #endif
+      #endif
+    };
+
+    LOOP_L_N(e, COUNT(temp_dir)) {
+      const int8_t tdir = temp_dir[e];
+      if (tdir) {
+        const int16_t rawtemp = temp_hotend[e].raw * tdir; // normal direction, +rawtemp, else -rawtemp
+        if (rawtemp > temp_range[e].raw_max * tdir) max_temp_error((heater_id_t)e);
 
-  raw_temps_ready = false;
+        const bool heater_on = temp_hotend[e].target > 0;
+        if (heater_on && rawtemp < temp_range[e].raw_min * tdir && !is_preheating(e)) {
+          #if MAX_CONSECUTIVE_LOW_TEMPERATURE_ERROR_ALLOWED > 1
+            if (++consecutive_low_temperature_error[e] >= MAX_CONSECUTIVE_LOW_TEMPERATURE_ERROR_ALLOWED)
+          #endif
+              min_temp_error((heater_id_t)e);
+        }
+        #if MAX_CONSECUTIVE_LOW_TEMPERATURE_ERROR_ALLOWED > 1
+          else
+            consecutive_low_temperature_error[e] = 0;
+        #endif
+      }
+    }
+
+  #endif // HAS_HOTEND
+
+  #if ENABLED(THERMAL_PROTECTION_BED)
+    #define BEDCMP(A,B) (TEMPDIR(BED) < 0 ? ((A)<(B)) : ((A)>(B)))
+    if (BEDCMP(temp_bed.raw, maxtemp_raw_BED)) max_temp_error(H_BED);
+    if (temp_bed.target > 0 && BEDCMP(mintemp_raw_BED, temp_bed.raw)) min_temp_error(H_BED);
+  #endif
+
+  #if BOTH(HAS_HEATED_CHAMBER, THERMAL_PROTECTION_CHAMBER)
+    #define CHAMBERCMP(A,B) (TEMPDIR(CHAMBER) < 0 ? ((A)<(B)) : ((A)>(B)))
+    if (CHAMBERCMP(temp_chamber.raw, maxtemp_raw_CHAMBER)) max_temp_error(H_CHAMBER);
+    if (temp_chamber.target > 0 && CHAMBERCMP(mintemp_raw_CHAMBER, temp_chamber.raw)) min_temp_error(H_CHAMBER);
+  #endif
+
+  #if BOTH(HAS_COOLER, THERMAL_PROTECTION_COOLER)
+    #define COOLERCMP(A,B) (TEMPDIR(COOLER) < 0 ? ((A)<(B)) : ((A)>(B)))
+    if (cutter.unitPower > 0 && COOLERCMP(temp_cooler.raw, maxtemp_raw_COOLER)) max_temp_error(H_COOLER);
+    if (COOLERCMP(mintemp_raw_COOLER, temp_cooler.raw)) min_temp_error(H_COOLER);
+  #endif
 }
 
 #if THERMO_SEPARATE_SPI
@@ -2657,6 +2713,9 @@ void Temperature::disable_all_heaters() {
 
 /**
  * Update raw temperatures
+ *
+ * Called by ISR => readings_ready when new temperatures have been set by updateTemperaturesFromRawValues.
+ * Applies all the accumulators to the current raw temperatures.
  */
 void Temperature::update_raw_temperatures() {
 
@@ -2686,14 +2745,19 @@ void Temperature::update_raw_temperatures() {
   TERN_(HAS_JOY_ADC_X, joystick.x.update());
   TERN_(HAS_JOY_ADC_Y, joystick.y.update());
   TERN_(HAS_JOY_ADC_Z, joystick.z.update());
-
-  raw_temps_ready = true;
 }
 
+/**
+ * Called by the Temperature ISR when all the ADCs have been processed.
+ * Reset all the ADC accumulators for another round of updates.
+ */
 void Temperature::readings_ready() {
 
-  // Update the raw values if they've been read. Else we could be updating them during reading.
+  // Update raw values only if they're not already set.
-  if (!raw_temps_ready) update_raw_temperatures();
+  if (!raw_temps_ready) {
+    update_raw_temperatures();
+    raw_temps_ready = true;
+  }
 
   // Filament Sensor - can be read any time since IIR filtering is used
   TERN_(FILAMENT_WIDTH_SENSOR, filwidth.reading_ready());
@@ -2711,75 +2775,6 @@ void Temperature::readings_ready() {
   TERN_(HAS_JOY_ADC_X, joystick.x.reset());
   TERN_(HAS_JOY_ADC_Y, joystick.y.reset());
   TERN_(HAS_JOY_ADC_Z, joystick.z.reset());
-
-  #if HAS_HOTEND
-
-    static constexpr int8_t temp_dir[] = {
-      TERN(TEMP_SENSOR_0_IS_MAX_TC, 0, TEMPDIR(0))
-      #if HAS_MULTI_HOTEND
-        , TERN(TEMP_SENSOR_1_IS_MAX_TC, 0, TEMPDIR(1))
-        #if HOTENDS > 2
-          #define _TEMPDIR(N) , TEMPDIR(N)
-          REPEAT_S(2, HOTENDS, _TEMPDIR)
-        #endif
-      #endif
-    };
-
-    LOOP_L_N(e, COUNT(temp_dir)) {
-      const int8_t tdir = temp_dir[e];
-      if (tdir) {
-        const int16_t rawtemp = temp_hotend[e].raw * tdir; // normal direction, +rawtemp, else -rawtemp
-        if (rawtemp > temp_range[e].raw_max * tdir) max_temp_error((heater_id_t)e);
-
-        const bool heater_on = (temp_hotend[e].target > 0 || TERN0(PIDTEMP, temp_hotend[e].soft_pwm_amount > 0));
-        if (heater_on && rawtemp < temp_range[e].raw_min * tdir && !is_preheating(e)) {
-          #if MAX_CONSECUTIVE_LOW_TEMPERATURE_ERROR_ALLOWED > 1
-            if (++consecutive_low_temperature_error[e] >= MAX_CONSECUTIVE_LOW_TEMPERATURE_ERROR_ALLOWED)
-          #endif
-              min_temp_error((heater_id_t)e);
-        }
-        #if MAX_CONSECUTIVE_LOW_TEMPERATURE_ERROR_ALLOWED > 1
-          else
-            consecutive_low_temperature_error[e] = 0;
-        #endif
-      }
-    }
-
-  #endif // HAS_HOTEND
-
-  #if ENABLED(THERMAL_PROTECTION_BED)
-    #if TEMPDIR(BED) < 0
-      #define BEDCMP(A,B) ((A)<(B))
-    #else
-      #define BEDCMP(A,B) ((A)>(B))
-    #endif
-    const bool bed_on = (temp_bed.target > 0) || TERN0(PIDTEMPBED, temp_bed.soft_pwm_amount > 0);
-    if (BEDCMP(temp_bed.raw, maxtemp_raw_BED)) max_temp_error(H_BED);
-    if (bed_on && BEDCMP(mintemp_raw_BED, temp_bed.raw)) min_temp_error(H_BED);
-  #endif
-
-  #if BOTH(HAS_HEATED_CHAMBER, THERMAL_PROTECTION_CHAMBER)
-    #if TEMPDIR(CHAMBER) < 0
-      #define CHAMBERCMP(A,B) ((A)<(B))
-    #else
-      #define CHAMBERCMP(A,B) ((A)>(B))
-    #endif
-    const bool chamber_on = (temp_chamber.target > 0);
-    if (CHAMBERCMP(temp_chamber.raw, maxtemp_raw_CHAMBER)) max_temp_error(H_CHAMBER);
-    if (chamber_on && CHAMBERCMP(mintemp_raw_CHAMBER, temp_chamber.raw)) min_temp_error(H_CHAMBER);
-  #endif
-
-  #if BOTH(HAS_COOLER, THERMAL_PROTECTION_COOLER)
-    #if TEMPDIR(COOLER) < 0
-      #define COOLERCMP(A,B) ((A)<(B))
-    #else
-      #define COOLERCMP(A,B) ((A)>(B))
-    #endif
-    if (cutter.unitPower > 0) {
-      if (COOLERCMP(temp_cooler.raw, maxtemp_raw_COOLER)) max_temp_error(H_COOLER);
-    }
-    if (COOLERCMP(mintemp_raw_COOLER, temp_cooler.raw)) min_temp_error(H_COOLER);
-  #endif
 }
 
 /**

Marlin/src/module/temperature.h

@@ -419,8 +419,6 @@ class Temperature {
 
   private:
 
-    static volatile bool raw_temps_ready;
-
     #if ENABLED(WATCH_HOTENDS)
       static hotend_watch_t watch_hotend[HOTENDS];
     #endif
@@ -880,9 +878,19 @@ class Temperature {
     #endif
 
   private:
+
+    // Reading raw temperatures and converting to Celsius when ready
+    static volatile bool raw_temps_ready;
     static void update_raw_temperatures();
     static void updateTemperaturesFromRawValues();
+    static inline bool updateTemperaturesIfReady() {
+      if (!raw_temps_ready) return false;
+      updateTemperaturesFromRawValues();
+      raw_temps_ready = false;
+      return true;
+    }
 
+    // MAX Thermocouples
     #if HAS_MAX_TC
       #define MAX_TC_COUNT 1 + BOTH(TEMP_SENSOR_0_IS_MAX_TC, TEMP_SENSOR_1_IS_MAX_TC)
       #if MAX_TC_COUNT > 1