MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
15.97k stars 19.09k forks source link

[BUG] Junction Deviation (enabled) causes curve stuttering #17920

Closed hrabbot closed 3 years ago

hrabbot commented 4 years ago

Bug Description

Curved extrusions stutter (multiple full stops in the middle of a continuous curve) with Junction Deviation enabled. Enabling/Disabling S Curve and Linear Advance (indepentently and in combination) does not remedy the problem. Adjustments to acceleration and junction deviation value do not remedy the problem.

Disabling Junction Deviation remedies the problem, even with S Curve and Linear Advance both set.

My Configurations

Required: Please include a ZIP file containing your Configuration.h and Configuration_adv.h files. Working_configs.zip Stutter_configs.zip

Expected behavior: Smooth curve motion, as expected based on baseline experience.

Actual behavior: Stutter/halting during curve motion.

Additional Information

Configs and example video attached.

qwewer0 commented 4 years ago

Could you isolate a half/full circle of the gcode that stutters, and upload it?

hrabbot commented 4 years ago

Could you isolate a half/full circle of the gcode that stutters, and upload it?

Here's a snippet:

G1 X221.745 Y15.794 E9.0641 G1 X221.904 Y15.798 E0.0099 G1 X222.890 Y15.846 E0.0615 G1 X223.207 Y15.877 E0.0198 G1 X224.262 Y16.034 E0.0664 G1 X224.574 Y16.096 E0.0198 G1 X225.580 Y16.348 E0.0647 G1 X225.876 Y16.437 E0.0193 G1 X226.904 Y16.802 E0.0679 G1 X227.207 Y16.927 E0.0204 G1 X228.176 Y17.385 E0.0668 G1 X228.457 Y17.536 E0.0199 G1 X229.373 Y18.086 E0.0666 G1 X229.641 Y18.265 E0.0201 G1 X230.505 Y18.908 E0.0671 G1 X230.747 Y19.107 E0.0195 G1 X231.519 Y19.807 E0.0650 G1 X231.743 Y20.030 E0.0197 G1 X232.454 Y20.814 E0.0660 G1 X232.655 Y21.059 E0.0197 G1 X233.296 Y21.921 E0.0669 G1 X233.476 Y22.190 E0.0202 G1 X234.032 Y23.118 E0.0674 G1 X234.181 Y23.398 E0.0198 G1 X234.633 Y24.355 E0.0659 G1 X234.754 Y24.647 E0.0197 G1 X235.112 Y25.646 E0.0661 G1 X235.205 Y25.954 E0.0201 G1 X235.467 Y27.006 E0.0675 G1 X235.528 Y27.314 E0.0196 G1 X235.682 Y28.352 E0.0654 G1 X235.713 Y28.669 E0.0198 G1 X235.762 Y29.654 E0.0614 G1 X235.766 Y29.813 E0.0099

qwewer0 commented 4 years ago

How fast did you print?

qwewer0 commented 4 years ago

Can't really feel stutter in the given gcode snippet, or Can feel it a little stutter all the time.

Try it out: JD_single_arc_test_2.zip

XDA-Bam commented 4 years ago

Gcode: Issue_17920.zip

Speed limit profile (not accounting for nominal_speed_sqr) should look roughly like this: Filter_none_issue-17920_rev4 (JD 0.013, accel 500, as per OPs config)

The problem is, that we're just on the verge of 1 mm length on those segments. So we jump between regular >1 mm v_max_junction_sqr (white background)) and the <1 mm case limit_sqr (grey background).

EDITED: Realized, that the segment length is the problem, not the junction angle (that's around 177.5° for most).

hrabbot commented 4 years ago

See zipped video example. Also note blobby curves already on printbed. stutter.zip

daleckystepan commented 4 years ago

So we should come up with solution where transition between speeds will be smooth. What about get rid of 1mm block and >135° condition and do the math always? Maybe we can do it with another/faster approximation and without division at the end of calculation (I have this approximation on my github). But I'm just guessing - I don't understand the code and movement enough.

XDA-Bam commented 4 years ago

Here's the block or segment length for the GCode in discussion: Issue_17920_block_length

XDA-Bam commented 4 years ago

So we should come up with solution where transition between speeds will be smooth. What about get rid of 1mm block and >135° condition and do the math always? ...

Dropping those if-checks sadly doesn't solve the stutter in this case. limit_sqr is bumpy itself, as you can see from the speed limit plot.

This is a general problem of the discretization and the fact that we can't look ahead. The two JD code paths (>135° && < 1 mm vs. <135° || > 1 mm) have different problems due to this, but in essence, the code is "right" in stuttering like it does. The GCode is sliced in a way, which dictates these speed changes. From the point of view of the planner, it is logically impossible to tell if this one small element it is looking at right now with that slightly larger change in angle than the one before is the beginning of a sharper curve, or just a slicer artifact inside of a smooth, continous circle. No amount of transition between the two code paths can solve this problem (in general, it might work for specific cases).

There's only one way to safely prevent this: When planning a block, look ahead a couple of blocks to see where we are going. Then, fit an analytical function to our path (backwards and ahead) and calculate the second derivative for the block currently being planned. That would require extensive changes to the planner code and quite a bit of computational resources when running. Also, I wouldn't know how to implement this in C++ 😄

thisiskeithb commented 4 years ago

Should we revert back to classic jerk before the next release?

Lord-Quake commented 4 years ago

I'm not sure if it is necessary to default back to classic jerk since for the most part JD is seems to work. Going from 5.0.5.3 and then looking at each commit I was able to determine that the problem was introduced with commit 2020.05.04 and I believe this could be the culprit: https://github.com/MarlinFirmware/Marlin/commit/c55475e8e7c7b389b30ca1d154618d73faba52ab

Below my results with the jump from good to bad: Junction_Deviation_Curve_Stuttering

Edit: Corrected the commit dates.

thinkyhead commented 4 years ago

Should we revert back to classic jerk before the next release?

It's preferable for many cases, so it might be the best choice for certain example configurations. However, this kind of suggestion implies that we are no closer to a solution.

Since jerk is relatively inexpensive, perhaps the best thing would be a hybrid approach.

Although, I am still curious to see if the "angular change within X total distance" accumulator approach could also work, insofar as it can be tracked in an efficient way.

I feel I could get pretty far with this sort of algorithm experimentation if I had nothing else to do. I mean, what we ultimately want to do is produce the ideal and perfect, but horribly expensive version first. Then try to chop that up into something simpler.

But maybe Classic Jerk is superior in every way? I can't think of too many reasons to fault it.

thinkyhead commented 4 years ago

I believe this could be the culprit

@Lord-Quake — You may be right. Does it look better if you disable JD_USE_LOOKUP_TABLE (inside of planner.h)? If so then the LUT is missing something that the old approximation has "more right."

Lord-Quake commented 4 years ago

@thinkyhead Using commit 2020.05.04 I disabled

//#define JD_USE_LOOKUP_TABLE

Result is no more stuttering. The print turned out fine.

You've now got something to work with :-)

Lord-Quake commented 4 years ago

Just for the sake of completeness I tried the latest commit https://github.com/MarlinFirmware/Marlin/commit/86c112538084f7718252966f5f9d9278c4837fb2 and disabled //#define JD_USE_LOOKUP_TABLE with the same result. The stuttering has stopped, print is fine.

I did find a new issue (cosmetics only though)

XDA-Bam commented 4 years ago

Since #17938 has already been merged and might influence this, it would be nice if someone affected could check this again (with the lookup table enabled) using most recent bugfix. It may be improved, I don't think it will be fixed.

Lord-Quake commented 4 years ago

Well testing with my Ender 3 Pro is over for now..... Uploaded the firmware as requested. While starting the print I touched the X-Stepper motor and got a big electric shock. The X-axis no longer moves. Looks like the stepper driver or some component related to the X-axis is defective. I will have to get a new main board :-(

Lord-Quake commented 4 years ago

@XDA-Bam While my Ender 3 Pro now dead atm, with which I do all my tests I decided to update my Anet A8. Since I can't compare the degree I can say the stuttering is still evident. Maybe someone else can also report?

lzamel commented 4 years ago

Using the latest commit (1475fd312a1572e1d43978f669bce02a72f63dab) with #define JD_USE_LOOKUP_TABLE and I'm no longer observing any stuttering. If there's any it is minimal. Before the whole machine was shaking on arcs.

thinkyhead commented 4 years ago

By disabling JD_USE_LOOKUP_TABLE the planner is using the older JD code. The newer JD code is supposed to be more accurate and use less processing, but it also seems to produce the side effects you are seeing. We are continuing to experiment, and will do a comparison of the numbers that come out of the current code versus the older code.

cwizard commented 4 years ago

@XDA-Bam While my Ender 3 Pro now dead atm, with which I do all my tests I decided to update my Anet A8. Since I can't compare the degree I can say the stuttering is still evident. Maybe someone else can also report?

Bro if you are in the USA I have a spare stock motherboard I can send to you to get you back up. I used bugfix for a day and a half before I realized JD was ruining my prints.

Lord-Quake commented 4 years ago

@cwizard Thanks for the offer! I'm in Germany and will try to get a replacement.

davevo22 commented 4 years ago

Hi, where I can comment on the function JD_USE_LOOKUP_TABLE? I have CONFIGURATION_H_VERSION 020005 and I have the same problem with motion stutter. Thank you for answer

Lord-Quake commented 4 years ago

@davevo22 \Marlin\src\module\planner.h

davevo22 commented 4 years ago

I searched there but did not find it. Row number?

Lord-Quake commented 4 years ago

Around line 43

davevo22 commented 4 years ago

interesting but this function is not there: D

davevo22 commented 4 years ago

hear is all code https://pastebin.pl/view/9138260a

Lord-Quake commented 4 years ago

It's in the latest commits. You are most likely using an older version of Marlin.

davevo22 commented 4 years ago

Im using last stable version 2.0.5.3

Lord-Quake commented 4 years ago

Oh, in that case you won't find it. Stay with 2.0.5.3 unless you have problems that might have been fixed in the latest bugfix version.

XDA-Bam commented 4 years ago

I currently see two possible reasons for the stutter:

  1. There's a bug in the LUT or the code is compiled incorrectly.
  2. The higher precision and valid range of junction_theta from the LUT is screwing with something else.

I'd ilke to check number 2 and made a quick edit to planner.cpp, limiting the range of junction_theta to the one we get out of the Minimax poly: 2020-05-16_planner_fix_1.zip

The only change to current bugfix is the line NOLESS(junction_theta, RADIANS(180 - 178.11));, but feel free to check yourself.

To those affected: Please test, if this reduces the stutter to a similar level as when using the polynomial approx. Please make sure to enable the LUT by uncommenting #define JD_USE_LOOKUP_TABLE in planner.h. If unsure, just download the latest bugfix-2.0.x and just copy over the replacement planner.cpp.

qwewer0 commented 4 years ago

@XDA-Bam It feels worse, with a larger bump at the small segments.

davevo22 commented 4 years ago

None of the variants can be compiled. :((((

qwewer0 commented 4 years ago

None of the variants can be compiled. :((((

With the latest bugfix?

davevo22 commented 4 years ago

yes https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.0.x/Marlin/src/module/planner.cpp

qwewer0 commented 4 years ago

Just that planner.cpp, or all the bugfix?

davevo22 commented 4 years ago

only planner.cpp

qwewer0 commented 4 years ago

only planner.cpp

You need to use the whole bugfix marlin.

XDA-Bam commented 4 years ago

@Lord-Quake How about your A8? Does the modified planner.cpp make any difference concerning the stutter?

@davevo22 You need the whole bugfix-2.0.x package. Then, overwrite the original planner.cpp with the provided file.

Lord-Quake commented 4 years ago

@XDA-Bam Normally I don't touch the only printer I have left as it needs to be working. However, if I do test with the A8 what commit do you want me to use?

XDA-Bam commented 4 years ago

If the machine is critical to you, it's probably better not to test some random experimental build on it. We can wait a couple of days until someone else checks it or your replacement board arrives.

Lord-Quake commented 4 years ago

Still, what commit would you want me to use?

XDA-Bam commented 4 years ago

847bdee would be good.

Lord-Quake commented 4 years ago

@XDA-Bam I did the test with the above commit on my A8 and used the same gcode file as with my Ender 3. The curved stuttering was not present and the print surfaces be it flat or curved are all same in quality.

XDA-Bam commented 4 years ago

OK, so conflicting results.

@Lord-Quake and @qwewer0 Just to make sure: You both ran this test, using the updated planner.cpp with #define JD_USE_LOOKUP_TABLE still enabled, right? So, using the LUT?

Lord-Quake commented 4 years ago

I'm including my configs to eliminate any misunderstandings :-)

Configurations_LQ.zip

qwewer0 commented 4 years ago

Yes, using the planner.cpp and with #define JD_USE_LOOKUP_TABLE enabled.

Edit: Latest Bugfix-2.0.x

XDA-Bam commented 4 years ago

Well, that's just... 🐳 Am I correctly assuming, that the Anet A8 is running some form of ATMega, while @qwewer0 is running an STM32F103 chip?

Lord-Quake commented 4 years ago

I'm using "STM32F103RC_btt_512K" SKR mini 1.1