search

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

[BUG] G35 ASSISTED TRAMMING #20937

Closed rmcbc closed 3 years ago

rmcbc commented 3 years ago

Guys, I develop the first version of this feature and to be more intuitive the coordinates of the probing point where the absolute position of the probe. It means that some one that is gathering points, moves the probe where it must be read and add that point to the TRAMMING_POINT_XY. Why changing this simple way to the one that uses the nozzle position? When we are using the assisted tramming we don't care where the nozzle is but only the visual location of the probe. Please think in the user experience before complicating a simple thing.

Example:

Doesn't make sense that I have to count with the probe offset and probe margin.

qwewer0 commented 3 years ago

Last time I checked (a week ago), TRAMMING_POINT_XY { { 35, 35 }, { 200, 35 }, { 200, 200 }, { 35, 200 } } probed exactly those absolute positions.

Could you attach your configuration files?

rmcbc commented 3 years ago

You can see that with branch bugfix-2.0.x and in 2.0.7.2 is ok. Correction: I can't be sure that it was working in 2.0.7.2, because in my first printer the difference is so small that I probably didn't notice. Only when I got the Ender 3 I found this problem.

qwewer0 commented 3 years ago

You can see that with branch bugfix-2.0.x and in 2.0.7.2 is ok.

Forgot to mention. I also did my test on the latest bugfix-2.0.x

I might be able to test it tomorrow, but I think the code was modified 22, 22, 29 days ago, so the results should be that same.

Could you attach your configuration files?

rmcbc commented 3 years ago

There they are:

AM8.zip [Ender 3.zip](https://github.com/MarlinFirmware/Marlin/files/5898907/Ender.3.zip

The validation (tramming.h):

#define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \ "TRAMMING_POINT_XY point " STRINGIFY(N) " is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN.") VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4); VALIDATE_TRAMMING_POINT(5);

rmcbc commented 3 years ago

This was the pull request that changed the way it should work:

Validate defined probe points (#20572) Submited by @sjasonsmith

sjasonsmith commented 3 years ago

I shouldn’t have changed the points it probes. I only added validation to make sure the probe can reach the specified points. Many people were confused because they were specifying unreachable points.

I would expect them to be probe points, not nozzle points. Perhaps the configured NOZZLE_TO_PROBE offset is incorrect.

rmcbc commented 3 years ago

@sjasonsmith How do you explain this?

I have to put this in bugfix-2.0.x configurations files:

NOZZLE_TO_PROBE_OFFSET { 31, 5, 0 } PROBING_MARGIN 0 TRAMMING_POINT_XY { { -22+31, 0+5 }, { 189+31, 0+5 }, { 189+31, 210+5 }, { -22+31, 210+5 } }

to get the probe in this position:

TRAMMING_POINT_XY { { -22, 0 }, { 189, 0 }, { 189, 210 }, { -22, 210 } }

Something has been changed since my initial implementation.

sjasonsmith commented 3 years ago

I didn’t intentionally change that behavior. If you can really show that my PR impacted it you are welcome to fix it.

My changes were not intended to change behavior, only to allow point validation at compile time.

I’m not really actively debugging and fixing things right now. I’ve been too busy with other things since the new year.

qwewer0 commented 3 years ago

Tested G35 on the latest bugfix-2.0.x 92b4c05, and I wasn't able to reproduce the issue.

@rmcbc If you can give a step by step instruction that you think will let us reproduce the issue, then please share it and I will test that way too.

rmcbc commented 3 years ago

@qwewer0 my previous configuration files included this fix:

NOZZLE_TO_PROBE_OFFSET { 31, 5, 0 } PROBING_MARGIN 0 TRAMMING_POINT_XY { { -12+31, 10+5 }, { 179+31, 10+5 }, { 179+31, 200+5 }, { -12+31, 200+5 } }

The ones here have my normal and correct configuration:

NOZZLE_TO_PROBE_OFFSET { 31, 5, 0 } PROBING_MARGIN 10 TRAMMING_POINT_XY { { -22, 0 }, { 189, 0 }, { 189, 210 }, { -22, 210 } }

untitled folder.zip

My printer can reach those positions: `// The size of the print bed

define X_BED_SIZE 220

define Y_BED_SIZE 220

// Travel limits (mm) after homing, corresponding to endstop positions.

define X_MIN_POS -32 //-20 //-28

define Y_MIN_POS -23 //-11

define Z_MIN_POS 0

define X_MAX_POS 240 //246

define Y_MAX_POS Y_BED_SIZE

define Z_MAX_POS 220`

qwewer0 commented 3 years ago

My settings:

#define TRAMMING_POINT_XY { { 32, 198 }, { 32,  32 }, { 202, 32 }, { 202, 198 } }

#define NOZZLE_TO_PROBE_OFFSET { -45.2, -4.5, -1.155 }

#define X_BED_SIZE 234
#define Y_BED_SIZE 230

#define X_MIN_POS -1.7
#define Y_MIN_POS -1.7
#define Z_MIN_POS 0
#define X_MAX_POS 270
#define Y_MAX_POS Y_BED_SIZE
#define Z_MAX_POS 250

Configuration.zip

rmcbc commented 3 years ago

@qwewer0 Compiling yours with fresh bugfix-2.0.x and in platformio file default_envs = STM32F103RC_btt_512K because you have a MOTHERBOARD BOARD_BTT_SKR_MINI_E3_V1_2

In my first clean compilation get this error:

Compiling .pio/build/STM32F103RC_btt_512K/src/src/gcode/bedlevel/ubl/G29.cpp.o In file included from Marlin/src/feature/../inc/MarlinConfig.h:31:0, from Marlin/src/feature/tramming.h:24, from Marlin/src/feature/tramming.cpp:27: Marlin/src/feature/tramming.cpp:32:22: error: 'TRAMMING_POINT_NAME_1' was not declared in this scope PGMSTR(point_name_1, TRAMMING_POINT_NAME_1); ^ Marlin/src/feature/../inc/../HAL/HAL.h:51:46: note: in definition of macro 'PGMSTR'

define PGMSTR(NAM,STR) const char NAM[] = STR

                                          ^~~

Marlin/src/feature/tramming.cpp:32:22: note: suggested alternative: 'TRAMMING_POINT_NAME_2' PGMSTR(point_name_1, TRAMMING_POINT_NAME_1); ^ Marlin/src/feature/../inc/../HAL/HAL.h:51:46: note: in definition of macro 'PGMSTR'

define PGMSTR(NAM,STR) const char NAM[] = STR

                                          ^~~

*** [.pio/build/STM32F103RC_btt_512K/src/src/feature/tramming.cpp.o] Error 1

Then recompile the upper error is fixed and get this error:

In file included from Marlin/src/feature/tramming.cpp:27: Marlin/src/feature/tramming.h:36:71: error: static assertion failed: TRAMMING_POINT_XY point 0 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN. 36 | #define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \ | ^ Marlin/src/feature/tramming.h:38:1: note: in expansion of macro 'VALIDATE_TRAMMING_POINT' 38 | VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4); VALIDATE_TRAMMING_POINT(5); | ^~~~~~~ Marlin/src/feature/tramming.h:36:71: error: static assertion failed: TRAMMING_POINT_XY point 1 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN. 36 | #define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \ | ^ Marlin/src/feature/tramming.h:38:29: note: in expansion of macro 'VALIDATE_TRAMMING_POINT' 38 | VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4); VALIDATE_TRAMMING_POINT(5); | ^~~~~~~ Marlin/src/feature/tramming.h:36:71: error: static assertion failed: TRAMMING_POINT_XY point 3 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN. 36 | #define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \ | ^ Marlin/src/feature/tramming.h:38:85: note: in expansion of macro 'VALIDATE_TRAMMING_POINT' 38 | VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4); VALIDATE_TRAMMING_POINT(5); | ^~~~~~~ In file included from Marlin/src/gcode/bedlevel/G35.cpp:43: Marlin/src/gcode/bedlevel/../../feature/tramming.h:36:71: error: static assertion failed: TRAMMING_POINT_XY point 0 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN. 36 | #define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \ | ^ Marlin/src/gcode/bedlevel/../../feature/tramming.h:38:1: note: in expansion of macro 'VALIDATE_TRAMMING_POINT' 38 | VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4); VALIDATE_TRAMMING_POINT(5); | ^~~~~~~ Marlin/src/gcode/bedlevel/../../feature/tramming.h:36:71: error: static assertion failed: TRAMMING_POINT_XY point 1 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN. 36 | #define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \ | ^ Marlin/src/gcode/bedlevel/../../feature/tramming.h:38:29: note: in expansion of macro 'VALIDATE_TRAMMING_POINT' 38 | VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4); VALIDATE_TRAMMING_POINT(5); | ^~~~~~~ Marlin/src/gcode/bedlevel/../../feature/tramming.h:36:71: error: static assertion failed: TRAMMING_POINT_XY point 3 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN. 36 | #define VALIDATE_TRAMMING_POINT(N) static_assert(N >= G35_PROBE_COUNT || Probe::build_time::can_reach(screws_tilt_adjust_pos[N]), \ | ^ Marlin/src/gcode/bedlevel/../../feature/tramming.h:38:85: note: in expansion of macro 'VALIDATE_TRAMMING_POINT' 38 | VALIDATE_TRAMMING_POINT(0); VALIDATE_TRAMMING_POINT(1); VALIDATE_TRAMMING_POINT(2); VALIDATE_TRAMMING_POINT(3); VALIDATE_TRAMMING_POINT(4); VALIDATE_TRAMMING_POINT(5); | ^~~~~~~ [.pio/build/LPC1769/src/src/feature/tramming.cpp.o] Error 1 [.pio/build/LPC1769/src/src/gcode/bedlevel/G35.cpp.o] Error 1

rmcbc commented 3 years ago

I can see two things that changed the correct way:

  1. VALIDATE_TRAMMING_POINT can't include the default NOZZLE_TO_PROBE offset and PROBING_MARGIN, only X_BED_SIZE, Y_BED_SIZE 230, X_MIN_POS, Y_MIN_POS, X_MAX_POS and Y_MAX_POS.

  2. In G35 code, the nozzle move to position must be absolute position without using the default NOZZLE_TO_PROBE offset and PROBING_MARGIN for calculations.

qwewer0 commented 3 years ago

@rmcbc Sorry, last minute I edited my configs carelessly, here is the fixed one: Configuration.zip

It compiles and G35 works as intended, probing each point then moving to the wait position.

rmcbc commented 3 years ago

In G35.cpp this line must be changed:

const float z_probed_height = probe.probe_at_point(screws_tilt_adjust_pos[i], PROBE_PT_RAISE, 0, true);

to

const float z_probed_height = probe.probe_at_point(screws_tilt_adjust_pos[i], PROBE_PT_RAISE, 0, false);

Because is not a relative position but absolute.

And I'm analyzing the VALIDATION to get it right.

qwewer0 commented 3 years ago

In G35.cpp this line must be changed:

const float z_probed_height = probe.probe_at_point(screws_tilt_adjust_pos[i], PROBE_PT_RAISE, 0, true);

to

const float z_probed_height = probe.probe_at_point(screws_tilt_adjust_pos[i], PROBE_PT_RAISE, 0, false);

Because is not a relative position but absolute.

And I'm analyzing the VALIDATION to get it right.

That line hasn't changes since https://github.com/MarlinFirmware/Marlin/commit/ac50a355a37673e521a052dc2d38d991144b4d05

rmcbc commented 3 years ago

In G35.cpp this line must be changed: const float z_probed_height = probe.probe_at_point(screws_tilt_adjust_pos[i], PROBE_PT_RAISE, 0, true); to const float z_probed_height = probe.probe_at_point(screws_tilt_adjust_pos[i], PROBE_PT_RAISE, 0, false); Because is not a relative position but absolute. And I'm analyzing the VALIDATION to get it right.

That line hasn't changes since ac50a35

Probably some other fix change the way that "probe.probe_at_point" works.

rmcbc commented 3 years ago

@qwewer0 if you get a fresh bugfix-2.0.x and put in TRAMMING_POINT_XY the X,Y coordinate for the position that the probe is on top of the screw. When you execute G35 the probe goes exactly to those positions or it adjust the position using the NOZZLE_TO_PROBE values? In both my printer it adjust the position using NOZZLE_TO_PROBE so the probe is not on top of the screws.

qwewer0 commented 3 years ago

@rmcbc Did you do M520, M500 after flashing the new firmware?

Probably some other fix change the way that "probe.probe_at_point" works.

I had a headache with the probe_relative=true, but as far as I know, it is as it suppose to be, and it mainly only do npos -= offset_xy;, so that the coordinates are absolute relative to the probe and not to the nozzle.

@qwewer0 if you get a fresh bugfix-2.0.x and put in TRAMMING_POINT_XY the X,Y coordinate for the position that the probe is on top of the screw. When you execute G35 the probe goes exactly to those positions or it adjust the position using the NOZZLE_TO_PROBE values? In both my printer it adjust the position using NOZZLE_TO_PROBE so the probe is not on top of the screws.

I retested it just now, and it probes on all four point as it is defied in the firmware, so on top of all the screws.

rmcbc commented 3 years ago

@qwewer0 yes I did.

Try to see with me this:

`// The size of the print bed

define X_BED_SIZE 220

define Y_BED_SIZE 220

// Travel limits (mm) after homing, corresponding to endstop positions.

define X_MIN_POS -32

define Y_MIN_POS -23

define Z_MIN_POS 0

define X_MAX_POS 240

define Y_MAX_POS Y_BED_SIZE

define Z_MAX_POS 220

define NOZZLE_TO_PROBE_OFFSET { 31, 5, 0 }

define PROBING_MARGIN 10`

My printer and the probe can reach those positions:

#define TRAMMING_POINT_XY { { -22, 0 }, { 189, 0 }, { 189, 210 }, { -22, 210 } }

And I can compile it because it says:

Marlin/src/gcode/bedlevel/../../feature/tramming.h:36:71: error: static assertion failed: TRAMMING_POINT_XY point 3 is not reachable with the default NOZZLE_TO_PROBE offset and PROBING_MARGIN.

I've to configure like this to get it to compile a probe at those positions:

`#define NOZZLE_TO_PROBE_OFFSET { 31, 5, 0 }

define PROBING_MARGIN 10

define TRAMMING_POINT_XY { { -22+10+31, 0+10+5 }, { 189-10+31, 0+10+5 }, { 189-10+31, 210-10+5 }, { -22+10+31, 210-10+5 } }`

qwewer0 commented 3 years ago

My printer and the probe can reach those positions:

define TRAMMING_POINT_XY { { -22, 0 }, { 189, 0 }, { 189, 210 }, { -22, 210 } }

Both probe and the nozzle can reach X=-22? In order for the probe to reach X=-22, the nozzle would need to be at X=-53, this doesn't seem correct considering X_MIN_POS -32.

Do want to probe these { { 19, 15 }, { 210, 15 }, { 210, 231}, { 19, 231 } } points?

rmcbc commented 3 years ago

My printer and the probe can reach those positions:

define TRAMMING_POINT_XY { { -22, 0 }, { 189, 0 }, { 189, 210 }, { -22, 210 } }

Both probe and the nozzle can reach X=-22? In order for the probe to reach X=-22, the nozzle would need to be at X=-53, this doesn't seem correct considering X_MIN_POS -32.

Do want to probe these { { 19, 15 }, { 210, 15 }, { 210, 231}, { 19, 231 } } points?

The nozzle X position -22 so the probe is at -22+31 = 9 and 9 is where I want to read not for the value 9 but because I see that the probe is on the screw. You have to put on TRAMMING_POINT_XY the coordinate that are shown in the display. Of course they are the nozzle position, but you get those ones when you put the probe on the place that you want to probe. When I create G35 I was explicit on this, you put the probe where you want to read the height and peak the coordinates that you see on the screen. This is the most user friendly way to register the coordinates in TRAMMING_POINT_XY.

qwewer0 commented 3 years ago

The nozzle X position -22 so the probe is at -22+31 = 9 and 9 is where I want to read not for the value 9 but because I see that the probe is on the screw.

Are these incorrect?

When I create G35 I was explicit on this, you put the probe where you want to read the height and peak the coordinates that you see on the screen. This is the most user friendly way to register the coordinates in TRAMMING_POINT_XY.

I don't remember the first G35 edition to work like that. The coordinates that we had to use was absolute in the bed area. So we could move the nozzle where we want to probe and that is the coordinate to use (or just use a ruler).

rmcbc commented 3 years ago

The nozzle X position -22 so the probe is at -22+31 = 9 and 9 is where I want to read not for the value 9 but because I see that the probe is on the screw.

Are these incorrect?

  • 0, 0 is at the front left of the bed
  • with NOZZLE_TO_PROBE_OFFSET { 31, 5, 0 } the probe is at the back right side of the nozzle.

When I create G35 I was explicit on this, you put the probe where you want to read the height and peak the coordinates that you see on the screen. This is the most user friendly way to register the coordinates in TRAMMING_POINT_XY.

I don't remember the first G35 edition to work like that. The coordinates that we had to use was absolute in the bed area. So we could move the nozzle where we want to probe and that is the coordinate to use (or just use a ruler).

The new way don't make sense because you may not able to probe the screw, because you are using the nozzle position on top of the screw and expecting that the probe can get there. Someone along the way mix all up, this is not a probing operation like G28/G29, this is to measure the the height for bed manual adjust. So it must not have any restriction imposed by NOZZLE_TO_PROBE offset and PROBING_MARGIN, it only needs to know if the head can move to that position. Why complicate a thing that was so easy to setup and now I can't probe on top of the screw because of NOZZLE_TO_PROBE offset and PROBING_MARGIN.

rmcbc commented 3 years ago

You can see they way it was implemented. https://github.com/MarlinFirmware/Marlin/pull/16897

qwewer0 commented 3 years ago

You can see they way it was implemented.

16897

I just compiled the 2.0.6 version of marlin, where your G35: Bed tramming assistant was added, and tested G35 with the same values as I have for the latest bugfix-2.0.x, and it behaves the same.

#define NOZZLE_TO_PROBE_OFFSET { -45.2, -4.5, -1.155 }

#define X_BED_SIZE 234
#define Y_BED_SIZE 230

#define X_MIN_POS -1.7
#define Y_MIN_POS -1.7
#define Z_MIN_POS 0
#define X_MAX_POS 270
#define Y_MAX_POS Y_BED_SIZE
#define Z_MAX_POS 255

#define TRAMMING_POINT_XY { { 32, 198 }, { 32,  32 }, { 202, 32 }, { 202, 198 } }
rmcbc commented 3 years ago

That was not my initial way of implementing this, I could have done something badly in my implementation or probably the system was changed after I submit the first PR. https://github.com/MarlinFirmware/Marlin/pull/16897/commits/194b9bf4fcff924268bfe6cb68e87af01121e1a3 But ok, I'm going to redo this implementation to get it the right way, then you all can decide the best way to use it, as is or my more user friendly way of doing it, without doing some math to get the point or use a rule.

sjasonsmith commented 3 years ago

I don’t think you should change it back. By eliminating math for you, you are adding math for someone that just uses a ruler to measure their points.

Anywhere Marlin configs specify distance it is measured from the 0,0 origin. It doesn’t matter whether the nozzle or probe will be used at the point.

sjasonsmith commented 3 years ago

@rmcbc your method seems intuitive to you because you thought of it. I don’t think other people would have ever know they should use that technique.

It would make sense as a wizard-guided method to find points. When the user is on their own to find points, I think they should always be as measured from the origin point.

rmcbc commented 3 years ago

I don’t think you should change it back. By eliminating math for you, you are adding math for someone that just uses a ruler to measure their points.

Anywhere Marlin configs specify distance it is measured from the 0,0 origin. It doesn’t matter whether the nozzle or probe will be used at the point.

Unfortunately It seems that we are not on the same page.

If for G29 you put PROBING_MARGIN with 30 this is very useful to measure points 30mm inside de bed, but you are also restricting people when using G35, because the first/last 30mm on each corner can't be used and the probe gets there. Remember that some printer have the screws 5~10mm from the border.

Do you see my concern? PROBING_MARGIN should not be a restriction when using G35.

Am I being misunderstood, explaining bad or I don't understand why you can accept that it's wrong the way it's currently implemented.

rmcbc commented 3 years ago

@rmcbc your method seems intuitive to you because you thought of it. I don’t think other people would have ever know they should use that technique.

It would make sense as a wizard-guided method to find points. When the user is on their own to find points, I think they should always be as measured from the origin point.

Do you see an easier way? Simply move the probe to left front corner in top of the screw pick X,Y from screen, move probe to right front corner in top of screw and pick X,Y from screen, and continue until the last. Is this only easy for me? I don't have to measure nothing nor use rules, I have only to copy the information from the screen.

qwewer0 commented 3 years ago

If for G29 you put PROBING_MARGIN with 30 this is very useful to measure points 30mm inside de bed, but you are also restricting people when using G35, because the first/last 30mm on each corner can't be used and the probe gets there. Remember that some printer have the screws 5~10mm from the border.

Do you see my concern? PROBING_MARGIN should not be a restriction when using G35.

Am I being misunderstood, explaining bad or I don't understand why you can accept that it's wrong the way it's currently implemented.

PROBING_MARGIN should always affect the probing ability as a safety net, and there should not be any exception. There are most likely reasons to the set values on setups, even if it is set so that the screws would be outside the probable area.

sjasonsmith commented 3 years ago

If your probe is able to probe outside of PROBING_MARGIN, than you are probably using PROBING_MARGIN incorrectly.

Do you see an easier way?

Your way is fine if you:

  1. Know that is the expectation. I see no indication that this would have ever been known to a user. The only instructions in header from the first commit in your PR was this. I removed this because it confused people. I don't even know what it means. use the hotend as reference not the sensor
  2. Have a screen or other simple way to manually control. Not all my printers do.

In G35.cpp this line must be changed: const float z_probed_height = probe.probe_at_point(screws_tilt_adjust_pos[i], PROBE_PT_RAISE, 0, true);

I looked at every commit in your PR, and it looks like it has always passed true to probe_at_point. Even following your own link it has true everywhere.

sjasonsmith commented 3 years ago

Based on this discussion I have to consider this to not be a bug.

My reasoning is:

  1. It has been this way since it was contributed.
  2. Documentation never clearly indicated the technique that was described in this report.
  3. It is already consistent with the way points are defined for other features using the probe. All points are relative to the X0Y0 origin point (normally the corner of the bed)
  4. Changing behavior now would break all existing G35 users.

I think there are ways the feature could be improved, but I don't think this change is it. To improve the issue of unreachable points, it might be best to make the G35 mechanism auto-select probe points if the provided points are not reachable. For example, if points are in a square and one edge is not usable, automatically shrink the square on both sides so the distance to the screw is equal on both sides.

@rmcbc I hope you do not take this personally. I appreciate the contribution that you have made, and I know a lot of people have found it useful. In this situation it just seems like the suggested change will cause unintentional harm to other users.

github-actions[bot] commented 3 years ago

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.