diff --git a/Marlin/src/HAL/HAL_AVR/endstop_interrupts.h b/Marlin/src/HAL/HAL_AVR/endstop_interrupts.h index 609fed98b..358b97f6a 100644 --- a/Marlin/src/HAL/HAL_AVR/endstop_interrupts.h +++ b/Marlin/src/HAL/HAL_AVR/endstop_interrupts.h @@ -43,7 +43,7 @@ #include "../../module/endstops.h" // One ISR for all EXT-Interrupts -void endstop_ISR(void) { endstops.check_possible_change(); } +void endstop_ISR(void) { endstops.update(); } /** * Patch for pins_arduino.h (...\Arduino\hardware\arduino\avr\variants\mega\pins_arduino.h) diff --git a/Marlin/src/HAL/HAL_DUE/endstop_interrupts.h b/Marlin/src/HAL/HAL_DUE/endstop_interrupts.h index b662804cd..42cdba19f 100644 --- a/Marlin/src/HAL/HAL_DUE/endstop_interrupts.h +++ b/Marlin/src/HAL/HAL_DUE/endstop_interrupts.h @@ -40,7 +40,7 @@ #include "../../module/endstops.h" // One ISR for all EXT-Interrupts -void endstop_ISR(void) { endstops.check_possible_change(); } +void endstop_ISR(void) { endstops.update(); } /** * Endstop interrupts for Due based targets. diff --git a/Marlin/src/HAL/HAL_ESP32/endstop_interrupts.h b/Marlin/src/HAL/HAL_ESP32/endstop_interrupts.h index 6ba9c8100..b064f16bf 100644 --- a/Marlin/src/HAL/HAL_ESP32/endstop_interrupts.h +++ b/Marlin/src/HAL/HAL_ESP32/endstop_interrupts.h @@ -40,9 +40,7 @@ #include "../../module/endstops.h" // One ISR for all EXT-Interrupts -void ICACHE_RAM_ATTR endstop_ISR(void) { - endstops.check_possible_change(); -} +void ICACHE_RAM_ATTR endstop_ISR(void) { endstops.update(); } void setup_endstop_interrupts(void) { #if HAS_X_MAX diff --git a/Marlin/src/HAL/HAL_LPC1768/endstop_interrupts.h b/Marlin/src/HAL/HAL_LPC1768/endstop_interrupts.h index 25da1f95f..9167361c6 100644 --- a/Marlin/src/HAL/HAL_LPC1768/endstop_interrupts.h +++ b/Marlin/src/HAL/HAL_LPC1768/endstop_interrupts.h @@ -43,7 +43,7 @@ #include "../../module/endstops.h" // One ISR for all EXT-Interrupts -void endstop_ISR(void) { endstops.check_possible_change(); } +void endstop_ISR(void) { endstops.update(); } 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 c2a07a7cf..27ef0a487 100644 --- a/Marlin/src/HAL/HAL_STM32F1/endstop_interrupts.h +++ b/Marlin/src/HAL/HAL_STM32F1/endstop_interrupts.h @@ -52,7 +52,7 @@ #include "../../module/endstops.h" // One ISR for all EXT-Interrupts -void endstop_ISR(void) { endstops.check_possible_change(); } +void endstop_ISR(void) { endstops.update(); } 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 1b1cffc49..2ccfd5706 100644 --- a/Marlin/src/HAL/HAL_STM32F4/endstop_interrupts.h +++ b/Marlin/src/HAL/HAL_STM32F4/endstop_interrupts.h @@ -27,7 +27,7 @@ #include "../../module/endstops.h" // One ISR for all EXT-Interrupts -void endstop_ISR(void) { endstops.check_possible_change(); } +void endstop_ISR(void) { endstops.update(); } 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 5ae338d76..5b2cdb3a9 100644 --- a/Marlin/src/HAL/HAL_STM32F7/endstop_interrupts.h +++ b/Marlin/src/HAL/HAL_STM32F7/endstop_interrupts.h @@ -29,7 +29,7 @@ #include "../../module/endstops.h" // One ISR for all EXT-Interrupts -void endstop_ISR(void) { endstops.check_possible_change(); } +void endstop_ISR(void) { endstops.update(); } 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 3ba40bdc8..00e1b8b38 100644 --- a/Marlin/src/HAL/HAL_TEENSY35_36/endstop_interrupts.h +++ b/Marlin/src/HAL/HAL_TEENSY35_36/endstop_interrupts.h @@ -40,7 +40,7 @@ #include "../../module/endstops.h" // One ISR for all EXT-Interrupts -void endstop_ISR(void) { endstops.check_possible_change(); } +void endstop_ISR(void) { endstops.update(); } /** * Endstop interrupts for Due based targets. diff --git a/Marlin/src/module/endstops.cpp b/Marlin/src/module/endstops.cpp index acf0be796..0b3baf5dc 100644 --- a/Marlin/src/module/endstops.cpp +++ b/Marlin/src/module/endstops.cpp @@ -36,12 +36,6 @@ #include HAL_PATH(../HAL, endstop_interrupts.h) #endif -#if HAS_BED_PROBE - #define ENDSTOPS_ENABLED (enabled || z_probe_enabled) -#else - #define ENDSTOPS_ENABLED enabled -#endif - Endstops endstops; // public: @@ -50,9 +44,9 @@ bool Endstops::enabled, Endstops::enabled_globally; // Initialized by settings.l volatile uint8_t Endstops::hit_state; Endstops::esbits_t Endstops::live_state = 0; + #if ENABLED(ENDSTOP_NOISE_FILTER) - Endstops::esbits_t Endstops::old_live_state, - Endstops::validated_live_state; + Endstops::esbits_t Endstops::validated_live_state; uint8_t Endstops::endstop_poll_count; #endif @@ -222,9 +216,6 @@ void Endstops::init() { } // Endstops::init -// Called from ISR. A change was detected. Find out what happened! -void Endstops::check_possible_change() { if (ENDSTOPS_ENABLED) update(); } - // Called from ISR: Poll endstop state if required void Endstops::poll() { @@ -232,8 +223,10 @@ void Endstops::poll() { run_monitor(); // report changes in endstop status #endif - #if DISABLED(ENDSTOP_INTERRUPTS_FEATURE) || ENABLED(ENDSTOP_NOISE_FILTER) - if (ENDSTOPS_ENABLED) update(); + #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE) && ENABLED(ENDSTOP_NOISE_FILTER) + if (endstop_poll_count) update(); + #elif DISABLED(ENDSTOP_INTERRUPTS_FEATURE) || ENABLED(ENDSTOP_NOISE_FILTER) + update(); #endif } @@ -241,7 +234,7 @@ void Endstops::enable_globally(const bool onoff) { enabled_globally = enabled = onoff; #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE) - if (onoff) update(); // If enabling, update state now + update(); #endif } @@ -250,7 +243,7 @@ void Endstops::enable(const bool onoff) { enabled = onoff; #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE) - if (onoff) update(); // If enabling, update state now + update(); #endif } @@ -259,7 +252,7 @@ void Endstops::not_homing() { enabled = enabled_globally; #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE) - if (enabled) update(); // If enabling, update state now + update(); #endif } @@ -269,7 +262,7 @@ void Endstops::not_homing() { z_probe_enabled = onoff; #if ENABLED(ENDSTOP_INTERRUPTS_FEATURE) - if (enabled) update(); // If enabling, update state now + update(); #endif } #endif @@ -396,10 +389,12 @@ void Endstops::M119() { // Check endstops - Could be called from ISR! void Endstops::update() { - // UPDATE_ENDSTOP_BIT: set the current endstop bits for an endstop to its status + #if DISABLED(ENDSTOP_NOISE_FILTER) + if (!abort_enabled()) return; + #endif + #define UPDATE_ENDSTOP_BIT(AXIS, MINMAX) SET_BIT_TO(live_state, _ENDSTOP(AXIS, MINMAX), (READ(_ENDSTOP_PIN(AXIS, MINMAX)) != _ENDSTOP_INVERTING(AXIS, MINMAX))) - // COPY_BIT: copy the value of SRC_BIT to DST_BIT in DST - #define COPY_BIT(DST, SRC_BIT, DST_BIT) SET_BIT_TO(DST, DST_BIT, TEST(DST, SRC_BIT)) + #define COPY_LIVE_STATE(SRC_BIT, DST_BIT) SET_BIT_TO(live_state, DST_BIT, TEST(live_state, SRC_BIT)) #if ENABLED(G38_PROBE_TARGET) && PIN_EXISTS(Z_MIN_PROBE) && !(CORE_IS_XY || CORE_IS_XZ) // If G38 command is active check Z_MIN_PROBE for ALL movement @@ -444,7 +439,7 @@ void Endstops::update() { #if HAS_X2_MIN UPDATE_ENDSTOP_BIT(X2, MIN); #else - COPY_BIT(live_state, X_MIN, X2_MIN); + COPY_LIVE_STATE(X_MIN, X2_MIN); #endif #else if (X_MIN_TEST) UPDATE_ENDSTOP_BIT(X, MIN); @@ -458,7 +453,7 @@ void Endstops::update() { #if HAS_X2_MAX UPDATE_ENDSTOP_BIT(X2, MAX); #else - COPY_BIT(live_state, X_MAX, X2_MAX); + COPY_LIVE_STATE(X_MAX, X2_MAX); #endif #else if (X_MAX_TEST) UPDATE_ENDSTOP_BIT(X, MAX); @@ -475,7 +470,7 @@ void Endstops::update() { #if HAS_Y2_MIN UPDATE_ENDSTOP_BIT(Y2, MIN); #else - COPY_BIT(live_state, Y_MIN, Y2_MIN); + COPY_LIVE_STATE(Y_MIN, Y2_MIN); #endif #else UPDATE_ENDSTOP_BIT(Y, MIN); @@ -489,7 +484,7 @@ void Endstops::update() { #if HAS_Y2_MAX UPDATE_ENDSTOP_BIT(Y2, MAX); #else - COPY_BIT(live_state, Y_MAX, Y2_MAX); + COPY_LIVE_STATE(Y_MAX, Y2_MAX); #endif #else UPDATE_ENDSTOP_BIT(Y, MAX); @@ -506,7 +501,7 @@ void Endstops::update() { #if HAS_Z2_MIN UPDATE_ENDSTOP_BIT(Z2, MIN); #else - COPY_BIT(live_state, Z_MIN, Z2_MIN); + COPY_LIVE_STATE(Z_MIN, Z2_MIN); #endif #elif ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN) if (z_probe_enabled) UPDATE_ENDSTOP_BIT(Z, MIN); @@ -528,7 +523,7 @@ void Endstops::update() { #if HAS_Z2_MAX UPDATE_ENDSTOP_BIT(Z2, MAX); #else - COPY_BIT(live_state, Z_MAX, Z2_MAX); + COPY_LIVE_STATE(Z_MAX, Z2_MAX); #endif #elif DISABLED(Z_MIN_PROBE_ENDSTOP) || Z_MAX_PIN != Z_MIN_PROBE_PIN // If this pin isn't the bed probe it's the Z endstop @@ -538,36 +533,31 @@ void Endstops::update() { } } - // All endstops were updated. #if ENABLED(ENDSTOP_NOISE_FILTER) - if (old_live_state != live_state) { // We detected a change. Reinit the timeout - /** - * Filtering out noise on endstops requires a delayed decision. Let's assume, due to noise, - * that 50% of endstop signal samples are good and 50% are bad (assuming normal distribution - * of random noise). Then the first sample has a 50% chance to be good or bad. The 2nd sample - * also has a 50% chance to be good or bad. The chances of 2 samples both being bad becomes - * 50% of 50%, or 25%. That was the previous implementation of Marlin endstop handling. It - * reduces chances of bad readings in half, at the cost of 1 extra sample period, but chances - * still exist. The only way to reduce them further is to increase the number of samples. - * To reduce the chance to 1% (1/128th) requires 7 samples (adding 7ms of delay). - */ + /** + * Filtering out noise on endstops requires a delayed decision. Let's assume, due to noise, + * that 50% of endstop signal samples are good and 50% are bad (assuming normal distribution + * of random noise). Then the first sample has a 50% chance to be good or bad. The 2nd sample + * also has a 50% chance to be good or bad. The chances of 2 samples both being bad becomes + * 50% of 50%, or 25%. That was the previous implementation of Marlin endstop handling. It + * reduces chances of bad readings in half, at the cost of 1 extra sample period, but chances + * still exist. The only way to reduce them further is to increase the number of samples. + * To reduce the chance to 1% (1/128th) requires 7 samples (adding 7ms of delay). + */ + static esbits_t old_live_state; + if (old_live_state != live_state) { endstop_poll_count = 7; old_live_state = live_state; } else if (endstop_poll_count && !--endstop_poll_count) validated_live_state = live_state; - #else - - // Lets accept the new endstop values as valid - We assume hardware filtering of lines - esbits_t validated_live_state = live_state; + if (!abort_enabled()) return; #endif - // Endstop readings are validated in validated_live_state - // Test the current status of an endstop - #define TEST_ENDSTOP(ENDSTOP) (TEST(validated_live_state, ENDSTOP)) + #define TEST_ENDSTOP(ENDSTOP) (TEST(state(), ENDSTOP)) // Record endstop was hit #define _ENDSTOP_HIT(AXIS, MINMAX) SBI(hit_state, _ENDSTOP(AXIS, MINMAX)) diff --git a/Marlin/src/module/endstops.h b/Marlin/src/module/endstops.h index fe5c3a7b7..830b1515b 100644 --- a/Marlin/src/module/endstops.h +++ b/Marlin/src/module/endstops.h @@ -70,9 +70,10 @@ class Endstops { private: static esbits_t live_state; static volatile uint8_t hit_state; // Use X_MIN, Y_MIN, Z_MIN and Z_MIN_PROBE as BIT index + #if ENABLED(ENDSTOP_NOISE_FILTER) - static esbits_t old_live_state, // Old endstop value for debouncing and denoising - validated_live_state; // The validated (accepted as true) endstop bits + static esbits_t validated_live_state; + uint8_t Endstops::endstop_poll_count; static uint8_t endstop_poll_count; // Countdown from threshold for polling #endif @@ -85,10 +86,15 @@ 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 + * Are endstops or the probe set to abort the move? */ - static void check_possible_change(); + FORCE_INLINE static bool abort_enabled() { + return (enabled + #if HAS_BED_PROBE + || z_probe_enabled + #endif + ); + } /** * Periodic call to poll endstops if required. Called from temperature ISR @@ -96,7 +102,9 @@ class Endstops { static void poll(); /** - * Update the endstops bits from the pins + * Update endstops bits from the pins. Apply filtering to get a verified state. + * If abort_enabled() and moving towards a triggered switch, abort the current move. + * Called from ISR contexts. */ static void update(); diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 369cadfb5..45f23b7a1 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -1742,7 +1742,7 @@ uint32_t Stepper::stepper_block_phase_isr() { // 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(); + endstops.update(); #if ENABLED(Z_LATE_ENABLE) // If delayed Z enable, enable it now. This option will severely interfere with