From 5d27f3362ae937b0ac9be520993323c116ec1991 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 8 Apr 2020 22:27:57 +0200 Subject: [PATCH 1/6] Remove empty line --- Firmware/Marlin_main.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index 8b3d4a73..6999db99 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -7116,7 +7116,6 @@ Sigma_Exit: { float e = code_value(); #ifndef LA_NOCOMPAT - e = la10c_jerk(e); #endif cs.max_jerk[E_AXIS] = e; From 9ec0ac9c64324067fa220886d3d8e5a0ceb9ce27 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 8 Apr 2020 22:30:39 +0200 Subject: [PATCH 2/6] Always reset e_steps between blocks If e_steps are scheduled, but not ticked, they're just lost. Only carry over the pressure state. --- Firmware/stepper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firmware/stepper.cpp b/Firmware/stepper.cpp index 6e0937cf..c7105da7 100644 --- a/Firmware/stepper.cpp +++ b/Firmware/stepper.cpp @@ -354,10 +354,10 @@ FORCE_INLINE void stepper_next_block() max_adv_steps = current_block->max_adv_steps; e_step_loops = current_block->advance_step_loops; } else { - e_steps = 0; e_step_loops = 1; current_adv_steps = 0; } + e_steps = 0; nextAdvanceISR = ADV_NEVER; LA_phase = -1; #endif From 919386c957eaf35ff0cca442c9e3000ccf36f2ae Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 8 Apr 2020 22:36:27 +0200 Subject: [PATCH 3/6] Remove several globals by using a single target pressure In the current code we initialize the LA state on-demand already at the right step, which makes keeping track of the tick position no longer necessary. Make the advance ISR almost stateless by removing the last vestiges of the original implementation and introduce a single target pressure. This will be needed later in order to trigger the LA isr inside the cruising phase. --- Firmware/stepper.cpp | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/Firmware/stepper.cpp b/Firmware/stepper.cpp index c7105da7..bc860b14 100644 --- a/Firmware/stepper.cpp +++ b/Firmware/stepper.cpp @@ -128,9 +128,7 @@ volatile signed char count_direction[NUM_AXIS] = { 1, 1, 1, 1}; static uint16_t eISR_Err; static uint16_t current_adv_steps; - static uint16_t final_adv_steps; - static uint16_t max_adv_steps; - static uint32_t LA_decelerate_after; + static uint16_t target_adv_steps; static int8_t e_steps; static uint8_t e_step_loops; @@ -349,10 +347,8 @@ FORCE_INLINE void stepper_next_block() #ifdef LIN_ADVANCE if (current_block->use_advance_lead) { - LA_decelerate_after = current_block->decelerate_after; - final_adv_steps = current_block->final_adv_steps; - max_adv_steps = current_block->max_adv_steps; e_step_loops = current_block->advance_step_loops; + target_adv_steps = current_block->max_adv_steps; } else { e_step_loops = 1; current_adv_steps = 0; @@ -827,11 +823,14 @@ FORCE_INLINE void isr() { uint16_t timer = calc_timer(step_rate, step_loops); _NEXT_ISR(timer); deceleration_time += timer; + #ifdef LIN_ADVANCE if (current_block->use_advance_lead) { la_state = ADV_DECELERATE; - if (step_events_completed.wide <= (unsigned long int)current_block->decelerate_after + step_loops) + if (step_events_completed.wide <= (unsigned long int)current_block->decelerate_after + step_loops) { + target_adv_steps = current_block->final_adv_steps; la_state |= ADV_INIT; + } } #endif } @@ -898,7 +897,7 @@ FORCE_INLINE void isr() { // Timer interrupt for E. e_steps is set in the main routine. FORCE_INLINE void advance_isr() { - if (step_events_completed.wide > LA_decelerate_after && current_adv_steps > final_adv_steps) { + if (current_adv_steps > target_adv_steps) { // decompression e_steps -= e_step_loops; if (e_steps) WRITE_NC(E0_DIR_PIN, e_steps < 0? INVERT_E0_DIR: !INVERT_E0_DIR); @@ -908,7 +907,7 @@ FORCE_INLINE void advance_isr() { current_adv_steps = 0; nextAdvanceISR = eISR_Rate; } - else if (step_events_completed.wide < LA_decelerate_after && current_adv_steps < max_adv_steps) { + else if (current_adv_steps < target_adv_steps) { // compression e_steps += e_step_loops; if (e_steps) WRITE_NC(E0_DIR_PIN, e_steps < 0? INVERT_E0_DIR: !INVERT_E0_DIR); @@ -1233,9 +1232,6 @@ void st_init() nextMainISR = 0; nextAdvanceISR = ADV_NEVER; main_Rate = ADV_NEVER; - e_steps = 0; - e_step_loops = 1; - LA_phase = -1; current_adv_steps = 0; #endif From 02a36c498cfb087fc4d1e470a7490d08a10484a5 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Wed, 8 Apr 2020 22:49:48 +0200 Subject: [PATCH 4/6] Release excess pressure within cruising blocks LA assumes all the nozzle pressure is released at the end of each extrusion, which makes calculating the required pressure advance during travels and retracts not normally necessary. This is not always true in our planner, since the E axis is explicitly ignored when not in use, but also due to E-jerk allowing a non-linear jump in speed. And since the compression factor is currently tied by XYZ axes and not independently calculated, this can result in a wrong estimation of final pressure in several conditions. To avoid overburdening the planner, change the underlying assumptions about backpressure: 1) Pressure is no longer lost when LA is disabled: if a retract is followed by an unretract of the same length, the pressure will be likely maintained entirely. This also holds true during travels, as long as the retract length can overcome all the backpressure (which is the case in all but the most noodly materials) 2) Pressure is released as soon as possible during travels: we now enable LA also during travels, but under the sole condition of undoing excess pressure. We do that by checking for backpressure at the start of any segment without an acceleration phase that doesn't have any E-steps (a result which can happen due to the above). If pressure is not nominal, we run the extruder in reverse at maximum jerk as long as the segment allows us, since proper acceleration would be prohibitive at this stage. As the pressure difference resulting by the above is still _very_ low, any wipe or short travel will be able to equalize the nozzle pressure *before* extrusion is resumed, avoiding ooze. --- Firmware/planner.cpp | 17 ++++++++++------- Firmware/stepper.cpp | 19 ++++++++++++++++++- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/Firmware/planner.cpp b/Firmware/planner.cpp index 57d7b89b..35031ab2 100644 --- a/Firmware/planner.cpp +++ b/Firmware/planner.cpp @@ -1061,14 +1061,12 @@ Having the real displacement of the head, we can calculate the total movement le /** * Use LIN_ADVANCE within this block if all these are true: * - * block->steps_e : This is a print move, because we checked for X, Y, Z steps before. * extruder_advance_K : There is an advance factor set. - * delta_mm[E_AXIS] > 0 : Extruder is running forward (e.g., for "Wipe while retracting" (Slic3r) or "Combing" (Cura) moves) + * delta_mm[E_AXIS] >= 0 : Extruding or traveling, but _not_ retracting. * |delta_mm[Z_AXIS]| < 0.5 : Z is only moved for leveling (_not_ for priming) */ - block->use_advance_lead = block->steps_e.wide - && extruder_advance_K - && delta_mm[E_AXIS] > 0 + block->use_advance_lead = extruder_advance_K + && delta_mm[E_AXIS] >= 0 && abs(delta_mm[Z_AXIS]) < 0.5; if (block->use_advance_lead) { e_D_ratio = (e - position_float[E_AXIS]) / @@ -1082,7 +1080,7 @@ Having the real displacement of the head, we can calculate the total movement le // 100mm wide lines using 3mm filament or 35mm wide lines using 1.75mm filament. if (e_D_ratio > 3.0) block->use_advance_lead = false; - else { + else if (e_D_ratio > 0) { const uint32_t max_accel_steps_per_s2 = cs.max_jerk[E_AXIS] / (extruder_advance_K * e_D_ratio) * steps_per_mm; if (block->acceleration_st > max_accel_steps_per_s2) { block->acceleration_st = max_accel_steps_per_s2; @@ -1133,9 +1131,14 @@ Having the real displacement of the head, we can calculate the total movement le block->adv_comp = extruder_advance_K * e_D_ratio * cs.axis_steps_per_unit[E_AXIS]; block->max_adv_steps = block->nominal_speed * block->adv_comp; + float advance_speed; + if (e_D_ratio > 0) + advance_speed = (extruder_advance_K * e_D_ratio * block->acceleration * cs.axis_steps_per_unit[E_AXIS]); + else + advance_speed = cs.max_jerk[E_AXIS] * cs.axis_steps_per_unit[E_AXIS]; + // to save more space we avoid another copy of calc_timer and go through slow division, but we // still need to replicate the *exact* same step grouping policy (see below) - float advance_speed = (extruder_advance_K * e_D_ratio * block->acceleration * cs.axis_steps_per_unit[E_AXIS]); if (advance_speed > MAX_STEP_FREQUENCY) advance_speed = MAX_STEP_FREQUENCY; float advance_rate = (F_CPU / 8.0) / advance_speed; if (advance_speed > 20000) { diff --git a/Firmware/stepper.cpp b/Firmware/stepper.cpp index bc860b14..de068166 100644 --- a/Firmware/stepper.cpp +++ b/Firmware/stepper.cpp @@ -351,7 +351,6 @@ FORCE_INLINE void stepper_next_block() target_adv_steps = current_block->max_adv_steps; } else { e_step_loops = 1; - current_adv_steps = 0; } e_steps = 0; nextAdvanceISR = ADV_NEVER; @@ -840,6 +839,24 @@ FORCE_INLINE void isr() { // the initial interrupt blocking. OCR1A_nominal = calc_timer(uint16_t(current_block->nominal_rate), step_loops); step_loops_nominal = step_loops; + +#ifdef LIN_ADVANCE + if(current_block->use_advance_lead) { + if(current_adv_steps < target_adv_steps) { + // after reaching cruising speed, halt compression. if we couldn't accumulate the + // required pressure in the acceleration phase due to lost ticks it's unlikely we + // could undo all of it during deceleration either + target_adv_steps = current_adv_steps; + } + else if (!nextAdvanceISR && current_adv_steps > target_adv_steps) { + // we're cruising in a block with excess backpressure and without a previous + // acceleration phase - this *cannot* happen during a regular block, but it's + // likely in result of chained a wipe move. release the pressure earlier by + // forcedly enabling LA while cruising! + la_state = ADV_INIT; + } + } +#endif } _NEXT_ISR(OCR1A_nominal); } From 13b0e27cd76e014c7856ce8037f618242dad7e37 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Thu, 9 Apr 2020 22:34:30 +0200 Subject: [PATCH 5/6] Do not overflow during LA acceleration limiting Perform the check one step earlier, avoiding 32bit overflow for very low compression factors. Fixes #2566 (although for K15 to have effect the conversion probably needs to be adjusted on the low end) --- Firmware/planner.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Firmware/planner.cpp b/Firmware/planner.cpp index 35031ab2..1bc97de6 100644 --- a/Firmware/planner.cpp +++ b/Firmware/planner.cpp @@ -1081,9 +1081,9 @@ Having the real displacement of the head, we can calculate the total movement le if (e_D_ratio > 3.0) block->use_advance_lead = false; else if (e_D_ratio > 0) { - const uint32_t max_accel_steps_per_s2 = cs.max_jerk[E_AXIS] / (extruder_advance_K * e_D_ratio) * steps_per_mm; - if (block->acceleration_st > max_accel_steps_per_s2) { - block->acceleration_st = max_accel_steps_per_s2; + const float max_accel_per_s2 = cs.max_jerk[E_AXIS] / (extruder_advance_K * e_D_ratio); + if (cs.acceleration > max_accel_per_s2) { + block->acceleration_st = ceil(max_accel_per_s2 * steps_per_mm); #ifdef LA_DEBUG SERIAL_ECHOLNPGM("LA: Block acceleration limited due to max E-jerk"); #endif From ae4abdf11f5ae57478333f8b08e10255ecdd03d4 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Sun, 12 Apr 2020 01:17:45 +0200 Subject: [PATCH 6/6] Unify LA for all trapezoid steps Handle uniformly compression & decompression at any stage of the trapezoid. Compared to before, this now enables LA compression also in the cruising step (handling the converse of a chained wipe), as well as decompression during acceleration. Both of these can happen as a result of jerk moves, but are incredibly rare. This is mostly needed to allow rapid decompression directly at the acceleration step during travels between a retraction&deretraction. We also check for the pressure level in a single place, reducing code size as well as disabling LA earlier when not needed for the rest of the block. --- Firmware/planner.cpp | 4 +++- Firmware/stepper.cpp | 54 +++++++++++++++++++++++++++----------------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/Firmware/planner.cpp b/Firmware/planner.cpp index 1bc97de6..0d77900e 100644 --- a/Firmware/planner.cpp +++ b/Firmware/planner.cpp @@ -1065,10 +1065,12 @@ Having the real displacement of the head, we can calculate the total movement le * delta_mm[E_AXIS] >= 0 : Extruding or traveling, but _not_ retracting. * |delta_mm[Z_AXIS]| < 0.5 : Z is only moved for leveling (_not_ for priming) */ - block->use_advance_lead = extruder_advance_K + block->use_advance_lead = extruder_advance_K > 0 && delta_mm[E_AXIS] >= 0 && abs(delta_mm[Z_AXIS]) < 0.5; if (block->use_advance_lead) { + // all extrusion moves with LA require a compression which is proportional to the + // extrusion_length to distance ratio (e/D) e_D_ratio = (e - position_float[E_AXIS]) / sqrt(sq(x - position_float[X_AXIS]) + sq(y - position_float[Y_AXIS]) diff --git a/Firmware/stepper.cpp b/Firmware/stepper.cpp index de068166..1b4c3b9a 100644 --- a/Firmware/stepper.cpp +++ b/Firmware/stepper.cpp @@ -117,8 +117,8 @@ volatile signed char count_direction[NUM_AXIS] = { 1, 1, 1, 1}; void advance_isr(); static const uint16_t ADV_NEVER = 0xFFFF; - static const uint8_t ADV_INIT = 0b01; - static const uint8_t ADV_DECELERATE = 0b10; + static const uint8_t ADV_INIT = 0b01; // initialize LA + static const uint8_t ADV_ACC_VARY = 0b10; // varying acceleration phase static uint16_t nextMainISR; static uint16_t nextAdvanceISR; @@ -130,9 +130,10 @@ volatile signed char count_direction[NUM_AXIS] = { 1, 1, 1, 1}; static uint16_t current_adv_steps; static uint16_t target_adv_steps; - static int8_t e_steps; - static uint8_t e_step_loops; - static int8_t LA_phase; + static int8_t e_steps; // scheduled e-steps during each isr loop + static uint8_t e_step_loops; // e-steps to execute at most in each isr loop + static uint8_t e_extruding; // current move is an extrusion move + static int8_t LA_phase; // LA compensation phase #define _NEXT_ISR(T) main_Rate = nextMainISR = T #else @@ -366,11 +367,17 @@ FORCE_INLINE void stepper_next_block() counter_y.lo = counter_x.lo; counter_z.lo = counter_x.lo; counter_e.lo = counter_x.lo; +#ifdef LIN_ADVANCE + e_extruding = current_block->steps_e.lo != 0; +#endif } else { counter_x.wide = -(current_block->step_event_count.wide >> 1); counter_y.wide = counter_x.wide; counter_z.wide = counter_x.wide; counter_e.wide = counter_x.wide; +#ifdef LIN_ADVANCE + e_extruding = current_block->steps_e.wide != 0; +#endif } step_events_completed.wide = 0; // Set directions. @@ -806,7 +813,7 @@ FORCE_INLINE void isr() { #ifdef LIN_ADVANCE if (current_block->use_advance_lead) { if (step_events_completed.wide <= (unsigned long int)step_loops) - la_state = ADV_INIT; + la_state = ADV_INIT | ADV_ACC_VARY; } #endif } @@ -825,10 +832,9 @@ FORCE_INLINE void isr() { #ifdef LIN_ADVANCE if (current_block->use_advance_lead) { - la_state = ADV_DECELERATE; if (step_events_completed.wide <= (unsigned long int)current_block->decelerate_after + step_loops) { target_adv_steps = current_block->final_adv_steps; - la_state |= ADV_INIT; + la_state = ADV_INIT | ADV_ACC_VARY; } } #endif @@ -842,17 +848,10 @@ FORCE_INLINE void isr() { #ifdef LIN_ADVANCE if(current_block->use_advance_lead) { - if(current_adv_steps < target_adv_steps) { - // after reaching cruising speed, halt compression. if we couldn't accumulate the - // required pressure in the acceleration phase due to lost ticks it's unlikely we - // could undo all of it during deceleration either - target_adv_steps = current_adv_steps; - } - else if (!nextAdvanceISR && current_adv_steps > target_adv_steps) { - // we're cruising in a block with excess backpressure and without a previous - // acceleration phase - this *cannot* happen during a regular block, but it's - // likely in result of chained a wipe move. release the pressure earlier by - // forcedly enabling LA while cruising! + if (!nextAdvanceISR) { + // Due to E-jerk, there can be discontinuities in pressure state where an + // acceleration or deceleration can be skipped or joined with the previous block. + // If LA was not previously active, re-check the pressure level la_state = ADV_INIT; } } @@ -865,10 +864,23 @@ FORCE_INLINE void isr() { #ifdef LIN_ADVANCE // avoid multiple instances or function calls to advance_spread - if (la_state & ADV_INIT) eISR_Err = current_block->advance_rate / 4; + if (la_state & ADV_INIT) { + if (current_adv_steps == target_adv_steps) { + // nothing to be done in this phase + la_state = 0; + } + else { + eISR_Err = current_block->advance_rate / 4; + if ((la_state & ADV_ACC_VARY) && e_extruding && (current_adv_steps > target_adv_steps)) { + // LA could reverse the direction of extrusion in this phase + LA_phase = 0; + } + } + } if (la_state & ADV_INIT || nextAdvanceISR != ADV_NEVER) { + // update timers & phase for the next iteration advance_spread(main_Rate); - if (la_state & ADV_DECELERATE) { + if (LA_phase >= 0) { if (step_loops == e_step_loops) LA_phase = (eISR_Rate > main_Rate); else {