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
16.18k stars 19.22k forks source link

[Discussion] Handling of junction speeds and jerk #9917

Closed Sebastianv650 closed 4 years ago

Sebastianv650 commented 6 years ago

This is a follow-up to a topic which already was here a long time ago, but I can't find it anymore. If someone remebers it and can find the issue, you might link it here.

During the research for strange jerk behaviour today I stumbled around the suboptimal handling of junction speeds again. In one sentence: Marlins way of handling jerk and junction speeds leads to exceeded jerk in some daily situations. The attached test gcode and the results in the table should clarify things a bit. The gcode does the following things:

I printed the following values over serial:

I did two runs, first with X and Y jerk set to 5 and E jerk set to 1mm/s, and a second one switching the values. As you can see, as soon as we are switching between extruder only moves and XY moves things start to get wired. Same should be true for Z only moves and XY moves. The problem is that Marlin connects each segment by one single junction speed, which is not always what is needed. A simple example like the one from H13 in the table, lets assume a X Jerk of 1, an E jerk of 5. Let's do a print move along X followed by a prime move. What we want ist:

The two axis X and E are not linked, also their junction speeds shouldn't be linked. What Marlin does is in this case is:

jerk

It becomes crucial when we replace the first print move by a Z move. Z axis often have 0.x jerk values, but the final speed would be also 5mm/s due to the following retract move.

Conclusion:

Files: Marlin Junction speeds.zip

thinkyhead commented 6 years ago

GRBL will not help

Ah, well so much for my first suggestion…

Well, at some point we should try implementing a proper "junction deviation" and see whether it helps … and whether it's expensive to compute. We may be able to get away with using a table or some other cleverness to avoid having to do trigonometry.

But before we get into that, we should definitely figure out how to de-couple the E axis from the linear axes. And, it seems XY also needs to be less bothered by Z direction changes. Maybe for Z direction changes we can relax the Z jerk limit a bit. The thing is that Z seldom has anything like the same inertia as the XY axes. You'd think Z would be more amenable to direction changes, especially when going from raising to lowering.

Sebastianv650 commented 6 years ago

I think the way we calculating jerk at the moment is already quite good. Check for all axis jerk limitation, reduce entry / exit speed if one axis is exceeding the limit. But as we have to check entry and exit speed for jerk, we need to know about the previous and (that would be new) the next segment. At the moment, the planner assumes exit speed = next entry speed is always OK. So it needs to go into the forward or reverse planner. And it has to make a decision if the junction speed has to be linked or not. Some draft ideas:

Clearly we need to do test prints to simulate some of the effects before start coding. I think I could have some fun doing that, but it's something for the middle future - we don't want to release such a big planner change without intensive tests..

Sebastianv650 commented 6 years ago

I want to put some questions here to think about them. Maybe there are some intresting answers comming?

I'm looking forward to read your opinions!

thinkyhead commented 6 years ago

Why do we link the junctions speeds at all?

This seems to be a legacy of the XY gantry design of the Ultimaker that probably should not apply to Mendel X carriage / Y bed setups. And neither of those should be linked to Z. Before bed leveling came along, in 3D printing Z moves were almost always isolated Z or Z/E moves.

The theory of XY gantries seems sound — you do want to limit the inertial load of the carriage block, even though XY each have their own separate inertial loads too. I think you're onto something in pointing out that separated axes should only apply load limits on a per-axis basis and not in terms of the combined motion in the XY plane.

Better not to think too hard about deltas.

ruevs commented 6 years ago

Hi, I apologize for barging in on the conversation, but I think I have some relevant information. In my opinion deltas are an important group of machines and Marlin has treated them as "second class citizens" long enough.

I have been trying Marlin 1.1.8 (https://github.com/ruevs/Marlin_Anycubic_Kossel) and Repetier 0.92.9 (https://github.com/ruevs/repetier_anycubic_kossel) on the same delta printer. The configurations are as close as possible to each other. Max speeds and accelerations are the same. The jerk is 10 in Marlin and 20 in Repetier. As I understand that after this https://github.com/MarlinFirmware/Marlin/pull/8888 those values should lead to similar results. And now the interesting part. I tested the printer with this G-Code SpeedTest.zip with both firmwares. And here is the result: https://youtu.be/JCflNCotl70?t=1758 Repetier drives the steppers "smoother" (the sound is softer and less "scratchy"), finishes the test a bit faster and with less vibration/jerk. In my opinion something in the way Marlin drives the steppers in deltas is... different and not optimal.

Sebastianv650 commented 6 years ago

@ruevs please try the current bugfix-1.1.x. We patched a basic fault in the delta planner code which resulted in real speeds (and all derived values) way off the defined value and I'm quite sure this PR was done quite some time after 1.8 was released.

thinkyhead commented 6 years ago

In my opinion deltas are an important group of machines and Marlin has treated them as "second class citizens" long enough.

We continue to incrementally improve Marlin's delta support, and have done loads of optimization in recent releases and in the upcoming release. If someone brings us a comprehensive overhaul that addresses delta concerns in the form of a nice PR we would be more than thrilled.

Roxy-3D commented 6 years ago

In my opinion deltas are an important group of machines and Marlin has treated them as "second class citizens" long enough.

Perhaps nobody decided to treat Deltas as "Second Class Citizens". Maybe there is another explanation.

Deltas have only a small percentage of the users that Cartesian machines have. So this means first of all, it is much harder to get code developed and tested on Deltas. But it also means there are less people even interested in writing code for them. With that said, we do try hard to make the code work well on all machine types.

ruevs commented 6 years ago

Please do not take my comment the wrong way. I realize that Marlin has improved a lot in 1.1.x in terms of delta support. I only said it in response to thinkyhead's comment:

Better not to think too hard about deltas.

As for a pull request - if I ever have enough time to understand the code deep enough to actually improve or revamp the planner and stepper motor control logic for deltas I would certainly do so. Unfortunately this is unlikely.

thinkyhead commented 6 years ago

Better not to think too hard about deltas.

That is to say, for any indirect kinematic system Jerk and Acceleration apply to the movement of the steppers, and not to the linear movement of the tool. Thinking about this too hard may lead to headaches.

Sebastianv650 commented 6 years ago

@thinkyhead Just retested the initial test pattern of this issue with the new junction code (#10650, JUNCTION_DEVIATION_INCLUDE_E is disabled) , this is the result:

new_jerk

I see some intresting results compared to the initial results:

Enabling JUNCTION_DEVIATION_INCLUDE_E increases the jerk speed for print moves and e-only moves slightly.

thinkyhead commented 6 years ago

Thanks for bringing this back on topic. I definitely want to make the junction behavior more sensible, whether using the old jerk method or junction deviation. What we really need is to decide what the ideal behavior should be first, and then make needed adjustments to get it to produce those results as closely as possible. Your earlier points can act as a starting guide.

Sebastianv650 commented 6 years ago

Just a short addition to my last post: Not only e-only moves suffer due to the new jerk code, also z moves (layer shifts) are affected. As z-axis often uses very low jerk limitations (0.x), this can definitly result in missed z steps. Maybe even worse, on a system which uses 2 z steppers, only one might miss a step resulting in a skewed axis.

The code change was a good first step, but some more work is necessary to make it secure and fully usable.

thinkyhead commented 6 years ago

Not only e-only moves suffer due to the new jerk code,

@Sebastianv650 — Which "new jerk code" are you referring to in your comment? We have the sort-of-new jerk code related to the planner refactor (though it should be essentially the same limit-per-axis method) and then we have the new (but optional) JUNCTION_DEVIATION jerk code. The Bézier S-Curve acceleration is also new, but it follows after the main jerk calculations.

Sebastianv650 commented 6 years ago

I'm refering to JUNCTION_DEVIATION.

thinkyhead commented 6 years ago

Should Z be excepted from the Junction Deviation technique? It's certainly not like the XY axes. Likewise, with any move not involving X and/or Y…

p3p commented 6 years ago

Should Z be excepted from the Junction Deviation technique

Only if you want to disregard 3D CNC Routing, my Z axis is the same as XY (velocity and acceleration wise) and in finishing passes on organic geometry can be moving pretty fast.

thinkyhead commented 6 years ago

I think we probably want to disregard any axes that have low top speeds. That would filter Z and E on typical 3D printers, while allowing you to use CNC and arcs in vertical workspace planes.

Sebastianv650 commented 6 years ago

Maybe we need a two-step approach. For example on acceleration we already use a print acceleration and a max. acceleration per axis. If one axis max. acceleration is violated by the print acceleration, it will get lowered.

Same procedure can be applied on junction jerk. If the speed change on an axis from current to previous is bigger than axis max. jerk, limit the junction speed. If the user sets all max. jerk per axis values to a high number (as we already have it for max. acceleration on many printers), this limit will never engage. So no problem for 3D CNC for example.

This might also solve the problem with wrong junction speeds which I pointed out at the start of this issue?

thinkyhead commented 6 years ago

What is Grbl doing to mitigate the issue? Have we failed to fully duplicate their approach?

Sebastianv650 commented 6 years ago

I asked myself the same question, how GRBL sets limits for each axis, when I did my small Excel sheet for junction speed calculation. After reading their docs and code, I came to the conclusion that there is actualy no way to do this in GRBL since they use this jerk method.

Sebastianv650 commented 6 years ago

OK I have to correct what I just wrote. It's not very obvious, but I guess @thinkyhead we are realy missing an important piece of GRBL for JUNCTION_DEVIATION.

See https://github.com/gnea/grbl/blob/master/grbl/planner.c#L446, the key is junction_accelerationwhich is an acceleration value based on used acceleration (we would call it travel or print acceleration) and the junction_unit_vec. What they do is to calculate an acceleration which will not violate the acceleration limit per axis. For Z, which has the low jerk problem, we also use low accelerations. I checked that on my machines and the example configs inside Marlin. So, a low max jerk means also a low max acceleration on an axis. Given that, a lower junction_accelerationand therefore block->max_junction_speed_sqr is the result.

In the Marlin implementation, we use a fixed JUNCTION_ACCELERATION. This completely ignores the axis acceleration limits!

Should be easy to fix, lets see if I can patch it..

Squid116 commented 6 years ago

@sebastianv650 there is a hint in the comments in the main fork of the grbl that it should take the minimum acceleration of the two blocks, I think you might need this to cover off both sides of this problem.

Squid116 commented 6 years ago

I am completely on board with @Sebastianv650's suggestion of using the actual junction acceleration (I never really understood why we moved to a fixed value). In normal movement this means that junction deviation will factor a heavy slow moving accelerating axis (y) in the calculations. This would mean that the nonagon example (https://github.com/MarlinFirmware/Marlin/issues/10341#issuecomment-391051458) might not always produce the same junction speed - a move with more acceleration on a slower accelerating axis (i.e. mostly y) should slow down more.

However, I'm not completely convinced that it will solve this problem completely. Could it be as simple as hard setting the junction speed to 0 for E or Z only moves (with an option to not do this on Z for our CNC friends)? This method could also be applied to both Jerk and Junction Deviation solving the problem in both scenarios.

Edit: If done, how would setting a 0 junction speed work (or not work) for the z-hop in fwretract?

Sebastianv650 commented 6 years ago

@Squid116

there is a hint in the comments in the main fork of the grbl that it should take the minimum acceleration of the two blocks.

I read that, but this note is no longer present in the recent 1.1 GRBL code with the use of junction_acceleration. But I have that in mind to try it out. Basicaly it should make

Could it be as simple as hard setting the junction speed to 0 for E or Z only moves

not necessary.

I got the junction_acceleration to work yesterday and I will create a PR for testing purposes in the next minutes or hours depending on my test results satisfaction.

Squid116 commented 6 years ago

But I have that in mind to try it out.

I think it would really be needed in order to get this working in all cases. Take the heavy Y bed as an example, if you go from a move along Y to a 90deg corner travelling along X, then this is only going to consider the acceleration on X for this junction, not the lower deceleration needed for stopping Y. The result will be a faster junction speed than Y can sensibly handle.

Sebastianv650 commented 6 years ago

That's the best-looking junction ("jerk") speed result I have ever seen from Marlin! Now we head into the right direction 👍

So what do we see here: It's my small junction speed test code using my patched JUNCTION_DEVIATION code. On the left side, you can see the gcode lines colored into three sections.

This gcode can also be used for Z-axis test if you replace the limit values of the E axis in Marlin by values for Z. On the right two collumns you see the initial and final speed of each move. The acceleration limits for each axis are 3000mm/s² for X and Y, 200mm/s² for E. The print and travel acceleration is set to 1000mm/s² and JUNCTION_DEVIATION_MMis set to 0.02mm.

Things that can be checked due to this result:

jerktest

Sebastianv650 commented 6 years ago

@Squid116

Take the heavy Y bed as an example, if you go from a move along Y to a 90deg corner travelling along X, then this is only going to consider the acceleration on X for this junction, not the lower deceleration needed for stopping Y. The result will be a faster junction speed than Y can sensibly handle.

Not any longer with the new jerk code. I will create a quick and dirty test-PR next so we can test it.

Squid116 commented 6 years ago

Awesome work - I look forward to testing it out this evening. :)

Squid116 commented 6 years ago

Not any longer with the new jerk code.

Looking at the PR, you are 100% correct - the junction_unit_vec factors in the previous block so all should be 👍 .

Squid116 commented 6 years ago

Just thinking, now that junction acceleration is set proportional to the junction vector, do we still need the special case for blocks < 1mm? It cant hurt the output as it always takes the minimum, but is it necessary to add these extra calculations on small moves?

Sebastianv650 commented 6 years ago

I wasn't looking at the <1mm code section. What was the intention for that special handling? The code change I did in the PR shoudln't alter the handling of short moves, as all vectors are always unit vectors, therefore their length is always 1.

Squid116 commented 6 years ago

The special case was for a 90 degree corner that was broken up by the slicer into several smaller turns, with a fixed junction_acceleration (i.e. not proportional to the vector) it was basically slamming into a 90 degree corner at full speed.

Squid116 commented 6 years ago

Here is where the 'special' case was devised: https://github.com/MarlinFirmware/Marlin/issues/10341#issuecomment-387922944

Sebastianv650 commented 6 years ago

If we have a very small fillet so that a 90° corner is split into some small segments as you described it, this behaviour will not be change by my PR.

On the other hand, this cornering at full speed is quite fine according to the theory and also handled like this by the original Marlin jerk code. Many small segments result in tiny angular deviations from segment to segment, therefore no need to slow down. Even if this segments form a radius of 0.001mm in reality.

So maybe if we slow that down in JUNCTION_DEVIATION code, the same has to be applied to original jerk code. Or we handle it not at all, as we did all the years in the past. The centrifugal force should be limited by the acceleration setting even without that <1mm thing..?

Or in other words, I'm asking myself if that's only a problem due to an improper jerk setting.

thinkyhead commented 6 years ago

If we have enough look-ahead we should be able to discern that an acute angle is formed by two or more very short moves. But also, Marlin will skip moves that are so small as to be pointless. With Junction Deviation enabled, perhaps it makes sense to simply skip any moves under —let's say— 0.125mm, and just let these tiny moves get merged into one less-tiny move.

Squid116 commented 6 years ago

It would be good if @hoffbaked couldn't this new change both with and without his special case for short segments - he implemented this due to problems he encountered in this scenario with a fixed acceleration.

I should have some time to get a closer look tonight, and run a few tests, but I never had the problem hoffbaked saw, mind you I only tested a) using block->acceleration and b) JUNCTION_ACCELERATION with the special case handling.

hoffbaked commented 6 years ago

I’ll take a closer look in a few hours, but the issue I was running into was a rounded corner with sides about 1/10th of a mm and an angle approaching 180 at the junction. It added up to a 90 degree turn over the course of less than a mm, but the normal junction code perceived it as a nearly straight junction and didn’t slow down at all.

hoffbaked commented 6 years ago

Okay, so that looks like a good way to go, but it seems like there's a problem, if I'm reading this right. They're subtracting the two vectors and then calculating the acceleration based on those proportions. Wouldn't it make a lot more sense to use:

float junction_acceleration = MIN(
   limit_value_by_axis_maximum(settings.acceleration, unit_vec),
   limit_value_by_axis_maximum(settings.acceleration, pl.previous_unit_vec)
);

I'm assuming limit_value_by_axis_maximum uses the movement proportions and the axis max acceleration.

With subtracting the two vectors, it seems like a long move and then a z hop wouldn't correctly limit to the z acceleration value since it's a way shorter move. If I'm thinking about this right, it seems like you'd need to consider the two moves independently.

And we would definitely still need to check for rounded corners. The grbl code handled it by just slowing down as the line segments got shorter in a completely different section of code.

thinkyhead commented 6 years ago

I'm not sure whether it's better or not. Something Grbl missed?

And, this might be quicker than calling limit_value_by_axis_maximum twice…

float junction_acceleration = limit_value_by_axis_maximum(
  settings.acceleration,
  MIN(unit_vec, previous_unit_vec)
);
thinkyhead commented 6 years ago

The grbl code handled it by just slowing down as the line segments got shorter

Interesting approach. Since we have a lot of short segments, due to kinematics and leveling, this might cause some weird side-effects.

hoffbaked commented 6 years ago

I'm assuming that the function finds the proportionate maximum for the vector, such that a primarily Z move will return primarily the Z limit? Even if the Z hop is half a mm, it still should be setting the speed limit for that particular junction, I would think.

Sebastianv650 commented 6 years ago

@hoffbaked can you link to the GRBL which causes short segments to slow down?

It seems like a long move and then a z hop wouldn't use the z acceleration value since it's way shorter of a move.

As we are talking about unit vectors, I don't think so. If we have a 100mm move along X, followed by a 0.5mm Z hop, the vectors are [1,0,0] and [0,0,1]. The length isn't important at all, only the direction of the move.

I would be very careful now making changes compared to the GRBL code. We now have a working junction speed code the first time since I started to use Marlin, we shouldn't screw this up again. So, every change should be proved by numbers, not something like "it seems to move right".

Squid116 commented 6 years ago

@hoffbaked the junction_unit_vec accounts for the previous block - so you don't need to do the minimum of the two blocks.

hoffbaked commented 6 years ago

I thought the vectors were of the deltas? Even so, their code looks weird. If you have a 10mm Z move, followed by another 10mm Z move with 1mm X, the Z moves get subtracted out, but it seems like X would win.

Sebastianv650 commented 6 years ago

@hoffbaked yes X "wins" here, and that's perfectly fine as we only have a change along X in your example. Let's go through it: The unit vector of the first move is [0, 0, 1]. The second move is [0.1, 0, 0.995]. The junction vector is the delta between the two: 0.1 - 0 = 0.1 0 - 0 = 0 0.995 - 1 = -0.005 and then normalized, so the vectors lenth is 1: [0.995, 0, -0.05]. This is now used to check against axis acceleration limits. It's basicaly the same thing Marlin already does inside the planner when it checks if a move has to be executed with travel or print acceleration, where it checks each axis after that to see if the acceleration has to be lowered due to some slow axis. Just done by vectors instead.

hoffbaked commented 6 years ago

Okay, the above scenario is fine, but it still seems like the junction speed on the long movement, Z hop scenario is not going to take the Z max acceleration into account, and the junction speed will be higher than intended. It'll be treated as a 90 degree corner using the maximum acceleration value of the long move.

hoffbaked commented 6 years ago

The vectors are in millimeters, and are not normalized. You can see they're used above to calculate the block->millimeters.

Sebastianv650 commented 6 years ago

@hoffbaked they are calculated and normalized in this step: https://github.com/MarlinFirmware/Marlin/blob/bugfix-1.1.x/Marlin/planner.cpp#L2123

hoffbaked commented 6 years ago

Oh, jeez. Thanks!! I missed that. I guess it at least would end up the average of the two maxes on a Z hop, which is still not ideal, but it would at least make a dent.