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

[BUG] Incorrect Offset handling for Dual Extruders plus ZProbe #24056

Closed JoeScotty closed 2 years ago

JoeScotty commented 2 years ago

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

Yes, and the problem still exists.

Bug Description

For my self made cartesian Printer (dual extruder, BLTouch, Rumba32 board, future extruderlift) I'm going to switch to Marlin. Therefore I have a kind of rudimentary test system for checking all basic features. Regarding Offset handling there is probably a bug, if I didn't miss important settings:

Defined following positions for X Offsets (Y-Offsets are all set to 0): ZProbe: -10 E0: 0 E1: 20

Same behaviour, if X-offsets are 0 and Y-offsets have ththe above values.

Bug Timeline

first try was 2.0.9.3. Don't know about earlier versions

Expected behavior

When switching from E1 to ZProbe, the movement should be (Offset(E1)-Offset(ZProbe)

Actual behavior

The offset difference between E0 and E1 seems not to be calculated correctly, when switching from E1 to ZProbe

Steps to Reproduce

Parameters:

#define HOTEND_OFFSET_X { 0.0, 20.00 } // (mm) relative X-offset for each nozzle
#define HOTEND_OFFSET_Y { 0.0, 0.00 }  // (mm) relative Y-offset for each nozzle
#define HOTEND_OFFSET_Z { 0.0, 0.00 }  // (mm) relative Z-offset for each nozzle

#define NOZZLE_TO_PROBE_OFFSET { -10, 0, 0 }

#define X_BED_SIZE 200
#define Y_BED_SIZE 200

// Travel limits (mm) after homing, corresponding to endstop positions.
#define X_MIN_POS -20 
#define Y_MIN_POS 0
#define Z_MIN_POS 0
#define X_MAX_POS 190
#define Y_MAX_POS Y_BED_SIZE
#define Z_MAX_POS 200

M218 T1 X20.00 Y0.00 Z0.000 ; set hotend offset

Test sequence:

Boot up Activate E0 G28 ; Z_Probe in center of bed. G1 X0 Y100 ; E0 moves to physical position X=0 G30 X0 ; Move X+10, ZProbe now at physical pos X=0. Thats OK G1 X0 100 ; Move X-10, E0 now at physical pos X=0. Thats OK Activate E1 ; Move X-20, E1 now at physical pos X=0. Thats OK G30 X0 Y100 ; Move X+10, but I expected it to move x+30

For this Movement the offset difference between E0 and E1, 20mm, seems to be forgotten.

Version of Marlin Firmware

bugfix-2.0.x

Printer model

self made cartesian

Electronics

Rumba32

Add-ons

BLTouch

Bed Leveling

ABL Bilinear mesh

Your Slicer

Prusa Slicer

Host Software

Repetier Host

Additional information & file uploads

configuration files.zip

JoeScotty commented 2 years ago

Since there is no reaction after 3 weeks, is it recommend to add more (or less ;-) information? Just a hint (source code expressions) to search for would be appreciated. Thanks.

InsanityAutomation commented 2 years ago

Just not many of us with hardware to test. I'm packed solid for a bit but I may make it to this if someone else doesn't beat me to it.

JoeScotty commented 2 years ago

Did some research in the code. Being no software developer, so please forgive if being nonsense.

I think, hardware is not necessary to find the bug, but maybe for testing.

Stated that the offset problem was detected when performing a single probe using...

The offset move is the same in both cases despite there is a offset between T0 and T1. The offset move should be different depending on the active tool.

No I found in G30.cpp:

void GcodeSuite::G30() {

  const xy_pos_t pos = { parser.linearval('X', current_position.x + probe.offset_xy.x),
                         parser.linearval('Y', current_position.y + probe.offset_xy.y) };

  if (!probe.can_reach(pos)) return;

In my opinion the active tool and its offset is missing here, but I may be wrong ;-)

InsanityAutomation commented 2 years ago

The issue would be the code throughout Marlin is written as probe is always on T0, and being as there is no check of active tool... We need to decide if we want to abort, or switch to T0. In most cases, we switch to the required tool. Ive made that change in a branch here - https://github.com/InsanityAutomation/Marlin/tree/AddExplicitToolChangeToG30

Can you give this a try and see if it works as expected? Havnt had a moment to sit in front of a machine.

JoeScotty commented 2 years ago

Thanks for your help and mofifications. Got the branch. Unfortunately I now have no access to the printer for the next weeks.

Switching to tool 0, than activate probing, and switching back to old tool should solve the offset problem. In my case better than abortion.

Will test as soon as possible and report. Will then also test the offset handling when performing G29 bilinear in case Tool 1 ist active.

There are two criteria: probing at the correct positions. And avoiding unallowed positions. Both should be possible with switching to tool 0, then offsetting correctly to probe and at the end switching back to previous tool.

InsanityAutomation commented 2 years ago

Sounds good. If you confirm the resolution, ill get a PR open with it.

JoeScotty commented 2 years ago

Thanks for your patience, finally I had a chance to give it a try:

Compared to the Bugfix-2.0.x version dated April 18th, which I used when posting the issue, I just replaced the file G30.cpp, because I wanted to test with only less changes to avoid side effects.

The compiler showed an error " 'tool_change' not declared in this scope". Added

include "../../module/tool_change.h"

so I could compile and test. Hope this procedure is OK.

My test results: Basically it works:

My test sequence for Extruder 0:

 M851 X-10 Y0 Z0    ; Set Probe Offset
 M218 T1 X20 Y0 Z0  ; set hotend offset
 T0         ; Activate E0
 G28            ; Home all, probing at center of bed.
 G1 X25         ; E 0 moves to position X=25
 G30 X25        ; than 2 steps happened:
                1) Head moves 10 to X+, so Zprobe is at X=25,
                2) probing 
                info: no move back to T0 offset, so T0 is at X=35. that's OK
 G1 X25         ; E0 moves to X=25

Test part 2, Extruder 1 Version April 2022 with bug

E1              ; Head moves 20 to X-, so E1 is at X=25, OK
G30 X25         ; here it moved X +10, but I expected it to move x+30.
            For this move the offset between E0 and E1, 20mm, seems to be forgotten.

Test part 2, Extruder 1 Version May25 Because the test sample of my planned printer has an extruder lift, there are some more effects:

E1              ; Head moves 20 to X-, so E1 is at X=25. OK
G30 X25         ; than several steps happen:
                1) Extruderlift is moving to E0 Position
                2) Head moves 20mm to the right so E0 is at X=25
                3) Head moves 10mm to the right so Zprobe is at X=25
                4) Probing
                5) Extruderlift is moving to E1 position
                6) Head moves 20mm left, so E1 is at position X=35
G1 X25          ; E1 moves to X=25

So the bug is fixed using this method, but its still a bit bumpy because of those many moves. But: How often is this really going to happen? I think, G30 will not happen very often. The reason for the ticket was avoiding an issue which could have effect on more use cases than only G30. If there are no more bumpy cases than just G30, I am happy with the bugfix.

JoeScotty commented 2 years ago

Today I did some more tests with GCode commands, which could be affected by the selected Extruder and their offsets. First, the extruder lift movements described as bumpy (my mechanical concept allows zprobing with extruder lift in E0 as well as in E1 position) has the advantage of being more on the safe side to handle all situations correctly.

I also tested G28 with previously activated E0 or E1. In case of E1 it activates E0, process G28, then switch back to E1

The same result for bed leveling, G29. In case of E1 it activates E0, process G29, then switch back to E1.

So we have the same handling in all those cases now with your fix.

Looks pretty well now. Thanks for your help!

thisiskeithb commented 2 years ago

Closing since there's now a PR with a fix: https://github.com/MarlinFirmware/Marlin/pull/24511

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