Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Advance extrusion algorithm – LIN_ADVANCE #3676

Merged
merged 5 commits into from
Jun 14, 2016

Conversation

thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented May 4, 2016

@Sebastianv650 has been working on this over on his fork. This is a patched up PR for compatibility with stepper_indirection.h which already handles multiple steppers for different features. This also keeps Stepper::advance_isr compatible with the upcoming Mixing Extruder feature.

  • Disable LIN_ADVANCE by default
  • Added a sanity check to prevent enabling both ADVANCE and LIN_ADVANCE
  • Blocks use ENABLED(LIN_ADVANCE)
  • Blocks use #if ... #elif ... #endif to group LIN_ADVANCE with ADVANCE
  • The advance_isr can be simplified due to macros in stepper_indirection.h

References: MarlinFirmware/MarlinDev#402 #3655

@thinkyhead thinkyhead added PR: Improvement PR: New Feature Needs: Testing Testing is needed for this change S: Don't Merge Work in progress or under discussion. labels May 4, 2016

#if ENABLED(LIN_ADVANCE)

for (int i = 0; i < EXTRUDERS; i++) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that in the near future, a single extruder might have multiple steppers. So e_steps will not depend on EXTRUDERS but probably something named E_MOTORS or similar.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thinkyhead shouldn't we clean up the "old" ADVANCE feature ? Having two advance features using two different methods but when only one has active support and is known to work.. I would cull the other one.

Copy link
Contributor

@paulusjacobus paulusjacobus May 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbrazio in order to fix the advance feature, it needs a lot of discussion. You could create a wiki collaboration page on this. I have spent the last week a lot of thinking of this feature and looked the implementations across github like Sailfish (JNK) and Smoothieboard.

The lessons learnt by me are worth discussing on a wiki page. For example, to give you a small taste: the XY motion is calculated by the planner which is the old Grbl planner written a long time ago. It was written for CNC machines. The extruder was added later and needs to be incorporated into the planner in a smarter and more logical way. The priming and de-priming of the hot-end need to be ahead of the xy planned motion in terms of time/steps. Lots of ideas around that in terms of implementation like a third pass of the planning cycle to fill the buffer with the extruder values and using boost/Lapack libraries to simplify calculations etc.

Basically I suggest to take a structured approach in collaboration on a wiki page for this JNK feature rather than discussing via a issue thread (you lose the thinking and conclusions and new comers has to scroll through large threads whilst differentiating between opinions/rubbish and real facts) and throwing in the implementations from members which I don't understand. What do you think?

On 5 May 2016 at 09:28, João Brázio [email protected] wrote:

In Marlin/stepper.cpp
#3676 (comment):

@@ -601,14 +639,28 @@ void Stepper::init() {
TCNT1 = 0;
ENABLE_STEPPER_DRIVER_INTERRUPT();

  • #if ENABLED(ADVANCE)
  • #if ENABLED(ADVANCE) || ENABLED(LIN_ADVANCE)
  • #if ENABLED(LIN_ADVANCE)
  •  for (int i = 0; i < EXTRUDERS; i++) {
    

@thinkyhead https://github.com/thinkyhead shouldn't we clean up the
"old" ADVANCE feature ? Having two advance features using two different
methods but when only one has active support and is known to work.. I would
cull the other one.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
https://github.com/MarlinFirmware/Marlin/pull/3676/files/b497db094e421700112ec5a07e32a20bb9deaf6c#r62131409

Copy link
Member Author

@thinkyhead thinkyhead May 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbrazio I didn't want to completely drop the old ADVANCE feature because someone out there might find it charming and want to make it work better. I think it's probably a Good Thing™ to incorporate all the extant advance algorithms, because it makes a great reference to have the code in-situ and where they can be compared and refined over time. In the longer run, we will probably end up with a nice unified algorithm.

If the old ADVANCE feature is irredeemably broken, then we may as well drop it, but I would like to understand it better (and what the original developer was thinking) before excising it completely.

@paulusjacobus A wiki page is good to work on the feature documentation, but for figuring things out, it's fine to just keep discussing it in a Marlin issue. Everything I've been reading indicates that using a separate ISR for the E steppers produces nicer results, so I'm very enthusiastic about these algorithms.

Copy link
Contributor

@paulusjacobus paulusjacobus May 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to completely drop the old ADVANCE feature because someone out there might find it charming and want to make it work better

Yes, that's me!

Everything I've been reading indicates that using a separate ISR for the E steppers produces nicer results, so >I'm very enthusiastic about these algorithms.

Actually you need to understand in detail how the planner works in order to spot the weaknesses, shortcomings so you can collaborate and understand how to fix it. For me, this means I require pictures to explain current issues with definitions so collaborators speak the same language. For example the velocity model: trapezoid and the relationship between velocity, acceleration and jerk.
image
Researchers from universities are now writing papers about the best strategies like maximise jerk and greedy feeding algorithms. Currently I cannot optimise the feeding buffersize or change the trapezoid profile. Ideally I would like play with these and observe the quality of the print and enhance the planner maths/model. I believe there is a lot of improvement here to be made since nobody has improved the Grbl planner. See http://maxwellsci.com/print/rjaset/v5-3543-3548.pdf

At this moment of speaking @jbrazio is closing the issues with information (I must say, in most cases it is justified). That means for me, some research or learning is lost. Conclusions and learnings need to be captured in a wiki so that can be the spec for designing and building the overhaul for a complex item (not a bug fix or simple additional feature). Anyway that is just my opinion based on my way of learning and designing.

@Sebastianv650
Copy link
Contributor

@thinkyhead thanks for updating the PR and streamlining the code!
There is still a problem when the advance_isr is used which results in blobs on line starts. It's hard to hunt this down: It's not there in the RC4 implementation that also uses the 2nd isr as long as the FW retract speed is kept at about 35mm/min. At 55mm/min there is also blobbing. I guess it has something to do with the extruder can't handle multiple esteps at once. I will try to modify the advance_ISR to distribute the steps equaly on the time line given between two main isr calls..

@jbrazio @thinkyhead : There might still be a reason for keeping another advance method like the original one. My approach leads to better print results as it is closer to reality, but it requires very fast extruder movements which may not be possible at every printer. At last it should fail on flexible filaments - think about how much compression they would need to get a propper extrusion pressure when you start from 0..
So a method like the original advance that distributes the esteps equaly through the acceleration phase could be used to have a not as good pressure regulation but a working one on slower extruders or softer filament types.
If you want to go this way, the next question would be if this fall back solution should be done with the math given in the original advance, which needs a seperate k factor for each layer height to width ratio. Beside this, fixing the feature is quite straight forward. There are only a few missing lines that seems to be got lost back in time.
Or do you want to keep the math equal for both step distribution methods, which means calculating k* extruderdv accoring to my calculations for the "fall back solution" inside planner.cpp and spread the extra steps equal through the amount of accel. steps.

@paulusjacobus
Copy link
Contributor

@Sebastianv650 I am very curious about the missing lines. When I go through the code I cannot find the actual maths being carried out. Just the block is filled with advance steps initial rate and advance rate. I am currently looking at the sailfish code which has a lot more code: lead-prime and de-prime routines. The porting is not that straight forward despite the Grbl planner being nearly identical. Sailfish does some multiplication/square root operations in assembler and the acceleration variable is already quadratic (a*a) and used as such in the other calculations to save on multiplication tasks. What is missing in the overall approach is that the advance priming and depriming of the hotend needs to be ahead of the xy motion planner. I think a separate ring buffer would be required with sync between the two buffers. In that way you can adjust the priming timing of the hotend and the xy motion. What do you think?

Sent from my iPad

On 5 May 2016, at 5:19 PM, Sebastianv650 [email protected] wrote:

@thinkyhead thanks for updating the PR and streamlining the code!
There is still a problem when the advance_isr is used which results in blobs on line starts. It's hard to hunt this down: It's not there in the RC4 implementation that also uses the 2nd isr as long as the FW retract speed is kept at about 35mm/min. At 55mm/min there is also blobbing. I guess it has something to do with the extruder can't handle multiple esteps at once. I will try to modify the advance_ISR to distribute the steps equaly on the time line given between two main isr calls..

@jbrazio @thinkyhead : There might still be a reason for keeping another advance method like the original one. My approach leads to better print results as it is closer to reality, but it requires very fast extruder movements which may not be possible at every printer. At last it should fail on flexible filaments - think about how much compression they would need to get a propper extrusion pressure when you start from 0..
So a method like the original advance that distributes the esteps equaly through the acceleration phase could be used to have a not as good pressure regulation but a working one on slower extruders or softer filament types.
If you want to go this way, the next question would be if this fall back solution should be done with the math given in the original advance, which needs a seperate k factor for each layer height to width ratio. Beside this, fixing the feature is quite straight forward. There are only a few missing lines that seems to be got lost back in time.
Or do you want to keep the math equal for both step distribution methods, which means calculating k* extruderdv accoring to my calculations for the "fall back solution" inside planner.cpp and spread the extra steps equal through the amount of accel. steps.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@Blue-Marlin
Copy link
Contributor

That means for me, some research or learning is lost.

Closed issues/PRs are not lost. You just have to include them in your search.

@Sebastianv650
Copy link
Contributor

Sebastianv650 commented May 5, 2016

@paulusjacobus lets see if I have all the bugs in my mind..:
First, advance_rate is calculated inside planner.cpp and written to the block, but it's never loaded inside stepper.cpp. Inside trapezoid_generator_reset, the line
advance_rate = current_block->advance_rate;
is missing.
Second, it's doing this:

e_steps[current_block->active_extruder] += ((advance >> 8) - old_advance);
old_advance = advance >> 8;

which is wrong because it's assuming there is always at last one step todo. It's like truncating a comma. It will run with higher k values, but do nothing with smaller ones. This code fixes that:

short steps_todo = (advance >> 8) - (old_advance >> 8); //usualy, this term is < 1 so we have to wait until a full step is needed. Original implementation tried to add 0.xx to an integer value which is always +0 (steps)
if (steps_todo) {
   e_steps[current_block->active_extruder] += steps_todo;
   old_advance = advance;
}

Simmilar code has to be used for deceleration.

Don't become confused by the sailfish code. They do a lot more than only "normal" advance. lead-prime and de-prime are disabled by default as far as I found out. This functions are an addon to the JKN advance, meaning it's releasing the pressure completely after the last print move and restoring it before the next one. You can think about that like a replacement for usual retracts.
I think this is not worth to struggle with. Even only with normale advance enabled, you can reduce your retraction length. Instead of releasing the complete pressure, with enabled advance you only have a very small amount of remaining pressure to release by retract. To be precise, it's the pressure from jerk speed / 2 because Marlin stops there.

The math of sailfish is another one that can burn your (at last my) brain. They use fancy fixed comma calculations to speed up the code, which makes it realy hard to read. But that's not important as we only have to understand how the thing works in general, we don't want to copy and paste.

No you don't want to prime the nozzle in advance. If you execute the advance esteps prior to the print move start, you will have a blob where nothing should be printed. Each extra step leads to an instant pressure change, you can't say I'm doing 5 steps now to have the pressure 10 ISR loops later. It's true that the sync of X,Y, Z and extruder is broken with advance. But not along time, only the speed / acceleration is not synchron anymore.

@jbrazio
Copy link
Contributor

jbrazio commented May 5, 2016

Ok maybe I'm only basing my decision on rumors, but word in the street is ADVANCE do not work and/or people do not know how to tune it.

But ok let's not kill it, your advices have been heard.

@Sebastianv650
Copy link
Contributor

@jbrazio I'm sorry but I don't get the meaning of what you want to say. I guess my english isn't good enough this time..

@jbrazio
Copy link
Contributor

jbrazio commented May 5, 2016

Or mine was a piece of poop.
:-D

Let me try again, I was insisting to remove the existing ADVANCE because you have now implemented a new method.

My opinion to remove the existing feature is based in the rumors I have been reading around that the existing feature does not work properly and/or people do not know how to tune it.

@paulusjacobus
Copy link
Contributor

paulusjacobus commented May 5, 2016

@Sebastianv650 that explanation makes fully sense now after I went through the code again in the last two hours. Indeed I can see the missing links and logical errors. However I think We should or at least I will definitely give it a try to improve it. Also had a look at smoothieboard which does a similar thing. Shouldn't the xy motion and extruder be in sync through their spot in the buffer when properly calculated and inserted (together in critical section)?

Code in Sailfish is indeed hard to read but once you get it, it very similar to Marlin's code. Except the code has a better description of the maths behind the planner and the advance algorithm. In Marlin, the planner description is very succinct.

Note: Smoothie board has a good description of the planner for those interested and want to get a basic understanding about what the Planner algorithm.

Sent from my iPad

On 5 May 2016, at 8:51 PM, Sebastianv650 [email protected] wrote:

@paulusjacobus lets see if I have all the bugs in my mind..:
First, advance_rate is calculated inside planner.cpp and written to the block, but it's never loaded inside stepper.cpp. Inside trapezoid_generator_reset, the line
advance_rate = current_block->advance_rate;
is missing.
Second, it's doing this:

e_steps[current_block->active_extruder] += ((advance >> 8) - old_advance);
old_advance = advance >> 8;
which is wrong because it's assuming there is always at last one step todo. It's like truncating a comma. It will run with higher k values, but do nothing with smaller ones. This code fixes that:

short steps_todo = (advance >> 8) - (old_advance >> 8); //usualy, this term is < 1 so we have to wait until a full step is needed. Original implementation tried to add 0.xx to an integer value which is always +0 (steps)
if (steps_todo) {
e_steps[current_block->active_extruder] += steps_todo;
old_advance = advance;
}
Simmilar code has to be used for deceleration.

Don't become confused by the sailfish code. They do a lot more than only "normal" advance. lead-prime and de-prime are disabled by default as far as I found out. This functions are an addon to the JKN advance, meaning it's releasing the pressure completely after the last print move and restoring it before the next one. You can think about that like a replacement for usual retracts.
I think this is not worth to struggle with. Even only with normale advance enabled, you can reduce your retraction length. Instead of releasing the complete pressure, with enabled advance you only have a very small amount of remaining pressure to release by retract. To be precise, it's the pressure from jerk speed / 2 because Marlin stops there.

The math of sailfish is another one that can burn your (at last my) brain. They use fancy fixed comma calculations to speed up the code, which makes it realy hard to read. But that's not important as we only have to understand how the think works in general, we don't want to copy and paste.

No you don't want to prime the nozzle in advance. If you execute the advance esteps prior to the print move start, you will have a blob where nothing should be printed. Each extra step leads to an instant pressure change, you can't say I'm doing 5 steps now to have the pressure 10 ISR loops later. It's true that the sync of X,Y, Z and extruder is broken with advance. But not along time, the speed / acceleration is not synchron anymore.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@Sebastianv650
Copy link
Contributor

Sebastianv650 commented May 5, 2016

@paulusjacobus do you already know my small presentation? If not you might have a look at it.
If you play with the original advance, there are two important things to keep in mind: Advance has to be dependent on extrusion speed. This is linked to the print speed (X,Y), but not constant if layer height is changing. Second thing I was struggling with: Don't think you can calculate the advance based on the start/end speed of a block and it's maximum speed. Marlin does speed jumps between blocks due to jerk, if you are not calculating with an absolute speed you will miss steps which leads to over/underextrusion.

Shouldn't the xy motion and extruder be in sync through their spot in the buffer when properly calculated and inserted (together in critical section)?

Can you explain this a bit more?

@jbrazio Got it! Regarding the calibration, this will be the hard part. I think it should be quite clear for someone who's understanding all the details that are happening in the printer, but a detailed step by step guide with pictures will be necessary for the normal user..
If someone could test the feature with a bowden system and ABS (which should be softer than my PLA) and it works, then I would also vote for removing the old advance completely. If the needed high k value leads to problems, a "softer" advance version would be good.
If someone want's to do that, read the next block - don't do the test with the RCBugFix code.

@ALL The blobs at line starts are solved :-) The extruder doesn't like more than one step at once. This changes to the advance isr fix that: Sebastianv650@0661d08
This was only a quick check change - I will redo it so it works with more than 1 extruder of course!

The commit is based on my RC4 version, I don't know how to append that to this PR. The changes will basicaly work in both versions.
This also fixes ~90% of the communication problems described in #3680 - but only in RC4. It's also better in RCBugFix, but still so worse that it did a strange XY movement to the edge of my print bed during a 1hour print.

@paulusjacobus
Copy link
Contributor

Shouldn't the xy motion and extruder be in sync through their spot in the
buffer when properly calculated and inserted (together in critical section)?

Can you explain this a bit more?

@Sebastianv650 Basically what I failed to express was:
If you add these advance steps up in the ISR of stepper.cpp, you assume the
ISR is called in equal time steps (=equal delta velocity). But that’s not
true, the ISR is called in equal distances because it has to take care of
the fact that the
stepper have to be stepped according to their steps/mm setting!

In other words: delta velocity is not equal in each ISR step!

Your presentation is an excellent way to express what is happening than a
Github issue thread. It is a linear line of thought with evidence and
conclusions, well done.

If you calibrate k for fixed printing settings both calculations will lead
to the same print result! Changes may appear as you change print speed or
layer height/line width ratio without adopting k.

This is caused by the friction and heat loss in the nozzle which is
different for layer heights. The solution for this could be using a lookup
table like we do for thermistors.

I seriously start to reconsider my thought about keeping the current
advance code. The stepper code is significantly changed since release
1.0.x, so trying to fix this in 1.1.x seems impossible if it is already
wrong in 1.0.x. A logical step would be fixing in 1.0.x and then move to
1.1.x but that seems a lot of investment, I have little appetite for that
pfff.
If you coding effort has reached almost maturity and is based on JNK rather
than Roberts Advance Algorithm then I tend to give your fork a try. We
might be able to derive that K table through a couple of test prints for
each layer height. I guess that approach is worth a try. I'm curious about
your thoughts? @thinkyhead and Roxy what do you think?

On 6 May 2016 at 00:11, Sebastianv650 [email protected] wrote:

@paulusjacobus https://github.com/paulusjacobus do you already know my
small presentation? If not you might habe a look at it.
https://drive.google.com/open?id=0B5UvosQgK3adaHVtdUI5OFR3VUU
If you play with the original advance, there are two important things to
keep in mind: Advance has to be dependent on extrusion speed. This is
linked to the print speed (X,Y), but not constant if layer height is
changing. Second thing I was struggling with: Don't think you can calculate
the advance based on the start/end speed of a block and it's maximum speed.
Marlin does speed jumps between blocks due to jerk, if you are not
calculating with an absolute speed you will miss steps which leads to
over/underextrusion.

Shouldn't the xy motion and extruder be in sync through their spot in the
buffer when properly calculated and inserted (together in critical section)?

Can you explain this a bit more?

@jbrazio https://github.com/jbrazio Got it! Regarding the calibration,
this will be the hard part. I think it should be quite clear for someone
who's understanding all the details that are happening in the printer, but
a detailed step by step guide with pictures will be necessary for the
normal user..
If someone could test the feature with a bowden system and ABS (which
should be softer than my PLA) and it works, then I would also vote for
removing the old advance completely. If the needed high k value leads to
problems, a "softer" advance version would be good.
If someone want's to do that, read the next block - don't do the test with
the RCBugFix code.

@ALL https://github.com/all The blobs at line starts are solved :-) The
extruder doesn't like more than one step at once. This changes to the
advance isr fix that: Sebastianv650/Marlin@0661d08
Sebastianv650@0661d08

The commit is based on my RC4 version, I don't know how to append that to
this PR. The changes will basicaly work in both versions.
This also fixes ~90% of the communication problems described in #3680
#3680 - but only in RC4.
It's also better in RCBugFix, but still so worse that it did a strange XY
movement to the edge of my print bed during a 1hour print.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3676 (comment)

@thinkyhead
Copy link
Member Author

Once again, I am pressed for time. I will read through tonight and comment tomorrow!

@paulusjacobus
Copy link
Contributor

No problem @thinkyhead things will take some time to discuss and absorb, we
waited long enough for JNK so another day won't hurt (your support is not a
paid job). @Sebastianv650 has made some good progress so we might be well
on track of Marlin's development :)

On 6 May 2016 at 13:52, Scott Lahteine [email protected] wrote:

Once again, I am pressed for time. I will read through tonight and comment
tomorrow!


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3676 (comment)

@Sebastianv650
Copy link
Contributor

@paulusjacobus

to the same print result! Changes may appear as you change print speed or layer height/line width ratio without adopting k. This is caused by the friction and heat loss in the nozzle which is different for layer heights. The solution for this could be using a lookup table like we do for thermistors.

The issue with k not beeing constant for different layer height is only true for the original marlin advance and the sailfish JKN implementation. This is due to the fact described in slide 4: All fomulas are based on extrusion speed. But for whatever reason, both implementations are calculating with the speed of the master axis, which most likely is never the e axis speed.
I'm working around this by calculating e_speed_multiplier8 in planner.cpp, which gives you the relation between master axis speed and e axis speed. This way the k factor is realy a fixed value for one extruder, that should only change with different filament materials (PLA, ABS, PETG, ..).
If someone want's to be realy scientific: If we would know the spring constant for each kind of filament (k) and you know your free length of filament between the hobbed bolt and the hot end (L), the advance_k factor should be: advance_k=f(k,L)

@paulusjacobus
Copy link
Contributor

Thanks @Sebastianv650 is your Fork working with a print speed of 40mm/s? I
would really like to try it out for myself. Since you are printing a lot it
must be stable enough?

On 6 May 2016 at 16:53, Sebastianv650 [email protected] wrote:

@paulusjacobus https://github.com/paulusjacobus

to the same print result! Changes may appear as you change print speed or
layer height/line width ratio without adopting k. This is caused by the
friction and heat loss in the nozzle which is different for layer heights.
The solution for this could be using a lookup table like we do for
thermistors.

The issue with k not beeing constant for different layer height is only
true for the original marlin advance and the sailfish JKN implementation.
This is due to the fact described in slide 4: All fomulas are based on
extrusion speed. But for whatever reason, both implementations are
calculating with the speed of the master axis, which most likely is never
the e axis speed.
I'm working around this by calculating e_speed_multiplier8 in planner.cpp,
which gives you the relation between master axis speed and e axis speed.
This way the k factor is realy a fixed value for one extruder, that should
only change with different filament materials (PLA, ABS, PETG, ..).
If someone want's to be realy scientific: If we would know the spring
constant for each kind of filament (k) and you know your free length of
filament between the hobbed bolt and the hot end (L), the advance_k factor
should be: advance_k=f(k,L)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3676 (comment)

@Sebastianv650
Copy link
Contributor

The RC4 thing is working well for me: https://github.com/Sebastianv650/MarlinRC_LIN_ADVANCE
I would recommend to start with this one, as long as the communication isn't working save with this for BugFix.

@jbrazio
Copy link
Contributor

jbrazio commented May 6, 2016

@paulusjacobus You said it well, if broken should be fixed for next release. But if the feature is broken we should at least SanityCheck it with and #error in case the user activates it so we can ship the GA.

@paulusjacobus
Copy link
Contributor

@Sebastianv650 I am trying your RC4 version and will post my findings here.

@jbrazio yes agreed, investigation from @Sebastianv650 shows it cannot
logically work since the buffer is never read in the stepper.

On 6 May 2016 at 18:26, João Brázio [email protected] wrote:

@paulusjacobus https://github.com/paulusjacobus You said it well, if
broken should be fixed for next release. But if the feature is broken we
should at least SanityCheck it with and #error in case the user activates
it so we can ship the GA.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3676 (comment)

@paulusjacobus
Copy link
Contributor

photo5
photo2 cube without JNK enabled
photo1 cube with JNK enabled

@Sebastianv650 the tests are very promising so far. I found a K=125 for my direct drive extruder with a printing speed of 60mm/s. Since RC6 I print with 40mm/s so it seems that RC4 is smoother and allows higher printing speeds? Also less communication errors (just 1 vs. 5 error line numbers). Wonder if anyone else noticed a degradation of speed? I blamed it on my configuration plus wear and tear.

I also saw that with a K=150, the ghosting disappears further at the start of the trapezoid but the extrusion at the end of the trapezoid leaves an open corner. Wonder if we can break the K into two parts: K1 for the start and K2 for the end of the block? Last observation is that the corners of the cube have rounder profile/finish which I actually don't mind. Well done!

@Sebastianv650
Copy link
Contributor

So you tested both the RC6 and RC4 with advance?

Personaly I would say your k value is too high, the corners should get rounded. But of course in the end it's up to you what looks best in your opinion!
My calibration way is to print flat cubes (20x20x3mm) with a low acceleration of 500 and high perimeter and infill speed, say 60mm/s. Start with k=0. Then take a metal ruler and hold it along one side of the square, look against a light source (a window, not a 400W halogen lamp :-p) with it. You will see the edges are the highest point. Then increase k in 10 or 20 unit steps per print and compare. The best value is found when the edges and the middle part of the cube side where the nozzle has crusing speed is in one line. If k is increased further, the edges start to get rounded and are pulled inward.
Also compare the top surface quality, where the infill lines changes direction on the perimeters. It should be somewhat rough with k=0 and much more flat with a propper k value.

You can also use the JKN documentation how to find the best value, in the end it's the same as my description above:
http://www.sailfishfirmware.com/doc/tuning-jkn-advance.html

The open corner you are seeing is another proof that k is too high. No, it's basicaly not possible to make a different k for accel. and deceleration. Think about it with this example:
Say you need 50 esteps for a line, start and end speed =0. With advance enabled, you will start extrusion with a higher speed during acceleration, maybe you and up with 98% of your esteps done when you are at the point of deceleration. At the end of the line, even with adance enabled, you need 50 esteps done - so you have to subtract exactly the amount of advance esteps you added during acceleration. The material will not vaporise, if you don't subtract the same amount you get underextrusion or over extrusion depending if you deceleration k is higher or lower than acceleration k.
I tried this by mistake in my first code version because I wasn't thinking about the jerk between two line ends ;-) Ended in blobs and voids..

I guess your next question will be that JKN does something (but the opposite) from what you asked: K2. K2 is subtracting more steps on deceleration that where never added before or after that. A negativ K2 value would lead to what you asked. I don't like K2 because it makes steps to disapear and it's also not necessary. I guess if they would use the absolute e speed for their calculations as I did in Marlin, they would recognise K2 is no longer needed..

@thinkyhead
Copy link
Member Author

thinkyhead commented May 6, 2016

Sounds like things are working out. I will take the latest changes and integrate them ASAP.

@paulusjacobus
Am I reading it correctly that you saw some degradation in "performance" between our release versions RC4 and RC6? If so, then in what way? (Or do you just mean in terms of the JKN?)

@thinkyhead
Copy link
Member Author

@Sebastianv650 I was just making the commit with your new changes, which look pretty good. One thing I noticed is that the variable interrupt rate has an interesting behavior as maxesteps goes towards zero. At 4 steps and up it runs at fastest speed, then when it gets to 3, 2, and 1, those last three interrupts are gradually slower, so they are not precisely "in sync" with the XY movement. (They might be off by about 1/14th of a second.)

Perhaps it would be better to get maxesteps before starting the move, and set a nominal timer rate when the move starts, instead of choosing it within the ISR itself. This would also save many CPU cycles.

I also noticed that, regarding the counter for OCR0A, we'd need to follow a "Bresenham" type scheme to get "absolutely precise" timing. However, on review I see this isn't a problem. The only counter that isn't an even divisor of 52 (necessary to keep pace with the stepper ISR) is 17, and that only happens for one e-step at the end of a move, which is hardly going to throw the timing off.

@thinkyhead thinkyhead force-pushed the rc_lin_advance_feature branch 3 times, most recently from a006df6 to fa7a232 Compare May 6, 2016 18:35
@thinkyhead
Copy link
Member Author

thinkyhead commented May 6, 2016

I started to implement the concept. But in the process, I realized why you can't set the timer rate in advance. At the same time, I can see that the divider of 17 could compound to create a larger timing issue, because e_steps are always being incremented by the main Stepper::isr.

52 ÷ 17 is just a tiny bit larger than 3 (3.08 is 102.67% of 3). Meanwhile, 52 ÷ 18 = 2.88, 96% of 3. Some Bresenham-like method could smooth these out.

@jbrazio jbrazio removed the Needs: Testing Testing is needed for this change label Jun 16, 2016
@paulusjacobus
Copy link
Contributor

@Sebastianv650 I am more then happy to review it. Just let me know when it is ready for review. Maybe an explanation of the implementation as a separate reference would be interesting for the more experience users.

Sent from my iPad

On 17 Jun 2016, at 1:42 AM, Sebastianv650 [email protected] wrote:

Yes I think that's necessary. I think the documentation should contain:

What's the goal of this feature (reduced or no blobbing in corners, ..)
How to calibrate k (printing flat test cubes procedure)
For the interested ones: A short description what is actualy done by this feature (filament acting as a spring)
further ideas?
Maybe it could be part of the wiki inside this repository, including images if this is possible?
I can provide text and images in a propper format, but I think I need somebody afterwards who reads through it for two main reasons: Is it understandable for a somebody who doesn't know about pressure advance before? And maybe it should be "translated" from medium quality german english into real english ^^

I think I can start writing this weekend..


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@Sebastianv650
Copy link
Contributor

What do you think about that:

Purpose of the LIN_ADVANCE feature

Let's take the common test cube as an example: You may have noticed that the corners are not sharp, they are bleeding out. Also the top solid infill has some roughness where it comes to print direction changes on the perimeters. These problems are minor at very low print speeds and become much more noticeable when you try to print fast.

The root cause of this print quality issues is an improper pressure inside the nozzle. In a nutshell, LIN_ADVANCE enables a pressure control feature which sets the pressure inside the nozzle to the needed one according to the print speed. This way, bleeding edges and rough solid infill can be nearly eleminated.

Advantages with enabled LIN_ADVANCE:

  • Higher dimensional precission due to reduced bleeding edges.
  • Higher print speeds are possible without loosing print quality.
  • With same print speed, visual and feelable print quality will be increased.
  • High acceleration and jerk values are not longer needed to get sharp edges.

Important notes before you use the LIN_ADVANCE feature

Please consider the following points before you enable pressure control or you may end up with strange and hard to trace back issues:

  • Your printer has to produce good prints before. For example, your extruder steps/mm value has to be calibrated precisely - do this at a low print speed like 20mm/s to keep nozzle pressure effects low in this step! Don't enable a new feature if you have unresoved issues!
  • Check the play in your extruder drive gears (if you have any). Excessive play will lead to strange noises during the extra extruder movements!
  • Some slicers have options to control the nozzle pressure in some ways. Common names are: Pressure advance, Coast at end, extra restart length after retract. Disable them all! The firmware will do a much better job than the best slicer can do, and they would influence themselfes in a bad way. I also strongly recommend to disable further features like wipe while retract. You can try to enable it again after you have calibrated your K factor (see next step), but I guess you don't need it anymore.
  • After you have a calibrated, working pressure advance feature, you may want to recheck your retraction distance. You will find out that you can cut it down to nearly 0. This is caused by the pressure control, it reduces the nozzle pressure at the and of a line to about zero. For example, I was using 1mm retract length for PLA, with enabled LIN_ADVANCE I'm now using 0.5mm.
  • Keep in mind that enabling this feature will add extra load to the processor and also your extruder hardware. I recommend to stay at 115200baud communcation speed, especialy if you get error messages in your print host software or you get wired movements of your print head. Ensure your print host software is using line numbers and checksums, this is disabled by default in Simplify3D.
    On the extruder side, the needed extra movements may lead to an increased wear on the printed gears of a gregs wade extruder.
  • Most testing of this new feature was done on direct drive extruder printers. If you have problems with bowden printers due to the needed high K values, please report them on Marlin GitHub and/or try to reduce your acceleration value.
  • Using this feature with very flexible filaments like Ninjaflex is untested and will most likely not work due to the extremly high needed K values.

How to calibrate the K factor

For a gregs wade extruder you can use K=75 as a start point for PLA and K=150 for nGen. K is a unique value for an extruder with one filament material. If you are using different filament materials like PLA and ABS, you will have one K value per material. The value will be higher for more flexible filaments.

Use a test cube with the dimensions 25x25x2mm for testing. Set the acceleration of your printer to a low value like 500mm/s² to eleminate other effects like overshooting fluid filament on direction changes. Set all print speeds for the cube to the same high value, 70mm/s for 0.2mm layer height for example - but be sure to not exceed the flow rate your hotend can handle! Ensure your slicers cooling settings will not slow down the print speed. If it would do this, disable the cooling feature or increase the length and width of the test cube.

Print some test cubes, each with another K value. You can set the value with "M905 K.." before you start the print, where .. is the value you want it to be. Start with K0 to disable the pressure control, which will give you a reference print how your cube would look like without the feature enabled.
Then increase K maybe in 25 units steps. On every print note down the used K value.

The best value is the one where you have no bleeding edges (bleeding edges = K too low) and no inward rounded edges (K too high). Check by holding a metal ruler on each of the 4 sides of the test prints. You will also see and even more feel the difference in the quality of the top infill, where it changes print direction at the perimeters. But top infill alone is no sufficient calibration criterion as it is also influenced by the infill overlap % that's between 15-20% in most slicers.

When you have found a good value, there are two ways to make them permanent in your firmware. If you are using only one filament material, the best way is to set the K value inside Configuration_adv.h and reflash the firmware. If you are using different materials, you might want to insert a "M905 K.." line into the start code inside your slicer profile used for the corresponding material.

Details about the feature for interested ones and programmers

The force needed to move the filament through the nozzle depends on the speed you are trying to push it through. If you push it fast (=printing fast), the filament will be compressed in a first step before the pressure inside the nozzle is high enough to extruder the required amount of material.
If you take a single, fast printed line, this results in underextrusion at the line start point (the filament gets compressed, but needed pressure isn't reached) and overextrusion with a blob at the lines end (the filament is still compressed when the nozzle comes to the stop which results in oozing).
For a complete print, this leads to bleeding edges at corners (corners are stop/end points of lines) and in extreme cases even gaps between perimeters due to underextrusion at their start points.

The LIN_ADVANCE pressure control handles this free filament length as a spring, where K is a spring constant. When the nozzle starts to print a line, it takes the extrusion speed as a reference. Additional to the needed extrusion length for a line segment, which is defined by the slicer, it calculates the needed extra compression of the filament to reach the needed nozzle pressure so that the extrusion length defined by the slicer is realy extruded. This is done in every loop of the stepper ISR.
During deceleration, the filament compression is released again by the same formula: advance_steps = delta_extrusion_speed * K. During deceleration, delta_extrusion_speed is negative therefore the advance_steps values is negative which leads to a retract movement relaxing the filament again.

The basic formula (advance_steps = delta_extrusion_speed * K) is the same as in the famous JKN pressure control, but with one important difference: JKN is calculating the sum of all needed advance extruder steps inside the planner loop and distrubutes them equaly over every acceleration and deceleration stepper ISR loop. This leads to a wrong distribution of the advance steps, resulting in a not perfect print result. LIN_ADVANCE is calculating the needed extra steps on the fly in every stepper ISR loop, therefore executing the needed steps precisely where needed.
For further details and graphs have a look into this presentation, slides 7-9.

Inside the Marlin code, stepper and planner files are the ones where the work is done. Inside the planner loop, LIN_ADVANCE checks if a move needs pressure control. This is true for a print move, but false for travel moves and extruder only moves like retract and prime events.
Inside the stepper file, LIN_ADVANCE calculates the amount of needed extra steps to reach the needed nozzle pressure inside the stepper ISR. To avoid missing steps, it's not executing the steps at once. Instead, I reused and modified the extruder stepper ISR implemented with the old Marlin ADVANCE feature. The normal extruder steps are executed together with the extra steps due to LIN_ADVANCE in the given time frame before the next stepper ISR loop will fire.

At the moment, there is no check if the extruder acceleration, speed or jerk will exceed the limits set inside Configuration.h. To achieve such checks, the planner needs to be completely rewritten because a variable acceleration rate is needed to compensate too fast pressure advance movments. I'm quite sure the power of the used ATMega isn't sufficient to do this.. But up to now, I don't know about problems according to this in real world printing.

@thinkyhead
Copy link
Member Author

That's a good overview of advance extrusion. I would only do a little tweaking to make the language more flowy. It may help to use some images to illustrate the concepts.

@jbrazio
Copy link
Contributor

jbrazio commented Jun 29, 2016

@Sebastianv650 have a look here: http://www.marlinfw.org/articles/features/lin_advance.html

@Sebastianv650
Copy link
Contributor

Sebastianv650 commented Jun 30, 2016

Looks good! I will make pictures of a test cube with K= to low and K= to high for this article, and maybe I should also create a single simple graph that shows the advance step distribution during an acceleration phase compared to the JKN distribution, which will also clarify the problem for real world implementation into the FW for future developers.

But my first priority is optimization at the moment. I'm absolutely not satisfied with the execution speed impact LIN_ADVANCE does to Marlin. A normal stepper acceleration loop takes 50µs, 80 with enabled advance. I'm working on an idea that will reduce the time to 63µs, but I need to do some more tweaks to make it safe even with high K values..

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 1, 2016

A normal stepper acceleration loop takes 50µs

And how often does it need to run? It would be good to know what our margins are. Let's see…

If it takes 160 steps-per-mm for a typical X axis, and you want to get a high speed of 100mm/s, then you need to be able to reach 16,000 steps per second. If we allow up to 4 steps per-stepper in each call of the ISR then the ISR needs to run at least 4000 times per second, so it has to stay under 1/4000 seconds (250µs) execution time. At 50µs to execute, only about 1/5 of the available time is being used up by the ISR.

That sounds like a wide margin. But I'm sure I'm forgetting some extra complexities here.

@Sebastianv650
Copy link
Contributor

Sebastianv650 commented Jul 1, 2016

At 16kHz step frequency, Marlin will switch to double steps which results in 8kHz. So the stepper ISR on its own uses 8000 * 0.00005s = 0.4s per second = 40% of the time. Which isn't the whole truth, because it's now using a bit more time per loop to fire at last 1 stepper twice.

With a stepper loop length of 80µs with advance, 64% of the time goes to the stepper.

With a needed calculation time of about 2ms for a new line segment (planner) + all the ISR interruptions, this makes a nice diference in points per seconds that can be executed - and we have not even taken into account the time needed for serial communication here..

@thinkyhead thinkyhead mentioned this pull request Jul 8, 2016
@jbrazio jbrazio modified the milestone: 1.1.0 Jul 18, 2016
@lonelymyp
Copy link

I have a problem on round surfaces. especially at the beginning of the perimeter, after retract.
img_20160808_201057

part of the problem is solved if I put a very small acceleration, 500 or so. on accelerating the 1000 or 1500 the problems begin on round surfaces.

but if the item does not contain a large round surfaces, everything is OK.
the lower part is printed with high acceleration and speed.

coreXY
Marlin 1.1.0-RC7
K = 150
80 mm/c
accel 1500 mm/c2
retract 4mm, bowden.
115200 uart

@Sebastianv650
Copy link
Contributor

The problem might come from a combination of high K factor together with a more or less high jerk setting. If the needed prime steps have to be executed faster than the e stepper can handle, steps will be missing.
I tested 3 different aproaches of executing (distributing) the prime steps since my last update, none of them is working flawlessly in all situations / possible parameter combinations. So I'm not satisfied up to now with the implementation but I give up a little bit the last weeks.
In a nutshell, it should work well with direct drive extruders and quite stiff materials like PLA, but it might fail with ABS/nGen and others. The problems is jerk, if we have infitine speed changes in a point the needed pressure adjustment has no real time to take place.

I will try to find some new ideas after my holiday, if everything fails I will offer a JKN like approach with a linear advance step distribution over acceleration and deceleration phase, but this will lead to a clearly visible lower quality then with mathematicaly correct distribution. But of course, better than nothing.

@Blue-Marlin
Copy link
Contributor

@lonelymyp
If you are using FWRETRACT this could be fixed by "Correct typo in retract() #4503" in RCBugFix.

@Kaibob2
Copy link
Contributor

Kaibob2 commented Aug 11, 2016

@Sebastianv650: I tried to find an answer myself, but only found direct extruder K-values suggestions.
I want to test the lin_advance feature, and only need a hint what would be a realistic k-value in a setup which has two 45cm Bowdens.
I'm using a dual extruder setup, one for PLA at 210°C, the other for ABS at 255°C. Both nozzles are 0,4mm.

@Sebastianv650
Copy link
Contributor

You will have to find it out yourself - up to now I have no figures for bowden setup. It might even fail, for example if you need a K of 200 but your extruder can only keep up up to a K value of 100.
In the end, it will be a compromise because up to now there is only one possible K value at a time, so if you use PLA and ABS in one print you have to decide.

PS I will be on holiday the next two weeks, I will answer further questions when I'm back again.

@thinkyhead
Copy link
Member Author

I will be on holiday

Someday I hope we in the USA will have such a custom!

@mosh1
Copy link
Contributor

mosh1 commented Aug 13, 2016

I love this feature, it's amazing! However, I recently switched to 1/16 microstep on X & Y where before I was 1/8 on Y and 1/16 on X. This put enough load on the CPU with LIN_ADVANCE that it seems if I do 120mm/s print moves the extruder gets stalled out. This is with the E3D BigBox and E3D Titan extruder. Does the math add up that I will hit the stall rate at this print speed - and any suggestions for freeing up CPU for me to test? I am using the latest RCBugFix.

@Kaibob2
Copy link
Contributor

Kaibob2 commented Aug 13, 2016

I did several test prints and I think the 45cm Bowden is really a problem. The Lin_advance effect starts to appear from K=100 upwards. X, Y and E axis are all at 1/16. Acceleration at 1000 mm/s, inner perimeter at 70mm/s outer perimeter at 21mm/s. I guess a K of around 200 would be fine for me, but the extruder has problems to keep up. It works, but the print looks better with S3Ds coasting and additional restart distance enabled. In this case I have a uniformer outline, but small infills stay hollow because the outline perimeter nozzle pressure is far lower than it will have to be for the infill.

@brainscan
Copy link

brainscan commented Aug 13, 2016

I run this with a Bowden that's 600mm using nGen with K165 and prints very well but I had to follow the instructions and reduce my acceleration to 500 for print moves and 1500 for travel. Also I get better prints keeping my print speeds in a smaller range 30mm/s outer, 45mm/s inner and 55mm/s infill. I use 80% speed for first layer, solid or top fill.

@Kaibob2
Copy link
Contributor

Kaibob2 commented Aug 15, 2016

I lowered the acceleration for printing to 500 and the travel to 1000. Printing speed 50mm/s inner perimeter and infill, 25mm/s outer perimeter. 0,2mm layer height, ABS@260°C. 3 Perimeters, 1mm retract. K=135
I did all tests printing a gecko from thingiverse, this object has a lot of speed changes and travel moves.
The result was quite good, not really much better than without lin_advance, but okay.
I then printed another object which i had printed many times before to calibrate other stuff, so that i knew what to expect. The result was disappointing, see pictures. Left object printed with the settings from above, right object printed with K=0, Coasting 0,8mm, Extra restart dist. 0,05mm, Retract 5mm
20160815_084529_bildgrosse andern
20160815_084548_bildgrosse andern
20160815_084552_bildgrosse andern
20160815_084557_bildgrosse andern

The missing head resulted from K=200 :)

I think i stay with coasting, etc. first...

CONSULitAS pushed a commit to CONSULitAS/Marlin-K8200 that referenced this pull request Aug 18, 2016
…N_ADVANCE)

・Update forgotten example configurations
@thinkyhead
Copy link
Member Author

thinkyhead commented Aug 19, 2016

It's best to test on a cube, because the main fix Advance provides is on sharp corners. Rounded objects will be about the same as always.

@lonelymyp
Copy link

you often do cubes?
models do not always look like a cube.
on the cube algorithm works fine, but not on any other model.

@Kaibob2
Copy link
Contributor

Kaibob2 commented Aug 19, 2016

This was exactly my thought :) I did the initial calibration of Advance with a 40x40x40mm cube. The Advance effect was clearly visible, especially when switching from (slow) outer perimeter to (fast) infill. The absolute beginning of the infill grid line was exactly as wide as the end of the infill line. Without Advance the beginning is quite flimsy. So, the thing itself is working and, like @thinkyhead mentioned, great for cubes. When i was happy with the cube calibration settings i went to print my calibration gecko. I slightly modified some parameters like K and Retract and it came out quite nice, but, like i said, not really better than without advance. The Moai sculpture was printed to test if the "gecko settings" apply to any other object, but sadly they don't.

@mosh1
Copy link
Contributor

mosh1 commented Aug 19, 2016

@Kaibob2 did you print the one w/Advance with coasting & wipe turned off? Also try turning retract off as well..

@Kaibob2
Copy link
Contributor

Kaibob2 commented Aug 19, 2016

The one with advance was printed without coasting and wipe but with 1mm retract. The 1mm retract was necessary to avoid massive stringing when printing the gecko. I selected optimize start points and only retract when crossing open space in S3D, so there where almost no retracts on the Maoi.

@mosh1
Copy link
Contributor

mosh1 commented Aug 20, 2016

I am getting much better results, but am not running Bowden so my K is only set to 50.

@thinkyhead
Copy link
Member Author

thinkyhead commented Aug 22, 2016

you often do cubes?

Yes, @lonelymyp, when I'm trying to tune things, I most often use a small cube.

Then I move on to other things and continue to tune. But, yes, a cube is a good start when it comes to advance extrusion, retraction, and other factors, and that was the point of my suggestion.

@Sebastianv650
Copy link
Contributor

@Kaibob2 I know the defects visible in your pictures, I had it a lot with nGen using higher K values. The problem isn't that lin_advance is only true for cubes, but that a cuved surface results in a lot of speed changes within fractions of a mm (depending on your stl resolution) due to jerk setting and acceleration. What you see here is the extruder stepper raising the white flag - it's missing steps during the try to adjust the nozzle pressure.

I'm afraid with my limited programming skills, I can't provide a much better solution that is able to run on the 16MHz CPU. The big challenge is to find a way how to adjust the pressure with real time calculations and stay within the acceleration / jerk limits of the extruder stepper. At the moment, the X,Y speed isn't changed with lin_advance so the estepper may be overloaded. But I see no way how to keep track of the jerk / acceleration due to pressure adjustment and limit X,Y speed if it's too high. At the point we are calculating the pressure adjustment steps, the X,Y speed path is completed by the planner and can't be changed easily..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.