From f4562bee45ea219784e4794c3ab8c8dea60c0062 Mon Sep 17 00:00:00 2001 From: John Robertson Date: Sun, 17 Mar 2024 15:26:47 +0000 Subject: [PATCH 01/25] Fix planner wrong trap generation If the planner `entry_rate` or `final_rate` are larger thanthe `block->nominal_rate` then the trapezoid entry ramp continuously accelerates. Only happens if feed rate is less than MAXIMAL_STEP_RATE. --- Marlin/src/module/planner.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 8f7c4ceb72c7..a0d23b369ba7 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -800,6 +800,7 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t // Limit minimal step rate (Otherwise the timer will overflow.) NOLESS(initial_rate, uint32_t(MINIMAL_STEP_RATE)); NOLESS(final_rate, uint32_t(MINIMAL_STEP_RATE)); + NOLESS(block->nominal_rate, (uint32_t)MINIMAL_STEP_RATE); #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) // If we have some plateau time, the cruise rate will be the nominal rate From a7a4152977bcae47fa72e64e73d10c9ad0087704 Mon Sep 17 00:00:00 2001 From: John Robertson Date: Sun, 17 Mar 2024 15:33:09 +0000 Subject: [PATCH 02/25] Update planner.cpp removed #define MINIMAL_STEP_RATE 120 --- Marlin/src/module/planner.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index a0d23b369ba7..f51b0bf413cb 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -730,8 +730,6 @@ void Planner::init() { #endif #endif -#define MINIMAL_STEP_RATE 120 - /** * Get the current block for processing * and mark the block as busy. From 0257384ff5893c163a303d9cd0cb7a7f0dd1ad08 Mon Sep 17 00:00:00 2001 From: John Robertson Date: Sun, 17 Mar 2024 15:37:36 +0000 Subject: [PATCH 03/25] Update planner.h Moved MINIMAL_STEP_RATE to this file. --- Marlin/src/module/planner.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index 856e70e5821a..a2c147bc3dc7 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -267,6 +267,7 @@ typedef struct PlannerBlock { final_adv_steps; // Advance steps for exit speed pressure #endif + #define MINIMAL_STEP_RATE 120 // to prevent timer overflow? uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec initial_rate, // The jerk-adjusted step rate at start of block final_rate, // The minimal rate at exit From cdb0b1cbb7184fca371e603e3a9d1a4108049857 Mon Sep 17 00:00:00 2001 From: John Robertson Date: Sun, 17 Mar 2024 16:19:06 +0000 Subject: [PATCH 04/25] Update planner.h Added calc for MINIMAL_STEP_RATE --- Marlin/src/module/planner.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index a2c147bc3dc7..c1ad7c3b6d74 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -267,7 +267,7 @@ typedef struct PlannerBlock { final_adv_steps; // Advance steps for exit speed pressure #endif - #define MINIMAL_STEP_RATE 120 // to prevent timer overflow? + #define MINIMAL_STEP_RATE _MIN((1 / (STEPPER_TIMER_RATE / HAL_TIMER_TYPE_MAX), 1) // to prevent timer overflow? uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec initial_rate, // The jerk-adjusted step rate at start of block final_rate, // The minimal rate at exit From 3b887b468b3fd8a063f60c29e02b1d3dc2ef64f3 Mon Sep 17 00:00:00 2001 From: John Robertson Date: Sun, 17 Mar 2024 17:06:28 +0000 Subject: [PATCH 05/25] Update planner.h fix minimal_step_rate calc --- Marlin/src/module/planner.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index c1ad7c3b6d74..bb3f524c06ca 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -267,7 +267,7 @@ typedef struct PlannerBlock { final_adv_steps; // Advance steps for exit speed pressure #endif - #define MINIMAL_STEP_RATE _MIN((1 / (STEPPER_TIMER_RATE / HAL_TIMER_TYPE_MAX), 1) // to prevent timer overflow? + #define MINIMAL_STEP_RATE _MAX((STEPPER_TIMER_RATE / HAL_TIMER_TYPE_MAX), 1) // steps/s max. To prevent timer overflow, slowest is 1 step/s uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec initial_rate, // The jerk-adjusted step rate at start of block final_rate, // The minimal rate at exit From 5e0158a8ee1340e5b0e6a7313eb5f5f7058bfa15 Mon Sep 17 00:00:00 2001 From: John Robertson Date: Sun, 7 Apr 2024 15:43:58 +0100 Subject: [PATCH 06/25] Update planner.cpp Remove MINIMAL_STEP_RATE --- Marlin/src/module/planner.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index e3df54aa045d..3cdfecf318ef 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -795,11 +795,6 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t uint32_t initial_rate = CEIL(block->nominal_rate * entry_factor), final_rate = CEIL(block->nominal_rate * exit_factor); // (steps per second) - // Limit minimal step rate (Otherwise the timer will overflow.) - NOLESS(initial_rate, uint32_t(MINIMAL_STEP_RATE)); - NOLESS(final_rate, uint32_t(MINIMAL_STEP_RATE)); - NOLESS(block->nominal_rate, (uint32_t)MINIMAL_STEP_RATE); - #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) // If we have some plateau time, the cruise rate will be the nominal rate uint32_t cruise_rate = block->nominal_rate; From 3da5d0c00102620dc7eddf46a30044773770a667 Mon Sep 17 00:00:00 2001 From: John Robertson Date: Sun, 7 Apr 2024 15:44:15 +0100 Subject: [PATCH 07/25] Update planner.h Remove MINIMAL_STEP_RATE --- Marlin/src/module/planner.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index 2455f78f7db0..b7b1abbb6153 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -267,7 +267,6 @@ typedef struct PlannerBlock { final_adv_steps; // Advance steps for exit speed pressure #endif - #define MINIMAL_STEP_RATE _MAX((STEPPER_TIMER_RATE / HAL_TIMER_TYPE_MAX), 1) // steps/s max. To prevent timer overflow, slowest is 1 step/s uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec initial_rate, // The jerk-adjusted step rate at start of block final_rate, // The minimal rate at exit From 9f2b99b4bd55169e9f353f315527b2ba21c5f52c Mon Sep 17 00:00:00 2001 From: JohnR Date: Sat, 11 May 2024 16:31:39 +0100 Subject: [PATCH 08/25] Revert "Update planner.cpp" This reverts commit 5e0158a8ee1340e5b0e6a7313eb5f5f7058bfa15. Revert "Update planner.h" This reverts commit 3da5d0c00102620dc7eddf46a30044773770a667. --- Marlin/src/module/planner.cpp | 5 +++++ Marlin/src/module/planner.h | 1 + 2 files changed, 6 insertions(+) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 3cdfecf318ef..e3df54aa045d 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -795,6 +795,11 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t uint32_t initial_rate = CEIL(block->nominal_rate * entry_factor), final_rate = CEIL(block->nominal_rate * exit_factor); // (steps per second) + // Limit minimal step rate (Otherwise the timer will overflow.) + NOLESS(initial_rate, uint32_t(MINIMAL_STEP_RATE)); + NOLESS(final_rate, uint32_t(MINIMAL_STEP_RATE)); + NOLESS(block->nominal_rate, (uint32_t)MINIMAL_STEP_RATE); + #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) // If we have some plateau time, the cruise rate will be the nominal rate uint32_t cruise_rate = block->nominal_rate; diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index b7b1abbb6153..2455f78f7db0 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -267,6 +267,7 @@ typedef struct PlannerBlock { final_adv_steps; // Advance steps for exit speed pressure #endif + #define MINIMAL_STEP_RATE _MAX((STEPPER_TIMER_RATE / HAL_TIMER_TYPE_MAX), 1) // steps/s max. To prevent timer overflow, slowest is 1 step/s uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec initial_rate, // The jerk-adjusted step rate at start of block final_rate, // The minimal rate at exit From 22f7de591cd8548125cf372a29c2ad15343e27cb Mon Sep 17 00:00:00 2001 From: John Robertson Date: Sat, 11 May 2024 17:03:29 +0100 Subject: [PATCH 09/25] Update planner.cpp --- Marlin/src/module/planner.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index e3df54aa045d..925edc8f91ab 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -798,7 +798,7 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t // Limit minimal step rate (Otherwise the timer will overflow.) NOLESS(initial_rate, uint32_t(MINIMAL_STEP_RATE)); NOLESS(final_rate, uint32_t(MINIMAL_STEP_RATE)); - NOLESS(block->nominal_rate, (uint32_t)MINIMAL_STEP_RATE); + NOLESS(block->nominal_rate, uint32_t(MINIMAL_STEP_RATE); #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) // If we have some plateau time, the cruise rate will be the nominal rate From 2d46cc37872a72bc78c97032bb1e7d99a18980db Mon Sep 17 00:00:00 2001 From: John Robertson Date: Sat, 11 May 2024 17:14:34 +0100 Subject: [PATCH 10/25] Update planner.h --- Marlin/src/module/planner.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index 2455f78f7db0..08a6cc8c7656 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -267,7 +267,7 @@ typedef struct PlannerBlock { final_adv_steps; // Advance steps for exit speed pressure #endif - #define MINIMAL_STEP_RATE _MAX((STEPPER_TIMER_RATE / HAL_TIMER_TYPE_MAX), 1) // steps/s max. To prevent timer overflow, slowest is 1 step/s + #define MINIMAL_STEP_RATE _MAX((STEPPER_TIMER_RATE / HAL_TIMER_TYPE_MAX), 1UL) // min. steps/s. To prevent timer overflow, slowest is 1 step/s uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec initial_rate, // The jerk-adjusted step rate at start of block final_rate, // The minimal rate at exit From e2ecb030dd7d6506f6ab5cd848bc2d6ae61df2e1 Mon Sep 17 00:00:00 2001 From: John Robertson Date: Sat, 11 May 2024 17:19:07 +0100 Subject: [PATCH 11/25] Update planner.cpp --- Marlin/src/module/planner.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 925edc8f91ab..16fa1faa57b8 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -798,7 +798,7 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t // Limit minimal step rate (Otherwise the timer will overflow.) NOLESS(initial_rate, uint32_t(MINIMAL_STEP_RATE)); NOLESS(final_rate, uint32_t(MINIMAL_STEP_RATE)); - NOLESS(block->nominal_rate, uint32_t(MINIMAL_STEP_RATE); + NOLESS(block->nominal_rate, uint32_t(MINIMAL_STEP_RATE)); #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) // If we have some plateau time, the cruise rate will be the nominal rate From bf5a532104539ead71aa7c8727108473eec9d8a5 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Sat, 11 May 2024 12:43:10 -0500 Subject: [PATCH 12/25] ws --- Marlin/src/module/planner.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index d092f53643f1..2f6289eb1661 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -269,6 +269,7 @@ typedef struct PlannerBlock { #endif #define MINIMAL_STEP_RATE _MAX((STEPPER_TIMER_RATE / HAL_TIMER_TYPE_MAX), 1UL) // min. steps/s. To prevent timer overflow, slowest is 1 step/s + uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec initial_rate, // The jerk-adjusted step rate at start of block final_rate, // The minimal rate at exit From 2396c0fd1196b859a9a4d79cf2c37e8b3b2b36f5 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Sat, 11 May 2024 13:41:07 -0500 Subject: [PATCH 13/25] Apply to min_step_rate --- Marlin/src/HAL/ESP32/timers.h | 3 +-- Marlin/src/module/planner.cpp | 12 +++++++++--- Marlin/src/module/planner.h | 10 ++++++++-- Marlin/src/module/stepper.cpp | 4 ++-- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/Marlin/src/HAL/ESP32/timers.h b/Marlin/src/HAL/ESP32/timers.h index aa4e1551f066..3f336fbfc962 100644 --- a/Marlin/src/HAL/ESP32/timers.h +++ b/Marlin/src/HAL/ESP32/timers.h @@ -53,12 +53,11 @@ typedef uint64_t hal_timer_t; #if ENABLED(I2S_STEPPER_STREAM) #define STEPPER_TIMER_PRESCALE 1 #define STEPPER_TIMER_RATE 250000 // 250khz, 4µs pulses of i2s word clock - #define STEPPER_TIMER_TICKS_PER_US ((STEPPER_TIMER_RATE) / 1000000) // stepper timer ticks per µs // wrong would be 0.25 #else #define STEPPER_TIMER_PRESCALE 40 #define STEPPER_TIMER_RATE ((HAL_TIMER_RATE) / (STEPPER_TIMER_PRESCALE)) // frequency of stepper timer, 2MHz - #define STEPPER_TIMER_TICKS_PER_US ((STEPPER_TIMER_RATE) / 1000000) // stepper timer ticks per µs #endif +#define STEPPER_TIMER_TICKS_PER_US ((STEPPER_TIMER_RATE) / 1000000) // stepper timer ticks per µs #define STEP_TIMER_MIN_INTERVAL 8 // minimum time in µs between stepper interrupts diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 6a74bc139840..4202b41efa7d 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -783,6 +783,9 @@ block_t* Planner::get_current_block() { /** * Calculate trapezoid parameters, multiplying the entry- and exit-speeds * by the provided factors. + * The factors come from the current and next entry speeds divided by the nominal speed, + * which is the top speed achievable during the move. Since entry and exit are presumed to + * be smaller, these factors should always be <= 1.0. ** * ############ VERY IMPORTANT ############ * NOTE that the PRECONDITION to call this function is that the block is @@ -796,9 +799,12 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t final_rate = CEIL(block->nominal_rate * exit_factor); // (steps per second) // Limit minimal step rate (Otherwise the timer will overflow.) - NOLESS(initial_rate, uint32_t(MINIMAL_STEP_RATE)); - NOLESS(final_rate, uint32_t(MINIMAL_STEP_RATE)); - NOLESS(block->nominal_rate, uint32_t(MINIMAL_STEP_RATE)); + NOLESS(initial_rate, MINIMAL_STEP_RATE); + NOLESS(final_rate, MINIMAL_STEP_RATE); + NOLESS(block->nominal_rate, MINIMAL_STEP_RATE); + + //NOMORE(initial_rate, block->nominal_rate); + //NOMORE(final_rate, block->nominal_rate); #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) // If we have some plateau time, the cruise rate will be the nominal rate diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index 2f6289eb1661..7d8e129270b8 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -78,6 +78,14 @@ #include "../feature/closedloop.h" #endif +constexpr uint32_t MINIMAL_STEP_RATE = ( + #ifdef CPU_32_BIT + _MAX((STEPPER_TIMER_RATE) / HAL_TIMER_TYPE_MAX, 1U) // 32-bit shouldn't go below 1 + #else + (F_CPU) / 500000U // AVR shouldn't go below 32 (16MHz) or 40 (20MHz) + #endif +); + // Feedrate for manual moves #ifdef MANUAL_FEEDRATE constexpr xyze_feedrate_t manual_feedrate_mm_m = MANUAL_FEEDRATE, @@ -268,8 +276,6 @@ typedef struct PlannerBlock { final_adv_steps; // Advance steps for exit speed pressure #endif - #define MINIMAL_STEP_RATE _MAX((STEPPER_TIMER_RATE / HAL_TIMER_TYPE_MAX), 1UL) // min. steps/s. To prevent timer overflow, slowest is 1 step/s - uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec initial_rate, // The jerk-adjusted step rate at start of block final_rate, // The minimal rate at exit diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 252bf5526bcd..eb959af57c3a 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -2166,15 +2166,15 @@ void Stepper::pulse_phase_isr() { // Calculate timer interval, with all limits applied. hal_timer_t Stepper::calc_timer_interval(uint32_t step_rate) { + constexpr uint32_t min_step_rate = MINIMAL_STEP_RATE; + #ifdef CPU_32_BIT // A fast processor can just do integer division - constexpr uint32_t min_step_rate = uint32_t(STEPPER_TIMER_RATE) / HAL_TIMER_TYPE_MAX; return step_rate > min_step_rate ? uint32_t(STEPPER_TIMER_RATE) / step_rate : HAL_TIMER_TYPE_MAX; #else - constexpr uint32_t min_step_rate = (F_CPU) / 500000U; // i.e., 32 or 40 if (step_rate >= 0x0800) { // higher step rate // AVR is able to keep up at around 65kHz Stepping ISR rate at most. // So values for step_rate > 65535 might as well be truncated. From a67be24afd4f7592f0628968af1cf511a1075143 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Wed, 15 May 2024 15:06:47 -0500 Subject: [PATCH 14/25] misc cosmetics --- Marlin/src/HAL/HC32/timers.h | 6 +++--- Marlin/src/HAL/TEENSY31_32/timers.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Marlin/src/HAL/HC32/timers.h b/Marlin/src/HAL/HC32/timers.h index 17d8967982c2..a750d2639ad6 100644 --- a/Marlin/src/HAL/HC32/timers.h +++ b/Marlin/src/HAL/HC32/timers.h @@ -41,7 +41,7 @@ extern Timer0 step_timer; // TODO: some calculations (step irq min_step_rate) require the timer rate to be known at compile time // this is not possible with the HC32F460, as the timer rate depends on PCLK1 // as a workaround, PCLK1 = 50MHz is assumed (check with clock dump in MarlinHAL::init()) -#define HAL_TIMER_RATE 50000000 // 50MHz +#define HAL_TIMER_RATE 50000000UL // 50MHz // #define HAL_TIMER_RATE TIMER0_BASE_FREQUENCY // TODO: CYCLES_PER_MICROSECOND seems to be used by Marlin to calculate the number of cycles per microsecond in the timer ISRs @@ -52,14 +52,14 @@ extern Timer0 step_timer; // Temperature timer #define TEMP_TIMER_NUM (&temp_timer) #define TEMP_TIMER_PRIORITY DDL_IRQ_PRIORITY_02 -#define TEMP_TIMER_PRESCALE 16ul +#define TEMP_TIMER_PRESCALE 16UL #define TEMP_TIMER_RATE 1000 // 1kHz #define TEMP_TIMER_FREQUENCY TEMP_TIMER_RATE // Alias for Marlin // Stepper timer #define STEP_TIMER_NUM (&step_timer) #define STEP_TIMER_PRIORITY DDL_IRQ_PRIORITY_01 -#define STEPPER_TIMER_PRESCALE 16ul +#define STEPPER_TIMER_PRESCALE 16UL // TODO: STEPPER_TIMER_RATE seems to work fine like this, but requires further testing... #define STEPPER_TIMER_RATE (HAL_TIMER_RATE / STEPPER_TIMER_PRESCALE) // 50MHz / 16 = 3.125MHz diff --git a/Marlin/src/HAL/TEENSY31_32/timers.h b/Marlin/src/HAL/TEENSY31_32/timers.h index 9fcbb6f232c9..3bbc2421e006 100644 --- a/Marlin/src/HAL/TEENSY31_32/timers.h +++ b/Marlin/src/HAL/TEENSY31_32/timers.h @@ -41,7 +41,7 @@ typedef uint32_t hal_timer_t; #define FTM0_TIMER_PRESCALE_BITS 0b011 #define FTM1_TIMER_PRESCALE_BITS 0b010 -#define FTM0_TIMER_RATE (F_BUS / (FTM0_TIMER_PRESCALE)) // 60MHz / 8 = 7500kHz +#define FTM0_TIMER_RATE (F_BUS / (FTM0_TIMER_PRESCALE)) // 60MHz / 8 = 7.5MHz #define FTM1_TIMER_RATE (F_BUS / (FTM1_TIMER_PRESCALE)) // 60MHz / 4 = 15MHz #define HAL_TIMER_RATE (FTM0_TIMER_RATE) From 551dfa2e3f8fb87b8a9ace1e28cdb5e6e20b434a Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 16 May 2024 19:56:23 -0500 Subject: [PATCH 15/25] comment --- Marlin/src/module/planner.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 14605b5e1717..a794f7a52d3e 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -802,6 +802,7 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t NOLESS(final_rate, MINIMAL_STEP_RATE); NOLESS(block->nominal_rate, MINIMAL_STEP_RATE); + // Assume these are handled elsewhere. Retained for debugging. //NOMORE(initial_rate, block->nominal_rate); //NOMORE(final_rate, block->nominal_rate); From 7c6daa7db3fa24154ad035093e0bc87ed19354e1 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 16 May 2024 20:00:26 -0500 Subject: [PATCH 16/25] allow merge sooner --- Marlin/src/module/planner.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index a794f7a52d3e..7c9768a53f29 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -797,6 +797,10 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t uint32_t initial_rate = CEIL(block->nominal_rate * entry_factor), final_rate = CEIL(block->nominal_rate * exit_factor); // (steps per second) + // Keep existing kludge against acceleration spikes until #27035 + NOLESS(initial_rate, MIN(120, block->nominal_rate)); + NOLESS(final_rate, MIN(120, block->nominal_rate)); + // Limit minimal step rate (Otherwise the timer will overflow.) NOLESS(initial_rate, MINIMAL_STEP_RATE); NOLESS(final_rate, MINIMAL_STEP_RATE); From f2d6cfdf59f964967ef5eb59cb2e8d2b0a7c0469 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 16 May 2024 20:13:14 -0500 Subject: [PATCH 17/25] not needed --- Marlin/src/module/planner.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 7c9768a53f29..a794f7a52d3e 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -797,10 +797,6 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t uint32_t initial_rate = CEIL(block->nominal_rate * entry_factor), final_rate = CEIL(block->nominal_rate * exit_factor); // (steps per second) - // Keep existing kludge against acceleration spikes until #27035 - NOLESS(initial_rate, MIN(120, block->nominal_rate)); - NOLESS(final_rate, MIN(120, block->nominal_rate)); - // Limit minimal step rate (Otherwise the timer will overflow.) NOLESS(initial_rate, MINIMAL_STEP_RATE); NOLESS(final_rate, MINIMAL_STEP_RATE); From cd6b8c8d41e65db39b46069c215ce8628651ce7b Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Fri, 17 May 2024 15:09:25 -0500 Subject: [PATCH 18/25] use _MAX, remove NOMOREs --- Marlin/src/module/planner.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 26a7f2e67fbd..8092204147b4 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -794,18 +794,12 @@ block_t* Planner::get_current_block() { */ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t entry_factor, const_float_t exit_factor) { - uint32_t initial_rate = LROUND(block->nominal_rate * entry_factor), - final_rate = LROUND(block->nominal_rate * exit_factor); // (steps per second) + const uint32_t initial_rate = _MAX(LROUND(block->nominal_rate * entry_factor), MINIMAL_STEP_RATE), + final_rate = _MAX(LROUND(block->nominal_rate * exit_factor), MINIMAL_STEP_RATE); // (steps per second) - // Limit minimal step rate (Otherwise the timer will overflow.) - NOLESS(initial_rate, uint32_t(MINIMAL_STEP_RATE)); // Enforce the minimum speed - NOLESS(final_rate, uint32_t(MINIMAL_STEP_RATE)); + // Now ensure the nominal rate is above minimum NOLESS(block->nominal_rate, MINIMAL_STEP_RATE); - // Assume these are handled elsewhere. Retained for debugging. - //NOMORE(initial_rate, block->nominal_rate); // NOTE: The nominal rate may be less than MINIMAL_STEP_RATE! - //NOMORE(final_rate, block->nominal_rate); - #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) // If we have some plateau time, the cruise rate will be the nominal rate uint32_t cruise_rate = block->nominal_rate; From 481369e483fd00f08c22fc627d56555485b88fe6 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Fri, 17 May 2024 15:12:32 -0500 Subject: [PATCH 19/25] tweak --- Marlin/src/module/planner.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 8092204147b4..e6dc7c8905c7 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -794,8 +794,8 @@ block_t* Planner::get_current_block() { */ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t entry_factor, const_float_t exit_factor) { - const uint32_t initial_rate = _MAX(LROUND(block->nominal_rate * entry_factor), MINIMAL_STEP_RATE), - final_rate = _MAX(LROUND(block->nominal_rate * exit_factor), MINIMAL_STEP_RATE); // (steps per second) + const uint32_t initial_rate = _MAX(MINIMAL_STEP_RATE, LROUND(block->nominal_rate * entry_factor)), // (steps per second) + final_rate = _MAX(MINIMAL_STEP_RATE, LROUND(block->nominal_rate * exit_factor)); // Now ensure the nominal rate is above minimum NOLESS(block->nominal_rate, MINIMAL_STEP_RATE); From a63df47bc2a7d2505370c3e498dfdef66c1bb287 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Fri, 17 May 2024 15:16:11 -0500 Subject: [PATCH 20/25] type happy --- Marlin/src/module/planner.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index e6dc7c8905c7..a836525c0187 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -794,8 +794,8 @@ block_t* Planner::get_current_block() { */ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t entry_factor, const_float_t exit_factor) { - const uint32_t initial_rate = _MAX(MINIMAL_STEP_RATE, LROUND(block->nominal_rate * entry_factor)), // (steps per second) - final_rate = _MAX(MINIMAL_STEP_RATE, LROUND(block->nominal_rate * exit_factor)); + const uint32_t initial_rate = _MAX((long int)MINIMAL_STEP_RATE, LROUND(block->nominal_rate * entry_factor)), // (steps per second) + final_rate = _MAX((long int)MINIMAL_STEP_RATE, LROUND(block->nominal_rate * exit_factor)); // Now ensure the nominal rate is above minimum NOLESS(block->nominal_rate, MINIMAL_STEP_RATE); From 65af7d2730ecfc1a1bb003d75e2feeb83141a65d Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Mon, 1 Jul 2024 12:45:09 -0500 Subject: [PATCH 21/25] shorten lines --- Marlin/src/module/planner.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index a836525c0187..6938341b8ab2 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -794,11 +794,13 @@ block_t* Planner::get_current_block() { */ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t entry_factor, const_float_t exit_factor) { - const uint32_t initial_rate = _MAX((long int)MINIMAL_STEP_RATE, LROUND(block->nominal_rate * entry_factor)), // (steps per second) - final_rate = _MAX((long int)MINIMAL_STEP_RATE, LROUND(block->nominal_rate * exit_factor)); + constexpr long int minimal_step_rate = MINIMAL_STEP_RATE; + + const uint32_t initial_rate = _MAX(minimal_step_rate, LROUND(block->nominal_rate * entry_factor)), // (steps per second) + final_rate = _MAX(minimal_step_rate, LROUND(block->nominal_rate * exit_factor)); // Now ensure the nominal rate is above minimum - NOLESS(block->nominal_rate, MINIMAL_STEP_RATE); + NOLESS(block->nominal_rate, minimal_step_rate); #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) // If we have some plateau time, the cruise rate will be the nominal rate From fe400a284c902a53cf5d74d17f79b0c53dcb7db5 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Mon, 1 Jul 2024 13:26:05 -0500 Subject: [PATCH 22/25] hygdmfsc --- Marlin/src/module/planner.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 6938341b8ab2..633d1af513ee 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -796,10 +796,12 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t constexpr long int minimal_step_rate = MINIMAL_STEP_RATE; - const uint32_t initial_rate = _MAX(minimal_step_rate, LROUND(block->nominal_rate * entry_factor)), // (steps per second) - final_rate = _MAX(minimal_step_rate, LROUND(block->nominal_rate * exit_factor)); + uint32_t initial_rate = LROUND(block->nominal_rate * entry_factor), // (steps per second) + final_rate = LROUND(block->nominal_rate * exit_factor); // Now ensure the nominal rate is above minimum + NOLESS(initial_rate, minimal_step_rate); + NOLESS(final_rate, minimal_step_rate); NOLESS(block->nominal_rate, minimal_step_rate); #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) From c32f6c6cb8ff720072816fb498ae364e6c39d42d Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Mon, 1 Jul 2024 14:06:06 -0500 Subject: [PATCH 23/25] merge followup --- Marlin/src/module/planner.cpp | 12 +++++------- Marlin/src/module/planner.h | 8 -------- Marlin/src/module/stepper.cpp | 8 +++----- Marlin/src/module/stepper.h | 10 ++++++++++ 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 3b143423f9e3..a896966c1141 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -790,15 +790,13 @@ block_t* Planner::get_current_block() { */ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t entry_speed, const_float_t exit_speed) { - constexpr long minimal_step_rate = MINIMAL_STEP_RATE; - - uint32_t initial_rate = LROUND(block->nominal_rate * entry_factor), // (steps per second) - final_rate = LROUND(block->nominal_rate * exit_factor); + uint32_t initial_rate = LROUND(block->nominal_rate * entry_speed), // (steps per second) + final_rate = LROUND(block->nominal_rate * exit_speed); // Now ensure the nominal rate is above minimum - NOLESS(initial_rate, minimal_step_rate); - NOLESS(final_rate, minimal_step_rate); - NOLESS(block->nominal_rate, minimal_step_rate); + NOLESS(initial_rate, stepper.minimal_step_rate); + NOLESS(final_rate, stepper.minimal_step_rate); + NOLESS(block->nominal_rate, stepper.minimal_step_rate); #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) // If we have some plateau time, the cruise rate will be the nominal rate diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index b3d75069a846..1053a27688d3 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -78,14 +78,6 @@ #include "../feature/closedloop.h" #endif -constexpr uint32_t MINIMAL_STEP_RATE = ( - #ifdef CPU_32_BIT - _MAX((STEPPER_TIMER_RATE) / HAL_TIMER_TYPE_MAX, 1U) // 32-bit shouldn't go below 1 - #else - (F_CPU) / 500000U // AVR shouldn't go below 32 (16MHz) or 40 (20MHz) - #endif -); - // Feedrate for manual moves #ifdef MANUAL_FEEDRATE constexpr xyze_feedrate_t manual_feedrate_mm_m = MANUAL_FEEDRATE, diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 91fc290d7914..9da12dee1186 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -2198,12 +2198,10 @@ void Stepper::pulse_phase_isr() { // Calculate timer interval, with all limits applied. hal_timer_t Stepper::calc_timer_interval(uint32_t step_rate) { - constexpr uint32_t min_step_rate = MINIMAL_STEP_RATE; - #ifdef CPU_32_BIT // A fast processor can just do integer division - return step_rate > min_step_rate ? uint32_t(STEPPER_TIMER_RATE) / step_rate : HAL_TIMER_TYPE_MAX; + return step_rate > minimal_step_rate ? uint32_t(STEPPER_TIMER_RATE) / step_rate : HAL_TIMER_TYPE_MAX; #else @@ -2220,8 +2218,8 @@ hal_timer_t Stepper::calc_timer_interval(uint32_t step_rate) { const uint8_t gain = uint8_t(pgm_read_byte(table_address + 2)); return base - MultiU8X8toH8(uint8_t(step_rate & 0x00FF), gain); } - else if (step_rate > min_step_rate) { // lower step rates - step_rate -= min_step_rate; // Correct for minimal speed + else if (step_rate > minimal_step_rate) { // lower step rates + step_rate -= minimal_step_rate; // Correct for minimal speed const uintptr_t table_address = uintptr_t(&speed_lookuptable_slow[uint8_t(step_rate >> 3)]); return uint16_t(pgm_read_word(table_address)) - ((uint16_t(pgm_read_word(table_address + 2)) * uint8_t(step_rate & 0x0007)) >> 3); diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index 3586c23e7060..2a171bebd0be 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -295,6 +295,16 @@ class Stepper { public: + // The minimal step rate ensures calculations stay within limits + // and avoid the most unreasonably slow step rates. + static constexpr uint32_t minimal_step_rate = ( + #ifdef CPU_32_BIT + _MAX((STEPPER_TIMER_RATE) / HAL_TIMER_TYPE_MAX, 1U) // 32-bit shouldn't go below 1 + #else + (F_CPU) / 500000U // AVR shouldn't go below 32 (16MHz) or 40 (20MHz) + #endif + ); + #if ANY(HAS_EXTRA_ENDSTOPS, Z_STEPPER_AUTO_ALIGN) static bool separate_multi_axis; #endif From 03ef8d20998acc599c878009e321ebc92affe008 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Mon, 1 Jul 2024 14:13:37 -0500 Subject: [PATCH 24/25] keep comment --- Marlin/src/module/planner.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index a896966c1141..6a92e2aef4a2 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -793,7 +793,9 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t uint32_t initial_rate = LROUND(block->nominal_rate * entry_speed), // (steps per second) final_rate = LROUND(block->nominal_rate * exit_speed); - // Now ensure the nominal rate is above minimum + // Removing code to constrain values produces judder in direction-switching moves because the + // current discrete stepping math diverges from physical motion under constant acceleration + // when acceleration_steps_per_s2 is large compared to initial/final_rate. NOLESS(initial_rate, stepper.minimal_step_rate); NOLESS(final_rate, stepper.minimal_step_rate); NOLESS(block->nominal_rate, stepper.minimal_step_rate); From 02f9dc0986a2a60e7d7351e1fd5faa767acbcb20 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 4 Jul 2024 20:53:39 -0500 Subject: [PATCH 25/25] followup --- Marlin/src/module/planner.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 6a92e2aef4a2..a6e5ef84eec9 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -790,12 +790,10 @@ block_t* Planner::get_current_block() { */ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t entry_speed, const_float_t exit_speed) { - uint32_t initial_rate = LROUND(block->nominal_rate * entry_speed), // (steps per second) - final_rate = LROUND(block->nominal_rate * exit_speed); + const float spmm = block->steps_per_mm; + uint32_t initial_rate = entry_speed ? LROUND(entry_speed * spmm) : block->initial_rate, + final_rate = LROUND(exit_speed * spmm); - // Removing code to constrain values produces judder in direction-switching moves because the - // current discrete stepping math diverges from physical motion under constant acceleration - // when acceleration_steps_per_s2 is large compared to initial/final_rate. NOLESS(initial_rate, stepper.minimal_step_rate); NOLESS(final_rate, stepper.minimal_step_rate); NOLESS(block->nominal_rate, stepper.minimal_step_rate);