Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

G35/ASSISTED_TRAMMING always fails on point 1 #20412

Closed
Pyro-Fox opened this issue Dec 9, 2020 · 25 comments · Fixed by #20572
Closed

G35/ASSISTED_TRAMMING always fails on point 1 #20412

Pyro-Fox opened this issue Dec 9, 2020 · 25 comments · Fixed by #20572
Assignees

Comments

@Pyro-Fox
Copy link
Contributor

Pyro-Fox commented Dec 9, 2020

Bug Description

G35/ASSISTED_TRAMMING always fails on point 1. Additionally, if I uncomment and try to use ASSISTED_TRAMMING_WIZARD, it can only probe TRAMMING_POINT_NAME_1. If I try probe any of the other points, it will report the last measurements as gibberish on my LCD, and not move. Even if I try to measure the other points as my first operation from entering the wizard, it will just move to the default Z height and sit there.

Configuration Files

MarlinConfig.zip

Steps to Reproduce

  1. Send G35 via serial, or select "Assisted Tramming" from the LCD.

Expected behavior:

Honestly, I don't know. I'm just trying to use a new feature.

Actual behavior:

(see Bug Description)

Additional Information

BTW, I'm really just trying to improve my overall hot bed replacement and UBL probing process. I figure if I can get the corners level quickly with auto-tramming, then G29 P1 is more likely to be in the ballpark on the first try. Yes, my 12x12 grid + MULTIPLE_PROBING 3 is probably unnecessary, but damnit... it makes printing on clean glass so great, even if building the mesh takes about 30 minutes. (Seriously, if someone could add an M-code to control GRID_MAX_POINTS, that would solve my terrible workflow problems too 😅).

Here's the G-code output from G35 and my EEPROM settings.

M503
23:33:37.780 > echo:  G21    ; Units in mm (mm)  
23:33:37.780 > echo:  M149 C ; Units in Celsius  
23:33:37.780 >
23:33:37.780 > echo:; Filament settings: Disabled
23:33:37.780 > echo:  M200 S0 D1.75
23:33:37.780 > echo:; Steps per unit:
23:33:37.780 > echo: M92 X80.00 Y80.00 Z400.00 E415.00
23:33:37.780 > echo:; Maximum feedrates (units/s):
23:33:37.780 > echo:  M203 X300.00 Y300.00 Z5.00 E25.00
23:33:37.780 > echo:; Maximum Acceleration (units/s2):
23:33:37.780 > echo:  M201 X3000.00 Y3000.00 Z100.00 E10000.00
23:33:37.780 > echo:; Acceleration (units/s2): P<print_accel> R<retract_accel> T<travel_accel>
23:33:37.780 > echo:  M204 P3000.00 R3000.00 T3000.00
23:33:37.780 > echo:; Advanced: B<min_segment_time_us> S<min_feedrate> T<min_travel_feedrate> J<junc_dev>
23:33:37.780 > echo:  M205 B20000.00 S0.00 T0.00 J0.03
23:33:37.780 > echo:; Home offset:
23:33:37.780 > echo:  M206 X0.00 Y0.00 Z0.00
23:33:37.780 > echo:; Unified Bed Leveling:
23:33:37.780 > echo:  M420 S1 Z10.00
23:33:37.780 >
23:33:37.780 > Unified Bed Leveling System v1.01 active
23:33:37.842 > 
23:33:37.843 > echo:; Active Mesh Slot: 3
23:33:37.843 > echo:; EEPROM can hold 5 meshes.
23:33:37.843 >
23:33:37.843 > echo:; Material heatup parameters:
23:33:37.843 > echo:  M145 S0 H180 B65 F0
23:33:37.843 > echo:  M145 S1 H200 B85 F0
23:33:37.843 > echo:; PID settings:
23:33:37.843 > echo:  M301 P40.70 I3.00 D69.20
23:33:37.843 > echo:; Z-Probe Offset (mm):
23:33:37.843 > echo:  M851 X-23.42 Y-48.30 Z-0.94
23:33:37.843 > echo:; Linear Advance:
23:33:37.843 > echo:  M900 K0.08
23:33:37.843 > ok
G35
23:33:49.257 > echo:busy: processing
23:33:51.911 > echo:busy: processing
23:33:53.910 > echo:busy: processing
23:33:57.219 > echo:busy: processing
23:33:59.218 > echo:busy: processing
23:34:01.216 > echo:busy: processing
23:34:03.214 > echo:busy: processing
23:34:05.292 > echo:busy: processing
23:34:07.290 > echo:busy: processing
23:34:09.290 > echo:busy: processing
23:34:11.288 > echo:busy: processing
23:34:12.150 > X:173.42 Y:198.30 Z:10.94 E:0.00 Count X:13874 Y:15864 Z:4378
23:34:13.286 > echo:busy: processing
23:34:15.284 > echo:busy: processing
23:34:18.497 > echo:busy: processing
23:34:20.496 > echo:busy: processing
23:34:22.494 > echo:busy: processing
23:34:24.556 > echo:busy: processing
23:34:26.747 > echo:busy: processing
23:34:28.745 > echo:busy: processing
23:34:30.743 > echo:busy: processing
23:34:32.742 > echo:busy: processing
23:34:34.741 > echo:busy: processing
23:34:36.738 > echo:busy: processing
23:34:37.453 > G35 failed at point 1 (Rear-Left) X280.00 Y20.00
23:34:37.453 > G35 aborted.
@Pyro-Fox
Copy link
Contributor Author

Pyro-Fox commented Dec 9, 2020

And I'll admit in advance, I probably have a configuration issue, in which the bug is a missing safety check, or there's actually something wrong in the tramming code. 🤔

@hamster65
Copy link

I ran into this problem too.
For me, the solution was to define the tramming points so that they are inside the area defined by
PROBING_MARGIN and min and max for x and y.

I think the relevance of PROBING_MARGIN should be mentioned in documentation, or perhaps PROBING_MARGIN should be ignored for tramming.

@boelle
Copy link
Contributor

boelle commented Dec 9, 2020

@Pyro-Fox can you confirm what @hamster65 says?

@MarlinFirmware/docs-maintainers

@FanDjango
Copy link
Contributor

FanDjango commented Dec 9, 2020

Ditto, same experience here. Nozzle_XY gets adjusted to Probe_XY with the probe offsets, this results in a probe destination XY, which is subject to the MIN and MAX range of the bed, and also the PROBE_MARGIN as @hamster65 said.

To see the problem, turn on debug levelling and watch for the "destination cannot be reached" error message.

@hamster65
Copy link

hamster65 commented Dec 9, 2020

Instead of checking if probe points can be reached during runtime, wouldn't a sanity check with a comprehensive error message work? It would save space and prevent users from running into that issue.

I set probe_margin to 30 so that my bltouch does not collide with the bed clamps. At the same time, I'd like tramming to probe the outer edges of the bed, as close to the screws as possible. This is even outside of my prinable area. So I don't think tramming should be restricted by probe_margin or the bed size.

@Pyro-Fox
Copy link
Contributor Author

Pyro-Fox commented Dec 9, 2020

If I have to account for PROBE_MARGIN, then how am I supposed to interpret this help message?

// Define positions for probing points, use the hotend as reference not the sensor.

I know the probe can't reach the spots based on my offsets (NOZZLE_TO_PROBE_OFFSET { -23.42, -48.3, 0 }), but the documentation suggests that it will move the nozzle to that point and probe, it. So, with PROBING_MARGIN 1, and tramming point 2 being {280, 20}, that makes the actual probe point... {-3.42, 231.7}? If that math were right, my first tramming point of {20, 20} should also fail, but it doesn't... 🤔. I just enabled DEBUG_LEVELING_FEATURE and will update with what I see.

@Pyro-Fox
Copy link
Contributor Author

Pyro-Fox commented Dec 9, 2020

DEBUG_LEVELING_FEATURE doesn't add any new output to the console when I run G35 😕. Also, I tried adjusting the tramming points to compensate for PROBE_MARGIN and NOZZLE_TO_PROBE_OFFSET, and confirmed that Marlin is already doing that for me. The first probing point is now exactly one NOZZLE_TO_PROBE_OFFSET distance away from the corner 😅.

Any other ideas?

@Pyro-Fox
Copy link
Contributor Author

Pyro-Fox commented Dec 9, 2020

Also, I noticed the order of positions defined in TRAMMING_POINT_XY aren't the same order the TRAMMING_POINT_NAME_x's are consumed. Case in point:

Configuration_adv.h

#define ASSISTED_TRAMMING
#if ENABLED(ASSISTED_TRAMMING)

  // Define positions for probing points, use the hotend as reference not the sensor.
  #define TRAMMING_POINT_XY { {  1, 1 }, { 299,  1 }, { 299, 299 }, { 1, 299 } }

  // Define positions names for probing points.
  #define TRAMMING_POINT_NAME_1 "POINT 1"
  #define TRAMMING_POINT_NAME_2 "POINT 2"
  #define TRAMMING_POINT_NAME_3 "POINT 3"
  #define TRAMMING_POINT_NAME_4 "POINT 4"

Serial output (minus "busy: processing" messages)

G35
10:15:01.464 > X:173.42 Y:198.30 Z:10.94 E:0.00 Count X:13874 Y:15864 Z:4378
10:15:26.964 > G35 failed at point 1 (POINT 2) X299.00 Y1.00
10:15:26.964 > G35 aborted.

@hamster65
Copy link

I guess it starts counting at zero, so "failed at point 1" means the second point.

I found the sentence "Define positions for probing points, use the hotend as reference not the sensor." confusing as well. Of course it probes the coordinates specified by TRAMMING_POINT_XY and of course it has to move the probe to this coordinates by adding the probe offsets.

To rule out Probe_margin, you could try setting that to a negative value.
Or set your tramming point positions to somewhere in the middle of the bed, just for a test.
You are using UBL... could it be affected my mesh_inset as well, or any other constraints?

@Pyro-Fox
Copy link
Contributor Author

Pyro-Fox commented Dec 9, 2020

Hrmmmmmmmmmm. TRAMMING_POINT_XY { { 100, 100 }, { 200, 100 }, { 200, 200 }, { 100, 200 } } did succeed. So what is the limiting factor? If the first position is 1,1 it DOES move and probe that point, but fails to move to the next point.

@Pyro-Fox
Copy link
Contributor Author

Pyro-Fox commented Dec 9, 2020

Hrmmmmmmmmmm. TRAMMING_POINT_XY { { 100, 100 }, { 200, 100 }, { 200, 200 }, { 100, 200 } } did succeed. So what is the limiting factor? If the first position is 1,1 it DOES move and probe that point, but fails to move to the next point.

OK. It is NOZZLE_TO_PROBE_OFFSET after all. The TRAMMING_POINT_XY array wants nozzle positions, but it's going to try to touch the probe to those locations. Here are my working settings:

TRAMMING_POINT_XY { {  1, 1 }, { 275.58,  1 }, { 275.58, 250.7 }, { 1, 250.7 } }

Is this something that could be added to sanity check?

@FanDjango
Copy link
Contributor

@Pyro-Fox I know what I saw, DEBUG_LEVELING_FEATURE also needs the M111 S32 command to enable it, preferably right before you do the G35, not from the menu but from the serial terminal (Octoprint?)

@hamster65 I agree a sanitycheck would be a good idea. In order for the corner points to be in a symmetric pattern, the formula for recommended points is a nice task, also.

Instead of checking if probe points can be reached during runtime

probe_pt(...) and the motion subroutines behind that NEED to check reachability. Doing a sanity check on G35 config won't make that code go away.

@Pyro-Fox
Copy link
Contributor Author

Pyro-Fox commented Dec 9, 2020

@Pyro-Fox I know what I saw, DEBUG_LEVELING_FEATURE also needs the M111 S32 command to enable it, preferably right before you do the G35, not from the menu but from the serial terminal (Octoprint?)

I was unaware of needing M111 S32. I'll remember that for next time 😁.

@brianredbeard
Copy link

Out of curiosity, can anyone provide insight into why with LEVEL_BED_CORNERS relative inset value is used (LEVEL_CORNERS_INSET_LFRB) while with TRAMMING_POINT_XY it's absolute? cc @rmcbc

@sjasonsmith
Copy link
Contributor

I have a local implementation of a sanity check for G35 points. I need to do some cleanup on it but can post something soon.

This will catch initial configuration errors, but will not help if you store a different probe offset into the EEPROM.

@FanDjango, in your assessment, what do you think this needs to be able to close the issue. Is just the sanity check enough?

@FanDjango
Copy link
Contributor

FanDjango commented Dec 23, 2020

This issue is not a bug. The OP has succeeded in setting 4 points fo G35. So the issue itself can be closed.

BUT

A sanity check will help towards preventing further such issues, albeit not totally.

BECAUSE

All over the place, where probing points (X, Y) can be specified, the methodology is different to a degree or rather, it is not transparent and consistent to the user.

The repeated questions in support channels re. XMIN/YMIN, MARGINS and INSETS, and BED_SIZE_XY together with PROBE_OFFSET_XY for probe-reachability show that there is room for improvement.

Since the maximum probe-reachable area can be easily calculated, would not it be nice if Marlin could inform the user of the four outermost places on the bed that the probe can reach?

I lifted the following code from z_stepper_align.cpp to show the principle:

    constexpr xyz_pos_t dpo = NOZZLE_TO_PROBE_OFFSET;

    #define LTEST(N) (xy_init[N].x >= _MAX(X_MIN_BED + PROBING_MARGIN_LEFT,  X_MIN_POS + dpo.x) - 0.00001f)
    #define RTEST(N) (xy_init[N].x <= _MIN(X_MAX_BED - PROBING_MARGIN_RIGHT, X_MAX_POS + dpo.x) + 0.00001f)
    #define FTEST(N) (xy_init[N].y >= _MAX(Y_MIN_BED + PROBING_MARGIN_FRONT, Y_MIN_POS + dpo.y) - 0.00001f)
    #define BTEST(N) (xy_init[N].y <= _MIN(Y_MAX_BED - PROBING_MARGIN_BACK,  Y_MAX_POS + dpo.y) + 0.00001f)

    static_assert(LTEST(0) && RTEST(0), "The 1st Z_STEPPER_ALIGN_XY X is unreachable with the default probe X offset.");
    static_assert(FTEST(0) && BTEST(0), "The 1st Z_STEPPER_ALIGN_XY Y is unreachable with the default probe Y offset.");
    static_assert(LTEST(1) && RTEST(1), "The 2nd Z_STEPPER_ALIGN_XY X is unreachable with the default probe X offset.");
    static_assert(FTEST(1) && BTEST(1), "The 2nd Z_STEPPER_ALIGN_XY Y is unreachable with the default probe Y offset.");
    #if NUM_Z_STEPPER_DRIVERS >= 3
      static_assert(LTEST(2) && RTEST(2), "The 3rd Z_STEPPER_ALIGN_XY X is unreachable with the default probe X offset.");
      static_assert(FTEST(2) && BTEST(2), "The 3rd Z_STEPPER_ALIGN_XY Y is unreachable with the default probe Y offset.");
      #if NUM_Z_STEPPER_DRIVERS >= 4
        static_assert(LTEST(3) && RTEST(3), "The 4th Z_STEPPER_ALIGN_XY X is unreachable with the default probe X offset.");
        static_assert(FTEST(3) && BTEST(3), "The 4th Z_STEPPER_ALIGN_XY Y is unreachable with the default probe Y offset.");
      #endif
    #endif

Something very like this should be common code for all users of probe.cpp. Called once on init, sensible error messages etc. And a suggestion for the maximum symmetric probe-reachable area on the bed.

G29 (ABL) generates a mesh automagically using similar maths. It is of course historic that each routine cooks it own soup.

If it can be done in the preprocessor without generating code, so much the better.

But since the preprocessor is so limited, how about a small python "config assistant" as a pre extra script, that scans for the above X/Y limits, Offsets, Bed Size and so on and makes the calculations, informs the user and maybe terminates.

@FanDjango
Copy link
Contributor

The following output is produced by a quick and dirty python pre-check:

BED SIZE, BED POSITION and PROBING AREA
=======================================

NOZZLE_TO_PROBE_OFFSET_X = -41.7       
NOZZLE_TO_PROBE_OFFSET_Y = -10.2       
PROBING_MARGIN = 5
X_BED_SIZE = 350
Y_BED_SIZE = 350
X_MIN_POS = 0
Y_MIN_POS = 0
X_MAX_POS = 360
Y_MAX_POS = 360

Reachable probing points

Quadrant (Xmin,Ymin):
P = ( 5 , 5 )

Quadrant (Xmax,Ymin):
P = ( 318.299 , 5 )

Quadrant (Xmax,Ymax):
P = ( 318.299 , 345 )

Quadrant (Xmin,Ymax):
P = ( 5 , 345 )

Symmetric largest rectangle

Quadrant (Xmin,Ymin):
P = ( 31.701 , 31.701 )

Quadrant (Xmax,Ymin):
P = ( 318.299 , 31.701 )

Quadrant (Xmax,Ymax):
P = ( 318.299 , 318.299 )

Quadrant (Xmin,Ymax):
P = ( 31.701 , 318.299 )

and here is the code to produce that:

FNAME_CONFIG     = "Marlin/configuration.h"

GET_C = ["NOZZLE_TO_PROBE_OFFSET"] 

GET_N = ["PROBING_MARGIN", 
         "X_BED_SIZE", "Y_BED_SIZE", 
         "X_MIN_POS", "X_MAX_POS", "Y_MIN_POS", "Y_MAX_POS"]

try:
    with open(FNAME_CONFIG) as f:
        print("BED SIZE, BED POSITION and PROBING AREA")
        print("=======================================")
        print()
        for line in f:
            line = line.strip()
            if line.startswith("//"):
                continue
            words = line.split(" ")
            if words[0] != "#define":
                continue
            if words[1] in GET_C:
                s = words[1] + "_X = " + words[3].rstrip(",")
                print(s)
                exec(s)
                s = words[1] + "_Y = " + words[4].rstrip(",")
                print(s)
                exec(s)
                continue
            if words[1] in GET_N:
                s = words[1] + " = " + words[2]
                print(s)
                exec(s)
except IOError:
    print("Failed reading configuration.h")

print()
print("Reachable probing points")

Xrmin = round(max(X_MIN_POS + NOZZLE_TO_PROBE_OFFSET_X - 0.001, PROBING_MARGIN), 3)
Yrmin = round(max(Y_MIN_POS + NOZZLE_TO_PROBE_OFFSET_Y - 0.001, PROBING_MARGIN), 3)

Xrmax = round(min(X_MAX_POS + NOZZLE_TO_PROBE_OFFSET_X - 0.001, X_BED_SIZE - PROBING_MARGIN), 3)
Yrmax = round(min(Y_MAX_POS + NOZZLE_TO_PROBE_OFFSET_Y - 0.001, Y_BED_SIZE - PROBING_MARGIN), 3)

print()
print("Quadrant (Xmin,Ymin): ")
print("P = (", Xrmin, ",", Yrmin, ")")

print()
print("Quadrant (Xmax,Ymin): ")
print("P = (", Xrmax, ",", Yrmin, ")")

print()
print("Quadrant (Xmax,Ymax): ")
print("P = (", Xrmax, ",", Yrmax, ")")

print()
print("Quadrant (Xmin,Ymax): ")
print("P = (", Xrmin, ",", Yrmax, ")")

print()

dist = max(Xrmin, Yrmin, X_BED_SIZE - Xrmax, Y_BED_SIZE - Yrmax)
Xrmin = round(dist, 3)
Yrmin = round(dist, 3)

Xrmax = round(X_BED_SIZE - dist, 3)
Yrmax = round(Y_BED_SIZE - dist, 3)

print("Symmetric largest rectangle")

print()
print("Quadrant (Xmin,Ymin): ")
print("P = (", Xrmin, ",", Yrmin, ")")

print()
print("Quadrant (Xmax,Ymin): ")
print("P = (", Xrmax, ",", Yrmin, ")")

print()
print("Quadrant (Xmax,Ymax): ")
print("P = (", Xrmax, ",", Yrmax, ")")

print()
print("Quadrant (Xmin,Ymax): ")
print("P = (", Xrmin, ",", Yrmax, ")")

@sjasonsmith
Copy link
Contributor

My implementation that I will post soon transforms the same probe.h helpers used at runtime into constexpr functions that I can use in build-time static_asserts. That will ensure the same rules are used everywhere.

That provides some imperfect protection, since changing offsets using M851 would invalidate those checks.

We have for a long time needed better messaging when a probe fails. Maybe it is time to add a more descriptive message as well. Specifically when a point is not reachable.

I personally won’t be adding anything right now for auto point selection or a scripted helper. I’m not saying they’re bad ideas, it’s just further than I feel like going with it.

@FanDjango
Copy link
Contributor

@sjasonsmith full ack. Just wanted to add this to the long range to-do list.

@Pyro-Fox
Copy link
Contributor Author

You can probably tag this as a feature request then if that makes tracking easier. You are correct, I was able to resolve this myself once I understood the math.

@sjasonsmith sjasonsmith self-assigned this Dec 24, 2020
@sjasonsmith
Copy link
Contributor

I was hoping to post the build-time sanity checks today, but I realized my solution increased firmware size on AVR by about 600 bytes. I have reworked it to eliminate this increase, but the solution seems a little messier. I need to spend a little more time with it and will get it posted as soon as possible.

@sjasonsmith sjasonsmith linked a pull request Jan 1, 2021 that will close this issue
@github-actions
Copy link

This issue has had no activity in the last 30 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 7 days.

@samsp-msft
Copy link

I am trying to configure tramming for my Ender 3 v2 with a BL Touch. I have it working with the default settings, but the probe points are off from where the bed screws are located (approx 33mm in from the edges) The probe is offset by -43,-8 to the left/front, so even with a right probe margin of 0, it won't accept x probe positions over the bed screws.
Physically the x axis can travel beyond the print area, and the probe will reach a couple of mm further than the screw position. I can't set a negative Right Probe margin which I think would enable it to move beyond the print area. That seems to be a build macro check rather than a runtime limitation. Is it possible to enable negative probe margins to enable this scenario?

@thinkyhead
Copy link
Member

@samsp-msft

This Issue Queue is for Marlin bug reports and development-related issues, and we prefer not to handle user-support questions here. (As noted on this page.) For best results getting help with configuration and troubleshooting, please use the following resources:

After seeking help from the community, if the consensus points to a bug in Marlin, then you should post a bug report.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants