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
15.97k stars 19.09k forks source link

[bugfix-1.1.x] Dual X carriage kinematics broken with UBL #11412

Closed bruce356 closed 5 years ago

bruce356 commented 5 years ago

https://github.com/MarlinFirmware/Marlin/issues/11330 https://github.com/MarlinFirmware/Marlin/issues/10749 Posting this as I feel it was missed by @Roxy-3D & @thinkyhead Thanks

silentninja1 commented 5 years ago

In looking over things, I think I can see some of what can happen with ubl and a tool change. Now that a commit was added to make the planner aware of corrected position, it may no longer occur however for dual x carriage we can try mimicking the home command for disabling and restoring bed leveling.

#if HAS_LEVELING
    #if ENABLED(RESTORE_LEVELING_AFTER_G28)
      const bool leveling_was_active = planner.leveling_active;
    #endif
    set_bed_leveling_enabled(false);
#endif

If the bed has an even tilt, which kills duplicate mode btw so for an idex machine shouldnt be left there, i can see ubl adjusting following the park and recall commands causing #11330 however its likely not noticed often as duplicate mode requires fairly accurate bed leveling, using ubl as a visual aid for it. Now that the planner is aware of the position, can you see if this still occurs? If so, ill send you a patched branch with UBL disabled and restored around tool change. Im hesitant to slant the bed on my only idex machine enough to cause this, since ive still got delivery times to make.

bruce356 commented 5 years ago

@silentninja1 , Hi thanks for picking up on this issue. My beds do not have slope problems (beds are adjusted with dial indicator to minimise overall deviation) but the borosilicate glass beds are not perfectly flat and this varies with bed temperature. I am willing to test changes you make to help eliminate the problem with UBL but I know nothing about programming so I would rely on your guidance. Regards - bruce

InsanityAutomation commented 5 years ago

Same person here - phones on a different login. First off, can you verify if it still occurs on the current bugfix now that the planner is aware of ubl corrections with a commit from the last couple days? If it still occurs, I'll give you a patched file to try next.

bruce356 commented 5 years ago

I can do that I will have to download the latest bugfix v1.1.x and transfer my configs. I will let you know result after testing.

InsanityAutomation commented 5 years ago

Sounds good. What machine are you testing on?

bruce356 commented 5 years ago

Its basically a MTW Create but modified for Dual X Carriage and Rambo Board was changed out for a RUMBA board as it has 6 driver slots required for idex. 20180813_110228

bruce356 commented 5 years ago

I do not know but this latest bugfix v1.1.x seems really mucked up re UBL and idex. On tool change the Z axis drops 2mm, when I did a reprint the Z axis just went up until it hit the physical stop. On trying to do a bed level UBL G28 first that was ok but G29 P1 the probe moved to its first point but then the Z axis just oscillated up and down without the probe contacting the bed (BLtouch) and of course constantly showing an error on computer screen. This is all in the first 10 minutes not very thorough testing. In transferring configs a few changes have been made but only with drivers used in Configuration.h and two extra entries in configuration_adv.h

silentninja1 commented 5 years ago

Thats quite different results all around, that dont seem related to anything that im aware of changing.

Can you compare with whats here for 1.1.9 https://github.com/InsanityAutomation/Marlin/tree/TM_Trex and here for 2.0.x

https://github.com/InsanityAutomation/Marlin/tree/TM_Trex2+_2.0.x

Ive run both of these builds on a Formbot Trex2+ here.

Ill get a indicator strapped to my trex tomorrow and run back to back tool changes and see what I find. In the meantime, ill toss together a quick change to disable ubl during tool change and shoot it to you in the morning to test, after midnight here now!

bruce356 commented 5 years ago

v1.1.9 as per your link with my configs does not function at all, the GLCD just constantly squeals loudly, the display is blank, tried a G28 and there is no response from the printer and hot ends heated uncontrollably .

v2 comes up with compile error (Error compiling for board Arduino/Genuino Mega or Mega 2560) error file attached, I used my configuration with both of these versions of Marlin. Compile Errors Marlinv2.zip

InsanityAutomation commented 5 years ago

Fyi, I did confirm the height change with ubl on during tool change today. In the back and forth, a couple other things for idex that are in 1.1.9 we're not completely brought to 2.0 and the softstop for e1 isn't set correctly. I'll be finishing those up then chasing the ubl height change. The extra moves around parking seem to be gone since @thinkyhead added the synchronize command though.

bruce356 commented 5 years ago

@InsanityAutomation hi, so its not just me the problem does exist. Thanks for going to the trouble of rectifying this. I have never used Marlin Parking I have a script in S3D for that. I look forward to testing. regards - bruce

silentninja1 commented 5 years ago

Some of the disable, restore leveling is there on tool change but there is motion before and after it, im going to test moving it around synchronize commands to make sure its not a race condition tonight and report back. It does appear that recent changes have resolved #10749 issues 2 and 3, so only issue 1 remains there, which is a duplicate of #11330. Also, I just submitted #11556 to get the 2.0.x branch caught up with the fixes in 1.1.9 - more testing when I get home tonight!

silentninja1 commented 5 years ago

FYI, just a bit of reordering and im 3 prints in without this showing back up.... Ill watch it for another hour or so, then put in the change. @orcinus Can you verify that everything you were looking at is closed as well once I get this next pull request in?

silentninja1 commented 5 years ago

My 8mm test print came out between 8.06 and 8.11 every time, so im calling it good on my end. I did find another crash case where E1 rams into E2 during homing to chase down. Also it seems update softstops is not respecting the new variable for X1 max, so ill chase into that as well. Need to run a 40hr job on my Formbot Trex2+ (kinda need idex for the fine details with PLA/PVA) then ill be back towards the remaining items and anything you dig up testing on your end.

bruce356 commented 5 years ago

Hi, downloaded "Marlin-IDEX-UBL-Prevent-z-offset-creep" made no changes yet, needed to check if it compiled. Recieved error In file included from sketch\MarlinConfig.h:31:0,

             from sketch\G26_Mesh_Validation_Tool.cpp:27:

Configuration_adv.h:1052: error: missing binary operator before token "("

if HAS_DRIVER(TMC26X)

           ^

Configuration_adv.h:1280: error: missing binary operator before token "("

if HAS_DRIVER(L6470)

           ^

exit status 1 missing binary operator before token "("

regards - bruce Should have mentioned I use Arduino v1.8.5, is there another version that would successfully compile. I downloaded the latest version of today

Roxy-3D commented 5 years ago

@silentninja1

That is strange! It did pass all the Travis tests... And I just looked (again) at all the changes...

https://github.com/MarlinFirmware/Marlin/pull/11559/files

There is nothing in these changes that would cause that problem. Maybe the problem showed up on an earlier commit?

silentninja1 commented 5 years ago

The latest beta's of the arduino ide have been having issues parsing comments. Yet the only options for 2.0 are either the somewhat complicated platformio setup, or arduino ide betas. Ive moved to atom and platformio here because of it. Eclipse didnt seem to play nice with everything needed.

Roxy-3D commented 5 years ago

Thank You for the explanation!!!!!

bruce356 commented 5 years ago

Managed to get Marlin compiled with a Nightly version of Arduino. There is no difference to the perfomance as reported earlier. UBL really messes up the operation of my printer.

G28 homes, G29 P1 Zaxis oscillates up and down with BLTouch not contacting the bed, but if left long enough eventually the BLtouch contacts bed and T0 moves to the next sensing point but fails again soon after with an error and printer halts. With every oscillation the Z axis moves down a fraction and that is why the BLTouch eventually makes contact with the bed. After homing Z0 ends up with the hotend being about 8mm above the bed. With dual x printing at every tool change the Z axis drops by 2mm. In the videos trying to show oscillation and Z axis moving down by 2mm each tool change.

Configuration.zip Video.zip

orcinus commented 5 years ago

Sorry, haven't managed to test this yet, as i've been printing 24/7 for a client the past few weeks. But judging by what @bruce356 said, seems like the original issue is still present (drop on every tool change).

silentninja1 commented 5 years ago

Can you try a few config changes :

define MULTIPLE_PROBING 2

define Z_CLEARANCE_DEPLOY_PROBE 10

define Z_CLEARANCE_BETWEEN_PROBES 10 // Z Clearance between probe points

define Z_CLEARANCE_MULTI_PROBE 10

define Z_PROBE_LOW_POINT -4

The z probe low point is likely the source of alot of your trouble. With a z offset of -.91, that leaves you only 0.09 left defore itll stop, so unless youre z probe for home is at the low point, youre in trouble. BLTouch documentation recommends 10mm clearance. I know you can get closer, but lets stick to the doc's for testing.

silentninja1 commented 5 years ago

@bruce356 Also, maybe try setting

define UBL_Z_RAISE_WHEN_OFF_MESH 0

Then when you test it, please provide the output of G29 T on the terminal with youre results.

Thanks

bruce356 commented 5 years ago

Hi, downloaded the latest Bugfix version 2.0.x db30650 and the homing and bed level probing, Zaxis rising on XY movement issues have been rectified -thanks to dmitry-kutergin- but the UBL issue at tool change still remains when UBL is used. It seems more extenuated now where with every tool change the Z axis lowers by about 2.2mm, determined by looking at the Z lead screw coupling so only approxiamate. regards - bruce

silentninja1 commented 5 years ago

@bruce356 I expect youll find its moving by 2.5mm, which is the off mesh adjustment height. It appears to be getting reapplied which is why I added the late suggestion.

bruce356 commented 5 years ago

@silentninja1 ,hi just noticed your post, can you update your "IDEX-UBL-Prevent-z-offset-creep" to the latest version to overcome the above post problems, or have you already thanks - bruce

silentninja1 commented 5 years ago

@bruce356 Just looked at that commit. Even though it makes the code shorter, that highlights why I dont care for that style conditional. Short and inline, but obscures necessary bits like grouping ()'s quite a bit. Those kinds of things are tough to find!

Ill bump that shortly, check again in 5min

silentninja1 commented 5 years ago

@bruce356 The change from that is in the base branch now, so its there when you updated to the latest.

silentninja1 commented 5 years ago

@bruce356 Another potential config issue, see config adv #define BABYSTEP_ZPROBE_OFFSET - if you babystep away from the value set as the probe z offset, it will then be off.

bruce356 commented 5 years ago

@silentninja1 I have reconfigured as suggested,

  1. define UBL_Z_RAISE_WHEN_OFF_MESH 0, this definitely made a huge difference it got rid of the 2.5mm lowering with every tool change (T0 & T1). Still getting Z axis lowering on tool change but only every time T1 is activated. The distance the Z axis is lowered each time T1 is called is about 0.15mm

  2. define BABYSTEP_ZPROBE_OFFSET turning this off I did not notice any difference.

  3. Changing these numbers from my originals did not make any difference except for the length of time to probe the bed,

    define MULTIPLE_PROBING 2

    define Z_CLEARANCE_DEPLOY_PROBE 10

    define Z_CLEARANCE_BETWEEN_PROBES 10 // Z Clearance between probe points

    define Z_CLEARANCE_MULTI_PROBE 10

    define Z_PROBE_LOW_POINT -4

    Mesh Data 1.15pm.zip

InsanityAutomation commented 5 years ago
  1. If you run T0; T1; T0; T1; a bunch, can you measure the drop getting lower, or is it consistent that the gantry is .15mm lower for t1 and at 0 for t0?

  2. This might cause an issue if you babystep far from the programmed offset. It's a preventative.

  3. Didn't expect a difference aside from that here. Like I said when I suggested it, just adhering to documentation while debugging to be safe :) The multiple probing is a huge gain on accuracy btw, I recommend it for any form of able, especially since probing is a one time deal for ubl.

bruce356 commented 5 years ago

@InsanityAutomation hi, Every time T1 moves to the bed to print the Z axis lowers by lets say 0.2mm, this occurs every time the printing tool changes to T1 so it is cumulative.

After Homing I lowered Z axis with GLCD to 0.0mm and measured with a vernier (no lcd display) from X axis slide Rail to the bed, the dimension was 129.1mm

I then measured from rail to bed with every tool change, pausing the print to enable measuring.

T0 with GLCD readout of Z 0.25 distance was 129.1 T1 with GLCD readout of Z 0.25 distance was 128.9 T0 with GLCD readout of Z 0.25 distance was 128.9 T1 with GLCD readout of Z 0.25 distance was 128.7 T0 with GLCD readout of Z 0.25 distance was 128.7 T1 with GLCD readout of Z 0.25 distance was 128.45 T0 with GLCD readout of Z 0.25 distance was 128.45 T1 with GLCD readout of Z 0.25 distance was 128.3 T0 with GLCD readout of Z 0.25 distance was 128.3 T1 with GLCD readout of Z 0.25 distance was 128.1 T0 with GLCD readout of Z 0.25 distance was 128.1 T1 with GLCD readout of Z 0.25 distance was 127.9 T0 with GLCD readout of Z 0.25 distance was 127.9 T1 with GLCD readout of Z 0.25 distance was 127.75

Printing finished with T1. My measurements are not 100% due to none digital readout vernier but they are close. Hope the above gives you the infomation you need.

I have attached an image of the simulated printed object (Simplify3D) it is only 0.25mm high therefore one layer only but there are 6 skirts per tool. The print was simulated otherwise the tool would crash into the bed. test print single layer 0 25mm high

InsanityAutomation commented 5 years ago

Gives me a direction to chase! My machine is tied up until late tomorrow/ early Mon but I'll be getting a second idex machine next week so it'll be easier to get time on it.

bruce356 commented 5 years ago

@InsanityAutomation hi, just wondering if any progress has been made, your probably busy. thanks - bruce

InsanityAutomation commented 5 years ago

Not much progress outside of reading a few things. No indication where that specific value comes from yet. What's you're z offset value set to? Also, can you post output from a g29 T?

Roxy-3D commented 5 years ago

@bruce356 @InsanityAutomation I'm still in the calibration process. It will be a few more days... But my new development machine is an IDEX machine. And for sure, I'll be bringing up UBL on it ASAP.

Please keep this thread updated with any issues you see. And I'll do the same. (and tell you about any merges I do to fix problems.) I've added some LCD Menu's for IDEX modes. I'm going to be adding some sanity checks to the tool_change() function for IDEX because right now it assumes the new tool can always reach the position of the old tool, and that is not true. )

Oh! And I've changed G28 so the IDEX mode survives across a G28. That is important because if you are in Duplication mode, you want to be able to send the printer a normal .GCode file (with a G28) and have it do two prints instead of one.

InsanityAutomation commented 5 years ago

@Roxy-3D my batch of changes adding the no move override should have resolved this for idex. Thats what the x1 min and Max separate values we're added for. Unless one of those got reverted somewhere along the line. I'll double check both branches tomorrow.

Roxy-3D commented 5 years ago

Are those merged???? If not... I'll put those at the front of the list and expedite that!

InsanityAutomation commented 5 years ago

I believe they were. It was the batch of stuff that caused an issue for a un-named manufacturer and I redid a different way. Scott had gone over adding the default configs and merging after we initially proved it out. I should have time to go over it again tomorrow.

bruce356 commented 5 years ago

@InsanityAutomation @Roxy-3D , that is excellent news that Roxy-3D has an IDEX. Not sure which Z offset you meant (between T0 & T1 or Probe offset)

define HOTEND_OFFSET_X {0.0, 291.2} // (in mm) for each extruder, offset of the hotend on the X axis

define HOTEND_OFFSET_Y {0.0, -0.3} // (in mm) for each extruder, offset of the hotend on the Y axis

define HOTEND_OFFSET_Z {0.0, -0.00} // (in mm) for each extruder, offset of the hotend on the Z axis

define X_PROBE_OFFSET_FROM_EXTRUDER -1 // X offset: -left +right [of the nozzle]

define Y_PROBE_OFFSET_FROM_EXTRUDER -35 // Y offset: -front +behind [the nozzle]

define Z_PROBE_OFFSET_FROM_EXTRUDER -0.91 //was -0.95 and that was too close to bed

My Bed Topography Report G29 T 3rd Create Bed Topography Repory.zip Regards - bruce

Roxy-3D commented 5 years ago

That Topography Map looks pretty good!

bruce356 commented 5 years ago

@Roxy-3D hi, its a 1/8 inch Borosilicate glass bed, some sheets I purchased are very flat others are like 0.010" inch variation so UBL is essential. How are you doing with your new IDEX, have you found the same lowering of the Z axis when printing with both T0 & T1and UBL active? regards - bruce

Roxy-3D commented 5 years ago

I have not yet explored the lowering of the Z Axis when switching back and forth between the different tools. I'm doing other IDEX related work. But I will get to that as soon as I can. It is on my 'To Do' list. It just isn't at the top of the list yet.

InsanityAutomation commented 5 years ago

Pretty much the same story here. Day job is been fairly crazy so I haven't had a chance to sit down and correct some of those stuff with the last batch of commits that Scott had converted over for 2. 0 or get into this again. with the holiday weekend coming up here, hopefully I'll have a little bit of time to poke at it. we already know definitively that the off mesh move is being calculated even when leveling is disabled, so that needs fixing in the planner. The other one I'll have to do a little more digging to find where exactly that move is coming from.

Roxy-3D commented 5 years ago

@bruce356 You have two people wanting to help you! One of us will find the way to sneak it into our schedule!

Roxy-3D commented 5 years ago

@InsanityAutomation I have a quick question for you... If the IDEX Printer is in DXC_DUPLICATION_MODE, is it appropriate to allow it to do a tool change? I'm of the opinion a T1 command should return invalid_extruder_error(tmp_extruder); Maybe something like this in tool_change() :

#if ENABLED(DUAL_X_CARRIAGE)
  if (tmp_extruder != 0)
    if (dual_x_carriage_mode == DXC_DUPLICATION_MODE)
       return invalid_extruder_error(tmp_extruder); 
#endif
InsanityAutomation commented 5 years ago

Exactly! I even 3D printed some round tuits the other day but they disappeared amazingly fast LOL

InsanityAutomation commented 5 years ago

Roxy I would agree with that, it should not be allowed to do a tool change in duplication mode as it's not valid.

Roxy-3D commented 5 years ago

I'm also of the opinion this change should be made (in two places). From this:

      // Park old head: 1) raise 2) move to park position 3) lower
      for (uint8_t i = 0; i < 3; i++)
        planner.buffer_line(
          i == 0 ? current_position[X_AXIS] : xhome,
          current_position[Y_AXIS],
          i == 2 ? current_position[Z_AXIS] : raised_z,
          current_position[E_AXIS],
          planner.max_feedrate_mm_s[i == 1 ? X_AXIS : Z_AXIS],
          active_extruder
        );
      planner.synchronize();

to:

      // Park old head: 1) raise 2) move to park position 3) lower

      #define CUR_X current_position[X_AXIS]
      #define CUR_Y current_position[Y_AXIS]
      #define CUR_Z current_position[Z_AXIS]
      #define CUR_E current_position[E_AXIS]

      planner.buffer_line( CUR_X, CUR_Y, raised_z, CUR_E, planner.max_feedrate_mm_s[Z_AXIS], active_extruder);
      planner.buffer_line( xhome, CUR_Y, raised_z, CUR_E, planner.max_feedrate_mm_s[X_AXIS], active_extruder);
      planner.buffer_line( xhome, CUR_Y, CUR_Z,    CUR_E, planner.max_feedrate_mm_s[Z_AXIS], active_extruder);
      planner.synchronize();

Do you agree? Or would you prefer the current form?

InsanityAutomation commented 5 years ago

Funny you should say that, in one of the other threads on idexx issues I mentioned how annoying this style of iteration is with the inline conditionals and did exactly what you have here while I was debugging the issue from the LCD with jogging the wrong axis lol

Tldr I agree completely, that loop is annoying! I'll read it explicitly when I'm near a laptop again :)

Roxy-3D commented 5 years ago

OK... I'm going to start updating the IDEX code with the changes I have so far. I have one more question if I can bother you for an opinion:

I added code to the LCD Panel to have an IDEX Mode selection. That lets the user load a normal .GCode file (that only uses the left side of the bed) and do a Duplicate print. But in order to do that, I needed to change the G28 so it preserves the IDEX mode. This is because most Slicer's will put a G28 at the start of the print. By preserving the IDEX mode across the G28, it lets the user put the printer into DXC_DUPLICATION_MODE (using the LCD Panel) and then just send it a normal .GCode file.

Are you uncomfortable with that? If so... I'll add an option to let the user select the behavior when the code is built.