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

[BUG] G34 not working properly #25560

Closed
1 task done
V1EngineeringInc opened this issue Mar 25, 2023 · 35 comments · Fixed by #25631
Closed
1 task done

[BUG] G34 not working properly #25560

V1EngineeringInc opened this issue Mar 25, 2023 · 35 comments · Fixed by #25631

Comments

@V1EngineeringInc
Copy link
Contributor

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

When running G34 on any bugfix branch after the 14th when the bed levels itself it then does not move down again to deploy the probe properly.

The leveling works as expected, bed moves out of the way, probe deploys, it probes properly, does all three points, and on the third point it adjusts but immediately probes again without moving down enough causing errors. I forced it to clear and each time after they adjust it does the same thing and does not move to deploy the probe fully.

The 13th of this month works, 18th and 23rd do not.

Bug Timeline

between 3/14 and 3/18

Expected behavior

It should move to deploy the probe after adjusting the leveling.

Actual behavior

It adjusts the tram, then does not move to deploy the probe and tries to take the next reading.

Steps to Reproduce

G28
G34

Version of Marlin Firmware

bugfix 2.1.x

Printer model

MP3DP

Electronics

SKR Pro

Add-ons

No response

Bed Leveling

ABL Bilinear mesh

Your Slicer

Prusa Slicer

Host Software

SD Card (headless)

Don't forget to include

  • A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

No response

@V1EngineeringInc
Copy link
Contributor Author

Marlin.zip

@classicrocker883
Copy link
Contributor

classicrocker883 commented Mar 26, 2023

check the differences of the code in the files, compare and contrast certain files like probe.cpp or probe.h.

there may be some values or parameters that are different in the versions that dont work.

@hexitex
Copy link

hexitex commented Mar 29, 2023

I can confirm this happens on z auto alignment as well, does three probes on left, makes a tiny lift then does normal lift, moves right, does three probes and after last one does a tiny lift but cannot deploy probe as too close to bed. This is new behaviour from three-ish weeks ago when it worked fine.

@V1EngineeringInc
Copy link
Contributor Author

Glad it has been confirmed on printers other than ones I designed and set up the firmware for.

@thisiskeithb
Copy link
Member

When running G34 on any bugfix branch after the 14th

Would you be able to test some bugfix-2.1.x commits to narrow this down?

The strategy is to find a commit from some point in the "distant" past where the feature works. Then, test a commit from halfway between that date and today…. And then you keep going to the commit halfway in between your "known to work" commit and your "known to be broken" commit until you find the exact commit where it broke.

If you started from a point 256 commits in the past, it would take no more than 8 tests to find the exact commit that broke it.

Most probe/tramming related commits have been within the past few weeks, so it shouldn't take long to find.

@V1EngineeringInc
Copy link
Contributor Author

To the best of my abilities, it was the PR's on 3/14, but I do not see any way that directly affects this. I did test a handful so it is between 3/14 and 3/18 (nothing works after the 18th I was working backwards). I was hoping someone who made those or knows more might spot the issue without me testing each days PR's, I can If I need to but I do not have time for a couple days.

@thinkyhead
Copy link
Member

I'm in the midst of trying to obtain consistent probe behavior across different probing procedures. I've added more debugging to get a better idea where and how to adjust things. Please test the latest code…

  • Download Marlin bugfix-2.1.x to test with the latest code.
  • Enable DEBUG_LEVELING_FEATURE and M114_DETAIL and re-flash the firmware.
  • Connect to your printer from host software such as Cura, Printrun or Repetier Host.
  • Issue the command M111 S247 to enable maximum logging.
  • Perform a G28 to do your standard homing procedure.
  • Do a G29 to probe the bed. This will also enable bed leveling.
  • Copy the log output into a .TXT file and attach it to your next reply.

Repeat this procedure, if needed, to demonstrate inconsistencies. From these logs I can get a better idea of where the raises are occurring and apply additional refinements.

@hexitex
Copy link

hexitex commented Mar 30, 2023

I think this change is the culprit https://github.com/MarlinFirmware/Marlin/pull/25498 . Enabling HS mode on the bltouch solved the issue - I don't do c code very well but did spot some relationship with hsmode in a confusing custom tern function :) around lines 910+ in probe

@classicrocker883
Copy link
Contributor

module/probe.cpp
line ~916

#if ENABLED(BLTOUCH)
// Reset a BLTouch in HS mode if already triggered
if (bltouch.high_speed_mode && bltouch.triggered()) bltouch._reset();
#endif
// Use a safe Z height for the XY move
const float safe_z = _MAX(current_position.z, SUM_TERN(BLTOUCH, Z_CLEARANCE_BETWEEN_PROBES, bltouch.z_extra_clearance()));

maybe this code can be looked at again.

line ~960

under if (!isnan(measured_z)) it appears asdo_z_clearance was changed from do_blocking_move_to_z .

i dont know the significance but there must be a reason why, im just saying maybe it should be looked over. just a hunch.

@jamespearson04
Copy link
Contributor

I think this change is the culprit https://github.com/MarlinFirmware/Marlin/pull/25498 . Enabling HS mode on the bltouch solved the issue - I don't do c code very well but did spot some relationship with hsmode in a confusing custom tern function :) around lines 910+ in probe

@hexitex must be right, all the other commits between the 14th and 18th don't affect the probing itself, just the points that UBL request be probed.

@V1EngineeringInc
Copy link
Contributor Author

Sorry I forgot to add the extra detail, here is my output.

g34log.txt

@V1EngineeringInc
Copy link
Contributor Author

I enabled HS mode and that did not help. It errored out in a whole different way, and tried to drive my Z through my extruder. It is still missing the clearing move.

G34 with HS.txt

@jamespearson04
Copy link
Contributor

jamespearson04 commented Apr 1, 2023

Have you tried changing Z_CLEARANCE_MULTI_PROBE and Z_AFTER_PROBING to around 5 or higher?
your Z_CLEARANCE_MULTI_PROBE is 1, which would be too low if your z nozzle to probe offset is over 1

ETA: I know that nozzle to probe should be automatically added, but with the recent changes it may have been affected, and it's easyish to test

@jamespearson04
Copy link
Contributor

Yeah, behaviour was changed in 49f1cc8:
do_z_clearance(current_position.z + (big_raise ? 25 : Z_CLEARANCE_BETWEEN_PROBES));
becomes:
do_z_clearance(Z_CLEARANCE_BETWEEN_PROBES);

I don't know if the intention was to do the addition inside the function instead, but that hasn't been done.

@hexitex
Copy link

hexitex commented Apr 1, 2023

Might be worth adding that PROBETOOL seems to have been introduced in the time window, which I think was defaulted to 0. Sorry don't have access to machine at the moment to run debugging, I will later

@dwzg
Copy link
Contributor

dwzg commented Apr 1, 2023

Since 49f1cc8, my Nozzle is crashing into the printbed during G34. Had no issues with G34 beforehand and reverting that commit fixes the problem for me. Don't know if this is connected to this issue though

@V1EngineeringInc
Copy link
Contributor Author

Have you tried changing

I tried changing all of them to 10, still doesn't work.

@jamespearson04
Copy link
Contributor

jamespearson04 commented Apr 1, 2023

I think replacing line 797 in Marlin/src/module/motion.cpp should fix it:

    const float  zdest = _MIN(zclear, Z_MAX_POS);

With:

    float zdest = zclear + current_position.z;
    zdest = _MIN(zdest, Z_MAX_POS);

Unfortunately I can't test that for myself as I don't have dual z axes

@V1EngineeringInc
Copy link
Contributor Author

Errored out,
Screenshot 2023-04-01 155638

So I tried, const float zdest = _MIN(zclear + current_position.z, Z_MAX_POS); That compiled but the first point failed.

@jamespearson04
Copy link
Contributor

jamespearson04 commented Apr 1, 2023

My bad try the above, but without const (above comment corrected in case anyone else tries it)

@V1EngineeringInc
Copy link
Contributor Author

Okay , sorry for the delay, I started with a fresh copy of the bugfix, that fix you posted 2 above worked!

Now it is moving down twice, same as the g29 issue.

@jamespearson04
Copy link
Contributor

Interesting, the fix above shouldn't add any extra moves, just ensure the ones that were already there actually move upwards. Does it do a double move for G29 for you, or just on G34? also is that for every probe it does, or only some?

I suspect the commit that broke G34 fixed the double lift on G29, so if it's not there on G29, the same changes will just need applying over to G34.

@jamespearson04
Copy link
Contributor

To add, if it only happens on G34 now, it's likely because G34 provides extra clearance to guard against gantry slope causing collisions, so in that case it's worth considering whether or not it's undesireable behaviour. A description/video of when it does the double lift would be helpful (is it before every probe? or just before switching sides?)

@jamespearson04
Copy link
Contributor

More info on this: The bug occurs because of this "dirty trick", that makes Marlin think the bed is further than it is:
https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.1.x/Marlin/src/gcode/calibrate/G34_M422.cpp#L168-L169

In most situations this will shift the z coordinates up by around 3-6mm, so without the above fix, when probe.probe_at_point() calls do_z_clearance(Z_CLEARANCE_BETWEEN_PROBES) or do_z_clearance(Z_CLEARANCE_MULTI_PROBE) after triggering, where Z_CLEARANCE_MULTI_PROBE = Z_CLEARANCE_MULTI_PROBE = 5, the resulting clearance lift can be as low as 0-2mm.

As the probe is likely to trigger at a height of about 2mm anyway, the shift will move this up to around 5mm. As a result, the amount of clearance provided is reduced by this amount.

@V1EngineeringInc
Copy link
Contributor Author

V1EngineeringInc commented Apr 2, 2023

Interesting, the fix above shouldn't add any extra moves, just ensure the ones that were already there actually move upwards. Does it do a double move for G29 for you, or just on G34? also is that for every probe it does, or only some?

It actually seems to do it on all G28, 29, 34. I end up sitting at z=26.4mm after G28 and/or G34. I am going to change all of my Z moves to separate numbers so I can try to figure out what is actually going on. It used to sit at Z=10, My probe offset is 3.2 so for some reason that gets doubled to get the 6.4 part and most of my other Z moves are set to 10mm so it could be any of them. I made a log it that helps I am not seeing the info to figure it out.

Basic g28 log,
4-1-g28.txt

I will be testing more in a couple hours.

@jamespearson04
Copy link
Contributor

Yeah, I've been going through and doing some testing of my own to get to the bottom of it, I know what the cause is, just trying to find the best solution.

@V1EngineeringInc
Copy link
Contributor Author

I just gave the BLTouch HS mode another try....much better. No added move, and it is super smooth.

@thisiskeithb
Copy link
Member

thisiskeithb commented Apr 4, 2023

Here's a copy of my comment from PR #25631:

I haven't tested a BLTouch-based config yet, but this PR solves the issue with the nozzle dragging across the bed after the final iteration of G34/Z_STEPPER_AUTO_ALIGN on a Biqu B1 SE Plus with dual+independent Z drivers & motors (stock only has a single Z motor/leadscrew) that has a strain gauge hotend (NOZZLE_AS_PROBE).

For reference, I'm using the following Z_CLEARANCE_* settings:

#define Z_CLEARANCE_DEPLOY_PROBE    2 // Z Clearance for Deploy/Stow
#define Z_CLEARANCE_BETWEEN_PROBES  2 // Z Clearance between probe points
#define Z_CLEARANCE_MULTI_PROBE     2 // Z Clearance between multiple probes
//#define Z_AFTER_PROBING           5 // Z position after probing is done

Full config: Biqu_B1_SE_Plus_Dual_Independent_Z.zip

@thisiskeithb
Copy link
Member

Please test #25631 and report your findings in that PR.

@V1EngineeringInc
Copy link
Contributor Author

Flawless!!!! I ran it with BLTOUCH HS mode and it was great.
G34 had the right offest to account for potential bed tilt.
G29 did the minimal, single, offset.

Back to perfect

@classicrocker883
Copy link
Contributor

changing #define Z_CLEARANCE_DEPLOY_PROBE from 10 to 5 fixed the similar issue using the Aquila.

no problem with 427 creality board, but it seems like the probe likes the value to be 5 or less.

@thisiskeithb
Copy link
Member

changing #define Z_CLEARANCE_DEPLOY_PROBE from 10 to 5 fixed the similar issue using the Aquila.

You should be able to use 10/any value you want.

Please report your finding in PR #25631

@V1EngineeringInc
Copy link
Contributor Author

Should I close this issue and just follow the PR?

@jamespearson04
Copy link
Contributor

I'd keep it open until the PR gets merged in, it's not technically resolved until then.

@github-actions
Copy link

github-actions bot commented Jun 5, 2023

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 5, 2023
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.

7 participants