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.06k stars 19.16k forks source link

Homing & Leveling Bugs #2040

Closed thinkyhead closed 8 years ago

thinkyhead commented 9 years ago

We've gotten rid of a lot of little bugs in homing and leveling lately, but the current Development code still has an obscure Z positioning problem with G29. So, I've narrowed down the current leveling issues to a small handful of recent changes, and rolled back the leveling code to around March 26 at the branch https://github.com/thinkyhead/Marlin/tree/before_delta_merge

The differences between that code and the current leveling / homing code are embodied in commit https://github.com/thinkyhead/Marlin/commit/3f0a990360d04a45c11ba6ebdfeff2b16d26bf34 One of the changes in this commit is almost certainly responsible for the current weird behavior.

So, I suggest starting with before_delta_merge to see if it behaves properly. Then we can try to determine which of those recent changes is responsible for any remaining problems.

Once these leveling bugs are squashed, we can close all these issues:

1507 #1833 #1844 #1894 #1701 #1730 #1731 #1737

emartinez167 commented 9 years ago

Sounds like a plan! Ready for testing when you release it!

Regards,

Ernesto

On 9 May 2015, at 12:44, Scott Lahteine notifications@github.com wrote:

We've gotten rid of a lot of little bugs in homing and leveling lately, but the current Development code still has an obscure Z positioning problem with G29. But I've narrowed down the current leveling issues to a small handful of recent changes, and rolled back the leveling code to around March 26 at the branch https://github.com/thinkyhead/Marlin/tree/before_delta_merge

The differences between that code and the current leveling / homing code are embodied in commit thinkyhead@3f0a990 One of the changes in this commit is almost certainly responsible for the current weird behavior.

So, I suggest starting with before_delta_merge to see if it behaves properly. Then we can try to determine which of those recent changes is responsible for any remaining problems.

Once these leveling bugs are squashed, we can close all these issues:

1507 #1833 #1844 #1856 #1894 #1701 #1730 #1731 #1737

— Reply to this email directly or view it on GitHub.

TechMasterJoe commented 9 years ago

think do you want me to test this and start swapping old and new looking for the problem if so i can start on sunday when my ABL machine is done printing

thinkyhead commented 9 years ago

@TechMasterJoe That would be great. I'm going to try to do some testing with @boelle tomorrow during our hangout too. Hopefully that will narrow down the root cause, and we'll have something better by Sunday.

boelle commented 9 years ago

me just got up.... let me get my oat meal and then i will wire up the rest.... i only managed to get one end near extruder done... @thinkyhead i plan to attach an raspberry pi and i could load octo print on it if that helps anything?

boelle commented 9 years ago

since i closed my conversion from old marlin i thought i would use it to tell me discoveries with getting auto level running: https://github.com/MarlinFirmware/Marlin/issues/1979

Sniffle commented 9 years ago

So I have been playing with the before_delta_merge branch some. Trying to get it to a somewhat usable state so that i can actually print with it while using bed compensation. I am running into an issue that might give a clue as to where the problem may lie in the arbitrary offset values.

Now to explain.

When setting the offset lower to account for the space left after G28/G29/G1 X150 Y150 Z0

I noticed that as i moved the offset down the probe when deploying(for the first probe) became closer and closer to the bed until the probe was unable to deploy and would just hit the bed and then endlessly try to move Z down.

So as a remedy to this problem, I increased my Z_Raise_before_Probing, which then allowed my probe to deploy, but as a result left my hotend dangling in the air once again after the above sequence.

I realize this may or may not help, but it is definitely a bug that prevents the extruder from lowering to the proper height, and unless your Z probe offset is 0 or your raise before probing is 0, you can never make it be correct it becomes an infinite loop of adjusting trying to compensate for the offset and the raise height.

Roxy-3D commented 9 years ago

A short while ago ThinkyHead was considering changing the Z_RAISE_xxx_xxx items to absolute numbers. If instead we had stuff like Z_HEIGHT_BEFORE_PROBING and Z_HEIGHT_BETWEEN_PROBES I think all of this crazyness would go away. Probably this air printing problem is happening because an extra Z_RAISE_xxx_xxx is getting done when it shouldn't.

It might be worthwhile to make a quick branch and see how hard it is to convert over. Going forward, it would be a lot simpler to keep the code working too.

Sniffle commented 9 years ago

You need to be able to adjust the number because if the set number is less than your probe offset then you run into problems... but it could probably be Z-Probe_Offset +5 and call it a day

Roxy-3D commented 9 years ago

You need to be able to adjust the number because if the set number is less than your probe offset then you run into problems... but it could probably be Z-Probe_Offset +5 and call it a day

I was trying to suggest that a different mindset on how the numbers are communicated to the firmware might resolve some of these issues. If we had 3 or 4 Z_HEIGHT_at_some_event we could probably define how we want things to behave. And in that mindset, if an extra movement got initiated that didn't really need to be done, it wouldn't matter because it would just go to the right place.

Sniffle commented 9 years ago

I'm actually about to try replacing before between and after with Z_probe_offset+5 and see how it works. I already have the code changes done, just gotta flash it and see what happens :-)

IF it works, it can be implemented some other way, but it will let us know if going that route would fix things

Sniffle commented 9 years ago

@thinkyhead @Roxy-3DPrintBoard

I am going to pastebin my marlin_main.cpp so that it can be diffed against the current marlin main. and a proper code fix can be implemented. By replacing the before during and after values with a static amount of Z_probe_offset +- a static amount, it has made auto leveling act properly. I hope this helps, I will even take a video of you don't believe me :-P

and just for accuracy's sake, This is from @thinkyhead 's fork of marlin and the Before_delta_merge branch, having been pulled as of this morning.

http://pastebin.com/01sv2vS4

edit:I just noticed that i lost the raise after probing for some reason, I probably changed one too many values that i shouldnt have...

edit2: as a thought on how to implement to a single arbitrary value, before dueing and after could become booleans with a single value being the amount added or subtracted from the Z offset.

edit3: After a little more fiddling with numbers jsut to get things to act the way i wanted, It is printing beautifully :-)

thinkyhead commented 9 years ago

Thanks for keeping on this… Reading through these posts now…

thinkyhead commented 9 years ago

as a remedy to this problem, I increased my Z_RAISE_BEFORE_PROBING

Good, that is what it's for, even though the name doesn't make it clear.

which then allowed my probe to deploy, but as a result left my hotend dangling in the air…

Unless you see a raise happening to some height that is wrong, the problem is most likely that after probing the Z coordinate is left set to 0 (or rather, -zprobe_zoffset). But… are there any Z_RAISE_BEFORE_PROBING values which don't cause this to happen?

Actually, I'm not sure how Z_RAISE_BEFORE_PROBING would have an effect in that scenario.

@Sniffle How is the current Development HEAD code behaving? Is it as bad as it was before? I can use the debug_leveling branch that I still have synchronized with Development to try things out that are suggested by the before_delta_merge branch.

thinkyhead commented 9 years ago

@Sniffle Still catching up on your changes – downloaded and comparing…

thinkyhead commented 9 years ago

Ok, so I see what you did there. It's a different constant from the Z_RAISE value. So, in that case what are the values of Z_RAISE_BEFORE_PROBING and Z_PROBE_OFFSET_FROM_EXTRUDER? Others have thrown in "magic numbers" before in order to make it work, but all it indicates is that the error is constant, and therefore something to do with bad logic. At least we can say it is consistent.

So that's interesting. It means that something I changed – or haven't changed yet – is not so innocuous. My original hope was to have before_delta_merge behave as the code actually did on its origin date, but that's also an option if you want to test that… What bugs existed (in leveling only) on the date of origin? To find out, try this branch https://github.com/thinkyhead/Marlin/tree/wayback_machine

Sniffle commented 9 years ago

I also has to change a symbol because it wasnt acting right... I had to subtract instead of add. Btw. I didnt add a magic number. I literally added 5 to the offset so that it would consistently add 5mm to the offset so that a single number could work. Where you saw 10 is where i just went through where the code already had + 5 and combined. On May 12, 2015 5:14 AM, "Scott Lahteine" notifications@github.com wrote:

Ok, so I see what you did there. It's a different constant from the Z_RAISE value. So, in that case what are the values of Z_RAISE_BEFORE_PROBING and Z_PROBE_OFFSET_FROM_EXTRUDER? Others have thrown in "magic numbers" before in order to make it work, but all it indicates is that the error is constant, and therefore something to do with bad logic. At least we can say it is consistent.

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/2040#issuecomment-101222355 .

Sniffle commented 9 years ago

my raise_before_probing is 15 from where i left it trying to get the before delta merge working.

my offset from extruder is 4.7, which is withing a reasonable level of my original measured offset(my dual extruder isnt level right now)

Even if things aren't quite right this is a basis to compare with the current branch as far as the before during and after code.

I'll test this theory with the current devel branch this evening I have to get some printing done first.

lrpirlet commented 9 years ago

@thinkyhead https://github.com/thinkyhead In the base note you give a pointer: thinkyhead@3f0a990 https://github.com/thinkyhead/Marlin/commit/3f0a990360d04a45c11ba6ebdfeff2b16d26bf34 that list the differences between a working G29 on my Prusa i3 under before_delta_merge and a non working G29 under 1.0.3 dev... The question: is any of the block of instruction depending from another block? In other word, could I take before_delta_merge and apply one block of instructions at a time until it fails to discover the "broken" part? (I am trying to define the risk factor... I do not care about misbehaviour, but I would not like to destroy something in my hardware by lack of understanding...)

2015-05-12 16:11 GMT+02:00 Todd Swindoll notifications@github.com:

my raise_before_probing is 15 from where i left it trying to get the before delta merge working.

my offset from extruder is 4.7, which is withing a reasonable level of my original measured offset(my dual extruder isnt level right now)

Even if things aren't quite right this is a basis to compare with the current branch as far as the before during and after code.

I'll test this theory with the current devel branch this evening I have to get some printing done first.

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/2040#issuecomment-101296229 .

thinkyhead commented 9 years ago

In the base note you give a pointer: thinkyhead@3f0a990

@lrpirlet The changes at 1044 and 2056 go together. And those at 1117 and 1148 go together. The main risk is the nozzle trying to move too low (if that were to happen) which could mess up or break a bed. It sounds like you are getting closer.

If the problem persists, then the best thing to do will be to use that diff as a guide to work backward from the current code (or, better, the debug_leveling branch) to see the effects of undoing the changes that it describes as having been already made.

Again, my hope was that the code at this point in time: wayback_machine would exhibit no significant anomalies, and make a better basis for comparison. So that would be another thing to check. Maybe we need to go back a week or two earlier.

lrpirlet commented 9 years ago

@thinkyhead https://github.com/thinkyhead I have examined both before_delta_merge https://github.com/thinkyhead/Marlin/tree/before_delta_merge and _waybackmachine... To my surprise they behave diffenrently. So before_delta_merge https://github.com/thinkyhead/Marlin/tree/before_delta_merge does give a good G28, a good G29 and a wrong probe behaviour, while wayback_machine does give a good G28, a wrong G29 and a good probe handling.

At first I though it would be easy... How wrong was I... several factors seems to interact...

First of all, the plan generated by G29 is always parallel to the bed. this is a good new. Then the height between the physical bed and the generated plan is variable... It even depend on the "horizontality" of the bed... so a bed more tilted will make the height more important. I discovered that the only way to keep the height constant whatever the slope of the bed is to keep the lines 1117 and 1148: ( current_position[Z_AXIS] = zprobe_zoffset; ) . As I understand it, this is the only place where we "connect" the generated plane with the physical bed...

In before_delta_merge https://github.com/thinkyhead/Marlin/tree/before_delta_merge the "connection" between the generated plane and the bed is (?almost?) perfect. The generated plane and the physical bed are in the same plane... Adding any other probe move in the loop such as lifting the hotend between probe does modify the "connection" distance...

Here I found that the lifting between probe routine does not apply for the first probe (I am using grid probe... invoking G29 V4 P3 T), I guess the routine to lift between probe should be moved around.

However, I think we first need to correct the value in line 1117 and 1148 to take care of the probe position before to execute sync_plan_position() that makes the "connection". I am trying to understand the code, because I do not know if we have to use the "real" value or the "corrected" value...

I hope this helps.

2015-05-13 10:44 GMT+02:00 Scott Lahteine notifications@github.com:

In the base note you give a pointer: thinkyhead@3f0a990 https://github.com/thinkyhead/Marlin/commit/3f0a990

@lrpirlet https://github.com/lrpirlet The changes at 1044 and 2056 go together. And those at 1117 and 1148 go together. The main risk is the nozzle trying to move too low (if that were to happen) which could mess up or break a bed. The best thing to do is perhaps use that diff as a guide to work backward from the current code (or, better, the debug_leveling branch) to see the effects of undoing the changes that it describes as having bee already made.

Again, my hope was that the code at this point in time: https://github.com/thinkyhead/Marlin/tree/wayback_machine) would exhibit no significant anomalies, and make a better basis for comparison. So that would be the first thing to check. Maybe we need to go back a week or two earlier.

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/2040#issuecomment-101576609 .

lrpirlet commented 9 years ago

@thinkyhead Ok, I think I made it work now... You want in set_bed_level_equation_lsq() and in set_bed_level_equation_3pts() the following code...

//lrp1123 with that line removed we just create a plan parallel to the bed... without fixing a common point between the parallel plan and the bed plan
//lrp this line should move the parallel plan OVER the bed plan... 
//lrp do NOT add Z_RAISE_BETWEEN_PROBINGS to current position while enabling Z_RAISE_BETWEEN_PROBINGS since it does NOT get added to the last probe -the one that counts-
//lrp do add Z_RAISE_AFTER_PROBING to current position while enabling Z_RAISE_AFTER_PROBING since it does get added to the last probe -the one that counts-. Another solution would be to remove the z move from within stow_z_probe()
        current_position[Z_AXIS] = zprobe_zoffset + Z_RAISE_AFTER_PROBING; 

just before the

sync_plan_position();

Note that Z_RAISE_AFTER_PROBING = 0 if not defined so it can be there all the time... but care should be taken is someone adds a Z move that could find its way into the G29 loop and be applied on the last probe...

I have tried this modification in an hybrid off before_delta_merge and wayback_machine... G28 works, G29 works and probe works as expected... Tomorow, I download the current développement and try the modif...

Wurstnase commented 9 years ago

When you fix this issue, i will give you a small medal of honor ;)

thinkyhead commented 9 years ago

Nice! I will try this and hand it to another tester too. I was trying another trick, adding sync_plan_position at the top of probe_pt with limited success. The z correction after setting the plane equation would seem to be important though!

lrpirlet commented 9 years ago

@thinkyhead Ok, I had not all the time I wanted... But could test the following code in 1.0.3 int.... In one word: it works (that is 2 by the way :-) )

Note I only tested the GRID ABL.. I had no time for 3 probes... Now this marlin_main.cpp is so similar to the wayback_machine that I do not expect any problems... I did try but failed to correct the distance... Give me another 5 hours to understand and I may fix that... please fix it if you knows how to...

      static void set_bed_level_equation_lsq(double *plane_equation_coefficients) {

        vector_3 planeNormal = vector_3(-plane_equation_coefficients[0], -plane_equation_coefficients[1], 1);
        planeNormal.debug("planeNormal");
        plan_bed_level_matrix = matrix_3x3::create_look_at(planeNormal);
        //bedLevel.debug("bedLevel");

        //plan_bed_level_matrix.debug("bed level before");
        //vector_3 uncorrected_position = plan_get_position_mm();
        //uncorrected_position.debug("position before");

        vector_3 corrected_position = plan_get_position();
        //corrected_position.debug("position after");
        current_position[X_AXIS] = corrected_position.x;
        current_position[Y_AXIS] = corrected_position.y;
        current_position[Z_AXIS] = corrected_position.z;
//lrp1123 with that line removed we just create a plan parallel to the bed... without fixing a common point between the parallel plan and the bed plan
//lrp this line should move the parallel plan OVER the bed plan... We know that the bed and hot-end are at zprobe_zoffset when the probe click on the bed
//lrp do NOT add Z_RAISE_BETWEEN_PROBINGS to current position while enabling Z_RAISE_BETWEEN_PROBINGS since it does NOT get added to the last probe -the one that counts-
//lrp do add Z_RAISE_AFTER_PROBING to current position while enabling Z_RAISE_AFTER_PROBING since it does get added to the last probe -the one that counts-. 
//lrp finally we MAY want to correct the calculated distance for the coordinates used now (but how??? sorry beyond my current understanding)
        current_position[Z_AXIS] = zprobe_zoffset + Z_RAISE_AFTER_PROBING;
        sync_plan_position();
      }

    #endif // !DELTA

  #else // !AUTO_BED_LEVELING_GRID

    static void set_bed_level_equation_3pts(float z_at_pt_1, float z_at_pt_2, float z_at_pt_3) {

      plan_bed_level_matrix.set_to_identity();

      vector_3 pt1 = vector_3(ABL_PROBE_PT_1_X, ABL_PROBE_PT_1_Y, z_at_pt_1);
      vector_3 pt2 = vector_3(ABL_PROBE_PT_2_X, ABL_PROBE_PT_2_Y, z_at_pt_2);
      vector_3 pt3 = vector_3(ABL_PROBE_PT_3_X, ABL_PROBE_PT_3_Y, z_at_pt_3);
      vector_3 planeNormal = vector_3::cross(pt1 - pt2, pt3 - pt2).get_normal();

      if (planeNormal.z < 0) {
        planeNormal.x = -planeNormal.x;
        planeNormal.y = -planeNormal.y;
        planeNormal.z = -planeNormal.z;
      }

      plan_bed_level_matrix = matrix_3x3::create_look_at(planeNormal);

      vector_3 corrected_position = plan_get_position();
      current_position[X_AXIS] = corrected_position.x;
      current_position[Y_AXIS] = corrected_position.y;
      current_position[Z_AXIS] = corrected_position.z;
//lrp1154 with that line removed we just create a plan parallel to the bed... without fixing a common point between the parallel plan and the bed plan
//lrp this line should move the parallel plan OVER the bed plan... We know that the bed and hot-end are at zprobe_zoffset when the probe click on the bed then
//lrp do NOT add Z_RAISE_BETWEEN_PROBINGS to current position while enabling Z_RAISE_BETWEEN_PROBINGS since it does NOT get added to the last probe -the one that counts- then
//lrp do add Z_RAISE_AFTER_PROBING to current position while enabling Z_RAISE_AFTER_PROBING since it does get added to the last probe -the one that counts-. 
//lrp finally we MAY want to correct the calculated distance for the coordinates used now (but how??? sorry beyond my current understanding)
      current_position[Z_AXIS] = zprobe_zoffset + Z_RAISE_AFTER_PROBING;

      sync_plan_position();
    }
Wurstnase commented 9 years ago

Should it not be

current_position[Z_AXIS] = corrected_position.z + zprobe_zoffset + Z_RAISE_AFTER_PROBING;

else you just overwrite current_position[Z_AXIS].

lrpirlet commented 9 years ago

@Wurstnase, to ovewrite current_position[Z_AXIS] .is exactly what needs to be done... In fact current_position[Z_AXIS] is quite random as the very first distance is NOT KNOWN... Remember you can enter G29 without having defined the Z zero position. The tested condition to execute G29 is known position of X and Y (G28 X Y answer the test)

Wurstnase commented 9 years ago

ok, just confused me. So you should probably comment the first current_position[Z_AXIS]?!?

But the current_position[Z_AXIS] should be known because you need a G28 Z before homing.

lrpirlet commented 9 years ago

Yes, you are right, it would cut some useless code (that would probably be optimized out by the compiler)... However, this code is not complete... As I said in my last comment I read C++ very painfully. I think the correction I propose is valid for a SMALL slope under 1-2%... To be mathematically correct, I should apply a rotation correction on my calculated current_position[Z_AXIS] BUT I lack the knowledge presently...

Wurstnase commented 9 years ago

corrected_position.z should be a small number. So this should be the part for the bigger slope.

When I get my leveling back to working I can start with testing also.

Could you SERIAL_PRINT the corrected_position.x to .z?

Wurstnase commented 9 years ago

Or is corrected_postition.z something close to the offset? So maybe current_position[Z_AXIS] = corrected_position.z + Z_RAISE_AFTER_PROBING; is right?!?

nophead commented 9 years ago

corrected_postition.z is the Z position corrected for the tilt but not the offset. The offset should come from the plane equation but in Marlin it seems to come from the last point to be probed. That will be the same if the bed is perfectly flat but inclined. If not I think the plane equation is much better as it is derived from all the points.

See http://hydraraptor.blogspot.co.uk/2011/04/auto-bed-leveling.html for the correct maths with respect to the plane equation.

thinkyhead commented 9 years ago

corrected_postition.z is the Z position corrected for the tilt but not the offset

@lrpirlet @Wurstnase @nophead

When these bed equation functions are called, it's assumed by them that the last probe was just completed, and where is Z at this point? Is it at the zprobe_zoffset height with the probe still touching the bed? Or has it already moved up by Z_RAISE_AFTER_PROBING? (Today it should be the latter.)

In either case, overwriting the current Z with a "corrected" position makes the Z position no longer match its true physical distance from the bed. So, if you were to send Z to 0 right away, it will not end up where it should.

That makes me realize something! Shouldn't the carriage do one more move at the end of probing? It should move so that the nozzle XY is where the probe XY used to be. Then it will be at the correct XY for the measured (and current) Z.

Sniffle commented 9 years ago

so, if my thinking of your logic is correct, we should raise after probing to retract the probe then lower back down the same distance to get the positioning correct for where the Z position is expected to be for when corrected_position.z is called.

or did i miss something?

On Wed, May 20, 2015 at 9:40 PM, Scott Lahteine notifications@github.com wrote:

corrected_postition.z is the Z position corrected for the tilt but not the offset

When these bed equation functions are called, it's assumed by them that the last probe was just completed, and where is Z at this point? Is it at the zprobe_zoffset height with the probe still touching the bed? Or has it already moved up by Z_RAISE_AFTER_PROBING? (Today it should be the latter.)

In either case, overwriting the current Z with a "corrected" position makes the Z position no longer match its true physical distance from the bed. So, if you send Z to 0 right away, it will not end up where it should.

That makes me realize something! Shouldn't the carriage do one more move at the end of probing? It should move so that the nozzle XY is where the probe XY used to be. Then it will be at the correct XY for the measured Z.

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/2040#issuecomment-104106260 .

thinkyhead commented 9 years ago

@Sniffle I'd be interested to see what the difference would be in that case. Even though we've both talked about moving the actual carriage, I'm sure these things can also be accomplished by some clever mathematics.

Sniffle commented 9 years ago

here's a thought... please keep in mind i'm just spit balling but we know that the planner and calculations are expecting the nozzle to be at the offset position after the last point is probed, and the raise after probing is throwing that off because we are too high and then the corrected position is throwing it off even further once the matrix is applied to the position... So... what if we changed the Z_Probe_offset = Z_Probe_offset+Z_Raise_after_probing, in a behind the scenes way....

would that not correct where the planner is expecting the probe, and correct the height positioning of everything?

On Wed, May 20, 2015 at 11:45 PM, Scott Lahteine notifications@github.com wrote:

@Sniffle https://github.com/Sniffle I'd be interested to see what the difference would be in that case. Even though we've both talked about moving the actual carriage, I'm sure these things can also be accomplished by some clever mathematics.

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/2040#issuecomment-104135064 .

thinkyhead commented 9 years ago

@Sniffle No, we don't want to do that. The probe offset is a high-level concept. The planner is told by the higher-level code where it is now and where it should go, relative to that. Sometimes a position is set to 0 and the planner is told this, just as a setup for a move.

I think we can probably solve the leveling inaccuracy just by applying the right offset, without having to do any kludges like modifying the zprobe_zoffset. I'm going to apply the recommended patch to my debug_leveling branch and post it for testing soon.

thinkyhead commented 9 years ago

Here, try this one… https://github.com/thinkyhead/Marlin/tree/debug_leveling_set_z

It uses this line to get the current Z:

current_position[Z_AXIS] = 0 + home_offset[Z_AXIS] + zprobe_zoffset + Z_RAISE_AFTER_PROBING; // The true distance from the bed (adjusted by home_offset).
lrpirlet commented 9 years ago

@thinkyhead When I talk about a corrected distance, I mean that the distance calculated: current_position[Z_AXIS] = 0 + home_offset[Z_AXIS] + zprobe_zoffset + Z_RAISE_AFTER_PROBING; should be corrected by a very small amount...

A right triangle whose sides are Delta X (DX) and Delta Y (DY) and the hypotenuse (DXY) the side that describe both the axis of rotation and the tip shift distance in the xy plane... (Delta being the difference between the G1 argument and the calculated position)

the relationship between the three sides is DX² + DY² = DXY²

If I look now at a right angles to the axis of rotation, the physical distance PD is the hypotenuse of a triangle with DXY² one side and the corrected distance CD the other side...

So for this triangle: CD² + DXY² = PD² = CD² + DX² + DY²...

The corrected distance is therefore CD = sqrt (PD² - DX² - DY²)...

The difference between CD and PD should stay under 0.15 mm (Arbitrary choice I made)... We acheive that by keeping current_position[Z_AXIS] = 0 + home_offset[Z_AXIS] + zprobe_zoffset + Z_RAISE_AFTER_PROBING; under 20 mm and the slope under 2-3 %...

To make a slope of 30 degres so that one can print a 60 degre hangout without support WILL FAIL... This is a WORKING APPROXIMATION TO CORRECT THE BED SLOPE... NO MORE

You have been warned... (But I am sure to see some folk demonstrating otherwize on Utube LOL)

Roxy-3D commented 9 years ago

lrpirlet commented 37 minutes ago So for this triangle: CD² + DXY² = PD² = CD² + DX² + DY²... The corrected distance is therefore CD = sqrt (PD² - DX² - DY²)...

In simple words... You are pointing out that the Z_PROBE_OFFSET is a first order approximation of the needed correction. The problem is, the nozzle is not perfectly normal to the bed plane and that is going to affect how much of the Z_PROBE_OFFSET really needs to be applied.

In a radically tilted bed, you would be able to see (and measure) the Z_PROBE_OFFSET and also see at a normal to the bed plane the distance is actually less than Z_PROBE_OFFSET.

The early G29 code did correct the nozzle height based on the bed tilt. But somehow that thinking was lost.

(And this ignores the fact that the X_PROBE_OFFSET and Y_PROBE_OFFSET factor into the calculation! The distance the probe is from the nozzle start to factor into the correction as it becomes non-zero.)

thinkyhead commented 9 years ago

somehow that thinking was lost

Not lost, just unfortunately obscured by changes attempting to simplify existing code and integrate new delta stuff as well. But the old code can easily be resurrected and compared. Some corrections have already been made by that method.

should be corrected by a very small amount

Let me go through the thought-process once more, and see what shakes out… Feel free to add more items to this list, where I'm trying to state the procedure very simply, without the trig we all know so well:

First, let me define some points on a diagram:

image

A: Probe current XYZ, uncorrected B: Nozzle current XYZ, uncorrected. Same as current_position. C: Probe XY, Nozzle Z D: Probe XY, Z touching the bed E: Nozzle XY, Z touching the bed (Sorry, line should've been drawn from C to G here) F: Nozzle corrected XY, Z touching the bed G: Probe corrected XY, Z touching the bed

And now the lines:

AD: Should be the same as Z_RAISE_AFTER_PROBING CD: Z_RAISE_AFTER_PROBING - zprobe_zoffset AG: Bed-perpendicular line from probe to its "real" XY BF: Bed-perpendicular line from the nozzle to its "real" XY

So, you can see part of the problem. The Z position of the nozzle is relevant at the probe XY, but not at the current_position[X/Y]. If the nozzle was moved to the probe XY, that XYZ could be used as-is in the inverse-transform. To get the corrected nozzle position without this move is trickier, but could be done if it were possible to ask the matrix the Z difference for any given XY difference.

lrpirlet commented 9 years ago

@Roxy-3DPrintBoard @thinkyhead

Sorry, I must have been confusing everyone now... I did miss part of my explanation... (I can think in English in my job, I just revert to my French mother language when using my old school knowledge ;-) )

Thanks both, reading your comment made me think... When considering the bed and the end corrected plan, I may see it as follow. Visualize the YX plan... a parallel plan intersect the bed plan at a "mean" distance from all the point that makes the matrix. (whatever definition of "mean"... I could not find it in my reverse engineering... ?yet?)

At any point exists a Delta Z due to the tilt of the bed along a line parallel to the X axis... call that DX.

At any point exists a Delta Z due to the tilt of the bed along a line parallel to the Y axis... call that DY.

Obviously, there must be ONE point on the bed where the DX and the DY value is equal to zero... let us call that point the rotation point... I GUESS that at this point, the calculated Z height (via the matrix formula) would be corresponding to the physical height...

Unluckily, my C++ skills are way too small to even try program it in a reasonable time...

Sniffle commented 9 years ago

@thinkyhead I'll be testing debug_leveling_set_z over the weekend, I'll report back

Roxy-3D commented 9 years ago

Sorry, I must have been confusing everyone now... I did miss part of my explanation... (I can think in English in my job, I just revert to my French mother language when using my old school knowledge ;-) )

I thought your explanation was very good! I like all the math and formulas. It helps define exactly what is going on. I was actually worried you would not like me 're-interpreting' what you said in normal English. The whole reason I did that was so everybody else could benefit from what you were thinking. I believe what you said is exactly correct.

So, you can see part of the problem. The Z position of the nozzle is relevant at the probe XY, but not at the current_position[X/Y]. If the nozzle was moved to the probe XY, that XYZ could be used as-is in the inverse-transform. To get the corrected nozzle position without this move is trickier, but could be done if it were possible to ask the matrix the Z difference for any given XY difference.

OK... So I haven't really drilled down on this. But I have a couple of thoughts. First, I do not believe any movement of the nozzle has to take place to have an accurate update of the Z position. The reason I say this is all of the probing is producing a fairly accurate representation of the bed plane. It is producing an accurate 'correction matrix' that describes the slope of the bed plane in both the X and Y direction.

The problem is how much to offset the Z at the end of this. The simplest case is to just use the Z_OFFSET_FROM_PROBE number. Given the bed is pretty level this actually works in many cases. The reason this can work is because of the fact the Limit of cos(w)=1 as w approaches 0. When there is very little bed tilt, the extra error coming from the fact you a measuring something offset from the nozzle doesn't matter. The error in the Z_OFFSET_FROM_PROBE compared to the corrected planes distance from the plane to nozzle are almost identical.

BUT... as you increase the X_OFFSET_FROM_PROBE and Y_OFFSET_FROM_PROBE the error gets magnified. And as you increase the bed tilt that needs to be corrected this error gets further magnified. At some point you need to correct for the bed tilt using all of the parameters.

The old code did it like this. And I believe this is exactly what is in those equations lrpirlet posted and it conforms to the diagram up above that ThinkyHead posted:

        // The following code correct the Z height difference from z-probe position and hotend tip position.
        // The Z height on homing is measured by Z-Probe, but the probe is quite far from the hotend.
        // When the bed is uneven, this height must be corrected.
        real_z = float(st_get_position(Z_AXIS))/axis_steps_per_unit[Z_AXIS];  //get the real Z (since the auto bed leveling is already correcting the plane)
        x_tmp = current_position[X_AXIS] + X_PROBE_OFFSET_FROM_EXTRUDER;
        y_tmp = current_position[Y_AXIS] + Y_PROBE_OFFSET_FROM_EXTRUDER;
        z_tmp = current_position[Z_AXIS];

        apply_rotation_xyz(plan_bed_level_matrix, x_tmp, y_tmp, z_tmp);         //Apply the correction sending the probe offset
        current_position[Z_AXIS] = z_tmp - real_z + current_position[Z_AXIS];   //The difference is added to current position and sent to planner.
        plan_set_position(current_position[X_AXIS], current_position[Y_AXIS], current_position[Z_AXIS], current_position[E_AXIS])
thinkyhead commented 9 years ago

@Roxy-3DPrintBoard The code you've posted is still there. But the correction that seems to be messing things up is the one in set_bed_level_equation_3pts – where it takes the current XYZ and treats it as if it was already rotated, then un-rotates it.

Seeing as that code at the end of G29 is now rotating it again, maybe the correction in set_bed_level_equation_3pts is unnecessary…?

Sniffle commented 9 years ago

@thinkyhead I pulled your "debug_z" branch and it had the I/O bug in it... So I did what any intelligent person would do... I changed the same line in the current Development branch and flashed it with my usual settings… My offset is wrong but… That being said… It definitely was consistently moving the same distance back to the bed once I got ABL to run... I had to tweak the bounding areas for ABL so that it would actually run…

All that being said… I have a video uploading to youtube that sill be done sometime in the night while I'm asleep... I will post a link to it and pastebin my Configuration.h and Marlin_main.cpp so that you can see whether I changed the proper line to mirror the change to your "debug_z" branch…

Now to get back to a working version of Marlin... I've got a lot of printing to do again :-) On May 22, 2015 6:33 PM, "Scott Lahteine" notifications@github.com wrote:

@Roxy-3DPrintBoard https://github.com/Roxy-3DPrintBoard The code you've posted is still there. But the correction that seems to be messing things up is the one in set_bed_level_equation_3pts – where it takes the current XYZ and treats it as if it was already rotated, then un-rotates it.

Seeing as that code at the end of G29 is now rotating it again, maybe the correction in set_bed_level_equation_3pts is unnecessary…?

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/2040#issuecomment-104801699 .

Roxy-3D commented 9 years ago

But the correction that seems to be messing things up is the one in set_bed_level_equation_3pts – where it takes the current XYZ and treats it as if it was already rotated, then un-rotates it.

Seeing as that code at the end of G29 is now rotating it again, maybe the correction in set_bed_level_equation_3pts is unnecessary…?

That code you pointed at is the 3 point bed leveling. Is that the only flavor that has a problem? Because if so, that would be the explanation. You most certainly don't want to correct the Z_OFFSET_FROM_PROBE twice. (I don't think it is being un-rotated. I suspect it is getting rotated twice. But either way, that is wrong.)

Looking through the G29 code, it is obvious it needs to be broken into separate files. Probably only the flavor being used should be pulled in. It would clean things up a lot to have separate files for the 3-Point, n x n Grid and Mesh leveling.

Sniffle commented 9 years ago

@thinkyhead here's the Youtube video of the printer in action through a G28, G29, and G1 X150 Y150 Z0 F5000, as well as my marlin main,cpp, and configuration.h

maybe seeing whts hapening in action will help straighten this mess out... I will say that having the z offset hard coded maded it drop down into place consistently... if i had taken the time to set it properly i feel pretty confident that it would have remained consistent... but there are several actions that in my mind were out of order from what they should have been... like not raising before initially deploying the probe, raising a 2nd time even though the probe was already deployed, only probing half the bed... if i had thought that i could get a print off of this i would have tried but the way it is i didn't think it would work... so i didn;t put a lot of effort into setting proper z height...

https://youtu.be/pAogPOZQBHc

marlin_main.cpp http://pastebin.com/WmZpFTsm

configuration.h http://pastebin.com/PDdVs9sn

Sniffle commented 9 years ago

probbaly need to break delta leveling out into its own as well... not sure about the other printing styles...

On Sat, May 23, 2015 at 9:08 AM, Roxy-3DPrintBoard <notifications@github.com

wrote:

But the correction that seems to be messing things up is the one in set_bed_level_equation_3pts – where it takes the current XYZ and treats it as if it was already rotated, then un-rotates it.

Seeing as that code at the end of G29 is now rotating it again, maybe the correction in set_bed_level_equation_3pts is unnecessary…?

That code you pointed at is the 3 point bed leveling. Is that the only flavor that has a problem? Because if so, that would be the explanation. You most certainly don't want to correct the Z_OFFSET_FROM_PROBE twice. (I don't think it is being un-rotated. I suspect it is getting rotated twice. But either way, that is wrong.)

Looking through the G29 code, it is obvious it needs to be broken into separate files. Probably only the flavor being used should be pulled in. It would clean things up a lot to have separate files for the 3-Point, n x n Grid and Mesh leveling.

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/2040#issuecomment-104900801 .

Roxy-3D commented 9 years ago

probbaly need to break delta leveling out into its own as well... not sure about the other printing styles...

Agreed! I didn't remember that one as I was typing.

thinkyhead commented 9 years ago

pulled you debug z branch and it had the io bug in it... So i did what any intelligent person would do

Ah good. It's all patched up now in my debug branches as well.