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.26k stars 19.23k forks source link

Z height errors on tool change (with ABL Bilinear) #10130

Closed thinkyhead closed 6 years ago

thinkyhead commented 6 years ago

From @FNeo31 (#10088)…

I put Marlin 1.1.8 on my 3D printer ( senhai3D - one acrilic Chinese version of prusa i3) And work well in single extruder. So I make some parts and reassemble all my printer like the attached image, and change the configuration for dual extruder and put my printer running again. I test the first extruder and everything was well, test the second and all was well to, so I tried to test the dual extrusion print to calibrate the relative position of my extruders and I check some situations:

1: When I print with the two extruders at same time the height of my printed object was wrong 8.20mm instead of 5mm 2: When I tried to change between extruders like :

G0 T0; 
G0 T1;
G0 T0; 
G0 T1;
G0 T0; 
G0 T1;
G0 T0; 
G0 T1;
G0 T0; 
G0 T1;

The extruders goes down at each change.

Can you help?

Photos ![img_20180119_002440](https://user-images.githubusercontent.com/37299777/37309734-b7518b18-2639-11e8-94da-dac54f21c788.jpg) ![img_20180112_014912](https://user-images.githubusercontent.com/37299777/37309776-d47812b6-2639-11e8-913e-aee16184b08b.jpg) ![img_20180112_010312](https://user-images.githubusercontent.com/37299777/37309817-fab4875c-2639-11e8-80f1-a692e9ed8202.jpg) ![img_20180312_193136](https://user-images.githubusercontent.com/37299777/37310005-8fa8975e-263a-11e8-80f4-1925287480cf.jpg) ![img_20180312_193049](https://user-images.githubusercontent.com/37299777/37310053-b1ab5aa8-263a-11e8-8e20-faf631f10698.jpg) ![img_20180312_193108](https://user-images.githubusercontent.com/37299777/37310100-ca76f448-263a-11e8-8ec8-32fc46d2a012.jpg) ![img_20180312_193117](https://user-images.githubusercontent.com/37299777/37310110-d8aacc56-263a-11e8-8330-bdf13ffd5a88.jpg) ![701953_1091637194201986_2643843535408775026_o](https://user-images.githubusercontent.com/37299777/37310250-3c2f8cda-263b-11e8-9a46-98b06615424e.jpg)
thinkyhead commented 6 years ago

Note that G0 T0 doesn't do anything. The correct command is T0 on a line by itself.

FNeo31 commented 6 years ago

Yes I check that when I made the log file. I work with a lot of CNC machines every day and my first instinct is put the G0 before the T0 or T1. But is this the cause of the trouble? Today I will try to test with other versions of Marlin

thinkyhead commented 6 years ago

The source of the trouble seems to be a genuine bug, because the Z-raise for one tool-change should be matched by Z-lower for the other tool change. While you're trying out other Marlin versions I'll make that test branch with extra logging.

If you're using Repetier Host, it will filter out some output (like position updates). There might be an option to show all output in the console. That should be enabled for the next log.

FNeo31 commented 6 years ago

@thinkyhead do you can say to me where are the conditions of tool changing to be possible to me understand what happens ao how are been calculated? if you can say it to me will save my time of search for the zone in the code. if I found a solution I post-it here to you check if its ok or not for Marlin to correct the trouble. And I can make a solution for my case and solve the problem more quickly than put and test all older versions.

thinkyhead commented 6 years ago

The easiest way to isolate behavior in tool_change is to use an editor that can collapse blocks, and collapse all the blocks that don't apply. image image

FNeo31 commented 6 years ago

@thinkyhead how can I force a move in XY without calculate Z from Bed Level? Just linear move like the machine made in G28?

All do_blocking_move_to pass on Bed level.

thinkyhead commented 6 years ago

It is handled in the planner, so all moves have leveling applied when leveling is enabled.

FNeo31 commented 6 years ago

So if I create one exception to make moves without compensation in the the planer could be a solution for tool changing right?

thinkyhead commented 6 years ago

@FNeo31 — When leveling is enabled we only need to properly compensate for the change in Z when suddenly at a new XY position. If you enable the M114_DETAIL option then you can use M114 D to see extra information about leveling compensation before and after tool change.

I'll make the extra logging branch shortly to see where the error lies, but you may be able to get some clue about the disparity from M114 D.

thinkyhead commented 6 years ago

So, this branch…

https://github.com/thinkyhead/Marlin/archive/bf1_tool_change_debug.zip

…adds logging of the leveling at the current position (with DEBUG_LEVELING_FEATURE). It will give some additional details about the internal state before and after the Z correction is attempted. Please do the usual test of tool changes with leveling enabled, and hopefully it will give us enough to go on.

FNeo31 commented 6 years ago

I test and made the same.

on position G0 X40 Y50 the machines goes down at each T0 to T1 and on position G0 X70 Y70 the machines goes up at each T1 to T0.

Exactly the same.

New Test Gcode with the two positions attached (tool change test.txt)

tool change test.txt

LOG file.txt

FNeo31 commented 6 years ago

I tried several times to put a condition in planer to don't apply the leveling on tool changing to solve the problem. Like put a flag like In_tool_change =1 on beginning and In_tool_change =0 in the end of tool changing in main cpp but for some reason (some error - I don't have to much experience in advanced code) make errors of "don't defined variables" on planner. can you try to make it to solve the problem or will not solve at all?

joris57 commented 6 years ago

FWIW, This or a very similar issue was reported already some time ago : #7469.

thinkyhead commented 6 years ago

Hi @joris57 — I've modified my test branch to apply a missing correction to Z according to the same method used for Mesh Bed Leveling. Please re-download that branch and give it a test. If it still gives the wrong Z correction then we should look at the updated debugging log:

FNeo31 commented 6 years ago

I test right now with the new branch and make the same but with double effect. Z move much more distance on tool change. Why don't make a condition to move the extruders without compensation? Just with the difference between extruders in XYZ.

thinkyhead commented 6 years ago

Why don't make a condition to move the extruders without compensation? Just with the difference between extruders in XYZ.

The only thing we need to do is make sure that the newly-selected nozzle's position is correctly established. Then subsequent moves will work as they should.

I could really use the logging output that I requested so that I can understand the issue. Without data I am just taking stabs in the dark.

bruce356 commented 6 years ago

removed

joris57 commented 6 years ago

@bruce356 I'm not sure why you think your issue is related to the topic being discussed here (Z height errors on tool change (with ABL Bilinear)). Butting in on conversations with not-related comments or issues is both impolite and counterproductive, so can you please start your own issue? Thanks.

And have a look at config.h in section //#define PARKING_EXTRUDER

#define HOTEND_OFFSET_Z { 0.0, 1.3 } // Z-offsets of the two hotends. The first must be 0.

alexxy commented 6 years ago

Seems like I also have issues with HOTEND_OFFSET_Z and UBL. Looks loke offset doesnt applyed

thinkyhead commented 6 years ago

@FNeo31 / @alexxy — Let's try a simplified approach, now posted at:

https://github.com/thinkyhead/Marlin/archive/bf1_tool_change_debug.zip

Instead of attempting to do correction of XYZ with in-place logic based on the type of leveling, it takes advantage of set_bed_leveling_enabled to simply disable leveling before doing any custom movements connected with the various types of extruder carriages. After all that, leveling is re-enabled, and finally the new nozzle moves to the position of the old nozzle. Assuming that set_bed_leveling_enabled is trustworthy, this should get all the positioning correct with respect to the physical position of the nozzle, and should properly handle Skew compensation, Z fade, etc.

With a dual nozzle it would also make sense to do a small raise before moving the new nozzle into place. But so far this hasn't been part of the tool_change logic.

joris57 commented 6 years ago

@thinkyhead I'm still in the process of seting up the test as you described above. Let me know if this is not needed anymore ...

FNeo31 commented 6 years ago

@thinkyhead I make some modifications in the main code and in my case it's working. I don't make all the tests but the first ones had good results. I'm not in home right now to put here the modifications but I put here tomorrow. The code can be optimized and have some flag variables to "jump" the planner on toolchange.

I will explain better later with the modifys

joris57 commented 6 years ago

@thinkyhead In the meantime, here's the log you requested anyway.

serial (2).log

bruce356 commented 6 years ago

@joris57, thanks for your reply, I was aware of that option and I had tested but unfortunately it is not compatible with Dual-X. Error on Compiling:- SanityCheck.h:535: error: #error "PARKING_EXTRUDER and DUAL_X_CARRIAGE are incompatible."

I am not experienced with github so to keep this topic on track I have removed post.

FNeo31 commented 6 years ago

@joris57, in the log like mine it's difficult to see what happened. Because on reality you don't have one straight plane to move and when your bed have a little curvature you have the same problem that I have. I will explain what I made and why I made like that. The principal way that I went was make the tool change without compensation to make the calculation easier. But it's possible to make with compensation but you need to go to deep in the code to make a preview of final location of T0 to make the good compensation for final T1 position when you change from T0 to T1. In my opinion it's easier to solve the problem make the toolchange moves without compensation and in the code make a clearance of 1mm (for example) and guarantee that I don't have colision between tool change

Roxy-3D commented 6 years ago

This can probably be optimized. But lifting the nozzle a small amount, moving the new nozzle to the original nozzle's (X,Y) position, and then lowering the new nozzle to the same height the original nozzle was at should work. That final move to the original Z-Height should result in the appropriate amount of Z Correction being added in.

Bob-the-Kuhn commented 6 years ago

@Roxy-3D - please look at Issue #10202. There's a section of code that needs to be updated to be used with UBL & I have no idea how to do it properly.

joris57 commented 6 years ago

@FNeo31 @Roxy-3D Sounds like the right approach to me, since the X/Y position of the nozzle doesnt't change, neither should the Z correction/position (with the possible exception of the HOTEND_OFFSET_Z).

I reported this issue several months ago for UBL and that seems to be solved in the meantime. How was it solved there?

FNeo31 commented 6 years ago

I don't know about UBL because on my first time with the UBL the nozzle crash a lot of times into bed every time that goes out of scanned zone (mesh zone) so I return to ABL Bilinear. But when I transformed the printer from 1 extruder to 2 extruders I check the error of tool change.

thinkyhead commented 6 years ago

Any better with https://github.com/thinkyhead/Marlin/archive/bf1_tool_change_debug.zip ? It should do all the right stuff, as it does disable leveling to simplify the leveling compensation portion of the tool change.

Anyway, yes, instead of updating the position to that of the new nozzle then doing the "move back," we could instead do a raw raise-move-lower instead. It just means adding another case to the tool-change code that handles it in isolation and retains the exact same current position.

thinkyhead commented 6 years ago

on my first time with the UBL the nozzle crash a lot of times into bed every time that goes out of scanned zone

  //#define UBL_Z_RAISE_WHEN_OFF_MESH 2.5 // When the nozzle is off the mesh, this value is used
                                          // as the Z-Height correction value.
thinkyhead commented 6 years ago

I've updated https://github.com/thinkyhead/Marlin/archive/bf1_tool_change_debug.zip once more, adding the Z lift before moving the nozzle into place. It retains the behavior of disabling leveling initially, then re-enabling it after establishing the new nozzle's position. Please test at your convenience.

Roxy-3D commented 6 years ago

It retains the behavior of disabling leveling initially, then re-enabling it after establishing the new nozzle's position.

Would it make sense to have it remember the bed leveling activation state when the routine is entered... And have it restore the state at exit? It would seem that would be the 'most correct' behavior.

thinkyhead commented 6 years ago

Would it make sense to have it remember the bed leveling activation state when the routine is entered... And have it restore the state at exit?

Yes, and the linked branch does just that! (But it restores the bed leveling state before the "move back.")

Roxy-3D commented 6 years ago

Yes, and the linked branch does just that! (But it restores the bed leveling state before the "move back.")

For UBL... That is the 'correct' thing to do. The reason is the Z-Height correction is calculated for the destination point of the move. And the 'current' location is assumed to be correct.

thinkyhead commented 6 years ago

My fingers are crossed it all works. It has the added benefit of making the tool change that much simpler.

FNeo31 commented 6 years ago

@thinkyhead, @Roxy-3D and @joris57

Like I said before, attach here what I did in the code, see the images I comment the code to best understand. I know that this way of code changing isn't the best and can make some conflicts in other configurations. I don't know to much of code so it's clear that the code can be optimized.

Like I said here before, I create some variables that I know it's possible to use the originals but I don't know what main code do with the original offset variables so I create new variables just to make sure that isn't make some more calculations.

My idea was basically:

turn off bed level compensation make the change in "off compensation mode" with the original offset's of active extruder says to next extruder that he is on same place of old one turn on bed leveling compensation

During the change I thought about make a safety tool change so I make a raise move just to put the tools in a clear zone for change.

in the images I put the clear zone 1mm upper from tool position can see in the video. the trouble is my oozle sheild comes useless so I change the clear plan to 0.05mm it's the maximum diference between extruders (all code attached).

Any doubt please ask.

@thinkyhead, sorry but I don't check your last revision. but if you want I can check if continues wrong or if it's ok now, do you want that I make the test?

img1 img2 img3 img4

VID_20180325201005.zip

Marlin-bugfix-1.1.x_dual extruder_tool_change modified.zip

Roxy-3D commented 6 years ago

do you want that I make the test?

Normally... If your code was available as a Pull Request, we would just review that and go with it if correct. If you can pull down Thinky's code and try it that would be very helpful. One of the big problems with these types of problems is finding a machine that exhibits the bad behaviour. If we can duplicate the problem on our machine's, it is usually not too hard to fix the problem. But if we can't do that... we need the users that can duplicate the problem to check out if the 'fix' really is a 'fix'.

FNeo31 commented 6 years ago

I'm not sure if today I can do that, but I will try to test @thinkyhead code tomorrow and I will send you the feedback.

thinkyhead commented 6 years ago

Sorry if I gave the impression that attaching images of code was a Good Thing™. I just wanted to show you the code folding, which cannot be done otherwise. If you want to present code for examination it's much better to just paste the code surrounded by ```cpp```.

I'll examine your changes and see how well they track with my proposed solution, which is both simpler and generalized. Obviously we can't use your solution for all possible cases.

Please test the linked branch at your earliest convenience. I will be posting it as a PR for others to test as well.

joris57 commented 6 years ago

I've tested https://github.com/thinkyhead/Marlin/archive/bf1_tool_change_debug.zip.

The discrepancy is still there with one difference: it now also occurs with UBL inactive (with UBL active the discrepancy is even bigger). M420 Z0 or moving beyond fade height does not seem to make a difference.

I won't be available to test the coming week, but anyone should be able to test on a Marlin with fwretract and UBL on with the gcode below; This code should just execute a number of retracts and recovers, but as it stands, you will see the z-axis go up :

M207 Z0.2
G29 I 999
G29 P3 C0.1 R999
G29 A
G28
G1 Z3.0 F300.0
G10
G11
G10
G11
G10
G11
G10
G11
G10
G11
G10
G11
G10
G11
G10
G11
FNeo31 commented 6 years ago

Hello,

I tried https://github.com/thinkyhead/Marlin/archive/bf1_tool_change_debug.zip branch and ( like mine loss the position if you make just T0 T1 T0 T1 T0 T1 T0 T1, but when we print a block test (like I post before) the final Z height its good now.

Just 3 things that I check and I have this working on the version that I change:

-Is it possible to put the Z offset between tools? (I have that on the code that I change and it's a good improve for my prints - I have a difference between nozzles of 0.05mm)

thinkyhead commented 6 years ago

The discrepancy is still there with one difference: it now also occurs with UBL inactive (with UBL active the discrepancy is even bigger). M420 Z0 or moving beyond fade height does not seem to make a difference. This code should just execute a number of retracts and recovers, but as it stands, you will see the z-axis go up.

Bizarre and unexpected. This has nothing to do with tool-change though, right? You're reporting to the wrong issue if the topic is FWRETRACT. You should be posting about that on #10200

thinkyhead commented 6 years ago

I tried https://github.com/thinkyhead/Marlin/archive/bf1_tool_change_debug.zip branch and ( like mine loss the position if you make just T0 T1 T0 T1 T0 T1 T0 T1, but when we print a block test (like I post before) the final Z height its good now.

Hmm, that is very interesting! It sounds like the Z position is getting corrected as it should, but it's not completely done in tool_change. It sounds like there needs to be an additional G0/G1-style move at the end to get the planner in sync.

Is it possible to put the Z offset between tools?

Dual nozzles attached to the same carriage are assumed to be identical in height, so we don't offer a Z offset. Any amount of Z offset in such a setup is going to be bad. The lower nozzle can't avoid running into the object.

Is it possible to change how much the printer retract in z axis?

I'm testing with 0.1 currently to avoid knocking over the object on the fast sideways move. Maybe later you can request it in a separate issue.

Let's concentrate on solving this bug first before we add more to the pile. I'd rather not chase three things at once while this bug is one of the last issues holding back the release of 1.1.9.

FNeo31 commented 6 years ago

With 0.1 I think that will be okay. And will work in my case. So in ABL Bilinear I think that is solved (with the error of forget the position if you change between T0 and T1 without moving) but work when print.

thinkyhead commented 6 years ago

change between T0 and T1 without moving

Hopefully this isolated issue will be easier to fix. Everything else is now merged.

github-actions[bot] commented 4 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.