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.17k stars 19.21k forks source link

UBL: Z-Raises to Infinity #6581

Closed Koshu closed 5 years ago

Koshu commented 7 years ago

Flashed 1.1.x a moment ago and noticed a bug in UBL. My Mesh is build with an inset of 30. Created a new Mesh with G29 P1 after initializing the memory. Now, if I move the x-axis to Position 200 (G01 X200) the z-axis starts to rise after x reaches the position and never stops until I pull the plug. Also, when I moved to X0 the z-axis started to rise "slightly" (10mm) and never moves back down.

Configs attached.

Configuration.h.txt Configuration_adv.h.txt

Roxy-3D commented 7 years ago

Do you have any un-probed areas in your mesh? Can you post your mesh please?

What you are describing tends to happen when you are going off the mesh. If you have an inset of 30 you are only allowed to go from (30,30) to (170,170) Going to X=200 is off the mesh.

We probably need to clamp the movement to stop this type of thing. But your nozzle is supposed to stay within the bounds.

Do you really need an inset? Can you set your inset to 0 and regenerate the mesh? You can very quickly fill the mesh with a couple of G29 P3 O commands. So to try that probably won't take you very much time.

pmbroth commented 7 years ago

i had the same issue, but mine rose to the top of my zaxis.

Roxy-3D commented 7 years ago

i had the same issue, but mine rose to the top of my zaxis.

I better add some code to clamp the movement. In your case... Did you go off the mesh?

Koshu commented 7 years ago

I already flashed an old version (can redo the steps if needed) but there where no unprobed points in the mesh and everything was in a range +- 0.150mm. The inset was set to 30 so that the probe can reach every point, without manual probing (mine has an offset of the nozzle of 30 in x direction). I assumed that the UBL has a boundary handling like described in the the (unofficial) UBL starting guide in Issue #6510, so that it compensates the z-offset "as good as it can with the available information". I didn't expect it to do unreasonable movements.

So should the mesh always cover the complete movable area, even so there is no build plate at some points? If that is the case, why can the user even set the boundaries of the mesh?

Edit: Just noticed that the starting guide change a bit. The boundary handling isn't realy mentioned anymore except for the blue corner.

Koshu commented 7 years ago

Just an idea (or maybe it was already planned that way?): I think it would be nice to have an "extrapolation" option. That way the mesh can always cover the complete movement area and points that are not reachable by the probe or are just outside the build plate get filled with estimations. Right now only a constant value is available.

pmbroth commented 7 years ago

Yes mine did go off mesh. Thank you

Roxy-3D commented 7 years ago

I'm not sure any one answer is going to cover everybody's needs... Maybe we need a set of options for the user to pick from:

//#define UBL_KILL_IF_OFF_THE_MESH         // if off mesh...  Kill print job
//#define UBL_OFF_MESH_CORRECTION 0.00     // if off mesh...  use Z-Height correction of this number
//#define UBL_OFF_MESH_EXTRAPOLATE_AS_BEST_AS_POSSIBLE   // Obviously, this name changes

Any others?

pmbroth commented 7 years ago

When mine went off mwah the probing occurred before my bed. Meaning it was probing in air the bed was in front of the probe. Hope that makes sense. I may have to adjust my ymax position I currently have the bed bdiknaions at 400 by 740 may make it 710 so my bed does not move forward as much.

Roxy-3D commented 7 years ago

Yes! You need to have your bed size (and min/max positions) set correctly!!!! Otherwise, bad things happen.

pmbroth commented 7 years ago

I'm changing the bed settings now... will let you know if any issues still occur

Koshu commented 7 years ago

Ok, maybe I configured something wrong or I am just confused. Where can I define the bed size?

Only place i know with min/max limits is this part:

// Travel limits after homing (units are in mm)
#define X_MIN_POS 0
#define Y_MIN_POS 0
#define Z_MIN_POS 0
#define X_MAX_POS 200
#define Y_MAX_POS 200
#define Z_MAX_POS 200

But it says travel limits not bed size. Which is correct in my opinion. Depending on your setup you might want to move outside the bed (for parking, cleaning nozzle ...). Or did I missunderstood/overlook something?

Also, might have some free time this weekend. If @Roxy-3D can give me some hints to the best place to implement boundary handling I might be able to help with the extrapolation part.

Roxy-3D commented 7 years ago

Here are a couple of hints... Get everything set up and working with UBL turned off. With those numbers up above... You should be able to go to (0,0) and then (200,200) and be at opposite corners on the bed. make sure all of your bed dimensions and XYZ_PROBE_OFFSET_FROM_EXTRUDER numbers are right.

And then... When we turn on UBL... Everything will go smoothly.

Koshu commented 7 years ago

But for example 0/0 is outside the bed, like 5mm on every side, since my bed is a little bit smaller. However i use this position for parking, so i dont want to set travel_limits to 5.

Shouldn't there be options to define the travel limits independent from the bed size? i.e. if you have a nozzle cleaning device: That will be way outside the bed, but you still need to be able to move to that position (i don't have one but played with the idea of adding one).

Roxy-3D commented 7 years ago

I have a similar issue. I have a very large print bed. And I have a 400mm x 400mm heatpad and sheet of glass in the center of it. My machine homes off the glass in both the X and Y axis. I set my inset to 50mm and I'm on the glass.

You can do the same thing. Define your real positions. And then define an inset that is on the bed. And most likely your real inset will not be square shaped. You can go into Configuration_adv.h and adjust where each side of the inset ends up.

Koshu commented 7 years ago

Playing around with UBL again. Lets see if i can find a configuration that works for my printer.

While trying out stuff i noticed something else: When doing a G29 Z i can't move the z-Axis. The extruder moves up to 1.5mm, my LCD shows my some menu with the 1.5mm (I guess to adjust the height) but turning the rotary switch does nothing. However, I can press it to return to the normal menu.

Is this a bug and should I open a separate issue for that?

Roxy-3D commented 7 years ago

I'm not sure what we want G29 Z to do... I don't use it. If you need it to do something useful... We can make it do that.

Koshu commented 7 years ago

Nah, not really needing it or more precise i guess it doesn't need to be in UBL.

What I thought it does and what would be kind of neat would be a routine to adjust the extruder <-> probe z-offset. So, you go to a point of the bed, move the extruder down with the rotary switch until you are happy with the height (using your piece of paper or whatever) and then just click the rotary switch to adjust the probe offset. Right now i do it more manually, like writing it down and transfer it to the appropriate config value.

However that could be independent from UBL since it would be useful for everybody with a probe.

Roxy-3D commented 7 years ago

Yeah... Or maybe a manual version of the M851 Z-Probe_Offset. Instead of specifying the number... You can dial it in and watch the nozzle move.

Koshu commented 7 years ago

Ok, few updates, some praise and one serious problem.

So played around a bit. Noticed I was an idiot because my nozzle never leaves the build plate (before it did, but i printed a new x-carriage last week). So i set the inset to zero, so that the mesh fills the complete travel area and most of my build plate. And that seems to work (see attached image). Still need to do some fine tuning but the print lines in the first layer seem to be of constant width. Thats the first time mesh-leveling actually worked for me (switched between different versions of marlin and repetier), so kudos for that!

photo_2017-05-05_20-50-09

However not everything is fine and one parts definitely needs to be addressed: I set cura to execute an G01 X0 Y200 when the print is finished so the printhead is out of the way. X0 as well as Y200 is exactly at the boundary of the mesh. Now, the printer smashes the x-axis into my endstops and didn't stop until I hit the emergency stop. That really didn't sound nice.

I tried out some things and this is what I could find out so far:

Would try other things but i'm seriously concerned that i will break something in my printer if i do that to often.

P.S: I'm on 1.1.x-bugfix now because of the sd-card bug.

Roxy-3D commented 7 years ago

@Koshu Can you attach your Configuration.h and Configuration_adv.h file (as .txt files... Please do NOT cut and paste them) ?

What happens if you:

Koshu commented 7 years ago

Here are the configs: Configuration.h.txt Configuration_adv.h.txt

Doing:

also doing a G01 X0 Y200 while Y is already at 200 is also fine. Like this:

As far as i can tell both axis need to be moving at the same time for the bug to occur.

This works reliable for me to produce the error:

Also here is my mesh:


Bed Topography Report:

(0,9)                                                                           (9,9)
(0,200)                                                                        (200,200)
 -0.174   -0.174   -0.174   -0.169   -0.231   -0.299   -0.419   -0.564   -0.741   -0.869  

  0.004    0.004    0.004    0.011   -0.109   -0.194   -0.246   -0.434   -0.581   -0.756  

  0.186    0.164    0.141    0.119    0.086   -0.041   -0.129   -0.304   -0.446   -0.644  

  0.344    0.306    0.269    0.231    0.156    0.131    0.016   -0.144   -0.321   -0.551  

  0.464    0.416    0.369    0.321    0.254    0.181    0.041   -0.096   -0.294   -0.454  

  0.476    0.436    0.396  [ 0.356]   0.294    0.221    0.151   -0.066   -0.256   -0.456  

  0.404    0.404    0.404    0.424    0.301    0.219    0.086   -0.069   -0.221   -0.434  

  0.379    0.369    0.359    0.349    0.314    0.174    0.056   -0.104   -0.279   -0.461  

  0.366    0.329    0.291    0.254    0.224    0.111    0.016   -0.149   -0.344   -0.544  

  0.366    0.329    0.291    0.254    0.224    0.111    0.016   -0.149   -0.344   -0.544  
(0,0)                                                                            (200,0)
(0,0)                                                                             (9,0)
ok

Edit: Added missing command

Roxy-3D commented 7 years ago

Can you tell me what happens with:

I think we need to clamp some numbers.

Koshu commented 7 years ago

Just tried. No problem.

Roxy-3D commented 7 years ago

OK... I think we need to clamp some numbers when we are off the mesh. The question is: How best, how most simply do we do it?

Koshu commented 7 years ago

Ok, I think I found the culprit, however I'm not sure what the best way to fix it is. We have an Integer underflow as well as an endless loop here. The problem is in ubl_motion.cpp line 394 and following: while (xi_cnt > 0 || yi_cnt > 0) { The loop terminates only if the correct number of x- and y-meshlines are crossed. What type of line is crossed is checked here: if (left_flag == (x > next_mesh_line_x)) { // Check if we hit the Y line first Now, the problem is: If there is no "next"-next_mesh_line_x this statement will never be true and only the the else part is executed. This results in the yi_cnt will never be decreased and the while loop never terminates. In my case this happens when in x = 0 and next_mesh_line_x = 0. Normally there should be a smaller next_mesh_line, so that in the next iteration of the while loop the y-part gets executed but since this is the smallest mesh_line it stays forever at 0.

This also results in the underflow since the index current_xi gets constantly decreased (but this seems to be fine? Not nice anyway)

To test this I hacked a few ugly debug serial outputs in the code:

...
    while (xi_cnt > 0 || yi_cnt > 0) {

    SERIAL_ECHOPGM("x-mesh-idx= ");
    SERIAL_ECHO_F(current_xi + dxi, 6);
    SERIAL_ECHOPGM(", y-mesh-idx= ");
    SERIAL_ECHO_F(current_yi + dyi, 6);

      const float next_mesh_line_x = LOGICAL_X_POSITION(pgm_read_float(&ubl.mesh_index_to_xpos[current_xi + dxi])),
                  next_mesh_line_y = LOGICAL_Y_POSITION(pgm_read_float(&ubl.mesh_index_to_ypos[current_yi + dyi])),
                  y = m * next_mesh_line_x + c,   // Calculate Y at the next X mesh line
                  x = (next_mesh_line_y - c) / m; // Calculate X at the next Y mesh line
                                                  // (No need to worry about m being zero.
                                                  //  If that was the case, it was already detected
                                                  //  as a vertical line move above.)
     SERIAL_ECHOPGM("next_mesh_line_x= ");
    SERIAL_ECHO_F(next_mesh_line_x, 6);
    SERIAL_ECHOPGM(", next_mesh_line_y= ");
    SERIAL_ECHO_F(next_mesh_line_y, 6);
    SERIAL_ECHOPGM("xi_cnt= ");
    SERIAL_ECHO_F(xi_cnt, 6);
    SERIAL_ECHOPGM(", yi_cnt= ");
    SERIAL_ECHO_F(yi_cnt, 6);
    SERIAL_ECHOPGM("x= ");
    SERIAL_ECHO_F(x, 6);
    SERIAL_ECHOPGM(", y= ");
    SERIAL_ECHO_F(y, 6);
    SERIAL_EOL;
....

An "alowed" movement like G01 X0 Y199.9 produces this:

ok
x-mesh-idx= 3, y-mesh-idx= 5 next_mesh_line_x= 66.666664, next_mesh_line_y= 111.111106 xi_cnt= 3, yi_cnt= 4 x= 58.689540, y= 99.042861
x-mesh-idx= 2, y-mesh-idx= 5 next_mesh_line_x= 44.444442, next_mesh_line_y= 111.111106 xi_cnt= 2, yi_cnt= 4 x= 58.689540, y= 132.661911
x-mesh-idx= 2, y-mesh-idx= 10 next_mesh_line_x= 44.444442, next_mesh_line_y=133.333328 xi_cnt= 2, yi_cnt= 3 x= 44.000633, y= 132.661911
x-mesh-idx= 1, y-mesh-idx= 10 next_mesh_line_x= 22.222221, next_mesh_line_y= 133.333328 xi_cnt= 1, yi_cnt= 3 x= 44.000633, y= 166.280960
x-mesh-idx= 1, y-mesh-idx= 11 next_mesh_line_x= 22.222221, next_mesh_line_y= 155.555541 xi_cnt= 1, yi_cnt= 2 x= 29.311731, y= 166.280960
x-mesh-idx= 1, y-mesh-idx= 12 next_mesh_line_x= 22.222221, next_mesh_line_y= 177.777770 xi_cnt= 1, yi_cnt= 1 x= 14.622819, y= 166.280960
x-mesh-idx= 0, y-mesh-idx= 12 next_mesh_line_x= 0.000000, next_mesh_line_y= 177.777770 @xi_cnt= 0, yi_cnt= 1 x= 14.622819, y= 199.900009

However the "forbidden" G01 X0 Y200 produces this:

ok
x-mesh-idx= 4, y-mesh-idx= 5 next_mesh_line_x= 88.888885, next_mesh_line_y= 111.111106 xi_cnt= 4, yi_cnt= 5 x= 88.888893, y= 111.111114
x-mesh-idx= 4, y-mesh-idx= 10 next_mesh_line_x= 88.888885, next_mesh_line_y= 133.333328 xi_cnt= 4, yi_cnt= 4 x= 66.666671, y= 111.111114
x-mesh-idx= 3, y-mesh-idx= 10 next_mesh_line_x= 66.666664, next_mesh_line_y= 133.333328 xi_cnt= 3, yi_cnt= 4 x= 66.666671, y= 133.333343
x-mesh-idx= 3, y-mesh-idx= 11 next_mesh_line_x= 66.666664, next_mesh_line_y= 155.555541 xi_cnt= 3, yi_cnt= 3 x= 44.444458, y= 133. 333343
x-mesh-idx= 2, y-mesh-idx= 11 next_mesh_line_x= 44.444442, next_mesh_line_y= 155.555541 xi_cnt= 2, yi_cnt= 3 x= 44.444458, y= 155.555557
x-mesh-idx= 2, y-mesh-idx= 12 next_mesh_line_x= 44.444442, next_mesh_line_y= 177.777770 xi_cnt= 2, yi_cnt= 2 x= 22.222229, y= 155.555557
x-mesh-idx= 1, y-mesh-idx= 12 next_mesh_line_x= 22.222221, next_mesh_line_y= 177.777770 xi_cnt= 1, yi_cnt= 2 x= 22.222229, y= 177.777770
x-mesh-idx= 1, y-mesh-idx= 13 next_mesh_line_x= 22.222221, next_mesh_line_y= 200.000000 xi_cnt= 1, yi_cnt= 1 x= 0.000000, y= 177.777770
x-mesh-idx= 0, y-mesh-idx= 13 next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 0, yi_cnt= 1 x= 0.000000, y= 200.000000
x-mesh-idx= 1550104015503, y-mesh-idx= 13 next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 1550104015503, yi_cnt= 1 x= 0.000000, y= 200.000000
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=200.00, x1_i=-1, yi=8)
x-mesh-idx= 1550104015502, y-mesh-idx= 13 next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 1550104015502, yi_cnt= 1 x= 0.000000, y= -0.000000
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=-0.00, x1_i=-2, yi=8)
x-mesh-idx= 1550104015501, y-mesh-idx= 13 next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 1550104015501, yi_cnt= 1 x= 0.000000, y= -0.000000
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=-0.00, x1_i=-3, yi=8)
x-mesh-idx= 1550104015500, y-mesh-idx= 13 next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 1550104015500, yi_cnt= 1 x= 0.000000, y= -0.000000
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=-0.00, x1_i=-4, yi=8)
x-mesh-idx= 1550104015455, y-mesh-idx= 13 next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 1550104015455, yi_cnt= 1x = 0.000000, y= 200.000000
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=200.00, x1_i=-5, yi=8)
x-mesh-idx= 1550104015454, y-mesh-idx= 13n ext_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 1550104015454, yi_cnt= 1 x= 0.000000, y= -0.000000
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=-0.00, x1_i=-6, yi=8)
...
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=200.00,  x1_i=-21,  yi=8)
<-- at this point i hit the emergency stop

As can be seen yi_cnt never reaches 0 and some other stuff happens.

I'm not quite sure why the x axis is still moving since there is only a x=0 written into the planer_buffer but might be because it eats all the CPU time or something.

Now for the fix: The best way i could think of to fix it is to write a wrapper for the functions executed in 396 and 397:

const float next_mesh_line_x = LOGICAL_X_POSITION(pgm_read_float(&ubl.mesh_index_to_xpos[current_xi + dxi])),
                  next_mesh_line_y = LOGICAL_Y_POSITION(pgm_read_float(&ubl.mesh_index_to_ypos[current_yi + dyi])),

The wrapper would return "dummy"-positions for mesh-lines even if they don't really exist. If you combine that with a dummy mesh z-value of zero that will probably solve all kinds of boundary problems, since the algorithm can now work on an quasi-infinite mesh and no special boundary handling is required. That probably would solve the "rising z"-issue as well. I don't know in which places this would need additional adjustments.

But that was just my first idea and I don't know the code very well, so you might have a better one.

Edit: Fixed some typos. It is quite late here in germany, I better go to bed now. Edited by Roxy (This Post is super valuable! But I needed to be able to read the data easily.)

Roxy-3D commented 7 years ago

Good work! I'm going to re-read this several times! THANK YOU for the help!

Koshu commented 7 years ago

I'm going to re-read this several times!

I hope not because my english is so terrible :) If you need anything else or something isn't clear feel free to ask.

Roxy-3D commented 7 years ago

No... Your English is fine. This is complicated stuff.

But one thing I suspect right now is if you are willing to set your mesh inset to 1... I bet this problem goes away until we have a more elegant solution in place. If do try that... Please report whether that is a work around for the problem.

Koshu commented 7 years ago

Right now my workaround is to not move to x0 y200 by changing my parking position. But will try anyway as soon as I have time.

Roxy-3D commented 7 years ago

@Koshu I'm wondering if you can add a little more debug code and capture another trace for me? Here is the thing I don't understand. Up above, xi_cnt is counting down from 5 to 0. xi_cnt is an integer and that means its range is limited from +32767 to -32768. Why is it going to 1550104015503 ?

Is this a data corruption problem where some of the variables are getting hammered? And even if that was true... Why are the SERIAL_PROTOCOL() routines thinking it switched from being an int to a long ?

Koshu commented 7 years ago

Sure but have to finish some prints first, which I promised a friend. Maybe tomorrow. One idea: I copied the debug code from your debug function without looking at it to much. Is SERIAL_ECHO_F an echo function for floats? That might explain the funky numbers for negative or very large integers.

bgort commented 7 years ago

@Roxy-3D @Koshu I'm wondering if you can add a little more debug code and capture another trace for me? Here is the thing I don't understand. Up above, xi_cnt is counting down from 5 to 0. xi_cnt is an integer and that means its range is limited from +32767 to -32768. Why is it going to 1550104015503 ?

We have

   SERIAL_ECHOPGM("x-mesh-idx= ");
   SERIAL_ECHO_F(current_xi + dxi, 6);

and the output:

x-mesh-idx= 0, ... snip
x-mesh-idx= 1550104015503, ... snip

current_xi and dxi are int, so the sum is int... but we are calling SERIAL_ECHO_F, which is overloaded such that if called with a float, the second argument (6 in this case) is the output precision, but if called with an int, is the base of the number (for formatting purposes) ..

So because we're using SERIAL_ECHO_F(XXX, 6) we're getting XXX output in base 6, which explains the weirdness. As these are ints, should just use SERIAL_ECHO() ..

bgort commented 7 years ago

I'm still parsing through things and don't fully understand how all of this works yet, but I do have a few ideas that may help, at least:

When this is occurring, we're ending up here:

    while (xi_cnt > 0 || yi_cnt > 0) {
      ...snip...
      if (left_flag == (x > next_mesh_line_x)) { // Check if we hit the Y line first
        ...snip...
      else {   // Yes!  Crossing a X Mesh Line next
        float z0 = ubl.z_correction_for_y_on_vertical_mesh_line(y, current_xi + dxi, current_yi - down_flag);
        ...snip...

For checking if we hit the Y line first, there's an explicit test.. and it fails.. so we end up in the else ...

Instead of the else, does it make sense to do an explicit check in the form of an else if(...) for either crossing a X mesh line, -or- whether (current_xi+dxi) is not negative, and then have an alternate else/default case that dumps us out of the loop because something has gone wrong?

Another possibility is simply checking to ensure that xi_cnt and yi_cnt remain positive, and if they don't we exit the loop?

It's not likely that either of these are an actual solution to the problem, but they may do the job?

I'm going to keep reviewing until I actually understand what's going on in this function and how ubl works overall, but those are my early thoughts and I figure a solution may be obvious to Roxy where it isn't yet to me...

(Also, I found a few places that needed some cleanup/improvement - mostly in ubl.h - so I have a working branch with those in them.)

Koshu commented 7 years ago

...So because we're using SERIAL_ECHO_F(XXX, 6) we're getting XXX output in base 6, which explains the weirdness. As these are ints, should just use SERIAL_ECHO() ..

Figured that something like that happened. 1550104015503 base 6 is 0xFFFFFFFF in hex, kinda makes sense. The incremental changes of the variables however are still valid.

Should I redo the test with cleaner debug code anyway or is unnecessary now?

bgort commented 7 years ago

I don't see a need right now, but thank you.

Koshu commented 7 years ago

Instead of the else, does it make sense to do an explicit check in the form of an else if(...) for either crossing a X mesh line, -or- whether (current_xi+dxi) is not negative, and then have an alternate else/default case that dumps us out of the loop because something has gone wrong?

I don't understand the ubl-code 100% but I think that won't work or at least needs some additional handling. Problem is the final move to the endposition. Most of the time that one won't cross a gridline anymore. Right now (i guess) the else part takes care of that ...... <-- nevermind, how does the code handle the endposition? Shouldn't it move to next_mesh_line_x regardless of the real endposition because the else part gets executed? Couldn't identify how this works.

bgort commented 7 years ago

Yeah, not sure about what I suggested -- still sorting through it all myself.

Koshu commented 7 years ago

nevermind, how does the code handle the endposition? Shouldn't it move to next_mesh_line_x regardless of the real endposition because the else part gets executed? Couldn't identify how this works.

Ah, found it, there is a goto FINAL_MOVE; after the while loop. An "else if" should work, i think. (Should think before i write stuff) An "else if" might work, at least for cartesian printers but not nescessarily for deltas. The problem isn't (only) that that the else part is executed but also that the other descition-branch isn't. I think in some (rare) cases there might be missings moves or unproper bedleveling at the end.

bgort commented 7 years ago

Yeah -- just trying to make sure nothing else gets fargled if we just dump out of the loop and let it do the FINAL_MOVE ...

Koshu commented 7 years ago

I still think a "virtual" extension of the mesh is the easiest solution. If next_mesh_line_x would be set to -20 mm when it hits the boundary the code should execute properly (In case your gridlines are 20 mm apart)

bgort commented 7 years ago

I feels semi-dangerous to me to continue making 'virtual' corrections when we don't know the topology of the bed outside the ubl/mesh area. I'm thinking of cases where there are large insets, or where the bed shape changes dramatically outside the mesh area.

Starting to get a handle on the code, but I'm thinking Roxy's more-complicated-but-safer-proposal of a system of defines like ...

//#define UBL_KILL_IF_OFF_THE_MESH         // if off mesh...  Kill print job
//#define UBL_OFF_MESH_CORRECTION 0.00     // if off mesh...  use Z-Height correction of this number
//#define UBL_OFF_MESH_EXTRAPOLATE_AS_BEST_AS_POSSIBLE   // Obviously, this name changes

... likely makes more sense. Still reviewing and thinking, though.

Roxy-3D commented 7 years ago

That SERIAL_PROTOCOL_F() explanation helped! But... Here is a clue. I wonder if we are fighting a round off error. Scroll way to the right and look at:

ly0=200.00, x1_i=-1, yi=8)
x-mesh-idx= 1550104015502, y-mesh-idx= 13 next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 1550104015502, yi_cnt= 1 x= 0.000000, y= -0.000000
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=-0.00, x1_i=-2, yi=8)
x-mesh-idx= 1550104015501, y-mesh-idx= 13 next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 1550104015501, yi_cnt= 1 x= 0.000000, y= -0.000000
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=-0.00, x1_i=-3, yi=8)
x-mesh-idx= 1550104015500, y-mesh-idx= 13 next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 1550104015500, yi_cnt= 1 x= 0.000000, y= -0.000000
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=-0.00, x1_i=-4, yi=8)
x-mesh-idx= 1550104015455, y-mesh-idx= 13 next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000 xi_cnt= 1550104015455, yi_cnt= 1x = 0.000000, y= 200.000000
? in get_z_correction_along_vertical_mesh_line_at_specific_x(ly0=200.00, x1_i=-5, yi=8)

How are we getting a -.00000 when the edge of the bed is at 0.0 ??? I bet this is involved in the failure.

bgort commented 7 years ago

That is weird, but it seems to be outputting everything that way -- like the 200.00000, etc. ... not sure what to make of it.

This is obviously a problem:

      const float next_mesh_line_x = LOGICAL_X_POSITION(pgm_read_float(&ubl.mesh_index_to_xpos[current_xi + dxi])),
                  next_mesh_line_y = LOGICAL_Y_POSITION(pgm_read_float(&ubl.mesh_index_to_ypos[current_yi + dyi])),

Because as we iterate through the loop, current_xi+dxi (and the y variant) ends up way out of bounds and so we're doing a pgm_read_float on a random address...

bgort commented 7 years ago

Maybe a simple:

if (xi_cnt < 0 || yi_cnt < 0)   //we've gone too far, or out of the mesh, so move directly via FINAL_MOVE
        break;

..at the end of the while loop will take care of it?

Obviously we still need to think about accommodating the edge cases, but that at least fixes ~the bug and~ the driving of carriages into the ends of the axes, I believe?

Roxy-3D commented 7 years ago

Yes... But the reason is the loop isn't terminating when it should. If the Y location has a round off error and is -0.0000, that puts it in the previous (non-existent) mesh cell. And in that case... Yeah, it goes of the front end of the array.

bgort commented 7 years ago

Maybe increasing the precision from 6 to 10 or whatever will give us some additional insight.

Koshu commented 7 years ago

I feels semi-dangerous to me to continue making 'virtual' corrections when we don't know the topology of the bed outside the ubl/mesh area. I'm thinking of cases where there are large insets, or where the bed shape changes dramatically outside the mesh area.

Well it adds an correction of 0 right now anyway (if NaN is returned).

But i have an easier fix (or sort of a starting point). All we need to do is make sure that the true-branch is executed if all x-gridlines are passed (because it should be the only thing left to do or the loop should already have terminated).

So all that really needs to be changed is: if (left_flag == (x > next_mesh_line_x)) { // Check if we hit the Y line first to if (left_flag == (x > next_mesh_line_x) || !WITHIN(current_xi + dxi, 0, GRID_MAX_POINTS_X - 1)) { // Check if we hit the Y line first

I tested this a moment ago and it prevents the x-carriage from crashing.

after

I now get this (same ugly serial output):

ok
x-mesh-idx= 3, y-mesh-idx= 5next_mesh_line_x= 66.666664, next_mesh_line_y= 111.111106xi_cnt= 3, yi_cnt= 5x= 58.700214, y= 99.047622
x-mesh-idx= 2, y-mesh-idx= 5next_mesh_line_x= 44.444442, next_mesh_line_y= 111.111106xi_cnt= 2, yi_cnt= 5x= 58.700214, y= 132.698425
x-mesh-idx= 2, y-mesh-idx= 10next_mesh_line_x= 44.444442, next_mesh_line_y= 133.333328xi_cnt= 2, yi_cnt= 4x= 44.025161, y= 132.698425
x-mesh-idx= 1, y-mesh-idx= 10next_mesh_line_x= 22.222221, next_mesh_line_y= 133.333328xi_cnt= 1, yi_cnt= 4x= 44.025161, y= 166.349212
x-mesh-idx= 1, y-mesh-idx= 11next_mesh_line_x= 22.222221, next_mesh_line_y= 155.555541xi_cnt= 1, yi_cnt= 3x= 29.350114, y= 166.349212
x-mesh-idx= 1, y-mesh-idx= 12next_mesh_line_x= 22.222221, next_mesh_line_y= 177.777770xi_cnt= 1, yi_cnt= 2x= 14.675058, y= 166.349212
x-mesh-idx= 0, y-mesh-idx= 12next_mesh_line_x= 0.000000, next_mesh_line_y= 177.777770xi_cnt= 0, yi_cnt= 2x= 14.675058, y= 200.000000
x-mesh-idx= 0, y-mesh-idx= 13next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000xi_cnt= 0, yi_cnt= 1x= 0.000000, y= 200.000000
x-mesh-idx= 1550104015503, y-mesh-idx= 13next_mesh_line_x= 0.000000, next_mesh_line_y= 200.000000xi_cnt= -1, yi_cnt= 1x= 0.000000, y= 200.000000
? in z_correction_for_x_on_horizontal_mesh_line(lx0=0.00,x1_i=-1,yi=9)
ok

Isn't a 100% solution yet, as the code still writes X0 Y200 twice in the buffer (once with disabled mesh leveling).

Would try more things but have to go back to finish my printings.

Roxy-3D commented 7 years ago

If we went this way... I think we would want to check for the opposite cases too:

if (xi_cnt < 0 || yi_cnt < 0 || xi_cnt>=GRID_MAX_POINTS_X || yi_cnt>=GRID_MAX_POINTS_Y)   //we've gone too far, or out of the mesh, so move directly via FINAL_MOVE
        break;

And maybe... We should actually be checking the X & Y values too?

bgort commented 7 years ago

Well we're decrementing xi_cnt and yi_cnt, not incrementing, no? So they'll never get bigger.. but I think checking the coordinates may make sense.

Roxy-3D commented 7 years ago

Well we're decrementing xi_cnt and yi_cnt, not incrementing, no?

RIGHT!