Linear Advance 1.5 Fixes #2785
Merged
Linear Advance 1.5 Fixes #2785
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PR prusa3d#2591 made LA compression always account for retractions instead of discarding the current compression steps. While this fixed overextrusion in short segments followed by wipes, it uncovered another issue in how the compression steps are spread during the trapezoid calculations leading to gaps in segments followed by retractions (as highlighted by /some/ prints in prusa3d#2693). LA1.5 always computes the required target compression steps for a segment at nominal speed. Because of how the extra steps are allocated using multiples of the accelerating frequency, if the segment is truncated before cruising is reached, an additional cycle of steps can be inserted before deceleration starts. Deceleration is also not guaranteed to be symmetric where up to _two_ cycles can be skipped depending on the stepping cycle, leading to a situation where a symmetric acceleration/deceleration block will lead up to a cycle of accumulated compression. While forcing an the extra step during deceleration is possible by tweaking the error term (eISR_Err), this doesn't guarantee balance in all cases. The underlying issue is that the function is aiming a compression which cannot be reached (nominal speed), and not at the effective max speed reached in the trapezoid, thus moving the average result higher over time. We fix this by calculating the effective maximum speed (and compression) reached during the trapezoid, which stops compression on the required cycle irregardless of the error term, balancing the result. This is the first unoptimized POC: this is not for production: a lot of calculations are redundand and could work directly in steps/s^2.
Reduce per-trapezoid calculations
Remove another division by precomputing the division directly in adv_comp.
There used to be a single stage where an extruder reversal could occur, but since PR prusa3d#2591 reversals can happen up to two times per trapezoid. Reset LA_phase when ADV_INIT is set, since it is re-inizialized only when needed a few lines afterward. This improves performance by avoiding to check the phase continuosly to the end of the trapezoid. Likewise, always set ADV_INIT during the first cruising step, also to force a LA_phase reset.
Simplify and fix the broken timer check when scheduling advance ticks. This dates back to the original LA15 PR, an old bug...
Avoid sqrt when possible
When switching to a new trapezoid step with the right pressure, cancel any pending eISR right away. Similarly do not schedule another eISR if the pressure will be reached by the end of the eISR. This was done in the past to preserve the current LA_phase. This is not needed anymore, since it will be reset at each trapezoid step when LA is re-initialized.
Before PR prusa3d#2591 LA was automatically capped during cruising or deceleration. However we now rely on reaching the current pressure state exactly to stop. When dual/quad stepping inside the eISR we might incur in oscillating behavior if we do not handle it correctly. This might be the cause behind prusa3d#2757 This now changes e_step_loops to be a phase-local variable, so we now reset it each phase too (instead of per-segment).
The local interval calculated by advance_spread() might oscillate too much in narrow intervals.
The logic was inverted, causing the fastest isr to always retract instead of counter-balance the acceleration properly.
Turns out for high-res curved models the numerical error and the SLOWDOWN handling in the planner can cause enough variance in the calculated pressure to trigger LA to continuosly, making matters worse. Clamp LA again, but only during extrusion, so that the runaway error is limited by the current segment length.
The e/D ratio should be calculated using the extrusion length. As such, purify the e_D_ratio from the current extruder multiplier in order to account correctly for flow adjustments.
If you're using flow to correct for an incorrect source diameter, which is probably the main usage when using the LCD, then LA shouldn't be adjusted. It's still unclear what the effect of M221 in gcode should be regarding overall extrusion width. If M221 means "thicker lines", then LA should also be adjusted accordingly. This stems from the fact that the source diameter/length needs to be known in order to determine a compression factor which is independent of the extrusion width, but the FW only ever sees one value currently (the extrusion length) which combines both. This makes it impossible for the FW to adjust for one OR the other scenario, depending on what you expect for M221 to mean.
Remove most of the original complexity from advance_spread. Instead of accumulating time to be scheduled, plan ahead of time each eISR tick using the next main interval + an accumulator (eISR_Err), which keeps everything much simpler. The distribution of the advance ticks is now using the real LA frequency, which leaves a bit more time between the last LA tick and the main stepper isr. We take advantage of the accumulator to force a LA tick right after the first main tick, which removes a +/- 1 scheduling error at higher step rates. When decompressing, we force 2 steps instead, so that the direction reversal happens immediately (first tick zeros esteps, second inverts the sign), removing another +/- 1 error at higher step rates.
|
Resources previous pulled PR vs this PR:
|
DRracer
approved these changes
Aug 5, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes multiple issues with LA1.5 for both stock and geared extruders, as discovered and reported by multiple users in #2543, #2674, #2693, #2751, #2757, #2760.
Major issues covered:
Since the error can be large and difficult to overcome without further performance penalties, this was fixed by artificially limiting LA back to the acceleration/deceleration phase so that short segments can necessarily only have a limited influence. The effective compression will still converge to an average result which is more stable than what is being calculated currently (c08f37d, c54474f). This still preserves the ability to handle chained wipes, but now is doing so only when wiping (no extrusion).
The code scheduling advance ticks has been simplified and speed-up (50a0982, feafc5e). Other commits only perform minor cleanups.
Notably, There is now also an optional automatic LA adjustment for flow control (9b8f642) which can change the behavior of M221 or the FLOW settings in the LCD. Since this behavior is highly dependent on the intended behavior of the adjustment, we believe that keeping this disabled is actually the intended behavior. See a08ca19 for a discussion.
PFW-1125