From 092b84a3ed1ee48104135b86faa8bd9e6603ffd0 Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Thu, 30 Jun 2022 08:31:48 +0100 Subject: [PATCH 01/19] Use the correct extruder for LA --- Marlin/src/module/planner.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index de1351babf92..b6d5bb305beb 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -1213,7 +1213,7 @@ void Planner::recalculate_trapezoids(TERN_(HINTS_SAFE_EXIT_SPEED, const_float_t calculate_trapezoid_for_block(block, current_entry_speed * nomr, next_entry_speed * nomr); #if ENABLED(LIN_ADVANCE) if (block->use_advance_lead) { - const float comp = block->e_D_ratio * extruder_advance_K[active_extruder] * settings.axis_steps_per_mm[E_AXIS]; + const float comp = block->e_D_ratio * extruder_advance_K[block->extruder] * settings.axis_steps_per_mm[E_AXIS_N(block->extruder)]; block->max_adv_steps = block->nominal_speed * comp; block->final_adv_steps = next_entry_speed * comp; } @@ -1253,7 +1253,7 @@ void Planner::recalculate_trapezoids(TERN_(HINTS_SAFE_EXIT_SPEED, const_float_t calculate_trapezoid_for_block(block, current_entry_speed * nomr, next_entry_speed * nomr); #if ENABLED(LIN_ADVANCE) if (block->use_advance_lead) { - const float comp = block->e_D_ratio * extruder_advance_K[active_extruder] * settings.axis_steps_per_mm[E_AXIS]; + const float comp = block->e_D_ratio * extruder_advance_K[block->extruder] * settings.axis_steps_per_mm[E_AXIS_N(block->extruder)]; block->max_adv_steps = block->nominal_speed * comp; block->final_adv_steps = next_entry_speed * comp; } @@ -2535,14 +2535,14 @@ bool Planner::_populate_block( /** * Use LIN_ADVANCE for blocks if all these are true: * - * esteps : This is a print move, because we checked for A, B, C steps before. + * esteps : This is a print move, because we checked for A, B, C steps before. * - * extruder_advance_K[active_extruder] : There is an advance factor set for this extruder. + * extruder_advance_K[extruder] : There is an advance factor set for this extruder. * - * de > 0 : Extruder is running forward (e.g., for "Wipe while retracting" (Slic3r) or "Combing" (Cura) moves) + * de > 0 : Extruder is running forward (e.g., for "Wipe while retracting" (Slic3r) or "Combing" (Cura) moves) */ block->use_advance_lead = esteps - && extruder_advance_K[active_extruder] + && extruder_advance_K[extruder] && de > 0; if (block->use_advance_lead) { @@ -2561,7 +2561,7 @@ bool Planner::_populate_block( if (block->e_D_ratio > 3.0f) block->use_advance_lead = false; else { - const uint32_t max_accel_steps_per_s2 = MAX_E_JERK(extruder) / (extruder_advance_K[active_extruder] * block->e_D_ratio) * steps_per_mm; + const uint32_t max_accel_steps_per_s2 = MAX_E_JERK(extruder) / (extruder_advance_K[extruder] * block->e_D_ratio) * steps_per_mm; if (TERN0(LA_DEBUG, accel > max_accel_steps_per_s2)) SERIAL_ECHOLNPGM("Acceleration limited."); NOMORE(accel, max_accel_steps_per_s2); @@ -2594,9 +2594,9 @@ bool Planner::_populate_block( #endif #if ENABLED(LIN_ADVANCE) if (block->use_advance_lead) { - block->advance_speed = (STEPPER_TIMER_RATE) / (extruder_advance_K[active_extruder] * block->e_D_ratio * block->acceleration * settings.axis_steps_per_mm[E_AXIS_N(extruder)]); + block->advance_speed = (STEPPER_TIMER_RATE) / (extruder_advance_K[extruder] * block->e_D_ratio * block->acceleration * settings.axis_steps_per_mm[E_AXIS_N(extruder)]); #if ENABLED(LA_DEBUG) - if (extruder_advance_K[active_extruder] * block->e_D_ratio * block->acceleration * 2 < block->nominal_speed * block->e_D_ratio) + if (extruder_advance_K[extruder] * block->e_D_ratio * block->acceleration * 2 < block->nominal_speed * block->e_D_ratio) SERIAL_ECHOLNPGM("More than 2 steps per eISR loop executed."); if (block->advance_speed < 200) SERIAL_ECHOLNPGM("eISR running at > 10kHz."); From de35fbd6de72d32694f80b99abc2b3840233087a Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Thu, 30 Jun 2022 08:38:42 +0100 Subject: [PATCH 02/19] Removed redundant multiplications --- 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 b6d5bb305beb..0045cde30c5a 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -2596,7 +2596,7 @@ bool Planner::_populate_block( if (block->use_advance_lead) { block->advance_speed = (STEPPER_TIMER_RATE) / (extruder_advance_K[extruder] * block->e_D_ratio * block->acceleration * settings.axis_steps_per_mm[E_AXIS_N(extruder)]); #if ENABLED(LA_DEBUG) - if (extruder_advance_K[extruder] * block->e_D_ratio * block->acceleration * 2 < block->nominal_speed * block->e_D_ratio) + if (extruder_advance_K[extruder] * block->acceleration * 2 < block->nominal_speed) SERIAL_ECHOLNPGM("More than 2 steps per eISR loop executed."); if (block->advance_speed < 200) SERIAL_ECHOLNPGM("eISR running at > 10kHz."); From 4f3c183fba2c6272a0420f173dbf0cf0331031ba Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Sat, 2 Jul 2022 13:08:51 +0100 Subject: [PATCH 03/19] Generate only one extruder pulse train for linear advance --- Marlin/src/module/planner.cpp | 43 ++---- Marlin/src/module/planner.h | 6 +- Marlin/src/module/stepper.cpp | 249 +++++++++++++--------------------- Marlin/src/module/stepper.h | 66 ++++----- 4 files changed, 141 insertions(+), 223 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 0045cde30c5a..e62b4af2b081 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -1211,13 +1211,6 @@ void Planner::recalculate_trapezoids(TERN_(HINTS_SAFE_EXIT_SPEED, const_float_t // NOTE: Entry and exit factors always > 0 by all previous logic operations. const float nomr = 1.0f / block->nominal_speed; calculate_trapezoid_for_block(block, current_entry_speed * nomr, next_entry_speed * nomr); - #if ENABLED(LIN_ADVANCE) - if (block->use_advance_lead) { - const float comp = block->e_D_ratio * extruder_advance_K[block->extruder] * settings.axis_steps_per_mm[E_AXIS_N(block->extruder)]; - block->max_adv_steps = block->nominal_speed * comp; - block->final_adv_steps = next_entry_speed * comp; - } - #endif } // Reset current only to ensure next trapezoid is computed - The @@ -1251,13 +1244,6 @@ void Planner::recalculate_trapezoids(TERN_(HINTS_SAFE_EXIT_SPEED, const_float_t const float nomr = 1.0f / block->nominal_speed; calculate_trapezoid_for_block(block, current_entry_speed * nomr, next_entry_speed * nomr); - #if ENABLED(LIN_ADVANCE) - if (block->use_advance_lead) { - const float comp = block->e_D_ratio * extruder_advance_K[block->extruder] * settings.axis_steps_per_mm[E_AXIS_N(block->extruder)]; - block->max_adv_steps = block->nominal_speed * comp; - block->final_adv_steps = next_entry_speed * comp; - } - #endif } // Reset block to ensure its trapezoid is computed - The stepper is free to use @@ -2502,13 +2488,13 @@ bool Planner::_populate_block( // Compute and limit the acceleration rate for the trapezoid generator. const float steps_per_mm = block->step_event_count * inverse_millimeters; uint32_t accel; + TERN_(LIN_ADVANCE, bool use_advance_lead = false); if (NUM_AXIS_GANG( !block->steps.a, && !block->steps.b, && !block->steps.c, && !block->steps.i, && !block->steps.j, && !block->steps.k, && !block->steps.u, && !block->steps.v, && !block->steps.w) ) { // Is this a retract / recover move? accel = CEIL(settings.retract_acceleration * steps_per_mm); // Convert to: acceleration steps/sec^2 - TERN_(LIN_ADVANCE, block->use_advance_lead = false); // No linear advance for simple retract/recover } else { #define LIMIT_ACCEL_LONG(AXIS,INDX) do{ \ @@ -2541,12 +2527,10 @@ bool Planner::_populate_block( * * de > 0 : Extruder is running forward (e.g., for "Wipe while retracting" (Slic3r) or "Combing" (Cura) moves) */ - block->use_advance_lead = esteps - && extruder_advance_K[extruder] - && de > 0; + use_advance_lead = esteps && extruder_advance_K[extruder] && de > 0; - if (block->use_advance_lead) { - block->e_D_ratio = (target_float.e - position_float.e) / + if (use_advance_lead) { + float e_D_ratio = (target_float.e - position_float.e) / #if IS_KINEMATIC block->millimeters #else @@ -2558,10 +2542,10 @@ bool Planner::_populate_block( // Check for unusual high e_D ratio to detect if a retract move was combined with the last print move due to min. steps per segment. Never execute this with advance! // This assumes no one will use a retract length of 0mm < retr_length < ~0.2mm and no one will print 100mm wide lines using 3mm filament or 35mm wide lines using 1.75mm filament. - if (block->e_D_ratio > 3.0f) - block->use_advance_lead = false; + if (e_D_ratio > 3.0f) + use_advance_lead = false; else { - const uint32_t max_accel_steps_per_s2 = MAX_E_JERK(extruder) / (extruder_advance_K[extruder] * block->e_D_ratio) * steps_per_mm; + const uint32_t max_accel_steps_per_s2 = MAX_E_JERK(extruder) / (extruder_advance_K[extruder] * e_D_ratio) * steps_per_mm; if (TERN0(LA_DEBUG, accel > max_accel_steps_per_s2)) SERIAL_ECHOLNPGM("Acceleration limited."); NOMORE(accel, max_accel_steps_per_s2); @@ -2593,15 +2577,16 @@ bool Planner::_populate_block( block->acceleration_rate = (uint32_t)(accel * (float(1UL << 24) / (STEPPER_TIMER_RATE))); #endif #if ENABLED(LIN_ADVANCE) - if (block->use_advance_lead) { - block->advance_speed = (STEPPER_TIMER_RATE) / (extruder_advance_K[extruder] * block->e_D_ratio * block->acceleration * settings.axis_steps_per_mm[E_AXIS_N(extruder)]); + if (use_advance_lead) { + // the Bresenham algorithm will convert this step rate into extruder steps + block->la_advance_rate = extruder_advance_K[extruder] * block->acceleration_steps_per_s2; #if ENABLED(LA_DEBUG) - if (extruder_advance_K[extruder] * block->acceleration * 2 < block->nominal_speed) - SERIAL_ECHOLNPGM("More than 2 steps per eISR loop executed."); - if (block->advance_speed < 200) - SERIAL_ECHOLNPGM("eISR running at > 10kHz."); + if (block->la_advance_rate > 10000) + SERIAL_ECHOLNPGM("eISR running at > 10kHz: ", block->la_advance_rate); #endif } + else + block->la_advance_rate = 0; #endif float vmax_junction_sqr; // Initial limit on the segment entry velocity (mm/s)^2 diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index 6eb5272071e6..a1fa6dba457a 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -239,11 +239,7 @@ typedef struct PlannerBlock { // Advance extrusion #if ENABLED(LIN_ADVANCE) - bool use_advance_lead; - uint16_t advance_speed, // STEP timer value for extruder speed offset ISR - max_adv_steps, // max. advance steps to get cruising speed pressure (not always nominal_speed!) - final_adv_steps; // advance steps due to exit speed - float e_D_ratio; + uint32_t la_advance_rate; // The rate at which steps are added whilst accelerating #endif uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 593c8f7c6f7e..a6fde722b6c5 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -217,18 +217,10 @@ uint32_t Stepper::advance_divisor = 0, #endif #if ENABLED(LIN_ADVANCE) - uint32_t Stepper::nextAdvanceISR = LA_ADV_NEVER, - Stepper::LA_isr_rate = LA_ADV_NEVER; - uint16_t Stepper::LA_current_adv_steps = 0, - Stepper::LA_final_adv_steps, - Stepper::LA_max_adv_steps; - - int8_t Stepper::LA_steps = 0; - - bool Stepper::LA_use_advance_lead; - -#endif // LIN_ADVANCE + Stepper::la_advance_rate = 0, + Stepper::la_interval = LA_ADV_NEVER; +#endif #if ENABLED(INTEGRATED_BABYSTEPPING) uint32_t Stepper::nextBabystepISR = BABYSTEP_NEVER; @@ -588,29 +580,27 @@ void Stepper::set_directions() { TERN_(HAS_V_DIR, SET_STEP_DIR(V)); TERN_(HAS_W_DIR, SET_STEP_DIR(W)); - #if DISABLED(LIN_ADVANCE) - #if ENABLED(MIXING_EXTRUDER) - // Because this is valid for the whole block we don't know - // what E steppers will step. Likely all. Set all. - if (motor_direction(E_AXIS)) { - MIXER_STEPPER_LOOP(j) REV_E_DIR(j); - count_direction.e = -1; - } - else { - MIXER_STEPPER_LOOP(j) NORM_E_DIR(j); - count_direction.e = 1; - } - #elif HAS_EXTRUDERS - if (motor_direction(E_AXIS)) { - REV_E_DIR(stepper_extruder); - count_direction.e = -1; - } - else { - NORM_E_DIR(stepper_extruder); - count_direction.e = 1; - } - #endif - #endif // !LIN_ADVANCE + #if ENABLED(MIXING_EXTRUDER) + // Because this is valid for the whole block we don't know + // what E steppers will step. Likely all. Set all. + if (motor_direction(E_AXIS)) { + MIXER_STEPPER_LOOP(j) REV_E_DIR(j); + count_direction.e = -1; + } + else { + MIXER_STEPPER_LOOP(j) NORM_E_DIR(j); + count_direction.e = 1; + } + #elif HAS_EXTRUDERS + if (motor_direction(E_AXIS)) { + REV_E_DIR(stepper_extruder); + count_direction.e = -1; + } + else { + NORM_E_DIR(stepper_extruder); + count_direction.e = 1; + } + #endif DIR_WAIT_AFTER(); } @@ -1470,7 +1460,12 @@ void Stepper::isr() { if (!nextMainISR) pulse_phase_isr(); // 0 = Do coordinated axes Stepper pulses #if ENABLED(LIN_ADVANCE) - if (!nextAdvanceISR) nextAdvanceISR = advance_isr(); // 0 = Do Linear Advance E Stepper pulses + if (!nextAdvanceISR) { // 0 = Do Linear Advance E Stepper pulses + advance_isr(); + nextAdvanceISR = la_interval; + } + else if (nextAdvanceISR == LA_ADV_NEVER) // Start LA steps if neccessary + nextAdvanceISR = la_interval; #endif #if ENABLED(INTEGRATED_BABYSTEPPING) @@ -1796,21 +1791,17 @@ void Stepper::pulse_phase_isr() { PULSE_PREP(W); #endif - #if EITHER(LIN_ADVANCE, MIXING_EXTRUDER) - delta_error.e += advance_dividend.e; - if (delta_error.e >= 0) { - #if ENABLED(LIN_ADVANCE) - delta_error.e -= advance_divisor; - // Don't step E here - But remember the number of steps to perform - motor_direction(E_AXIS) ? --LA_steps : ++LA_steps; - #else + if (TERN1(LIN_ADVANCE, !la_advance_rate)) { + #if ENABLED(MIXING_EXTRUDER) + delta_error.e += advance_dividend.e; + if (delta_error.e >= 0) { count_position.e += count_direction.e; step_needed.e = true; - #endif - } - #elif HAS_E0_STEP - PULSE_PREP(E); - #endif + } + #elif HAS_E0_STEP + PULSE_PREP(E); + #endif + } } #if ISR_MULTI_STEPS @@ -1849,13 +1840,13 @@ void Stepper::pulse_phase_isr() { PULSE_START(W); #endif - #if DISABLED(LIN_ADVANCE) + if (TERN1(LIN_ADVANCE, !la_advance_rate)) { #if ENABLED(MIXING_EXTRUDER) if (step_needed.e) E_STEP_WRITE(mixer.get_next_stepper(), !INVERT_E_STEP_PIN); #elif HAS_E0_STEP PULSE_START(E); #endif - #endif + } TERN_(I2S_STEPPER_STREAM, i2s_push_sample()); @@ -1894,7 +1885,7 @@ void Stepper::pulse_phase_isr() { PULSE_STOP(W); #endif - #if DISABLED(LIN_ADVANCE) + if (TERN1(LIN_ADVANCE, !la_advance_rate)) { #if ENABLED(MIXING_EXTRUDER) if (delta_error.e >= 0) { delta_error.e -= advance_divisor; @@ -1903,7 +1894,7 @@ void Stepper::pulse_phase_isr() { #elif HAS_E0_STEP PULSE_STOP(E); #endif - #endif + } #if ISR_MULTI_STEPS if (events_to_do) START_LOW_PULSE(); @@ -1921,6 +1912,8 @@ uint32_t Stepper::block_phase_isr() { // If no queued movements, just wait 1ms for the next block uint32_t interval = (STEPPER_TIMER_RATE) / 1000UL; + TERN_(LIN_ADVANCE, la_interval = LA_ADV_NEVER); + // If there is a current block if (current_block) { // If current block is finished, reset pointer and finalize state @@ -1968,11 +1961,10 @@ uint32_t Stepper::block_phase_isr() { acceleration_time += interval; #if ENABLED(LIN_ADVANCE) - if (LA_use_advance_lead) { - // Fire ISR if final adv_rate is reached - if (LA_steps && LA_isr_rate != current_block->advance_speed) nextAdvanceISR = 0; + if (la_advance_rate) { + const uint32_t la_step_rate = acc_step_rate + la_advance_rate; + la_interval = calc_timer_interval(la_step_rate); } - else if (LA_steps) nextAdvanceISR = 0; #endif /** @@ -2039,14 +2031,31 @@ uint32_t Stepper::block_phase_isr() { deceleration_time += interval; #if ENABLED(LIN_ADVANCE) - if (LA_use_advance_lead) { - // Wake up eISR on first deceleration loop and fire ISR if final adv_rate is reached - if (step_events_completed <= decelerate_after + steps_per_isr || (LA_steps && LA_isr_rate != current_block->advance_speed)) { - initiateLA(); - LA_isr_rate = current_block->advance_speed; + if (la_advance_rate && la_advance_rate != acc_step_rate) { + uint32_t la_step_rate; + if (la_advance_rate < acc_step_rate) { + la_step_rate = acc_step_rate - la_advance_rate; + } + else { + la_step_rate = la_advance_rate - acc_step_rate; + + if (!motor_direction(E_AXIS)) { + SBI(last_direction_bits, E_AXIS); + count_direction.e = -1; + + DIR_WAIT_BEFORE(); + + #if ENABLED(MIXING_EXTRUDER) + MIXER_STEPPER_LOOP(j) REV_E_DIR(j); + #else + REV_E_DIR(stepper_extruder); + #endif + + DIR_WAIT_AFTER(); + } } + la_interval = calc_timer_interval(la_step_rate); } - else if (LA_steps) nextAdvanceISR = 0; #endif // LIN_ADVANCE /* @@ -2069,11 +2078,6 @@ uint32_t Stepper::block_phase_isr() { } else { // Must be in cruise phase otherwise - #if ENABLED(LIN_ADVANCE) - // If there are any esteps, fire the next advance_isr "now" - if (LA_steps && LA_isr_rate != current_block->advance_speed) initiateLA(); - #endif - // Calculate the ticks_nominal for this nominal speed, if not done yet if (ticks_nominal < 0) { // step_rate to timer interval and loops for the nominal speed @@ -2082,6 +2086,10 @@ uint32_t Stepper::block_phase_isr() { // The timer interval is just the nominal value for the nominal speed interval = ticks_nominal; + + #if ENABLED(LIN_ADVANCE) + if (la_advance_rate) la_interval = interval; + #endif } /** @@ -2310,18 +2318,7 @@ uint32_t Stepper::block_phase_isr() { // Initialize the trapezoid generator from the current block. #if ENABLED(LIN_ADVANCE) - #if DISABLED(MIXING_EXTRUDER) && E_STEPPERS > 1 - // If the now active extruder wasn't in use during the last move, its pressure is most likely gone. - if (stepper_extruder != last_moved_extruder) LA_current_adv_steps = 0; - #endif - - if ((LA_use_advance_lead = current_block->use_advance_lead)) { - LA_final_adv_steps = current_block->final_adv_steps; - LA_max_adv_steps = current_block->max_adv_steps; - initiateLA(); // Start the ISR - LA_isr_rate = current_block->advance_speed; - } - else LA_isr_rate = LA_ADV_NEVER; + la_advance_rate = current_block->la_advance_rate; #endif if ( ENABLED(DUAL_X_CARRIAGE) // TODO: Find out why this fixes "jittery" small circles @@ -2376,6 +2373,13 @@ uint32_t Stepper::block_phase_isr() { // Calculate the initial timer interval interval = calc_timer_interval(current_block->initial_rate, &steps_per_isr); + + #if ENABLED(LIN_ADVANCE) + if (la_advance_rate) { + const uint32_t la_step_rate = acc_step_rate + la_advance_rate; + la_interval = calc_timer_interval(la_step_rate); + } + #endif } } @@ -2386,71 +2390,14 @@ uint32_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) // Timer interrupt for E. LA_steps is set in the main routine - uint32_t Stepper::advance_isr() { - uint32_t interval; - - if (LA_use_advance_lead) { - if (step_events_completed > decelerate_after && LA_current_adv_steps > LA_final_adv_steps) { - LA_steps--; - LA_current_adv_steps--; - interval = LA_isr_rate; - } - else if (step_events_completed < decelerate_after && LA_current_adv_steps < LA_max_adv_steps) { - LA_steps++; - LA_current_adv_steps++; - interval = LA_isr_rate; - } - else - interval = LA_isr_rate = LA_ADV_NEVER; - } - else - interval = LA_ADV_NEVER; - - if (!LA_steps) return interval; // Leave pins alone if there are no steps! - - DIR_WAIT_BEFORE(); - - #if ENABLED(MIXING_EXTRUDER) - // We don't know which steppers will be stepped because LA loop follows, - // with potentially multiple steps. Set all. - if (LA_steps > 0) { - MIXER_STEPPER_LOOP(j) NORM_E_DIR(j); - count_direction.e = 1; - } - else if (LA_steps < 0) { - MIXER_STEPPER_LOOP(j) REV_E_DIR(j); - count_direction.e = -1; - } - #else - if (LA_steps > 0) { - NORM_E_DIR(stepper_extruder); - count_direction.e = 1; - } - else if (LA_steps < 0) { - REV_E_DIR(stepper_extruder); - count_direction.e = -1; - } - #endif - - DIR_WAIT_AFTER(); - - //const hal_timer_t added_step_ticks = hal_timer_t(ADDED_STEP_TICKS); - - // Step E stepper if we have steps - #if ISR_MULTI_STEPS - bool firstStep = true; - USING_TIMED_PULSE(); - #endif - - while (LA_steps) { - #if ISR_MULTI_STEPS - if (firstStep) - firstStep = false; - else - AWAIT_LOW_PULSE(); - #endif - + void Stepper::advance_isr() { + // Apply Bresenham algorithm so that linear advance can piggy back on + // the acceleration and speed values calculated in block_phase_isr(). + // This helps keep LA in sync with, for example, S_CURVE_ACCELERATION. + delta_error.e += advance_dividend.e; + if (delta_error.e >= 0) { count_position.e += count_direction.e; + delta_error.e -= advance_divisor; // Set the STEP pulse ON #if ENABLED(MIXING_EXTRUDER) @@ -2461,12 +2408,8 @@ uint32_t Stepper::block_phase_isr() { // Enforce a minimum duration for STEP pulse ON #if ISR_PULSE_CONTROL + USING_TIMED_PULSE(); START_HIGH_PULSE(); - #endif - - LA_steps < 0 ? ++LA_steps : --LA_steps; - - #if ISR_PULSE_CONTROL AWAIT_HIGH_PULSE(); #endif @@ -2476,15 +2419,7 @@ uint32_t Stepper::block_phase_isr() { #else E_STEP_WRITE(stepper_extruder, INVERT_E_STEP_PIN); #endif - - // For minimum pulse time wait before looping - // Just wait for the requested pulse duration - #if ISR_PULSE_CONTROL - if (LA_steps) START_LOW_PULSE(); - #endif - } // LA_steps - - return interval; + } } #endif // LIN_ADVANCE diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index 787599ce3115..fa61a6bce951 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -415,10 +415,9 @@ class Stepper { #if ENABLED(LIN_ADVANCE) static constexpr uint32_t LA_ADV_NEVER = 0xFFFFFFFF; - static uint32_t nextAdvanceISR, LA_isr_rate; - static uint16_t LA_current_adv_steps, LA_final_adv_steps, LA_max_adv_steps; // Copy from current executed block. Needed because current_block is set to NULL "too early". - static int8_t LA_steps; - static bool LA_use_advance_lead; + static uint32_t nextAdvanceISR, + la_advance_rate, // Rate in steps/s calculated from current_block->acceleration_rate + la_interval; // Interval between ISR calls for LA #endif #if ENABLED(INTEGRATED_BABYSTEPPING) @@ -473,8 +472,7 @@ class Stepper { #if ENABLED(LIN_ADVANCE) // The Linear advance ISR phase - static uint32_t advance_isr(); - FORCE_INLINE static void initiateLA() { nextAdvanceISR = 0; } + static void advance_isr(); #endif #if ENABLED(INTEGRATED_BABYSTEPPING) @@ -629,9 +627,37 @@ class Stepper { // Set the current position in steps static void _set_position(const abce_long_t &spos); - FORCE_INLINE static uint32_t calc_timer_interval(uint32_t step_rate, uint8_t *loops) { + FORCE_INLINE static uint32_t calc_timer_interval(uint32_t step_rate) { uint32_t timer; + #ifdef CPU_32_BIT + // In case of high-performance processor, it is able to calculate in real-time + timer = uint32_t(STEPPER_TIMER_RATE) / step_rate; + #else + constexpr uint32_t min_step_rate = (F_CPU) / 500000U; + NOLESS(step_rate, min_step_rate); + step_rate -= min_step_rate; // Correct for minimal speed + if (step_rate >= (8 * 256)) { // higher step rate + const uint8_t tmp_step_rate = (step_rate & 0x00FF); + const uint16_t table_address = (uint16_t)&speed_lookuptable_fast[(uint8_t)(step_rate >> 8)][0], + gain = (uint16_t)pgm_read_word(table_address + 2); + timer = MultiU16X8toH16(tmp_step_rate, gain); + timer = (uint16_t)pgm_read_word(table_address) - timer; + } + else { // lower step rates + uint16_t table_address = (uint16_t)&speed_lookuptable_slow[0][0]; + table_address += ((step_rate) >> 1) & 0xFFFC; + timer = (uint16_t)pgm_read_word(table_address) + - (((uint16_t)pgm_read_word(table_address + 2) * (uint8_t)(step_rate & 0x0007)) >> 3); + } + // (there is no need to limit the timer value here. All limits have been + // applied above, and AVR is able to keep up at 30khz Stepping ISR rate) + #endif + + return timer; + } + + FORCE_INLINE static uint32_t calc_timer_interval(uint32_t step_rate, uint8_t *loops) { // Scale the frequency, as requested by the caller step_rate <<= oversampling_factor; @@ -662,31 +688,7 @@ class Stepper { #endif *loops = multistep; - #ifdef CPU_32_BIT - // In case of high-performance processor, it is able to calculate in real-time - timer = uint32_t(STEPPER_TIMER_RATE) / step_rate; - #else - constexpr uint32_t min_step_rate = (F_CPU) / 500000U; - NOLESS(step_rate, min_step_rate); - step_rate -= min_step_rate; // Correct for minimal speed - if (step_rate >= (8 * 256)) { // higher step rate - const uint8_t tmp_step_rate = (step_rate & 0x00FF); - const uint16_t table_address = (uint16_t)&speed_lookuptable_fast[(uint8_t)(step_rate >> 8)][0], - gain = (uint16_t)pgm_read_word(table_address + 2); - timer = MultiU16X8toH16(tmp_step_rate, gain); - timer = (uint16_t)pgm_read_word(table_address) - timer; - } - else { // lower step rates - uint16_t table_address = (uint16_t)&speed_lookuptable_slow[0][0]; - table_address += ((step_rate) >> 1) & 0xFFFC; - timer = (uint16_t)pgm_read_word(table_address) - - (((uint16_t)pgm_read_word(table_address + 2) * (uint8_t)(step_rate & 0x0007)) >> 3); - } - // (there is no need to limit the timer value here. All limits have been - // applied above, and AVR is able to keep up at 30khz Stepping ISR rate) - #endif - - return timer; + return calc_timer_interval(step_rate); } #if ENABLED(S_CURVE_ACCELERATION) From 0242cfb3d084ed8ea12e254e7aab1e59d6441d60 Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Sat, 2 Jul 2022 14:55:12 +0100 Subject: [PATCH 04/19] Reduce ISR frequency to the minimum possible for LA --- Marlin/src/module/planner.cpp | 16 +++++++++++++--- Marlin/src/module/planner.h | 1 + Marlin/src/module/stepper.cpp | 11 +++++++---- Marlin/src/module/stepper.h | 3 +++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index e62b4af2b081..97edeb410d6d 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -2545,6 +2545,7 @@ bool Planner::_populate_block( if (e_D_ratio > 3.0f) use_advance_lead = false; else { + // Scale E acceleration so that it will be possible to jump to the advance speed. const uint32_t max_accel_steps_per_s2 = MAX_E_JERK(extruder) / (extruder_advance_K[extruder] * e_D_ratio) * steps_per_mm; if (TERN0(LA_DEBUG, accel > max_accel_steps_per_s2)) SERIAL_ECHOLNPGM("Acceleration limited."); @@ -2577,16 +2578,25 @@ bool Planner::_populate_block( block->acceleration_rate = (uint32_t)(accel * (float(1UL << 24) / (STEPPER_TIMER_RATE))); #endif #if ENABLED(LIN_ADVANCE) + block->la_advance_rate = 0; + block->la_scaling = 0; + if (use_advance_lead) { // the Bresenham algorithm will convert this step rate into extruder steps block->la_advance_rate = extruder_advance_K[extruder] * block->acceleration_steps_per_s2; + + // Minimise LA ISR frequency by calling it only often enough to ensure that there will + // never be more than one extruder step per call. Since we use the Bresenham algorithm + // this means E steps * 2 ^ la_scaling is at least half of step_event_count and no more + // step_event_count. + for (uint32_t dividend = block->steps.e << 1; dividend <= block->step_event_count; dividend <<= 1) + block->la_scaling++; + #if ENABLED(LA_DEBUG) - if (block->la_advance_rate > 10000) + if (block->la_advance_rate >> block->la_scaling > 10000) SERIAL_ECHOLNPGM("eISR running at > 10kHz: ", block->la_advance_rate); #endif } - else - block->la_advance_rate = 0; #endif float vmax_junction_sqr; // Initial limit on the segment entry velocity (mm/s)^2 diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index a1fa6dba457a..1432af5c7452 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -240,6 +240,7 @@ typedef struct PlannerBlock { // Advance extrusion #if ENABLED(LIN_ADVANCE) uint32_t la_advance_rate; // The rate at which steps are added whilst accelerating + uint8_t la_scaling; // Scale ISR frequency down and step frequency up by 2 ^ la_scaling #endif uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index a6fde722b6c5..ed8c505a72a7 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -220,6 +220,7 @@ uint32_t Stepper::advance_divisor = 0, uint32_t Stepper::nextAdvanceISR = LA_ADV_NEVER, Stepper::la_advance_rate = 0, Stepper::la_interval = LA_ADV_NEVER; + uint8_t Stepper::la_scaling = 0; #endif #if ENABLED(INTEGRATED_BABYSTEPPING) @@ -1963,7 +1964,7 @@ uint32_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (la_advance_rate) { const uint32_t la_step_rate = acc_step_rate + la_advance_rate; - la_interval = calc_timer_interval(la_step_rate); + la_interval = calc_timer_interval(la_step_rate) << la_scaling; } #endif @@ -2054,7 +2055,7 @@ uint32_t Stepper::block_phase_isr() { DIR_WAIT_AFTER(); } } - la_interval = calc_timer_interval(la_step_rate); + la_interval = calc_timer_interval(la_step_rate) << la_scaling; } #endif // LIN_ADVANCE @@ -2088,7 +2089,7 @@ uint32_t Stepper::block_phase_isr() { interval = ticks_nominal; #if ENABLED(LIN_ADVANCE) - if (la_advance_rate) la_interval = interval; + if (la_advance_rate) la_interval = interval << la_scaling; #endif } @@ -2319,6 +2320,8 @@ uint32_t Stepper::block_phase_isr() { // Initialize the trapezoid generator from the current block. #if ENABLED(LIN_ADVANCE) la_advance_rate = current_block->la_advance_rate; + la_scaling = current_block->la_scaling; + advance_dividend.e <<= la_scaling; #endif if ( ENABLED(DUAL_X_CARRIAGE) // TODO: Find out why this fixes "jittery" small circles @@ -2377,7 +2380,7 @@ uint32_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (la_advance_rate) { const uint32_t la_step_rate = acc_step_rate + la_advance_rate; - la_interval = calc_timer_interval(la_step_rate); + la_interval = calc_timer_interval(la_step_rate) << la_scaling; } #endif } diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index fa61a6bce951..27d0d7316bc9 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -418,8 +418,11 @@ class Stepper { static uint32_t nextAdvanceISR, la_advance_rate, // Rate in steps/s calculated from current_block->acceleration_rate la_interval; // Interval between ISR calls for LA + static uint8_t la_scaling; // Scale ISR frequency down and step frequency up by 2 ^ la_scaling #endif +friend class GcodeSuite; + #if ENABLED(INTEGRATED_BABYSTEPPING) static constexpr uint32_t BABYSTEP_NEVER = 0xFFFFFFFF; static uint32_t nextBabystepISR; From 88bb80839757cebb677dcb353a8214dbf05b66ba Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Sat, 2 Jul 2022 18:21:06 +0100 Subject: [PATCH 05/19] Use the right step rate for the phase --- Marlin/src/module/stepper.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index ed8c505a72a7..426e8f8b7a1b 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -2032,13 +2032,13 @@ uint32_t Stepper::block_phase_isr() { deceleration_time += interval; #if ENABLED(LIN_ADVANCE) - if (la_advance_rate && la_advance_rate != acc_step_rate) { + if (la_advance_rate && la_advance_rate != step_rate) { uint32_t la_step_rate; - if (la_advance_rate < acc_step_rate) { - la_step_rate = acc_step_rate - la_advance_rate; + if (la_advance_rate < step_rate) { + la_step_rate = step_rate - la_advance_rate; } else { - la_step_rate = la_advance_rate - acc_step_rate; + la_step_rate = la_advance_rate - step_rate; if (!motor_direction(E_AXIS)) { SBI(last_direction_bits, E_AXIS); @@ -2379,7 +2379,7 @@ uint32_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (la_advance_rate) { - const uint32_t la_step_rate = acc_step_rate + la_advance_rate; + const uint32_t la_step_rate = current_block->initial_rate + la_advance_rate; la_interval = calc_timer_interval(la_step_rate) << la_scaling; } #endif From 820153b6c57ae031fe928d72a673b7d99d542878 Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Sun, 3 Jul 2022 22:17:39 +0100 Subject: [PATCH 06/19] Take ADVANCE_STEP_SMOOTHING into account in LIN_ADVANCE. Also consolidated references to oversampling_factor within block_phase_isr(). --- Marlin/src/module/stepper.cpp | 19 ++++++++++++------- Marlin/src/module/stepper.h | 3 --- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 426e8f8b7a1b..a9e50f1ed038 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -1958,7 +1958,7 @@ uint32_t Stepper::block_phase_isr() { // acc_step_rate is in steps/second // step_rate to timer interval and steps per stepper isr - interval = calc_timer_interval(acc_step_rate, &steps_per_isr); + interval = calc_timer_interval(acc_step_rate << oversampling_factor, &steps_per_isr); acceleration_time += interval; #if ENABLED(LIN_ADVANCE) @@ -2028,7 +2028,7 @@ uint32_t Stepper::block_phase_isr() { #endif // step_rate to timer interval and steps per stepper isr - interval = calc_timer_interval(step_rate, &steps_per_isr); + interval = calc_timer_interval(step_rate << oversampling_factor, &steps_per_isr); deceleration_time += interval; #if ENABLED(LIN_ADVANCE) @@ -2082,14 +2082,15 @@ uint32_t Stepper::block_phase_isr() { // Calculate the ticks_nominal for this nominal speed, if not done yet if (ticks_nominal < 0) { // step_rate to timer interval and loops for the nominal speed - ticks_nominal = calc_timer_interval(current_block->nominal_rate, &steps_per_isr); + ticks_nominal = calc_timer_interval(current_block->nominal_rate << oversampling_factor, &steps_per_isr); } // The timer interval is just the nominal value for the nominal speed interval = ticks_nominal; #if ENABLED(LIN_ADVANCE) - if (la_advance_rate) la_interval = interval << la_scaling; + // No more acceleration, so re-use ticks_nominal but discount the effect of oversampling_factor + if (la_advance_rate) la_interval = (ticks_nominal << la_scaling) << oversampling_factor; #endif } @@ -2320,8 +2321,12 @@ uint32_t Stepper::block_phase_isr() { // Initialize the trapezoid generator from the current block. #if ENABLED(LIN_ADVANCE) la_advance_rate = current_block->la_advance_rate; - la_scaling = current_block->la_scaling; - advance_dividend.e <<= la_scaling; + if (la_advance_rate) { + la_scaling = current_block->la_scaling; + advance_dividend.e <<= la_scaling; + // discount the effect of frequency scaling for the stepper + advance_dividend.e <<= oversampling; + } #endif if ( ENABLED(DUAL_X_CARRIAGE) // TODO: Find out why this fixes "jittery" small circles @@ -2375,7 +2380,7 @@ uint32_t Stepper::block_phase_isr() { #endif // Calculate the initial timer interval - interval = calc_timer_interval(current_block->initial_rate, &steps_per_isr); + interval = calc_timer_interval(current_block->initial_rate << oversampling_factor, &steps_per_isr); #if ENABLED(LIN_ADVANCE) if (la_advance_rate) { diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index 27d0d7316bc9..b029a50472a0 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -661,9 +661,6 @@ friend class GcodeSuite; } FORCE_INLINE static uint32_t calc_timer_interval(uint32_t step_rate, uint8_t *loops) { - // Scale the frequency, as requested by the caller - step_rate <<= oversampling_factor; - uint8_t multistep = 1; #if DISABLED(DISABLE_MULTI_STEPPING) From aaf7d0e9012f0caab23568b301ba150298cf1f98 Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Sun, 10 Jul 2022 21:28:55 +0100 Subject: [PATCH 07/19] First step timing improvements --- Marlin/src/module/stepper.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index a9e50f1ed038..76bcdf880b2d 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -2381,6 +2381,7 @@ uint32_t Stepper::block_phase_isr() { // Calculate the initial timer interval interval = calc_timer_interval(current_block->initial_rate << oversampling_factor, &steps_per_isr); + acceleration_time += interval; #if ENABLED(LIN_ADVANCE) if (la_advance_rate) { From f7d7f07926891b2f4ab1679a985afe9c638d7b85 Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Wed, 6 Jul 2022 19:25:11 +0100 Subject: [PATCH 08/19] Reduce E jitter when E steps count is small compared to block->step_event_count. --- 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 97edeb410d6d..90a600e8b6d3 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -2585,11 +2585,9 @@ bool Planner::_populate_block( // the Bresenham algorithm will convert this step rate into extruder steps block->la_advance_rate = extruder_advance_K[extruder] * block->acceleration_steps_per_s2; - // Minimise LA ISR frequency by calling it only often enough to ensure that there will - // never be more than one extruder step per call. Since we use the Bresenham algorithm - // this means E steps * 2 ^ la_scaling is at least half of step_event_count and no more - // step_event_count. - for (uint32_t dividend = block->steps.e << 1; dividend <= block->step_event_count; dividend <<= 1) + // reduce LA ISR frequency by calling it only often enough to ensure that there will + // never be more than four extruder steps per call + for (uint32_t dividend = block->steps.e << 1; dividend <= (block->step_event_count >> 2); dividend <<= 1) block->la_scaling++; #if ENABLED(LA_DEBUG) From 7689153d21ca8424b520da35fad9cf2b870f2bcb Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Thu, 14 Jul 2022 22:28:26 +0100 Subject: [PATCH 09/19] No need to copy la_advance_rate and la_scaling from current_block, but we do need to prevent a final call to Stepper::advance_isr() after the block is complete --- Marlin/src/module/stepper.cpp | 39 ++++++++++++++++------------------- Marlin/src/module/stepper.h | 3 +-- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 76bcdf880b2d..920c85462f12 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -218,9 +218,7 @@ uint32_t Stepper::advance_divisor = 0, #if ENABLED(LIN_ADVANCE) uint32_t Stepper::nextAdvanceISR = LA_ADV_NEVER, - Stepper::la_advance_rate = 0, Stepper::la_interval = LA_ADV_NEVER; - uint8_t Stepper::la_scaling = 0; #endif #if ENABLED(INTEGRATED_BABYSTEPPING) @@ -1792,7 +1790,7 @@ void Stepper::pulse_phase_isr() { PULSE_PREP(W); #endif - if (TERN1(LIN_ADVANCE, !la_advance_rate)) { + if (TERN1(LIN_ADVANCE, !current_block->la_advance_rate)) { #if ENABLED(MIXING_EXTRUDER) delta_error.e += advance_dividend.e; if (delta_error.e >= 0) { @@ -1841,7 +1839,7 @@ void Stepper::pulse_phase_isr() { PULSE_START(W); #endif - if (TERN1(LIN_ADVANCE, !la_advance_rate)) { + if (TERN1(LIN_ADVANCE, !current_block->la_advance_rate)) { #if ENABLED(MIXING_EXTRUDER) if (step_needed.e) E_STEP_WRITE(mixer.get_next_stepper(), !INVERT_E_STEP_PIN); #elif HAS_E0_STEP @@ -1886,7 +1884,7 @@ void Stepper::pulse_phase_isr() { PULSE_STOP(W); #endif - if (TERN1(LIN_ADVANCE, !la_advance_rate)) { + if (TERN1(LIN_ADVANCE, !current_block->la_advance_rate)) { #if ENABLED(MIXING_EXTRUDER) if (delta_error.e >= 0) { delta_error.e -= advance_divisor; @@ -1962,9 +1960,9 @@ uint32_t Stepper::block_phase_isr() { acceleration_time += interval; #if ENABLED(LIN_ADVANCE) - if (la_advance_rate) { - const uint32_t la_step_rate = acc_step_rate + la_advance_rate; - la_interval = calc_timer_interval(la_step_rate) << la_scaling; + if (current_block->la_advance_rate) { + const uint32_t la_step_rate = acc_step_rate + current_block->la_advance_rate; + la_interval = calc_timer_interval(la_step_rate) << current_block->la_scaling; } #endif @@ -2032,13 +2030,13 @@ uint32_t Stepper::block_phase_isr() { deceleration_time += interval; #if ENABLED(LIN_ADVANCE) - if (la_advance_rate && la_advance_rate != step_rate) { + if (current_block->la_advance_rate && current_block->la_advance_rate != step_rate) { uint32_t la_step_rate; - if (la_advance_rate < step_rate) { - la_step_rate = step_rate - la_advance_rate; + if (current_block->la_advance_rate < step_rate) { + la_step_rate = step_rate - current_block->la_advance_rate; } else { - la_step_rate = la_advance_rate - step_rate; + la_step_rate = current_block->la_advance_rate - step_rate; if (!motor_direction(E_AXIS)) { SBI(last_direction_bits, E_AXIS); @@ -2055,7 +2053,7 @@ uint32_t Stepper::block_phase_isr() { DIR_WAIT_AFTER(); } } - la_interval = calc_timer_interval(la_step_rate) << la_scaling; + la_interval = calc_timer_interval(la_step_rate) << current_block->la_scaling; } #endif // LIN_ADVANCE @@ -2090,7 +2088,8 @@ uint32_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) // No more acceleration, so re-use ticks_nominal but discount the effect of oversampling_factor - if (la_advance_rate) la_interval = (ticks_nominal << la_scaling) << oversampling_factor; + if (current_block->la_advance_rate) + la_interval = (ticks_nominal << current_block->la_scaling) << oversampling_factor; #endif } @@ -2320,10 +2319,8 @@ uint32_t Stepper::block_phase_isr() { // Initialize the trapezoid generator from the current block. #if ENABLED(LIN_ADVANCE) - la_advance_rate = current_block->la_advance_rate; - if (la_advance_rate) { - la_scaling = current_block->la_scaling; - advance_dividend.e <<= la_scaling; + if (current_block->la_advance_rate) { + advance_dividend.e <<= current_block->la_scaling; // discount the effect of frequency scaling for the stepper advance_dividend.e <<= oversampling; } @@ -2384,9 +2381,9 @@ uint32_t Stepper::block_phase_isr() { acceleration_time += interval; #if ENABLED(LIN_ADVANCE) - if (la_advance_rate) { - const uint32_t la_step_rate = current_block->initial_rate + la_advance_rate; - la_interval = calc_timer_interval(la_step_rate) << la_scaling; + if (current_block->la_advance_rate) { + const uint32_t la_step_rate = current_block->initial_rate + current_block->la_advance_rate; + la_interval = calc_timer_interval(la_step_rate) << current_block->la_scaling; } #endif } diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index b029a50472a0..80057dd80e59 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -416,9 +416,7 @@ class Stepper { #if ENABLED(LIN_ADVANCE) static constexpr uint32_t LA_ADV_NEVER = 0xFFFFFFFF; static uint32_t nextAdvanceISR, - la_advance_rate, // Rate in steps/s calculated from current_block->acceleration_rate la_interval; // Interval between ISR calls for LA - static uint8_t la_scaling; // Scale ISR frequency down and step frequency up by 2 ^ la_scaling #endif friend class GcodeSuite; @@ -511,6 +509,7 @@ friend class GcodeSuite; current_block = nullptr; axis_did_move = 0; planner.release_current_block(); + TERN_(LIN_ADVANCE, nextAdvanceISR = LA_ADV_NEVER); } // Quickly stop all steppers From c6146b84b9b8cf9a6ce5529f2b79a39e9d3643dd Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Sat, 16 Jul 2022 14:16:00 +0100 Subject: [PATCH 10/19] Cleaner MIXING_EXTRUDER pulse preparation --- Marlin/src/module/stepper.cpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 920c85462f12..18b9002b5455 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -1791,13 +1791,7 @@ void Stepper::pulse_phase_isr() { #endif if (TERN1(LIN_ADVANCE, !current_block->la_advance_rate)) { - #if ENABLED(MIXING_EXTRUDER) - delta_error.e += advance_dividend.e; - if (delta_error.e >= 0) { - count_position.e += count_direction.e; - step_needed.e = true; - } - #elif HAS_E0_STEP + #if HAS_E0_STEP || ENABLED(MIXING_EXTRUDER) PULSE_PREP(E); #endif } @@ -1886,10 +1880,7 @@ void Stepper::pulse_phase_isr() { if (TERN1(LIN_ADVANCE, !current_block->la_advance_rate)) { #if ENABLED(MIXING_EXTRUDER) - if (delta_error.e >= 0) { - delta_error.e -= advance_divisor; - E_STEP_WRITE(mixer.get_stepper(), INVERT_E_STEP_PIN); - } + if (step_needed.e) E_STEP_WRITE(mixer.get_stepper(), INVERT_E_STEP_PIN); #elif HAS_E0_STEP PULSE_STOP(E); #endif From a55f31545b84a812638a82078a1ae526521e342e Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Sat, 16 Jul 2022 15:48:44 +0100 Subject: [PATCH 11/19] Ensure advance steps do not accumulate over time due to rounding and timing errors --- Marlin/src/module/planner.cpp | 11 +++- Marlin/src/module/planner.h | 4 +- Marlin/src/module/stepper.cpp | 99 ++++++++++++++++++++--------------- Marlin/src/module/stepper.h | 2 + 4 files changed, 72 insertions(+), 44 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 90a600e8b6d3..dbeac588a7f2 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -788,7 +788,7 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t NOLESS(initial_rate, uint32_t(MINIMAL_STEP_RATE)); NOLESS(final_rate, uint32_t(MINIMAL_STEP_RATE)); - #if ENABLED(S_CURVE_ACCELERATION) + #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; #endif @@ -820,7 +820,7 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t accelerate_steps = _MIN(uint32_t(_MAX(accelerate_steps_float, 0)), block->step_event_count); decelerate_steps = block->step_event_count - accelerate_steps; - #if ENABLED(S_CURVE_ACCELERATION) + #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) // We won't reach the cruising rate. Let's calculate the speed we will reach cruise_rate = final_speed(initial_rate, accel, accelerate_steps); #endif @@ -848,6 +848,13 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t block->cruise_rate = cruise_rate; #endif block->final_rate = final_rate; + #if ENABLED(LIN_ADVANCE) + if (block->la_advance_rate) { + const float comp = extruder_advance_K[block->extruder] * block->steps.e / block->step_event_count; + block->max_adv_steps = cruise_rate * comp; + block->final_adv_steps = final_rate * comp; + } + #endif #if ENABLED(LASER_POWER_TRAP) /** diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index 1432af5c7452..0eeb2a85c6c0 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -241,6 +241,8 @@ typedef struct PlannerBlock { #if ENABLED(LIN_ADVANCE) uint32_t la_advance_rate; // The rate at which steps are added whilst accelerating uint8_t la_scaling; // Scale ISR frequency down and step frequency up by 2 ^ la_scaling + uint16_t max_adv_steps, // Max advance steps to get cruising speed pressure + final_adv_steps; // Advance steps for exit speed pressure #endif uint32_t nominal_rate, // The nominal step rate for this block in step_events/sec @@ -1015,7 +1017,7 @@ class Planner { return target_velocity_sqr - 2 * accel * distance; } - #if ENABLED(S_CURVE_ACCELERATION) + #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) /** * Calculate the speed reached given initial speed, acceleration and distance */ diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 18b9002b5455..fdea74e95839 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -219,6 +219,8 @@ uint32_t Stepper::advance_divisor = 0, #if ENABLED(LIN_ADVANCE) uint32_t Stepper::nextAdvanceISR = LA_ADV_NEVER, Stepper::la_interval = LA_ADV_NEVER; + int32_t Stepper::la_delta_error = 0, + Stepper::la_advance_steps = 0; #endif #if ENABLED(INTEGRATED_BABYSTEPPING) @@ -1790,11 +1792,19 @@ void Stepper::pulse_phase_isr() { PULSE_PREP(W); #endif - if (TERN1(LIN_ADVANCE, !current_block->la_advance_rate)) { - #if HAS_E0_STEP || ENABLED(MIXING_EXTRUDER) - PULSE_PREP(E); + #if HAS_E0_STEP || ENABLED(MIXING_EXTRUDER) + PULSE_PREP(E); + + #if ENABLED(LIN_ADVANCE) + if (step_needed.e && current_block->la_advance_rate) { + // don't actually step here, but do subtract movements steps + // from the linear advance step count + step_needed.e = false; + count_position.e -= count_direction.e; + la_advance_steps--; + } #endif - } + #endif } #if ISR_MULTI_STEPS @@ -1833,13 +1843,11 @@ void Stepper::pulse_phase_isr() { PULSE_START(W); #endif - if (TERN1(LIN_ADVANCE, !current_block->la_advance_rate)) { - #if ENABLED(MIXING_EXTRUDER) - if (step_needed.e) E_STEP_WRITE(mixer.get_next_stepper(), !INVERT_E_STEP_PIN); - #elif HAS_E0_STEP - PULSE_START(E); - #endif - } + #if ENABLED(MIXING_EXTRUDER) + if (step_needed.e) E_STEP_WRITE(mixer.get_next_stepper(), !INVERT_E_STEP_PIN); + #elif HAS_E0_STEP + PULSE_START(E); + #endif TERN_(I2S_STEPPER_STREAM, i2s_push_sample()); @@ -1878,13 +1886,11 @@ void Stepper::pulse_phase_isr() { PULSE_STOP(W); #endif - if (TERN1(LIN_ADVANCE, !current_block->la_advance_rate)) { - #if ENABLED(MIXING_EXTRUDER) - if (step_needed.e) E_STEP_WRITE(mixer.get_stepper(), INVERT_E_STEP_PIN); - #elif HAS_E0_STEP - PULSE_STOP(E); - #endif - } + #if ENABLED(MIXING_EXTRUDER) + if (step_needed.e) E_STEP_WRITE(mixer.get_stepper(), INVERT_E_STEP_PIN); + #elif HAS_E0_STEP + PULSE_STOP(E); + #endif #if ISR_MULTI_STEPS if (events_to_do) START_LOW_PULSE(); @@ -1952,8 +1958,8 @@ uint32_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (current_block->la_advance_rate) { - const uint32_t la_step_rate = acc_step_rate + current_block->la_advance_rate; - la_interval = calc_timer_interval(la_step_rate) << current_block->la_scaling; + const uint32_t la_step_rate = la_advance_steps < current_block->max_adv_steps ? current_block->la_advance_rate : 0; + la_interval = calc_timer_interval(acc_step_rate + la_step_rate) << current_block->la_scaling; } #endif @@ -2021,30 +2027,35 @@ uint32_t Stepper::block_phase_isr() { deceleration_time += interval; #if ENABLED(LIN_ADVANCE) - if (current_block->la_advance_rate && current_block->la_advance_rate != step_rate) { - uint32_t la_step_rate; - if (current_block->la_advance_rate < step_rate) { - la_step_rate = step_rate - current_block->la_advance_rate; - } - else { - la_step_rate = current_block->la_advance_rate - step_rate; + if (current_block->la_advance_rate) { + const uint32_t la_step_rate = la_advance_steps > current_block->final_adv_steps ? current_block->la_advance_rate : 0; + if (la_step_rate != step_rate) { + bool reverse_e = la_step_rate > step_rate; + la_interval = calc_timer_interval(reverse_e ? la_step_rate - step_rate : step_rate - la_step_rate) << current_block->la_scaling; - if (!motor_direction(E_AXIS)) { - SBI(last_direction_bits, E_AXIS); - count_direction.e = -1; + if (reverse_e != motor_direction(E_AXIS)) { + TBI(last_direction_bits, E_AXIS); + count_direction.e = -count_direction.e; DIR_WAIT_BEFORE(); - #if ENABLED(MIXING_EXTRUDER) - MIXER_STEPPER_LOOP(j) REV_E_DIR(j); - #else - REV_E_DIR(stepper_extruder); - #endif + if (reverse_e) { + #if ENABLED(MIXING_EXTRUDER) + MIXER_STEPPER_LOOP(j) REV_E_DIR(j); + #else + REV_E_DIR(stepper_extruder); + #endif + } else { + #if ENABLED(MIXING_EXTRUDER) + MIXER_STEPPER_LOOP(j) NORM_E_DIR(j); + #else + NORM_E_DIR(stepper_extruder); + #endif + } DIR_WAIT_AFTER(); } } - la_interval = calc_timer_interval(la_step_rate) << current_block->la_scaling; } #endif // LIN_ADVANCE @@ -2292,6 +2303,7 @@ uint32_t Stepper::block_phase_isr() { // Initialize Bresenham delta errors to 1/2 delta_error = -int32_t(step_event_count); + TERN_(LIN_ADVANCE, la_delta_error = -int32_t(step_event_count)); // Calculate Bresenham dividends and divisors advance_dividend = current_block->steps << 1; @@ -2310,6 +2322,10 @@ uint32_t Stepper::block_phase_isr() { // Initialize the trapezoid generator from the current block. #if ENABLED(LIN_ADVANCE) + #if DISABLED(MIXING_EXTRUDER) && E_STEPPERS > 1 + // If the now active extruder wasn't in use during the last move, its pressure is most likely gone. + if (stepper_extruder != last_moved_extruder) la_advance_steps = 0; + #endif if (current_block->la_advance_rate) { advance_dividend.e <<= current_block->la_scaling; // discount the effect of frequency scaling for the stepper @@ -2373,8 +2389,8 @@ uint32_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (current_block->la_advance_rate) { - const uint32_t la_step_rate = current_block->initial_rate + current_block->la_advance_rate; - la_interval = calc_timer_interval(la_step_rate) << current_block->la_scaling; + const uint32_t la_step_rate = la_advance_steps < current_block->max_adv_steps ? current_block->la_advance_rate : 0; + la_interval = calc_timer_interval(current_block->initial_rate + la_step_rate) << current_block->la_scaling; } #endif } @@ -2391,10 +2407,11 @@ uint32_t Stepper::block_phase_isr() { // Apply Bresenham algorithm so that linear advance can piggy back on // the acceleration and speed values calculated in block_phase_isr(). // This helps keep LA in sync with, for example, S_CURVE_ACCELERATION. - delta_error.e += advance_dividend.e; - if (delta_error.e >= 0) { + la_delta_error += advance_dividend.e; + if (la_delta_error >= 0) { count_position.e += count_direction.e; - delta_error.e -= advance_divisor; + la_advance_steps += count_direction.e; + la_delta_error -= advance_divisor; // Set the STEP pulse ON #if ENABLED(MIXING_EXTRUDER) diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index 80057dd80e59..b0d8abb7e587 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -417,6 +417,8 @@ class Stepper { static constexpr uint32_t LA_ADV_NEVER = 0xFFFFFFFF; static uint32_t nextAdvanceISR, la_interval; // Interval between ISR calls for LA + static int32_t la_delta_error, // Analogue of delta_error.e for E steps in LA ISR + la_advance_steps; // Count of steps added to increase nozzle pressure #endif friend class GcodeSuite; From 869d15e83654e6b809b734fb73effbbb0cfb5f27 Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Thu, 21 Jul 2022 14:49:30 +0100 Subject: [PATCH 12/19] Save 1000 bytes of flash (on AVR) for slightly longer stepper ISR --- Marlin/src/module/stepper.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index b0d8abb7e587..de0065248cb0 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -631,7 +631,7 @@ friend class GcodeSuite; // Set the current position in steps static void _set_position(const abce_long_t &spos); - FORCE_INLINE static uint32_t calc_timer_interval(uint32_t step_rate) { + static uint32_t calc_timer_interval(uint32_t step_rate) { uint32_t timer; #ifdef CPU_32_BIT @@ -661,7 +661,7 @@ friend class GcodeSuite; return timer; } - FORCE_INLINE static uint32_t calc_timer_interval(uint32_t step_rate, uint8_t *loops) { + static uint32_t calc_timer_interval(uint32_t step_rate, uint8_t *loops) { uint8_t multistep = 1; #if DISABLED(DISABLE_MULTI_STEPPING) From 0606ee0c21e50e5aa295a25332a1cc4764309f4c Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Mon, 25 Jul 2022 14:31:30 -0500 Subject: [PATCH 13/19] cleanup --- Marlin/src/module/planner.cpp | 155 +++++++++++++++++----------------- Marlin/src/module/planner.h | 2 +- Marlin/src/module/stepper.cpp | 5 +- 3 files changed, 81 insertions(+), 81 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index dbeac588a7f2..efe221c8d7d0 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -788,7 +788,7 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t NOLESS(initial_rate, uint32_t(MINIMAL_STEP_RATE)); NOLESS(final_rate, uint32_t(MINIMAL_STEP_RATE)); - #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) + #if EITHER(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; #endif @@ -820,7 +820,7 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t accelerate_steps = _MIN(uint32_t(_MAX(accelerate_steps_float, 0)), block->step_event_count); decelerate_steps = block->step_event_count - accelerate_steps; - #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) + #if EITHER(S_CURVE_ACCELERATION, LIN_ADVANCE) // We won't reach the cruising rate. Let's calculate the speed we will reach cruise_rate = final_speed(initial_rate, accel, accelerate_steps); #endif @@ -848,6 +848,7 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t block->cruise_rate = cruise_rate; #endif block->final_rate = final_rate; + #if ENABLED(LIN_ADVANCE) if (block->la_advance_rate) { const float comp = extruder_advance_K[block->extruder] * block->steps.e / block->step_event_count; @@ -906,75 +907,76 @@ void Planner::calculate_trapezoid_for_block(block_t * const block, const_float_t #endif // LASER_POWER_TRAP } -/* PLANNER SPEED DEFINITION - +--------+ <- current->nominal_speed - / \ - current->entry_speed -> + \ - | + <- next->entry_speed (aka exit speed) - +-------------+ - time --> - - Recalculates the motion plan according to the following basic guidelines: - - 1. Go over every feasible block sequentially in reverse order and calculate the junction speeds - (i.e. current->entry_speed) such that: - a. No junction speed exceeds the pre-computed maximum junction speed limit or nominal speeds of - neighboring blocks. - b. A block entry speed cannot exceed one reverse-computed from its exit speed (next->entry_speed) - with a maximum allowable deceleration over the block travel distance. - c. The last (or newest appended) block is planned from a complete stop (an exit speed of zero). - 2. Go over every block in chronological (forward) order and dial down junction speed values if - a. The exit speed exceeds the one forward-computed from its entry speed with the maximum allowable - acceleration over the block travel distance. - - When these stages are complete, the planner will have maximized the velocity profiles throughout the all - of the planner blocks, where every block is operating at its maximum allowable acceleration limits. In - other words, for all of the blocks in the planner, the plan is optimal and no further speed improvements - are possible. If a new block is added to the buffer, the plan is recomputed according to the said - guidelines for a new optimal plan. - - To increase computational efficiency of these guidelines, a set of planner block pointers have been - created to indicate stop-compute points for when the planner guidelines cannot logically make any further - changes or improvements to the plan when in normal operation and new blocks are streamed and added to the - planner buffer. For example, if a subset of sequential blocks in the planner have been planned and are - bracketed by junction velocities at their maximums (or by the first planner block as well), no new block - added to the planner buffer will alter the velocity profiles within them. So we no longer have to compute - them. Or, if a set of sequential blocks from the first block in the planner (or a optimal stop-compute - point) are all accelerating, they are all optimal and can not be altered by a new block added to the - planner buffer, as this will only further increase the plan speed to chronological blocks until a maximum - junction velocity is reached. However, if the operational conditions of the plan changes from infrequently - used feed holds or feedrate overrides, the stop-compute pointers will be reset and the entire plan is - recomputed as stated in the general guidelines. - - Planner buffer index mapping: - - block_buffer_tail: Points to the beginning of the planner buffer. First to be executed or being executed. - - block_buffer_head: Points to the buffer block after the last block in the buffer. Used to indicate whether - the buffer is full or empty. As described for standard ring buffers, this block is always empty. - - block_buffer_planned: Points to the first buffer block after the last optimally planned block for normal - streaming operating conditions. Use for planning optimizations by avoiding recomputing parts of the - planner buffer that don't change with the addition of a new block, as describe above. In addition, - this block can never be less than block_buffer_tail and will always be pushed forward and maintain - this requirement when encountered by the Planner::release_current_block() routine during a cycle. - - NOTE: Since the planner only computes on what's in the planner buffer, some motions with many short - segments (e.g., complex curves) may seem to move slowly. This is because there simply isn't - enough combined distance traveled in the entire buffer to accelerate up to the nominal speed and - then decelerate to a complete stop at the end of the buffer, as stated by the guidelines. If this - happens and becomes an annoyance, there are a few simple solutions: - - - Maximize the machine acceleration. The planner will be able to compute higher velocity profiles - within the same combined distance. - - - Maximize line motion(s) distance per block to a desired tolerance. The more combined distance the - planner has to use, the faster it can go. - - - Maximize the planner buffer size. This also will increase the combined distance for the planner to - compute over. It also increases the number of computations the planner has to perform to compute an - optimal plan, so select carefully. - - - Use G2/G3 arcs instead of many short segments. Arcs inform the planner of a safe exit speed at the - end of the last segment, which alleviates this problem. -*/ +/** + * PLANNER SPEED DEFINITION + * +--------+ <- current->nominal_speed + * / \ + * current->entry_speed -> + \ + * | + <- next->entry_speed (aka exit speed) + * +-------------+ + * time --> + * + * Recalculates the motion plan according to the following basic guidelines: + * + * 1. Go over every feasible block sequentially in reverse order and calculate the junction speeds + * (i.e. current->entry_speed) such that: + * a. No junction speed exceeds the pre-computed maximum junction speed limit or nominal speeds of + * neighboring blocks. + * b. A block entry speed cannot exceed one reverse-computed from its exit speed (next->entry_speed) + * with a maximum allowable deceleration over the block travel distance. + * c. The last (or newest appended) block is planned from a complete stop (an exit speed of zero). + * 2. Go over every block in chronological (forward) order and dial down junction speed values if + * a. The exit speed exceeds the one forward-computed from its entry speed with the maximum allowable + * acceleration over the block travel distance. + * + * When these stages are complete, the planner will have maximized the velocity profiles throughout the all + * of the planner blocks, where every block is operating at its maximum allowable acceleration limits. In + * other words, for all of the blocks in the planner, the plan is optimal and no further speed improvements + * are possible. If a new block is added to the buffer, the plan is recomputed according to the said + * guidelines for a new optimal plan. + * + * To increase computational efficiency of these guidelines, a set of planner block pointers have been + * created to indicate stop-compute points for when the planner guidelines cannot logically make any further + * changes or improvements to the plan when in normal operation and new blocks are streamed and added to the + * planner buffer. For example, if a subset of sequential blocks in the planner have been planned and are + * bracketed by junction velocities at their maximums (or by the first planner block as well), no new block + * added to the planner buffer will alter the velocity profiles within them. So we no longer have to compute + * them. Or, if a set of sequential blocks from the first block in the planner (or a optimal stop-compute + * point) are all accelerating, they are all optimal and can not be altered by a new block added to the + * planner buffer, as this will only further increase the plan speed to chronological blocks until a maximum + * junction velocity is reached. However, if the operational conditions of the plan changes from infrequently + * used feed holds or feedrate overrides, the stop-compute pointers will be reset and the entire plan is + * recomputed as stated in the general guidelines. + * + * Planner buffer index mapping: + * - block_buffer_tail: Points to the beginning of the planner buffer. First to be executed or being executed. + * - block_buffer_head: Points to the buffer block after the last block in the buffer. Used to indicate whether + * the buffer is full or empty. As described for standard ring buffers, this block is always empty. + * - block_buffer_planned: Points to the first buffer block after the last optimally planned block for normal + * streaming operating conditions. Use for planning optimizations by avoiding recomputing parts of the + * planner buffer that don't change with the addition of a new block, as describe above. In addition, + * this block can never be less than block_buffer_tail and will always be pushed forward and maintain + * this requirement when encountered by the Planner::release_current_block() routine during a cycle. + * + * NOTE: Since the planner only computes on what's in the planner buffer, some motions with many short + * segments (e.g., complex curves) may seem to move slowly. This is because there simply isn't + * enough combined distance traveled in the entire buffer to accelerate up to the nominal speed and + * then decelerate to a complete stop at the end of the buffer, as stated by the guidelines. If this + * happens and becomes an annoyance, there are a few simple solutions: + * + * - Maximize the machine acceleration. The planner will be able to compute higher velocity profiles + * within the same combined distance. + * + * - Maximize line motion(s) distance per block to a desired tolerance. The more combined distance the + * planner has to use, the faster it can go. + * + * - Maximize the planner buffer size. This also will increase the combined distance for the planner to + * compute over. It also increases the number of computations the planner has to perform to compute an + * optimal plan, so select carefully. + * + * - Use G2/G3 arcs instead of many short segments. Arcs inform the planner of a safe exit speed at the + * end of the last segment, which alleviates this problem. + */ // The kernel called by recalculate() when scanning the plan from last to first entry. void Planner::reverse_pass_kernel(block_t * const current, const block_t * const next @@ -2495,7 +2497,9 @@ bool Planner::_populate_block( // Compute and limit the acceleration rate for the trapezoid generator. const float steps_per_mm = block->step_event_count * inverse_millimeters; uint32_t accel; - TERN_(LIN_ADVANCE, bool use_advance_lead = false); + #if ENABLED(LIN_ADVANCE) + bool use_advance_lead = false; + #endif if (NUM_AXIS_GANG( !block->steps.a, && !block->steps.b, && !block->steps.c, && !block->steps.i, && !block->steps.j, && !block->steps.k, @@ -2538,14 +2542,11 @@ bool Planner::_populate_block( if (use_advance_lead) { float e_D_ratio = (target_float.e - position_float.e) / - #if IS_KINEMATIC - block->millimeters - #else + TERN(IS_KINEMATIC, block->millimeters, SQRT(sq(target_float.x - position_float.x) + sq(target_float.y - position_float.y) + sq(target_float.z - position_float.z)) - #endif - ; + ); // Check for unusual high e_D ratio to detect if a retract move was combined with the last print move due to min. steps per segment. Never execute this with advance! // This assumes no one will use a retract length of 0mm < retr_length < ~0.2mm and no one will print 100mm wide lines using 3mm filament or 35mm wide lines using 1.75mm filament. diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index 0eeb2a85c6c0..09afee7db1b5 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -1017,7 +1017,7 @@ class Planner { return target_velocity_sqr - 2 * accel * distance; } - #if ANY(S_CURVE_ACCELERATION, LIN_ADVANCE) + #if EITHER(S_CURVE_ACCELERATION, LIN_ADVANCE) /** * Calculate the speed reached given initial speed, acceleration and distance */ diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index fdea74e95839..e760b3517e69 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -1792,7 +1792,7 @@ void Stepper::pulse_phase_isr() { PULSE_PREP(W); #endif - #if HAS_E0_STEP || ENABLED(MIXING_EXTRUDER) + #if EITHER(HAS_E0_STEP, MIXING_EXTRUDER) PULSE_PREP(E); #if ENABLED(LIN_ADVANCE) @@ -2302,8 +2302,7 @@ uint32_t Stepper::block_phase_isr() { step_event_count = current_block->step_event_count << oversampling; // Initialize Bresenham delta errors to 1/2 - delta_error = -int32_t(step_event_count); - TERN_(LIN_ADVANCE, la_delta_error = -int32_t(step_event_count)); + TERN_(LIN_ADVANCE, la_delta_error =) delta_error = -int32_t(step_event_count); // Calculate Bresenham dividends and divisors advance_dividend = current_block->steps << 1; From 4ec719f946564e51c210f0fd90d04bcf5234df1f Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Mon, 25 Jul 2022 14:49:07 -0500 Subject: [PATCH 14/19] optimize calc_timer_interval --- Marlin/src/module/stepper.h | 43 +++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index de0065248cb0..1d18362a5295 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -632,36 +632,41 @@ friend class GcodeSuite; static void _set_position(const abce_long_t &spos); static uint32_t calc_timer_interval(uint32_t step_rate) { - uint32_t timer; + uint32_t interval; #ifdef CPU_32_BIT // In case of high-performance processor, it is able to calculate in real-time - timer = uint32_t(STEPPER_TIMER_RATE) / step_rate; + interval = uint32_t(STEPPER_TIMER_RATE) / step_rate; #else constexpr uint32_t min_step_rate = (F_CPU) / 500000U; - NOLESS(step_rate, min_step_rate); - step_rate -= min_step_rate; // Correct for minimal speed - if (step_rate >= (8 * 256)) { // higher step rate - const uint8_t tmp_step_rate = (step_rate & 0x00FF); - const uint16_t table_address = (uint16_t)&speed_lookuptable_fast[(uint8_t)(step_rate >> 8)][0], - gain = (uint16_t)pgm_read_word(table_address + 2); - timer = MultiU16X8toH16(tmp_step_rate, gain); - timer = (uint16_t)pgm_read_word(table_address) - timer; + if (step_rate <= min_step_rate) { + step_rate = 0; + uintptr_t table_address = (uintptr_t)&speed_lookuptable_slow[0][0]; + interval = uint16_t(pgm_read_word(table_address)); } - else { // lower step rates - uint16_t table_address = (uint16_t)&speed_lookuptable_slow[0][0]; - table_address += ((step_rate) >> 1) & 0xFFFC; - timer = (uint16_t)pgm_read_word(table_address) - - (((uint16_t)pgm_read_word(table_address + 2) * (uint8_t)(step_rate & 0x0007)) >> 3); + else { + step_rate -= min_step_rate; // Correct for minimal speed + if (step_rate >= (8 * 256)) { // higher step rate + const uint8_t rate_mod_256 = (step_rate & 0x00FF); + const uintptr_t table_address = uintptr_t(&speed_lookuptable_fast[uint8_t(step_rate >> 8)][0]), + gain = uint16_t(pgm_read_word(table_address + 2)); + interval = uint16_t(pgm_read_word(table_address)) - MultiU16X8toH16(rate_mod_256, gain); + } + else { // lower step rates + uintptr_t table_address = uintptr_t(&speed_lookuptable_slow[0][0]); + table_address += ((step_rate) >> 1) & 0xFFFC; + interval = uint16_t(pgm_read_word(table_address)) + - ((uint16_t(pgm_read_word(table_address + 2)) * uint8_t(step_rate & 0x0007)) >> 3); + } + // (there is no need to limit the interval here. All limits have been + // applied above, and AVR is able to keep up at 30khz Stepping ISR rate) } - // (there is no need to limit the timer value here. All limits have been - // applied above, and AVR is able to keep up at 30khz Stepping ISR rate) #endif - return timer; + return interval; } - static uint32_t calc_timer_interval(uint32_t step_rate, uint8_t *loops) { + static uint32_t calc_timer_interval(uint32_t step_rate, uint8_t * const loops) { uint8_t multistep = 1; #if DISABLED(DISABLE_MULTI_STEPPING) From 6235796b6830f606126f8c8a162a3e5c270d1878 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Mon, 25 Jul 2022 15:10:32 -0500 Subject: [PATCH 15/19] move non-inline to cpp --- Marlin/src/module/stepper.cpp | 71 +++++++++++++++++++++++++++++++++-- Marlin/src/module/stepper.h | 68 ++------------------------------- 2 files changed, 70 insertions(+), 69 deletions(-) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index e760b3517e69..e51a904aa30e 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -1899,6 +1899,69 @@ void Stepper::pulse_phase_isr() { } while (--events_to_do); } +// Calculate timer interval, with all limits applied. +uint32_t Stepper::calc_timer_interval(uint32_t step_rate) { + #ifdef CPU_32_BIT + // In case of high-performance processor, it is able to calculate in real-time + return uint32_t(STEPPER_TIMER_RATE) / step_rate; + #else + // AVR is able to keep up at 30khz Stepping ISR rate. + constexpr uint32_t min_step_rate = (F_CPU) / 500000U; + if (step_rate <= min_step_rate) { + step_rate = 0; + uintptr_t table_address = (uintptr_t)&speed_lookuptable_slow[0][0]; + return uint16_t(pgm_read_word(table_address)); + } + else { + step_rate -= min_step_rate; // Correct for minimal speed + if (step_rate >= 0x0800) { // higher step rate + const uint8_t rate_mod_256 = (step_rate & 0x00FF); + const uintptr_t table_address = uintptr_t(&speed_lookuptable_fast[uint8_t(step_rate >> 8)][0]), + gain = uint16_t(pgm_read_word(table_address + 2)); + return uint16_t(pgm_read_word(table_address)) - MultiU16X8toH16(rate_mod_256, gain); + } + else { // lower step rates + uintptr_t table_address = uintptr_t(&speed_lookuptable_slow[0][0]); + table_address += (step_rate >> 1) & 0xFFFC; + return uint16_t(pgm_read_word(table_address)) + - ((uint16_t(pgm_read_word(table_address + 2)) * uint8_t(step_rate & 0x0007)) >> 3); + } + } + #endif +} + +// Get the timer interval and the number of loops to perform per tick +uint32_t Stepper::calc_timer_interval(uint32_t step_rate, uint8_t &loops) { + uint8_t multistep = 1; + #if DISABLED(DISABLE_MULTI_STEPPING) + + // The stepping frequency limits for each multistepping rate + static const uint32_t limit[] PROGMEM = { + ( MAX_STEP_ISR_FREQUENCY_1X ), + ( MAX_STEP_ISR_FREQUENCY_2X >> 1), + ( MAX_STEP_ISR_FREQUENCY_4X >> 2), + ( MAX_STEP_ISR_FREQUENCY_8X >> 3), + ( MAX_STEP_ISR_FREQUENCY_16X >> 4), + ( MAX_STEP_ISR_FREQUENCY_32X >> 5), + ( MAX_STEP_ISR_FREQUENCY_64X >> 6), + (MAX_STEP_ISR_FREQUENCY_128X >> 7) + }; + + // Select the proper multistepping + uint8_t idx = 0; + while (idx < 7 && step_rate > (uint32_t)pgm_read_dword(&limit[idx])) { + step_rate >>= 1; + multistep <<= 1; + ++idx; + }; + #else + NOMORE(step_rate, uint32_t(MAX_STEP_ISR_FREQUENCY_1X)); + #endif + loops = multistep; + + return calc_timer_interval(step_rate); +} + // This is the last half of the stepper interrupt: This one processes and // properly schedules blocks from the planner. This is executed after creating // the step pulses, so it is not time critical, as pulses are already done. @@ -1953,7 +2016,7 @@ uint32_t Stepper::block_phase_isr() { // acc_step_rate is in steps/second // step_rate to timer interval and steps per stepper isr - interval = calc_timer_interval(acc_step_rate << oversampling_factor, &steps_per_isr); + interval = calc_timer_interval(acc_step_rate << oversampling_factor, steps_per_isr); acceleration_time += interval; #if ENABLED(LIN_ADVANCE) @@ -2023,7 +2086,7 @@ uint32_t Stepper::block_phase_isr() { #endif // step_rate to timer interval and steps per stepper isr - interval = calc_timer_interval(step_rate << oversampling_factor, &steps_per_isr); + interval = calc_timer_interval(step_rate << oversampling_factor, steps_per_isr); deceleration_time += interval; #if ENABLED(LIN_ADVANCE) @@ -2082,7 +2145,7 @@ uint32_t Stepper::block_phase_isr() { // Calculate the ticks_nominal for this nominal speed, if not done yet if (ticks_nominal < 0) { // step_rate to timer interval and loops for the nominal speed - ticks_nominal = calc_timer_interval(current_block->nominal_rate << oversampling_factor, &steps_per_isr); + ticks_nominal = calc_timer_interval(current_block->nominal_rate << oversampling_factor, steps_per_isr); } // The timer interval is just the nominal value for the nominal speed @@ -2383,7 +2446,7 @@ uint32_t Stepper::block_phase_isr() { #endif // Calculate the initial timer interval - interval = calc_timer_interval(current_block->initial_rate << oversampling_factor, &steps_per_isr); + interval = calc_timer_interval(current_block->initial_rate << oversampling_factor, steps_per_isr); acceleration_time += interval; #if ENABLED(LIN_ADVANCE) diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index 1d18362a5295..ad34726343f3 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -631,71 +631,9 @@ friend class GcodeSuite; // Set the current position in steps static void _set_position(const abce_long_t &spos); - static uint32_t calc_timer_interval(uint32_t step_rate) { - uint32_t interval; - - #ifdef CPU_32_BIT - // In case of high-performance processor, it is able to calculate in real-time - interval = uint32_t(STEPPER_TIMER_RATE) / step_rate; - #else - constexpr uint32_t min_step_rate = (F_CPU) / 500000U; - if (step_rate <= min_step_rate) { - step_rate = 0; - uintptr_t table_address = (uintptr_t)&speed_lookuptable_slow[0][0]; - interval = uint16_t(pgm_read_word(table_address)); - } - else { - step_rate -= min_step_rate; // Correct for minimal speed - if (step_rate >= (8 * 256)) { // higher step rate - const uint8_t rate_mod_256 = (step_rate & 0x00FF); - const uintptr_t table_address = uintptr_t(&speed_lookuptable_fast[uint8_t(step_rate >> 8)][0]), - gain = uint16_t(pgm_read_word(table_address + 2)); - interval = uint16_t(pgm_read_word(table_address)) - MultiU16X8toH16(rate_mod_256, gain); - } - else { // lower step rates - uintptr_t table_address = uintptr_t(&speed_lookuptable_slow[0][0]); - table_address += ((step_rate) >> 1) & 0xFFFC; - interval = uint16_t(pgm_read_word(table_address)) - - ((uint16_t(pgm_read_word(table_address + 2)) * uint8_t(step_rate & 0x0007)) >> 3); - } - // (there is no need to limit the interval here. All limits have been - // applied above, and AVR is able to keep up at 30khz Stepping ISR rate) - } - #endif - - return interval; - } - - static uint32_t calc_timer_interval(uint32_t step_rate, uint8_t * const loops) { - uint8_t multistep = 1; - #if DISABLED(DISABLE_MULTI_STEPPING) - - // The stepping frequency limits for each multistepping rate - static const uint32_t limit[] PROGMEM = { - ( MAX_STEP_ISR_FREQUENCY_1X ), - ( MAX_STEP_ISR_FREQUENCY_2X >> 1), - ( MAX_STEP_ISR_FREQUENCY_4X >> 2), - ( MAX_STEP_ISR_FREQUENCY_8X >> 3), - ( MAX_STEP_ISR_FREQUENCY_16X >> 4), - ( MAX_STEP_ISR_FREQUENCY_32X >> 5), - ( MAX_STEP_ISR_FREQUENCY_64X >> 6), - (MAX_STEP_ISR_FREQUENCY_128X >> 7) - }; - - // Select the proper multistepping - uint8_t idx = 0; - while (idx < 7 && step_rate > (uint32_t)pgm_read_dword(&limit[idx])) { - step_rate >>= 1; - multistep <<= 1; - ++idx; - }; - #else - NOMORE(step_rate, uint32_t(MAX_STEP_ISR_FREQUENCY_1X)); - #endif - *loops = multistep; - - return calc_timer_interval(step_rate); - } + // Calculate timing interval for the given step rate + static uint32_t calc_timer_interval(uint32_t step_rate); + static uint32_t calc_timer_interval(uint32_t step_rate, uint8_t &loops); #if ENABLED(S_CURVE_ACCELERATION) static void _calc_bezier_curve_coeffs(const int32_t v0, const int32_t v1, const uint32_t av); From ded2ccd69714291d9f848855f0e2e3d10053b1ce Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Mon, 25 Jul 2022 15:24:12 -0500 Subject: [PATCH 16/19] fix some header / dependencies --- Marlin/src/core/utility.cpp | 2 +- Marlin/src/core/utility.h | 5 +++++ Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp | 1 - Marlin/src/feature/bedlevel/ubl/ubl_motion.cpp | 1 - Marlin/src/feature/dac/dac_dac084s085.cpp | 1 - Marlin/src/feature/fwretract.cpp | 1 - Marlin/src/feature/max7219.cpp | 1 - Marlin/src/feature/pause.cpp | 5 ++++- Marlin/src/feature/power.cpp | 3 ++- Marlin/src/feature/tmc_util.cpp | 5 ----- Marlin/src/gcode/bedlevel/G26.cpp | 1 - Marlin/src/gcode/bedlevel/abl/G29.cpp | 1 - Marlin/src/gcode/bedlevel/mbl/G29.cpp | 2 +- Marlin/src/gcode/calibrate/G28.cpp | 3 ++- Marlin/src/gcode/calibrate/G33.cpp | 2 +- Marlin/src/gcode/calibrate/G34.cpp | 5 ++++- Marlin/src/gcode/config/M540.cpp | 2 +- Marlin/src/gcode/control/M17_M18_M84.cpp | 1 + Marlin/src/gcode/control/M226.cpp | 2 +- Marlin/src/gcode/control/M3-M5.cpp | 2 +- Marlin/src/gcode/control/M400.cpp | 2 +- Marlin/src/gcode/control/M605.cpp | 1 - Marlin/src/gcode/feature/advance/M900.cpp | 1 - Marlin/src/gcode/feature/trinamic/M122.cpp | 2 +- Marlin/src/gcode/geometry/G53-G59.cpp | 2 -- Marlin/src/gcode/geometry/G92.cpp | 1 - Marlin/src/gcode/motion/G0_G1.cpp | 2 +- Marlin/src/gcode/motion/G4.cpp | 2 +- Marlin/src/gcode/probe/G38.cpp | 2 +- Marlin/src/module/stepper.cpp | 8 ++++---- Marlin/src/module/stepper.h | 2 -- 31 files changed, 33 insertions(+), 38 deletions(-) diff --git a/Marlin/src/core/utility.cpp b/Marlin/src/core/utility.cpp index 9cdf8dec7bcf..e4fd52592475 100644 --- a/Marlin/src/core/utility.cpp +++ b/Marlin/src/core/utility.cpp @@ -51,7 +51,7 @@ void safe_delay(millis_t ms) { #include "../module/probe.h" #include "../module/motion.h" - #include "../module/stepper.h" + #include "../module/planner.h" #include "../libs/numtostr.h" #include "../feature/bedlevel/bedlevel.h" diff --git a/Marlin/src/core/utility.h b/Marlin/src/core/utility.h index 10c8201610ea..2731e62b6754 100644 --- a/Marlin/src/core/utility.h +++ b/Marlin/src/core/utility.h @@ -59,6 +59,11 @@ void safe_delay(millis_t ms); // Delay ensuring that temperatures are #define log_machine_info() NOOP #endif +/** + * A restorer instance remembers a variable's value before setting a + * new value, then restores the old value when it goes out of scope. + * Put operator= on your type to get extended behavior on value change. + */ template class restorer { T& ref_; diff --git a/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp b/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp index a02918ff297e..f1e88006ff82 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp +++ b/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp @@ -31,7 +31,6 @@ #include "../../../libs/hex_print.h" #include "../../../module/settings.h" #include "../../../lcd/marlinui.h" -#include "../../../module/stepper.h" #include "../../../module/planner.h" #include "../../../module/motion.h" #include "../../../module/probe.h" diff --git a/Marlin/src/feature/bedlevel/ubl/ubl_motion.cpp b/Marlin/src/feature/bedlevel/ubl/ubl_motion.cpp index 8121a0b9b5bb..18110c67fa8e 100644 --- a/Marlin/src/feature/bedlevel/ubl/ubl_motion.cpp +++ b/Marlin/src/feature/bedlevel/ubl/ubl_motion.cpp @@ -26,7 +26,6 @@ #include "../bedlevel.h" #include "../../../module/planner.h" -#include "../../../module/stepper.h" #include "../../../module/motion.h" #if ENABLED(DELTA) diff --git a/Marlin/src/feature/dac/dac_dac084s085.cpp b/Marlin/src/feature/dac/dac_dac084s085.cpp index b88aaf802bdf..772bb68de42e 100644 --- a/Marlin/src/feature/dac/dac_dac084s085.cpp +++ b/Marlin/src/feature/dac/dac_dac084s085.cpp @@ -11,7 +11,6 @@ #include "dac_dac084s085.h" #include "../../MarlinCore.h" -#include "../../module/stepper.h" #include "../../HAL/shared/Delay.h" dac084s085::dac084s085() { } diff --git a/Marlin/src/feature/fwretract.cpp b/Marlin/src/feature/fwretract.cpp index 172c97accdda..28355640d223 100644 --- a/Marlin/src/feature/fwretract.cpp +++ b/Marlin/src/feature/fwretract.cpp @@ -34,7 +34,6 @@ FWRetract fwretract; // Single instance - this calls the constructor #include "../module/motion.h" #include "../module/planner.h" -#include "../module/stepper.h" #include "../gcode/gcode.h" diff --git a/Marlin/src/feature/max7219.cpp b/Marlin/src/feature/max7219.cpp index 285a86ca63f5..2fdfcba32d21 100644 --- a/Marlin/src/feature/max7219.cpp +++ b/Marlin/src/feature/max7219.cpp @@ -44,7 +44,6 @@ #include "max7219.h" #include "../module/planner.h" -#include "../module/stepper.h" #include "../MarlinCore.h" #include "../HAL/shared/Delay.h" diff --git a/Marlin/src/feature/pause.cpp b/Marlin/src/feature/pause.cpp index e9cb2df594d2..106b2516778e 100644 --- a/Marlin/src/feature/pause.cpp +++ b/Marlin/src/feature/pause.cpp @@ -35,10 +35,13 @@ #include "../gcode/gcode.h" #include "../module/motion.h" #include "../module/planner.h" -#include "../module/stepper.h" #include "../module/printcounter.h" #include "../module/temperature.h" +#if HAS_EXTRUDERS + #include "../module/stepper.h" +#endif + #if ENABLED(AUTO_BED_LEVELING_UBL) #include "bedlevel/bedlevel.h" #endif diff --git a/Marlin/src/feature/power.cpp b/Marlin/src/feature/power.cpp index c2ed169aa809..8a16628bac45 100644 --- a/Marlin/src/feature/power.cpp +++ b/Marlin/src/feature/power.cpp @@ -30,7 +30,7 @@ #include "power.h" #include "../module/planner.h" -#include "../module/stepper.h" +#include "../module/stepper/indirection.h" // for restore_stepper_drivers #include "../module/temperature.h" #include "../MarlinCore.h" @@ -46,6 +46,7 @@ Power powerManager; bool Power::psu_on; #if ENABLED(AUTO_POWER_CONTROL) + #include "../module/stepper.h" #include "../module/temperature.h" #if BOTH(USE_CONTROLLER_FAN, AUTO_POWER_CONTROLLERFAN) diff --git a/Marlin/src/feature/tmc_util.cpp b/Marlin/src/feature/tmc_util.cpp index cb970c7ebc87..0867686363ca 100644 --- a/Marlin/src/feature/tmc_util.cpp +++ b/Marlin/src/feature/tmc_util.cpp @@ -33,17 +33,12 @@ #include "../gcode/gcode.h" #if ENABLED(TMC_DEBUG) - #include "../module/planner.h" #include "../libs/hex_print.h" #if ENABLED(MONITOR_DRIVER_STATUS) static uint16_t report_tmc_status_interval; // = 0 #endif #endif -#if HAS_MARLINUI_MENU - #include "../module/stepper.h" -#endif - /** * Check for over temperature or short to ground error flags. * Report and log warning of overtemperature condition. diff --git a/Marlin/src/gcode/bedlevel/G26.cpp b/Marlin/src/gcode/bedlevel/G26.cpp index 21fa08fc107a..d3a8d39dff43 100644 --- a/Marlin/src/gcode/bedlevel/G26.cpp +++ b/Marlin/src/gcode/bedlevel/G26.cpp @@ -107,7 +107,6 @@ #include "../../MarlinCore.h" #include "../../module/planner.h" -#include "../../module/stepper.h" #include "../../module/motion.h" #include "../../module/tool_change.h" #include "../../module/temperature.h" diff --git a/Marlin/src/gcode/bedlevel/abl/G29.cpp b/Marlin/src/gcode/bedlevel/abl/G29.cpp index a2c53b5ab253..0fef5ad68374 100644 --- a/Marlin/src/gcode/bedlevel/abl/G29.cpp +++ b/Marlin/src/gcode/bedlevel/abl/G29.cpp @@ -32,7 +32,6 @@ #include "../../../feature/bedlevel/bedlevel.h" #include "../../../module/motion.h" #include "../../../module/planner.h" -#include "../../../module/stepper.h" #include "../../../module/probe.h" #include "../../queue.h" diff --git a/Marlin/src/gcode/bedlevel/mbl/G29.cpp b/Marlin/src/gcode/bedlevel/mbl/G29.cpp index b9440f78b2e9..e98f3d5ee3a3 100644 --- a/Marlin/src/gcode/bedlevel/mbl/G29.cpp +++ b/Marlin/src/gcode/bedlevel/mbl/G29.cpp @@ -36,7 +36,7 @@ #include "../../../libs/buzzer.h" #include "../../../lcd/marlinui.h" #include "../../../module/motion.h" -#include "../../../module/stepper.h" +#include "../../../module/planner.h" #if ENABLED(EXTENSIBLE_UI) #include "../../../lcd/extui/ui_api.h" diff --git a/Marlin/src/gcode/calibrate/G28.cpp b/Marlin/src/gcode/calibrate/G28.cpp index e312bf07b528..e22e3cb5f8b1 100644 --- a/Marlin/src/gcode/calibrate/G28.cpp +++ b/Marlin/src/gcode/calibrate/G28.cpp @@ -24,8 +24,9 @@ #include "../gcode.h" -#include "../../module/stepper.h" #include "../../module/endstops.h" +#include "../../module/planner.h" +#include "../../module/stepper.h" // for various #if HAS_MULTI_HOTEND #include "../../module/tool_change.h" diff --git a/Marlin/src/gcode/calibrate/G33.cpp b/Marlin/src/gcode/calibrate/G33.cpp index ffe53b63fbef..656c23cb78ff 100644 --- a/Marlin/src/gcode/calibrate/G33.cpp +++ b/Marlin/src/gcode/calibrate/G33.cpp @@ -27,7 +27,7 @@ #include "../gcode.h" #include "../../module/delta.h" #include "../../module/motion.h" -#include "../../module/stepper.h" +#include "../../module/planner.h" #include "../../module/endstops.h" #include "../../lcd/marlinui.h" diff --git a/Marlin/src/gcode/calibrate/G34.cpp b/Marlin/src/gcode/calibrate/G34.cpp index 6fdebb69b0f0..1be3952ffe2e 100644 --- a/Marlin/src/gcode/calibrate/G34.cpp +++ b/Marlin/src/gcode/calibrate/G34.cpp @@ -26,9 +26,12 @@ #include "../gcode.h" #include "../../module/motion.h" -#include "../../module/stepper.h" #include "../../module/endstops.h" +#if ANY(HAS_MOTOR_CURRENT_SPI, HAS_MOTOR_CURRENT_PWM, HAS_TRINAMIC_CONFIG) + #include "../../module/stepper.h" +#endif + #if HAS_LEVELING #include "../../feature/bedlevel/bedlevel.h" #endif diff --git a/Marlin/src/gcode/config/M540.cpp b/Marlin/src/gcode/config/M540.cpp index 54d52f3a31e2..e751248dd641 100644 --- a/Marlin/src/gcode/config/M540.cpp +++ b/Marlin/src/gcode/config/M540.cpp @@ -25,7 +25,7 @@ #if ENABLED(SD_ABORT_ON_ENDSTOP_HIT) #include "../gcode.h" -#include "../../module/stepper.h" +#include "../../module/planner.h" /** * M540: Set whether SD card print should abort on endstop hit (M540 S<0|1>) diff --git a/Marlin/src/gcode/control/M17_M18_M84.cpp b/Marlin/src/gcode/control/M17_M18_M84.cpp index c2c8a702a1b6..4ff48568faea 100644 --- a/Marlin/src/gcode/control/M17_M18_M84.cpp +++ b/Marlin/src/gcode/control/M17_M18_M84.cpp @@ -24,6 +24,7 @@ #include "../../MarlinCore.h" // for stepper_inactive_time, disable_e_steppers #include "../../lcd/marlinui.h" #include "../../module/motion.h" // for e_axis_mask +#include "../../module/planner.h" #include "../../module/stepper.h" #if ENABLED(AUTO_BED_LEVELING_UBL) diff --git a/Marlin/src/gcode/control/M226.cpp b/Marlin/src/gcode/control/M226.cpp index 63f022e82bd8..4eb3db4bc31f 100644 --- a/Marlin/src/gcode/control/M226.cpp +++ b/Marlin/src/gcode/control/M226.cpp @@ -26,7 +26,7 @@ #include "../gcode.h" #include "../../MarlinCore.h" // for pin_is_protected and idle() -#include "../../module/stepper.h" +#include "../../module/planner.h" void protected_pin_err(); diff --git a/Marlin/src/gcode/control/M3-M5.cpp b/Marlin/src/gcode/control/M3-M5.cpp index 3c51de6f6f94..5d5d44e8bfe8 100644 --- a/Marlin/src/gcode/control/M3-M5.cpp +++ b/Marlin/src/gcode/control/M3-M5.cpp @@ -26,7 +26,7 @@ #include "../gcode.h" #include "../../feature/spindle_laser.h" -#include "../../module/stepper.h" +#include "../../module/planner.h" /** * Laser: diff --git a/Marlin/src/gcode/control/M400.cpp b/Marlin/src/gcode/control/M400.cpp index 9a5ad4e9df80..6058fb894e97 100644 --- a/Marlin/src/gcode/control/M400.cpp +++ b/Marlin/src/gcode/control/M400.cpp @@ -21,7 +21,7 @@ */ #include "../gcode.h" -#include "../../module/stepper.h" +#include "../../module/planner.h" /** * M400: Finish all moves diff --git a/Marlin/src/gcode/control/M605.cpp b/Marlin/src/gcode/control/M605.cpp index 06d92c844811..e3ca43e17fd5 100644 --- a/Marlin/src/gcode/control/M605.cpp +++ b/Marlin/src/gcode/control/M605.cpp @@ -28,7 +28,6 @@ #include "../gcode.h" #include "../../module/motion.h" -#include "../../module/stepper.h" #include "../../module/tool_change.h" #include "../../module/planner.h" diff --git a/Marlin/src/gcode/feature/advance/M900.cpp b/Marlin/src/gcode/feature/advance/M900.cpp index 8b59e88fb114..db09faa88310 100644 --- a/Marlin/src/gcode/feature/advance/M900.cpp +++ b/Marlin/src/gcode/feature/advance/M900.cpp @@ -26,7 +26,6 @@ #include "../../gcode.h" #include "../../../module/planner.h" -#include "../../../module/stepper.h" #if ENABLED(EXTRA_LIN_ADVANCE_K) float other_extruder_advance_K[EXTRUDERS]; diff --git a/Marlin/src/gcode/feature/trinamic/M122.cpp b/Marlin/src/gcode/feature/trinamic/M122.cpp index 29416324064f..07fe9e5bd82d 100644 --- a/Marlin/src/gcode/feature/trinamic/M122.cpp +++ b/Marlin/src/gcode/feature/trinamic/M122.cpp @@ -26,7 +26,7 @@ #include "../../gcode.h" #include "../../../feature/tmc_util.h" -#include "../../../module/stepper/indirection.h" +#include "../../../module/stepper/indirection.h" // for restore_stepper_drivers /** * M122: Debug TMC drivers diff --git a/Marlin/src/gcode/geometry/G53-G59.cpp b/Marlin/src/gcode/geometry/G53-G59.cpp index 092c141228fb..c51c29f4233f 100644 --- a/Marlin/src/gcode/geometry/G53-G59.cpp +++ b/Marlin/src/gcode/geometry/G53-G59.cpp @@ -25,8 +25,6 @@ #if ENABLED(CNC_COORDINATE_SYSTEMS) -#include "../../module/stepper.h" - //#define DEBUG_M53 /** diff --git a/Marlin/src/gcode/geometry/G92.cpp b/Marlin/src/gcode/geometry/G92.cpp index 58ed26a15aa8..b36f21d3c08f 100644 --- a/Marlin/src/gcode/geometry/G92.cpp +++ b/Marlin/src/gcode/geometry/G92.cpp @@ -22,7 +22,6 @@ #include "../gcode.h" #include "../../module/motion.h" -#include "../../module/stepper.h" #if ENABLED(I2C_POSITION_ENCODERS) #include "../../feature/encoder_i2c.h" diff --git a/Marlin/src/gcode/motion/G0_G1.cpp b/Marlin/src/gcode/motion/G0_G1.cpp index 933bf3d5d9ef..cee2f050807e 100644 --- a/Marlin/src/gcode/motion/G0_G1.cpp +++ b/Marlin/src/gcode/motion/G0_G1.cpp @@ -32,7 +32,7 @@ #include "../../sd/cardreader.h" #if ENABLED(NANODLP_Z_SYNC) - #include "../../module/stepper.h" + #include "../../module/planner.h" #endif extern xyze_pos_t destination; diff --git a/Marlin/src/gcode/motion/G4.cpp b/Marlin/src/gcode/motion/G4.cpp index df3f5b010e97..ebaa6aabc062 100644 --- a/Marlin/src/gcode/motion/G4.cpp +++ b/Marlin/src/gcode/motion/G4.cpp @@ -21,7 +21,7 @@ */ #include "../gcode.h" -#include "../../module/stepper.h" +#include "../../module/planner.h" #include "../../lcd/marlinui.h" /** diff --git a/Marlin/src/gcode/probe/G38.cpp b/Marlin/src/gcode/probe/G38.cpp index ed24ce3258d9..1b2da756b189 100644 --- a/Marlin/src/gcode/probe/G38.cpp +++ b/Marlin/src/gcode/probe/G38.cpp @@ -28,7 +28,7 @@ #include "../../module/endstops.h" #include "../../module/motion.h" -#include "../../module/stepper.h" +#include "../../module/planner.h" #include "../../module/probe.h" inline void G38_single_probe(const uint8_t move_value) { diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index e51a904aa30e..edde86db512e 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -1458,19 +1458,19 @@ void Stepper::isr() { // Enable ISRs to reduce USART processing latency hal.isr_on(); - if (!nextMainISR) pulse_phase_isr(); // 0 = Do coordinated axes Stepper pulses + if (!nextMainISR) pulse_phase_isr(); // 0 = Do coordinated axes Stepper pulses #if ENABLED(LIN_ADVANCE) - if (!nextAdvanceISR) { // 0 = Do Linear Advance E Stepper pulses + if (!nextAdvanceISR) { // 0 = Do Linear Advance E Stepper pulses advance_isr(); nextAdvanceISR = la_interval; } - else if (nextAdvanceISR == LA_ADV_NEVER) // Start LA steps if neccessary + else if (nextAdvanceISR == LA_ADV_NEVER) // Start LA steps if necessary nextAdvanceISR = la_interval; #endif #if ENABLED(INTEGRATED_BABYSTEPPING) - const bool is_babystep = (nextBabystepISR == 0); // 0 = Do Babystepping (XY)Z pulses + const bool is_babystep = (nextBabystepISR == 0); // 0 = Do Babystepping (XY)Z pulses if (is_babystep) nextBabystepISR = babystepping_isr(); #endif diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index ad34726343f3..a1a17322b9d7 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -421,8 +421,6 @@ class Stepper { la_advance_steps; // Count of steps added to increase nozzle pressure #endif -friend class GcodeSuite; - #if ENABLED(INTEGRATED_BABYSTEPPING) static constexpr uint32_t BABYSTEP_NEVER = 0xFFFFFFFF; static uint32_t nextBabystepISR; From d97e6cd34a96715111a2c48e414bfc9f8fd7c80f Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Fri, 29 Jul 2022 17:46:36 +0100 Subject: [PATCH 17/19] Resolved step frequency alignment issues between pulse_phase_isr() and advance_isr() --- Marlin/src/module/stepper.cpp | 21 +++++++++------------ Marlin/src/module/stepper.h | 3 ++- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index edde86db512e..802301455f9b 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -220,6 +220,7 @@ uint32_t Stepper::advance_divisor = 0, uint32_t Stepper::nextAdvanceISR = LA_ADV_NEVER, Stepper::la_interval = LA_ADV_NEVER; int32_t Stepper::la_delta_error = 0, + Stepper::la_dividend = 0, Stepper::la_advance_steps = 0; #endif @@ -1971,8 +1972,6 @@ uint32_t Stepper::block_phase_isr() { // If no queued movements, just wait 1ms for the next block uint32_t interval = (STEPPER_TIMER_RATE) / 1000UL; - TERN_(LIN_ADVANCE, la_interval = LA_ADV_NEVER); - // If there is a current block if (current_block) { // If current block is finished, reset pointer and finalize state @@ -2146,16 +2145,15 @@ uint32_t Stepper::block_phase_isr() { if (ticks_nominal < 0) { // step_rate to timer interval and loops for the nominal speed ticks_nominal = calc_timer_interval(current_block->nominal_rate << oversampling_factor, steps_per_isr); + + #if ENABLED(LIN_ADVANCE) + if (current_block->la_advance_rate) + la_interval = calc_timer_interval(current_block->nominal_rate) << current_block->la_scaling; + #endif } // The timer interval is just the nominal value for the nominal speed interval = ticks_nominal; - - #if ENABLED(LIN_ADVANCE) - // No more acceleration, so re-use ticks_nominal but discount the effect of oversampling_factor - if (current_block->la_advance_rate) - la_interval = (ticks_nominal << current_block->la_scaling) << oversampling_factor; - #endif } /** @@ -2389,9 +2387,8 @@ uint32_t Stepper::block_phase_isr() { if (stepper_extruder != last_moved_extruder) la_advance_steps = 0; #endif if (current_block->la_advance_rate) { - advance_dividend.e <<= current_block->la_scaling; - // discount the effect of frequency scaling for the stepper - advance_dividend.e <<= oversampling; + // apply LA scaling and discount the effect of frequency scaling + la_dividend = (advance_dividend.e << current_block->la_scaling) << oversampling; } #endif @@ -2469,7 +2466,7 @@ uint32_t Stepper::block_phase_isr() { // Apply Bresenham algorithm so that linear advance can piggy back on // the acceleration and speed values calculated in block_phase_isr(). // This helps keep LA in sync with, for example, S_CURVE_ACCELERATION. - la_delta_error += advance_dividend.e; + la_delta_error += la_dividend; if (la_delta_error >= 0) { count_position.e += count_direction.e; la_advance_steps += count_direction.e; diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h index 3eb0271a1b42..ccf342b573e7 100644 --- a/Marlin/src/module/stepper.h +++ b/Marlin/src/module/stepper.h @@ -420,6 +420,7 @@ class Stepper { static uint32_t nextAdvanceISR, la_interval; // Interval between ISR calls for LA static int32_t la_delta_error, // Analogue of delta_error.e for E steps in LA ISR + la_dividend, // Analogue of advance_dividend.e for E steps in LA ISR la_advance_steps; // Count of steps added to increase nozzle pressure #endif @@ -511,7 +512,7 @@ class Stepper { current_block = nullptr; axis_did_move = 0; planner.release_current_block(); - TERN_(LIN_ADVANCE, nextAdvanceISR = LA_ADV_NEVER); + TERN_(LIN_ADVANCE, la_interval = nextAdvanceISR = LA_ADV_NEVER); } // Quickly stop all steppers From c9d7d423025e9a0d7a3d343b4a55bca2df814954 Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Fri, 29 Jul 2022 17:57:08 +0100 Subject: [PATCH 18/19] Set delta_error.e in XYZEval::operator=() and set la_delta_error to the correct value (not XYZEval::operator bool()) --- Marlin/src/core/types.h | 2 +- Marlin/src/module/stepper.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Marlin/src/core/types.h b/Marlin/src/core/types.h index 335aa3a33449..c9bb7d8c30d1 100644 --- a/Marlin/src/core/types.h +++ b/Marlin/src/core/types.h @@ -688,7 +688,7 @@ struct XYZEval { FI const T& operator[](const int n) const { return pos[n]; } // Assignment operator overrides do the expected thing - FI XYZEval& operator= (const T v) { set(LIST_N_1(NUM_AXES, v)); return *this; } + FI XYZEval& operator= (const T v) { set(LOGICAL_AXIS_LIST_1(v)); return *this; } FI XYZEval& operator= (const XYval &rs) { set(rs.x, rs.y); return *this; } FI XYZEval& operator= (const XYZval &rs) { set(NUM_AXIS_ELEM(rs)); return *this; } diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 802301455f9b..a27e46d51059 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -2363,7 +2363,7 @@ uint32_t Stepper::block_phase_isr() { step_event_count = current_block->step_event_count << oversampling; // Initialize Bresenham delta errors to 1/2 - TERN_(LIN_ADVANCE, la_delta_error =) delta_error = -int32_t(step_event_count); + delta_error = TERN_(LIN_ADVANCE, la_delta_error =) -int32_t(step_event_count); // Calculate Bresenham dividends and divisors advance_dividend = current_block->steps << 1; From 5630aeaa6b813314f828b347bec8f664c7c13969 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Sat, 30 Jul 2022 19:01:13 -0500 Subject: [PATCH 19/19] cleanup --- Marlin/src/module/stepper.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index a27e46d51059..cac2161a474c 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -2107,7 +2107,8 @@ uint32_t Stepper::block_phase_isr() { #else REV_E_DIR(stepper_extruder); #endif - } else { + } + else { #if ENABLED(MIXING_EXTRUDER) MIXER_STEPPER_LOOP(j) NORM_E_DIR(j); #else