repetier / Repetier-Firmware

Firmware for Arduino based RepRap 3D printer.
815 stars 734 forks source link

Problems with Advance and round objects #837

Closed MacFlieger closed 5 years ago

MacFlieger commented 5 years ago

I have problems with the use of Advance_L and round objekts, perhaps of the small segments.

I am using a CoreXY (Sparkcube) with Repetier Firmware 1.0.3, a direct drive extruder and an e3dv6-Hotend.

At the beginning of a movement there is to few material extruded and at the end to much. Also resulting in not sharp edges. I think this is caused of nut enough pressure in the hotted at the start od a movement and to much pressure at the end. The advance-algorithm ist exactly for this problem. If I activate advance_l, than the results on simple straight object is very good. The edges are sharp and the extrusion is constant even at the beginning and end. But with the same parameters round objects look strange, because of a strange pattern on the object.

The following towers are printed without retract and with a varying Advance_L. 0 at the bottom and increasing by 10 every 10mm up to 90 at the top. They have a diameter of 20mm and are printed with 60mm/s. From left to right they are a square, an octagon, a cylinder with 60 segments and a cylinder with 360 segments.

img_1292

The sides of the square and octagon are flat (sharper edges to the top), but the two cylinders show a pattern, which is increasing with increasing Advance_L.

I have already tried a lot (including slower printing), but the pattern is always there with Advance_L. With Advance_L=0 the patterns doesn't exist.

Effective I can use Advance_L very good with object, that have mostly straight lines, but with round objects the pattern is very disturbing.

I don't know, what to do.

Even I don't understand, why Advance_L causes any problems on round objects. In my opinion Advance_L should only make a difference at points, where the speed changes. But on a cylinder is a constant speed besides the start and end of the circle (the seam). Why is Advance_L doing anything on the circle movement?

Edit: All objects are printed from SD-Card.

repetier commented 5 years ago

In deed a very strange pattern you found. What is your jerk setting? If jerk is too low you will have accelerations between the segments which can cause advance having an influence. Only if the speed stays constant advance will have no effect.

There is debug command for advance M232 it will show the biggest advance since last call and then reset distance for next call. Not sure if it would deliver anything useful. Would increase with every 10mm at the spot z was raised.

Advance steps at max speed are computed like this:

        float advlin = fabs(speedE) * Extruder::current->advanceL * 0.001 * Printer::axisStepsPerMM[E_AXIS];
        advanceL = (uint16_t)((65536L * advlin) / vMax); //advanceLscaled = (65536*vE*k2)/vMax

I wonder if it is still present in V2 where advance should work a bit better. But I agree that with good jerk the cylinders should show no such pattern. Only way I see is that the computed advanceL varies between segments. This can happen a bit with rounding errors from extrsuion vs. xy distance as both have limited precision, but would not expect such a visible impact.

MacFlieger commented 5 years ago

At the moment i use the lowest possible MAX_JERK 10.0, but even with 20.0 it was no difference.

Yes I have thought about JERK too, but as I understand JERK ist calculated by sqrt(deltavx^2+deltavy^2) deltavx= Delta of velocity in x-axis deltavy= Delta of velocity in y-axis

With the cylinder with 60 segments (6°) and 60mm/s: JERK=6.3 With the cylinder with 360 segments (1°) and 60mm/s: JERK=1.05 Both are lower than MAX_JERK 10 or 20.

Yes, I don't think it is caused by rounding errors, too. The pattern is much easier and clearer to see in reality as on the photo. It is very difficult to make a good photo of it. The pattern is builded from small areas, where the extruded material is much to small.

I know, that v2 has a better advance integrated. I have already tried to compile v2, but it has failed. But I have only tried it for an hour. Perhaps I have misconfigured something. Is v2 at the point, that I can use it with an CoreXY? Then I will try it again.

I will try the M232. Put a M232 just before every change of advance_l?

I have another question: Does Advance works good with CoreXY? Perhaps it is a problem only with them.

repetier commented 5 years ago

V2 should work fine with corexy but I have none for testing. I have updated docs see readme for the link to docs. But it is still in heavy development so only due based boards will work at the moment.

There is no reason advance should not work on corexy as it only depends on extrusion speed, so drive system is not relevant.

I would put M232 before and after changes of advance so it resets and you get a new value for each 10mm segment.

Nut sure with jerk as it changed between versions but 10 should suffice for circles with that many segments regardless of the function.

Can you post the gcode of the 2 cylinders. Then I can test it with V2. Guess I only need to adjust temperatures.

MacFlieger commented 5 years ago

OK, so I realize why I can't compile v2. I have an Arduino Mega 2560 and no due.

I have thought about M232 and one before and after every M233 is not sufficient. Because at the start and end of every circle is a stop and therefore there is much advance. I have know put three M232 in the GCode before every M233, one at the left corner and two at the right corner of the cylinder. So the first M232 resets the value, the seconds show advance for the rear half with the layer change and the third one shows advance for the front half. I will post it, when ready.

Here are the Codes: Zyl.zip Zyl1.gcode has 60 segments. Zyl2.gcode has 360 segments

MacFlieger commented 5 years ago

I have thought about your comment with the rounding errors.

I have copied the G1 commands of one circle into a calculation table, then calculated the length of every X-Y-move and the length of the extruder-move. With the given speed of 60mm/s for XY I than calculated the speed of Extruder. It is not constant! Than I calculated with the above formula advlin for advance_L=80. advlin varies between 16774 and 16801 and looks like this probably would reproduce the pattern. advlin varies about a range of 27. How can I calculate how much this will be in mm of Extruder?

bildschirmfoto 2018-12-23 um 16 26 57

Also the second cylinder doesn't have 360 segments like constructed in OpenSCAD. It has only 103 segments.

repetier commented 5 years ago

Ok, printed Zyl1 with V2 firmware. Really interesting to see the seam change over height. At bottom it is thick getting nearly perfect in middle and open at the top as it starts with not enough filament.

There are no visible patterns over height as in your sample, so at least V2 is doing what it is supposed to do with advance. Will test with V1 the next days when I need to switch to V1 for my printer to see if I can then reproduce the problem.

repetier commented 5 years ago

You converted it to steps so divide by steps per mm for extruder again and you have the extrusion difference. I guess you have a high e resolution with these values?

My test printer has only 147 steps per mm for extrusion.

repetier commented 5 years ago

Maybe your steps per mm for extruder is too high causing the problem. The stepper interrupt for extruder has a limited rate and is decoupled from xyz timer interrupt. Diameter 20mm takes 1,05s at 60mm/s. But I do not think extruder interrupt is faster then 15000hz so it will take more then 1s to add the 16000 steps.

Sound a bit too high for me, so maybe there is a math error in your sheet. What does M232 say about maximum extra steps?

MacFlieger commented 5 years ago

Yes, the seam varies a lot like the sharpness of edges. This is one of the two reasons, why I want to use Advance and optimize Advance_L with this towers.

I have 100 steps per mm extrusion. Perhaps I have miscalculated advlin. Is speedE the actual speed of the extruder in mm/s? Yes I think there is a math error in scaling. Edit: Got the error. The factor 0.001 was missing. bildschirmfoto 2018-12-23 um 16 56 04 So it varies around 0.027 steps. I don't think, that this would have a noticeable effect.

But if your v2 doesn't show the problem, the reason should not be the GCode.

repetier commented 5 years ago

Yes speedE is e speed. So if xy moves 60mm/s it will be maybe 3-5mm/s. Didn't check the ratio. 16 Steps is also something I can believe and should be no problem for the interrupt then.

MacFlieger commented 5 years ago

This are the results from M232: linear L:0.00 back: linear steps:0, speed=0.00 front: linear steps:0, speed=0.00

linear L:10.00 back: linear steps:3, speed=3.09 front: linear steps:2, speed=2.35

linear L:20.00 back: linear steps:5, speed=2.51 front: linear steps:4, speed=2.36

linear L:30.00 back: linear steps:7, speed=2.35 front: linear steps:7, speed=2.35

linear L:40.00 back: linear steps:10, speed=2.51 front: linear steps:9, speed=2.36

linear L:50.00 back: linear steps:11, speed=2.35 front: linear steps:11, speed=2.35

linear L:60.00 back: linear steps:15, speed=2.51 front: linear steps:14, speed=2.36

linear L:70.00 back: linear steps:16, speed=2.35 front: linear steps:16, speed=2.35

linear L:80.00 back: linear steps:18, speed=2.35 front: linear steps:18, speed=2.35

linear L:90.00 back: linear steps:22, speed=2.51 front: linear steps:21, speed=2.36

I have read about the different timers for xyz and e in v1. Is it possible, that they interfere, so that the execution of one interrupt disturbs the other one?

repetier commented 5 years ago

Ok, so speeds match what you expected from last computation. So far it looks good. Interrupts always interfere but that is no problem as they are short and you loose no steps. The splitting makes timing not perfect, which is why V2 should be better. Through the new system we do not need to split and get a better averaging of step distribution. None the less I do not expect it to be that visible, but everything you test is as it should be. SO maybe I add tomorrow the test with V1 if I get the time. I could be busy for some reason:-) But you made me curious and that way we can exclude that the printer has an effect on it.

MacFlieger commented 5 years ago

Yes, I will be very interesting in tests from other people. But don't bother. I really understand, if you or anybody else has more important things to do the next days. :) I don't know, if the effect will show on a due and has wondered, why there is so few written about advance. In my opinion this is a important point for quality.

Merry Christmas.

repetier commented 5 years ago

Ok now also did the test on V1. Advance steps did go up to 32 which matches your result if I scale it to my 147 steps/mm. I also got the ripples with increasing advance while the seam at the back looks identical to V2 going from thick to good to open. So what must be happening is that the overall advance is being set but in the small segments there must be some extra steps up/down that cause the ripples.

The big difference is that in V1 advance steps computation is pure integer based while v2 uses floating point math for this. So my guess is that since pattern is quite regular it is a combination of rounding errors for speed (which is also steps per second) and scaling the max value causes a few steps deviation which becomes visible where no difference should be and the overall steps still work so the big zones like edges still work as expected.

I will check if I can see some space for improvement like scaling integers up for higher precision.

MacFlieger commented 5 years ago

Thanks a lot for your testing and support. I am really glad, that you can reproduce the problem. This shows, that not my printer or configuration is the problem.

I am wondering, why nobody else has found this problem before. It should happen to anyone who is using advance and is printing not only straight objects.

Do you have a clue, when v2 will be possible to compile for Arduino Mega 2560?

repetier commented 5 years ago

I thought I had found the issue as my computations showed an overflow of the advanceL which was only 16 bit, but fixing that unfortunately seems to make no difference.

No idea why nobody else noted resp. reported this. Guess there are many not reporting problems like this.

8bit support for V2 is hard to say. I'm currently quite busy getting it working with some required stuff for some manufactors while finishing next server release. Guess it will still take some month to finish 8 bit and then I hope 8 bit is fast enough for the new system. While it seem superior it has more floating math which can limit speed, also the stepper interrupt it self is much faster now.

Nibbels commented 5 years ago

Awsome tests @MacFlieger ! I read your thread at christmas day but was not able to comment from home.

In fact I can remember a part that looked like this pattern. My following test-towers have always been rectangular so I was not able to link this problem to advance. Nice find! Not many people understand Advance and others dont know how to configure it unless they find https://github.com/repetier/Repetier-Firmware/wiki/Hardware-settings-and-print-quality#extruder-pressure-control Thatwhy your test-towers are awsome and ad some good info about V6 tuning. They show that the k=40..70 for this type of Hotend seems to be really true.

Even better: Those pictures show newbies what effects get fixed by advance. You guys should consider including some picture of an advance-tower within Repetier-Firmware/wiki I remember advance being quite hard to link to a 3dprinting problem just by reading theory. It took me days to understand the need and optimize my printer once. (Pictures: http://www.rf1000.de/viewtopic.php?f=67&t=1928#p19435)

As this bug might have disturbed repetiers christmas a tiny bit for me and some other people a patch that makes our beloved printers a bit better is the perfect christmas present 🥇 I cant wait to pull any incremental patch on this.

repetier commented 5 years ago

Ok, looks that after all this is not really a bug just a result of limited precision.

If I check the last circle I get the following extrusion distances (de) and speeds (se)

> 18:27:21.034: de:0.03401 se:1.50
> 18:27:21.047: de:0.03401 se:1.50
> 18:27:21.055: de:0.04082 se:1.80
> 18:27:21.063: de:0.03401 se:1.50
> 18:27:21.071: de:0.03401 se:1.50
> 18:27:21.080: de:0.03401 se:1.50
> 18:27:21.096: de:0.04082 se:1.80
> 18:27:21.117: de:0.03401 se:1.50
> 18:27:21.141: de:0.03401 se:1.50
> 18:27:21.162: de:0.03401 se:1.50
> 18:27:21.186: de:0.03401 se:1.50
> 18:27:21.211: de:0.04082 se:1.80
> 18:27:21.231: de:0.03401 se:1.50
> 18:27:21.256: de:0.03401 se:1.50
> 18:27:21.276: de:0.03401 se:1.50
> 18:27:21.301: de:0.04082 se:1.80
> 18:27:21.321: de:0.03401 se:1.50
> 18:27:21.346: de:0.03401 se:1.50
> 18:27:21.366: de:0.03401 se:1.50
> 18:27:21.391: de:0.04082 se:1.80
> 18:27:21.415: de:0.03401 se:1.50
> 18:27:21.436: de:0.03401 se:1.50
> 18:27:21.457: de:0.03401 se:1.50
> 18:27:21.481: de:0.04082 se:1.80
> 18:27:21.502: de:0.03401 se:1.50
> 18:27:21.526: de:0.03401 se:1.50
> 18:27:21.551: de:0.03401 se:1.50
> 18:27:21.571: de:0.04082 se:1.80
> 18:27:21.596: de:0.03401 se:1.50
> 18:27:21.628: de:0.03401 se:1.48
> 18:27:21.710: de:0.03401 se:1.52
> 18:27:21.719: de:0.04082 se:1.80
> 18:27:21.751: de:0.03401 se:1.50
> 18:27:21.776: de:0.03401 se:1.50
> 18:27:21.800: de:0.03401 se:1.50
> 18:27:21.821: de:0.04082 se:1.80
> 18:27:21.841: de:0.03401 se:1.50
> 18:27:21.866: de:0.03401 se:1.50
> 18:27:21.887: de:0.03401 se:1.50
> 18:27:21.911: de:0.04082 se:1.80
> 18:27:21.932: de:0.03401 se:1.50
> 18:27:22.087: de:0.03401 se:1.50
> 18:27:22.095: de:0.03401 se:1.50
> 18:27:22.103: de:0.04082 se:1.80
> 18:27:22.112: de:0.03401 se:1.50
> 18:27:22.120: de:0.03401 se:1.50
> 18:27:22.128: de:0.03401 se:1.50
> 18:27:22.136: de:0.04082 se:1.80
> 18:27:22.144: de:0.03401 se:1.52
> 18:27:22.153: de:0.03401 se:1.48
> 18:27:22.161: de:0.03401 se:1.50
> 18:27:22.181: de:0.04082 se:1.80
> 18:27:22.206: de:0.03401 se:1.50
> 18:27:22.227: de:0.03401 se:1.50
> 18:27:22.251: de:0.03401 se:1.50
> 18:27:22.272: de:0.04082 se:1.80
> 18:27:22.296: de:0.03401 se:1.50
> 18:27:22.321: de:0.03401 se:1.50
> 18:27:22.341: de:0.03401 se:1.50
> 18:27:22.366: de:0.03401 se:1.59

14 * 0,04082 + 46 * 0,03401 = 2,136 mm
e total = 60 * 0,03568 = 2,1408mm

If you take the difference from values it is always 0,03568-0,03569 but when firmware subtracts e values from gcodes it gets 0,03401 or 0,04082.

32485G1 X104.245 Y103.524 E1069.55867 ; perimeter
32486G1 X103.524 Y104.245 E1069.59435 ; perimeter de = 0,03568
32487G1 X102.731 Y104.888 E1069.63003 ; perimeter de = 0,03568
32488G1 X101.875 Y105.443 E1069.66571 ; perimeter de = 0,03568
32489G1 X100.966 Y105.907 E1069.70140 ; perimeter de = 0,03569
32490G1 X100.013 Y106.272 E1069.73708 ; perimeter de = 0,03568
32491G1 X99.027 Y106.537 E1069.77276 ; perimeter  de = 0,03568
32492G1 X98.019 Y106.696 E1069.80845 ; perimeter  de = 0,03569
32493G1 X97.060 Y106.747 E1069.84203 ; perimeter  de = 0,03358

The problem is that a float ha sonly precision for 6-7 digits. E1069.84203 is at it's best only E1069.842 So in overall firmware extrudes the right amount as it sometimes extrudes more for a segment to adjust for the rounded down values. These are the blobs you are seeing.

That is one reason why you should at least at each layer switch add a G92 E0 to set back the distance, so you get more precision. In your example you worsened the problem by having tiny moves so extrusion is very small per move so these rounding errors make more difference.

What I'm still wondering about is that the error is the same over height also at lower positions the E value is smaller and we should get a better precision there. But the log I get does not show a difference. The other thing is why does V2 print it better. It should lack the same precision problem. My current guess is the answer to the question 1 would also explain 2.

repetier commented 5 years ago

Ok, I tried making a cylinder. Had only 40 sides but I used relative extrusion distances and reset E with G92 E0 and it had no artefacts, seam at the back was as with the others.

You can try this file here: Zylinder60_advance.gcode.zip

Maybe you can also recreate yours with 60 faces and relative extrusion with reset. Seems like the reset makes a big difference or it is that the segments are longer now.

There is one pattern i still see and I need to figure out if it the slicer or not. Slicer has inserted sometimes different segment length and that is what I see as pattern. But not the bulbs I had before.

repetier commented 5 years ago

Ok I now understand what the problem is. In my case I have 147 steps per mm for e and 640 steps per mm for xy. So on small segments xy have a good per step resolution but 147 steps per mm means 1 step = 0,0068 mm

but for 1mm segments I need 5 steps = 0,03401 6 steps = 0,04082 That is a big difference in percent if you pack this on the same xy distance you get different E speeds and advance has to go up and down as well as total extrusion. This also is exact the difference in speed I see in my tests. V2 uses the float and does not compute speeds on step level and that makes it immune to this problem. It still has to round steps for extrusion but will not add advance steps in addition to the wrong rounding.

So in this example the increasing E values have no effect. None the less will the precision at some point suffer from the 7 digit precision so using relative extrusion and reset E position on layer change and tool change is still a good idea to improve quality for bigger prints.

Now I have to think how I can use float distances for speed computations without breaking the V1 firmware system that was build on motor positions.

MacFlieger commented 5 years ago

I agree, that the precision of floating point ist a possible cause for problems and therefore a G92 E0 every layer would cure this problem. But I don't think, that this is the problem here, because the problem with high values of advance_l is from the first layer and nearly constant over the height. Nevertheless using G92 E0 at start of a layer is not a real solution for the problem of FP precision. The test objects is very simpel and with few extrusion, so big numbers are only at the top of the object. If the object is greater with much more extrusion in a layer, the FP precision also could be important at the end of an layer. Perhaps it would be better to decode E-Value not as a normal FP. It could be better to decode it with integer separate for the number before and after the decimal point. And only the difference of E-values, wich is needed for further für movement, as an FP. But I think, this is a complete different thing to look at.

OK, so the really problem here is the speed of the axis E (speedE), wich is used to calculate the advance steps, cause this speed is calculated on base of the steps of E (integer)? If this is correct, I understand why my towers show this patterns.

I have looked and searched in the source. speedE is only used to calculate the advance steps and eJerk. In my opinion it would be sufficient, to calculate speedE not based on the steps (integer), but based on the E-values in the GCode.

I will try to make such a change and test ist. I have searched a little bit in the code and it seems to be complicated. The calculation of speedE ist deep in the calculation of movements, wich are all based on steps (integer). I had hoped, that only a change at this point could help, but this part is far away (separated) from the part, where the E-Values are used. Perhaps it is much easier to buy a Due an to compile v2... :)

repetier commented 5 years ago

Using V2 is easier, but does not solve problem for V1 users. I currently try to convert steps into virtual steps with higher precision (16 times) so the problem becomes negligible due to increased resolution. Maybe 150 places to change the code and works to 90% already. Only acceleration and extrusion during move and z probe seem still to not work. Hard part is to decide which parameter is in real steps and which in hires steps. So I just need to multiply/divide at the right places. Problem is not to omit one:-) But this shows clearly that my new approach in V2 has even more benefits as it has no resolution problems. Steps are computed at last possible state.

Nibbels commented 5 years ago

I once put a virtual speed multiplier into the main interrupt. I have DMS force sensors measuring the hotends inner pressure. This change was made to make the printing speed preasure dependant. -> To avoid division and floats while manipulating the timers interval I used a factor like 1024 and a 10 bit shift for division.

It did not make my prints worse.

MacFlieger commented 5 years ago

I hope, that a factor of 16 will be enough, but I'm not sure.

I am trying to preserve the normal movement as it is and only use a different speedE, which is not calculated from the steps but from the E-values. Sorry, I have really tried to add something like "Printer::currentExtruderMM" and "Printer::destinationExtruder" which can be set from them GCode and should give a better speedE. But there are so many places where the corresponding currentPositionSteps and destinationSteps are modified and I don't know why and how. I will wait for your solution.

repetier commented 5 years ago

The factor can be any 2^x value. It just needs to make sure no integer overflow happens. But I think 16 is more then enough. Values already look very konstant. Only thing is extrusion with you sample gcode doe snot work. With longer extrusions no problem. All other problems are already fixed I think. So when I find the reason why it computes a e move and does not execute I will see if it helps:-)

repetier commented 5 years ago

One more thing. In my case xy have good resultion with 640 steps per mm. If xy has low resolution that will also have influence on speed as then the distance is wrong and with short distances again the error increases. That is why I want to increase all axis virtually in resolution so all errors get reduced.

repetier commented 5 years ago

Problem solved. See picture: 2018-12-29 10 56 44

Form left to right: V1 as on github V1 with 16 times resolution enhancement V2

Now I see V2 has some difference at the joints of the segments but that will be something different. Guess it is not exactly the same path or has to do with different velocity profile. Will test this more when I get some more time. Have to finish urgent things first. I will also wait a bit with the firmware update on github. Need to run some more tests as this is a severe modification. But at least I can say that solution in general is working fine. Next update will contain the fix.

MacFlieger commented 5 years ago

Thanks for your fast solution. I will wait for the update.

repetier commented 5 years ago

Looks like I was too fast:-( The higher resolution now causes rounding errors for xyz moves so they get skewed. Will see how I can modify the algorithm to remove rounding problems. One reason i wanted to test first as I feared this could happen

Nibbels commented 5 years ago

Because I have to pull those modifications into the Conrad Renkforce-fork derived community modded firmware I will read against your changes line by line when you released it.

Btw.: You once did this trick with extrusion calculations using Printer::extrudeMultiplyError: https://github.com/repetier/Repetier-Firmware/commit/07e46fb79d4dbd1f579971d787c861e7d8b94c8b#diff-593812a66d7348c87b711b15b1ad5195R1378

Wouldnt a simliar approach help? (Dont know because I would have to remember/reread all the code I already forgot ^^) But I guess that within the interupts we always come back to total steps. So we have to remember and somehow sync rounding errors for both: advance and standard extrusion.

repetier commented 5 years ago

That trick is before we do all the computations that need the higher resolution.

I currently use 16*delta values for bresenham so on average it worked. But 7 steps /16 is 0 steps. 10 Times 7 steps is still 0 steps also 70/16 = 4 steps should have been done. That is the kind of error I now see. I guess I have to reduce quality right after the speed calculations to original steps again. Then a step stays a step but speeds got computed on the right distances. A really wicked problem.

Nibbels commented 5 years ago

Hmm, when one move has 7 Steps and it gets rounded to "0 leftover 7" we should copy the leftover into some static E-variable and add it into the rounding calculation within the next move. So the next move would see "(move(n-1)leftover + move(n)steps) / 16" = (7+x)/16 ?

repetier commented 5 years ago

The main problem I think is I have no real position. All I get is delta values. I really need the sum as well so I can know if I need to round up or down. So yes, what you suggest is what needs to be done. Currently searching the best place to do so. We already have current and destination steps so I guess I need to send these to the algorithm as well instead of only sending the difference. That is where the required information gets lost.

repetier commented 5 years ago

No, that is not gonna work. Need to revert all my changes. With delta and subsegments and some other things this is not possible to handle. I need to maintain a float destination position as well and use that to compute speed while holding steps position in parallel. So back to start:-(

repetier commented 5 years ago

Ok, floating point solutions looks as good as on the picture and is even simpler from the math side. Will do some tests on delta tomorrow and then publish.

repetier commented 5 years ago

Looks like the third solution is now finally working. Have uploaded it so I can get feedback for things I did maybe overlook. Did test absolute and relative extrusion, extruder switch, cartesian and delta print. All seem to work so far. So please test and report if it works for you or created new problems I did not notice.

Nibbels commented 5 years ago

I read your commits on the problem the last days. Just by reading I was not able to spot problems. (I wouldnt know missing things though.) After some nonsense ideas this is all I got:

Two functions now seem to be unused:

And I guess that having a tiny bit more CPU time within the steps interrupt wouldnt really affect STEP_DOUBLER_FREQUENCY noticable(?).

^^ Greetings

repetier commented 5 years ago

No, it is much more. I also set now destinationPositionTransformed to compute delta move in float instead of multiplying steps with steps per mm. So you need to copy pretty much everything from the commit.

The part you noticed is to prevent overflow problems in some combinations of speeds.

Nibbels commented 5 years ago

I already think I have understood the full meaning of your advance patch. If it was that easy like just copying everything. Conrad Electronic added so much logic for milling. So I kind of had to rewrite in any case.

... So I spend this week changing all summation to float. Doing that I changed the logic of setting the last destination to the new destination after calculating axis deltas to some sort of error dragging. (I have no transform function, only cartesian.) Now last-destination-mm[] always represents `rounded´. All incomming coordinate changes go into destination-mm[]. Delta of those always should have the most perfect error.

Tomorrow I can test this, hopefully.

MacFlieger commented 5 years ago

A little bit late, but here is my feedback. The patch does work very good for me. There is no pattern anymore and I can use advance and therefore my prints are much better.

There is only a problem with low values of advance_l. For me it is no problem because I need very high values (around 100), but perhaps for other users. While using low values (in the example 10-20) the printing head does not move smooth around the circle, but sometimes stops at a specific angle and so produces a little blob as can be seen on the photo.

img_1300

These stutters and blobs are fully reproducable and every time at exact the same position when using the same parameters. When I use another layer height (e.g. 0.1mm instead of 0.2mm of the photo), the blobs are at different heights and for values of 10-30 advance_l, but also exactly reproducable.

As written, it is nor problem for me, but probably for other users.

Nibbels commented 5 years ago

Thats like a 5% chance, but do those stutters go away when you drop https://github.com/repetier/Repetier-Firmware/blob/c09783a0f4f1bc2bedefc8df2c90709172ae74b2/src/ArduinoAVR/Repetier/Configuration.h#L1215

to something lower like 8000? That would limit the interrupt rate and leave more performance for other things. Your printer might sound a bit weird when reaching that limit, but it might be worth a test.

Greetings

repetier commented 5 years ago

Good point. Extrusion is done in separate interrupt and if it gets not called often enough it will happen once there is enough time. looking at the picture I see a groove before each blob like it is adding the material being missed before. In my tests I did not have any blobs for any advance value.

So what board and steps per mm for each axis are you using? Ona due you can even set STEP_DOUBLER_FREQUENCY to 80000 without problems. That is what I used for my testing. On AVR boards the additional computations might make 12000 too high also with the speed of your sample code you need high resolution get that frequency.

MacFlieger commented 5 years ago

I have tried with STEP_DOUBLER_FREQUENCY 8000. For layer height of 0.2mm the blobs are now only for values of 10 (before 10-20) and are at different positions. So this interrupt seems do be the problem.

I am using:

Because I need high values for Advance_L (around 100) this problem is not important for me, cause it only appears with low values. I don't know why.

repetier commented 5 years ago

Ok, you have at 60mm/s at maximum 9600 interrupts per second depending on angle.

What I find still strange is that higher advance values have no problems. Fact is the higher advance is the more interrupt calls of extruder interrupt you need.

What you changed is where double stepping now takes part, so it may in deed depend on double stepping. Just not sure what the dependency is. Will see if I can also get it with 12000 on due. If it appears it should not depend on speed but more on the method.

Nibbels commented 5 years ago

I once flipped the limiter system. Instead of frequency I limit interval. This minimal interval in cpu-ticks was a bit more understandable for me. https://github.com/Nibbels/Repetier-Firmware/blob/edbfdcd5357a87e8c6e5d36c5272977947dbb36c/Repetier/motion.cpp#L1946

I once thought this inverted method somehow behaved differently even if I had similar settings. But didnt fully research it.

Instead of double-quad stepping this gives me double-tripple-quad-...-stepping for me.

Nibbels commented 5 years ago

I want to report back what happened on my side until now.

As you did, I fixed all the coordinates to float and pulled the 32bit calculation for ADVANCE. After all those changes I was very happy because the noise ADVANCE was producing before was gone. Formerly there was this snoring-noise which now seems to be fixed.

I had still one problem: When I printed tiny holes or thin cylinders I had a lot of disturbance. This was also visible as a non round print result. This effect was gone if I set ADVANCE_L to 0 and therefore disabled it.

Maybe because of move->isEmove() on-off-on-off-on effects when extruding tiny amounts?

When I removed the following (corresponding) piece of code everything was totally smooth. https://github.com/repetier/Repetier-Firmware/blob/c09783a0f4f1bc2bedefc8df2c90709172ae74b2/src/ArduinoAVR/Repetier/motion.cpp#L827

Was this the cause that you changed this line? https://github.com/repetier/Repetier-Firmware/commit/4a0172860274c68221ef6ce097efb2cb0f407ac7#diff-11347f18746fb080f5bda21c30428558R259

Greetings

PS: Thx for repetier server 0.91 update, i just saw it.

repetier commented 5 years ago

I want to report back what happened on my side until now.

As you did, I fixed all the coordinates to float and pulled the 32bit calculation for ADVANCE. After all those changes I was very happy because the noise ADVANCE was producing before was gone. Formerly there was this snoring-noise which now seems to be fixed.

I had still one problem: When I printed tiny holes or thin cylinders I had a lot of disturbance. This was also visible as a non round print result. This effect was gone if I set ADVANCE_L to 0 and therefore disabled it.

Maybe because of move->isEmove() on-off-on-off-on effects when extruding tiny amounts?

When I removed the following (corresponding) piece of code everything was totally smooth.

You mean including advance as well?

While it is good to know it causes the problem, the exception was for a reason. Case I want to catch is fast travel move and then start with a high extrusion speed. That means if angle is flat you will start with high extrusion speed and need to build up extruder pressure at an instance. So here starting at lower speed makes adding advance steps easy.

My guess is that you have consecutive E moves but sometimes they do not change a step so it is not an e move and does the same making it accelerate and break. Here the speed problem does not exist as much. The xy move is short and next one continues with e move. On the other side e speed for that segment is 0 so advance is also 0 and it will revert extruder - only the fact that xy move is short might prevent it from doing it completely.

Can you post the gcode and say where it happens. Would like to see the code so I can see if it is what I think. Small moves can always lead to numerical instabilities as small absolute errors can cause big relative errors.

Repetier-Firmware/src/ArduinoAVR/Repetier/motion.cpp

Line 827 in c09783a

if(Printer::isAdvanceActivated()) { Was this the cause that you changed this line? 4a01728#diff-11347f18746fb080f5bda21c30428558R259

That is not related to your problem. This was a fix to distortion correction using roundings to integer for float values. Caused unsteady moves and extrusion. Just did not set type correctly as I did on the non distortion parts.

Nibbels commented 5 years ago

Hi again,

sadly I have always been testing using "USB-Print" without saving the files and as I tested it on my second printer I discarded the profile. So I had to remember and recover the settings the best I could.

Just some minutes ago I have been reverting the firmware to reimplement the highspeed-advance-fix-rule. The effect seems to be there again, but not that visible that I had it before. https://youtu.be/hiht4Is5imk

In the second video I removed the rule that Emove and NonEMove slow down. https://youtu.be/AadnqleK_0k Sadly my cameras battery shut down at the end, but we clearly see that the speed change is enormous.

[Legend: For all which cannot understand German, In this video I am switching Advance from 0 to 20 and back. We see that the speed of printing the small inner circles changes alot. The inner circles have one perimeter and some sort of slow infill, probably S3Ds single wall extrusion or similar. Dont know - but that is tiny infill always slow. I am printing with a fork of repetier, not the original firmware. So my problem still can be my problem, not yours.]

Greetings

Files: 3DBenchy_-_LEGOcompatible-_Bottom_connection_CHANGE_STARTCODE!!_0.1firstlayer.zip screenshot_1 screenshot_4 3DBenchy_-_LEGOcompatible-_Bottom_connection.zip

Nibbels commented 5 years ago

Well, my printers axes steps/mm are: X 152 Y 152 Z 1280 E 320 @atmega 2560

repetier commented 5 years ago

Ok seems to be just what I thought. 1 extrusion step is 1/320 = 0.003125 Looking into the inner circle you have e.g. G1 X106.787 Y136.530 E0.0804 G1 X106.891 Y136.246 E0.0821

that is a delta E = 0,0017

So only every second segment you have a real step here. So also the rule should not trigger here the short segment makes it do exactly that.

Also removing make sit more fluent, it still has the error of moving advance up/down here. The better solution should be to leave the condition but modify the test.

Looking into my code I in deed already have if(axisDistanceMM[axis] != 0) p->setMoveOfAxis(axis); while you have

        if(p->delta[axis] >= 0)
            p->setPositiveDirectionForAxis(axis);
        else
            p->delta[axis] = -p->delta[axis];
        axisDistanceMM[axis] = p->delta[axis] * Printer::invAxisStepsPerMM[axis];
        if(p->delta[axis]) p->setMoveOfAxis(axis);

So i test floating point difference and you test on the integer which gets rounded down. I also saw you did use fabs for axisDistanceMM at least not in that loop.

In my code I also see i have direction based on integer, but that would not be good I fear as it might go wrong into the direction flag then. For e not so relevant but for xyz.

This solution should solve it, but you might get into trouble with your absolute coordinates if they get > 1000 you will start dropping precision at lower extrusion steps. Relative extrusion is better here. Or at least resetting extrusion every layer.