From 569df3fc0ccb8b000cc56dd55e6369008ed3f7a2 Mon Sep 17 00:00:00 2001 From: etagle Date: Wed, 16 May 2018 04:08:43 -0300 Subject: [PATCH] Fix interrupt-based endstop detection - Also implemented real endstop reading on interrupt. --- Marlin/src/HAL/HAL_AVR/endstop_interrupts.h | 21 +- Marlin/src/HAL/HAL_DUE/endstop_interrupts.h | 12 +- .../src/HAL/HAL_LPC1768/endstop_interrupts.h | 12 +- .../src/HAL/HAL_STM32F1/endstop_interrupts.h | 12 +- .../src/HAL/HAL_STM32F4/endstop_interrupts.h | 10 +- .../src/HAL/HAL_STM32F7/endstop_interrupts.h | 10 +- .../HAL/HAL_TEENSY35_36/endstop_interrupts.h | 12 +- Marlin/src/Marlin.cpp | 12 +- Marlin/src/module/endstops.cpp | 340 +++++++++++------- Marlin/src/module/endstops.h | 49 ++- Marlin/src/module/planner.cpp | 23 +- Marlin/src/module/stepper.cpp | 92 +++-- Marlin/src/module/stepper.h | 18 +- Marlin/src/module/temperature.cpp | 20 +- 14 files changed, 319 insertions(+), 324 deletions(-) diff --git a/Marlin/src/HAL/HAL_AVR/endstop_interrupts.h b/Marlin/src/HAL/HAL_AVR/endstop_interrupts.h index 88498057ca..609fed98b8 100644 --- a/Marlin/src/HAL/HAL_AVR/endstop_interrupts.h +++ b/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 - -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 ) { diff --git a/Marlin/src/HAL/HAL_DUE/endstop_interrupts.h b/Marlin/src/HAL/HAL_DUE/endstop_interrupts.h index 1a1d8fe82b..b662804cd1 100644 --- a/Marlin/src/HAL/HAL_DUE/endstop_interrupts.h +++ b/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. diff --git a/Marlin/src/HAL/HAL_LPC1768/endstop_interrupts.h b/Marlin/src/HAL/HAL_LPC1768/endstop_interrupts.h index cb0449629a..25da1f95fa 100644 --- a/Marlin/src/HAL/HAL_LPC1768/endstop_interrupts.h +++ b/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 diff --git a/Marlin/src/HAL/HAL_STM32F1/endstop_interrupts.h b/Marlin/src/HAL/HAL_STM32F1/endstop_interrupts.h index 21cecad63e..916e3ffee0 100644 --- a/Marlin/src/HAL/HAL_STM32F1/endstop_interrupts.h +++ b/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 diff --git a/Marlin/src/HAL/HAL_STM32F4/endstop_interrupts.h b/Marlin/src/HAL/HAL_STM32F4/endstop_interrupts.h index cd7d961926..38de2af819 100644 --- a/Marlin/src/HAL/HAL_STM32F4/endstop_interrupts.h +++ b/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 diff --git a/Marlin/src/HAL/HAL_STM32F7/endstop_interrupts.h b/Marlin/src/HAL/HAL_STM32F7/endstop_interrupts.h index 0908140fdb..aa6a5c4c45 100644 --- a/Marlin/src/HAL/HAL_STM32F7/endstop_interrupts.h +++ b/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 diff --git a/Marlin/src/HAL/HAL_TEENSY35_36/endstop_interrupts.h b/Marlin/src/HAL/HAL_TEENSY35_36/endstop_interrupts.h index 12c45db49f..3ba40bdc87 100644 --- a/Marlin/src/HAL/HAL_TEENSY35_36/endstop_interrupts.h +++ b/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. diff --git a/Marlin/src/Marlin.cpp b/Marlin/src/Marlin.cpp index 8eb02e427f..6f7fc4dae6 100644 --- a/Marlin/src/Marlin.cpp +++ b/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 diff --git a/Marlin/src/module/endstops.cpp b/Marlin/src/module/endstops.cpp index 17f9277ae4..d354774afe 100644 --- a/Marlin/src/module/endstops.cpp +++ b/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); - #else - if (X_MAX_TEST) UPDATE_ENDSTOP(X, MAX); - #endif - #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); + 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 - UPDATE_ENDSTOP(Y, MIN); + 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 - } - 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); - #else - UPDATE_ENDSTOP(Y, MAX); - #endif - #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 (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 - #if ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN) - if (z_probe_enabled) UPDATE_ENDSTOP(Z, MIN); - #else - UPDATE_ENDSTOP(Z, MIN); - #endif + COPY_BIT(current_endstop_bits, Y_MIN, Y2_MIN); #endif + test_dual_y_endstops(Y_MIN, Y2_MIN); + #else + UPDATE_ENDSTOP(Y, MIN); #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); - #endif - #endif - } + #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); + #else + UPDATE_ENDSTOP(Y, MAX); + #endif + #endif + } + } - } // stepper.current_block - - old_endstop_bits = current_endstop_bits; + 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 + 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); + #endif + #endif + } + } } // Endstops::update() #if ENABLED(PINS_DEBUGGING) diff --git a/Marlin/src/module/endstops.h b/Marlin/src/module/endstops.h index 5ff5af4548..da276cc729 100644 --- a/Marlin/src/module/endstops.h +++ b/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,23 +68,26 @@ 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 */ 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 */ @@ -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__ diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 0171f3d21c..8644a69f0e 100644 --- a/Marlin/src/module/planner.cpp +++ b/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; diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 3a7336f734..23a4acd3d4 100644 --- a/Marlin/src/module/stepper.cpp +++ b/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(); } diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index 450de469ad..bda1bd5b07 100644 --- a/Marlin/src/module/stepper.h +++ b/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); diff --git a/Marlin/src/module/temperature.cpp b/Marlin/src/module/temperature.cpp index 4ca119abd8..a32ed91293 100644 --- a/Marlin/src/module/temperature.cpp +++ b/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();