Fix interrupt-based endstop detection

- Also implemented real endstop reading on interrupt.

Marlin/src/HAL/HAL_AVR/endstop_interrupts.h

@@ -24,7 +24,7 @@
  * Endstop Interrupts
  *
  * Without endstop interrupts the endstop pins must be polled continually in
- * the stepper-ISR via endstops.update(), most of the time finding no change.
+ * the temperature-ISR via endstops.update(), most of the time finding no change.
  * With this feature endstops.update() is called only when we know that at
  * least one endstop has changed state, saving valuable CPU cycles.
  *
@@ -40,17 +40,10 @@
 
 #include "../../core/macros.h"
 #include <stdint.h>
-
-volatile uint8_t e_hit = 0; // Different from 0 when the endstops should be tested in detail.
-                            // Must be reset to 0 by the test function when finished.
-
-// This is what is really done inside the interrupts.
-FORCE_INLINE void endstop_ISR_worker( void ) {
-  e_hit = 2; // Because the detection of a e-stop hit has a 1 step debouncer it has to be called at least twice.
-}
+#include "../../module/endstops.h"
 
 // One ISR for all EXT-Interrupts
-void endstop_ISR(void) { endstop_ISR_worker(); }
+void endstop_ISR(void) { endstops.check_possible_change(); }
 
 /**
  * Patch for pins_arduino.h (...\Arduino\hardware\arduino\avr\variants\mega\pins_arduino.h)
@@ -95,19 +88,19 @@ void pciSetup(const int8_t pin) {
 
 // Handlers for pin change interrupts
 #ifdef PCINT0_vect
-  ISR(PCINT0_vect) { endstop_ISR_worker(); }
+  ISR(PCINT0_vect) { endstop_ISR(); }
 #endif
 
 #ifdef PCINT1_vect
-  ISR(PCINT1_vect) { endstop_ISR_worker(); }
+  ISR(PCINT1_vect) { endstop_ISR(); }
 #endif
 
 #ifdef PCINT2_vect
-  ISR(PCINT2_vect) { endstop_ISR_worker(); }
+  ISR(PCINT2_vect) { endstop_ISR(); }
 #endif
 
 #ifdef PCINT3_vect
-  ISR(PCINT3_vect) { endstop_ISR_worker(); }
+  ISR(PCINT3_vect) { endstop_ISR(); }
 #endif
 
 void setup_endstop_interrupts( void ) {

Marlin/src/HAL/HAL_DUE/endstop_interrupts.h

@@ -24,7 +24,7 @@
  * Endstop Interrupts
  *
  * Without endstop interrupts the endstop pins must be polled continually in
- * the stepper-ISR via endstops.update(), most of the time finding no change.
+ * the temperature-ISR via endstops.update(), most of the time finding no change.
  * With this feature endstops.update() is called only when we know that at
  * least one endstop has changed state, saving valuable CPU cycles.
  *
@@ -37,16 +37,10 @@
 #ifndef _ENDSTOP_INTERRUPTS_H_
 #define _ENDSTOP_INTERRUPTS_H_
 
-volatile uint8_t e_hit = 0; // Different from 0 when the endstops should be tested in detail.
-                            // Must be reset to 0 by the test function when finished.
-
-// This is what is really done inside the interrupts.
-FORCE_INLINE void endstop_ISR_worker( void ) {
-  e_hit = 2; // Because the detection of a e-stop hit has a 1 step debouncer it has to be called at least twice.
-}
+#include "../../module/endstops.h"
 
 // One ISR for all EXT-Interrupts
-void endstop_ISR(void) { endstop_ISR_worker(); }
+void endstop_ISR(void) { endstops.check_possible_change(); }
 
 /**
  *  Endstop interrupts for Due based targets.

Marlin/src/HAL/HAL_LPC1768/endstop_interrupts.h

@@ -24,7 +24,7 @@
  * Endstop Interrupts
  *
  * Without endstop interrupts the endstop pins must be polled continually in
- * the stepper-ISR via endstops.update(), most of the time finding no change.
+ * the temperature-ISR via endstops.update(), most of the time finding no change.
  * With this feature endstops.update() is called only when we know that at
  * least one endstop has changed state, saving valuable CPU cycles.
  *
@@ -40,16 +40,10 @@
 //Currently this is untested and broken
 #error "Please disable Endstop Interrupts LPC176x is currently an unsupported platform"
 
-volatile uint8_t e_hit = 0; // Different from 0 when the endstops should be tested in detail.
-                            // Must be reset to 0 by the test function when finished.
-
-// This is what is really done inside the interrupts.
-FORCE_INLINE void endstop_ISR_worker( void ) {
-  e_hit = 2; // Because the detection of a e-stop hit has a 1 step debouncer it has to be called at least twice.
-}
+#include "../../module/endstops.h"
 
 // One ISR for all EXT-Interrupts
-void endstop_ISR(void) { endstop_ISR_worker(); }
+void endstop_ISR(void) { endstops.check_possible_change(); }
 
 void setup_endstop_interrupts(void) {
   #if HAS_X_MAX

Marlin/src/HAL/HAL_STM32F1/endstop_interrupts.h

@@ -36,7 +36,7 @@
  * Endstop Interrupts
  *
  * Without endstop interrupts the endstop pins must be polled continually in
- * the stepper-ISR via endstops.update(), most of the time finding no change.
+ * the temperature-ISR via endstops.update(), most of the time finding no change.
  * With this feature endstops.update() is called only when we know that at
  * least one endstop has changed state, saving valuable CPU cycles.
  *
@@ -49,16 +49,10 @@
 #ifndef _ENDSTOP_INTERRUPTS_H_
 #define _ENDSTOP_INTERRUPTS_H_
 
-volatile uint8_t e_hit = 0; // Different from 0 when the endstops should be tested in detail.
-                            // Must be reset to 0 by the test function when finished.
-
-// This is what is really done inside the interrupts.
-FORCE_INLINE void endstop_ISR_worker( void ) {
-  e_hit = 2; // Because the detection of a e-stop hit has a 1 step debouncer it has to be called at least twice.
-}
+#include "../../module/endstops.h"
 
 // One ISR for all EXT-Interrupts
-void endstop_ISR(void) { endstop_ISR_worker(); }
+void endstop_ISR(void) { endstops.check_possible_change(); }
 
 void setup_endstop_interrupts(void) {
   #if HAS_X_MAX

Marlin/src/HAL/HAL_STM32F4/endstop_interrupts.h

@@ -24,16 +24,10 @@
 #ifndef _ENDSTOP_INTERRUPTS_H_
 #define _ENDSTOP_INTERRUPTS_H_
 
-volatile uint8_t e_hit = 0; // Different from 0 when the endstops should be tested in detail.
-                            // Must be reset to 0 by the test function when finished.
-
-// This is what is really done inside the interrupts.
-FORCE_INLINE void endstop_ISR_worker( void ) {
-  e_hit = 2; // Because the detection of a e-stop hit has a 1 step debouncer it has to be called at least twice.
-}
+#include "../../module/endstops.h"
 
 // One ISR for all EXT-Interrupts
-void endstop_ISR(void) { endstop_ISR_worker(); }
+void endstop_ISR(void) { endstops.check_possible_change(); }
 
 void setup_endstop_interrupts(void) {
   #if HAS_X_MAX

Marlin/src/HAL/HAL_STM32F7/endstop_interrupts.h

@@ -26,16 +26,10 @@
 #ifndef _ENDSTOP_INTERRUPTS_H_
 #define _ENDSTOP_INTERRUPTS_H_
 
-volatile uint8_t e_hit = 0; // Different from 0 when the endstops should be tested in detail.
-                            // Must be reset to 0 by the test function when finished.
-
-// This is what is really done inside the interrupts.
-FORCE_INLINE void endstop_ISR_worker( void ) {
-  e_hit = 2; // Because the detection of a e-stop hit has a 1 step debouncer it has to be called at least twice.
-}
+#include "../../module/endstops.h"
 
 // One ISR for all EXT-Interrupts
-void endstop_ISR(void) { endstop_ISR_worker(); }
+void endstop_ISR(void) { endstops.check_possible_change(); }
 
 void setup_endstop_interrupts(void) {
   #if HAS_X_MAX

Marlin/src/HAL/HAL_TEENSY35_36/endstop_interrupts.h

@@ -24,7 +24,7 @@
  * Endstop Interrupts
  *
  * Without endstop interrupts the endstop pins must be polled continually in
- * the stepper-ISR via endstops.update(), most of the time finding no change.
+ * the temperature-ISR via endstops.update(), most of the time finding no change.
  * With this feature endstops.update() is called only when we know that at
  * least one endstop has changed state, saving valuable CPU cycles.
  *
@@ -37,16 +37,10 @@
 #ifndef _ENDSTOP_INTERRUPTS_H_
 #define _ENDSTOP_INTERRUPTS_H_
 
-volatile uint8_t e_hit = 0; // Different from 0 when the endstops should be tested in detail.
-                            // Must be reset to 0 by the test function when finished.
-
-// This is what is really done inside the interrupts.
-FORCE_INLINE void endstop_ISR_worker( void ) {
-  e_hit = 2; // Because the detection of a e-stop hit has a 1 step debouncer it has to be called at least twice.
-}
+#include "../../module/endstops.h"
 
 // One ISR for all EXT-Interrupts
-void endstop_ISR(void) { endstop_ISR_worker(); }
+void endstop_ISR(void) { endstops.check_possible_change(); }
 
 /**
  *  Endstop interrupts for Due based targets.

Marlin/src/Marlin.cpp

@@ -95,10 +95,6 @@
   #include "feature/I2CPositionEncoder.h"
 #endif
 
-#if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
-  #include HAL_PATH(HAL, endstop_interrupts.h)
-#endif
-
 #if HAS_TRINAMIC
   #include "feature/tmc_util.h"
 #endif
@@ -748,7 +744,9 @@ void setup() {
 
   print_job_timer.init();   // Initial setup of print job timer
 
-  stepper.init();    // Initialize stepper, this enables interrupts!
+  endstops.init();          // Init endstops and pullups
+
+  stepper.init();           // Init stepper. This enables interrupts!
 
   #if HAS_SERVOS
     servo_init();
@@ -860,10 +858,6 @@ void setup() {
     i2c.onRequest(i2c_on_request);
   #endif
 
-  #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
-    setup_endstop_interrupts();
-  #endif
-
   #if DO_SWITCH_EXTRUDER
     move_extruder_servo(0);  // Initialize extruder servo
   #endif

Marlin/src/module/endstops.cpp

@@ -32,18 +32,27 @@
 #include "../module/temperature.h"
 #include "../lcd/ultralcd.h"
 
-// TEST_ENDSTOP: test the old and the current status of an endstop
-#define TEST_ENDSTOP(ENDSTOP) (TEST(current_endstop_bits & old_endstop_bits, ENDSTOP))
+#if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
+  #include HAL_PATH(../HAL, endstop_interrupts.h)
+#endif
+
+// TEST_ENDSTOP: test the current status of an endstop
+#define TEST_ENDSTOP(ENDSTOP) (TEST(current_endstop_bits, ENDSTOP))
+
+#if HAS_BED_PROBE
+  #define ENDSTOPS_ENABLED  (endstops.enabled || endstops.z_probe_enabled)
+#else
+  #define ENDSTOPS_ENABLED  endstops.enabled
+#endif
 
 Endstops endstops;
 
 // public:
 
 bool Endstops::enabled, Endstops::enabled_globally; // Initialized by settings.load()
-volatile char Endstops::endstop_hit_bits; // use X_MIN, Y_MIN, Z_MIN and Z_MIN_PROBE as BIT value
+volatile uint8_t Endstops::endstop_hit_bits; // use X_MIN, Y_MIN, Z_MIN and Z_MIN_PROBE as BIT value
 
-Endstops::esbits_t Endstops::current_endstop_bits = 0,
-                   Endstops::old_endstop_bits = 0;
+Endstops::esbits_t Endstops::current_endstop_bits = 0;
 
 #if HAS_BED_PROBE
   volatile bool Endstops::z_probe_enabled = false;
@@ -196,8 +205,93 @@ void Endstops::init() {
     #endif
   #endif
 
+  #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
+    setup_endstop_interrupts();
+  #endif
+
+  // Enable endstops
+  enable_globally(
+    #if ENABLED(ENDSTOPS_ALWAYS_ON_DEFAULT)
+      true
+    #else
+      false
+    #endif
+  );
+
 } // Endstops::init
 
+// Called from ISR. A change was detected. Find out what happened!
+void Endstops::check_possible_change() { if (ENDSTOPS_ENABLED) endstops.update(); }
+
+// Called from ISR: Poll endstop state if required
+void Endstops::poll() {
+
+  #if ENABLED(PINS_DEBUGGING)
+    endstops.run_monitor();  // report changes in endstop status
+  #endif
+
+  #if DISABLED(ENDSTOP_INTERRUPTS_FEATURE)
+    if (ENDSTOPS_ENABLED) endstops.update();
+  #endif
+}
+
+void Endstops::enable_globally(const bool onoff) {
+  enabled_globally = enabled = onoff;
+
+  #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
+    if (onoff) endstops.update(); // If enabling, update state now
+  #endif
+}
+
+// Enable / disable endstop checking
+void Endstops::enable(const bool onoff) {
+  enabled = onoff;
+
+  #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
+    if (onoff) endstops.update(); // If enabling, update state now
+  #endif
+}
+
+
+// Disable / Enable endstops based on ENSTOPS_ONLY_FOR_HOMING and global enable
+void Endstops::not_homing() {
+  enabled = enabled_globally;
+
+  #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
+    if (enabled) endstops.update(); // If enabling, update state now
+  #endif
+}
+
+// Clear endstops (i.e., they were hit intentionally) to suppress the report
+void Endstops::hit_on_purpose() {
+  endstop_hit_bits = 0;
+
+  #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
+    if (enabled) endstops.update(); // If enabling, update state now
+  #endif
+}
+
+// Enable / disable endstop z-probe checking
+#if HAS_BED_PROBE
+  void Endstops::enable_z_probe(bool onoff) {
+    z_probe_enabled = onoff;
+
+    #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
+      if (enabled) endstops.update(); // If enabling, update state now
+    #endif
+  }
+#endif
+
+#if ENABLED(PINS_DEBUGGING)
+  void Endstops::run_monitor() {
+    if (!monitor_flag) return;
+    static uint8_t monitor_count = 16;  // offset this check from the others
+    monitor_count += _BV(1);            //  15 Hz
+    monitor_count &= 0x7F;
+    if (!monitor_count) monitor();      // report changes in endstop status
+  }
+#endif
+
 void Endstops::report_state() {
   if (endstop_hit_bits) {
     #if ENABLED(ULTRA_LCD)
@@ -300,38 +394,41 @@ void Endstops::M119() {
   #endif
 } // Endstops::M119
 
+// The following routines are called from an ISR context. It could be the temperature ISR, the
+// endstop ISR or the Stepper ISR.
+
 #if ENABLED(X_DUAL_ENDSTOPS)
   void Endstops::test_dual_x_endstops(const EndstopEnum es1, const EndstopEnum es2) {
     const byte x_test = TEST_ENDSTOP(es1) | (TEST_ENDSTOP(es2) << 1); // bit 0 for X, bit 1 for X2
-    if (x_test && stepper.current_block->steps[X_AXIS] > 0) {
+    if (x_test && stepper.movement_non_null(X_AXIS)) {
       SBI(endstop_hit_bits, X_MIN);
       if (!stepper.performing_homing || (x_test == 0x3))  //if not performing home or if both endstops were trigged during homing...
-        stepper.kill_current_block();
+        stepper.quick_stop();
     }
   }
 #endif
 #if ENABLED(Y_DUAL_ENDSTOPS)
   void Endstops::test_dual_y_endstops(const EndstopEnum es1, const EndstopEnum es2) {
     const byte y_test = TEST_ENDSTOP(es1) | (TEST_ENDSTOP(es2) << 1); // bit 0 for Y, bit 1 for Y2
-    if (y_test && stepper.current_block->steps[Y_AXIS] > 0) {
+    if (y_test && stepper.movement_non_null(Y_AXIS)) {
       SBI(endstop_hit_bits, Y_MIN);
       if (!stepper.performing_homing || (y_test == 0x3))  //if not performing home or if both endstops were trigged during homing...
-        stepper.kill_current_block();
+        stepper.quick_stop();
     }
   }
 #endif
 #if ENABLED(Z_DUAL_ENDSTOPS)
   void Endstops::test_dual_z_endstops(const EndstopEnum es1, const EndstopEnum es2) {
     const byte z_test = TEST_ENDSTOP(es1) | (TEST_ENDSTOP(es2) << 1); // bit 0 for Z, bit 1 for Z2
-    if (z_test && stepper.current_block->steps[Z_AXIS] > 0) {
+    if (z_test && stepper.movement_non_null(Z_AXIS)) {
       SBI(endstop_hit_bits, Z_MIN);
       if (!stepper.performing_homing || (z_test == 0x3))  //if not performing home or if both endstops were trigged during homing...
-        stepper.kill_current_block();
+        stepper.quick_stop();
     }
   }
 #endif
 
-// Check endstops - Called from ISR!
+// Check endstops - Could be called from ISR!
 void Endstops::update() {
 
   #define _ENDSTOP(AXIS, MINMAX) AXIS ##_## MINMAX
@@ -358,9 +455,9 @@ void Endstops::update() {
     if (G38_move) {
       UPDATE_ENDSTOP_BIT(Z, MIN_PROBE);
       if (TEST_ENDSTOP(_ENDSTOP(Z, MIN_PROBE))) {
-        if      (stepper.current_block->steps[_AXIS(X)] > 0) { _ENDSTOP_HIT(X, MIN); planner.endstop_triggered(_AXIS(X)); }
-        else if (stepper.current_block->steps[_AXIS(Y)] > 0) { _ENDSTOP_HIT(Y, MIN); planner.endstop_triggered(_AXIS(Y)); }
-        else if (stepper.current_block->steps[_AXIS(Z)] > 0) { _ENDSTOP_HIT(Z, MIN); planner.endstop_triggered(_AXIS(Z)); }
+        if      (stepper.movement_non_null(_AXIS(X))) { _ENDSTOP_HIT(X, MIN); planner.endstop_triggered(_AXIS(X)); }
+        else if (stepper.movement_non_null(_AXIS(Y))) { _ENDSTOP_HIT(Y, MIN); planner.endstop_triggered(_AXIS(Y)); }
+        else if (stepper.movement_non_null(_AXIS(Z))) { _ENDSTOP_HIT(Z, MIN); planner.endstop_triggered(_AXIS(Z)); }
         G38_endstop_hit = true;
       }
     }
@@ -371,7 +468,7 @@ void Endstops::update() {
    */
 
   #if IS_CORE
-    #define S_(N) stepper.current_block->steps[CORE_AXIS_##N]
+    #define S_(N) stepper.movement_non_null(CORE_AXIS_##N)
     #define D_(N) stepper.motor_direction(CORE_AXIS_##N)
   #endif
 
@@ -391,7 +488,7 @@ void Endstops::update() {
     #define X_MOVE_TEST ( S_(1) != S_(2) || (S_(1) > 0 && D_(1) X_CMP D_(2)) )
     #define X_AXIS_HEAD X_HEAD
   #else
-    #define X_MOVE_TEST stepper.current_block->steps[X_AXIS] > 0
+    #define X_MOVE_TEST stepper.movement_non_null(X_AXIS)
     #define X_AXIS_HEAD X_AXIS
   #endif
 
@@ -411,7 +508,7 @@ void Endstops::update() {
     #define Y_MOVE_TEST ( S_(1) != S_(2) || (S_(1) > 0 && D_(1) Y_CMP D_(2)) )
     #define Y_AXIS_HEAD Y_HEAD
   #else
-    #define Y_MOVE_TEST stepper.current_block->steps[Y_AXIS] > 0
+    #define Y_MOVE_TEST stepper.movement_non_null(Y_AXIS)
     #define Y_AXIS_HEAD Y_AXIS
   #endif
 
@@ -431,13 +528,13 @@ void Endstops::update() {
     #define Z_MOVE_TEST ( S_(1) != S_(2) || (S_(1) > 0 && D_(1) Z_CMP D_(2)) )
     #define Z_AXIS_HEAD Z_HEAD
   #else
-    #define Z_MOVE_TEST stepper.current_block->steps[Z_AXIS] > 0
+    #define Z_MOVE_TEST stepper.movement_non_null(Z_AXIS)
     #define Z_AXIS_HEAD Z_AXIS
   #endif
 
   // With Dual X, endstops are only checked in the homing direction for the active extruder
   #if ENABLED(DUAL_X_CARRIAGE)
-    #define E0_ACTIVE stepper.current_block->active_extruder == 0
+    #define E0_ACTIVE stepper.movement_extruder() == 0
     #define X_MIN_TEST ((X_HOME_DIR < 0 && E0_ACTIVE) || (X2_HOME_DIR < 0 && !E0_ACTIVE))
     #define X_MAX_TEST ((X_HOME_DIR > 0 && E0_ACTIVE) || (X2_HOME_DIR > 0 && !E0_ACTIVE))
   #else
@@ -448,126 +545,119 @@ void Endstops::update() {
   /**
    * Check and update endstops according to conditions
    */
-  if (stepper.current_block) {
-
-    if (X_MOVE_TEST) {
-      if (stepper.motor_direction(X_AXIS_HEAD)) { // -direction
-        #if HAS_X_MIN
-          #if ENABLED(X_DUAL_ENDSTOPS)
-            UPDATE_ENDSTOP_BIT(X, MIN);
-            #if HAS_X2_MIN
-              UPDATE_ENDSTOP_BIT(X2, MIN);
-            #else
-              COPY_BIT(current_endstop_bits, X_MIN, X2_MIN);
-            #endif
-            test_dual_x_endstops(X_MIN, X2_MIN);
+  if (X_MOVE_TEST) {
+    if (stepper.motor_direction(X_AXIS_HEAD)) { // -direction
+      #if HAS_X_MIN
+        #if ENABLED(X_DUAL_ENDSTOPS)
+          UPDATE_ENDSTOP_BIT(X, MIN);
+          #if HAS_X2_MIN
+            UPDATE_ENDSTOP_BIT(X2, MIN);
           #else
-            if (X_MIN_TEST) UPDATE_ENDSTOP(X, MIN);
+            COPY_BIT(current_endstop_bits, X_MIN, X2_MIN);
           #endif
+          test_dual_x_endstops(X_MIN, X2_MIN);
+        #else
+          if (X_MIN_TEST) UPDATE_ENDSTOP(X, MIN);
         #endif
-      }
-      else { // +direction
-        #if HAS_X_MAX
-          #if ENABLED(X_DUAL_ENDSTOPS)
-            UPDATE_ENDSTOP_BIT(X, MAX);
-            #if HAS_X2_MAX
-              UPDATE_ENDSTOP_BIT(X2, MAX);
-            #else
-              COPY_BIT(current_endstop_bits, X_MAX, X2_MAX);
-            #endif
-            test_dual_x_endstops(X_MAX, X2_MAX);
+      #endif
+    }
+    else { // +direction
+      #if HAS_X_MAX
+        #if ENABLED(X_DUAL_ENDSTOPS)
+          UPDATE_ENDSTOP_BIT(X, MAX);
+          #if HAS_X2_MAX
+            UPDATE_ENDSTOP_BIT(X2, MAX);
           #else
-            if (X_MAX_TEST) UPDATE_ENDSTOP(X, MAX);
+            COPY_BIT(current_endstop_bits, X_MAX, X2_MAX);
           #endif
+          test_dual_x_endstops(X_MAX, X2_MAX);
+        #else
+          if (X_MAX_TEST) UPDATE_ENDSTOP(X, MAX);
         #endif
-      }
+      #endif
     }
+  }
 
-    if (Y_MOVE_TEST) {
-      if (stepper.motor_direction(Y_AXIS_HEAD)) { // -direction
-        #if HAS_Y_MIN
-          #if ENABLED(Y_DUAL_ENDSTOPS)
-            UPDATE_ENDSTOP_BIT(Y, MIN);
-            #if HAS_Y2_MIN
-              UPDATE_ENDSTOP_BIT(Y2, MIN);
-            #else
-              COPY_BIT(current_endstop_bits, Y_MIN, Y2_MIN);
-            #endif
-            test_dual_y_endstops(Y_MIN, Y2_MIN);
+  if (Y_MOVE_TEST) {
+    if (stepper.motor_direction(Y_AXIS_HEAD)) { // -direction
+      #if HAS_Y_MIN
+        #if ENABLED(Y_DUAL_ENDSTOPS)
+          UPDATE_ENDSTOP_BIT(Y, MIN);
+          #if HAS_Y2_MIN
+            UPDATE_ENDSTOP_BIT(Y2, MIN);
           #else
-            UPDATE_ENDSTOP(Y, MIN);
+            COPY_BIT(current_endstop_bits, Y_MIN, Y2_MIN);
           #endif
+          test_dual_y_endstops(Y_MIN, Y2_MIN);
+        #else
+          UPDATE_ENDSTOP(Y, MIN);
         #endif
-      }
-      else { // +direction
-        #if HAS_Y_MAX
-          #if ENABLED(Y_DUAL_ENDSTOPS)
-            UPDATE_ENDSTOP_BIT(Y, MAX);
-            #if HAS_Y2_MAX
-              UPDATE_ENDSTOP_BIT(Y2, MAX);
-            #else
-              COPY_BIT(current_endstop_bits, Y_MAX, Y2_MAX);
-            #endif
-            test_dual_y_endstops(Y_MAX, Y2_MAX);
+      #endif
+    }
+    else { // +direction
+      #if HAS_Y_MAX
+        #if ENABLED(Y_DUAL_ENDSTOPS)
+          UPDATE_ENDSTOP_BIT(Y, MAX);
+          #if HAS_Y2_MAX
+            UPDATE_ENDSTOP_BIT(Y2, MAX);
           #else
-            UPDATE_ENDSTOP(Y, MAX);
+            COPY_BIT(current_endstop_bits, Y_MAX, Y2_MAX);
           #endif
+          test_dual_y_endstops(Y_MAX, Y2_MAX);
+        #else
+          UPDATE_ENDSTOP(Y, MAX);
         #endif
-      }
+      #endif
     }
+  }
 
-    if (Z_MOVE_TEST) {
-      if (stepper.motor_direction(Z_AXIS_HEAD)) { // Z -direction. Gantry down, bed up.
-        #if HAS_Z_MIN
-          #if ENABLED(Z_DUAL_ENDSTOPS)
-            UPDATE_ENDSTOP_BIT(Z, MIN);
-            #if HAS_Z2_MIN
-              UPDATE_ENDSTOP_BIT(Z2, MIN);
-            #else
-              COPY_BIT(current_endstop_bits, Z_MIN, Z2_MIN);
-            #endif
-            test_dual_z_endstops(Z_MIN, Z2_MIN);
+  if (Z_MOVE_TEST) {
+    if (stepper.motor_direction(Z_AXIS_HEAD)) { // Z -direction. Gantry down, bed up.
+      #if HAS_Z_MIN
+        #if ENABLED(Z_DUAL_ENDSTOPS)
+          UPDATE_ENDSTOP_BIT(Z, MIN);
+          #if HAS_Z2_MIN
+            UPDATE_ENDSTOP_BIT(Z2, MIN);
+          #else
+            COPY_BIT(current_endstop_bits, Z_MIN, Z2_MIN);
+          #endif
+          test_dual_z_endstops(Z_MIN, Z2_MIN);
+        #else
+          #if ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)
+            if (z_probe_enabled) UPDATE_ENDSTOP(Z, MIN);
           #else
-            #if ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)
-              if (z_probe_enabled) UPDATE_ENDSTOP(Z, MIN);
-            #else
-              UPDATE_ENDSTOP(Z, MIN);
-            #endif
+            UPDATE_ENDSTOP(Z, MIN);
           #endif
         #endif
+      #endif
 
-        // When closing the gap check the enabled probe
-        #if ENABLED(Z_MIN_PROBE_ENDSTOP)
-          if (z_probe_enabled) {
-            UPDATE_ENDSTOP(Z, MIN_PROBE);
-            if (TEST_ENDSTOP(Z_MIN_PROBE)) SBI(endstop_hit_bits, Z_MIN_PROBE);
-          }
-        #endif
-      }
-      else { // Z +direction. Gantry up, bed down.
-        #if HAS_Z_MAX
-          // Check both Z dual endstops
-          #if ENABLED(Z_DUAL_ENDSTOPS)
-            UPDATE_ENDSTOP_BIT(Z, MAX);
-            #if HAS_Z2_MAX
-              UPDATE_ENDSTOP_BIT(Z2, MAX);
-            #else
-              COPY_BIT(current_endstop_bits, Z_MAX, Z2_MAX);
-            #endif
-            test_dual_z_endstops(Z_MAX, Z2_MAX);
-          // If this pin is not hijacked for the bed probe
-          // then it belongs to the Z endstop
-          #elif DISABLED(Z_MIN_PROBE_ENDSTOP) || Z_MAX_PIN != Z_MIN_PROBE_PIN
-            UPDATE_ENDSTOP(Z, MAX);
+      // When closing the gap check the enabled probe
+      #if ENABLED(Z_MIN_PROBE_ENDSTOP)
+        if (z_probe_enabled) {
+          UPDATE_ENDSTOP(Z, MIN_PROBE);
+          if (TEST_ENDSTOP(Z_MIN_PROBE)) SBI(endstop_hit_bits, Z_MIN_PROBE);
+        }
+      #endif
+    }
+    else { // Z +direction. Gantry up, bed down.
+      #if HAS_Z_MAX
+        // Check both Z dual endstops
+        #if ENABLED(Z_DUAL_ENDSTOPS)
+          UPDATE_ENDSTOP_BIT(Z, MAX);
+          #if HAS_Z2_MAX
+            UPDATE_ENDSTOP_BIT(Z2, MAX);
+          #else
+            COPY_BIT(current_endstop_bits, Z_MAX, Z2_MAX);
           #endif
+          test_dual_z_endstops(Z_MAX, Z2_MAX);
+        // If this pin is not hijacked for the bed probe
+        // then it belongs to the Z endstop
+        #elif DISABLED(Z_MIN_PROBE_ENDSTOP) || Z_MAX_PIN != Z_MIN_PROBE_PIN
+          UPDATE_ENDSTOP(Z, MAX);
         #endif
-      }
+      #endif
     }
-
-  } // stepper.current_block
-
-  old_endstop_bits = current_endstop_bits;
-
+  }
 } // Endstops::update()
 
 #if ENABLED(PINS_DEBUGGING)

Marlin/src/module/endstops.h

@@ -51,7 +51,7 @@ class Endstops {
   public:
 
     static bool enabled, enabled_globally;
-    static volatile char endstop_hit_bits; // use X_MIN, Y_MIN, Z_MIN and Z_MIN_PROBE as BIT value
+    static volatile uint8_t endstop_hit_bits; // use X_MIN, Y_MIN, Z_MIN and Z_MIN_PROBE as BIT value
 
     #if ENABLED(X_DUAL_ENDSTOPS) || ENABLED(Y_DUAL_ENDSTOPS) || ENABLED(Z_DUAL_ENDSTOPS)
       typedef uint16_t esbits_t;
@@ -68,17 +68,9 @@ class Endstops {
       typedef byte esbits_t;
     #endif
 
-    static esbits_t current_endstop_bits, old_endstop_bits;
+    static esbits_t current_endstop_bits;
 
-    Endstops() {
-      enable_globally(
-        #if ENABLED(ENDSTOPS_ALWAYS_ON_DEFAULT)
-          true
-        #else
-          false
-        #endif
-      );
-    };
+    Endstops() {};
 
     /**
      * Initialize the endstop pins
@@ -86,6 +78,17 @@ class Endstops {
     static void init();
 
     /**
+     * A change was detected or presumed to be in endstops pins. Find out what
+     * changed, if anything. Called from ISR contexts
+     */
+    static void check_possible_change();
+
+    /**
+     * Periodic call to poll endstops if required. Called from temperature ISR
+     */
+    static void poll();
+
+    /**
      * Update the endstops bits from the pins
      */
     static void update();
@@ -101,34 +104,28 @@ class Endstops {
     static void M119();
 
     // Enable / disable endstop checking globally
-    static void enable_globally(bool onoff=true) { enabled_globally = enabled = onoff; }
+    static void enable_globally(const bool onoff=true);
 
     // Enable / disable endstop checking
-    static void enable(bool onoff=true) { enabled = onoff; }
+    static void enable(const bool onoff=true);
 
     // Disable / Enable endstops based on ENSTOPS_ONLY_FOR_HOMING and global enable
-    static void not_homing() { enabled = enabled_globally; }
+    static void not_homing();
 
     // Clear endstops (i.e., they were hit intentionally) to suppress the report
-    static void hit_on_purpose() { endstop_hit_bits = 0; }
+    static void hit_on_purpose();
 
     // Enable / disable endstop z-probe checking
     #if HAS_BED_PROBE
       static volatile bool z_probe_enabled;
-      static void enable_z_probe(bool onoff=true) { z_probe_enabled = onoff; }
+      static void enable_z_probe(bool onoff=true);
     #endif
 
     // Debugging of endstops
     #if ENABLED(PINS_DEBUGGING)
       static bool monitor_flag;
       static void monitor();
-      FORCE_INLINE static void run_monitor() {
-        if (!monitor_flag) return;
-        static uint8_t monitor_count = 16;  // offset this check from the others
-        monitor_count += _BV(1);            //  15 Hz
-        monitor_count &= 0x7F;
-        if (!monitor_count) monitor();      // report changes in endstop status
-      }
+      static void run_monitor();
     #endif
 
   private:
@@ -146,10 +143,4 @@ class Endstops {
 
 extern Endstops endstops;
 
-#if HAS_BED_PROBE
-  #define ENDSTOPS_ENABLED  (endstops.enabled || endstops.z_probe_enabled)
-#else
-  #define ENDSTOPS_ENABLED  endstops.enabled
-#endif
-
 #endif // __ENDSTOPS_H__

Marlin/src/module/planner.cpp

@@ -758,8 +758,8 @@ void Planner::calculate_trapezoid_for_block(block_t* const block, const float &e
            final_rate = CEIL(block->nominal_rate * exit_factor); // (steps per second)
 
   // Limit minimal step rate (Otherwise the timer will overflow.)
-  NOLESS(initial_rate, MINIMAL_STEP_RATE);
-  NOLESS(final_rate, MINIMAL_STEP_RATE);
+  NOLESS(initial_rate, uint32_t(MINIMAL_STEP_RATE));
+  NOLESS(final_rate, uint32_t(MINIMAL_STEP_RATE));
 
   #if ENABLED(BEZIER_JERK_CONTROL)
     uint32_t cruise_rate = initial_rate;
@@ -1467,23 +1467,8 @@ void Planner::quick_stop() {
 }
 
 void Planner::endstop_triggered(const AxisEnum axis) {
-
-  /*NB: This will be called via endstops.update()
-    and endstops.update() can be called from the temperature
-    ISR. So Stepper interrupts are enabled */
-
-  // Disable stepper ISR
-  bool stepper_isr_enabled = STEPPER_ISR_ENABLED();
-  DISABLE_STEPPER_DRIVER_INTERRUPT();
-
-  // Record stepper position
+  // Record stepper position and discard the current block
   stepper.endstop_triggered(axis);
-
-  // Discard the active block that led to the trigger
-  discard_current_block();
-
-  // Reenable stepper ISR if it was enabled
-  if (stepper_isr_enabled) ENABLE_STEPPER_DRIVER_INTERRUPT();
 }
 
 float Planner::triggered_position_mm(const AxisEnum axis) {
@@ -1682,7 +1667,7 @@ bool Planner::_populate_block(block_t * const block, bool split_move,
   if (de < 0) SBI(dm, E_AXIS);
 
   const float esteps_float = de * e_factor[extruder];
-  const int32_t esteps = ABS(esteps_float) + 0.5;
+  const uint32_t esteps = ABS(esteps_float) + 0.5;
 
   // Clear all flags, including the "busy" bit
   block->flag = 0x00;

Marlin/src/module/stepper.cpp

@@ -96,7 +96,10 @@ block_t* Stepper::current_block = NULL;  // A pointer to the block currently bei
 
 // private:
 
-uint8_t Stepper::last_direction_bits = 0;        // The next stepping-bits to be output
+uint8_t Stepper::last_direction_bits = 0,       // The next stepping-bits to be output
+        Stepper::last_movement_extruder = 0xFF; // Last movement extruder, as computed when the last movement was fetched from planner
+bool Stepper::abort_current_block,              // Signals to the stepper that current block should be aborted
+     Stepper::last_movement_non_null[NUM_AXIS]; // Last Movement in the given direction is not null, as computed when the last movement was fetched from planner
 
 #if ENABLED(X_DUAL_ENDSTOPS)
   bool Stepper::locked_x_motor = false, Stepper::locked_x2_motor = false;
@@ -181,12 +184,12 @@ volatile int32_t Stepper::endstops_trigsteps[XYZ];
   #define DUAL_ENDSTOP_APPLY_STEP(A,V)                                                                                                           \
     if (performing_homing) {                                                                                                                        \
       if (A##_HOME_DIR < 0) {                                                                                                                    \
-        if (!(TEST(endstops.old_endstop_bits, A##_MIN) && count_direction[_AXIS(A)] < 0) && !LOCKED_##A##_MOTOR) A##_STEP_WRITE(V);     \
-        if (!(TEST(endstops.old_endstop_bits, A##2_MIN) && count_direction[_AXIS(A)] < 0) && !LOCKED_##A##2_MOTOR) A##2_STEP_WRITE(V);  \
+        if (!(TEST(endstops.current_endstop_bits, A##_MIN) && count_direction[_AXIS(A)] < 0) && !LOCKED_##A##_MOTOR) A##_STEP_WRITE(V);     \
+        if (!(TEST(endstops.current_endstop_bits, A##2_MIN) && count_direction[_AXIS(A)] < 0) && !LOCKED_##A##2_MOTOR) A##2_STEP_WRITE(V);  \
       }                                                                                                                                             \
       else {                                                                                                                                        \
-        if (!(TEST(endstops.old_endstop_bits, A##_MAX) && count_direction[_AXIS(A)] > 0) && !LOCKED_##A##_MOTOR) A##_STEP_WRITE(V);     \
-        if (!(TEST(endstops.old_endstop_bits, A##2_MAX) && count_direction[_AXIS(A)] > 0) && !LOCKED_##A##2_MOTOR) A##2_STEP_WRITE(V);  \
+        if (!(TEST(endstops.current_endstop_bits, A##_MAX) && count_direction[_AXIS(A)] > 0) && !LOCKED_##A##_MOTOR) A##_STEP_WRITE(V);     \
+        if (!(TEST(endstops.current_endstop_bits, A##2_MAX) && count_direction[_AXIS(A)] > 0) && !LOCKED_##A##2_MOTOR) A##2_STEP_WRITE(V);  \
       }                                                                                                                                             \
     }                                                                                                                                               \
     else {                                                                                                                                          \
@@ -315,10 +318,6 @@ void Stepper::set_directions() {
   #endif // !LIN_ADVANCE
 }
 
-#if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
-  extern volatile uint8_t e_hit;
-#endif
-
 #if ENABLED(BEZIER_JERK_CONTROL)
   /**
    *   We are using a quintic (fifth-degree) Bézier polynomial for the velocity curve.
@@ -1229,6 +1228,15 @@ hal_timer_t Stepper::isr_scheduler() {
 // as constant as possible!!!!
 void Stepper::stepper_pulse_phase_isr() {
 
+  // If we must abort the current block, do so!
+  if (abort_current_block) {
+    abort_current_block = false;
+    if (current_block) {
+      current_block = NULL;
+      planner.discard_current_block();
+    }
+  }
+
   // If there is no current block, do nothing
   if (!current_block) return;
 
@@ -1558,12 +1566,13 @@ uint32_t Stepper::stepper_block_phase_isr() {
           return interval; // No more queued movements!
       }
 
-      // Initialize the trapezoid generator from the current block.
-      static int8_t last_extruder = -1;
+      // Compute movement direction for proper endstop handling
+      LOOP_NA(i) last_movement_non_null[i] = !!current_block->steps[i];
 
+      // Initialize the trapezoid generator from the current block.
       #if ENABLED(LIN_ADVANCE)
         #if E_STEPPERS > 1
-          if (current_block->active_extruder != last_extruder) {
+          if (current_block->active_extruder != last_movement_extruder) {
             current_adv_steps = 0; // If the now active extruder wasn't in use during the last move, its pressure is most likely gone.
             LA_active_extruder = current_block->active_extruder;
           }
@@ -1576,12 +1585,21 @@ uint32_t Stepper::stepper_block_phase_isr() {
         }
       #endif
 
-      if (current_block->direction_bits != last_direction_bits || current_block->active_extruder != last_extruder) {
+      if (current_block->direction_bits != last_direction_bits || current_block->active_extruder != last_movement_extruder) {
         last_direction_bits = current_block->direction_bits;
-        last_extruder = current_block->active_extruder;
+        last_movement_extruder = current_block->active_extruder;
         set_directions();
       }
 
+      // At this point, we must ensure the movement about to execute isn't
+      // trying to force the head against a limit switch. If using interrupt-
+      // driven change detection, and already against a limit then no call to
+      // the endstop_triggered method will be done and the movement will be
+      // done against the endstop. So, check the limits here: If the movement
+      // is against the limits, the block will be marked as to be killed, and
+      // on the next call to this ISR, will be discarded.
+      endstops.check_possible_change();
+
       // No acceleration / deceleration time elapsed so far
       acceleration_time = deceleration_time = 0;
 
@@ -1614,11 +1632,6 @@ uint32_t Stepper::stepper_block_phase_isr() {
           counter_m[i] = -(current_block->mix_event_count[i] >> 1);
       #endif
 
-      #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
-        e_hit = 2; // Needed for the case an endstop is already triggered before the new move begins.
-                   // No 'change' can be detected.
-      #endif
-
       #if ENABLED(Z_LATE_ENABLE)
         // If delayed Z enable, enable it now. This option will severely interfere with
         //  timing between pulses when chaining motion between blocks, and it could lead
@@ -1894,9 +1907,6 @@ void Stepper::init() {
     if (!E_ENABLE_ON) E4_ENABLE_WRITE(HIGH);
   #endif
 
-  // Init endstops and pullups
-  endstops.init();
-
   #define _STEP_INIT(AXIS) AXIS ##_STEP_INIT
   #define _WRITE_STEP(AXIS, HIGHLOW) AXIS ##_STEP_WRITE(HIGHLOW)
   #define _DISABLE(AXIS) disable_## AXIS()
@@ -2034,29 +2044,14 @@ int32_t Stepper::position(const AxisEnum axis) {
   return v;
 }
 
-void Stepper::quick_stop() {
-  const bool was_enabled = STEPPER_ISR_ENABLED();
-  DISABLE_STEPPER_DRIVER_INTERRUPT();
-
-  if (current_block) {
-    step_events_completed = current_block->step_event_count;
-    current_block = NULL;
-  }
-
-  if (was_enabled) ENABLE_STEPPER_DRIVER_INTERRUPT();
-}
-
-void Stepper::kill_current_block() {
-  const bool was_enabled = STEPPER_ISR_ENABLED();
-  DISABLE_STEPPER_DRIVER_INTERRUPT();
-
-  if (current_block)
-    step_events_completed = current_block->step_event_count;
-
-  if (was_enabled) ENABLE_STEPPER_DRIVER_INTERRUPT();
-}
-
+// Signal endstops were triggered - This function can be called from
+// an ISR context  (Temperature, Stepper or limits ISR), so we must
+// be very careful here. If the interrupt being preempted was the
+// Stepper ISR (this CAN happen with the endstop limits ISR) then
+// when the stepper ISR resumes, we must be very sure that the movement
+// is properly cancelled
 void Stepper::endstop_triggered(const AxisEnum axis) {
+
   const bool was_enabled = STEPPER_ISR_ENABLED();
   if (was_enabled) DISABLE_STEPPER_DRIVER_INTERRUPT();
 
@@ -2074,14 +2069,7 @@ void Stepper::endstop_triggered(const AxisEnum axis) {
   #endif // !COREXY && !COREXZ && !COREYZ
 
   // Discard the rest of the move if there is a current block
-  if (current_block) {
-
-    // Kill the current block being executed
-    step_events_completed = current_block->step_event_count;
-
-    // Prep to get a new block after cleaning
-    current_block = NULL;
-  }
+  quick_stop();
 
   if (was_enabled) ENABLE_STEPPER_DRIVER_INTERRUPT();
 }

Marlin/src/module/stepper.h

@@ -75,7 +75,10 @@ class Stepper {
 
   private:
 
-    static uint8_t last_direction_bits;        // The next stepping-bits to be output
+    static uint8_t last_direction_bits,           // The next stepping-bits to be output
+                   last_movement_extruder;        // Last movement extruder, as computed when the last movement was fetched from planner
+    static bool abort_current_block,              // Signals to the stepper that current block should be aborted
+                last_movement_non_null[NUM_AXIS]; // Last Movement in the given direction is not null, as computed when the last movement was fetched from planner
 
     #if ENABLED(X_DUAL_ENDSTOPS)
       static bool locked_x_motor, locked_x2_motor;
@@ -189,13 +192,16 @@ class Stepper {
     static void wake_up();
 
     // Quickly stop all steppers
-    static void quick_stop();
+    FORCE_INLINE static void quick_stop() { abort_current_block = true; }
 
     // The direction of a single motor
     FORCE_INLINE static bool motor_direction(const AxisEnum axis) { return TEST(last_direction_bits, axis); }
 
-    // Kill current block
-    static void kill_current_block();
+    // The last movement direction was not null on the specified axis. Note that motor direction is not necessarily the same.
+    FORCE_INLINE static bool movement_non_null(const AxisEnum axis) { return last_movement_non_null[axis]; }
+
+    // The extruder associated to the last movement
+    FORCE_INLINE static uint8_t movement_extruder() { return last_movement_extruder; }
 
     // Handle a triggered endstop
     static void endstop_triggered(const AxisEnum axis);
@@ -249,7 +255,7 @@ class Stepper {
     FORCE_INLINE static uint32_t calc_timer_interval(uint32_t step_rate) {
       uint32_t timer;
 
-      NOMORE(step_rate, MAX_STEP_FREQUENCY);
+      NOMORE(step_rate, uint32_t(MAX_STEP_FREQUENCY));
 
       // TODO: HAL: tidy this up, use Conditionals_post.h
       #ifdef CPU_32_BIT
@@ -288,7 +294,7 @@ class Stepper {
         timer = uint32_t(HAL_STEPPER_TIMER_RATE) / step_rate;
         NOLESS(timer, min_time_per_step); // (STEP_DOUBLER_FREQUENCY * 2 kHz - this should never happen)
       #else
-        NOLESS(step_rate, F_CPU / 500000);
+        NOLESS(step_rate, uint32_t(F_CPU / 500000U));
         step_rate -= F_CPU / 500000; // Correct for minimal speed
         if (step_rate >= (8 * 256)) { // higher step rate
           uint8_t tmp_step_rate = (step_rate & 0x00FF);

Marlin/src/module/temperature.cpp

@@ -41,10 +41,6 @@
   #include "stepper.h"
 #endif
 
-#if ENABLED(ENDSTOP_INTERRUPTS_FEATURE) || ENABLED(PINS_DEBUGGING)
-  #include "endstops.h"
-#endif
-
 #include "printcounter.h"
 
 #if ENABLED(FILAMENT_WIDTH_SENSOR)
@@ -2247,20 +2243,8 @@ void Temperature::isr() {
     }
   #endif // BABYSTEPPING
 
-  #if ENABLED(PINS_DEBUGGING)
-    endstops.run_monitor();  // report changes in endstop status
-  #endif
-
-  // Update endstops state, if enabled
-  #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE)
-    extern volatile uint8_t e_hit;
-    if (e_hit && ENDSTOPS_ENABLED) {
-      endstops.update();
-      e_hit--;
-    }
-  #else
-    if (ENDSTOPS_ENABLED) endstops.update();
-  #endif
+  // Poll endstops state, if required
+  endstops.poll();
 
   // Periodically call the planner timer
   planner.tick();