"Unretract Speed" and "Pressure Multiplier" features #2018

Closed
wants to merge 1 commit into
from

Projects

None yet

5 participants

@llluis
Contributor
llluis commented May 6, 2014

As discussed in issue #1677, I've implemented 2 new features: "unretract speed" and "pressure advance".
This could be merged into master, as it presents great advantage for bowden systems and allows great speed changes.

@Caanon
Caanon commented May 8, 2014

I'll put a quick +1 in for this pull request. It has noticably improved my prints with my bowden setup and would love to see this integrated into the experimental branch.

@alexrj
Owner
alexrj commented May 9, 2014

@llluis, thanks for your research and effort. I'm really interested in merging a change like this that has been tested and confirmed by so many people. I like this kind of collaboration.
That said, I hope you'll understand that I have some requirements before merging, and I'm a bit strict since I'm likely going to maintain this code in the future. It needs to be robust, as Slic3r is a very complex beast.

From what I see, your pull request includes two distinct changes that are not strictly related (code-wise): a distinct speed setting for unretraction, and an advance algorithm.
If you were to provide a distinct commit just for unretract speed, I'd merge it quickly since it's a clean change with no problems.

Regarding the advance algorithm, I'll tell you what my concerns are. They can be fixed, so don't worry.

  • The feature should be implemented as an internal post-processing filter, just like CoolingBuffer, SpiralVase or ArcFitting. I see that your algorithm does not need any more information than what the G-code itself carries, so we should keep this logic separate. The Slic3r::GCode class should never use regexes because of speed and because it's going to be ported to C++ soon.
  • It's not clear to me why $adv_amnt is multiplied by ($self->extruder->pressure_multiplier/100), since you state that "1 means 100%".
  • pressure_multiplier should not accept negative coordinates (i.e. min should be set to 0)
  • I don't see why the upper limit on pressure_multiplier was set to 200 (thus 20000%?). Is there any reason an upper limit? If so, why did you choose this value?
  • Please use 4 spaces indentation instead of tabs :-)
  • The whole algorithm is not entirely clear to me and I can't find a detailed description in either the linked GitHub issue or branch README or code comments.
    • How do you tell the printer to use a longer retraction length according to the extra advance amount that was injected before last speed change?
    • Supposing all print speeds are the same throughout the print, how do you trigger the advance algorithm after a retraction? $v1 != $v2 will always be false.
    • Supposing bridge speed is set faster than perimeter speed, how will this algorithm affect bridging perimeters, where speed is changed while extruding the perimeter? Will the printer stop just before starting the bridge, then push some material into the extruder and then restart moving to make the perimeter? And when bridge is finished, will it stop and retract a bit before continuing with perimeter? Doesn't such behavior make visible blobs?
  • I believe some of the points I raised above might be implementation bugs (but I can be wrong). This shows the need for severe unit testing.

Thank you for your help… in advance ;-)

@llluis
Contributor
llluis commented May 9, 2014

Hey @alexrj
I sure understand your points. I was really looking for your feedback on this, that's why I submitted the pull request. I got things working, now it's time to learn how to do it properly. =)

  • I'll study how to implement it as a post-processing filter. However, I'm not sure I can get all information from the gcode only, for instance, the "extrusion length per distance unit" ($e). I understand the regex limitation. I could implement without it in the worst case (or in C++).
  • The "1 means 100%" was a (bad) way to explain how to tune the value (I got this asked from people testing the algorithm). It's like the extrusion multiplier: If you use 1, you get the full amount of plastic calculated by the algorithm. Using 0.9 you get only 90% and so on.
    • If the algorithm calculated that it needs to inject 3mm of filament to build pressure, "0.1" as the pressure multiplier would yield only 0.3mm of filament being injected.
    • It needs to be divided by 100 because of the formula of the advance.
    • In my setup, 1 yields the best results. For people using 1.75mm, a value slightly lower as 0.9 is needed. It's just fine tuning.
  • You are correct, it must always be positive value. Although the interface accepts negative values, the code is triggered only with values > 0. It was just my inability to properly code the interface property.
  • There's no need for a upper limit. I just set an arbitrary value high enough (it will never be more than 2 or 3). Again, we could blame my inability with the interface.
  • Sorry about the tabs. I saw your coding guidelines too late.

Let me try to explain the algorithm further: for each speed, it calculates an "advance amount", which is the filament that must the injected in the tube to create the necessary pressure for that speed (different speeds requires different pressure in the tube).
In the beginning of the print, it calculates the advance for the first speed and injects that amount. After that, at each speed change it verifies the amount necessary for the new speed and the amount already injected for the previous speed. Than, it injects (or retracts) the difference. That's why if the print is always printed at the same speed the algorithm does nothing (and it's not supposed to do as it's not needed: the pressure in the tube will be constant during the whole print).
The algorithm is triggered only at speed changes, it's not related to retracts. Also, bridges are "transparent" for it: if the bridge is printed at a different speed (doesn't matter if its a perimeter, infill, bridge), it will have filament injected or retracted from the tube before the start of the segment.
The retraction "merge" is to avoid sudden movements in the extruder, compensating a retract and right after pulling filament. It does only one movement. If not, we would see in the gcode:

G1 E-5.00000 F9000
G1 E1.10000 F9000

Instead, we get:

G1 E-3.90000 F9000

I don't believe there are bugs in the way it's today.
Just lack of explanation from my side and improvements to be made in the code.

Thank you for your time!

@alexrj
Owner
alexrj commented May 10, 2014

Hello @llluis, the extrusion length per distance unit can be calculated from G-code easily, since it's just (E distance)/(XY distance) where distances are relative to the previous G1 line. The Slic3r::GCode::Reader helper class used in internal post-processing filters provides such pre-calculated values in $info->{dist_E} and $info->{dist_XY}.
But don't worry, if I manage to understand the algorithm I can easily implement it myself.

So, 1 means +100%. In other words I guess it's intended as additional flow. Naming it "multiplier" made me think it was like the Extrusion Multiplier where 1 (100%) means no variation. Maybe we could express Pressure Multiplier the same way as Extrusion Multiplier in order to reduce confusion (i.e. 1 = 100% = no variation = algorithm disabled; min allowed value = 1).

I understand the good idea of merging subsequent retracts/unretracts into single command, and that's good. But I still have some doubts about the other two points I raised.
First point: if a print always uses a constant speed, but has retractions, it looks to me that the advance amount is not pushed anymore after each unretraction because $v1 != $v2 will be false. Am I right?

I just tested your branch with a basic bridge object like this:
schermata 2014-05-10 a 12 00 52

and I see several problems.

  • First problem: retraction does not take pressure discharge into account.
  • Second problem: unretraction is shorter than needed, causing missing extrusion (noticeable also in G-code preview done in gcode.ws and Repetier-Host).

For example:

G1 X86.975 Y93.900 E4.70299 ; perimeter
G1 X83.745 Y95.267 F7800.000 ; move to first fill point
G1 E4.79638 F1800.000 ; pressure advance   <-- pressure charge is +0.09mm
G1 X83.763 Y95.334 E4.79842 F3600.000 ; fill
G1 X82.490 Y96.607 E4.85135 ; fill
G1 X81.620 Y96.374 E4.87782 ; fill
...
G1 X76.217 Y97.368 E7.63017 ; fill
G1 F1800.000 E6.63017 ; retract  <-- retracting only by 1mm (= the configured retraction length) while we should have retracted by 1+0.09mm
G92 E0 ; reset extrusion distance
G1 Z6.750 F7800.000 ; move to next layer (16)
G1 X75.850 Y94.900 F7800.000 ; move to first perimeter point
G1 E0.91961 F1800.000 ; compensate retraction + pressure advance  <-- unretracting only by 0.91961mm which is LESS than the amount we retracted by (1mm!)

These problems need to be addressed with unit tests (like those in t/retraction.t) because it's so easy to break things. I'd say unit tests about such a feature are a requirement.

Later on I see another potential problem:

G1 X87.550 Y94.900 E1.25065 F1800.000 ; perimeter  <-- normal perimeter
G1 E1.49743 F1800.000 ; pressure advance  <-- stopping XY movement while doing perimeter, potential blob!
G1 X112.450 Y94.900 E2.18910 F5940.000 ; perimeter  <-- bridge portion of perimeter
G1 E1.94231 F1800.000 ; pressure advance  <-- stopping XY movement while doing perimeter, potential blob!
G1 X124.150 Y94.900 E2.27336 F1800.000 ; perimeter  <-- normal perimeter

I'm not sure about this, since it looks like the pause might be very short. But in my experience, any pause with a Bowden extruder makes a blob. This needs to be tested with a print.

@alexrj
Owner
alexrj commented May 10, 2014

In order to address the last problem I noticed, I think the algorithm should not introduce any retract-only or unretract-only line. It should try to combine pressure charge/discharge with existing retract/unretract lines, but if there are none (for example when changing speed during a continuous extrusion), the advance value should be applied to some final portion of the previous extrusion movement. In the above example, the first line (normal perimeter) should be split in two and the pressure charge amount should be applied to the second part of it. And the second line ("pressure advance") shouldn't be added at all. This way we would never pause the extruder XY movement.

@llluis
Contributor
llluis commented May 12, 2014

Hey @alexrj. Good to know about the helper class, really nice!
I will try it anyway to learn a bit more about Slic3r. I would like to help you coding this, so you can focus on other improvements. ;-)

I agree with your interpretation of the Pressure Multiplier. So let's use it like the Extrusion Multiplier. I will update the formula accordingly.

Regarding the retractions, you are correct, the pressure charge/discharge is never accounted in the retractions/unretractions, even when there are different speeds and the algorithm kicks in.
It's a nice feature to include in the implementation (I didn't thought this during the original development).
This is also why we need to use longer retraction distances in bowden setups, because of the pressure in the tube. If we account for that amount (the value calculated by the algorithm) in the retract, we could use shorter distances in the configuration, very similar to non-bowden printers.

Regarding your test object:

  • First point: as explained above, the extra pressure management length is never accounted in the retractions. It's just a matter of grouping the extruder gcodes together. The retract/unretract is always the original, plus or minus the pressure length.
  • Second point: let's forget the retraction for a while. Imagine the print without it. You would have pressure charges/discharges in every speed change. That's the way the algorithm works (as for now, without the improvement discussed above). In your example gcode (my comments are right after yours):
G1 X86.975 Y93.900 E4.70299 ; perimeter
G1 X83.745 Y95.267 F7800.000 ; move to first fill point
G1 E4.79638 F1800.000 ; pressure advance   <-- pressure charge is +0.09mm (correct)
G1 X83.763 Y95.334 E4.79842 F3600.000 ; fill
G1 X82.490 Y96.607 E4.85135 ; fill
G1 X81.620 Y96.374 E4.87782 ; fill
...
G1 X76.217 Y97.368 E7.63017 ; fill
G1 F1800.000 E6.63017 ; retract  <-- retracting only by 1mm (= the configured retraction length) while we should have retracted by 1+0.09mm (correct, it depends on the new implementation)
G92 E0 ; reset extrusion distance
G1 Z6.750 F7800.000 ; move to next layer (16)
G1 X75.850 Y94.900 F7800.000 ; move to first perimeter point
G1 E0.91961 F1800.000 ; compensate retraction + pressure advance  <-- unretracting only by 0.91961mm which is LESS than the amount we retracted by (1mm!)
It's less because you had a speed change, and thus, there's a pressure discharge to be made. You are going slower, so, you need less pressure in the tube. To decrease the pressure, you need to retract a bit of filament.
Instead of unretract 1mm and right after retract 0.08039mm to decrease the pressure, we unretract only 0.91961mm. 
Before the regex combining the 2 movements, I had the code unretracting the 1mm and retracting the 0.08039mm, which caused a "pressure spike" in the tube, causing a blob. =)
  • Third point: I did not notice any blobs in my prints when changing speeds during XY movements, but I agree that we should minimize this the way you suggested. Would it be possible to still use the post-processing filter if we need to split the last segment in two?
@alexrj
Owner
alexrj commented May 12, 2014

I understand your explanation if we ignore retractions, but retractions actually happen and we need to handle them correctly. Discharging pressure completely upon retraction is needed for two reasons:

  • when performing travel moves, there should be no pressure in the extruder otherwise the travel move will cause ooze (and that pressure would be likely exhausted when travel move is finished anyway)
  • unretracting by less the amount of the retraction causes G-code preview software to hide some extrusions (I haven't got the G-code with me right now, but try yourself) thus showing a very broken preview; this would cause instant tons of bug reports in my mailbox :-)

Regarding potential blobs caused by pausing I think the test bridge object I used above should be tested. And yes, the post-processing approach can be definitely used also when splitting the last segment in two. Actually, the length to take from that last segment should be decided, and that might include even more than the last segment itself if it's too short...

@llluis
Contributor
llluis commented May 12, 2014

Sorry, I didn't mean to completely ignore retractions, it was just to try to explain algorithm behavior.
I understand and agree that the pressure must be discharged (and the reason why).

I don't know how not to unretract by less the amount of retraction to not break the preview.
I've noticed that before (and I thought it was a bug. I can imagine the bug reports!), but figured it that was caused by the shorter unretraction.

Otherwise, we cannot merge retractions with pressure management and use exclusively the segment splitting technique.

@alexrj
Owner
alexrj commented May 12, 2014

Your pressure extra amount will always be positive after a retraction/unretraction since you have discharged completely before it. So the full unretraction needs to be performed (i.e. equal to the configured retract_length) + such positive extra amount for pressure. If you implement this correctly you'll never cause unretraction to be less than retraction.

@whosawhatsis whosawhatsis referenced this pull request in MarlinFirmware/Marlin May 13, 2014
Closed

New algorithm Velocity Extruding for more smooth print #916

@llluis
Contributor
llluis commented May 13, 2014

Are you proposing to charge / discharge all the pressure amount on every "speed segment"?
So, let's say there is a part of the print being printed at 50mm/s. At the beginning of this part we push the amount calculated by the algorithm and then at the end, before changing to a new speed, we pull the added filament completely.
For the next part, being printed at 100mm/s, we again inject all filament calculated by the algorithm and so on.

Is my understanding correct?

Today, the implementation push or pull just the difference between the amount calculated by the algorithm on speed transitions.

@alexrj
Owner
alexrj commented May 13, 2014

No, I'm only talking about discharging before each retraction

@llluis
Contributor
llluis commented May 13, 2014

I think I got it. Let me do some tests.

@malcom2073

+1 Any update on this? I could really use the pressure advance

@llluis
Contributor
llluis commented Jun 27, 2014

I've implemented the feature as a post-processing filter in the current Slic3r version.
It includes the computation of the advance length in the retracts and unretracts (and it works with relative or absolute E distances).
Now, it's just a matter of discharging the advance amount before any speed decrease to avoid breaking gcode visualization.

@alexrj would you prefer to close this pull request and open a new one with the code and referencing this?

@Caanon
Caanon commented Jul 7, 2014

Ping. Would love to have this feature integrated (if only to be able to not to have to merge @llluis 's branch with @alexrj updates the dev branch every time slic3r is updated :). I'm looking into writing my own python post-processing script to do exactly this type of pressure management, but it's quite rough around the edges at the moment...

@llluis
Contributor
llluis commented Jul 7, 2014

@Caanon, my current version is at https://github.com/llluis/Slic3r
The filter branch is synchronized with alexrj' master and contains my filter implementation.
You could test it.

@alexrj alexrj added a commit that referenced this pull request Nov 24, 2014
@alexrj New experimental feature for pressure management. Credits to @llluis
…for the original implementation. #1203 #1677 #2018
ff9b532
@alexrj
Owner
alexrj commented Nov 24, 2014

Hello @llluis, I just implemented pressure management in ff9b532 based on your proposed algorithm. My implementation is slightly different:

  • supports multiple extruders
  • supports Use firmware retraction
  • expresses the user-configurable value as a generic K constant instead of a percentage whose basis was not clear
  • discharges pressure fully before any retraction command

I'd be happy if you could test this and let me know how it works for you.

@llluis
Contributor
llluis commented Nov 24, 2014

Hi @alexrj, this is awesome, thanks!
I'll for sure test it and provide feedback.

One thing I noticed regarding the implementation as a filter triggered by the layer processing: when we have slowdowns due to minimal layer time, the pressure algorithm calculates its values using full speed, as the CoolingBuffer is triggered in the end, for the whole gcode. This will cause issues with very slow layers.

@alexrj
Owner
alexrj commented Nov 30, 2014

You're right about the speeds being altered by CoolingBuffer too late. I fixed this in 98cb9f0

@braddo99

I posted a new issue, but perhaps it should have just been a comment here... 1.2.5 is currently not working with firmware retract. 1.2.4 retracted incorrect huge amounts, this is now fixed, but 1.2.5 is still unretracting incorrect huge amounts of plastic. 1.2.5 is not usable with firmware retract as far as I can tell.

@llluis
Contributor
llluis commented Aug 10, 2015

Already implemented in the master branch.

@llluis llluis closed this Aug 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment